diff --git a/cli/command/stack/swarm/deploy.go b/cli/command/stack/swarm/deploy.go index d11c328f3f..df5f300026 100644 --- a/cli/command/stack/swarm/deploy.go +++ b/cli/command/stack/swarm/deploy.go @@ -25,25 +25,26 @@ const ( func RunDeploy(dockerCli command.Cli, opts options.Deploy, cfg *composetypes.Config) error { ctx := context.Background() - if err := validateResolveImageFlag(dockerCli, &opts); err != nil { + if err := validateResolveImageFlag(&opts); err != nil { return err } - - return deployCompose(ctx, dockerCli, opts, cfg) -} - -// validateResolveImageFlag validates the opts.resolveImage command line option -// and also turns image resolution off if the version is older than 1.30 -func validateResolveImageFlag(dockerCli command.Cli, opts *options.Deploy) error { - if opts.ResolveImage != ResolveImageAlways && opts.ResolveImage != ResolveImageChanged && opts.ResolveImage != ResolveImageNever { - return errors.Errorf("Invalid option %s for flag --resolve-image", opts.ResolveImage) - } // client side image resolution should not be done when the supported // server version is older than 1.30 if versions.LessThan(dockerCli.Client().ClientVersion(), "1.30") { opts.ResolveImage = ResolveImageNever } - return nil + + return deployCompose(ctx, dockerCli, opts, cfg) +} + +// validateResolveImageFlag validates the opts.resolveImage command line option +func validateResolveImageFlag(opts *options.Deploy) error { + switch opts.ResolveImage { + case ResolveImageAlways, ResolveImageChanged, ResolveImageNever: + return nil + default: + return errors.Errorf("Invalid option %s for flag --resolve-image", opts.ResolveImage) + } } // checkDaemonIsSwarmManager does an Info API call to verify that the daemon is diff --git a/cli/command/stack/swarm/deploy_composefile.go b/cli/command/stack/swarm/deploy_composefile.go index e4574fd987..2ab9be8a4f 100644 --- a/cli/command/stack/swarm/deploy_composefile.go +++ b/cli/command/stack/swarm/deploy_composefile.go @@ -77,11 +77,7 @@ func getServicesDeclaredNetworks(serviceConfigs []composetypes.ServiceConfig) ma return serviceNetworks } -func validateExternalNetworks( - ctx context.Context, - client dockerclient.NetworkAPIClient, - externalNetworks []string, -) error { +func validateExternalNetworks(ctx context.Context, client dockerclient.NetworkAPIClient, externalNetworks []string) error { for _, networkName := range externalNetworks { if !container.NetworkMode(networkName).IsUserDefined() { // Networks that are not user defined always exist on all nodes as @@ -101,11 +97,7 @@ func validateExternalNetworks( return nil } -func createSecrets( - ctx context.Context, - dockerCli command.Cli, - secrets []swarm.SecretSpec, -) error { +func createSecrets(ctx context.Context, dockerCli command.Cli, secrets []swarm.SecretSpec) error { client := dockerCli.Client() for _, secretSpec := range secrets { @@ -129,11 +121,7 @@ func createSecrets( return nil } -func createConfigs( - ctx context.Context, - dockerCli command.Cli, - configs []swarm.ConfigSpec, -) error { +func createConfigs(ctx context.Context, dockerCli command.Cli, configs []swarm.ConfigSpec) error { client := dockerCli.Client() for _, configSpec := range configs { @@ -157,12 +145,7 @@ func createConfigs( return nil } -func createNetworks( - ctx context.Context, - dockerCli command.Cli, - namespace convert.Namespace, - networks map[string]types.NetworkCreate, -) error { +func createNetworks(ctx context.Context, dockerCli command.Cli, namespace convert.Namespace, networks map[string]types.NetworkCreate) error { client := dockerCli.Client() existingNetworks, err := getStackNetworks(ctx, client, namespace.Name()) @@ -192,14 +175,8 @@ func createNetworks( return nil } -func deployServices( - ctx context.Context, - dockerCli command.Cli, - services map[string]swarm.ServiceSpec, - namespace convert.Namespace, - sendAuth bool, - resolveImage string, -) error { +// nolint: gocyclo +func deployServices(ctx context.Context, dockerCli command.Cli, services map[string]swarm.ServiceSpec, namespace convert.Namespace, sendAuth bool, resolveImage string) error { apiClient := dockerCli.Client() out := dockerCli.Out() @@ -214,10 +191,12 @@ func deployServices( } for internalName, serviceSpec := range services { - name := namespace.Scope(internalName) + var ( + name = namespace.Scope(internalName) + image = serviceSpec.TaskTemplate.ContainerSpec.Image + encodedAuth string + ) - encodedAuth := "" - image := serviceSpec.TaskTemplate.ContainerSpec.Image if sendAuth { // Retrieve encoded auth token from the image reference encodedAuth, err = command.RetrieveAuthTokenFromImage(ctx, dockerCli, image) @@ -231,30 +210,37 @@ func deployServices( updateOpts := types.ServiceUpdateOptions{EncodedRegistryAuth: encodedAuth} - switch { - case resolveImage == ResolveImageAlways || (resolveImage == ResolveImageChanged && image != service.Spec.Labels[convert.LabelImage]): + switch resolveImage { + case ResolveImageAlways: // 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 + case ResolveImageChanged: + if image != service.Spec.Labels[convert.LabelImage] { + // Query the registry to resolve digest for the updated image + updateOpts.QueryRegistry = true + } else { + // 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 + } + default: + if 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 + } } - // Stack deploy does not have a `--force` option. Preserve existing ForceUpdate - // value so that tasks are not re-deployed if not updated. + // Stack deploy does not have a `--force` option. Preserve existing + // ForceUpdate value so that tasks are not re-deployed if not updated. // TODO move this to API client? serviceSpec.TaskTemplate.ForceUpdate = service.Spec.TaskTemplate.ForceUpdate - response, err := apiClient.ServiceUpdate( - ctx, - service.ID, - service.Version, - serviceSpec, - updateOpts, - ) + response, err := apiClient.ServiceUpdate(ctx, service.ID, service.Version, serviceSpec, updateOpts) if err != nil { return errors.Wrapf(err, "failed to update service %s", name) } diff --git a/cli/command/stack/swarm/deploy_test.go b/cli/command/stack/swarm/deploy_test.go index cca29cf826..f5a44dcbf5 100644 --- a/cli/command/stack/swarm/deploy_test.go +++ b/cli/command/stack/swarm/deploy_test.go @@ -87,24 +87,26 @@ func TestServiceUpdateResolveImageChanged(t *testing.T) { 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, + for _, tc := range testcases { + tc := tc + t.Run(tc.image, func(t *testing.T) { + spec := map[string]swarm.ServiceSpec{ + "myservice": { + TaskTemplate: swarm.TaskSpec{ + ContainerSpec: &swarm.ContainerSpec{ + Image: tc.image, + }, }, }, - }, - } - err := deployServices(ctx, client, spec, namespace, false, ResolveImageChanged) - assert.NilError(t, err) - assert.Check(t, is.Equal(receivedOptions.QueryRegistry, testcase.expectedQueryRegistry)) - assert.Check(t, is.Equal(receivedService.TaskTemplate.ContainerSpec.Image, testcase.expectedImage)) - assert.Check(t, is.Equal(receivedService.TaskTemplate.ForceUpdate, testcase.expectedForceUpdate)) + } + err := deployServices(ctx, client, spec, namespace, false, ResolveImageChanged) + assert.NilError(t, err) + assert.Check(t, is.Equal(receivedOptions.QueryRegistry, tc.expectedQueryRegistry)) + assert.Check(t, is.Equal(receivedService.TaskTemplate.ContainerSpec.Image, tc.expectedImage)) + assert.Check(t, is.Equal(receivedService.TaskTemplate.ForceUpdate, tc.expectedForceUpdate)) - receivedService = swarm.ServiceSpec{} - receivedOptions = types.ServiceUpdateOptions{} + receivedService = swarm.ServiceSpec{} + receivedOptions = types.ServiceUpdateOptions{} + }) } }