libnet: update dnsNames on ContainerRename

The `(*Endpoint).rename()` method is changed to only mutate `ep.name`
and let a new method `(*Endpoint).UpdateDNSNames()` handle DNS updates.

As a consequence, the rollback code that was part of
`(*Endpoint).rename()` is now removed, and DNS updates are now
rolled back by `ContainerRename`.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>

daemon/rename.go

@@ -7,6 +7,7 @@ import (
 	"github.com/containerd/log"
 	"github.com/docker/docker/api/types/events"
 	dockercontainer "github.com/docker/docker/container"
+	"github.com/docker/docker/daemon/network"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/libnetwork"
 	"github.com/pkg/errors"
@@ -118,10 +119,57 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) (retErr error) {
 			return err
 		}
 
-		err = sb.Rename(strings.TrimPrefix(container.Name, "/"))
+		if err := sb.Rename(newName[1:]); err != nil {
-		if err != nil {
 			return err
 		}
+		defer func() {
+			if retErr != nil {
+				if err := sb.Rename(oldName); err != nil {
+					log.G(context.TODO()).WithFields(log.Fields{
+						"sandboxID": sid,
+						"oldName":   oldName,
+						"newName":   newName,
+						"error":     err,
+					}).Errorf("failed to revert sandbox rename")
+				}
+			}
+		}()
+
+		for nwName, epConfig := range container.NetworkSettings.Networks {
+			nw, err := daemon.FindNetwork(nwName)
+			if err != nil {
+				return err
+			}
+
+			ep, err := nw.EndpointByID(epConfig.EndpointID)
+			if err != nil {
+				return err
+			}
+
+			oldDNSNames := make([]string, len(epConfig.DNSNames))
+			copy(oldDNSNames, epConfig.DNSNames)
+
+			epConfig.DNSNames = buildEndpointDNSNames(container, epConfig.Aliases)
+			if err := ep.UpdateDNSNames(epConfig.DNSNames); err != nil {
+				return err
+			}
+
+			defer func(ep *libnetwork.Endpoint, epConfig *network.EndpointSettings, oldDNSNames []string) {
+				if retErr == nil {
+					return
+				}
+
+				epConfig.DNSNames = oldDNSNames
+				if err := ep.UpdateDNSNames(epConfig.DNSNames); err != nil {
+					log.G(context.TODO()).WithFields(log.Fields{
+						"sandboxID": sid,
+						"oldName":   oldName,
+						"newName":   newName,
+						"error":     err,
+					}).Errorf("failed to revert DNSNames update")
+				}
+			}(ep, epConfig, oldDNSNames)
+		}
 	}
 
 	daemon.LogContainerEventWithAttributes(container, events.ActionRename, attributes)

libnetwork/endpoint.go

@@ -598,71 +598,52 @@ func (ep *Endpoint) sbJoin(sb *Sandbox, options ...EndpointOption) (err error) {
 }
 
 func (ep *Endpoint) rename(name string) error {
-	var (
+	ep.mu.Lock()
-		err error
+	ep.name = name
-		ok  bool
+	ep.mu.Unlock()
-	)
 
-	n := ep.getNetwork()
+	// Update the store with the updated name
-	if n == nil {
+	if err := ep.getNetwork().getController().updateToStore(ep); err != nil {
-		return fmt.Errorf("network not connected for ep %q", ep.name)
+		return err
 	}
 
-	c := n.getController()
+	return nil
+}
 
+func (ep *Endpoint) UpdateDNSNames(dnsNames []string) error {
+	nw := ep.getNetwork()
+	c := nw.getController()
 	sb, ok := ep.getSandbox()
 	if !ok {
-		log.G(context.TODO()).Warnf("rename for %s aborted, sandbox %s is not anymore present", ep.ID(), ep.sandboxID)
+		log.G(context.TODO()).WithFields(log.Fields{
+			"sandboxID":  ep.sandboxID,
+			"endpointID": ep.ID(),
+		}).Warn("DNSNames update aborted, sandbox is not present anymore")
 		return nil
 	}
 
 	if c.isAgent() {
-		if err = ep.deleteServiceInfoFromCluster(sb, true, "rename"); err != nil {
+		if err := ep.deleteServiceInfoFromCluster(sb, true, "UpdateDNSNames"); err != nil {
-			return types.InternalErrorf("Could not delete service state for endpoint %s from cluster on rename: %v", ep.Name(), err)
+			return types.InternalErrorf("could not delete service state for endpoint %s from cluster on UpdateDNSNames: %v", ep.Name(), err)
+		}
+
+		ep.dnsNames = dnsNames
+		if err := ep.addServiceInfoToCluster(sb); err != nil {
+			return types.InternalErrorf("could not add service state for endpoint %s to cluster on UpdateDNSNames: %v", ep.Name(), err)
 		}
 	} else {
-		n.updateSvcRecord(ep, false)
+		nw.updateSvcRecord(ep, false)
-	}
 
-	oldName := ep.name
+		ep.dnsNames = dnsNames
-	oldAnonymous := ep.anonymous
+		nw.updateSvcRecord(ep, true)
-	ep.name = name
-	ep.anonymous = false
-
-	if c.isAgent() {
-		if err = ep.addServiceInfoToCluster(sb); err != nil {
-			return types.InternalErrorf("Could not add service state for endpoint %s to cluster on rename: %v", ep.Name(), err)
-		}
-		defer func() {
-			if err != nil {
-				if err2 := ep.deleteServiceInfoFromCluster(sb, true, "rename"); err2 != nil {
-					log.G(context.TODO()).WithField("main error", err).WithError(err2).Debug("Error during cleanup due deleting service info from cluster while cleaning up due to other error")
-				}
-				ep.name = oldName
-				ep.anonymous = oldAnonymous
-				if err2 := ep.addServiceInfoToCluster(sb); err2 != nil {
-					log.G(context.TODO()).WithField("main error", err).WithError(err2).Debug("Error during cleanup due adding service to from cluster while cleaning up due to other error")
-				}
-			}
-		}()
-	} else {
-		n.updateSvcRecord(ep, true)
-		defer func() {
-			if err != nil {
-				n.updateSvcRecord(ep, false)
-				ep.name = oldName
-				ep.anonymous = oldAnonymous
-				n.updateSvcRecord(ep, true)
-			}
-		}()
 	}
 
 	// Update the store with the updated name
-	if err = c.updateToStore(ep); err != nil {
+	if err := c.updateToStore(ep); err != nil {
 		return err
 	}
 
-	return err
+	return nil
 }
 
 func (ep *Endpoint) hasInterface(iName string) bool {