Browse code

Put clearNetworkResources() inline in its only caller

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

Rob Murray authored on 2025/09/11 18:27:34
Showing 3 changed files
... ...
@@ -176,17 +176,16 @@ func (c *Controller) defaultGwNetwork() (*Network, error) {
176 176
 	return n, err
177 177
 }
178 178
 
179
+func (sb *Sandbox) isGatewayEndpoint(epId string) bool {
180
+	gw4, gw6 := sb.getGatewayEndpoint()
181
+	return (gw4 != nil && epId == gw4.ID()) || (gw6 != nil && epId == gw6.ID())
182
+}
183
+
179 184
 // getGatewayEndpoint returns the endpoints providing external connectivity to
180 185
 // the sandbox. If the gateway is dual-stack, ep4 and ep6 will point at the same
181 186
 // endpoint. If there is no IPv4/IPv6 connectivity, nil pointers will be returned.
182 187
 func (sb *Sandbox) getGatewayEndpoint() (ep4, ep6 *Endpoint) {
183
-	return selectGatewayEndpoint(sb.Endpoints())
184
-}
185
-
186
-// selectGatewayEndpoint is like getGatewayEndpoint, but selects only from
187
-// endpoints.
188
-func selectGatewayEndpoint(endpoints []*Endpoint) (ep4, ep6 *Endpoint) {
189
-	for _, ep := range endpoints {
188
+	for _, ep := range sb.Endpoints() {
190 189
 		if ep.getNetwork().Type() == "null" || ep.getNetwork().Type() == "host" {
191 190
 			continue
192 191
 		}
... ...
@@ -820,17 +820,38 @@ func (ep *Endpoint) sbLeave(ctx context.Context, sb *Sandbox, n *Network, force
820 820
 	// Capture the addresses that were added to the container's /etc/hosts here,
821 821
 	// before the endpoint is deleted, so that they can be removed from /etc/hosts.
822 822
 	etcHostsAddrs := ep.getEtcHostsAddrs()
823
-
824
-	if err := sb.clearNetworkResources(ep); err != nil {
825
-		log.G(ctx).WithError(err).Warn("Failed to clean up network resources on container disconnect")
823
+	// Before removing the Endpoint from the Sandbox's list of endpoints, check whether
824
+	// it's acting as a gateway so that new gateways can be selected if it is.
825
+	wasGwEp := sb.isGatewayEndpoint(ep.id)
826
+
827
+	// Remove the sb's references to ep.
828
+	sb.mu.Lock()
829
+	osSbox := sb.osSbox
830
+	delete(sb.populatedEndpoints, ep.id)
831
+	delete(sb.epPriority, ep.id)
832
+	sb.endpoints = slices.DeleteFunc(sb.endpoints, func(other *Endpoint) bool { return other.id == ep.id })
833
+	sb.mu.Unlock()
834
+
835
+	// Delete interfaces, routes etc. from the OS.
836
+	if osSbox != nil {
837
+		releaseOSSboxResources(osSbox, ep)
838
+
839
+		// Even if the interface was initially created in the container's namespace, it's
840
+		// now been moved out. When a legacy link is deleted, the Endpoint is removed and
841
+		// then re-added to the Sandbox. So, to make sure the re-add works, note that the
842
+		// interface is now outside the container's netns.
843
+		ep.iface.createdInContainer = false
844
+	}
845
+
846
+	// Update gateway / static routes if the ep was the gateway.
847
+	var gwepAfter4, gwepAfter6 *Endpoint
848
+	if wasGwEp {
849
+		gwepAfter4, gwepAfter6 = sb.getGatewayEndpoint()
850
+		if err := sb.updateGateway(gwepAfter4, gwepAfter6); err != nil {
851
+			return fmt.Errorf("updating gateway endpoint: %w", err)
852
+		}
826 853
 	}
827 854
 
828
-	// Even if the interface was initially created in the container's namespace, it's
829
-	// now been moved out. When a legacy link is deleted, the Endpoint is removed and
830
-	// then re-added to the Sandbox. So, to make sure the re-add works, note that the
831
-	// interface is now outside the container's netns.
832
-	ep.iface.createdInContainer = false
833
-
834 855
 	// Update the store about the sandbox detach only after we
835 856
 	// have completed sb.clearNetworkResources above to avoid
836 857
 	// spurious logs when cleaning up the sandbox when the daemon
... ...
@@ -869,16 +890,17 @@ func (ep *Endpoint) sbLeave(ctx context.Context, sb *Sandbox, n *Network, force
869 869
 		sb.resolver.SetForwardingPolicy(sb.hasExternalAccess())
870 870
 	}
871 871
 
872
-	// Find new endpoint(s) to provide external connectivity for the sandbox.
873
-	gwepAfter4, gwepAfter6 := sb.getGatewayEndpoint()
874
-	if gwepAfter4 != nil {
875
-		if err := gwepAfter4.programExternalConnectivity(ctx, gwepAfter4, gwepAfter6); err != nil {
876
-			log.G(ctx).WithError(err).Error("Failed to set IPv4 gateway")
872
+	// Configure the endpoints that now provide external connectivity for the sandbox.
873
+	if wasGwEp {
874
+		if gwepAfter4 != nil {
875
+			if err := gwepAfter4.programExternalConnectivity(ctx, gwepAfter4, gwepAfter6); err != nil {
876
+				log.G(ctx).WithError(err).Error("Failed to set IPv4 gateway")
877
+			}
877 878
 		}
878
-	}
879
-	if gwepAfter6 != nil && gwepAfter6 != gwepAfter4 {
880
-		if err := gwepAfter6.programExternalConnectivity(ctx, gwepAfter4, gwepAfter6); err != nil {
881
-			log.G(ctx).WithError(err).Error("Failed to set IPv6 gateway")
879
+		if gwepAfter6 != nil && gwepAfter6 != gwepAfter4 {
880
+			if err := gwepAfter6.programExternalConnectivity(ctx, gwepAfter4, gwepAfter6); err != nil {
881
+				log.G(ctx).WithError(err).Error("Failed to set IPv6 gateway")
882
+			}
882 883
 		}
883 884
 	}
884 885
 
... ...
@@ -319,14 +319,10 @@ func (sb *Sandbox) addEndpoint(ep *Endpoint) {
319 319
 	sb.mu.Lock()
320 320
 	defer sb.mu.Unlock()
321 321
 
322
-	l := len(sb.endpoints)
323
-	i := sort.Search(l, func(j int) bool {
322
+	i := sort.Search(len(sb.endpoints), func(j int) bool {
324 323
 		return ep.Less(sb.endpoints[j])
325 324
 	})
326
-
327
-	sb.endpoints = append(sb.endpoints, nil)
328
-	copy(sb.endpoints[i+1:], sb.endpoints[i:])
329
-	sb.endpoints[i] = ep
325
+	sb.endpoints = slices.Insert(sb.endpoints, i, ep)
330 326
 }
331 327
 
332 328
 func (sb *Sandbox) removeEndpoint(ep *Endpoint) {
... ...
@@ -574,62 +570,6 @@ func (sb *Sandbox) DisableService() error {
574 574
 	return nil
575 575
 }
576 576
 
577
-func (sb *Sandbox) clearNetworkResources(origEp *Endpoint) error {
578
-	ep := sb.GetEndpoint(origEp.id)
579
-	if ep == nil {
580
-		return fmt.Errorf("could not find the sandbox endpoint data for endpoint %s",
581
-			origEp.id)
582
-	}
583
-
584
-	sb.mu.Lock()
585
-	osSbox := sb.osSbox
586
-	inDelete := sb.inDelete
587
-	sb.mu.Unlock()
588
-	if osSbox != nil {
589
-		releaseOSSboxResources(osSbox, ep)
590
-	}
591
-
592
-	sb.mu.Lock()
593
-	delete(sb.populatedEndpoints, ep.ID())
594
-
595
-	if len(sb.endpoints) == 0 {
596
-		// sb.endpoints should never be empty and this is unexpected error condition
597
-		// We log an error message to note this down for debugging purposes.
598
-		log.G(context.TODO()).Errorf("No endpoints in sandbox while trying to remove endpoint %s", ep.Name())
599
-		sb.mu.Unlock()
600
-		return nil
601
-	}
602
-
603
-	if !slices.Contains(sb.endpoints, ep) {
604
-		log.G(context.TODO()).Warnf("Endpoint %s has already been deleted", ep.Name())
605
-		sb.mu.Unlock()
606
-		return nil
607
-	}
608
-
609
-	gwepBefore4, gwepBefore6 := selectGatewayEndpoint(sb.endpoints)
610
-	sb.removeEndpointRaw(ep)
611
-	gwepAfter4, gwepAfter6 := selectGatewayEndpoint(sb.endpoints)
612
-	delete(sb.epPriority, ep.ID())
613
-
614
-	sb.mu.Unlock()
615
-
616
-	if (gwepAfter4 != nil && gwepBefore4 != gwepAfter4) || (gwepAfter6 != nil && gwepBefore6 != gwepAfter6) {
617
-		if err := sb.updateGateway(gwepAfter4, gwepAfter6); err != nil {
618
-			return fmt.Errorf("updating gateway endpoint: %w", err)
619
-		}
620
-	}
621
-
622
-	// Only update the store if we did not come here as part of
623
-	// sandbox delete. If we came here as part of delete then do
624
-	// not bother updating the store. The sandbox object will be
625
-	// deleted anyway
626
-	if !inDelete {
627
-		return sb.storeUpdate(context.TODO())
628
-	}
629
-
630
-	return nil
631
-}
632
-
633 577
 // Less defines an ordering over endpoints, with better candidates for the default
634 578
 // gateway sorted first.
635 579
 //