Return warnings from service create and service update when digest pinning fails

Modify the service update and create APIs to return optional warning
messages as part of the response. Populate these messages with an
informative reason when digest resolution fails.

This is a small API change, but significantly improves the UX. The user
can now get immediate feedback when they've specified a nonexistent
image or unreachable registry.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
(cherry picked from commit 948e60691e)
Signed-off-by: Victor Vieux <victorvieux@gmail.com>

api/server/router/swarm/backend.go

@@ -18,8 +18,8 @@ type Backend interface {
 	UnlockSwarm(req types.UnlockRequest) error
 	GetServices(basictypes.ServiceListOptions) ([]types.Service, error)
 	GetService(string) (types.Service, error)
-	CreateService(types.ServiceSpec, string) (string, error)
-	UpdateService(string, uint64, types.ServiceSpec, string, string) error
+	CreateService(types.ServiceSpec, string) (*basictypes.ServiceCreateResponse, error)
+	UpdateService(string, uint64, types.ServiceSpec, string, string) (*basictypes.ServiceUpdateResponse, error)
 	RemoveService(string) error
 	ServiceLogs(context.Context, string, *backend.ContainerLogsConfig, chan struct{}) error
 	GetNodes(basictypes.NodeListOptions) ([]types.Node, error)

api/server/router/swarm/cluster_routes.go

@@ -166,15 +166,13 @@ func (sr *swarmRouter) createService(ctx context.Context, w http.ResponseWriter,
 	// Get returns "" if the header does not exist
 	encodedAuth := r.Header.Get("X-Registry-Auth")
 
-	id, err := sr.backend.CreateService(service, encodedAuth)
+	resp, err := sr.backend.CreateService(service, encodedAuth)
 	if err != nil {
 		logrus.Errorf("Error creating service %s: %v", service.Name, err)
 		return err
 	}
 
-	return httputils.WriteJSON(w, http.StatusCreated, &basictypes.ServiceCreateResponse{
-		ID: id,
-	})
+	return httputils.WriteJSON(w, http.StatusCreated, resp)
 }
 
 func (sr *swarmRouter) updateService(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
@@ -194,11 +192,12 @@ func (sr *swarmRouter) updateService(ctx context.Context, w http.ResponseWriter,
 
 	registryAuthFrom := r.URL.Query().Get("registryAuthFrom")
 
-	if err := sr.backend.UpdateService(vars["id"], version, service, encodedAuth, registryAuthFrom); err != nil {
+	resp, err := sr.backend.UpdateService(vars["id"], version, service, encodedAuth, registryAuthFrom)
+	if err != nil {
 		logrus.Errorf("Error updating service %s: %v", vars["id"], err)
 		return err
 	}
-	return nil
+	return httputils.WriteJSON(w, http.StatusOK, resp)
 }
 
 func (sr *swarmRouter) removeService(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {

api/swagger.yaml

@@ -2218,6 +2218,16 @@ definitions:
       Deleted:
         description: "The image ID of an image that was deleted"
         type: "string"
+  ServiceUpdateResponse:
+    type: "object"
+    properties:
+      Warnings:
+        description: "Optional warning messages"
+        type: "array"
+        items:
+          type: "string"
+    example:
+      Warning: "unable to pin image doesnotexist:latest to digest: image library/doesnotexist:latest not found"
   ContainerSummary:
     type: "array"
     items:
@@ -6875,8 +6885,12 @@ paths:
               ID:
                 description: "The ID of the created service."
                 type: "string"
+              Warning:
+                description: "Optional warning message"
+                type: "string"
             example:
               ID: "ak7w3gjqoa3kuz8xcpnyy0pvl"
+              Warning: "unable to pin image doesnotexist:latest to digest: image library/doesnotexist:latest not found"
         409:
           description: "name conflicts with an existing service"
           schema:
@@ -6998,10 +7012,14 @@ paths:
   /services/{id}/update:
     post:
       summary: "Update a service"
-      operationId: "PostServicesUpdate"
+      operationId: "ServiceUpdate"
+      consumes: ["application/json"]
+      produces: ["application/json"]
       responses:
         200:
           description: "no error"
+          schema:
+            $ref: "#/definitions/ImageDeleteResponse"
         404:
           description: "no such service"
           schema:
@@ -7065,8 +7083,7 @@ paths:
           description: "A base64-encoded auth configuration for pulling from private registries. [See the authentication section for details.](#section/Authentication)"
           type: "string"
 
-      tags:
-        - "Services"
+      tags: [Service]
   /tasks:
     get:
       summary: "List tasks"

api/types/client.go

@@ -289,6 +289,8 @@ type ServiceCreateOptions struct {
 type ServiceCreateResponse struct {
 	// ID is the ID of the created service.
 	ID string
+	// Warnings is a set of non-fatal warning messages to pass on to the user.
+	Warnings []string `json:",omitempty"`
 }
 
 // Values for RegistryAuthFrom in ServiceUpdateOptions

api/types/service_update_response.go

@@ -0,0 +1,12 @@
+package types
+
+// This file was generated by the swagger tool.
+// Editing this file might prove futile when you re-run the swagger generate command
+
+// ServiceUpdateResponse service update response
+// swagger:model ServiceUpdateResponse
+type ServiceUpdateResponse struct {
+
+	// Optional warning messages
+	Warnings []string `json:"Warnings"`
+}

api/types/volume.go

@@ -11,7 +11,7 @@ type Volume struct {
 	// Required: true
 	Driver string `json:"Driver"`
 
-	// A mapping of abitrary key/value data set on this volume.
+	// User-defined key/value metadata.
 	// Required: true
 	Labels map[string]string `json:"Labels"`
 

api/types/volume/volumes_create.go

@@ -19,7 +19,7 @@ type VolumesCreateBody struct {
 	// Required: true
 	DriverOpts map[string]string `json:"DriverOpts"`
 
-	// A mapping of arbitrary key/value data to set on the volume.
+	// User-defined key/value metadata.
 	// Required: true
 	Labels map[string]string `json:"Labels"`
 

cli/command/service/create.go

@@ -90,6 +90,10 @@ func runCreate(dockerCli *command.DockerCli, opts *serviceOptions) error {
 		return err
 	}
 
+	for _, warning := range response.Warnings {
+		fmt.Fprintln(dockerCli.Err(), warning)
+	}
+
 	fmt.Fprintf(dockerCli.Out(), "%s\n", response.ID)
 	return nil
 }

cli/command/service/scale.go

@@ -82,11 +82,15 @@ func runServiceScale(dockerCli *command.DockerCli, serviceID string, scale uint6
 
 	serviceMode.Replicated.Replicas = &scale
 
-	err = client.ServiceUpdate(ctx, service.ID, service.Version, service.Spec, types.ServiceUpdateOptions{})
+	response, err := client.ServiceUpdate(ctx, service.ID, service.Version, service.Spec, types.ServiceUpdateOptions{})
 	if err != nil {
 		return err
 	}
 
+	for _, warning := range response.Warnings {
+		fmt.Fprintln(dockerCli.Err(), warning)
+	}
+
 	fmt.Fprintf(dockerCli.Out(), "%s scaled to %d\n", serviceID, scale)
 	return nil
 }

cli/command/service/update.go

@@ -133,11 +133,15 @@ func runUpdate(dockerCli *command.DockerCli, flags *pflag.FlagSet, serviceID str
 		updateOpts.RegistryAuthFrom = types.RegistryAuthFromSpec
 	}
 
-	err = apiClient.ServiceUpdate(ctx, service.ID, service.Version, *spec, updateOpts)
+	response, err := apiClient.ServiceUpdate(ctx, service.ID, service.Version, *spec, updateOpts)
 	if err != nil {
 		return err
 	}
 
+	for _, warning := range response.Warnings {
+		fmt.Fprintln(dockerCli.Err(), warning)
+	}
+
 	fmt.Fprintf(dockerCli.Out(), "%s\n", serviceID)
 	return nil
 }

cli/command/stack/deploy.go

@@ -408,15 +408,20 @@ func deployServices(
 			if sendAuth {
 				updateOpts.EncodedRegistryAuth = encodedAuth
 			}
-			if err := apiClient.ServiceUpdate(
+			response, err := apiClient.ServiceUpdate(
 				ctx,
 				service.ID,
 				service.Version,
 				serviceSpec,
 				updateOpts,
-			); err != nil {
+			)
+			if err != nil {
 				return err
 			}
+
+			for _, warning := range response.Warnings {
+				fmt.Fprintln(dockerCli.Err(), warning)
+			}
 		} else {
 			fmt.Fprintf(out, "Creating service %s\n", name)
 

client/interface.go

@@ -124,7 +124,7 @@ type ServiceAPIClient interface {
 	ServiceInspectWithRaw(ctx context.Context, serviceID string) (swarm.Service, []byte, error)
 	ServiceList(ctx context.Context, options types.ServiceListOptions) ([]swarm.Service, error)
 	ServiceRemove(ctx context.Context, serviceID string) error
-	ServiceUpdate(ctx context.Context, serviceID string, version swarm.Version, service swarm.ServiceSpec, options types.ServiceUpdateOptions) error
+	ServiceUpdate(ctx context.Context, serviceID string, version swarm.Version, service swarm.ServiceSpec, options types.ServiceUpdateOptions) (types.ServiceUpdateResponse, error)
 	ServiceLogs(ctx context.Context, serviceID string, options types.ContainerLogsOptions) (io.ReadCloser, error)
 	TaskInspectWithRaw(ctx context.Context, taskID string) (swarm.Task, []byte, error)
 	TaskList(ctx context.Context, options types.TaskListOptions) ([]swarm.Task, error)

client/service_update.go

@@ -1,6 +1,7 @@
 package client
 
 import (
+	"encoding/json"
 	"net/url"
 	"strconv"
 
@@ -10,7 +11,7 @@ import (
 )
 
 // ServiceUpdate updates a Service.
-func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version swarm.Version, service swarm.ServiceSpec, options types.ServiceUpdateOptions) error {
+func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version swarm.Version, service swarm.ServiceSpec, options types.ServiceUpdateOptions) (types.ServiceUpdateResponse, error) {
 	var (
 		headers map[string][]string
 		query   = url.Values{}
@@ -28,7 +29,13 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version
 
 	query.Set("version", strconv.FormatUint(version.Index, 10))
 
+	var response types.ServiceUpdateResponse
 	resp, err := cli.post(ctx, "/services/"+serviceID+"/update", query, service, headers)
-	ensureReaderClosed(resp)
-	return err
+	if err != nil {
+		return response, err
+	}
+
+	err = json.NewDecoder(resp.body).Decode(&response)
+	ensureReaderClosed(resp)
+	return response, err
 }

client/service_update_test.go

@@ -19,7 +19,7 @@ func TestServiceUpdateError(t *testing.T) {
 		client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")),
 	}
 
-	err := client.ServiceUpdate(context.Background(), "service_id", swarm.Version{}, swarm.ServiceSpec{}, types.ServiceUpdateOptions{})
+	_, err := client.ServiceUpdate(context.Background(), "service_id", swarm.Version{}, swarm.ServiceSpec{}, types.ServiceUpdateOptions{})
 	if err == nil || err.Error() != "Error response from daemon: Server error" {
 		t.Fatalf("expected a Server Error, got %v", err)
 	}
@@ -64,12 +64,12 @@ func TestServiceUpdate(t *testing.T) {
 				}
 				return &http.Response{
 					StatusCode: http.StatusOK,
-					Body:       ioutil.NopCloser(bytes.NewReader([]byte("body"))),
+					Body:       ioutil.NopCloser(bytes.NewReader([]byte("{}"))),
 				}, nil
 			}),
 		}
 
-		err := client.ServiceUpdate(context.Background(), "service_id", updateCase.swarmVersion, swarm.ServiceSpec{}, types.ServiceUpdateOptions{})
+		_, err := client.ServiceUpdate(context.Background(), "service_id", updateCase.swarmVersion, swarm.ServiceSpec{}, types.ServiceUpdateOptions{})
 		if err != nil {
 			t.Fatal(err)
 		}

daemon/cluster/cluster.go

@@ -1062,12 +1062,12 @@ func (c *Cluster) imageWithDigestString(ctx context.Context, image string, authC
 }
 
 // CreateService creates a new service in a managed swarm cluster.
-func (c *Cluster) CreateService(s types.ServiceSpec, encodedAuth string) (string, error) {
+func (c *Cluster) CreateService(s types.ServiceSpec, encodedAuth string) (*apitypes.ServiceCreateResponse, error) {
 	c.RLock()
 	defer c.RUnlock()
 
 	if !c.isActiveManager() {
-		return "", c.errNoManager()
+		return nil, c.errNoManager()
 	}
 
 	ctx, cancel := c.getRequestContext()
@@ -1075,17 +1075,17 @@ func (c *Cluster) CreateService(s types.ServiceSpec, encodedAuth string) (string
 
 	err := c.populateNetworkID(ctx, c.client, &s)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
 	serviceSpec, err := convert.ServiceSpecToGRPC(s)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
 	ctnr := serviceSpec.Task.GetContainer()
 	if ctnr == nil {
-		return "", fmt.Errorf("service does not use container tasks")
+		return nil, fmt.Errorf("service does not use container tasks")
 	}
 
 	if encodedAuth != "" {
@@ -1099,11 +1099,15 @@ func (c *Cluster) CreateService(s types.ServiceSpec, encodedAuth string) (string
 			logrus.Warnf("invalid authconfig: %v", err)
 		}
 	}
+
+	resp := &apitypes.ServiceCreateResponse{}
+
 	// pin image by digest
 	if os.Getenv("DOCKER_SERVICE_PREFER_OFFLINE_IMAGE") != "1" {
 		digestImage, err := c.imageWithDigestString(ctx, ctnr.Image, authConfig)
 		if err != nil {
 			logrus.Warnf("unable to pin image %s to digest: %s", ctnr.Image, err.Error())
+			resp.Warnings = append(resp.Warnings, fmt.Sprintf("unable to pin image %s to digest: %s", ctnr.Image, err.Error()))
 		} else {
 			logrus.Debugf("pinning image %s by digest: %s", ctnr.Image, digestImage)
 			ctnr.Image = digestImage
@@ -1112,10 +1116,11 @@ func (c *Cluster) CreateService(s types.ServiceSpec, encodedAuth string) (string
 
 	r, err := c.client.CreateService(ctx, &swarmapi.CreateServiceRequest{Spec: &serviceSpec})
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
-	return r.Service.ID, nil
+	resp.ID = r.Service.ID
+	return resp, nil
 }
 
 // GetService returns a service based on an ID or name.
@@ -1138,12 +1143,12 @@ func (c *Cluster) GetService(input string) (types.Service, error) {
 }
 
 // UpdateService updates existing service to match new properties.
-func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec types.ServiceSpec, encodedAuth string, registryAuthFrom string) error {
+func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec types.ServiceSpec, encodedAuth string, registryAuthFrom string) (*apitypes.ServiceUpdateResponse, error) {
 	c.RLock()
 	defer c.RUnlock()
 
 	if !c.isActiveManager() {
-		return c.errNoManager()
+		return nil, c.errNoManager()
 	}
 
 	ctx, cancel := c.getRequestContext()
@@ -1151,22 +1156,22 @@ func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec typ
 
 	err := c.populateNetworkID(ctx, c.client, &spec)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	serviceSpec, err := convert.ServiceSpecToGRPC(spec)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	currentService, err := getService(ctx, c.client, serviceIDOrName)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	newCtnr := serviceSpec.Task.GetContainer()
 	if newCtnr == nil {
-		return fmt.Errorf("service does not use container tasks")
+		return nil, fmt.Errorf("service does not use container tasks")
 	}
 
 	if encodedAuth != "" {
@@ -1180,14 +1185,14 @@ func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec typ
 			ctnr = currentService.Spec.Task.GetContainer()
 		case apitypes.RegistryAuthFromPreviousSpec:
 			if currentService.PreviousSpec == nil {
-				return fmt.Errorf("service does not have a previous spec")
+				return nil, fmt.Errorf("service does not have a previous spec")
 			}
 			ctnr = currentService.PreviousSpec.Task.GetContainer()
 		default:
-			return fmt.Errorf("unsupported registryAuthFromValue")
+			return nil, fmt.Errorf("unsupported registryAuthFromValue")
 		}
 		if ctnr == nil {
-			return fmt.Errorf("service does not use container tasks")
+			return nil, fmt.Errorf("service does not use container tasks")
 		}
 		newCtnr.PullOptions = ctnr.PullOptions
 		// update encodedAuth so it can be used to pin image by digest
@@ -1203,11 +1208,15 @@ func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec typ
 			logrus.Warnf("invalid authconfig: %v", err)
 		}
 	}
+
+	resp := &apitypes.ServiceUpdateResponse{}
+
 	// pin image by digest
 	if os.Getenv("DOCKER_SERVICE_PREFER_OFFLINE_IMAGE") != "1" {
 		digestImage, err := c.imageWithDigestString(ctx, newCtnr.Image, authConfig)
 		if err != nil {
 			logrus.Warnf("unable to pin image %s to digest: %s", newCtnr.Image, err.Error())
+			resp.Warnings = append(resp.Warnings, fmt.Sprintf("unable to pin image %s to digest: %s", newCtnr.Image, err.Error()))
 		} else if newCtnr.Image != digestImage {
 			logrus.Debugf("pinning image %s by digest: %s", newCtnr.Image, digestImage)
 			newCtnr.Image = digestImage
@@ -1224,7 +1233,8 @@ func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec typ
 			},
 		},
 	)
-	return err
+
+	return resp, err
 }
 
 // RemoveService removes a service from a managed swarm cluster.

docs/reference/api/docker_remote_api.md

@@ -165,6 +165,7 @@ This section lists each version from latest to oldest.  Each listing includes a
 * `POST /containers/create` now takes `StopTimeout` field.
 * `POST /services/create` and `POST /services/(id or name)/update` now accept `Monitor` and `MaxFailureRatio` parameters, which control the response to failures during service updates.
 * `POST /services/(id or name)/update` now accepts a `ForceUpdate` parameter inside the `TaskTemplate`, which causes the service to be updated even if there are no changes which would ordinarily trigger an update.
+* `POST /services/create` and `POST /services/(id or name)/update` now return a `Warnings` array.
 * `GET /networks/(name)` now returns field `Created` in response to show network created time.
 * `POST /containers/(id or name)/exec` now accepts an `Env` field, which holds a list of environment variables to be set in the context of the command execution.
 * `GET /volumes`, `GET /volumes/(name)`, and `POST /volumes/create` now return the `Options` field which holds the driver specific options to use for when creating the volume.

docs/reference/api/docker_remote_api_v1.25.md

@@ -5262,7 +5262,8 @@ image](#create-an-image) section for more details.
     Content-Type: application/json
 
     {
-      "ID":"ak7w3gjqoa3kuz8xcpnyy0pvl"
+      "ID": "ak7w3gjqoa3kuz8xcpnyy0pvl",
+      "Warnings": ["unable to pin image doesnotexist:latest to digest: image library/doesnotexist:latest not found"]
     }
 
 **Status codes**:
@@ -5628,6 +5629,16 @@ image](#create-an-image) section for more details.
 -   **404** – no such service
 -   **500** – server error
 
+**Example response**:
+
+    HTTP/1.1 200 OK
+    Content-Type: application/json
+
+    {
+      "Warnings": ["unable to pin image doesnotexist:latest to digest: image library/doesnotexist:latest not found"]
+    }
+
+
 ### Get service logs
 
 `GET /services/(id or name)/logs`

hack/generate-swagger-api.sh

@@ -8,7 +8,8 @@ swagger generate model -f api/swagger.yaml \
     -n ImageSummary \
     -n Plugin -n PluginDevice -n PluginMount -n PluginEnv -n PluginInterfaceType \
     -n ErrorResponse \
-    -n IdResponse
+    -n IdResponse \
+    -n ServiceUpdateResponse
 
 swagger generate operation -f api/swagger.yaml \
     -t api -a types -m types -C api/swagger-gen.yaml \