From ce26a165b07b15666ef8880287c62101ae03d340 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 19 Jan 2021 14:25:08 +0100 Subject: [PATCH 1/2] 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 Signed-off-by: Sebastiaan van Stijn --- cli/command/service/progress/progress.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/command/service/progress/progress.go b/cli/command/service/progress/progress.go index 3f118d81a4..261cfe55ae 100644 --- a/cli/command/service/progress/progress.go +++ b/cli/command/service/progress/progress.go @@ -140,7 +140,8 @@ func ServiceProgress(ctx context.Context, client client.APIClient, serviceID str return fmt.Errorf("service rollback paused: %s", service.UpdateStatus.Message) case swarm.UpdateStateRollbackCompleted: if !converged { - return fmt.Errorf("service rolled back: %s", service.UpdateStatus.Message) + progress.Messagef(progressOut, "", "service rolled back: %s", service.UpdateStatus.Message) + return nil } } } From 104469be0b9f26a9fc8881b0cac3a91a7dd6d4f0 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 19 Jan 2021 14:38:43 +0100 Subject: [PATCH 2/2] 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 Signed-off-by: Sebastiaan van Stijn --- cli/command/service/progress/progress.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/cli/command/service/progress/progress.go b/cli/command/service/progress/progress.go index 261cfe55ae..3ed41cb9e4 100644 --- a/cli/command/service/progress/progress.go +++ b/cli/command/service/progress/progress.go @@ -99,6 +99,7 @@ func ServiceProgress(ctx context.Context, client client.APIClient, serviceID str convergedAt time.Time monitor = 5 * time.Second rollback bool + message *progress.Progress ) for { @@ -140,9 +141,9 @@ func ServiceProgress(ctx context.Context, client client.APIClient, serviceID str return fmt.Errorf("service rollback paused: %s", service.UpdateStatus.Message) case swarm.UpdateStateRollbackCompleted: if !converged { - progress.Messagef(progressOut, "", "service rolled back: %s", service.UpdateStatus.Message) - return nil + message = &progress.Progress{ID: "rollback", Message: service.UpdateStatus.Message} } + rollback = true } } if converged && time.Since(convergedAt) >= monitor { @@ -150,7 +151,9 @@ func ServiceProgress(ctx context.Context, client client.APIClient, serviceID str ID: "verify", Action: "Service converged", }) - + if message != nil { + progressOut.WriteProgress(*message) + } return nil }