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 <github@gone.nl>

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
+	serviceUpdateFunc func(serviceID string, version swarm.Version, service swarm.ServiceSpec, options types.ServiceUpdateOptions) (types.ServiceUpdateResponse, error)
-	secretRemoveFunc   func(secretID string) error
+
-	configRemoveFunc   func(configID string) 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)

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,

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{}
+	}
+}