From c7d9599e3d95ce588113dc351facb2befac062fa Mon Sep 17 00:00:00 2001 From: Drew Erny Date: Wed, 29 May 2019 12:54:39 -0500 Subject: [PATCH] Revert docker/swarmkit#2804 Reverts the change to swarmkit that made all updates set UpdateStatus to Completed Signed-off-by: Drew Erny --- integration/service/update_test.go | 24 +++++++--- vendor.conf | 2 +- .../manager/orchestrator/update/updater.go | 45 +++++-------------- 3 files changed, 32 insertions(+), 39 deletions(-) diff --git a/integration/service/update_test.go b/integration/service/update_test.go index 92a4368545..8575e56857 100644 --- a/integration/service/update_test.go +++ b/integration/service/update_test.go @@ -33,7 +33,7 @@ func TestServiceUpdateLabel(t *testing.T) { service.Spec.Labels["foo"] = "bar" _, err := cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) assert.NilError(t, err) - poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + poll.WaitOn(t, serviceSpecIsUpdated(cli, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(t, cli, serviceID) assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{"foo": "bar"})) @@ -41,21 +41,21 @@ func TestServiceUpdateLabel(t *testing.T) { service.Spec.Labels["foo2"] = "bar" _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) assert.NilError(t, err) - poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + poll.WaitOn(t, serviceSpecIsUpdated(cli, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(t, cli, serviceID) assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{"foo": "bar", "foo2": "bar"})) delete(service.Spec.Labels, "foo2") _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) assert.NilError(t, err) - poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + poll.WaitOn(t, serviceSpecIsUpdated(cli, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(t, cli, serviceID) assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{"foo": "bar"})) delete(service.Spec.Labels, "foo") _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) assert.NilError(t, err) - poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + poll.WaitOn(t, serviceSpecIsUpdated(cli, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(t, cli, serviceID) assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{})) @@ -63,7 +63,7 @@ func TestServiceUpdateLabel(t *testing.T) { service.Spec.Labels["foo"] = "bar" _, err = cli.ServiceUpdate(ctx, serviceID, service.Version, service.Spec, types.ServiceUpdateOptions{}) assert.NilError(t, err) - poll.WaitOn(t, serviceIsUpdated(cli, serviceID), swarm.ServicePoll) + poll.WaitOn(t, serviceSpecIsUpdated(cli, serviceID, service.Version.Index), swarm.ServicePoll) service = getService(t, cli, serviceID) assert.Check(t, is.DeepEqual(service.Spec.Labels, map[string]string{"foo": "bar"})) @@ -271,3 +271,17 @@ func serviceIsUpdated(client client.ServiceAPIClient, serviceID string) func(log } } } + +func serviceSpecIsUpdated(client client.ServiceAPIClient, serviceID string, serviceOldVersion uint64) func(log poll.LogT) poll.Result { + return func(log poll.LogT) poll.Result { + service, _, err := client.ServiceInspectWithRaw(context.Background(), serviceID, types.ServiceInspectOptions{}) + switch { + case err != nil: + return poll.Error(err) + case service.Version.Index > serviceOldVersion: + return poll.Success() + default: + return poll.Continue("waiting for service %s to be updated", serviceID) + } + } +} diff --git a/vendor.conf b/vendor.conf index d7385ea4ef..018ebbd407 100644 --- a/vendor.conf +++ b/vendor.conf @@ -130,7 +130,7 @@ github.com/containerd/ttrpc f02858b1457c5ca3aaec3a0803eb github.com/gogo/googleapis d31c731455cb061f42baff3bda55bad0118b126b # v1.2.0 # cluster -github.com/docker/swarmkit 1e20cbf4caaeaba79c75af00bce5f41085db4082 +github.com/docker/swarmkit fb584e7b501ec4683b5c3e62476d76b8a7e7d9f6 github.com/gogo/protobuf ba06b47c162d49f2af050fb4c75bcbc86a159d5c # v1.2.1 github.com/cloudflare/cfssl 5d63dbd981b5c408effbb58c442d54761ff94fbd # 1.3.2 github.com/fernet/fernet-go 9eac43b88a5efb8651d24de9b68e87567e029736 diff --git a/vendor/github.com/docker/swarmkit/manager/orchestrator/update/updater.go b/vendor/github.com/docker/swarmkit/manager/orchestrator/update/updater.go index e5fe3fe720..7c977dba1c 100644 --- a/vendor/github.com/docker/swarmkit/manager/orchestrator/update/updater.go +++ b/vendor/github.com/docker/swarmkit/manager/orchestrator/update/updater.go @@ -141,17 +141,10 @@ func (u *Updater) Run(ctx context.Context, slots []orchestrator.Slot) { } // Abort immediately if all tasks are clean. if len(dirtySlots) == 0 { - if service.UpdateStatus == nil { - if u.annotationsUpdated(service) { - // Annotation-only update; mark the update as completed - u.completeUpdate(ctx, service.ID, true) - } - return - } - if service.UpdateStatus.State == api.UpdateStatus_UPDATING || service.UpdateStatus.State == api.UpdateStatus_ROLLBACK_STARTED { - // Update or rollback was started, and is now complete - u.completeUpdate(ctx, service.ID, true) - return + if service.UpdateStatus != nil && + (service.UpdateStatus.State == api.UpdateStatus_UPDATING || + service.UpdateStatus.State == api.UpdateStatus_ROLLBACK_STARTED) { + u.completeUpdate(ctx, service.ID) } return } @@ -310,7 +303,7 @@ slotsLoop: // have reached RUNNING by this point. if !stopped { - u.completeUpdate(ctx, service.ID, false) + u.completeUpdate(ctx, service.ID) } } @@ -623,32 +616,25 @@ func (u *Updater) rollbackUpdate(ctx context.Context, serviceID, message string) } } -func (u *Updater) completeUpdate(ctx context.Context, serviceID string, force bool) { +func (u *Updater) completeUpdate(ctx context.Context, serviceID string) { log.G(ctx).Debugf("update of service %s complete", serviceID) err := u.store.Update(func(tx store.Tx) error { service := store.GetService(tx, serviceID) - switch { - case service == nil: + if service == nil { return nil - case service.UpdateStatus == nil && force: - // Force marking the status as updated; to account for annotation-only updates. - service.UpdateStatus = &api.UpdateStatus{ - StartedAt: ptypes.MustTimestampProto(time.Now()), - State: api.UpdateStatus_COMPLETED, - Message: "update completed", - } - case service.UpdateStatus == nil: + } + if service.UpdateStatus == nil { // The service was changed since we started this update return nil - case service.UpdateStatus.State == api.UpdateStatus_ROLLBACK_STARTED: + } + if service.UpdateStatus.State == api.UpdateStatus_ROLLBACK_STARTED { service.UpdateStatus.State = api.UpdateStatus_ROLLBACK_COMPLETED service.UpdateStatus.Message = "rollback completed" - default: + } else { service.UpdateStatus.State = api.UpdateStatus_COMPLETED service.UpdateStatus.Message = "update completed" } - service.UpdateStatus.CompletedAt = ptypes.MustTimestampProto(time.Now()) return store.UpdateService(tx, service) @@ -658,10 +644,3 @@ func (u *Updater) completeUpdate(ctx context.Context, serviceID string, force bo log.G(ctx).WithError(err).Errorf("failed to mark update of service %s complete", serviceID) } } - -func (u *Updater) annotationsUpdated(service *api.Service) bool { - if service.PreviousSpec == nil { - return false - } - return !reflect.DeepEqual(service.Spec, service.PreviousSpec) -}