Restore the SetKey prestart hook.

Partially reverts 0046b16 "daemon: set libnetwork sandbox key w/o OCI hook"

Running SetKey to store the OCI Sandbox key after task creation, rather
than from the OCI prestart hook, meant it happened after sysctl settings
were applied by the runtime - which was the intention, we wanted to
complete Sandbox configuration after IPv6 had been disabled by a sysctl
if that was going to happen.

But, it meant '--sysctl' options for a specfic network interface caused
container task creation to fail, because the interface is only moved into
the network namespace during SetKey.

This change restores the SetKey prestart hook, and regenerates config
files that depend on the container's support for IPv6 after the task has
been created. It also adds a regression test that makes sure it's possible
to set an interface-specfic sysctl.

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

daemon/oci_linux.go

@@ -24,6 +24,7 @@ import (
 	"github.com/docker/docker/oci/caps"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/rootless/specconv"
+	"github.com/docker/docker/pkg/stringid"
 	volumemounts "github.com/docker/docker/volume/mounts"
 	"github.com/moby/sys/mount"
 	"github.com/moby/sys/mountinfo"
@@ -60,6 +61,28 @@ func withRlimits(daemon *Daemon, daemonCfg *dconfig.Config, c *container.Contain
 	}
 }
 
+// withLibnetwork sets the libnetwork hook
+func withLibnetwork(daemon *Daemon, daemonCfg *dconfig.Config, c *container.Container) coci.SpecOpts {
+	return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error {
+		if c.Config.NetworkDisabled {
+			return nil
+		}
+		for _, ns := range s.Linux.Namespaces {
+			if ns.Type == specs.NetworkNamespace && ns.Path == "" {
+				if s.Hooks == nil {
+					s.Hooks = &specs.Hooks{}
+				}
+				shortNetCtlrID := stringid.TruncateID(daemon.netController.ID())
+				s.Hooks.Prestart = append(s.Hooks.Prestart, specs.Hook{
+					Path: filepath.Join("/proc", strconv.Itoa(os.Getpid()), "exe"),
+					Args: []string{"libnetwork-setkey", "-exec-root=" + daemonCfg.GetExecRoot(), c.ID, shortNetCtlrID},
+				})
+			}
+		}
+		return nil
+	}
+}
+
 // withRootless sets the spec to the rootless configuration
 func withRootless(daemon *Daemon, daemonCfg *dconfig.Config) coci.SpecOpts {
 	return func(_ context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error {
@@ -1015,6 +1038,7 @@ func (daemon *Daemon) createSpec(ctx context.Context, daemonCfg *configStore, c
 		WithCapabilities(c),
 		WithSeccomp(daemon, c),
 		withMounts(daemon, daemonCfg, c, mounts),
+		withLibnetwork(daemon, &daemonCfg.Config, c),
 		WithApparmor(c),
 		WithSelinux(c),
 		WithOOMScore(&c.HostConfig.OomScoreAdj),

daemon/start.go

@@ -195,7 +195,7 @@ func (daemon *Daemon) containerStart(ctx context.Context, daemonCfg *configStore
 		}
 	}()
 
-	if err := daemon.initializeCreatedTask(ctx, tsk, container, spec); err != nil {
+	if err := daemon.initializeCreatedContainer(container, spec); err != nil {
 		return err
 	}
 

daemon/start_linux.go

@@ -1,30 +1,23 @@
 package daemon // import "github.com/docker/docker/daemon"
 
 import (
-	"context"
-	"fmt"
-
-	specs "github.com/opencontainers/runtime-spec/specs-go"
-
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/errdefs"
-	"github.com/docker/docker/libcontainerd/types"
 	"github.com/docker/docker/oci"
+	specs "github.com/opencontainers/runtime-spec/specs-go"
 )
 
 // initializeCreatedTask performs any initialization that needs to be done to
-// prepare a freshly-created task to be started.
-func (daemon *Daemon) initializeCreatedTask(ctx context.Context, tsk types.Task, container *container.Container, spec *specs.Spec) error {
+// prepare a freshly-created container to be started.
+func (daemon *Daemon) initializeCreatedContainer(container *container.Container, spec *specs.Spec) error {
 	if !container.Config.NetworkDisabled {
 		nspath, ok := oci.NamespacePath(spec, specs.NetworkNamespace)
 		if ok && nspath == "" { // the runtime has been instructed to create a new network namespace for tsk.
 			sb, err := daemon.netController.GetSandbox(container.ID)
 			if err != nil {
 				return errdefs.System(err)
 			}
-			if err := sb.SetKey(fmt.Sprintf("/proc/%d/ns/net", tsk.Pid())); err != nil {
-				return errdefs.System(err)
-			}
+			return sb.FinishConfig()
 		}
 	}
 	return nil

daemon/start_notlinux.go

@@ -3,15 +3,12 @@
 package daemon // import "github.com/docker/docker/daemon"
 
 import (
-	"context"
-
 	"github.com/docker/docker/container"
-	"github.com/docker/docker/libcontainerd/types"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 )
 
 // initializeCreatedTask performs any initialization that needs to be done to
-// prepare a freshly-created task to be started.
-func (daemon *Daemon) initializeCreatedTask(ctx context.Context, tsk types.Task, container *container.Container, spec *specs.Spec) error {
+// prepare a freshly-created container to be started.
+func (daemon *Daemon) initializeCreatedContainer(container *container.Container, spec *specs.Spec) error {
 	return nil
 }

integration/networking/bridge_test.go

@@ -675,3 +675,31 @@ func TestDisableIPv6Addrs(t *testing.T) {
 		})
 	}
 }
+
+// Test that it's possible to set a sysctl on an interface in the container.
+// Regression test for https://github.com/moby/moby/issues/47619
+func TestSetInterfaceSysctl(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows", "no sysctl on Windows")
+
+	ctx := setupTest(t)
+	d := daemon.New(t)
+	d.StartWithBusybox(ctx, t)
+	defer d.Stop(t)
+
+	c := d.NewClientT(t)
+	defer c.Close()
+
+	const scName = "net.ipv4.conf.eth0.forwarding"
+	opts := []func(config *container.TestContainerConfig){
+		container.WithCmd("sysctl", scName),
+		container.WithSysctls(map[string]string{scName: "1"}),
+	}
+
+	runRes := container.RunAttach(ctx, t, c, opts...)
+	defer c.ContainerRemove(ctx, runRes.ContainerID,
+		containertypes.RemoveOptions{Force: true},
+	)
+
+	stdout := runRes.Stdout.String()
+	assert.Check(t, is.Contains(stdout, scName))
+}

libnetwork/osl/namespace_linux.go

@@ -545,26 +545,38 @@ func (n *Namespace) Restore(interfaces map[Iface][]IfaceOption, routes []*types.
 	return nil
 }
 
-// IPv6LoEnabled checks whether the loopback interface has an IPv6 address ('::1'
-// is assigned by the kernel if IPv6 is enabled).
+// IPv6LoEnabled returns true if the loopback interface had an IPv6 address when
+// last checked. It's always checked on the first call, and by RefreshIPv6LoEnabled.
+// ('::1' is assigned by the kernel if IPv6 is enabled.)
 func (n *Namespace) IPv6LoEnabled() bool {
 	n.ipv6LoEnabledOnce.Do(func() {
-		// If anything goes wrong, assume no-IPv6.
-		iface, err := n.nlHandle.LinkByName("lo")
-		if err != nil {
-			log.G(context.TODO()).WithError(err).Warn("Unable to find 'lo' to determine IPv6 support")
-			return
-		}
-		addrs, err := n.nlHandle.AddrList(iface, nl.FAMILY_V6)
-		if err != nil {
-			log.G(context.TODO()).WithError(err).Warn("Unable to get 'lo' addresses to determine IPv6 support")
-			return
-		}
-		n.ipv6LoEnabledCached = len(addrs) > 0
+		n.RefreshIPv6LoEnabled()
 	})
+	n.mu.Lock()
+	defer n.mu.Unlock()
 	return n.ipv6LoEnabledCached
 }
 
+// RefreshIPv6LoEnabled refreshes the cached result returned by IPv6LoEnabled.
+func (n *Namespace) RefreshIPv6LoEnabled() {
+	n.mu.Lock()
+	defer n.mu.Unlock()
+
+	// If anything goes wrong, assume no-IPv6.
+	n.ipv6LoEnabledCached = false
+	iface, err := n.nlHandle.LinkByName("lo")
+	if err != nil {
+		log.G(context.TODO()).WithError(err).Warn("Unable to find 'lo' to determine IPv6 support")
+		return
+	}
+	addrs, err := n.nlHandle.AddrList(iface, nl.FAMILY_V6)
+	if err != nil {
+		log.G(context.TODO()).WithError(err).Warn("Unable to get 'lo' addresses to determine IPv6 support")
+		return
+	}
+	n.ipv6LoEnabledCached = len(addrs) > 0
+}
+
 // ApplyOSTweaks applies operating system specific knobs on the sandbox.
 func (n *Namespace) ApplyOSTweaks(types []SandboxType) {
 	for _, t := range types {

libnetwork/sandbox_linux.go

@@ -90,12 +90,8 @@ func (sb *Sandbox) updateGateway(ep *Endpoint) error {
 		return fmt.Errorf("failed to set gateway while updating gateway: %v", err)
 	}
 
-	// If IPv6 has been disabled in the sandbox a gateway may still have been
-	// configured, don't attempt to apply it.
-	if ipv6, ok := sb.ipv6Enabled(); !ok || ipv6 {
-		if err := osSbox.SetGatewayIPv6(joinInfo.gw6); err != nil {
-			return fmt.Errorf("failed to set IPv6 gateway while updating gateway: %v", err)
-		}
+	if err := osSbox.SetGatewayIPv6(joinInfo.gw6); err != nil {
+		return fmt.Errorf("failed to set IPv6 gateway while updating gateway: %v", err)
 	}
 
 	return nil
@@ -162,6 +158,10 @@ func (sb *Sandbox) SetKey(basePath string) error {
 		}
 	}
 
+	// Set up hosts and resolv.conf files. IPv6 support in the container can't be
+	// determined yet, as sysctls haven't been applied by the runtime. Calling
+	// FinishInit after the container task has been created, when sysctls have been
+	// applied will regenerate these files.
 	if err := sb.finishInitDNS(); err != nil {
 		return err
 	}
@@ -175,6 +175,27 @@ func (sb *Sandbox) SetKey(basePath string) error {
 	return nil
 }
 
+// FinishConfig completes Sandbox configuration. If called after the container task has been
+// created, and sysctl settings applied, the configuration will be based on the container's
+// IPv6 support.
+func (sb *Sandbox) FinishConfig() error {
+	if sb.config.useDefaultSandBox {
+		return nil
+	}
+
+	sb.mu.Lock()
+	osSbox := sb.osSbox
+	sb.mu.Unlock()
+	if osSbox == nil {
+		return nil
+	}
+
+	// If sysctl changes have been made, IPv6 may have been enabled/disabled since last checked.
+	osSbox.RefreshIPv6LoEnabled()
+
+	return sb.finishInitDNS()
+}
+
 // IPv6 support can always be determined for host networking. For other network
 // types it can only be determined once there's a container namespace to probe,
 // return ok=false in that case.
@@ -283,12 +304,7 @@ func (sb *Sandbox) populateNetworkResources(ep *Endpoint) error {
 
 		ifaceOptions = append(ifaceOptions, osl.WithIPv4Address(i.addr), osl.WithRoutes(i.routes))
 		if i.addrv6 != nil && i.addrv6.IP.To16() != nil {
-			// If IPv6 has been disabled in the Sandbox, an IPv6 address will still have
-			// been allocated. Don't apply it, because doing so would enable IPv6 on the
-			// interface.
-			if ipv6, ok := sb.ipv6Enabled(); !ok || ipv6 {
-				ifaceOptions = append(ifaceOptions, osl.WithIPv6Address(i.addrv6))
-			}
+			ifaceOptions = append(ifaceOptions, osl.WithIPv6Address(i.addrv6))
 		}
 		if len(i.llAddrs) != 0 {
 			ifaceOptions = append(ifaceOptions, osl.WithLinkLocalAddresses(i.llAddrs))