-
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
set 'replace' sync-option for crd-only app #69
Conversation
묵시적으로 -crds 앱인 경우 replace 옵션을 사용하기 보다 파라미터를 직접 받는 것은 어떨까요? (아직은 없지만) 특정 앱이 다른 sync 옵션을 사용해야 하는 경우에도 활용할 수 있고, 호출하는 입장에서도 나중에 잘못 이해하는 경우가 없을 듯 합니다. https://argo-cd.readthedocs.io/en/stable/user-guide/sync-options/ 의 모든 옵션을 제공하도록 제일 좋겠지만 일단 Replace만 지정할 수 있도록 해도 괜찮을 듯 합니다. enum 타입 파라미터 (https://github.com/argoproj/argo-workflows/blob/master/test/e2e/testdata/workflow-template-with-enum-values.yaml)로 하면 이후에 확장도 수월하게 될 것으로 생각합니다. |
말씀하신 방향이 더 바람직하긴 한데, parameter가 많아져서 살짝 고민이 되네요. 현재 아래와 같이 4개를 사용 중인데, 하나 더 늘어나도 크게 개의치 않으시는 거죠?
|
네, 파라미터 기본 값을 sync 기본 모드인 apply 로 하면 createapp 호출하는 다른 워크플로우 수정도 필요없고 괜찮을 것으로 생각합니다. |
네, 그렇게 구현 가능하면 best인데, 그러러면 아래 링크에 언급된 이슈를 해결해야 합니다. parameter를 입력하는 경우와 그냥 생략하는 경우를 모두 cover할 수 있도록 create-app 호출하는 로직의 수정이 필요할 것 같습니다. (이거 때문에 굳이 target_cluster를 전부 ""로 지정했었습니다) |
쉽게 풀기 어려운 문제네요. installApps 때문에 호출하는 쪽 입장에서 편리한 점이 있긴 한데 반대로 파라미터에 대한 제어나 기본 값 설정이 불가능하게 되었고, 차선으로 installApps 단계에서 sync 옵션(enum, 기본 값은 apply)을 받도록 하는 건 어떨까요? 내일 싱크 시간에 논의했으면 좋겠습니다. |
네, 제가 내일 딱 그 시간에 부스터샷 접종이라 참석은 힘들 것 같은데, 다른 분들과 논의해서 좋은 방법 고안해주시면 좋겠습니다. |
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. |
https://github.com/openinfradev/tks-issues/issues/137
(주의) openinfradev/decapod-base-yaml#147 과 동시에 merge되어야 합니다.
참고로, servicemesh workflow 는 #70 이 최종 merge되어야 본 PR 로직대로 수정 가능할 것 같습니다