Merge pull request #130 from thaJeztah/18.09_backport_fix_api_return_code

[18.09 backport] API: fix status code on conflicting service names
Upstream-commit: 42b58273f68c8240e25f1642e6add6435b894e87
Component: engine

components/engine/integration/internal/swarm/service.go

@@ -66,24 +66,27 @@ type ServiceSpecOpt func(*swarmtypes.ServiceSpec)
 // CreateService creates a service on the passed in swarm daemon.
 func CreateService(t *testing.T, d *daemon.Daemon, opts ...ServiceSpecOpt) string {
 	t.Helper()
-	spec := defaultServiceSpec()
-	for _, o := range opts {
-		o(&spec)
-	}
 
 	client := d.NewClientT(t)
 	defer client.Close()
 
+	spec := CreateServiceSpec(t, opts...)
 	resp, err := client.ServiceCreate(context.Background(), spec, types.ServiceCreateOptions{})
 	assert.NilError(t, err, "error creating service")
 	return resp.ID
 }
 
-func defaultServiceSpec() swarmtypes.ServiceSpec {
+// CreateServiceSpec creates a default service-spec, and applies the provided options
+func CreateServiceSpec(t *testing.T, opts ...ServiceSpecOpt) swarmtypes.ServiceSpec {
+	t.Helper()
 	var spec swarmtypes.ServiceSpec
 	ServiceWithImage("busybox:latest")(&spec)
 	ServiceWithCommand([]string{"/bin/top"})(&spec)
 	ServiceWithReplicas(1)(&spec)
+
+	for _, o := range opts {
+		o(&spec)
+	}
 	return spec
 }
 

components/engine/integration/service/create_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io/ioutil"
+	"net/http"
 	"testing"
 	"time"
 
@@ -14,6 +15,7 @@ import (
 	"github.com/docker/docker/integration/internal/network"
 	"github.com/docker/docker/integration/internal/swarm"
 	"github.com/docker/docker/internal/test/daemon"
+	"github.com/docker/docker/internal/test/request"
 	"gotest.tools/assert"
 	is "gotest.tools/assert/cmp"
 	"gotest.tools/poll"
@@ -122,6 +124,34 @@ func TestCreateServiceMultipleTimes(t *testing.T) {
 	poll.WaitOn(t, networkIsRemoved(client, overlayID), poll.WithTimeout(1*time.Minute), poll.WithDelay(10*time.Second))
 }
 
+func TestCreateServiceConflict(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows")
+	defer setupTest(t)()
+	d := swarm.NewSwarm(t, testEnv)
+	defer d.Stop(t)
+
+	serviceName := "TestService_" + t.Name()
+	serviceSpec := []swarm.ServiceSpecOpt{
+		swarm.ServiceWithName(serviceName),
+	}
+
+	swarm.CreateService(t, d, serviceSpec...)
+
+	spec := swarm.CreateServiceSpec(t, serviceSpec...)
+	res, body, err := request.Post(
+		"/services/create",
+		request.Host(d.Sock()),
+		request.JSONBody(spec),
+		request.JSON,
+	)
+	assert.NilError(t, err)
+	assert.Equal(t, res.StatusCode, http.StatusConflict)
+
+	buf, err := request.ReadBody(body)
+	assert.NilError(t, err)
+	assert.Check(t, is.Contains(string(buf), "service "+serviceName+" already exists"))
+}
+
 func TestCreateWithDuplicateNetworkNames(t *testing.T) {
 	skip.If(t, testEnv.DaemonInfo.OSType == "windows")
 	defer setupTest(t)()

components/engine/vendor.conf

@@ -130,7 +130,7 @@ github.com/containerd/ttrpc 2a805f71863501300ae1976d29f0454ae003e85a
 github.com/gogo/googleapis 08a7655d27152912db7aaf4f983275eaf8d128ef
 
 # cluster
-github.com/docker/swarmkit 6186e40fb04a7681e25a9101dbc7418c37ef0c8b # bump_v18.09 branch
+github.com/docker/swarmkit d61ff7666d7eba47137060c6c8fa8c29aa6198f5 # bump_v18.09 branch
 github.com/gogo/protobuf v1.0.0
 github.com/cloudflare/cfssl 1.3.2
 github.com/fernet/fernet-go 1b2437bc582b3cfbb341ee5a29f8ef5b42912ff2

components/engine/vendor/github.com/docker/swarmkit/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 {