From f367aa9330fae11bc442a3a03a4a0e38cc2802f2 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 11 May 2018 11:47:05 +0200 Subject: [PATCH] Fix service rollback options being cross-wired The "update" and "rollback" configurations were cross-wired, as a result, setting `--rollback-*` options would override the service's update-options. Creating a service with both update, and rollback configuration: docker service create \ --name=test \ --update-failure-action=pause \ --update-max-failure-ratio=0.6 \ --update-monitor=3s \ --update-order=stop-first \ --update-parallelism=3 \ --rollback-failure-action=continue \ --rollback-max-failure-ratio=0.5 \ --rollback-monitor=4s \ --rollback-order=start-first \ --rollback-parallelism=2 \ --tty \ busybox Before this change: docker service inspect --format '{{json .Spec.UpdateConfig}}' test \ && docker service inspect --format '{{json .Spec.RollbackConfig}}' test Produces: {"Parallelism":3,"FailureAction":"pause","Monitor":3000000000,"MaxFailureRatio":0.6,"Order":"stop-first"} {"Parallelism":3,"FailureAction":"pause","Monitor":3000000000,"MaxFailureRatio":0.6,"Order":"stop-first"} After this change: {"Parallelism":3,"FailureAction":"pause","Monitor":3000000000,"MaxFailureRatio":0.6,"Order":"stop-first"} {"Parallelism":2,"FailureAction":"continue","Monitor":4000000000,"MaxFailureRatio":0.5,"Order":"start-first"} Signed-off-by: Sebastiaan van Stijn --- cli/command/service/opts.go | 2 +- cli/command/service/opts_test.go | 62 ++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/cli/command/service/opts.go b/cli/command/service/opts.go index e8f9718c7c3d..98338766a960 100644 --- a/cli/command/service/opts.go +++ b/cli/command/service/opts.go @@ -645,7 +645,7 @@ func (options *serviceOptions) ToService(ctx context.Context, apiClient client.N }, Mode: serviceMode, UpdateConfig: options.update.updateConfig(flags), - RollbackConfig: options.update.rollbackConfig(flags), + RollbackConfig: options.rollback.rollbackConfig(flags), EndpointSpec: options.endpoint.ToEndpointSpec(), } diff --git a/cli/command/service/opts_test.go b/cli/command/service/opts_test.go index 0ef8ff9e2f33..0cacffec93ba 100644 --- a/cli/command/service/opts_test.go +++ b/cli/command/service/opts_test.go @@ -162,3 +162,65 @@ func TestToServiceNetwork(t *testing.T) { assert.NilError(t, err) assert.Check(t, is.DeepEqual([]swarm.NetworkAttachmentConfig{{Target: "id111"}, {Target: "id555"}, {Target: "id999"}}, service.TaskTemplate.Networks)) } + +func TestToServiceUpdateRollback(t *testing.T) { + expected := swarm.ServiceSpec{ + UpdateConfig: &swarm.UpdateConfig{ + Parallelism: 23, + Delay: 34 * time.Second, + Monitor: 54321 * time.Nanosecond, + FailureAction: "pause", + MaxFailureRatio: 0.6, + Order: "stop-first", + }, + RollbackConfig: &swarm.UpdateConfig{ + Parallelism: 12, + Delay: 23 * time.Second, + Monitor: 12345 * time.Nanosecond, + FailureAction: "continue", + MaxFailureRatio: 0.5, + Order: "start-first", + }, + } + + // Note: in test-situation, the flags are only used to detect if an option + // was set; the actual value itself is read from the serviceOptions below. + flags := newCreateCommand(nil).Flags() + flags.Set("update-parallelism", "23") + flags.Set("update-delay", "34s") + flags.Set("update-monitor", "54321ns") + flags.Set("update-failure-action", "pause") + flags.Set("update-max-failure-ratio", "0.6") + flags.Set("update-order", "stop-first") + + flags.Set("rollback-parallelism", "12") + flags.Set("rollback-delay", "23s") + flags.Set("rollback-monitor", "12345ns") + flags.Set("rollback-failure-action", "continue") + flags.Set("rollback-max-failure-ratio", "0.5") + flags.Set("rollback-order", "start-first") + + o := newServiceOptions() + o.mode = "replicated" + o.update = updateOptions{ + parallelism: 23, + delay: 34 * time.Second, + monitor: 54321 * time.Nanosecond, + onFailure: "pause", + maxFailureRatio: 0.6, + order: "stop-first", + } + o.rollback = updateOptions{ + parallelism: 12, + delay: 23 * time.Second, + monitor: 12345 * time.Nanosecond, + onFailure: "continue", + maxFailureRatio: 0.5, + order: "start-first", + } + + service, err := o.ToService(context.Background(), &fakeClient{}, flags) + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(service.UpdateConfig, expected.UpdateConfig)) + assert.Check(t, is.DeepEqual(service.RollbackConfig, expected.RollbackConfig)) +}