From 8e0bbb28986c9aca5c51f546ba6fd0f1041ace14 Mon Sep 17 00:00:00 2001 From: Santhosh Manohar Date: Fri, 23 Oct 2015 08:01:07 -0700 Subject: [PATCH 1/3] Add libnetwork call on daemon rename Signed-off-by: Santhosh Manohar --- daemon/rename.go | 55 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 10 deletions(-) diff --git a/daemon/rename.go b/daemon/rename.go index de80e609f1..1aab1f9303 100644 --- a/daemon/rename.go +++ b/daemon/rename.go @@ -1,18 +1,28 @@ package daemon import ( + "github.com/Sirupsen/logrus" derr "github.com/docker/docker/errors" + "github.com/docker/libnetwork" + "strings" ) // ContainerRename changes the name of a container, using the oldName // to find the container. An error is returned if newName is already // reserved. func (daemon *Daemon) ContainerRename(oldName, newName string) error { + var ( + err error + sid string + sb libnetwork.Sandbox + container *Container + ) + if oldName == "" || newName == "" { return derr.ErrorCodeEmptyRename } - container, err := daemon.Get(oldName) + container, err = daemon.Get(oldName) if err != nil { return err } @@ -27,19 +37,44 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error { container.Name = newName - undo := func() { - container.Name = oldName - daemon.reserveName(container.ID, oldName) - daemon.containerGraphDB.Delete(newName) - } + defer func() { + if err != nil { + container.Name = oldName + daemon.reserveName(container.ID, oldName) + daemon.containerGraphDB.Delete(newName) + } + }() - if err := daemon.containerGraphDB.Delete(oldName); err != nil { - undo() + if err = daemon.containerGraphDB.Delete(oldName); err != nil { return derr.ErrorCodeRenameDelete.WithArgs(oldName, err) } - if err := container.toDisk(); err != nil { - undo() + if err = container.toDisk(); err != nil { + return err + } + + if !container.Running { + container.logEvent("rename") + return nil + } + + defer func() { + if err != nil { + container.Name = oldName + if e := container.toDisk(); e != nil { + logrus.Errorf("%s: Failed in writing to Disk on rename failure: %v", container.ID, e) + } + } + }() + + sid = container.NetworkSettings.SandboxID + sb, err = daemon.netController.SandboxByID(sid) + if err != nil { + return err + } + + err = sb.Rename(strings.TrimPrefix(container.Name, "/")) + if err != nil { return err } From 3b16a8c91c77cbae99b04d641ae73693780f52ca Mon Sep 17 00:00:00 2001 From: Madhu Venugopal Date: Mon, 26 Oct 2015 17:49:50 -0700 Subject: [PATCH 2/3] integration-cli test for active container rename and reuse Signed-off-by: Madhu Venugopal Signed-off-by: Santhosh Manohar --- integration-cli/docker_cli_rename_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/integration-cli/docker_cli_rename_test.go b/integration-cli/docker_cli_rename_test.go index a1bf4b7427..df0b26862d 100644 --- a/integration-cli/docker_cli_rename_test.go +++ b/integration-cli/docker_cli_rename_test.go @@ -38,6 +38,27 @@ func (s *DockerSuite) TestRenameRunningContainer(c *check.C) { c.Assert(name, checker.Equals, "/"+newName, check.Commentf("Failed to rename container %s", name)) } +func (s *DockerSuite) TestRenameRunningContainerAndReuse(c *check.C) { + testRequires(c, DaemonIsLinux) + out, _ := dockerCmd(c, "run", "--name", "first_name", "-d", "busybox", "top") + c.Assert(waitRun("first_name"), check.IsNil) + + newName := "new_name" + ContainerID := strings.TrimSpace(out) + dockerCmd(c, "rename", "first_name", newName) + + name, err := inspectField(ContainerID, "Name") + c.Assert(err, checker.IsNil, check.Commentf("Failed to rename container %s", name)) + c.Assert(name, checker.Equals, "/"+newName, check.Commentf("Failed to rename container")) + + out, _ = dockerCmd(c, "run", "--name", "first_name", "-d", "busybox", "top") + c.Assert(waitRun("first_name"), check.IsNil) + newContainerID := strings.TrimSpace(out) + name, err = inspectField(newContainerID, "Name") + c.Assert(err, checker.IsNil, check.Commentf("Failed to reuse container name")) + c.Assert(name, checker.Equals, "/first_name", check.Commentf("Failed to reuse container name")) +} + func (s *DockerSuite) TestRenameCheckNames(c *check.C) { testRequires(c, DaemonIsLinux) dockerCmd(c, "run", "--name", "first_name", "-d", "busybox", "sh") From 1041f3967086052438a3e34cee3db14e2277b629 Mon Sep 17 00:00:00 2001 From: Santhosh Manohar Date: Fri, 23 Oct 2015 08:11:57 -0700 Subject: [PATCH 3/3] Vendor in libnetwork changes to support container rename Signed-off-by: Santhosh Manohar --- hack/vendor.sh | 2 +- .../docker/libnetwork/controller.go | 1 + .../github.com/docker/libnetwork/endpoint.go | 45 +++++++++++++++++++ .../docker/libnetwork/endpoint_cnt.go | 15 +++++++ .../github.com/docker/libnetwork/sandbox.go | 26 +++++++++++ .../src/github.com/docker/libnetwork/store.go | 33 ++++++++------ 6 files changed, 107 insertions(+), 15 deletions(-) diff --git a/hack/vendor.sh b/hack/vendor.sh index 8398e56c3c..d872d4a3fa 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 c92c21bca42a3581fd108d3222848faa16372249 +clone git github.com/docker/libnetwork abc0807d72e309f53155ec4f6374a77fd6613849 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 a81a9d67a0..3b75ae213c 100644 --- a/vendor/src/github.com/docker/libnetwork/controller.go +++ b/vendor/src/github.com/docker/libnetwork/controller.go @@ -143,6 +143,7 @@ type controller struct { watchCh chan *endpoint unWatchCh chan *endpoint svcDb map[string]svcMap + nmap map[string]*netWatch sync.Mutex } diff --git a/vendor/src/github.com/docker/libnetwork/endpoint.go b/vendor/src/github.com/docker/libnetwork/endpoint.go index 4288366a8a..de7b652dc5 100644 --- a/vendor/src/github.com/docker/libnetwork/endpoint.go +++ b/vendor/src/github.com/docker/libnetwork/endpoint.go @@ -431,6 +431,51 @@ func (ep *endpoint) sbJoin(sbox Sandbox, options ...EndpointOption) error { return sb.clearDefaultGW() } +func (ep *endpoint) rename(name string) error { + var err error + n := ep.getNetwork() + if n == nil { + return fmt.Errorf("network not connected for ep %q", ep.name) + } + + n.getController().Lock() + netWatch, ok := n.getController().nmap[n.ID()] + n.getController().Unlock() + + if !ok { + return fmt.Errorf("watch null for network %q", n.Name()) + } + + n.updateSvcRecord(ep, n.getController().getLocalEps(netWatch), false) + + oldName := ep.name + ep.name = name + + n.updateSvcRecord(ep, n.getController().getLocalEps(netWatch), true) + defer func() { + if err != nil { + n.updateSvcRecord(ep, n.getController().getLocalEps(netWatch), false) + ep.name = oldName + n.updateSvcRecord(ep, n.getController().getLocalEps(netWatch), true) + } + }() + + // Update the store with the updated name + if err = n.getController().updateToStore(ep); err != nil { + return err + } + // After the name change do a dummy endpoint count update to + // trigger the service record update in the peer nodes + + // Ignore the error because updateStore fail for EpCnt is a + // benign error. Besides there is no meaningful recovery that + // we can do. When the cluster recovers subsequent EpCnt update + // will force the peers to get the correct EP name. + n.getEpCnt().updateStore() + + return err +} + func (ep *endpoint) hasInterface(iName string) bool { ep.Lock() defer ep.Unlock() diff --git a/vendor/src/github.com/docker/libnetwork/endpoint_cnt.go b/vendor/src/github.com/docker/libnetwork/endpoint_cnt.go index 550a2a3cfd..507de393b8 100644 --- a/vendor/src/github.com/docker/libnetwork/endpoint_cnt.go +++ b/vendor/src/github.com/docker/libnetwork/endpoint_cnt.go @@ -108,6 +108,21 @@ func (ec *endpointCnt) EndpointCnt() uint64 { return ec.Count } +func (ec *endpointCnt) updateStore() error { + store := ec.n.getController().getStore(ec.DataScope()) + if store == nil { + return fmt.Errorf("store not found for scope %s on endpoint count update", ec.DataScope()) + } + for { + if err := ec.n.getController().updateToStore(ec); err == nil || err != datastore.ErrKeyModified { + return err + } + if err := store.GetObject(datastore.Key(ec.Key()...), ec); err != nil { + return fmt.Errorf("could not update the kvobject to latest on endpoint count update: %v", err) + } + } +} + func (ec *endpointCnt) atomicIncDecEpCnt(inc bool) error { retry: ec.Lock() diff --git a/vendor/src/github.com/docker/libnetwork/sandbox.go b/vendor/src/github.com/docker/libnetwork/sandbox.go index ff0c66fc9c..bb5097935c 100644 --- a/vendor/src/github.com/docker/libnetwork/sandbox.go +++ b/vendor/src/github.com/docker/libnetwork/sandbox.go @@ -34,6 +34,8 @@ type Sandbox interface { Refresh(options ...SandboxOption) error // SetKey updates the Sandbox Key SetKey(key string) error + // Rename changes the name of all attached Endpoints + Rename(name string) error // Delete destroys this container after detaching it from all connected endpoints. Delete() error } @@ -201,6 +203,30 @@ func (sb *sandbox) Delete() error { return nil } +func (sb *sandbox) Rename(name string) error { + var err error + + for _, ep := range sb.getConnectedEndpoints() { + if ep.endpointInGWNetwork() { + continue + } + + oldName := ep.Name() + lEp := ep + if err = ep.rename(name); err != nil { + break + } + + defer func() { + if err != nil { + lEp.rename(oldName) + } + }() + } + + return err +} + func (sb *sandbox) Refresh(options ...SandboxOption) error { // Store connected endpoints epList := sb.getConnectedEndpoints() diff --git a/vendor/src/github.com/docker/libnetwork/store.go b/vendor/src/github.com/docker/libnetwork/store.go index 7bbb9aa1bb..299e83ad6d 100644 --- a/vendor/src/github.com/docker/libnetwork/store.go +++ b/vendor/src/github.com/docker/libnetwork/store.go @@ -274,25 +274,30 @@ func (c *controller) networkWatchLoop(nw *netWatch, ep *endpoint, ecCh <-chan da continue } - if _, ok := nw.remoteEps[lEp.ID()]; ok { - delete(delEpMap, lEp.ID()) - continue + if ep, ok := nw.remoteEps[lEp.ID()]; ok { + // On a container rename EP ID will remain + // the same but the name will change. service + // records should reflect the change. + // Keep old EP entry in the delEpMap and add + // EP from the store (which has the new name) + // into the new list + if lEp.name == ep.name { + delete(delEpMap, lEp.ID()) + continue + } } - nw.remoteEps[lEp.ID()] = lEp addEp = append(addEp, lEp) - } c.Unlock() - for _, lEp := range addEp { - ep.getNetwork().updateSvcRecord(lEp, c.getLocalEps(nw), true) - } - for _, lEp := range delEpMap { ep.getNetwork().updateSvcRecord(lEp, c.getLocalEps(nw), false) } + for _, lEp := range addEp { + ep.getNetwork().updateSvcRecord(lEp, c.getLocalEps(nw), true) + } } } } @@ -378,13 +383,13 @@ func (c *controller) processEndpointDelete(nmap map[string]*netWatch, ep *endpoi c.Unlock() } -func (c *controller) watchLoop(nmap map[string]*netWatch) { +func (c *controller) watchLoop() { for { select { case ep := <-c.watchCh: - c.processEndpointCreate(nmap, ep) + c.processEndpointCreate(c.nmap, ep) case ep := <-c.unWatchCh: - c.processEndpointDelete(nmap, ep) + c.processEndpointDelete(c.nmap, ep) } } } @@ -392,7 +397,7 @@ func (c *controller) watchLoop(nmap map[string]*netWatch) { func (c *controller) startWatch() { c.watchCh = make(chan *endpoint) c.unWatchCh = make(chan *endpoint) - nmap := make(map[string]*netWatch) + c.nmap = make(map[string]*netWatch) - go c.watchLoop(nmap) + go c.watchLoop() }