-
Notifications
You must be signed in to change notification settings - Fork 114
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
Fix config-daemon not waiting for drain to complete #682
Conversation
Thanks for your PR,
To skip the vendors CIs use one of:
|
pkg/daemon/daemon.go
Outdated
@@ -484,10 +484,14 @@ func (dn *Daemon) nodeStateSyncHandler() error { | |||
if reqDrain || !utils.ObjectHasAnnotation(dn.desiredNodeState, | |||
consts.NodeStateDrainAnnotationCurrent, | |||
consts.DrainIdle) { | |||
if err := dn.handleDrain(reqReboot); err != nil { | |||
shouldReconcile, err := dn.handleDrain(reqReboot) |
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.
can you rename the variable to something like inProgress
or drainInProgress
? For me shouldReconcile
looks more like "should continue the current reconcile cycle", but not like "retry on next reconcile cycle" (the meaning which we want, If I understood correctly)
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.
sure! done
425037f
to
055f233
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
055f233
to
07254b5
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
Signed-off-by: Sebastian Sch <[email protected]>
Pull Request Test Coverage Report for Build 8720278678Details
💛 - Coveralls |
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
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
No description provided.