Skip to content
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

Resolve lma reinstallation failure #27

Merged
merged 3 commits into from
Feb 16, 2022
Merged

Conversation

ktkfree
Copy link
Contributor

@ktkfree ktkfree commented Feb 15, 2022

LMA 설치시 중복하여 요청할 경우 실패하던 현상을 변경합니다.

AS-IS : 동일한 cluster 에 동일한 externalLabel 로 서비스 설치를 요청할 경우, externalLabel 중복으로 인해 AppGroup 생성 오류가 발생합니다.

TO-BE : 동일한 clueter 에 동일한 externalLabel 이 존재할 경우 해당 AppGroupId 를 사용하며, 존재하지 않을 경우 AppGroup 을 생성합니다.

추가 변경 사항
. error response 의 경우 err 를 return 하도록 변경
. default logging-component 를 "efk" 에서 "loki" 로 변경

taekyu.kang added 3 commits February 15, 2022 15:19
 . apply gofmt result
 . change default logging component ( efk -> loki )
Copy link
Contributor

@robertchoi80 robertchoi80 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아래쪽 test 코드는 돌려보셨을 거라 믿고, 윗부분 주요 로직들 리뷰 후 승인합니다.
다만 시간 여유 되시면 rough하게라도 주석을 좀 추가해주면 가독성이 좋아질 것 같습니다.

@zugwan zugwan merged commit a979cbe into main Feb 16, 2022
@zugwan zugwan deleted the resolve_lma_reinstallation_failure branch February 16, 2022 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants