Stable Networking: Keep the same network settings across container restarts.

This change will allocate network settings (IP and public ports) at
container creation rather than start and keep them throughout the
lifetime of the container (i.e. until it gets destroyed) instead of
discarding them when the container is stopped.

Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>

daemon/container.go

@@ -552,7 +552,7 @@ func (container *Container) RestoreNetwork() error {
 	}
 
 	// Re-allocate any previously allocated ports.
-	for port, _ := range container.NetworkSettings.Ports {
+	for port := range container.NetworkSettings.Ports {
 		if err := container.allocatePort(eng, port, container.NetworkSettings.Ports); err != nil {
 			return err
 		}
@@ -563,8 +563,6 @@ func (container *Container) RestoreNetwork() error {
 // cleanup releases any network resources allocated to the container along with any rules
 // around how containers are linked together.  It also unmounts the container's root filesystem.
 func (container *Container) cleanup() {
-	container.ReleaseNetwork()
-
 	// Disable all active links
 	if container.activeLinks != nil {
 		for _, link := range container.activeLinks {
@@ -1008,8 +1006,14 @@ func (container *Container) initializeNetworking() error {
 		container.Config.NetworkDisabled = true
 		return container.buildHostnameAndHostsFiles("127.0.1.1")
 	}
-	if err := container.AllocateNetwork(); err != nil {
-		return err
+	// Backward compatibility:
+	// Network allocation used to be done when containers started, not when they
+	// were created, therefore we might be starting a legacy container that
+	// doesn't have networking.
+	if !container.isNetworkAllocated() {
+		if err := container.AllocateNetwork(); err != nil {
+			return err
+		}
 	}
 	return container.buildHostnameAndHostsFiles(container.NetworkSettings.IPAddress)
 }

daemon/create.go

@@ -83,6 +83,9 @@ func (daemon *Daemon) Create(config *runconfig.Config, hostConfig *runconfig.Hos
 	if container, err = daemon.newContainer(name, config, img); err != nil {
 		return nil, nil, err
 	}
+	if err := daemon.Register(container); err != nil {
+		return nil, nil, err
+	}
 	if err := daemon.createRootfs(container, img); err != nil {
 		return nil, nil, err
 	}
@@ -90,12 +93,13 @@ func (daemon *Daemon) Create(config *runconfig.Config, hostConfig *runconfig.Hos
 		if err := daemon.setHostConfig(container, hostConfig); err != nil {
 			return nil, nil, err
 		}
+		// We may only allocate the network if a host config was passed, otherwise we'll miss port mappings.
+		if err := container.AllocateNetwork(); err != nil {
+			return nil, nil, err
+		}
 	}
 	if err := container.ToDisk(); err != nil {
 		return nil, nil, err
 	}
-	if err := daemon.Register(container); err != nil {
-		return nil, nil, err
-	}
 	return container, warnings, nil
 }

daemon/delete.go

@@ -94,6 +94,8 @@ func (daemon *Daemon) Destroy(container *Container) error {
 		return err
 	}
 
+	container.ReleaseNetwork()
+
 	// Deregister the container before removing its directory, to avoid race conditions
 	daemon.idIndex.Delete(container.ID)
 	daemon.containers.Delete(container.ID)

integration-cli/docker_cli_run_test.go

@@ -1868,57 +1868,87 @@ func TestRunMutableNetworkFiles(t *testing.T) {
 	}
 }
 
-func TestRunHostsLinkedContainerUpdate(t *testing.T) {
-	deleteAllContainers()
-	out, _, err := runCommandWithOutput(exec.Command(dockerBinary, "run", "-d", "--name", "c1", "busybox", "sh", "-c", "while true; do sleep 1; done"))
-	if err != nil {
-		t.Fatal(err, out)
-	}
+func TestRunStableIPAndPort(t *testing.T) {
+	const nContainers = 2
+	var ids, ips, ports [nContainers]string
+
+	// Setup: Create a couple of containers and collect their IPs and public ports.
+	for i := 0; i < nContainers; i++ {
+		runCmd := exec.Command(dockerBinary, "run", "-d", "-p", "1234", "busybox", "top")
+		out, _, err := runCommandWithOutput(runCmd)
+		if err != nil {
+			t.Fatal(err)
+		}
+		ids[i] = strings.TrimSpace(out)
+		ips[i], err = inspectField(ids[i], "NetworkSettings.IPAddress")
+		errorOut(err, t, out)
+		if ips[i] == "" {
+			t.Fatal("IP allocation failed")
+		}
 
-	// TODO fix docker cp and /etc/hosts
-	out, _, err = runCommandWithOutput(exec.Command(dockerBinary, "run", "-d", "--link", "c1:c1", "--name", "c2", "busybox", "sh", "-c", "while true;do sleep 1; done"))
-	if err != nil {
-		t.Fatal(err, out)
+		portCmd := exec.Command(dockerBinary, "port", ids[i], "1234")
+		ports[i], _, err = runCommandWithOutput(portCmd)
+		errorOut(err, t, out)
+		if ports[i] == "" {
+			t.Fatal("Port allocation failed")
+		}
 	}
 
-	contID := strings.TrimSpace(out)
-
-	f, err := os.Open(filepath.Join("/var/lib/docker/containers", contID, "hosts"))
-	if err != nil {
-		t.Fatal(err)
+	// Stop them all.
+	for _, id := range ids {
+		cmd := exec.Command(dockerBinary, "stop", id)
+		out, _, err := runCommandWithOutput(cmd)
+		if err != nil {
+			t.Fatal(err, out)
+		}
 	}
 
-	originalContent, err := ioutil.ReadAll(f)
-	f.Close()
+	// Create a new container and ensure it's not getting the IP or port of some stopped container.
+	{
+		runCmd := exec.Command(dockerBinary, "run", "-d", "-p", "1234", "busybox", "top")
+		out, _, err := runCommandWithOutput(runCmd)
+		errorOut(err, t, out)
 
-	if err != nil {
-		t.Fatal(err)
-	}
+		id := strings.TrimSpace(out)
+		ip, err := inspectField(id, "NetworkSettings.IPAddress")
+		errorOut(err, t, out)
 
-	out, _, err = runCommandWithOutput(exec.Command(dockerBinary, "restart", "-t", "0", "c1"))
-	if err != nil {
-		t.Fatal(err, out)
-	}
+		portCmd := exec.Command(dockerBinary, "port", id, "1234")
+		port, _, err := runCommandWithOutput(portCmd)
+		errorOut(err, t, out)
 
-	f, err = os.Open(filepath.Join("/var/lib/docker/containers", contID, "hosts"))
-	if err != nil {
-		t.Fatal(err)
+		for i := range ids {
+			if ip == ips[i] {
+				t.Fatalf("Conflicting IP: %s", ip)
+			}
+			if port == ports[i] {
+				t.Fatalf("Conflicting port: %s", port)
+			}
+		}
 	}
 
-	newContent, err := ioutil.ReadAll(f)
-	f.Close()
+	// Start the containers back, and ensure they are getting the same IPs and ports.
+	for i, id := range ids {
+		runCmd := exec.Command(dockerBinary, "start", id)
+		out, _, err := runCommandWithOutput(runCmd)
+		errorOut(err, t, out)
 
-	if err != nil {
-		t.Fatal(err)
-	}
+		ip, err := inspectField(id, "NetworkSettings.IPAddress")
+		errorOut(err, t, out)
+		portCmd := exec.Command(dockerBinary, "port", ids[i], "1234")
+		port, _, err := runCommandWithOutput(portCmd)
+		errorOut(err, t, out)
 
-	if strings.TrimSpace(string(originalContent)) == strings.TrimSpace(string(newContent)) {
-		t.Fatalf("expected /etc/hosts to be updated, but wasn't")
+		if ips[i] != ip {
+			t.Fatalf("Container started with a different IP: %s != %s", ip, ips[i])
+		}
+		if ports[i] != port {
+			t.Fatalf("Container started with a different port: %s != %s", port, ports[i])
+		}
 	}
 
 	deleteAllContainers()
-
-	logDone("run - /etc/hosts updated in parent when restart")
+	logDone("run - ips and ports must not change")
 }
 
 // Ensure that CIDFile gets deleted if it's empty