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

[k8scluster] fix nodegroup's name #1826

Merged
merged 1 commit into from
Sep 25, 2024
Merged

Conversation

sykim-etri
Copy link
Member

@sykim-etri sykim-etri commented Sep 20, 2024

PR은 노드그룹의 이름에 UID 대신 사용자 지정 이름을 사용하도록 수정합니다.
현재는 노드그룹을 별도의 객체로 관리하지 않고 있습니다.

@seokho-son
Copy link
Member

다른 자원들의 관리와 마찬가지로 일관성 차원에서 기존 uid 사용을 유지하면 좋을 것 같습니다.

특히, Spider에 요청이 들어갈 때 사용하는 Identifier에 해당하는 name 을, CB-TB 시스템 내부에서 universal 하게 관리하여, 복잡한 캐스캐이딩 되는 복잡한 ID들을 줄이기 위해 결정한 사항이었습니다.
현재 CB-TB는 CB-TB가 지정한 ID로 CSP 자원의 ID도 명확하게 지정할 수 있도록 Spider에 id transformation 을 off 하고, 명시적인 uid로 요청하고 있습니다.
그리고 id transformation off 상태에서는 리소스 id/name의 길이 제약이 csp 별로 달라서, name의 길이를 csp별로 체크해줘야 하는 부분도 있습니다. uid를 활용하면, 이를 고려하지 않아도 되는 상황입니다.

추가적으로, 향후에는 노드그룹도 CB-TB 오브젝트로 관리하고, 굳이 CSP에 조회 필요가 없는 정보는 바로 오브젝트의 정보로 바로 제공하면 좋을 것 같습니다. (단, 제가 정확히 상황을 파악하지 못해서 드리는 코멘트일 수도 있습니다.)

이 경우, 다른 TB 오브젝트들과 마찬가지로, name으로 요청을 받고, id를 생성하는 패턴을 추가할 수 있겠네요.

@sykim-etri
Copy link
Member Author

  • 노드그룹을 CB-TB 오브젝트로 관리하지 않고 Name을 기반으로 동작하도록 개발되어진 상황이기에 현 상황에서는 오동작을 야기하고 있습니다.
  • UID의 활용성에 대해서는 대략 이해가 됩니다만 CB-SP에서 노드그룹은 다른 자원과는 다르게 관리(ID Transformation 미지원)되고 있기에 좀 더 상세한 분석이 필요합니다.
  • 차후 CB-TB 오브젝트로의 관리 필요성 등을 확인하여 개선 작업을 진행할 때 UID도 함께 고려하면 될 것 같습니다.

@powerkimhub
Copy link
Member

@sykim-etri (@seokho-son )


  • UID의 활용성에 대해서는 대략 이해가 됩니다만 CB-SP에서 노드그룹은 다른 자원과는 다르게 관리(ID Transformation 미지원)되고 있기에 좀 더 상세한 분석이 필요합니다.


@seokho-son
Copy link
Member

@sykim-etri

  • 노드그룹을 CB-TB 오브젝트로 관리하지 않고 Name을 기반으로 동작하도록 개발되어진 상황이기에 현 상황에서는 오동작을 야기하고 있습니다.
  • 말씀하신 오동작은 (CB-TB 생성) uid를 지정하여 nodegroup 생성시, nodegroup 명칭 길이 제약에 의해서 csp가 오류 처리하여 발생하는 상황이 맞나요? (이 이슈라면, uid 길이 조정하는 방식으로 처리하는게 바람직해보입니다.)
  • UID의 활용성에 대해서는 대략 이해가 됩니다만 CB-SP에서 노드그룹은 다른 자원과는 다르게 관리(ID Transformation 미지원)되고 있기에 좀 더 상세한 분석이 필요합니다.
  • CB-TB에서는 CB-SP의 ID Transformation을 이미 환경변수로 OFF하고 있는 상황이라, CB-SP에서 해당 기능을 활성화 해주셔도 어짜피 활용하지 않는 형태가 될 것입니다.
  • 차후 CB-TB 오브젝트로의 관리 필요성 등을 확인하여 개선 작업을 진행할 때 UID도 함께 고려하면 될 것 같습니다.
  • 일단 현황상 임시라도 급히 기존(name전달)대로 하는 형태로 유지해야 하는 것으로 보신다면, 해당 PR을 반영해두도록 하겠습니다. 아니라면, 이번에 적절히 수정하는 것도 좋은 방법일 것 같네요.

@sykim-etri
Copy link
Member Author

@seokho-son

  • 말씀하신 오동작은 (CB-TB 생성) uid를 지정하여 nodegroup 생성시, nodegroup 명칭 길이 제약에 의해서 csp가 오류 처리하여 발생하는 상황이 맞나요? (이 이슈라면, uid 길이 조정하는 방식으로 처리하는게 바람직해보입니다.)

현재의 오동작 이슈는 길이 제약 이슈가 아니고(물론 길이 이슈는 내재하고 있습니다.), CB-TB에 사용자가 지정한 이름과 UID를 맵핑하는 정보가 없는 상황이라 사용자가 특정 노드그룹을 삭제할 수가 없는 등 노드그룹을 관리할 수 없는 상황입니다.
uid로 변경된 버전(v0.9.8)부터 관련 기능(RemoveNodeGroup, SetNodeGroupAutoScaling, ChangeNodeGroupAutoscaleSize 등)이 동작하지 않을 것으로 예상됩니다.

  • 일단 현황상 임시라도 급히 기존(name전달)대로 하는 형태로 유지해야 하는 것으로 보신다면, 해당 PR을 반영해두도록 하겠습니다. 아니라면, 이번에 적절히 수정하는 것도 좋은 방법일 것 같네요.

혹시 다음 릴리즈 예정일이 언제일까요? 이에 따라 결정하면 좋을 것 같습니다.

@powerkimhub
Copy link
Member

CB-TB에서는 CB-SP의 ID Transformation을 이미 환경변수로 OFF하고 있는 상황이라, CB-SP에서 해당 기능을 활성화 해주셔도 어짜피 활용하지 않는 형태가 될 것입니다.

  • 참고로, 이전 버전에서는 NodeGroup ID Transform을 OFF 시킬 수 없습니다.
  • OFF 시키려면 patch 및 patch 이후 버전 활용 필요합니다.

@seokho-son
Copy link
Member

@powerkimhub 공유 감사합니다. off 자체가 안되는 상황이었군요.

@seokho-son
Copy link
Member

seokho-son commented Sep 23, 2024

@sykim-etri

현재의 오동작 이슈는 길이 제약 이슈가 아니고(물론 길이 이슈는 내재하고 있습니다.), CB-TB에 사용자가 지정한 이름과 UID를 맵핑하는 정보가 없는 상황이라 사용자가 특정 노드그룹을 삭제할 수가 없는 등 노드그룹을 관리할 수 없는 상황입니다. uid로 변경된 버전(v0.9.8)부터 관련 기능(RemoveNodeGroup, SetNodeGroupAutoScaling, ChangeNodeGroupAutoscaleSize 등)이 동작하지 않을 것으로 예상됩니다.

  • 그렇군요. 확인 감사합니다. 일단 기능에 오류가 발생하는 상황이니, 테스트라도 해볼 수 있도록 본 PR을 수용해두는 것이 좋을 것 같네요.
  • 일단 현황상 임시라도 급히 기존(name전달)대로 하는 형태로 유지해야 하는 것으로 보신다면, 해당 PR을 반영해두도록 하겠습니다. 아니라면, 이번에 적절히 수정하는 것도 좋은 방법일 것 같네요.

혹시 다음 릴리즈 예정일이 언제일까요? 이에 따라 결정하면 좋을 것 같습니다.

  • 주요 차기 릴리스는 CB-SP 신규 프릴릴리스 후, 연동 테스트를 완료하고 진행하려고 생각했었습니다. (대략 이번주 후반이나, 차주 초) 그러나 프리릴리스를 찍는 것은 언제든 수행할 수는 있는 상황입니다.

@seokho-son
Copy link
Member

@sykim-etri PR 승인, 현시점에 필요하신지 문의드립니다. :)

@sykim-etri
Copy link
Member Author

@seokho-son
예상되는 일부 기능의 오류 내재를 감내한다면.. 차차기 릴리즈 정도에 uid 반영을 위한 신규 PR을 제안할 수 있을 것으로 예상되며, 이 경우는 현 PR 승인을 불필요해 보입니다.

@seokho-son
Copy link
Member

승인 후, bug fix 차원에서 추가 릴리스 하겠습니다. :)

@seokho-son
Copy link
Member

/approve

@github-actions github-actions bot added the approved This PR is approved and will be merged soon. label Sep 25, 2024
@seokho-son seokho-son merged commit 0b76346 into cloud-barista:main Sep 25, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved This PR is approved and will be merged soon.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants