Skip to content

Commit

Permalink
Restore the SetKey prestart hook.
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
robmry committed Mar 27, 2024
1 parent 8b79278 commit 1014f48
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 32 deletions.
24 changes: 24 additions & 0 deletions daemon/oci_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
Expand Down
8 changes: 2 additions & 6 deletions daemon/start_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@ 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
Expand All @@ -22,9 +20,7 @@ func (daemon *Daemon) initializeCreatedTask(ctx context.Context, tsk types.Task,
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
Expand Down
28 changes: 28 additions & 0 deletions integration/networking/bridge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
40 changes: 26 additions & 14 deletions libnetwork/osl/namespace_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
40 changes: 28 additions & 12 deletions libnetwork/sandbox_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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.
Expand Down Expand Up @@ -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))
Expand Down

0 comments on commit 1014f48

Please sign in to comment.