From 95abde479ab87a998d3716b63e9d0a1691fbb0f6 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 20 Aug 2023 12:48:09 +0200 Subject: [PATCH] libnetwork: implement Controller.setupOSLSandbox osl.NewSandbox() always returns a nil interface on Windows (and other non-Linux platforms). This means that any code that these fields are always nil, and any code using these fields must be considered Linux-only; - libnetwork/Controller.defOsSbox - libnetwork/Sandbox.osSbox Ideally, these fields would live in Linux-only files, but they're referenced in various platform-neutral parts of the code, so let's start with moving the initialization code to Linux-only files. Signed-off-by: Sebastiaan van Stijn --- libnetwork/controller.go | 40 ++++------------------- libnetwork/controller_linux.go | 57 +++++++++++++++++++++++++++++++++ libnetwork/controller_others.go | 4 +++ libnetwork/sandbox_store.go | 3 +- 4 files changed, 69 insertions(+), 35 deletions(-) diff --git a/libnetwork/controller.go b/libnetwork/controller.go index ce9c684e27..257123b881 100644 --- a/libnetwork/controller.go +++ b/libnetwork/controller.go @@ -92,9 +92,7 @@ type Controller struct { svcRecords map[string]*svcInfo nmap map[string]*netWatch serviceBindings map[serviceKey]*service - defOsSbox osl.Sandbox ingressSandbox *Sandbox - sboxOnce sync.Once agent *nwAgent networkLocker *locker.Locker agentInitDone chan struct{} @@ -102,6 +100,10 @@ type Controller struct { keys []*types.EncryptionKey DiagnosticServer *diagnostic.Server mu sync.Mutex + + // FIXME(thaJeztah): defOsSbox is always nil on non-Linux: move these fields to Linux-only files. + defOsSboxOnce sync.Once + defOsSbox osl.Sandbox } // New creates a new instance of network controller. @@ -937,38 +939,8 @@ func (c *Controller) NewSandbox(containerID string, options ...SandboxOption) (_ if err := sb.setupResolutionFiles(); err != nil { return nil, err } - - if sb.config.useDefaultSandBox { - var err error - c.sboxOnce.Do(func() { - c.defOsSbox, err = osl.NewSandbox(sb.Key(), false, false) - }) - - if err != nil { - c.sboxOnce = sync.Once{} - return nil, fmt.Errorf("failed to create default sandbox: %v", err) - } - - sb.osSbox = c.defOsSbox - } - - if sb.osSbox == nil && !sb.config.useExternalKey { - var err error - if sb.osSbox, err = osl.NewSandbox(sb.Key(), !sb.config.useDefaultSandBox, false); err != nil { - return nil, fmt.Errorf("failed to create new osl sandbox: %v", err) - } - } - - if sb.osSbox != nil { - // Apply operating specific knobs on the load balancer sandbox - err := sb.osSbox.InvokeFunc(func() { - sb.osSbox.ApplyOSTweaks(sb.oslTypes) - }) - if err != nil { - log.G(context.TODO()).Errorf("Failed to apply performance tuning sysctls to the sandbox: %v", err) - } - // Keep this just so performance is not changed - sb.osSbox.ApplyOSTweaks(sb.oslTypes) + if err := c.setupOSLSandbox(sb); err != nil { + return nil, err } c.mu.Lock() diff --git a/libnetwork/controller_linux.go b/libnetwork/controller_linux.go index 2039b23d1a..91ae46e3e7 100644 --- a/libnetwork/controller_linux.go +++ b/libnetwork/controller_linux.go @@ -1,9 +1,15 @@ package libnetwork import ( + "context" + "fmt" + "sync" + + "github.com/containerd/containerd/log" "github.com/docker/docker/libnetwork/iptables" "github.com/docker/docker/libnetwork/netlabel" "github.com/docker/docker/libnetwork/options" + "github.com/docker/docker/libnetwork/osl" ) // enabledIptablesVersions returns the iptables versions that are enabled @@ -31,3 +37,54 @@ func (c *Controller) enabledIptablesVersions() []iptables.IPVersion { } return versions } + +// getDefaultOSLSandbox returns the controller's default [osl.Sandbox]. It +// creates the sandbox if it does not yet exist. +func (c *Controller) getDefaultOSLSandbox(key string) (osl.Sandbox, error) { + var err error + c.defOsSboxOnce.Do(func() { + c.defOsSbox, err = osl.NewSandbox(key, false, false) + }) + + if err != nil { + c.defOsSboxOnce = sync.Once{} + return nil, fmt.Errorf("failed to create default sandbox: %v", err) + } + return c.defOsSbox, nil +} + +// setupOSLSandbox sets the sandbox [osl.Sandbox], and applies operating- +// specific configuration. +// +// Depending on the Sandbox settings, it may either use the Controller's +// default sandbox, or configure a new one. +func (c *Controller) setupOSLSandbox(sb *Sandbox) error { + if sb.config.useDefaultSandBox { + defSB, err := c.getDefaultOSLSandbox(sb.Key()) + if err != nil { + return err + } + sb.osSbox = defSB + } + + if sb.osSbox == nil && !sb.config.useExternalKey { + newSB, err := osl.NewSandbox(sb.Key(), !sb.config.useDefaultSandBox, false) + if err != nil { + return fmt.Errorf("failed to create new osl sandbox: %v", err) + } + sb.osSbox = newSB + } + + if sb.osSbox != nil { + // Apply operating specific knobs on the load balancer sandbox + err := sb.osSbox.InvokeFunc(func() { + sb.osSbox.ApplyOSTweaks(sb.oslTypes) + }) + if err != nil { + log.G(context.TODO()).Errorf("Failed to apply performance tuning sysctls to the sandbox: %v", err) + } + // Keep this just so performance is not changed + sb.osSbox.ApplyOSTweaks(sb.oslTypes) + } + return nil +} diff --git a/libnetwork/controller_others.go b/libnetwork/controller_others.go index cb0e778071..d837c7cf3d 100644 --- a/libnetwork/controller_others.go +++ b/libnetwork/controller_others.go @@ -6,3 +6,7 @@ package libnetwork func (c *Controller) enabledIptablesVersions() []any { return nil } + +func (c *Controller) setupOSLSandbox(_ *Sandbox) error { + return nil +} diff --git a/libnetwork/sandbox_store.go b/libnetwork/sandbox_store.go index cb9d0b3fb1..08cee95668 100644 --- a/libnetwork/sandbox_store.go +++ b/libnetwork/sandbox_store.go @@ -266,7 +266,8 @@ func (c *Controller) sandboxCleanup(activeSandboxes map[string]interface{}) { continue } } else { - c.sboxOnce.Do(func() { + // FIXME(thaJeztah): osSbox (and thus defOsSbox) is always nil on non-Linux: move this code to Linux-only files. + c.defOsSboxOnce.Do(func() { c.defOsSbox = sb.osSbox }) }