Merge pull request #32122 from allencloud/choose-rpc-code-to-determine-status-code

choose rpc code to determine status code

Dockerfile

@@ -186,7 +186,7 @@ RUN set -x \
 	&& rm -rf "$GOPATH"
 
 # Get the "docker-py" source so we can run their integration tests
-ENV DOCKER_PY_COMMIT 4a08d04aef0595322e1b5ac7c52f28a931da85a5
+ENV DOCKER_PY_COMMIT dc2b24dcdd4ed7c8f6874ee5dd95716761ab6c93
 # To run integration tests docker-pycreds is required.
 # Before running the integration tests conftest.py is
 # loaded which results in loads auth.py that

Dockerfile.aarch64

@@ -142,7 +142,7 @@ RUN set -x \
 	&& rm -rf "$GOPATH"
 
 # Get the "docker-py" source so we can run their integration tests
-ENV DOCKER_PY_COMMIT 4a08d04aef0595322e1b5ac7c52f28a931da85a5
+ENV DOCKER_PY_COMMIT dc2b24dcdd4ed7c8f6874ee5dd95716761ab6c93
 # Before running the integration tests conftest.py is
 # loaded which results in loads auth.py that
 # imports the docker-pycreds module.

Dockerfile.armhf

@@ -137,7 +137,7 @@ RUN set -x \
 	&& rm -rf "$GOPATH"
 
 # Get the "docker-py" source so we can run their integration tests
-ENV DOCKER_PY_COMMIT e2655f658408f9ad1f62abdef3eb6ed43c0cf324
+ENV DOCKER_PY_COMMIT dc2b24dcdd4ed7c8f6874ee5dd95716761ab6c93
 RUN git clone https://github.com/docker/docker-py.git /docker-py \
 	&& cd /docker-py \
 	&& git checkout -q $DOCKER_PY_COMMIT \

Dockerfile.ppc64le

@@ -143,7 +143,7 @@ RUN set -x \
 	&& rm -rf "$GOPATH"
 
 # Get the "docker-py" source so we can run their integration tests
-ENV DOCKER_PY_COMMIT e2655f658408f9ad1f62abdef3eb6ed43c0cf324
+ENV DOCKER_PY_COMMIT dc2b24dcdd4ed7c8f6874ee5dd95716761ab6c93
 RUN git clone https://github.com/docker/docker-py.git /docker-py \
 	&& cd /docker-py \
 	&& git checkout -q $DOCKER_PY_COMMIT \

Dockerfile.s390x

@@ -136,7 +136,7 @@ RUN set -x \
 	&& rm -rf "$GOPATH"
 
 # Get the "docker-py" source so we can run their integration tests
-ENV DOCKER_PY_COMMIT e2655f658408f9ad1f62abdef3eb6ed43c0cf324
+ENV DOCKER_PY_COMMIT dc2b24dcdd4ed7c8f6874ee5dd95716761ab6c93
 RUN git clone https://github.com/docker/docker-py.git /docker-py \
 	&& cd /docker-py \
 	&& git checkout -q $DOCKER_PY_COMMIT \

api/server/httputils/errors.go

@@ -9,6 +9,7 @@ import (
 	"github.com/docker/docker/api/types/versions"
 	"github.com/gorilla/mux"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
 )
 
 // httpStatusError is an interface
@@ -44,11 +45,17 @@ func GetHTTPErrorStatusCode(err error) int {
 	case inputValidationError:
 		statusCode = http.StatusBadRequest
 	default:
+		statusCode = statusCodeFromGRPCError(err)
+		if statusCode != http.StatusInternalServerError {
+			return statusCode
+		}
+
 		// FIXME: this is brittle and should not be necessary, but we still need to identify if
 		// there are errors falling back into this logic.
 		// If we need to differentiate between different possible error types,
 		// we should create appropriate error types that implement the httpStatusError interface.
 		errStr := strings.ToLower(errMsg)
+
 		for _, status := range []struct {
 			keyword string
 			code    int
@@ -102,3 +109,36 @@ func MakeErrorHandler(err error) http.HandlerFunc {
 		}
 	}
 }
+
+// statusCodeFromGRPCError returns status code according to gRPC error
+func statusCodeFromGRPCError(err error) int {
+	switch grpc.Code(err) {
+	case codes.InvalidArgument: // code 3
+		return http.StatusBadRequest
+	case codes.NotFound: // code 5
+		return http.StatusNotFound
+	case codes.AlreadyExists: // code 6
+		return http.StatusConflict
+	case codes.PermissionDenied: // code 7
+		return http.StatusForbidden
+	case codes.FailedPrecondition: // code 9
+		return http.StatusBadRequest
+	case codes.Unauthenticated: // code 16
+		return http.StatusUnauthorized
+	case codes.OutOfRange: // code 11
+		return http.StatusBadRequest
+	case codes.Unimplemented: // code 12
+		return http.StatusNotImplemented
+	case codes.Unavailable: // code 14
+		return http.StatusServiceUnavailable
+	default:
+		// codes.Canceled(1)
+		// codes.Unknown(2)
+		// codes.DeadlineExceeded(4)
+		// codes.ResourceExhausted(8)
+		// codes.Aborted(10)
+		// codes.Internal(13)
+		// codes.DataLoss(15)
+		return http.StatusInternalServerError
+	}
+}

docs/api/version-history.md

@@ -29,6 +29,10 @@ keywords: "API, Docker, rcli, REST, documentation"
  generate and rotate to a new CA certificate/key pair.
 * `POST /service/create` and `POST /services/(id or name)/update` now take the field `Platforms` as part of the service `Placement`, allowing to specify platforms supported by the service.
 * `POST /containers/(name)/wait` now accepts a `condition` query parameter to indicate which state change condition to wait for. Also, response headers are now returned immediately to acknowledge that the server has registered a wait callback for the client.
+* `DELETE /secrets/(name)` now returns status code 404 instead of 500 when the secret does not exist.
+* `POST /secrets/create` now returns status code 409 instead of 500 when creating an already existing secret.
+* `POST /secrets/(name)/update` now returns status code 400 instead of 500 when updating a secret's content which is not the labels.
+* `POST /nodes/(name)/update` now returns status code 400 instead of 500 when demoting last node fails.
 
 ## v1.29 API changes
 

integration-cli/docker_api_swarm_config_test.go

@@ -114,5 +114,5 @@ func (s *DockerSwarmSuite) TestAPISwarmConfigsUpdate(c *check.C) {
 	status, out, err := d.SockRequest("POST", url, config.Spec)
 
 	c.Assert(err, checker.IsNil, check.Commentf(string(out)))
-	c.Assert(status, checker.Equals, http.StatusInternalServerError, check.Commentf("output: %q", string(out)))
+	c.Assert(status, checker.Equals, http.StatusBadRequest, check.Commentf("output: %q", string(out)))
 }

integration-cli/docker_api_swarm_secret_test.go

@@ -23,18 +23,25 @@ func (s *DockerSwarmSuite) TestAPISwarmSecretsCreate(c *check.C) {
 	d := s.AddDaemon(c, true, true)
 
 	testName := "test_secret"
-	id := d.CreateSecret(c, swarm.SecretSpec{
+	secretSpec := swarm.SecretSpec{
 		Annotations: swarm.Annotations{
 			Name: testName,
 		},
 		Data: []byte("TESTINGDATA"),
-	})
+	}
+
+	id := d.CreateSecret(c, secretSpec)
 	c.Assert(id, checker.Not(checker.Equals), "", check.Commentf("secrets: %s", id))
 
 	secrets := d.ListSecrets(c)
 	c.Assert(len(secrets), checker.Equals, 1, check.Commentf("secrets: %#v", secrets))
 	name := secrets[0].Spec.Annotations.Name
 	c.Assert(name, checker.Equals, testName, check.Commentf("secret: %s", name))
+
+	// create an already existing secret, daemon should return a status code of 409
+	status, out, err := d.SockRequest("POST", "/secrets/create", secretSpec)
+	c.Assert(err, checker.IsNil)
+	c.Assert(status, checker.Equals, http.StatusConflict, check.Commentf("secret create: %s", string(out)))
 }
 
 func (s *DockerSwarmSuite) TestAPISwarmSecretsDelete(c *check.C) {
@@ -55,6 +62,13 @@ func (s *DockerSwarmSuite) TestAPISwarmSecretsDelete(c *check.C) {
 	status, out, err := d.SockRequest("GET", "/secrets/"+id, nil)
 	c.Assert(err, checker.IsNil)
 	c.Assert(status, checker.Equals, http.StatusNotFound, check.Commentf("secret delete: %s", string(out)))
+
+	// delete non-existing secret, daemon should return a status code of 404
+	id = "non-existing"
+	status, out, err = d.SockRequest("DELETE", "/secrets/"+id, nil)
+	c.Assert(err, checker.IsNil)
+	c.Assert(status, checker.Equals, http.StatusNotFound, check.Commentf("secret delete: %s", string(out)))
+
 }
 
 func (s *DockerSwarmSuite) TestAPISwarmSecretsUpdate(c *check.C) {
@@ -114,5 +128,5 @@ func (s *DockerSwarmSuite) TestAPISwarmSecretsUpdate(c *check.C) {
 	status, out, err := d.SockRequest("POST", url, secret.Spec)
 
 	c.Assert(err, checker.IsNil, check.Commentf(string(out)))
-	c.Assert(status, checker.Equals, http.StatusInternalServerError, check.Commentf("output: %q", string(out)))
+	c.Assert(status, checker.Equals, http.StatusBadRequest, check.Commentf("output: %q", string(out)))
 }

integration-cli/docker_api_swarm_test.go

@@ -229,7 +229,7 @@ func (s *DockerSwarmSuite) TestAPISwarmPromoteDemote(c *check.C) {
 	url := fmt.Sprintf("/nodes/%s/update?version=%d", node.ID, node.Version.Index)
 	status, out, err := d1.SockRequest("POST", url, node.Spec)
 	c.Assert(err, checker.IsNil)
-	c.Assert(status, checker.Equals, http.StatusInternalServerError, check.Commentf("output: %q", string(out)))
+	c.Assert(status, checker.Equals, http.StatusBadRequest, check.Commentf("output: %q", string(out)))
 	// The warning specific to demoting the last manager is best-effort and
 	// won't appear until the Role field of the demoted manager has been
 	// updated.