diff --git a/daemon/container.go b/daemon/container.go index 0aa748727..1d38de311 100644 --- a/daemon/container.go +++ b/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) } diff --git a/daemon/create.go b/daemon/create.go index 3309d034c..5adbc3a37 100644 --- a/daemon/create.go +++ b/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 } diff --git a/daemon/delete.go b/daemon/delete.go index 4501d91e2..9a74adbfa 100644 --- a/daemon/delete.go +++ b/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) diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 01a3f5763..813c02fcb 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/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") + } + + 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") + } } - // 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) + // Stop them all. + for _, id := range ids { + cmd := exec.Command(dockerBinary, "stop", id) + out, _, err := runCommandWithOutput(cmd) + if err != nil { + t.Fatal(err, out) + } } - contID := strings.TrimSpace(out) + // 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) - f, err := os.Open(filepath.Join("/var/lib/docker/containers", contID, "hosts")) - if err != nil { - t.Fatal(err) + id := strings.TrimSpace(out) + ip, err := inspectField(id, "NetworkSettings.IPAddress") + errorOut(err, t, out) + + portCmd := exec.Command(dockerBinary, "port", id, "1234") + port, _, err := runCommandWithOutput(portCmd) + errorOut(err, t, out) + + for i := range ids { + if ip == ips[i] { + t.Fatalf("Conflicting IP: %s", ip) + } + if port == ports[i] { + t.Fatalf("Conflicting port: %s", port) + } + } } - originalContent, 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) - out, _, err = runCommandWithOutput(exec.Command(dockerBinary, "restart", "-t", "0", "c1")) - if err != nil { - t.Fatal(err, out) - } - - f, err = os.Open(filepath.Join("/var/lib/docker/containers", contID, "hosts")) - if err != nil { - t.Fatal(err) - } - - newContent, err := ioutil.ReadAll(f) - f.Close() - - if err != nil { - t.Fatal(err) - } - - 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