[20.10 backport] Fix swarm rollback exitcode, and fix skipping verify step by thaJeztah · Pull Request #2964 · docker/cli

thaJeztah · 2021-02-04T13:57:03Z

backport of #2939

fixes moby/moby#35312 'docker service rollback' returns error code 1

Two patches in this PR:

docker service rollback: fix non-zero exit code in some cases

Before this change:

$ docker service create --replicas=1 --name foo -p 8080:80 nginx:alpine
t33qvykv8y0zbz266rxynsbo3
overall progress: 1 out of 1 tasks
1/1: running   [==================================================>]
verify: Service converged

$ echo $?
0

$ docker service update --replicas=5 foo
foo
overall progress: 5 out of 5 tasks
1/5: running   [==================================================>]
2/5: running   [==================================================>]
3/5: running   [==================================================>]
4/5: running   [==================================================>]
5/5: running   [==================================================>]
verify: Service converged

$ echo $?
0

$ docker service rollback foo
foo
rollback: manually requested rollback
overall progress: rolling back update: 1 out of 1 tasks
1/1: running   [>                                                  ]
verify: Service converged

$ echo $?
0

$ docker service rollback foo
foo
service rolled back: rollback completed

$ echo $?
1

After this change:

$ docker service create --replicas=1 --name foo -p 8080:80 nginx:alpine
t33qvykv8y0zbz266rxynsbo3
overall progress: 1 out of 1 tasks
1/1: running   [==================================================>]
verify: Service converged

$ echo $?
0

$ docker service update --replicas=5 foo
foo
overall progress: 5 out of 5 tasks
1/5: running   [==================================================>]
2/5: running   [==================================================>]
3/5: running   [==================================================>]
4/5: running   [==================================================>]
5/5: running   [==================================================>]
verify: Waiting 1 seconds to verify that tasks are stable...

$ echo $?
0

$ docker service rollback foo
foo
rollback: manually requested rollback
overall progress: rolling back update: 1 out of 1 tasks
1/1: running   [>                                                  ]
verify: Service converged

$ echo $?
0

$ docker service rollback foo
foo
service rolled back: rollback completed

$ echo $?
0

$ docker service ps foo
ID             NAME      IMAGE          NODE             DESIRED STATE   CURRENT STATE           ERROR     PORTS
4dt4ms4c5qfb   foo.1     nginx:alpine   docker-desktop   Running         Running 2 minutes ago

Remaining issues with reconciliation

Note that both before, and after this change, the command sometimes terminates
early, and does not wait for the service to reconcile; this is most apparent
when rolling back is scaling up (so more tasks are deployed);

$ docker service rollback foo
foo
service rolled back: rollback completed

$ docker service rollback foo
foo
rollback: manually requested rollback
overall progress: rolling back update: 1 out of 5 tasks
1/5: pending   [=================================>                 ]
2/5: running   [>                                                  ]
3/5: pending   [=================================>                 ]
4/5: pending   [=================================>                 ]
5/5: pending   [=================================>                 ]
service rolled back: rollback completed

service rollback: always verify state

Prior to this change, progressbars would sometimes be hidden, and the function
would return early. In addition, the direction of the progressbars would sometimes
be "incrementing" (similar to "docker service update"), and sometimes be "decrementing"
(to indicate a "rollback" is being performed).

This fix makes sure that we always proceed with the "verifying" step, and now
prints a message after the verifying stage was completed;

$ docker service rollback foo
foo
overall progress: rolling back update: 5 out of 5 tasks
1/5: running   [>                                                  ]
2/5: starting  [===========>                                       ]
3/5: starting  [===========>                                       ]
4/5: running   [>                                                  ]
5/5: running   [>                                                  ]
verify: Service converged
rollback: rollback completed

$ docker service rollback foo
foo
overall progress: rolling back update: 1 out of 1 tasks
1/1: running   [>                                                  ]
verify: Service converged
rollback: rollback completed

- Description for the changelog

- Fix `docker service rollback` returning a non-zero exit code in some situations
- Fix inconsistent progress-bar direction on `docker service rollback`

Before this change: -------------------------------------------- $ docker service create --replicas=1 --name foo -p 8080:80 nginx:alpine t33qvykv8y0zbz266rxynsbo3 overall progress: 1 out of 1 tasks 1/1: running [==================================================>] verify: Service converged $ echo $? 0 $ docker service update --replicas=5 foo foo overall progress: 5 out of 5 tasks 1/5: running [==================================================>] 2/5: running [==================================================>] 3/5: running [==================================================>] 4/5: running [==================================================>] 5/5: running [==================================================>] verify: Service converged $ echo $? 0 $ docker service rollback foo foo rollback: manually requested rollback overall progress: rolling back update: 1 out of 1 tasks 1/1: running [> ] verify: Service converged $ echo $? 0 $ docker service rollback foo foo service rolled back: rollback completed $ echo $? 1 After this change: -------------------------------------------- $ docker service create --replicas=1 --name foo -p 8080:80 nginx:alpine t33qvykv8y0zbz266rxynsbo3 overall progress: 1 out of 1 tasks 1/1: running [==================================================>] verify: Service converged $ echo $? 0 $ docker service update --replicas=5 foo foo overall progress: 5 out of 5 tasks 1/5: running [==================================================>] 2/5: running [==================================================>] 3/5: running [==================================================>] 4/5: running [==================================================>] 5/5: running [==================================================>] verify: Waiting 1 seconds to verify that tasks are stable... $ echo $? 0 $ docker service rollback foo foo rollback: manually requested rollback overall progress: rolling back update: 1 out of 1 tasks 1/1: running [> ] verify: Service converged $ echo $? 0 $ docker service rollback foo foo service rolled back: rollback completed $ echo $? 0 $ docker service ps foo ID NAME IMAGE NODE DESIRED STATE CURRENT STATE ERROR PORTS 4dt4ms4c5qfb foo.1 nginx:alpine docker-desktop Running Running 2 minutes ago Remaining issues with reconciliation -------------------------------------------- Note that both before, and after this change, the command sometimes terminates early, and does not wait for the service to reconcile; this is most apparent when rolling back is scaling up (so more tasks are deployed); $ docker service rollback foo foo service rolled back: rollback completed $ docker service rollback foo foo rollback: manually requested rollback overall progress: rolling back update: 1 out of 5 tasks 1/5: pending [=================================> ] 2/5: running [> ] 3/5: pending [=================================> ] 4/5: pending [=================================> ] 5/5: pending [=================================> ] service rolled back: rollback completed Signed-off-by: Sebastiaan van Stijn <[email protected]> (cherry picked from commit ce26a16) Signed-off-by: Sebastiaan van Stijn <[email protected]>

Prior to this change, progressbars would sometimes be hidden, and the function would return early. In addition, the direction of the progressbars would sometimes be "incrementing" (similar to "docker service update"), and sometimes be "decrementing" (to indicate a "rollback" is being performed). This fix makes sure that we always proceed with the "verifying" step, and now prints a message _after_ the verifying stage was completed; $ docker service rollback foo foo overall progress: rolling back update: 5 out of 5 tasks 1/5: running [> ] 2/5: starting [===========> ] 3/5: starting [===========> ] 4/5: running [> ] 5/5: running [> ] verify: Service converged rollback: rollback completed $ docker service rollback foo foo overall progress: rolling back update: 1 out of 1 tasks 1/1: running [> ] verify: Service converged rollback: rollback completed Signed-off-by: Sebastiaan van Stijn <[email protected]> (cherry picked from commit 104469b) Signed-off-by: Sebastiaan van Stijn <[email protected]>

silvin-lubecki

LGTM

thaJeztah added 2 commits February 4, 2021 14:55

thaJeztah added impact/changelog status/2-code-review area/swarm labels Feb 4, 2021

thaJeztah added this to the 20.10.4 milestone Feb 4, 2021

silvin-lubecki approved these changes Feb 18, 2021

View reviewed changes

silvin-lubecki merged commit 1304844 into docker:20.10 Feb 18, 2021

thaJeztah deleted the 20.10_backport_fix_swarm_rollback_exitcode branch February 18, 2021 11:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[20.10 backport] Fix swarm rollback exitcode, and fix skipping verify step#2964

[20.10 backport] Fix swarm rollback exitcode, and fix skipping verify step#2964
silvin-lubecki merged 2 commits into
docker:20.10from
thaJeztah:20.10_backport_fix_swarm_rollback_exitcode

thaJeztah commented Feb 4, 2021

Uh oh!

silvin-lubecki left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

thaJeztah commented Feb 4, 2021

docker service rollback: fix non-zero exit code in some cases

Before this change:

After this change:

Remaining issues with reconciliation

service rollback: always verify state

Uh oh!

silvin-lubecki left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants