Delay Endpoint config until the osSbox exists

When the SetKey hook is used (by a build container) it's called after
Endpoint.sbJoin, which will have called Sandbox.populateNetworkResources
to set up address, routes, sysctls and so on - but it's not able to do
any config until the osSbox exists. So, Sandbox.populateNetworkResources
is called again by SetKey to finish that config.

But, that means the rest of Endpoint.sbJoin has already happened before
the osSbox existed - it will have configured DNS, /etc/hosts, gateways
and so on before anything was set up for the OS.

So, if the osSbox configuration isn't applied as expected (for example,
a sysctl disables IPv6 on the endpoint), that sbJoin configuration is
incorrect.

To avoid unnecessary config+cleanup in thoses cases - delay the config
currently done by sbJoin until the osSbox exists.

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

daemon/libnetwork/endpoint.go

@@ -560,22 +560,42 @@ func (ep *Endpoint) sbJoin(ctx context.Context, sb *Sandbox, options ...Endpoint
 		return err
 	}
 
-	// Current endpoint(s) providing external connectivity for the sandbox
+	// Current endpoint(s) providing external connectivity for the Sandbox.
+	// If ep is selected as a gateway endpoint once it's been added to the Sandbox,
+	// these are the endpoints that need to be un-gateway'd.
 	gwepBefore4, gwepBefore6 := sb.getGatewayEndpoint()
 
 	sb.addEndpoint(ep)
 
-	if err := sb.populateNetworkResources(ctx, ep); err != nil {
-		return err
-	}
-
-	if err := addEpToResolver(ctx, n.Name(), ep.Name(), &sb.config, ep.iface, n.Resolvers()); err != nil {
-		return errdefs.System(err)
+	// For Linux, at this point, in most cases, the container task has been created
+	// and the container's network namespace (sb.osSbox) is ready to be configured
+	// with addresses, routes and so on. The exception is when the SetKey re-exec is
+	// used by a build container. In that case, the osSbox doesn't exist yet. So,
+	// stop here and SetKey will finish off the configuration when it's ready.
+	// For Windows, canPopulateNetworkResources() is always true.
+	if sb.canPopulateNetworkResources() {
+		if err := sb.populateNetworkResources(ctx, ep); err != nil {
+			return err
+		}
+		if err := ep.populateNetworkResources(ctx, sb); err != nil {
+			return err
+		}
+		if err := ep.updateExternalConnectivity(ctx, sb, gwepBefore4, gwepBefore6); err != nil {
+			return err
+		}
 	}
 
 	if err := n.getController().storeEndpoint(ctx, ep); err != nil {
 		return err
 	}
+	return nil
+}
+
+func (ep *Endpoint) populateNetworkResources(ctx context.Context, sb *Sandbox) (retErr error) {
+	n := ep.getNetwork()
+	if err := addEpToResolver(ctx, n.Name(), ep.Name(), &sb.config, ep.iface, n.Resolvers()); err != nil {
+		return errdefs.System(err)
+	}
 
 	if err := ep.addDriverInfoToCluster(); err != nil {
 		return err
@@ -603,7 +623,14 @@ func (ep *Endpoint) sbJoin(ctx context.Context, sb *Sandbox, options ...Endpoint
 	if sb.resolver != nil {
 		sb.resolver.SetForwardingPolicy(sb.hasExternalAccess())
 	}
+	return nil
+}
 
+// updateExternalConnectivity configures an Endpoint when it becomes the gateway
+// endpoint for a network, revoking external connectivity from the previous gateway
+// endpoints, if necessary. (It does not update the Sandbox's default gateway, the
+// Sandbox takes care of that. This is just about network driver config.)
+func (ep *Endpoint) updateExternalConnectivity(ctx context.Context, sb *Sandbox, gwepBefore4, gwepBefore6 *Endpoint) (retErr error) {
 	gwepAfter4, gwepAfter6 := sb.getGatewayEndpoint()
 
 	log.G(ctx).Infof("sbJoin: gwep4 '%s'->'%s', gwep6 '%s'->'%s'",

daemon/libnetwork/sandbox_dns_windows.go

@@ -17,9 +17,7 @@ func (sb *Sandbox) restoreHostsPath() {}
 
 func (sb *Sandbox) restoreResolvConfPath() {}
 
-func (sb *Sandbox) addHostsEntries(_ context.Context, ifaceIP []netip.Addr) error {
-	return nil
-}
+func (sb *Sandbox) addHostsEntries(_ context.Context, ifaceIP []netip.Addr) {}
 
 func (sb *Sandbox) deleteHostsEntries(ifaceAddrs []netip.Addr) {}
 

daemon/libnetwork/sandbox_linux.go

@@ -161,6 +161,16 @@ func (sb *Sandbox) SetKey(ctx context.Context, basePath string) error {
 	oldosSbox := sb.osSbox
 	sb.mu.Unlock()
 
+	osSbox, err := osl.GetSandboxForExternalKey(basePath, sb.Key())
+	if err != nil {
+		return err
+	}
+
+	// Make sure the list of endpoints is stable while configuring them and selecting a
+	// gateway endpoint. Endpoints added after sbJoin will handle their setup.
+	sb.joinLeaveMu.Lock()
+	defer sb.joinLeaveMu.Unlock()
+
 	if oldosSbox != nil {
 		// If we already have an OS sandbox, release the network resources from that
 		// and destroy the OS snab. We are moving into a new home further down. Note that none
@@ -170,11 +180,6 @@ func (sb *Sandbox) SetKey(ctx context.Context, basePath string) error {
 		}
 	}
 
-	osSbox, err := osl.GetSandboxForExternalKey(basePath, sb.Key())
-	if err != nil {
-		return err
-	}
-
 	sb.mu.Lock()
 	sb.osSbox = osSbox
 	sb.mu.Unlock()
@@ -198,11 +203,11 @@ func (sb *Sandbox) SetKey(ctx context.Context, basePath string) error {
 		return err
 	}
 
-	for _, ep := range sb.Endpoints() {
-		if err = sb.populateNetworkResources(ctx, ep); err != nil {
-			return err
-		}
-	}
+	// If the Sandbox already has endpoints it's because sbJoin has been called for
+	// them - but configuration of addresses/routes (and so on) didn't complete because
+	// there was nowhere for it to go before the osSbox was set up. So, finish that
+	// configuration now.
+	sb.finishEndpointConfig(ctx)
 
 	return nil
 }
@@ -314,6 +319,42 @@ func (sb *Sandbox) restoreOslSandbox() error {
 	return nil
 }
 
+// finishEndpointConfig is to finish configuration of any Endpoint that was added to the
+// Sandbox (via sbJoin) before sb.osSbox had been set up.
+func (sb *Sandbox) finishEndpointConfig(ctx context.Context) error {
+	eps := sb.Endpoints()
+	if len(eps) == 0 {
+		return nil
+	}
+	for _, ep := range eps {
+		if err := sb.populateNetworkResources(ctx, ep); err != nil {
+			return err
+		}
+		if err := ep.populateNetworkResources(ctx, sb); err != nil {
+			return err
+		}
+	}
+
+	gwep4, gwep6 := sb.getGatewayEndpoint()
+	if gwep4 != nil {
+		if err := gwep4.updateExternalConnectivity(ctx, sb, nil, nil); err != nil {
+			return err
+		}
+	}
+	if gwep6 != nil && gwep6 != gwep4 {
+		if err := gwep6.updateExternalConnectivity(ctx, sb, nil, nil); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (sb *Sandbox) canPopulateNetworkResources() bool {
+	sb.mu.Lock()
+	defer sb.mu.Unlock()
+	return sb.osSbox != nil
+}
+
 func (sb *Sandbox) populateNetworkResources(ctx context.Context, ep *Endpoint) error {
 	ctx, span := otel.Tracer("").Start(ctx, "libnetwork.Sandbox.populateNetworkResources", trace.WithAttributes(
 		attribute.String("endpoint.Name", ep.Name())))
@@ -322,7 +363,7 @@ func (sb *Sandbox) populateNetworkResources(ctx context.Context, ep *Endpoint) e
 	sb.mu.Lock()
 	if sb.osSbox == nil {
 		sb.mu.Unlock()
-		return nil
+		return fmt.Errorf("cannot populate network resources for container %s, no osSbox", sb.ContainerID())
 	}
 	inDelete := sb.inDelete
 	sb.mu.Unlock()

daemon/libnetwork/sandbox_windows.go

@@ -33,6 +33,10 @@ func (sb *Sandbox) NetnsPath() (path string, ok bool) {
 	return "", false
 }
 
+func (sb *Sandbox) canPopulateNetworkResources() bool {
+	return true
+}
+
 func (sb *Sandbox) populateNetworkResources(context.Context, *Endpoint) error {
 	// not implemented on Windows (Sandbox.osSbox is always nil)
 	return nil