-
Notifications
You must be signed in to change notification settings - Fork 0
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 boot-ip-setter #2767
base: main
Are you sure you want to change the base?
Add boot-ip-setter #2767
Conversation
d5dcd60
to
6af14e9
Compare
Signed-off-by: Masayuki Ishii <[email protected]>
27f26a4
to
d51b74c
Compare
Signed-off-by: Masayuki Ishii <[email protected]>
e310635
to
9df6bae
Compare
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.
The PR looks good to me.
As for the parts I commented on, I think they're just a matter of preference, so feel free to merge it as is.
pkg/boot-ip-setter/main.go
Outdated
ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGTERM, syscall.SIGINT) | ||
defer stop() | ||
|
||
logger.Info("hello!", "interface", *flagInterface, "interval", *flagInterval, "listen address", *flagListenAddress) |
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.
While it might be a matter of preference, I felt it would be better to have a log message indicating that boot-ip-setter
has started.
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.
Thank you. That's right, and it is more polite.
Fixed 8faad74
pkg/boot-ip-setter/main.go
Outdated
os.Exit(1) | ||
} | ||
|
||
logger.Info("bye!") |
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.
Similarly, I felt it would be better to have a log message indicating that boot-ip-setter
has finished.
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.
Fixed 8faad74
This PR implementes #2357 .
Add a new daemon
boot-ip-setter
on boot servers, and it sets virtual IPs.