From 890f4c7aa550053b08a946605c0bfcd8b62725f8 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 19 Dec 2016 18:31:45 +0100 Subject: [PATCH] Merge pull request #26576 from allencloud/change-cluster-response-status-code update response status code for cluster request (cherry picked from commit a58b5830b175860e71174c93c6f3ec1cefcd5e0d) Signed-off-by: Sebastiaan van Stijn --- api/server/router/swarm/cluster_routes.go | 17 ++-- api/swagger.yaml | 111 +++++++++++++++++++--- daemon/cluster/cluster.go | 14 +-- daemon/cluster/helpers.go | 12 ++- integration-cli/docker_api_swarm_test.go | 4 +- 5 files changed, 125 insertions(+), 33 deletions(-) diff --git a/api/server/router/swarm/cluster_routes.go b/api/server/router/swarm/cluster_routes.go index fe976434bc..59420fe905 100644 --- a/api/server/router/swarm/cluster_routes.go +++ b/api/server/router/swarm/cluster_routes.go @@ -65,7 +65,8 @@ func (sr *swarmRouter) updateCluster(ctx context.Context, w http.ResponseWriter, rawVersion := r.URL.Query().Get("version") version, err := strconv.ParseUint(rawVersion, 10, 64) if err != nil { - return fmt.Errorf("Invalid swarm version '%s': %s", rawVersion, err.Error()) + err := fmt.Errorf("invalid swarm version '%s': %v", rawVersion, err) + return errors.NewBadRequestError(err) } var flags types.UpdateFlags @@ -73,7 +74,8 @@ func (sr *swarmRouter) updateCluster(ctx context.Context, w http.ResponseWriter, if value := r.URL.Query().Get("rotateWorkerToken"); value != "" { rot, err := strconv.ParseBool(value) if err != nil { - return fmt.Errorf("invalid value for rotateWorkerToken: %s", value) + err := fmt.Errorf("invalid value for rotateWorkerToken: %s", value) + return errors.NewBadRequestError(err) } flags.RotateWorkerToken = rot @@ -82,7 +84,8 @@ func (sr *swarmRouter) updateCluster(ctx context.Context, w http.ResponseWriter, if value := r.URL.Query().Get("rotateManagerToken"); value != "" { rot, err := strconv.ParseBool(value) if err != nil { - return fmt.Errorf("invalid value for rotateManagerToken: %s", value) + err := fmt.Errorf("invalid value for rotateManagerToken: %s", value) + return errors.NewBadRequestError(err) } flags.RotateManagerToken = rot @@ -91,7 +94,7 @@ func (sr *swarmRouter) updateCluster(ctx context.Context, w http.ResponseWriter, if value := r.URL.Query().Get("rotateManagerUnlockKey"); value != "" { rot, err := strconv.ParseBool(value) if err != nil { - return fmt.Errorf("invalid value for rotateManagerUnlockKey: %s", value) + return errors.NewBadRequestError(fmt.Errorf("invalid value for rotateManagerUnlockKey: %s", value)) } flags.RotateManagerUnlockKey = rot @@ -184,7 +187,8 @@ func (sr *swarmRouter) updateService(ctx context.Context, w http.ResponseWriter, rawVersion := r.URL.Query().Get("version") version, err := strconv.ParseUint(rawVersion, 10, 64) if err != nil { - return fmt.Errorf("Invalid service version '%s': %s", rawVersion, err.Error()) + err := fmt.Errorf("invalid service version '%s': %v", rawVersion, err) + return errors.NewBadRequestError(err) } // Get returns "" if the header does not exist @@ -294,7 +298,8 @@ func (sr *swarmRouter) updateNode(ctx context.Context, w http.ResponseWriter, r rawVersion := r.URL.Query().Get("version") version, err := strconv.ParseUint(rawVersion, 10, 64) if err != nil { - return fmt.Errorf("Invalid node version '%s': %s", rawVersion, err.Error()) + err := fmt.Errorf("invalid node version '%s': %v", rawVersion, err) + return errors.NewBadRequestError(err) } if err := sr.backend.UpdateNode(vars["id"], version, node); err != nil { diff --git a/api/swagger.yaml b/api/swagger.yaml index d19e8c9ca8..b5bd37dd00 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -6746,10 +6746,22 @@ paths: type: "array" items: $ref: "#/definitions/Node" + 400: + description: "bad parameter" + schema: + $ref: "#/definitions/ErrorResponse" + 404: + description: "no such node" + schema: + $ref: "#/definitions/ErrorResponse" 500: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "filters" in: "query" @@ -6781,6 +6793,10 @@ paths: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "id" in: "path" @@ -6802,6 +6818,10 @@ paths: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "id" in: "path" @@ -6829,6 +6849,10 @@ paths: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "id" in: "path" @@ -6893,10 +6917,18 @@ paths: UpdatedAt: "2016-08-15T16:32:09.623207604Z" Version: Index: 51 + 404: + description: "no such swarm" + schema: + $ref: "#/definitions/ErrorResponse" 500: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" tags: ["Swarm"] /swarm/init: post: @@ -6916,14 +6948,14 @@ paths: description: "bad parameter" schema: $ref: "#/definitions/ErrorResponse" - 406: - description: "node is already part of a swarm" - schema: - $ref: "#/definitions/ErrorResponse" 500: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is already part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "body" in: "body" @@ -7089,6 +7121,10 @@ paths: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" tags: ["Swarm"] /swarm/unlock: post: @@ -7117,6 +7153,10 @@ paths: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" tags: ["Swarm"] /services: get: @@ -7133,6 +7173,10 @@ paths: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "filters" in: "query" @@ -7167,6 +7211,10 @@ paths: example: ID: "ak7w3gjqoa3kuz8xcpnyy0pvl" Warning: "unable to pin image doesnotexist:latest to digest: image library/doesnotexist:latest not found" + 400: + description: "bad parameter" + schema: + $ref: "#/definitions/ErrorResponse" 403: description: "network is not eligible for services" schema: @@ -7180,7 +7228,7 @@ paths: schema: $ref: "#/definitions/ErrorResponse" 503: - description: "server error or node is not part of a swarm" + description: "node is not part of a swarm" schema: $ref: "#/definitions/ErrorResponse" parameters: @@ -7262,6 +7310,10 @@ paths: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "id" in: "path" @@ -7283,6 +7335,10 @@ paths: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "id" in: "path" @@ -7301,6 +7357,10 @@ paths: description: "no error" schema: $ref: "#/definitions/ImageDeleteResponse" + 400: + description: "bad parameter" + schema: + $ref: "#/definitions/ErrorResponse" 404: description: "no such service" schema: @@ -7309,6 +7369,10 @@ paths: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "id" in: "path" @@ -7397,6 +7461,10 @@ paths: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "id" in: "path" @@ -7566,11 +7634,14 @@ paths: Gateway: "10.255.0.1" Addresses: - "10.255.0.5/16" - 500: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "filters" in: "query" @@ -7604,6 +7675,10 @@ paths: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "id" in: "path" @@ -7636,6 +7711,10 @@ paths: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "filters" in: "query" @@ -7664,10 +7743,6 @@ paths: type: "string" example: ID: "ktnbjxoalbkvbvedmg1urrz8h" - 406: - description: "server error or node is not part of a swarm" - schema: - $ref: "#/definitions/ErrorResponse" 409: description: "name conflicts with an existing object" schema: @@ -7676,6 +7751,10 @@ paths: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "body" in: "body" @@ -7712,14 +7791,14 @@ paths: description: "secret not found" schema: $ref: "#/definitions/ErrorResponse" - 406: - description: "node is not part of a swarm" - schema: - $ref: "#/definitions/ErrorResponse" 500: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "id" in: "path" @@ -7743,6 +7822,10 @@ paths: description: "server error" schema: $ref: "#/definitions/ErrorResponse" + 503: + description: "node is not part of a swarm" + schema: + $ref: "#/definitions/ErrorResponse" parameters: - name: "id" in: "path" diff --git a/daemon/cluster/cluster.go b/daemon/cluster/cluster.go index 4af035b523..dd45357105 100644 --- a/daemon/cluster/cluster.go +++ b/daemon/cluster/cluster.go @@ -419,7 +419,7 @@ func (c *Cluster) Init(req types.InitRequest) (string, error) { if err := validateAndSanitizeInitRequest(&req); err != nil { c.Unlock() - return "", err + return "", apierrors.NewBadRequestError(err) } listenHost, listenPort, err := resolveListenAddr(req.ListenAddr) @@ -506,7 +506,7 @@ func (c *Cluster) Join(req types.JoinRequest) error { } if err := validateAndSanitizeJoinRequest(&req); err != nil { c.Unlock() - return err + return apierrors.NewBadRequestError(err) } listenHost, listenPort, err := resolveListenAddr(req.ListenAddr) @@ -803,7 +803,7 @@ func (c *Cluster) Update(version uint64, spec types.Spec, flags types.UpdateFlag // will be used to swarmkit. clusterSpec, err := convert.SwarmSpecToGRPC(spec) if err != nil { - return err + return apierrors.NewBadRequestError(err) } _, err = c.client.UpdateCluster( @@ -1085,7 +1085,7 @@ func (c *Cluster) CreateService(s types.ServiceSpec, encodedAuth string) (*apity serviceSpec, err := convert.ServiceSpecToGRPC(s) if err != nil { - return nil, err + return nil, apierrors.NewBadRequestError(err) } ctnr := serviceSpec.Task.GetContainer() @@ -1168,7 +1168,7 @@ func (c *Cluster) UpdateService(serviceIDOrName string, version uint64, spec typ serviceSpec, err := convert.ServiceSpecToGRPC(spec) if err != nil { - return nil, err + return nil, apierrors.NewBadRequestError(err) } currentService, err := getService(ctx, c.client, serviceIDOrName) @@ -1383,7 +1383,7 @@ func (c *Cluster) GetNodes(options apitypes.NodeListOptions) ([]types.Node, erro return nodes, nil } -// GetNode returns a node based on an ID or name. +// GetNode returns a node based on an ID. func (c *Cluster) GetNode(input string) (types.Node, error) { c.RLock() defer c.RUnlock() @@ -1413,7 +1413,7 @@ func (c *Cluster) UpdateNode(input string, version uint64, spec types.NodeSpec) nodeSpec, err := convert.NodeSpecToGRPC(spec) if err != nil { - return err + return apierrors.NewBadRequestError(err) } ctx, cancel := c.getRequestContext() diff --git a/daemon/cluster/helpers.go b/daemon/cluster/helpers.go index be5bf56e87..09cf1adcf2 100644 --- a/daemon/cluster/helpers.go +++ b/daemon/cluster/helpers.go @@ -3,6 +3,7 @@ package cluster import ( "fmt" + "github.com/docker/docker/api/errors" swarmapi "github.com/docker/swarmkit/api" "golang.org/x/net/context" ) @@ -14,7 +15,7 @@ func getSwarm(ctx context.Context, c swarmapi.ControlClient) (*swarmapi.Cluster, } if len(rl.Clusters) == 0 { - return nil, fmt.Errorf("swarm not found") + return nil, errors.NewRequestNotFoundError(errNoSwarm) } // TODO: assume one cluster only @@ -38,7 +39,8 @@ func getNode(ctx context.Context, c swarmapi.ControlClient, input string) (*swar } if len(rl.Nodes) == 0 { - return nil, fmt.Errorf("node %s not found", input) + err := fmt.Errorf("node %s not found", input) + return nil, errors.NewRequestNotFoundError(err) } if l := len(rl.Nodes); l > 1 { @@ -66,7 +68,8 @@ func getService(ctx context.Context, c swarmapi.ControlClient, input string) (*s } if len(rl.Services) == 0 { - return nil, fmt.Errorf("service %s not found", input) + err := fmt.Errorf("service %s not found", input) + return nil, errors.NewRequestNotFoundError(err) } if l := len(rl.Services); l > 1 { @@ -95,7 +98,8 @@ func getTask(ctx context.Context, c swarmapi.ControlClient, input string) (*swar } if len(rl.Tasks) == 0 { - return nil, fmt.Errorf("task %s not found", input) + err := fmt.Errorf("task %s not found", input) + return nil, errors.NewRequestNotFoundError(err) } if l := len(rl.Tasks); l > 1 { diff --git a/integration-cli/docker_api_swarm_test.go b/integration-cli/docker_api_swarm_test.go index 1f8eaec6de..33bd28c5c6 100644 --- a/integration-cli/docker_api_swarm_test.go +++ b/integration-cli/docker_api_swarm_test.go @@ -966,7 +966,7 @@ func (s *DockerSwarmSuite) TestAPISwarmInvalidAddress(c *check.C) { } status, _, err := d.SockRequest("POST", "/swarm/init", req) c.Assert(err, checker.IsNil) - c.Assert(status, checker.Equals, http.StatusInternalServerError) + c.Assert(status, checker.Equals, http.StatusBadRequest) req2 := swarm.JoinRequest{ ListenAddr: "0.0.0.0:2377", @@ -974,7 +974,7 @@ func (s *DockerSwarmSuite) TestAPISwarmInvalidAddress(c *check.C) { } status, _, err = d.SockRequest("POST", "/swarm/join", req2) c.Assert(err, checker.IsNil) - c.Assert(status, checker.Equals, http.StatusInternalServerError) + c.Assert(status, checker.Equals, http.StatusBadRequest) } func (s *DockerSwarmSuite) TestAPISwarmForceNewCluster(c *check.C) {