From d0bea64185218807ac007f14d903f2d2409da756 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 26 Jul 2017 15:33:57 +0200 Subject: [PATCH] Preserve resolved image-digest if QueryRegistry == false When re-deploying a stack without re-resolving the image digest, the service's ContainerSpec was updated with the image-reference as specified in the stack/compose file. As a result, the image-digest that was resolved in a previous deploy was overwritten, causing the service to be re-deployed. This patch preserves the previously resolve image-digest by copying it from the current service spec. A unit test is also added to verify that the image information in the service spec is not updated if QueryRegistry is disabled. Signed-off-by: Sebastiaan van Stijn --- cli/command/stack/client_test.go | 19 ++++-- cli/command/stack/deploy_composefile.go | 11 +++- cli/command/stack/deploy_test.go | 79 +++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 6 deletions(-) diff --git a/cli/command/stack/client_test.go b/cli/command/stack/client_test.go index d1d85193e7..bcb92db6c9 100644 --- a/cli/command/stack/client_test.go +++ b/cli/command/stack/client_test.go @@ -34,10 +34,13 @@ type fakeClient struct { nodeListFunc func(options types.NodeListOptions) ([]swarm.Node, error) taskListFunc func(options types.TaskListOptions) ([]swarm.Task, error) nodeInspectWithRaw func(ref string) (swarm.Node, []byte, error) - serviceRemoveFunc func(serviceID string) error - networkRemoveFunc func(networkID string) error - secretRemoveFunc func(secretID string) error - configRemoveFunc func(configID string) error + + serviceUpdateFunc func(serviceID string, version swarm.Version, service swarm.ServiceSpec, options types.ServiceUpdateOptions) (types.ServiceUpdateResponse, error) + + serviceRemoveFunc func(serviceID string) error + networkRemoveFunc func(networkID string) error + secretRemoveFunc func(secretID string) error + configRemoveFunc func(configID string) error } func (cli *fakeClient) ServerVersion(ctx context.Context) (types.Version, error) { @@ -132,6 +135,14 @@ func (cli *fakeClient) NodeInspectWithRaw(ctx context.Context, ref string) (swar return swarm.Node{}, nil, nil } +func (cli *fakeClient) ServiceUpdate(ctx context.Context, serviceID string, version swarm.Version, service swarm.ServiceSpec, options types.ServiceUpdateOptions) (types.ServiceUpdateResponse, error) { + if cli.serviceUpdateFunc != nil { + return cli.serviceUpdateFunc(serviceID, version, service, options) + } + + return types.ServiceUpdateResponse{}, nil +} + func (cli *fakeClient) ServiceRemove(ctx context.Context, serviceID string) error { if cli.serviceRemoveFunc != nil { return cli.serviceRemoveFunc(serviceID) diff --git a/cli/command/stack/deploy_composefile.go b/cli/command/stack/deploy_composefile.go index 1f50ae1a1d..16d583ed99 100644 --- a/cli/command/stack/deploy_composefile.go +++ b/cli/command/stack/deploy_composefile.go @@ -318,10 +318,17 @@ func deployServices( updateOpts := types.ServiceUpdateOptions{EncodedRegistryAuth: encodedAuth} - if resolveImage == resolveImageAlways || (resolveImage == resolveImageChanged && image != service.Spec.Labels[convert.LabelImage]) { + switch { + case resolveImage == resolveImageAlways || (resolveImage == resolveImageChanged && image != service.Spec.Labels[convert.LabelImage]): + // image should be updated by the server using QueryRegistry updateOpts.QueryRegistry = true + case image == service.Spec.Labels[convert.LabelImage]: + // image has not changed; update the serviceSpec with the + // existing information that was set by QueryRegistry on the + // previous deploy. Otherwise this will trigger an incorrect + // service update. + serviceSpec.TaskTemplate.ContainerSpec.Image = service.Spec.TaskTemplate.ContainerSpec.Image } - response, err := apiClient.ServiceUpdate( ctx, service.ID, diff --git a/cli/command/stack/deploy_test.go b/cli/command/stack/deploy_test.go index c04f86c251..f91a825f9b 100644 --- a/cli/command/stack/deploy_test.go +++ b/cli/command/stack/deploy_test.go @@ -5,6 +5,8 @@ import ( "github.com/docker/cli/cli/compose/convert" "github.com/docker/cli/cli/internal/test" + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/swarm" "github.com/stretchr/testify/assert" "golang.org/x/net/context" ) @@ -22,3 +24,80 @@ func TestPruneServices(t *testing.T) { pruneServices(ctx, dockerCli, namespace, services) assert.Equal(t, buildObjectIDs([]string{objectName("foo", "remove")}), client.removedServices) } + +// TestServiceUpdateResolveImageChanged tests that the service's +// image digest is preserved if the image did not change in the compose file +func TestServiceUpdateResolveImageChanged(t *testing.T) { + namespace := convert.NewNamespace("mystack") + + var ( + receivedOptions types.ServiceUpdateOptions + receivedService swarm.ServiceSpec + ) + + client := test.NewFakeCli(&fakeClient{ + serviceListFunc: func(options types.ServiceListOptions) ([]swarm.Service, error) { + return []swarm.Service{ + { + Spec: swarm.ServiceSpec{ + Annotations: swarm.Annotations{ + Name: namespace.Name() + "_myservice", + Labels: map[string]string{"com.docker.stack.image": "foobar:1.2.3"}, + }, + TaskTemplate: swarm.TaskSpec{ + ContainerSpec: swarm.ContainerSpec{ + Image: "foobar:1.2.3@sha256:deadbeef", + }, + }, + }, + }, + }, nil + }, + serviceUpdateFunc: func(serviceID string, version swarm.Version, service swarm.ServiceSpec, options types.ServiceUpdateOptions) (types.ServiceUpdateResponse, error) { + receivedOptions = options + receivedService = service + return types.ServiceUpdateResponse{}, nil + }, + }) + + var testcases = []struct { + image string + expectedQueryRegistry bool + expectedImage string + }{ + // Image not changed + { + image: "foobar:1.2.3", + expectedQueryRegistry: false, + expectedImage: "foobar:1.2.3@sha256:deadbeef", + }, + // Image changed + { + image: "foobar:1.2.4", + expectedQueryRegistry: true, + expectedImage: "foobar:1.2.4", + }, + } + + ctx := context.Background() + + for _, testcase := range testcases { + t.Logf("Testing image %q", testcase.image) + spec := map[string]swarm.ServiceSpec{ + "myservice": { + TaskTemplate: swarm.TaskSpec{ + ContainerSpec: swarm.ContainerSpec{ + Image: testcase.image, + }, + }, + }, + } + err := deployServices(ctx, client, spec, namespace, false, resolveImageChanged) + assert.NoError(t, err) + assert.Equal(t, testcase.expectedQueryRegistry, receivedOptions.QueryRegistry) + assert.Equal(t, testcase.expectedImage, receivedService.TaskTemplate.ContainerSpec.Image) + + receivedService = swarm.ServiceSpec{} + receivedOptions = types.ServiceUpdateOptions{} + } +}