Merge pull request #2779 from thaJeztah/fix_api_error_types

Return correct error-codes on conflicting names
moby · Nov 6, 2018 · 6931952 · 6931952
2 parents bc032e2 + 2061af7
commit 6931952
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 6 deletions.
diff --git a/manager/controlapi/service.go b/manager/controlapi/service.go
@@ -680,13 +680,14 @@ func (s *Server) CreateService(ctx context.Context, request *api.CreateServiceRe
 
 		return store.CreateService(tx, service)
 	})
-	if err != nil {
+	switch err {
+	case store.ErrNameConflict:
+		return nil, status.Errorf(codes.AlreadyExists, "service %s already exists", request.Spec.Annotations.Name)
+	case nil:
+		return &api.CreateServiceResponse{Service: service}, nil
+	default:
 		return nil, err
 	}
-
-	return &api.CreateServiceResponse{
-		Service: service,
-	}, nil
 }
 
 // GetService returns a Service given a ServiceID.
@@ -896,7 +897,12 @@ func (s *Server) ListServices(ctx context.Context, request *api.ListServicesRequ
 		}
 	})
 	if err != nil {
-		return nil, err
+		switch err {
+		case store.ErrInvalidFindBy:
+			return nil, status.Errorf(codes.InvalidArgument, err.Error())
+		default:
+			return nil, err
+		}
 	}
 
 	if request.Filters != nil {

diff --git a/manager/controlapi/service_test.go b/manager/controlapi/service_test.go
@@ -615,6 +615,14 @@ func TestCreateService(t *testing.T) {
 	_, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec})
 	assert.Error(t, err)
 	assert.Equal(t, codes.InvalidArgument, testutils.ErrorCode(err))
+
+	spec = createSpec("notunique", "image", 1)
+	_, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec})
+	assert.NoError(t, err)
+
+	r, err = ts.Client.CreateService(context.Background(), &api.CreateServiceRequest{Spec: spec})
+	assert.Error(t, err)
+	assert.Equal(t, codes.AlreadyExists, testutils.ErrorCode(err))
 }
 
 func TestSecretValidation(t *testing.T) {
@@ -870,6 +878,16 @@ func TestUpdateService(t *testing.T) {
 	})
 	assert.Error(t, err)
 
+	// Attempt to update service name; renaming is not implemented
+	r.Service.Spec.Annotations.Name = "newname"
+	_, err = ts.Client.UpdateService(context.Background(), &api.UpdateServiceRequest{
+		ServiceID:      service.ID,
+		Spec:           &r.Service.Spec,
+		ServiceVersion: version,
+	})
+	assert.Error(t, err)
+	assert.Equal(t, codes.Unimplemented, testutils.ErrorCode(err))
+
 	// test port conflicts
 	spec2 := createSpec("name2", "image", 1)
 	spec2.Endpoint = &api.EndpointSpec{Ports: []*api.PortConfig{