From 2361edbcea22113aea639a3d74edb35791fd8aef Mon Sep 17 00:00:00 2001 From: Madhu Venugopal Date: Fri, 30 Oct 2015 15:05:01 -0700 Subject: [PATCH 1/3] Vendoring in libnetwork to fix an ungraceful restart case Also picked up a minor typo fix Signed-off-by: Madhu Venugopal --- hack/vendor.sh | 2 +- .../src/github.com/docker/libnetwork/controller.go | 3 ++- .../github.com/docker/libnetwork/ipam/allocator.go | 2 +- vendor/src/github.com/docker/libnetwork/sandbox.go | 2 +- .../github.com/docker/libnetwork/sandbox_store.go | 12 ++++++------ 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/hack/vendor.sh b/hack/vendor.sh index fafc281911..07b0f6280a 100755 --- a/hack/vendor.sh +++ b/hack/vendor.sh @@ -21,7 +21,7 @@ clone git github.com/vdemeester/shakers 3c10293ce22b900c27acad7b28656196fcc2f73b clone git golang.org/x/net 3cffabab72adf04f8e3b01c5baf775361837b5fe https://github.com/golang/net.git #get libnetwork packages -clone git github.com/docker/libnetwork 5fc6ba506daa7914f4d58befb38480ec8e9c9f70 +clone git github.com/docker/libnetwork 47edb73dd3e64cfcc04234b073872205cd79694a clone git github.com/armon/go-metrics eb0af217e5e9747e41dd5303755356b62d28e3ec clone git github.com/hashicorp/go-msgpack 71c2886f5a673a35f909803f38ece5810165097b clone git github.com/hashicorp/memberlist 9a1e242e454d2443df330bdd51a436d5a9058fc4 diff --git a/vendor/src/github.com/docker/libnetwork/controller.go b/vendor/src/github.com/docker/libnetwork/controller.go index 3b75ae213c..77d5e68cbb 100644 --- a/vendor/src/github.com/docker/libnetwork/controller.go +++ b/vendor/src/github.com/docker/libnetwork/controller.go @@ -507,13 +507,14 @@ func (c *controller) NewSandbox(containerID string, options ...SandboxOption) (S return nil, err } + c.Lock() if sb.osSbox == nil && !sb.config.useExternalKey { if sb.osSbox, err = osl.NewSandbox(sb.Key(), !sb.config.useDefaultSandBox); err != nil { + c.Unlock() return nil, fmt.Errorf("failed to create new osl sandbox: %v", err) } } - c.Lock() c.sandboxes[sb.id] = sb c.Unlock() defer func() { diff --git a/vendor/src/github.com/docker/libnetwork/ipam/allocator.go b/vendor/src/github.com/docker/libnetwork/ipam/allocator.go index dfad30bdee..bec7c75343 100644 --- a/vendor/src/github.com/docker/libnetwork/ipam/allocator.go +++ b/vendor/src/github.com/docker/libnetwork/ipam/allocator.go @@ -195,7 +195,7 @@ func (a *Allocator) getAddrSpace(as string) (*addrSpace, error) { defer a.Unlock() aSpace, ok := a.addrSpaces[as] if !ok { - return nil, types.BadRequestErrorf("cannot find address space %s (most likey the backing datastore is not configured)", as) + return nil, types.BadRequestErrorf("cannot find address space %s (most likely the backing datastore is not configured)", as) } return aSpace, nil } diff --git a/vendor/src/github.com/docker/libnetwork/sandbox.go b/vendor/src/github.com/docker/libnetwork/sandbox.go index 5ea3f1f80f..96ef4c81a7 100644 --- a/vendor/src/github.com/docker/libnetwork/sandbox.go +++ b/vendor/src/github.com/docker/libnetwork/sandbox.go @@ -479,7 +479,7 @@ func (sb *sandbox) populateNetworkResources(ep *endpoint) error { for _, gwep := range sb.getConnectedEndpoints() { if len(gwep.Gateway()) > 0 { if gwep != ep { - return nil + break } if err := sb.updateGateway(gwep); err != nil { return err diff --git a/vendor/src/github.com/docker/libnetwork/sandbox_store.go b/vendor/src/github.com/docker/libnetwork/sandbox_store.go index 4844032b2b..cd61f696fe 100644 --- a/vendor/src/github.com/docker/libnetwork/sandbox_store.go +++ b/vendor/src/github.com/docker/libnetwork/sandbox_store.go @@ -197,28 +197,28 @@ func (c *controller) sandboxCleanup() { continue } + c.Lock() + c.sandboxes[sb.id] = sb + c.Unlock() + for _, eps := range sbs.Eps { n, err := c.getNetworkFromStore(eps.Nid) var ep *endpoint if err != nil { logrus.Errorf("getNetworkFromStore for nid %s failed while trying to build sandbox for cleanup: %v", eps.Nid, err) n = &network{id: eps.Nid, ctrlr: c, drvOnce: &sync.Once{}} - ep = &endpoint{id: eps.Eid, network: n} + ep = &endpoint{id: eps.Eid, network: n, sandboxID: sbs.ID} } else { ep, err = n.getEndpointFromStore(eps.Eid) if err != nil { logrus.Errorf("getEndpointFromStore for eid %s failed while trying to build sandbox for cleanup: %v", eps.Eid, err) - ep = &endpoint{id: eps.Eid, network: n} + ep = &endpoint{id: eps.Eid, network: n, sandboxID: sbs.ID} } } heap.Push(&sb.endpoints, ep) } - c.Lock() - c.sandboxes[sb.id] = sb - c.Unlock() - if err := sb.Delete(); err != nil { logrus.Errorf("failed to delete sandbox %s while trying to cleanup: %v", sb.id, err) } From 401632c7568408ee9689adc1da855cfb1409e906 Mon Sep 17 00:00:00 2001 From: Madhu Venugopal Date: Fri, 30 Oct 2015 15:06:44 -0700 Subject: [PATCH 2/3] fixing ungraceful daemon restart case where nw connect is not persisted For graceful restart case it was done when the container was brought down. But for ungraceful cases, the persistence is missing for nw connect Signed-off-by: Madhu Venugopal --- daemon/container_unix.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/daemon/container_unix.go b/daemon/container_unix.go index 4c13f7d036..8e668ffd83 100644 --- a/daemon/container_unix.go +++ b/daemon/container_unix.go @@ -945,7 +945,13 @@ func (container *Container) ConnectToNetwork(idOrName string) error { if !container.Running { return derr.ErrorCodeNotRunning.WithArgs(container.ID) } - return container.connectToNetwork(idOrName, true) + if err := container.connectToNetwork(idOrName, true); err != nil { + return err + } + if err := container.toDiskLocking(); err != nil { + return fmt.Errorf("Error saving container to disk: %v", err) + } + return nil } func (container *Container) connectToNetwork(idOrName string, updateSettings bool) error { @@ -1177,7 +1183,14 @@ func (container *Container) DisconnectFromNetwork(n libnetwork.Network) error { return derr.ErrorCodeNotRunning.WithArgs(container.ID) } - return container.disconnectFromNetwork(n) + if err := container.disconnectFromNetwork(n); err != nil { + return err + } + + if err := container.toDiskLocking(); err != nil { + return fmt.Errorf("Error saving container to disk: %v", err) + } + return nil } func (container *Container) disconnectFromNetwork(n libnetwork.Network) error { From e16e794805ec1e578951f271af7993d9619a4ede Mon Sep 17 00:00:00 2001 From: Alessandro Boch Date: Fri, 30 Oct 2015 14:51:02 -0700 Subject: [PATCH 3/3] IT for daemon restarts when container connected to multiple networks Signed-off-by: Alessandro Boch --- .../docker_cli_network_unix_test.go | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/integration-cli/docker_cli_network_unix_test.go b/integration-cli/docker_cli_network_unix_test.go index 3e11abb691..3e92891657 100644 --- a/integration-cli/docker_cli_network_unix_test.go +++ b/integration-cli/docker_cli_network_unix_test.go @@ -670,3 +670,66 @@ func (s *DockerSuite) TestInspectApiMultipeNetworks(c *check.C) { c.Assert(bridge.IPAddress, checker.Equals, versionedIP) c.Assert(bridge.IPAddress, checker.Equals, inspect121.NetworkSettings.IPAddress) } + +func connectContainerToNetworks(c *check.C, d *Daemon, cName string, nws []string) { + // Run a container on the default network + out, err := d.Cmd("run", "-d", "--name", cName, "busybox", "top") + c.Assert(err, checker.IsNil, check.Commentf(out)) + + // Attach the container to other three networks + for _, nw := range nws { + out, err = d.Cmd("network", "create", nw) + c.Assert(err, checker.IsNil, check.Commentf(out)) + out, err = d.Cmd("network", "connect", nw, cName) + c.Assert(err, checker.IsNil, check.Commentf(out)) + } +} + +func verifyContainerIsConnectedToNetworks(c *check.C, d *Daemon, cName string, nws []string) { + // Verify container is connected to all three networks + for _, nw := range nws { + out, err := d.Cmd("inspect", "-f", fmt.Sprintf("{{.NetworkSettings.Networks.%s}}", nw), cName) + c.Assert(err, checker.IsNil, check.Commentf(out)) + c.Assert(out, checker.Not(checker.Equals), "\n") + } +} + +func (s *DockerNetworkSuite) TestDockerNetworkMultipleNetworksGracefulDaemonRestart(c *check.C) { + cName := "bb" + nwList := []string{"nw1", "nw2", "nw3"} + + s.d.Start() + + connectContainerToNetworks(c, s.d, cName, nwList) + verifyContainerIsConnectedToNetworks(c, s.d, cName, nwList) + + // Reload daemon + s.d.Restart() + + _, err := s.d.Cmd("start", cName) + c.Assert(err, checker.IsNil) + + verifyContainerIsConnectedToNetworks(c, s.d, cName, nwList) +} + +func (s *DockerNetworkSuite) TestDockerNetworkMultipleNetworksUngracefulDaemonRestart(c *check.C) { + cName := "cc" + nwList := []string{"nw1", "nw2", "nw3"} + + s.d.Start() + + connectContainerToNetworks(c, s.d, cName, nwList) + verifyContainerIsConnectedToNetworks(c, s.d, cName, nwList) + + // Kill daemon and restart + if err := s.d.cmd.Process.Kill(); err != nil { + c.Fatal(err) + } + s.d.Restart() + + // Restart container + _, err := s.d.Cmd("start", cName) + c.Assert(err, checker.IsNil) + + verifyContainerIsConnectedToNetworks(c, s.d, cName, nwList) +}