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/4] 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/4] 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)() From 50be23f5a192af11b352d9bbefab091cf835ee54 Mon Sep 17 00:00:00 2001 From: Lifubang Date: Mon, 24 Sep 2018 12:17:31 +0800 Subject: [PATCH 3/4] fixes display text in Multiple IDs found with provided prefix Signed-off-by: Lifubang (cherry picked from commit 00eb3480dc4ceb6034f8f7463ff41d2e87fb5dcc) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 1043f40fb561ffbf23fbdde9989abcebd8e48279 Component: engine --- components/engine/pkg/truncindex/truncindex.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/pkg/truncindex/truncindex.go b/components/engine/pkg/truncindex/truncindex.go index d5c840cf13..b7a11a38c0 100644 --- a/components/engine/pkg/truncindex/truncindex.go +++ b/components/engine/pkg/truncindex/truncindex.go @@ -108,7 +108,7 @@ func (idx *TruncIndex) Get(s string) (string, error) { if id != "" { // we haven't found the ID if there are two or more IDs id = "" - return ErrAmbiguousPrefix{prefix: string(prefix)} + return ErrAmbiguousPrefix{prefix: s} } id = string(prefix) return nil From 79cbff3cd31548c8a1572b5d7b370add3885c753 Mon Sep 17 00:00:00 2001 From: "Iskander (Alex) Sharipov" Date: Tue, 11 Dec 2018 16:33:23 +0300 Subject: [PATCH 4/4] registry: use len(via)!=0 instead of via!=nil This avoids the corner case where `via` is not nil, but has a length of 0, so the updated code does not panic in that situation. Signed-off-by: Iskander Sharipov (cherry picked from commit a5c185b99404ea3fbab47ff9d7ba143392566bc1) Signed-off-by: Sebastiaan van Stijn Upstream-commit: 3482a3b14a6414977bd9860c513abf06dedd6bf7 Component: engine --- components/engine/registry/registry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/registry/registry.go b/components/engine/registry/registry.go index 7a84bbfb7e..6727b7dc32 100644 --- a/components/engine/registry/registry.go +++ b/components/engine/registry/registry.go @@ -145,7 +145,7 @@ func trustedLocation(req *http.Request) bool { // addRequiredHeadersToRedirectedRequests adds the necessary redirection headers // for redirected requests func addRequiredHeadersToRedirectedRequests(req *http.Request, via []*http.Request) error { - if via != nil && via[0] != nil { + if len(via) != 0 && via[0] != nil { if trustedLocation(req) && trustedLocation(via[0]) { req.Header = via[0].Header return nil