-
Notifications
You must be signed in to change notification settings - Fork 15
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
change priorty of reboot queue cancel #715
Conversation
server/control.go
Outdated
@@ -346,13 +346,15 @@ func (c Controller) runOnce(ctx context.Context, leaderKey string, tick <-chan t | |||
newlyDrained := op.ChooseDrainedNodes(cluster, apiServers, rqEntries) | |||
drainCompleted, drainTimedout, _ := op.CheckDrainCompletion(ctx, inf, nf.HealthyAPIServer(), cluster, rqEntries) | |||
rebootDequeued := op.CheckRebootDequeue(ctx, cluster, rqEntries) | |||
RebootCancelled := op.CheckRebootCancelled(ctx, cluster, rqEntries) |
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.
Please use lowercase.
RebootCancelled := op.CheckRebootCancelled(ctx, cluster, rqEntries) | |
rebootCancelled := op.CheckRebootCancelled(ctx, cluster, rqEntries) |
server/strategy.go
Outdated
@@ -883,6 +884,11 @@ func rebootOps(c *cke.Cluster, constraints *cke.Constraints, rebootArgs DecideOp | |||
return nil, false | |||
} | |||
|
|||
if len(rebootArgs.RebootCancelled) > 0 { | |||
phaseReboot = true | |||
ops = append(ops, op.RebootDequeueOp(rebootArgs.RebootCancelled)) |
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.
I want to change the display name of the op to reboot-cancel.
Is it possible?
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.
We can change the display name by duplicating RebootDequeueOp
and changing Name()
method.
The logic of Op is identical. so I created RebootCancelOp
which has embedded RebootDequeueOp
and override Name()
method.
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.
Thanks!
@@ -523,6 +523,25 @@ func (c rebootDequeueCommand) Command() cke.Command { | |||
} | |||
} | |||
|
|||
// |
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.
Forgot to write?
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.
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.
Oh, I see.
server/strategy.go
Outdated
@@ -883,6 +884,11 @@ func rebootOps(c *cke.Cluster, constraints *cke.Constraints, rebootArgs DecideOp | |||
return nil, false | |||
} | |||
|
|||
if len(rebootArgs.RebootCancelled) > 0 { | |||
phaseReboot = true | |||
ops = append(ops, op.RebootDequeueOp(rebootArgs.RebootCancelled)) |
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.
Thanks!
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
In the current CKE, the dequeue operation of cancelled reboot queue entry has lower priority than other operations like drain, reboot, ...etc.
This implementation causes the dequeue operation not to work correctly when we cancel an entry whose node is under operation.
This PR solves the problem by following changes.