From 91820b69412e2ec9e5b3201cd5c637ca1352fd39 Mon Sep 17 00:00:00 2001 From: Alessandro Boch Date: Wed, 15 Feb 2017 19:53:51 -0800 Subject: [PATCH] Release the network attachment on allocation failure - otherwise the attachment task will stay in store and consume IP addresses and there is no way to remove it. Signed-off-by: Alessandro Boch --- daemon/cluster/networks.go | 15 ++++++++++++++- integration-cli/docker_cli_swarm_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/daemon/cluster/networks.go b/daemon/cluster/networks.go index 69b34a0394..154fe4b704 100644 --- a/daemon/cluster/networks.go +++ b/daemon/cluster/networks.go @@ -176,18 +176,31 @@ func (c *Cluster) AttachNetwork(target string, containerID string, addresses []s close(attachCompleteCh) c.mu.Unlock() - logrus.Debugf("Successfully attached to network %s with tid %s", target, taskID) + logrus.Debugf("Successfully attached to network %s with task id %s", target, taskID) + + release := func() { + ctx, cancel := c.getRequestContext() + defer cancel() + if err := agent.ResourceAllocator().DetachNetwork(ctx, taskID); err != nil { + logrus.Errorf("Failed remove network attachment %s to network %s on allocation failure: %v", + taskID, target, err) + } + } var config *network.NetworkingConfig select { case config = <-attachWaitCh: case <-ctx.Done(): + release() return nil, fmt.Errorf("attaching to network failed, make sure your network options are correct and check manager logs: %v", ctx.Err()) } c.mu.Lock() c.attachers[aKey].config = config c.mu.Unlock() + + logrus.Debugf("Successfully allocated resources on network %s for task id %s", target, taskID) + return config, nil } diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go index 1a64731868..711ee942d8 100644 --- a/integration-cli/docker_cli_swarm_test.go +++ b/integration-cli/docker_cli_swarm_test.go @@ -387,6 +387,30 @@ func (s *DockerSwarmSuite) TestOverlayAttachableOnSwarmLeave(c *check.C) { c.Assert(out, checker.Not(checker.Contains), nwName) } +func (s *DockerSwarmSuite) TestOverlayAttachableReleaseResourcesOnFailure(c *check.C) { + d := s.AddDaemon(c, true, true) + + // Create attachable network + out, err := d.Cmd("network", "create", "-d", "overlay", "--attachable", "--subnet", "10.10.9.0/24", "ovnet") + c.Assert(err, checker.IsNil, check.Commentf(out)) + + // Attach a container with specific IP + out, err = d.Cmd("run", "-d", "--network", "ovnet", "--name", "c1", "--ip", "10.10.9.33", "busybox", "top") + c.Assert(err, checker.IsNil, check.Commentf(out)) + + // Attempt to attach another contianer with same IP, must fail + _, err = d.Cmd("run", "-d", "--network", "ovnet", "--name", "c2", "--ip", "10.10.9.33", "busybox", "top") + c.Assert(err, checker.NotNil) + + // Remove first container + out, err = d.Cmd("rm", "-f", "c1") + c.Assert(err, checker.IsNil, check.Commentf(out)) + + // Verify the network can be removed, no phantom network attachment task left over + out, err = d.Cmd("network", "rm", "ovnet") + c.Assert(err, checker.IsNil, check.Commentf(out)) +} + func (s *DockerSwarmSuite) TestSwarmRemoveInternalNetwork(c *check.C) { d := s.AddDaemon(c, true, true)