From c99df606e60aa3599927e756b25c2bdc2083d816 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 10 Dec 2018 12:18:32 +0100 Subject: [PATCH 1/2] Update swarmkit to return correct error-codes on conflicting names This updates the swarmkit vendoring to the latest version in the bump_v18.09 branch Signed-off-by: Sebastiaan van Stijn Upstream-commit: ad7105260f3c2ff32a375ff78dce9a96e01d87cb Component: engine --- components/engine/vendor.conf | 2 +- .../swarmkit/manager/controlapi/service.go | 18 ++++++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/components/engine/vendor.conf b/components/engine/vendor.conf index 26e29e73e4..6b9de70b64 100644 --- a/components/engine/vendor.conf +++ b/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 diff --git a/components/engine/vendor/github.com/docker/swarmkit/manager/controlapi/service.go b/components/engine/vendor/github.com/docker/swarmkit/manager/controlapi/service.go index 17bc2500e2..06141d281b 100644 --- a/components/engine/vendor/github.com/docker/swarmkit/manager/controlapi/service.go +++ b/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 { From 6155a653aa3b29b9ccae118ebe199561a34b8156 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 5 Nov 2018 15:59:26 +0100 Subject: [PATCH 2/2] Add test for status code on conflicting service names Signed-off-by: Sebastiaan van Stijn (cherry picked from commit b0de11cf3018b482dfcf8c873261dc11c8fce0b0) Signed-off-by: Sebastiaan van Stijn Upstream-commit: a69626afb12eb9ec3e374aa563b561c0ba28f27f Component: engine --- .../integration/internal/swarm/service.go | 13 ++++---- .../engine/integration/service/create_test.go | 30 +++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/components/engine/integration/internal/swarm/service.go b/components/engine/integration/internal/swarm/service.go index 5b9c5a97a8..9591a7fe90 100644 --- a/components/engine/integration/internal/swarm/service.go +++ b/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 } diff --git a/components/engine/integration/service/create_test.go b/components/engine/integration/service/create_test.go index 1d15be176e..aceb398032 100644 --- a/components/engine/integration/service/create_test.go +++ b/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)()