-
Notifications
You must be signed in to change notification settings - Fork 4
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
add a client to register endpoint to tks-info #3
Conversation
cmd/register_endpoint.go
Outdated
epportlist := flag.String("epportlist", "", | ||
"The list of port per app like (application_type, port) eg. {\"1\":\"80\",\"2\":\"10232\"}") | ||
eplist := flag.String("eplist", "", | ||
"The list of endpoints like (application_type, endpoint) eg. {\"1\":\"192.168.5.55\",\"2\":\"192.168.5.55:10232\"}") |
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.
'192.168.5.55:10232'은 '192.168.5.55'의 오타일까요?
그리고 Endpoint라고 하면 'IP:Port' 가 한 쌍일텐데 지금 CLI 옵션은 IP와 포트가 나누어져 있어서 일반적인 형식은 아닌 거 같습니다. 특별한 이유가 있는지 궁금합니다.
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.
이건 다양하게 적용할수 있도록 넣은 것입니다.
(openinfradev/decapod-flow#30 도 참조)
- 앱별 전체 url을 넣거나
- 대표 ip 적고 포트별로 넣거나
- 대표 ip만 적어서 default로 미리 정해진것 넣거나
위 세가지를 위한 것이고 일단 쓸것은 3번째 사용예입니다.
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.
192.168.5.55는 192.168.5.55:80의 의도였습니다만 꼭 http가 아닐수도 있으니 한번 변경을 고려해봐야겠네요.
그런데 예시라서..
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.
이것도 수정했어요
cmd/register_endpoint.go
Outdated
|
||
ip := flag.String("tks", "127.0.0.1", "An address of TKS-info") | ||
clusterid := flag.String("clusterid", "", "Cluster ID to apply. eg. 6abead61-ff2a-4af4-8f41-d2c44c745de7") | ||
appid := flag.String("appid", "", "Application ID of The endpoint. eg. 6abead61-ff2a-4af4-8f41-d2c44c745de7") |
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.
기존 appID가 appGroupID로 변경되어서 appid => appGroupID로 바뀌어야 할 것 같습니다.
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.
반영했습니다.
cmd/register_endpoint.go
Outdated
// three types to define eps | ||
// - eplist: set of entry (type, url) | ||
// - clusterep and epportlist: cluster endpoint and set of entry (type, port) | ||
// - clusterep only: cluster endpoint and default endpoint using the node port. (currently prometheus) |
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.
주석 의미가 좀 혼동됩니다. ep 가 두개라는 것처럼 보여서..
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.
걍 명확하게 default cluster endpoint 라고 해주심이..
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. |
This PR was closed because it has been stalled for 10 days with no activity. |
This PR was closed because it has been stalled for 10 days with no activity. |
This PR was closed because it has been stalled for 10 days with no activity. |
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. |
This PR was closed because it has been stalled for 10 days with no activity. |
lma 설치용 workflow에서 사용할 binary 생성