Put clearNetworkResources() inline in its only caller

Signed-off-by: Rob Murray <rob.murray@docker.com>

daemon/libnetwork/default_gateway.go

@@ -176,17 +176,16 @@ func (c *Controller) defaultGwNetwork() (*Network, error) {
 	return n, err
 }
 
+func (sb *Sandbox) isGatewayEndpoint(epId string) bool {
+	gw4, gw6 := sb.getGatewayEndpoint()
+	return (gw4 != nil && epId == gw4.ID()) || (gw6 != nil && epId == gw6.ID())
+}
+
 // getGatewayEndpoint returns the endpoints providing external connectivity to
 // the sandbox. If the gateway is dual-stack, ep4 and ep6 will point at the same
 // endpoint. If there is no IPv4/IPv6 connectivity, nil pointers will be returned.
 func (sb *Sandbox) getGatewayEndpoint() (ep4, ep6 *Endpoint) {
-	return selectGatewayEndpoint(sb.Endpoints())
+	for _, ep := range sb.Endpoints() {
-}
-
-// selectGatewayEndpoint is like getGatewayEndpoint, but selects only from
-// endpoints.
-func selectGatewayEndpoint(endpoints []*Endpoint) (ep4, ep6 *Endpoint) {
-	for _, ep := range endpoints {
 		if ep.getNetwork().Type() == "null" || ep.getNetwork().Type() == "host" {
 			continue
 		}

daemon/libnetwork/endpoint.go

@@ -820,16 +820,37 @@ func (ep *Endpoint) sbLeave(ctx context.Context, sb *Sandbox, n *Network, force
 	// Capture the addresses that were added to the container's /etc/hosts here,
 	// before the endpoint is deleted, so that they can be removed from /etc/hosts.
 	etcHostsAddrs := ep.getEtcHostsAddrs()
+	// Before removing the Endpoint from the Sandbox's list of endpoints, check whether
+	// it's acting as a gateway so that new gateways can be selected if it is.
+	wasGwEp := sb.isGatewayEndpoint(ep.id)
 
-	if err := sb.clearNetworkResources(ep); err != nil {
+	// Remove the sb's references to ep.
-		log.G(ctx).WithError(err).Warn("Failed to clean up network resources on container disconnect")
+	sb.mu.Lock()
+	osSbox := sb.osSbox
+	delete(sb.populatedEndpoints, ep.id)
+	delete(sb.epPriority, ep.id)
+	sb.endpoints = slices.DeleteFunc(sb.endpoints, func(other *Endpoint) bool { return other.id == ep.id })
+	sb.mu.Unlock()
+
+	// Delete interfaces, routes etc. from the OS.
+	if osSbox != nil {
+		releaseOSSboxResources(osSbox, ep)
+
+		// Even if the interface was initially created in the container's namespace, it's
+		// now been moved out. When a legacy link is deleted, the Endpoint is removed and
+		// then re-added to the Sandbox. So, to make sure the re-add works, note that the
+		// interface is now outside the container's netns.
+		ep.iface.createdInContainer = false
 	}
 
-	// Even if the interface was initially created in the container's namespace, it's
+	// Update gateway / static routes if the ep was the gateway.
-	// now been moved out. When a legacy link is deleted, the Endpoint is removed and
+	var gwepAfter4, gwepAfter6 *Endpoint
-	// then re-added to the Sandbox. So, to make sure the re-add works, note that the
+	if wasGwEp {
-	// interface is now outside the container's netns.
+		gwepAfter4, gwepAfter6 = sb.getGatewayEndpoint()
-	ep.iface.createdInContainer = false
+		if err := sb.updateGateway(gwepAfter4, gwepAfter6); err != nil {
+			return fmt.Errorf("updating gateway endpoint: %w", err)
+		}
+	}
 
 	// Update the store about the sandbox detach only after we
 	// have completed sb.clearNetworkResources above to avoid
@@ -869,16 +890,17 @@ func (ep *Endpoint) sbLeave(ctx context.Context, sb *Sandbox, n *Network, force
 		sb.resolver.SetForwardingPolicy(sb.hasExternalAccess())
 	}
 
-	// Find new endpoint(s) to provide external connectivity for the sandbox.
+	// Configure the endpoints that now provide external connectivity for the sandbox.
-	gwepAfter4, gwepAfter6 := sb.getGatewayEndpoint()
+	if wasGwEp {
-	if gwepAfter4 != nil {
+		if gwepAfter4 != nil {
-		if err := gwepAfter4.programExternalConnectivity(ctx, gwepAfter4, gwepAfter6); err != nil {
+			if err := gwepAfter4.programExternalConnectivity(ctx, gwepAfter4, gwepAfter6); err != nil {
-			log.G(ctx).WithError(err).Error("Failed to set IPv4 gateway")
+				log.G(ctx).WithError(err).Error("Failed to set IPv4 gateway")
+			}
 		}
-	}
+		if gwepAfter6 != nil && gwepAfter6 != gwepAfter4 {
-	if gwepAfter6 != nil && gwepAfter6 != gwepAfter4 {
+			if err := gwepAfter6.programExternalConnectivity(ctx, gwepAfter4, gwepAfter6); err != nil {
-		if err := gwepAfter6.programExternalConnectivity(ctx, gwepAfter4, gwepAfter6); err != nil {
+				log.G(ctx).WithError(err).Error("Failed to set IPv6 gateway")
-			log.G(ctx).WithError(err).Error("Failed to set IPv6 gateway")
+			}
 		}
 	}
 

daemon/libnetwork/sandbox.go

@@ -319,14 +319,10 @@ func (sb *Sandbox) addEndpoint(ep *Endpoint) {
 	sb.mu.Lock()
 	defer sb.mu.Unlock()
 
-	l := len(sb.endpoints)
+	i := sort.Search(len(sb.endpoints), func(j int) bool {
-	i := sort.Search(l, func(j int) bool {
 		return ep.Less(sb.endpoints[j])
 	})
-
+	sb.endpoints = slices.Insert(sb.endpoints, i, ep)
-	sb.endpoints = append(sb.endpoints, nil)
-	copy(sb.endpoints[i+1:], sb.endpoints[i:])
-	sb.endpoints[i] = ep
 }
 
 func (sb *Sandbox) removeEndpoint(ep *Endpoint) {
@@ -574,62 +570,6 @@ func (sb *Sandbox) DisableService() error {
 	return nil
 }
 
-func (sb *Sandbox) clearNetworkResources(origEp *Endpoint) error {
-	ep := sb.GetEndpoint(origEp.id)
-	if ep == nil {
-		return fmt.Errorf("could not find the sandbox endpoint data for endpoint %s",
-			origEp.id)
-	}
-
-	sb.mu.Lock()
-	osSbox := sb.osSbox
-	inDelete := sb.inDelete
-	sb.mu.Unlock()
-	if osSbox != nil {
-		releaseOSSboxResources(osSbox, ep)
-	}
-
-	sb.mu.Lock()
-	delete(sb.populatedEndpoints, ep.ID())
-
-	if len(sb.endpoints) == 0 {
-		// sb.endpoints should never be empty and this is unexpected error condition
-		// We log an error message to note this down for debugging purposes.
-		log.G(context.TODO()).Errorf("No endpoints in sandbox while trying to remove endpoint %s", ep.Name())
-		sb.mu.Unlock()
-		return nil
-	}
-
-	if !slices.Contains(sb.endpoints, ep) {
-		log.G(context.TODO()).Warnf("Endpoint %s has already been deleted", ep.Name())
-		sb.mu.Unlock()
-		return nil
-	}
-
-	gwepBefore4, gwepBefore6 := selectGatewayEndpoint(sb.endpoints)
-	sb.removeEndpointRaw(ep)
-	gwepAfter4, gwepAfter6 := selectGatewayEndpoint(sb.endpoints)
-	delete(sb.epPriority, ep.ID())
-
-	sb.mu.Unlock()
-
-	if (gwepAfter4 != nil && gwepBefore4 != gwepAfter4) || (gwepAfter6 != nil && gwepBefore6 != gwepAfter6) {
-		if err := sb.updateGateway(gwepAfter4, gwepAfter6); err != nil {
-			return fmt.Errorf("updating gateway endpoint: %w", err)
-		}
-	}
-
-	// Only update the store if we did not come here as part of
-	// sandbox delete. If we came here as part of delete then do
-	// not bother updating the store. The sandbox object will be
-	// deleted anyway
-	if !inDelete {
-		return sb.storeUpdate(context.TODO())
-	}
-
-	return nil
-}
-
 // Less defines an ordering over endpoints, with better candidates for the default
 // gateway sorted first.
 //