-
Notifications
You must be signed in to change notification settings - Fork 18.7k
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
Update the TestSwarmLockUnlockCluster
test to be less flakey.
#28997
Conversation
@@ -1079,10 +1079,9 @@ func (s *DockerSwarmSuite) TestSwarmLockUnlockCluster(c *check.C) { | |||
c.Assert(getNodeStatus(c, d2), checker.Equals, swarm.LocalNodeStateActive) | |||
|
|||
// once it's caught up, d2 is set to not be locked | |||
c.Assert(d2.Restart(), checker.IsNil) |
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.
I'm not sure this is necessary. The root cause looks like a bug in swarmkit. I'm going to submit a PR to fix it.
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.
@aaronlehmann The pending one? This one I think is where I'm not retrying after converting from autolocked to non-autolocked.
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.
Yes the pending one
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.
@aaronlehmann Ok, will remove the waiting for the state to not be in pending, then.
func getNodeStatus(c *check.C, d *SwarmDaemon) swarm.LocalNodeState { | ||
info, err := d.info() |
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.
info swarm.Info | ||
err error | ||
) | ||
waitAndAssert(c, defaultReconciliationTimeout, func(*check.C) (interface{}, check.CommentInterface) { |
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 is already a method d.checkLocalNodeState
for this
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!
func getNodeStatus(c *check.C, d *SwarmDaemon) swarm.LocalNodeState { | ||
waitAndAssert(c, defaultReconciliationTimeout, d.checkLocalNodeState, checker.NotEquals, swarm.LocalNodeStatePending) |
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.
checker.NotEquals is undefined, it should be checker.Not(checker.Equals)
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!
Signed-off-by: cyli <[email protected]>
TestSwarmLockUnlockCluster
test to be less flakey.
LGTM |
1 similar comment
LGTM |
This ensures that we wait until the state is not pending in order to check whether a swarm cluster is locked or unlocked, and ensure that we will optionally wait when transitioning from an autolocked state to a non-autolocked state and vice versa.
Would be useful for 1.13.
cc @tonistiigi @aaronlehmann