From ddf10ee1cdbe322cfa196424c2c7e663d4be663b Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Tue, 14 Jan 2025 16:36:49 +0000 Subject: [PATCH] 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 --- daemon/libnetwork/endpoint.go | 41 ++++++++++++--- daemon/libnetwork/sandbox_dns_windows.go | 4 +- daemon/libnetwork/sandbox_linux.go | 63 +++++++++++++++++++----- daemon/libnetwork/sandbox_windows.go | 4 ++ 4 files changed, 91 insertions(+), 21 deletions(-) diff --git a/daemon/libnetwork/endpoint.go b/daemon/libnetwork/endpoint.go index 0b23bf2491..45271b479b 100644 --- a/daemon/libnetwork/endpoint.go +++ b/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'", diff --git a/daemon/libnetwork/sandbox_dns_windows.go b/daemon/libnetwork/sandbox_dns_windows.go index 086d6e8b3d..e704995318 100644 --- a/daemon/libnetwork/sandbox_dns_windows.go +++ b/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) {} diff --git a/daemon/libnetwork/sandbox_linux.go b/daemon/libnetwork/sandbox_linux.go index 314c031b4f..8ba79c928d 100644 --- a/daemon/libnetwork/sandbox_linux.go +++ b/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() diff --git a/daemon/libnetwork/sandbox_windows.go b/daemon/libnetwork/sandbox_windows.go index b4cba9c3c2..3690011981 100644 --- a/daemon/libnetwork/sandbox_windows.go +++ b/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