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

Feature/create user cluster #15

Merged
merged 15 commits into from
Nov 2, 2021
Merged

Feature/create user cluster #15

merged 15 commits into from
Nov 2, 2021

Conversation

ktkfree
Copy link
Contributor

@ktkfree ktkfree commented Oct 14, 2021

issue : issue-11 issue-12 issue-14

tks-cluster-lcm 을 사용하여 user cluster 를 생성할 수 있다.
동기 방식으로 일단 구현하였으며, 추후 삭제를 위해 [DELETE BELOW] tag 를 추가하였습니다.

. http 방식의 argowf client 를 grpc 방식으로 변경
. 임시로 동기방식으로 처리하도록 변경 ( stream connection 을 맺어 workflow 가 종료될때까지 기다립니다. )

Copy link
Contributor

@Jaesang Jaesang left a comment

Choose a reason for hiding this comment

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

이 PR도 openinfradev/tks-contract#16 처럼 Demo에서 이미 검증된 부분인가요?

@ktkfree
Copy link
Contributor Author

ktkfree commented Oct 15, 2021

이 PR도 openinfradev/tks-contract#16 처럼 Demo에서 이미 검증된 부분인가요?

contract, lcm 모두 동기 방식으로 변경한 부분은 아직 검증되지 않았어요. 데모는 비동기 rest 방식으로...
클러스터 상태관리에 대한 부분은 여전히 고민스러운 부분으로 차주 워크샵에서 자세히 얘기해보면 좋겠어요.

@zugwan
Copy link
Contributor

zugwan commented Oct 15, 2021

tks-contract, tks-proto 모듈 업데이트가 우선 되어야 할 듯 합니다.

@ktkfree ktkfree requested a review from Jaesang October 25, 2021 02:13
@Jaesang
Copy link
Contributor

Jaesang commented Oct 27, 2021

@ktkfree sync방식에서 async로 다시 변경하기로 한거 맞나요?

…, setup-sealed-secrets-on-usercluster, aws-infrastructure )
@ktkfree
Copy link
Contributor Author

ktkfree commented Nov 1, 2021

@ktkfree sync방식에서 async로 다시 변경하기로 한거 맞나요?
네, 현재 올라가있는 버전은 async 방식입니다. 데모시 연속된 workflow 호출을 위해 sync 방식으로 구현하였으나, 클러스터 생성과 sealed secret 워크플로가 분리됨에 따라 더이상 sync 방식일 필요가 없습니다.
주관님께서 진행하신 "워크플로에서 클러스터 상태변경 처리"하는 기능과 더불어 async 방식으로 처리하면 좋을 것 같습니다.

ClusterId: appGroup.GetClusterId(),
AppGroup: appGroup,
})
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

동일한 클러스터에 동일한 종류의 AppGroup 설치가 중복으로 요청되면 (현재 기준으로 LMA나 SERVICE_MESH) appGroupId 생성이 안되면서 오류 처리 되는 지 궁금합니다. cluster_lcm 내 핸들로 쪽에서는 이런 내용을 확인 못해서 (혹은 제가 놓쳐서) 문의 드립니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

동일한 클러스터에 동일한 종류의 AppGroup 설치가 중복으로 요청되면 (현재 기준으로 LMA나 SERVICE_MESH) appGroupId 생성이 안되면서 오류 처리 되는 지 궁금합니다. cluster_lcm 내 핸들로 쪽에서는 이런 내용을 확인 못해서 (혹은 제가 놓쳐서) 문의 드립니다.

현재는 external_label 이 동일한지만 체크하고 있습니다.
만약 동일한 request 를 연속으로 보내면 실패처리 되겠지만, external_label 을 다르게 요청한다면 다른 AppGroup 이라고 인식하여 설치를 진행하게 되겠네요.

금번 스프린트에서는 validation이 강력하면 테스트가 어려운 부분이 있어 최소한도록 동작가능한 것에 맞추어 개발하였습니다.
다음 스프린트에서 DB data 기반으로 validation 처리를 좀 더 강력하게 하는 작업이 필요합니다.

@zugwan zugwan self-requested a review November 2, 2021 03:59
@zugwan zugwan merged commit 00b4e15 into main Nov 2, 2021
@zugwan zugwan deleted the feature/create_user_cluster branch November 2, 2021 04:01
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