-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactoring Service-Mesh workflow for tks and decapod #70
Refactoring Service-Mesh workflow for tks and decapod #70
Conversation
This PR is stale because it has been open 3 days with no activity. Remove stale label or comment or this will be closed in 3 days. |
This PR was closed because it has been stalled for 10 days with no activity. |
This PR was closed because it has been stalled for 10 days with no activity. |
This PR is stale because it has been open 3 days with no activity. Remove stale label or comment or this will be closed in 3 days. |
1ce8e1e
to
107baba
Compare
#========================================================= | ||
# Template Definition | ||
#========================================================= | ||
- name: delete-argocd-app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이미 argocd app 삭제하는 로직이 있으니 여기서 구현할 필요 없이 그걸 쓰시면 어떨까요? (LMA도 그렇게 되어있습니다)
https://github.com/openinfradev/decapod-flow/blob/main/templates/argo-cd/delete-apps-by-label-wftpl.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LMA 것을 활용했습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코멘트 남긴 부분들 확인 부탁드립니다.
5e31d4c
to
29c5ec5
Compare
29c5ec5
to
7c8f458
Compare
7c8f458
to
1ca19ff
Compare
현재까지의 코멘트 전부 반영했습니다. |
name: delete-apps | ||
template: DeleteAppsByLabel | ||
|
||
# - - name: delete-argocd-app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
주석으로 남겨진 코드들이 많이 있는데 이유가 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
나중에 활용할 수 있어서 남겨두었습니다. 먼저 커밋이 되면 이후 커밋에는 코멘트도 제거할 예정입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
커밋을 2개로 올리는 방법이 있겠군요. ㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코멘트 삭제했습니다.
retryStrategy: | ||
limit: 2 | ||
|
||
- name: delete-finalizer-app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
계속 force-push되다보니 히스토리가 좀 헷갈린데, 암튼 지금 시점 코드 보면 좀 의아한데요? delete-finalize-app 등 정의만 되어있고 사용되는 곳이 없지 않나요? (혹시 코드 꼬인거 아닌지..)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사용하지는 않지만 definition 으로 나둔 부분입니다. 그냥 utility 처럼 함수만 정의한 부분이라고 보시면 됩니다. "# Template Pipeline" 부분이 실제로 돌아가는 부분입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(히스토리 기록 차원에서) delete-apps-by-labels 코드 활용하면서 finalizer 삭제 등 수작업 전부 필요없게 되어서 definition만 남겨두고 실제로 호출은 하지 않음. PR 머지하겠습니다!
Service-Mesh 설치를 공통 createApp template 을 사용하게 하여 tks 재사용 가능하게 refactoring 했음.