Browse code

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 <aboch@docker.com>

Alessandro Boch authored on 2017/02/16 12:53:51
Showing 2 changed files
... ...
@@ -176,18 +176,31 @@ func (c *Cluster) AttachNetwork(target string, containerID string, addresses []s
176 176
 	close(attachCompleteCh)
177 177
 	c.mu.Unlock()
178 178
 
179
-	logrus.Debugf("Successfully attached to network %s with tid %s", target, taskID)
179
+	logrus.Debugf("Successfully attached to network %s with task id %s", target, taskID)
180
+
181
+	release := func() {
182
+		ctx, cancel := c.getRequestContext()
183
+		defer cancel()
184
+		if err := agent.ResourceAllocator().DetachNetwork(ctx, taskID); err != nil {
185
+			logrus.Errorf("Failed remove network attachment %s to network %s on allocation failure: %v",
186
+				taskID, target, err)
187
+		}
188
+	}
180 189
 
181 190
 	var config *network.NetworkingConfig
182 191
 	select {
183 192
 	case config = <-attachWaitCh:
184 193
 	case <-ctx.Done():
194
+		release()
185 195
 		return nil, fmt.Errorf("attaching to network failed, make sure your network options are correct and check manager logs: %v", ctx.Err())
186 196
 	}
187 197
 
188 198
 	c.mu.Lock()
189 199
 	c.attachers[aKey].config = config
190 200
 	c.mu.Unlock()
201
+
202
+	logrus.Debugf("Successfully allocated resources on network %s for task id %s", target, taskID)
203
+
191 204
 	return config, nil
192 205
 }
193 206
 
... ...
@@ -387,6 +387,30 @@ func (s *DockerSwarmSuite) TestOverlayAttachableOnSwarmLeave(c *check.C) {
387 387
 	c.Assert(out, checker.Not(checker.Contains), nwName)
388 388
 }
389 389
 
390
+func (s *DockerSwarmSuite) TestOverlayAttachableReleaseResourcesOnFailure(c *check.C) {
391
+	d := s.AddDaemon(c, true, true)
392
+
393
+	// Create attachable network
394
+	out, err := d.Cmd("network", "create", "-d", "overlay", "--attachable", "--subnet", "10.10.9.0/24", "ovnet")
395
+	c.Assert(err, checker.IsNil, check.Commentf(out))
396
+
397
+	// Attach a container with specific IP
398
+	out, err = d.Cmd("run", "-d", "--network", "ovnet", "--name", "c1", "--ip", "10.10.9.33", "busybox", "top")
399
+	c.Assert(err, checker.IsNil, check.Commentf(out))
400
+
401
+	// Attempt to attach another contianer with same IP, must fail
402
+	_, err = d.Cmd("run", "-d", "--network", "ovnet", "--name", "c2", "--ip", "10.10.9.33", "busybox", "top")
403
+	c.Assert(err, checker.NotNil)
404
+
405
+	// Remove first container
406
+	out, err = d.Cmd("rm", "-f", "c1")
407
+	c.Assert(err, checker.IsNil, check.Commentf(out))
408
+
409
+	// Verify the network can be removed, no phantom network attachment task left over
410
+	out, err = d.Cmd("network", "rm", "ovnet")
411
+	c.Assert(err, checker.IsNil, check.Commentf(out))
412
+}
413
+
390 414
 func (s *DockerSwarmSuite) TestSwarmRemoveInternalNetwork(c *check.C) {
391 415
 	d := s.AddDaemon(c, true, true)
392 416