From d4392659f7eefe1e7f7ae3692887aafe7c7cf625 Mon Sep 17 00:00:00 2001
From: Aaron Lehmann <aaron.lehmann@docker.com>
Date: Mon, 14 Nov 2016 18:08:24 -0800
Subject: [PATCH] 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 948e60691e523022f88e7f8129f02106a0f8826c)
Signed-off-by: Victor Vieux <victorvieux@gmail.com>
---
api/server/router/swarm/backend.go | 4 +-
api/server/router/swarm/cluster_routes.go | 11 +++--
api/swagger.yaml | 23 ++++++++--
api/types/client.go | 4 +-
api/types/service_update_response.go | 12 +++++
api/types/volume.go | 2 +-
api/types/volume/volumes_create.go | 2 +-
cli/command/service/create.go | 4 ++
cli/command/service/scale.go | 6 ++-
cli/command/service/update.go | 6 ++-
cli/command/stack/deploy.go | 9 +++-
client/interface.go | 2 +-
client/service_update.go | 11 ++++-
client/service_update_test.go | 6 +--
daemon/cluster/cluster.go | 44 ++++++++++++-------
docs/reference/api/docker_remote_api.md | 1 +
docs/reference/api/docker_remote_api_v1.25.md | 13 +++++-
hack/generate-swagger-api.sh | 3 +-
18 files changed, 120 insertions(+), 43 deletions(-)
create mode 100644 api/types/service_update_response.go
diff --git a/api/server/router/swarm/backend.go b/api/server/router/swarm/backend.go
index c65d705c5c..33840f0d30 100644
--- a/api/server/router/swarm/backend.go
+++ b/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)
diff --git a/api/server/router/swarm/cluster_routes.go b/api/server/router/swarm/cluster_routes.go
index 3ccbe2ba8b..10250da24f 100644
--- a/api/server/router/swarm/cluster_routes.go
+++ b/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 {
diff --git a/api/swagger.yaml b/api/swagger.yaml
index 9a7fca094c..c39c6f67b1 100644
--- a/api/swagger.yaml
+++ b/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"
diff --git a/api/types/client.go b/api/types/client.go
index 67e4446c61..78a000cfad 100644
--- a/api/types/client.go
+++ b/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
diff --git a/api/types/service_update_response.go b/api/types/service_update_response.go
new file mode 100644
index 0000000000..74ea64b1bb
--- /dev/null
+++ b/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"`
+}
diff --git a/api/types/volume.go b/api/types/volume.go
index f2db0cda7b..da4f8ebd9c 100644
--- a/api/types/volume.go
+++ b/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"`
diff --git a/api/types/volume/volumes_create.go b/api/types/volume/volumes_create.go
index 5f9513a227..679c16006f 100644
--- a/api/types/volume/volumes_create.go
+++ b/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"`
diff --git a/cli/command/service/create.go b/cli/command/service/create.go
index 061a36f06c..96c9f36da1 100644
--- a/cli/command/service/create.go
+++ b/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
}
diff --git a/cli/command/service/scale.go b/cli/command/service/scale.go
index ea30265bd7..cf89e90273 100644
--- a/cli/command/service/scale.go
+++ b/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
}
diff --git a/cli/command/service/update.go b/cli/command/service/update.go
index d2639a62db..20a4fc5708 100644
--- a/cli/command/service/update.go
+++ b/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
}
diff --git a/cli/command/stack/deploy.go b/cli/command/stack/deploy.go
index 683f0cad35..63adeacd62 100644
--- a/cli/command/stack/deploy.go
+++ b/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)
diff --git a/client/interface.go b/client/interface.go
index 7a3ebe8b4c..d46720e6c7 100644
--- a/client/interface.go
+++ b/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)
diff --git a/client/service_update.go b/client/service_update.go
index 8e03f7f483..afa94d47e2 100644
--- a/client/service_update.go
+++ b/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
}
diff --git a/client/service_update_test.go b/client/service_update_test.go
index 081649f492..76bea176bf 100644
--- a/client/service_update_test.go
+++ b/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)
}
diff --git a/daemon/cluster/cluster.go b/daemon/cluster/cluster.go
index 87b2e0d671..bcad194930 100644
--- a/daemon/cluster/cluster.go
+++ b/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.
diff --git a/docs/reference/api/docker_remote_api.md b/docs/reference/api/docker_remote_api.md
index c5b17efafe..ef28ab917e 100644
--- a/docs/reference/api/docker_remote_api.md
+++ b/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.
diff --git a/docs/reference/api/docker_remote_api_v1.25.md b/docs/reference/api/docker_remote_api_v1.25.md
index 3728ae7b80..5ab89bd8d2 100644
--- a/docs/reference/api/docker_remote_api_v1.25.md
+++ b/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`
diff --git a/hack/generate-swagger-api.sh b/hack/generate-swagger-api.sh
index d46eaa33de..a8e9f818a7 100755
--- a/hack/generate-swagger-api.sh
+++ b/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 \