-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Add network router CRUD tool #79
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
Conversation
| provider_network_type: str | None = None, | ||
| provider_physical_network: str | None = None, | ||
| provider_segmentation_id: int | None = None, | ||
| project_id: str | None = None, |
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.
project_id를 추가하신 이유가 있나요?
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.
네크워크는 프로젝트별로 생성할 수 있습니다~ 이번에 추가된 라우터는 다른 서브넷간 통신을 하기 위한 라우팅과 더불어 다른 프로젝트에 있는 네트워크 (서브넷 포함) 과 통신할때도 필요합니다. 저 파라미터를 받지 않으면 cloud.yaml에 지정된 계정의 프로젝트로만 네트워크가 생성되요. 예를들어 admin으로 로그인하면 admin 프로젝트에만 네트워크를 만들수 있어요.
| ) -> Router: | ||
| """ | ||
| Create a new Router. | ||
| Typical use-cases: |
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.
Usecase를 따로 작성하신 이유가 있으신가요?
몇몇 시나리오에서는 :param에 작성하셔도 될 것 같은데, 이유가 있을 것 같아 질문합니다.
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.
요청 바디의 중첩구조에 대해 LLM이 혼동하는 경우가 있어 추가하게 됐습니다.
param은 해당 인자에 대한 설명에 충실하는게 좋지 않을까요?
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.
그렇군요!
저도 앞으로 조건사항이 있으면 use case로 따로 작성해야겠네요.
| Typical use-cases: | ||
| - Create basic router: name="r1" (defaults to admin_state_up=True) | ||
| - Create distributed router: is_distributed=True | ||
| - Create with external gateway for north-south traffic: |
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.
north-south traffic이 무엇을 의미하는지 궁금합니다.
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.
네트워크에서 상하 계층간 통신방향을 구분하기 위해 사용되는 용어 입니다.
상위로 전달하는것을 northbound, 하위로 전달하는 것을 southbound 라고 합니다.
즉 이 경우 외부 게이트웨이(external gateway info)로 트래픽을 통신하기 위한 설정 use case 입니다.
(추가) 주로 SDN 분야에서 많이 쓰는 용어입니다!
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.
좋은 정보 알아갑니다!
| - Set distributed flag: is_distributed=True or False. | ||
| - Set external gateway: external_gateway_info={"network_id": "ext-net", "enable_snat": True, "external_fixed_ips": [...]}. | ||
| - Clear external gateway: clear_external_gateway=True (takes precedence over external_gateway_info). | ||
| - Replace static routes: routes=[{"destination": "192.0.2.0/24", "nexthop": "10.0.0.1"}]. Pass [] to remove all routes. |
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.
저는 이 문장이 두 가지로 해석되었어요.
- roter=
[]인 경우 모든 라우터를 삭제하므로 routes 값 업데이트를 건너뛴다. - 모든 라우터를 삭제하고 싶다면
[]전달한다.
이 문장이 어떤 의미를 가지고 있나요?
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.
None일 때 : static route를 변경하지 않는다[](빈 리스트) 일 때 : static route를 모두 삭제한다.[{"dest..": "192.168.0.0/24" ...}](값이 있는) 일 때 : static route를 완전히 교체한다.
답변이 되셨길 바랍니다.
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.
제가 실수로 approve를 눌러서 review 코멘트 남깁니다.
S0okJu
left a comment
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.
LGTM
|
익일 오전까지 다른 의견 기다리고 오후에 머지하겠습니다! |
paikend
left a comment
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.
LGTM
choieastsea
left a comment
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.
전체적으로 openstacksdk에서 제공하는 proxy와 cloud 계층이 혼용되어있는데, proxy 방법으로 통일해주시면 좋을 것 같습니다~!
choieastsea
left a comment
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.
LGTM
Overview
Key Changes
Related Issues
Additional context