From 78479b19158290e7338ceb3985bb809b83de5fd4 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Wed, 16 Aug 2023 20:11:10 +0200 Subject: [PATCH] libnet: Make sure network names are unique Fixes #18864, #20648, #33561, #40901. [This GH comment][1] makes clear network name uniqueness has never been enforced due to the eventually consistent nature of Classic Swarm datastores: > there is no guaranteed way to check for duplicates across a cluster of > docker hosts. And this is further confirmed by other comments made by @mrjana in that same issue, eg. [this one][2]: > we want to adopt a schema which can pave the way in the future for a > completely decentralized cluster of docker hosts (if scalability is > needed). This decentralized model is what Classic Swarm was trying to be. It's been superseded since then by Docker Swarm, which has a centralized control plane. To circumvent this drawback, the `NetworkCreate` endpoint accepts a `CheckDuplicate` flag. However it's not perfectly reliable as it won't catch concurrent requests. Due to this design decision, API clients like Compose have to implement workarounds to make sure names are really unique (eg. docker/compose#9585). And the daemon itself has seen a string of issues due to that decision, including some that aren't fixed to this day (for instance moby/moby#40901): > The problem is, that if you specify a network for a container using > the ID, it will add that network to the container but it will then > change it to reference the network by using the name. To summarize, this "feature" is broken, has no practical use and is a source of pain for Docker users and API consumers. So let's just remove it for _all_ API versions. [1]: https://github.com/moby/moby/issues/18864#issuecomment-167201414 [2]: https://github.com/moby/moby/issues/18864#issuecomment-167202589 Signed-off-by: Albin Kerouanton --- api/swagger.yaml | 8 +-- api/types/types.go | 11 +--- client/network_create.go | 5 ++ client/network_create_test.go | 7 +- .../cluster/executor/container/container.go | 13 ++-- daemon/cluster/executor/container/executor.go | 5 +- daemon/network.go | 21 +----- docs/api/version-history.md | 4 ++ hack/make/test-docker-py | 3 +- integration-cli/docker_api_network_test.go | 45 +------------ integration-cli/docker_api_swarm_test.go | 33 ---------- integration-cli/docker_cli_swarm_test.go | 66 ------------------- integration/internal/network/ops.go | 7 -- integration/network/delete_test.go | 4 +- integration/network/inspect_test.go | 1 - integration/network/network_test.go | 18 +++++ integration/network/service_test.go | 1 - integration/service/create_test.go | 53 --------------- libnetwork/controller.go | 11 ++++ 19 files changed, 58 insertions(+), 258 deletions(-) diff --git a/api/swagger.yaml b/api/swagger.yaml index 960ab1f951..27da02af2e 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -9962,13 +9962,7 @@ paths: type: "string" CheckDuplicate: description: | - Check for networks with duplicate names. Since Network is - primarily keyed based on a random ID and not on the name, and - network name is strictly a user-friendly alias to the network - which is uniquely identified using ID, there is no guaranteed - way to check for duplicates. CheckDuplicate is there to provide - a best effort checking of any networks which has the same name - but it is not guaranteed to catch all name collisions. + Deprecated: CheckDuplicate is now always enabled. type: "boolean" Driver: description: "Name of the network driver plugin to use." diff --git a/api/types/types.go b/api/types/types.go index a213f49192..84a85df786 100644 --- a/api/types/types.go +++ b/api/types/types.go @@ -443,14 +443,9 @@ type EndpointResource struct { // NetworkCreate is the expected body of the "create network" http request message type NetworkCreate struct { - // Check for networks with duplicate names. - // Network is primarily keyed based on a random ID and not on the name. - // Network name is strictly a user-friendly alias to the network - // which is uniquely identified using ID. - // And there is no guaranteed way to check for duplicates. - // Option CheckDuplicate is there to provide a best effort checking of any networks - // which has the same name but it is not guaranteed to catch all name collisions. - CheckDuplicate bool + // Deprecated: CheckDuplicate is deprecated since API v1.44, but it defaults to true when sent by the client + // package to older daemons. + CheckDuplicate bool `json:",omitempty"` Driver string Scope string EnableIPv6 bool diff --git a/client/network_create.go b/client/network_create.go index 278d9383a8..d77c1c24aa 100644 --- a/client/network_create.go +++ b/client/network_create.go @@ -5,6 +5,7 @@ import ( "encoding/json" "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/versions" ) // NetworkCreate creates a new network in the docker host. @@ -13,6 +14,10 @@ func (cli *Client) NetworkCreate(ctx context.Context, name string, options types NetworkCreate: options, Name: name, } + if versions.LessThan(cli.version, "1.44") { + networkCreateRequest.CheckDuplicate = true //nolint:staticcheck // ignore SA1019: CheckDuplicate is deprecated since API v1.44. + } + var response types.NetworkCreateResponse serverResp, err := cli.post(ctx, "/networks/create", nil, networkCreateRequest, nil) defer ensureReaderClosed(serverResp) diff --git a/client/network_create_test.go b/client/network_create_test.go index 064a59665b..10abc16e5d 100644 --- a/client/network_create_test.go +++ b/client/network_create_test.go @@ -53,10 +53,9 @@ func TestNetworkCreate(t *testing.T) { } networkResponse, err := client.NetworkCreate(context.Background(), "mynetwork", types.NetworkCreate{ - CheckDuplicate: true, - Driver: "mydriver", - EnableIPv6: true, - Internal: true, + Driver: "mydriver", + EnableIPv6: true, + Internal: true, Options: map[string]string{ "opt-key": "opt-value", }, diff --git a/daemon/cluster/executor/container/container.go b/daemon/cluster/executor/container/container.go index 107b84b412..29d308906f 100644 --- a/daemon/cluster/executor/container/container.go +++ b/daemon/cluster/executor/container/container.go @@ -639,13 +639,12 @@ func (c *containerConfig) networkCreateRequest(name string) (clustertypes.Networ options := types.NetworkCreate{ // ID: na.Network.ID, - Labels: na.Network.Spec.Annotations.Labels, - Internal: na.Network.Spec.Internal, - Attachable: na.Network.Spec.Attachable, - Ingress: convert.IsIngressNetwork(na.Network), - EnableIPv6: na.Network.Spec.Ipv6Enabled, - CheckDuplicate: true, - Scope: scope.Swarm, + Labels: na.Network.Spec.Annotations.Labels, + Internal: na.Network.Spec.Internal, + Attachable: na.Network.Spec.Attachable, + Ingress: convert.IsIngressNetwork(na.Network), + EnableIPv6: na.Network.Spec.Ipv6Enabled, + Scope: scope.Swarm, } if na.Network.Spec.GetNetwork() != "" { diff --git a/daemon/cluster/executor/container/executor.go b/daemon/cluster/executor/container/executor.go index e8325c3ef8..db0a1b6260 100644 --- a/daemon/cluster/executor/container/executor.go +++ b/daemon/cluster/executor/container/executor.go @@ -219,9 +219,8 @@ func (e *executor) Configure(ctx context.Context, node *api.Node) error { IPAM: &network.IPAM{ Driver: ingressNA.Network.IPAM.Driver.Name, }, - Options: ingressNA.Network.DriverState.Options, - Ingress: true, - CheckDuplicate: true, + Options: ingressNA.Network.DriverState.Options, + Ingress: true, } for _, ic := range ingressNA.Network.IPAM.Configs { diff --git a/daemon/network.go b/daemon/network.go index 6e3917e0ff..c5dae5c20c 100644 --- a/daemon/network.go +++ b/daemon/network.go @@ -307,24 +307,6 @@ func (daemon *Daemon) createNetwork(cfg *config.Config, create types.NetworkCrea create.EnableIPv6 = true } - var warning string - nw, err := daemon.GetNetworkByName(create.Name) - if err != nil { - if _, ok := err.(libnetwork.ErrNoSuchNetwork); !ok { - return nil, err - } - } - if nw != nil { - // check if user defined CheckDuplicate, if set true, return err - // otherwise prepare a warning message - if create.CheckDuplicate { - if !agent || nw.Dynamic() { - return nil, libnetwork.NetworkNameError(create.Name) - } - } - warning = fmt.Sprintf("Network with name %s (id : %s) already exists", nw.Name(), nw.ID()) - } - networkOptions := make(map[string]string) for k, v := range create.Options { networkOptions[k] = v @@ -397,8 +379,7 @@ func (daemon *Daemon) createNetwork(cfg *config.Config, create types.NetworkCrea daemon.LogNetworkEvent(n, events.ActionCreate) return &types.NetworkCreateResponse{ - ID: n.ID(), - Warning: warning, + ID: n.ID(), }, nil } diff --git a/docs/api/version-history.md b/docs/api/version-history.md index 082f1c1d8a..23118d04cc 100644 --- a/docs/api/version-history.md +++ b/docs/api/version-history.md @@ -41,6 +41,10 @@ keywords: "API, Docker, rcli, REST, documentation" * `POST /networks/create` now returns a 400 if the `IPAMConfig` has invalid values. Note that this change is _unversioned_ and applied to all API versions on daemon that support version 1.44. +* `POST /networks/create` with a duplicated name now fails systematically. As + such, the `CheckDuplicate` field is now deprecated. Note that this change is + _unversioned_ and applied to all API versions on daemon that support version + 1.44. ## v1.43 API changes diff --git a/hack/make/test-docker-py b/hack/make/test-docker-py index 0c1f41254b..66b51ddd61 100644 --- a/hack/make/test-docker-py +++ b/hack/make/test-docker-py @@ -24,7 +24,8 @@ source hack/make/.integration-test-helpers # TODO re-enable test_connect_with_ipv6_address after we updated to a version of docker-py with https://github.com/docker/docker-py/pull/3169 # TODO re-enable test_create_network_ipv6_enabled after we updated to a version of docker-py with https://github.com/docker/docker-py/pull/3169 # TODO re-enable test_create_with_ipv6_address after we updated to a version of docker-py with https://github.com/docker/docker-py/pull/3169 -: "${PY_TEST_OPTIONS:=--junitxml=${DEST}/junit-report.xml --deselect=tests/integration/api_container_test.py::AttachContainerTest::test_attach_no_stream --deselect=tests/integration/api_container_test.py::AttachContainerTest::test_run_container_reading_socket_ws --deselect=tests/integration/api_container_test.py::CreateContainerTest::test_create_with_restart_policy --deselect=tests/integration/errors_test.py::ErrorsTest::test_api_error_parses_json --deselect=tests/integration/api_network_test.py::TestNetworks::test_connect_with_ipv6_address --deselect=tests/integration/api_network_test.py::TestNetworks::test_create_network_ipv6_enabled --deselect=tests/integration/api_network_test.py::TestNetworks::test_create_with_ipv6_address}" +# TODO re-enable test_create_check_duplicate after we updated to a version of docker-py with https://github.com/docker/docker-py/pull/3170 +: "${PY_TEST_OPTIONS:=--junitxml=${DEST}/junit-report.xml --deselect=tests/integration/api_container_test.py::AttachContainerTest::test_attach_no_stream --deselect=tests/integration/api_container_test.py::AttachContainerTest::test_run_container_reading_socket_ws --deselect=tests/integration/api_container_test.py::CreateContainerTest::test_create_with_restart_policy --deselect=tests/integration/errors_test.py::ErrorsTest::test_api_error_parses_json --deselect=tests/integration/api_network_test.py::TestNetworks::test_connect_with_ipv6_address --deselect=tests/integration/api_network_test.py::TestNetworks::test_create_network_ipv6_enabled --deselect=tests/integration/api_network_test.py::TestNetworks::test_create_with_ipv6_address --deselect=tests/integration/api_network_test.py::TestNetworks::test_create_check_duplicate}" ( bundle .integration-daemon-start diff --git a/integration-cli/docker_api_network_test.go b/integration-cli/docker_api_network_test.go index a485ba2aae..ae8da51851 100644 --- a/integration-cli/docker_api_network_test.go +++ b/integration-cli/docker_api_network_test.go @@ -27,44 +27,6 @@ func (s *DockerAPISuite) TestAPINetworkGetDefaults(c *testing.T) { } } -func (s *DockerAPISuite) TestAPINetworkCreateCheckDuplicate(c *testing.T) { - testRequires(c, DaemonIsLinux) - name := "testcheckduplicate" - configOnCheck := types.NetworkCreateRequest{ - Name: name, - NetworkCreate: types.NetworkCreate{ - CheckDuplicate: true, - }, - } - configNotCheck := types.NetworkCreateRequest{ - Name: name, - NetworkCreate: types.NetworkCreate{ - CheckDuplicate: false, - }, - } - - // Creating a new network first - createNetwork(c, configOnCheck, http.StatusCreated) - assert.Assert(c, isNetworkAvailable(c, name)) - - // Creating another network with same name and CheckDuplicate must fail - isOlderAPI := versions.LessThan(testEnv.DaemonAPIVersion(), "1.34") - expectedStatus := http.StatusConflict - if isOlderAPI { - // In the early test code it uses bool value to represent - // whether createNetwork() is expected to fail or not. - // Therefore, we use negation to handle the same logic after - // the code was changed in https://github.com/moby/moby/pull/35030 - // -http.StatusCreated will also be checked as NOT equal to - // http.StatusCreated in createNetwork() function. - expectedStatus = -http.StatusCreated - } - createNetwork(c, configOnCheck, expectedStatus) - - // Creating another network with same name and not CheckDuplicate must succeed - createNetwork(c, configNotCheck, http.StatusCreated) -} - func (s *DockerAPISuite) TestAPINetworkFilter(c *testing.T) { testRequires(c, DaemonIsLinux) nr := getNetworkResource(c, getNetworkIDByName(c, "bridge")) @@ -248,12 +210,7 @@ func (s *DockerAPISuite) TestAPICreateDeletePredefinedNetworks(c *testing.T) { func createDeletePredefinedNetwork(c *testing.T, name string) { // Create pre-defined network - config := types.NetworkCreateRequest{ - Name: name, - NetworkCreate: types.NetworkCreate{ - CheckDuplicate: true, - }, - } + config := types.NetworkCreateRequest{Name: name} expectedStatus := http.StatusForbidden if versions.LessThan(testEnv.DaemonAPIVersion(), "1.34") { // In the early test code it uses bool value to represent diff --git a/integration-cli/docker_api_swarm_test.go b/integration-cli/docker_api_swarm_test.go index c7be7dd4da..7cae8441c2 100644 --- a/integration-cli/docker_api_swarm_test.go +++ b/integration-cli/docker_api_swarm_test.go @@ -917,39 +917,6 @@ func (s *DockerSwarmSuite) TestAPISwarmErrorHandling(c *testing.T) { assert.ErrorContains(c, err, "address already in use") } -// Test case for 30242, where duplicate networks, with different drivers `bridge` and `overlay`, -// caused both scopes to be `swarm` for `docker network inspect` and `docker network ls`. -// This test makes sure the fixes correctly output scopes instead. -func (s *DockerSwarmSuite) TestAPIDuplicateNetworks(c *testing.T) { - ctx := testutil.GetContext(c) - d := s.AddDaemon(ctx, c, true, true) - cli := d.NewClientT(c) - defer cli.Close() - - name := "foo" - networkCreate := types.NetworkCreate{ - CheckDuplicate: false, - } - - networkCreate.Driver = "bridge" - - n1, err := cli.NetworkCreate(testutil.GetContext(c), name, networkCreate) - assert.NilError(c, err) - - networkCreate.Driver = "overlay" - - n2, err := cli.NetworkCreate(testutil.GetContext(c), name, networkCreate) - assert.NilError(c, err) - - r1, err := cli.NetworkInspect(testutil.GetContext(c), n1.ID, types.NetworkInspectOptions{}) - assert.NilError(c, err) - assert.Equal(c, r1.Scope, "local") - - r2, err := cli.NetworkInspect(testutil.GetContext(c), n2.ID, types.NetworkInspectOptions{}) - assert.NilError(c, err) - assert.Equal(c, r2.Scope, "swarm") -} - // Test case for 30178 func (s *DockerSwarmSuite) TestAPISwarmHealthcheckNone(c *testing.T) { // Issue #36386 can be a independent one, which is worth further investigation. diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go index 856ef76c03..1daf4a52ee 100644 --- a/integration-cli/docker_cli_swarm_test.go +++ b/integration-cli/docker_cli_swarm_test.go @@ -18,7 +18,6 @@ import ( "time" "github.com/cloudflare/cfssl/helpers" - "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/integration-cli/checker" "github.com/docker/docker/integration-cli/cli" @@ -1656,71 +1655,6 @@ func (s *DockerSwarmSuite) TestSwarmReadonlyRootfs(c *testing.T) { assert.Equal(c, strings.TrimSpace(out), "true") } -func (s *DockerSwarmSuite) TestNetworkInspectWithDuplicateNames(c *testing.T) { - ctx := testutil.GetContext(c) - d := s.AddDaemon(ctx, c, true, true) - - name := "foo" - options := types.NetworkCreate{ - CheckDuplicate: false, - Driver: "bridge", - } - - cli := d.NewClientT(c) - defer cli.Close() - - n1, err := cli.NetworkCreate(testutil.GetContext(c), name, options) - assert.NilError(c, err) - - // Full ID always works - out, err := d.Cmd("network", "inspect", "--format", "{{.ID}}", n1.ID) - assert.NilError(c, err, out) - assert.Equal(c, strings.TrimSpace(out), n1.ID) - - // Name works if it is unique - out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", name) - assert.NilError(c, err, out) - assert.Equal(c, strings.TrimSpace(out), n1.ID) - - n2, err := cli.NetworkCreate(testutil.GetContext(c), name, options) - assert.NilError(c, err) - // Full ID always works - out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", n1.ID) - assert.NilError(c, err, out) - assert.Equal(c, strings.TrimSpace(out), n1.ID) - - out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", n2.ID) - assert.NilError(c, err, out) - assert.Equal(c, strings.TrimSpace(out), n2.ID) - - // Name with duplicates - out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", name) - assert.ErrorContains(c, err, "", out) - assert.Assert(c, strings.Contains(out, "2 matches found based on name"), out) - out, err = d.Cmd("network", "rm", n2.ID) - assert.NilError(c, err, out) - - // Duplicates with name but with different driver - options.Driver = "overlay" - - n2, err = cli.NetworkCreate(testutil.GetContext(c), name, options) - assert.NilError(c, err) - - // Full ID always works - out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", n1.ID) - assert.NilError(c, err, out) - assert.Equal(c, strings.TrimSpace(out), n1.ID) - - out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", n2.ID) - assert.NilError(c, err, out) - assert.Equal(c, strings.TrimSpace(out), n2.ID) - - // Name with duplicates - out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", name) - assert.ErrorContains(c, err, "", out) - assert.Assert(c, strings.Contains(out, "2 matches found based on name"), out) -} - func (s *DockerSwarmSuite) TestSwarmStopSignal(c *testing.T) { ctx := testutil.GetContext(c) testRequires(c, DaemonIsLinux, UserNamespaceROMount) diff --git a/integration/internal/network/ops.go b/integration/internal/network/ops.go index 33bd3d05f3..dc0fce06ef 100644 --- a/integration/internal/network/ops.go +++ b/integration/internal/network/ops.go @@ -19,13 +19,6 @@ func WithIPv6() func(*types.NetworkCreate) { } } -// WithCheckDuplicate sets the CheckDuplicate field on create network request -func WithCheckDuplicate() func(*types.NetworkCreate) { - return func(n *types.NetworkCreate) { - n.CheckDuplicate = true - } -} - // WithInternal enables Internal flag on the create network request func WithInternal() func(*types.NetworkCreate) { return func(n *types.NetworkCreate) { diff --git a/integration/network/delete_test.go b/integration/network/delete_test.go index 0fcabb919b..b5dd58309f 100644 --- a/integration/network/delete_test.go +++ b/integration/network/delete_test.go @@ -48,9 +48,7 @@ func TestNetworkCreateDelete(t *testing.T) { client := testEnv.APIClient() netName := "testnetwork_" + t.Name() - network.CreateNoError(ctx, t, client, netName, - network.WithCheckDuplicate(), - ) + network.CreateNoError(ctx, t, client, netName) assert.Check(t, IsNetworkAvailable(ctx, client, netName)) // delete the network and make sure it is deleted diff --git a/integration/network/inspect_test.go b/integration/network/inspect_test.go index 136eec508f..06eda3a89f 100644 --- a/integration/network/inspect_test.go +++ b/integration/network/inspect_test.go @@ -25,7 +25,6 @@ func TestInspectNetwork(t *testing.T) { networkName := "Overlay" + t.Name() overlayID := network.CreateNoError(ctx, t, c, networkName, network.WithDriver("overlay"), - network.WithCheckDuplicate(), ) var instances uint64 = 2 diff --git a/integration/network/network_test.go b/integration/network/network_test.go index 50ff58132e..4e6687d4df 100644 --- a/integration/network/network_test.go +++ b/integration/network/network_test.go @@ -258,3 +258,21 @@ func TestDefaultNetworkOpts(t *testing.T) { }) } } + +func TestForbidDuplicateNetworkNames(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType == "windows") + + ctx := testutil.StartSpan(baseContext, t) + + d := daemon.New(t) + d.StartWithBusybox(ctx, t) + defer d.Stop(t) + + c := d.NewClientT(t) + defer c.Close() + + network.CreateNoError(ctx, t, c, "testnet") + + _, err := c.NetworkCreate(ctx, "testnet", types.NetworkCreate{}) + assert.Error(t, err, "Error response from daemon: network with name testnet already exists", "2nd NetworkCreate call should have failed") +} diff --git a/integration/network/service_test.go b/integration/network/service_test.go index 5231ecf16e..54060ffae9 100644 --- a/integration/network/service_test.go +++ b/integration/network/service_test.go @@ -427,7 +427,6 @@ func TestServiceWithDefaultAddressPoolInit(t *testing.T) { name := "sthira" + t.Name() overlayID := network.CreateNoError(ctx, t, cli, name, network.WithDriver("overlay"), - network.WithCheckDuplicate(), ) var instances uint64 = 1 diff --git a/integration/service/create_test.go b/integration/service/create_test.go index 4d70361e3b..877dbb0284 100644 --- a/integration/service/create_test.go +++ b/integration/service/create_test.go @@ -88,7 +88,6 @@ func TestCreateServiceMultipleTimes(t *testing.T) { overlayName := "overlay1_" + t.Name() overlayID := network.CreateNoError(ctx, t, client, overlayName, - network.WithCheckDuplicate(), network.WithDriver("overlay"), ) @@ -193,58 +192,6 @@ func TestCreateServiceMaxReplicas(t *testing.T) { assert.NilError(t, err) } -func TestCreateWithDuplicateNetworkNames(t *testing.T) { - skip.If(t, testEnv.DaemonInfo.OSType == "windows") - ctx := setupTest(t) - d := swarm.NewSwarm(ctx, t, testEnv) - defer d.Stop(t) - client := d.NewClientT(t) - defer client.Close() - - name := "foo_" + t.Name() - n1 := network.CreateNoError(ctx, t, client, name, network.WithDriver("bridge")) - n2 := network.CreateNoError(ctx, t, client, name, network.WithDriver("bridge")) - - // Duplicates with name but with different driver - n3 := network.CreateNoError(ctx, t, client, name, network.WithDriver("overlay")) - - // Create Service with the same name - var instances uint64 = 1 - - serviceName := "top_" + t.Name() - serviceID := swarm.CreateService(ctx, t, d, - swarm.ServiceWithReplicas(instances), - swarm.ServiceWithName(serviceName), - swarm.ServiceWithNetwork(name), - ) - - poll.WaitOn(t, swarm.RunningTasksCount(ctx, client, serviceID, instances), swarm.ServicePoll) - - resp, _, err := client.ServiceInspectWithRaw(ctx, serviceID, types.ServiceInspectOptions{}) - assert.NilError(t, err) - assert.Check(t, is.Equal(n3, resp.Spec.TaskTemplate.Networks[0].Target)) - - // Remove Service, and wait for its tasks to be removed - err = client.ServiceRemove(ctx, serviceID) - assert.NilError(t, err) - poll.WaitOn(t, swarm.NoTasksForService(ctx, client, serviceID), swarm.ServicePoll) - - // Remove networks - err = client.NetworkRemove(ctx, n3) - assert.NilError(t, err) - - err = client.NetworkRemove(ctx, n2) - assert.NilError(t, err) - - err = client.NetworkRemove(ctx, n1) - assert.NilError(t, err) - - // Make sure networks have been destroyed. - poll.WaitOn(t, network.IsRemoved(ctx, client, n3), poll.WithTimeout(1*time.Minute), poll.WithDelay(10*time.Second)) - poll.WaitOn(t, network.IsRemoved(ctx, client, n2), poll.WithTimeout(1*time.Minute), poll.WithDelay(10*time.Second)) - poll.WaitOn(t, network.IsRemoved(ctx, client, n1), poll.WithTimeout(1*time.Minute), poll.WithDelay(10*time.Second)) -} - func TestCreateServiceSecretFileMode(t *testing.T) { skip.If(t, testEnv.DaemonInfo.OSType == "windows") ctx := setupTest(t) diff --git a/libnetwork/controller.go b/libnetwork/controller.go index e4727b3f0f..80b349ee5d 100644 --- a/libnetwork/controller.go +++ b/libnetwork/controller.go @@ -475,6 +475,17 @@ func (c *Controller) NewNetwork(networkType, name string, id string, options ... return nil, ErrInvalidName(name) } + // Make sure two concurrent calls to this method won't create conflicting + // networks, otherwise libnetwork will end up in an invalid state. + if name != "" { + c.networkLocker.Lock(name) + defer c.networkLocker.Unlock(name) + + if _, err := c.NetworkByName(name); err == nil { + return nil, NetworkNameError(name) + } + } + if id == "" { id = stringid.GenerateRandomID() }