Merge pull request #28421 from aaronlehmann/digest-pinning-warnings

Return warnings from service create/update when digest pinning fails

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

@@ -285,10 +285,12 @@ type ServiceCreateOptions struct {
 }
 
 // ServiceCreateResponse contains the information returned to a client
-// on the  creation of a new service.
+// on the creation of a new service.
 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)
+	if err != nil {
+		return response, err
+	}
+
+	err = json.NewDecoder(resp.body).Decode(&response)
 	ensureReaderClosed(resp)
-	return err
+	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

@@ -1059,12 +1059,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()
@@ -1072,17 +1072,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 != "" {
@@ -1096,11 +1096,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
@@ -1109,10 +1113,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.
@@ -1135,12 +1140,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()
@@ -1148,22 +1153,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 != "" {
@@ -1177,14 +1182,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
@@ -1200,11 +1205,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
@@ -1221,7 +1230,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

@@ -171,6 +171,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`

docs/reference/api/docker_remote_api_v1.26.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 \