8000 Add per-endpoint sysctls to DriverOpts · moby/moby@93e43d5 · GitHub
[go: up one dir, main page]
Skip to content

Commit 93e43d5

Browse files
robmryrobmry
committed
Add per-endpoint sysctls to DriverOpts
Until now it's been possible to set per-interface sysctls using, for example, '--sysctl net.ipv6.conf.eth0.accept_ra=2'. But, the index in the interface name is allocated serially, and the numbering in a container with more than one interface may change when a container is restarted. The change to make it possible to connect a container to more than one network when it's created increased the ambiguity. This change adds label "com.docker.network.endpoint.sysctls" to the DriverOpts in EndpointSettings. This option is explicitly associated with the interface. Settings in "--sysctl" for "eth0" are migrated to DriverOpts. Because using "--sysctl" with any interface apart from "eth0" would have unpredictable results, it is now an error to use any other interface name in the top level "--sysctl" option. The error message includes a hint at how to use the new per-interface setting. The per-endpoint sysctl name is a shortened form of the sysctl name, intended to limit settings to 'net.*', and to eliminate the need to identify the interface by name. For example: net.ipv6.conf.eth0.accept_ra=2 becomes: ipv6.conf.accept_ra=2 The value of DriverOpts["com.docker.network.endpoint.sysctls"] is a comma separated list of these short-form sysctls. Settings from '--sysctl' are applied by the runtime lib during task creation. So, task creation fails if the endpoint does not exist. Applying per-endpoint settings during interface configuration means the endpoint can be created later, which paves the way for removal of the SetKey OCI prestart hook. Unlike other DriverOpts, the sysctl label itself is not driver-specific, but each driver has a chance to check settings/values and raise an error if a setting would cause it a problem - no such checks have been added in this initial version. As a future extension, if required, it would be possible for the driver to echo back valid/extended/modified settings to libnetwork for it to apply to the interface. (At that point, the syntax for the options could become driver specific to allow, for example, a driver to create more than one interface). Signed-off-by: Rob Murray <rob.murray@docker.com>
1 parent 6cbeb3f commit 93e43d5

File tree

9 files changed

+302
-3
lines changed

9 files changed

+302
-3
lines changed

‎api/server/router/container/container_routes.go‎

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/docker/docker/api/types/versions"
2424
containerpkg "github.com/docker/docker/container"
2525
"github.com/docker/docker/errdefs"
26+
"github.com/docker/docker/libnetwork/netlabel"
2627
"github.com/docker/docker/pkg/ioutils"
2728
"github.com/docker/docker/runconfig"
2829
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -619,6 +620,12 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
619620
warnings = append(warnings, warn)
620621
}
621622

623+
if warn, err := handleSysctlBC(hostConfig, networkingConfig, version); err != nil {
624+
return err
625+
} else if warn != "" {
626+
warnings = append(warnings, warn)
627+
}
628+
622629
if hostConfig.PidsLimit != nil && *hostConfig.PidsLimit <= 0 {
623630
// Don't set a limit if either no limit was specified, or "unlimited" was
624631
// explicitly set.
@@ -675,7 +682,7 @@ func handleMACAddressBC(config *container.Config, hostConfig *container.HostConf
675682
return "", nil
676683
}
677684
var warning string
678-
if hostConfig.NetworkMode.IsDefault() || hostConfig.NetworkMode.IsBridge() || hostConfig.NetworkMode.IsUserDefined() {
685+
if hostConfig.NetworkMode.IsBridge() || hostConfig.NetworkMode.IsUserDefined() {
679686
ep, err := epConfigForNetMode(version, hostConfig.NetworkMode, networkingConfig)
680687
if err != nil {
681688
return "", errors.Wrap(err, "unable to migrate container-wide MAC address to a specific network")
@@ -694,6 +701,92 @@ func handleMACAddressBC(config *container.Config, hostConfig *container.HostConf
694701
return warning, nil
695702
}
696703

704+
// handleSysctlBC migrates top level network endpoint-specific '--sysctl'
705+
// settings to an DriverOpts for an endpoint. This is necessary because sysctls
706+
// are applied during container task creation, but sysctls that name an interface
707+
// (for example 'net.ipv6.conf.eth0.forwarding') cannot be applied until the
708+
// interface has been created. So, these settings are removed from hostConfig.Sysctls
709+
// and added to DriverOpts[netlabel.EndpointSysctls].
710+
//
711+
// Because interface names ('ethN') are allocated sequentially, and the order of
712+
// network connections is not deterministic on container restart, only 'eth0'
713+
// would work reliably in a top-level '--sysctl' option, and then only when
714+
// there's a single initial network connection. So, settings for 'eth0' are
715+
// migrated to the primary interface, identified by 'hostConfig.NetworkMode'.
716+
// Settings for other interfaces are treated as errors.
717+
//
718+
// In the DriverOpts, to restrict sysctl settings to those configuring network
719+
// interfaces and because the interface name cannot be determined in advance, a
720+
// short-form of the sysctl name is used. For example, 'net.ipv6.conf.eth0.forwarding'
721+
// becomes 'ipv6.conf.forwarding'. The value in DriverOpts is a comma-separated list
722+
// of these short-form settings.
723+
//
724+
// A warning is generated when settings are migrated.
725+
func handleSysctlBC(
726+
hostConfig *container.HostConfig,
727+
netConfig *network.NetworkingConfig,
728+
version string,
729+
) (string, error) {
730+
if !hostConfig.NetworkMode.IsPrivate() {
731+
return "", nil
732+
}
733+
734+
var ep *network.EndpointSettings
735+
var toDelete []string
736+
var netIfSysctls []string
737+
for k, v := range hostConfig.Sysctls {
738+
// If the sysctl name matches "net.*.*.eth0.*" ...
739+
if spl := strings.SplitN(k, ".", 5); len(spl) == 5 && spl[0] == "net" && strings.HasPrefix(spl[3], "eth") {
740+
// Transform the name to the endpoint-specific short form.
741+
netIfSysctl := fmt.Sprintf("%s.%s.%s=%s", spl[1], spl[2], spl[4], v)
742+
// Find the EndpointConfig to migrate settings to, if not already found.
743+
if ep == nil {
744+
var err error
745+
ep, err = epConfigForNetMode(version, hostConfig.NetworkMode, netConfig)
746+
if err != nil {
747+
return "", fmt.Errorf("unable to find a network for sysctl %s: %w", k, err)
748+
}
749+
}
750+
// Only try to migrate settings for "eth0", anything else would always
751+
// have behaved unpredictably.
752+
if spl[3] != "eth0" {
753+
return "", fmt.Errorf(`unable to determine network endpoint for sysctl %s, use driver option '%s' to set per-interface sysctls`,
754+
k, netlabel.EndpointSysctls)
755+
}
756+
// Prepare the migration.
757+
toDelete = append(toDelete, k)
758+
netIfSysctls = append(netIfSysctls, netIfSysctl)
759+
}
760+
}
761+
if ep == nil {
762+
return "", nil
763+
}
764+
765+
// TODO(robmry) - refuse to do the migration, generate an error if API > some-future-version.
766+
767+
newDriverOpt := strings.Join(netIfSysctls, ",")
768+
warning := fmt.Sprintf(`Migrated %s to DriverOpts{"%s":"%s"}.`,
769+
strings.Join(toDelete, ","),
770+
netlabel.EndpointSysctls, newDriverOpt)
771+
772+
// Append exiting per-endpoint sysctls to the migrated sysctls (give priority
773+
// to per-endpoint settings).
774+
if ep.DriverOpts == nil {
775+
ep.DriverOpts = map[string]string{}
776+
}
777+
if oldDriverOpt, ok := ep.DriverOpts[netlabel.EndpointSysctls]; ok {
778+
newDriverOpt += "," + oldDriverOpt
779+
}
780+
ep.DriverOpts[netlabel.EndpointSysctls] = newDriverOpt
781+
782+
// Delete migrated settings from the top-level sysctls.
783+
for _, k := range toDelete {
784+
delete(hostConfig.Sysctls, k)
785+
}
786+
787+
return warning, nil
788+
}
789+
697790
// epConfigForNetMode finds, or creates, an entry in netConfig.EndpointsConfig
698791
// corresponding to nwMode.
699792
//

‎api/server/router/container/container_routes_test.go‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package container
22

33
import (
4+
"strings"
45
"testing"
56

67
"github.com/docker/docker/api/types/container"
78
"github.com/docker/docker/api/types/network"
9+
"github.com/docker/docker/libnetwork/netlabel"
810
"gotest.tools/v3/assert"
911
is "gotest.tools/v3/assert/cmp"
1012
)
@@ -228,3 +230,85 @@ func TestEpConfigForNetMode(t *testing.T) {
228230
})
229231
}
230232
}
233+
234+
func TestHandleSysctlBC(t *testing.T) {
235+
testcases := []struct {
236+
name string
237+
networkMode string
238+
sysctls map[string]string
239+
expEpSysctls []string
240+
expSysctls map[string]string
241+
expWarningContains []string
242+
expError string
243+
}{
244+
{
245+
name: "migrate to new ep",
246+
networkMode: "mynet",
247+
sysctls: map[string]string{
248+
"net.ipv6.conf.all.disable_ipv6": "0",
249+
"net.ipv6.conf.eth0.accept_ra": "2",
250+
"net.ipv6.conf.eth0.forwarding": "1",
251+
},
252+
expSysctls: map[string]string{
253+
"net.ipv6.conf.all.disable_ipv6": "0",
254+
},
255+
expEpSysctls: []string{"ipv6.conf.forwarding=1", "ipv6.conf.accept_ra=2"},
256+
expWarningContains: []string{
257+
"Migrated",
258+
"net.ipv6.conf.eth0.accept_ra", "ipv6.conf.accept_ra=2",
259+
"net.ipv6.conf.eth0.forwarding", "ipv6.conf.forwarding=1",
260+
},
261+
},
262+
{
263+
name: "migrate nothing",
264+
networkMode: "mynet",
265+
sysctls: map[string]string{
266+
"net.ipv6.conf.all.disable_ipv6": "0",
267+
},
268+
expSysctls: map[string]string{
269+
"net.ipv6.conf.all.disable_ipv6": "0",
270+
},
271+
},
272+
}
273+
274+
for _, tc := range testcases {
275+
t.Run(tc.name, func(t *testing.T) {
276+
hostCfg := &container.HostConfig{
277+
NetworkMode: container.NetworkMode(tc.networkMode),
278+
Sysctls: map[string]string{},
279+
}
280+
for k, v := range tc.sysctls {
281+
hostCfg.Sysctls[k] = v
282+
}
283+
netCfg := &network.NetworkingConfig{}
284+
285+
warnings, err := handleSysctlBC(hostCfg, netCfg, "1.45")
286+
287+
if tc.expError == "" {
288+
assert.Check(t, err)
289+
} else {
290+
assert.Check(t, is.ErrorContains(err, tc.expError))
291+
}
292+
293+
for _, s := range tc.expWarningContains {
294+
assert.Check(t, is.Contains(warnings, s))
295+
}
296+
297+
assert.Check(t, is.DeepEqual(hostCfg.Sysctls, tc.expSysctls))
298+
299+
ep := netCfg.EndpointsConfig[tc.networkMode]
300+
if ep == nil {
301+
assert.Check(t, is.Nil(tc.expEpSysctls))
302+
} else {
303+
got, ok := ep.DriverOpts[netlabel.EndpointSysctls]
304+
assert.Check(t, ok)
305+
// Check for expected ep-sysctls.
306+
for _, want := range tc.expEpSysctls {
307+
assert.Check(t, is.Contains(got, want))
308+
}
309+
// Check for unexpected ep-sysctls.
310+
assert.Check(t, is.Len(got, len(strings.Join(tc.expEpSysctls, ","))))
311+
}
312+
})
313+
}
314+
}

‎daemon/container_operations.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,18 @@ func validateEndpointSettings(nw *libnetwork.Network, nwName string, epConfig *n
615615
}
616616
}
617617

618+
if sysctls, ok := epConfig.DriverOpts[netlabel.EndpointSysctls]; ok {
619+
for _, sysctl := range strings.Split(sysctls, ",") {
620+
scname := strings.SplitN(sysctl, ".", 3)
621+
if len(scname) != 3 || (scname[0] != "ipv4" && scname[0] != "ipv6") {
622+
errs = append(errs,
623+
fmt.Errorf(
624+
"unrecognised network interface sysctl '%s'; represent 'net.X.Y.ethN.Z=V' as 'X.Y.Z=V', 'X' must be 'ipv4' or 'ipv6'",
625+
sysctl))
626+
}
627+
}
628+
}
629+
618630
if err := multierror.Join(errs...); err != nil {
619631
return fmt.Errorf("invalid endpoint settings:\n%w", err)
620632
}

‎integration/networking/bridge_test.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ import (
1010

1111
"github.com/docker/docker/api/types"
1212
containertypes "github.com/docker/docker/api/types/container"
13+
apinetwork "github.com/docker/docker/api/types/network"
1314
"github.com/docker/docker/integration/internal/container"
1415
"github.com/docker/docker/integration/internal/network"
16+
"github.com/docker/docker/libnetwork/netlabel"
1517
"github.com/docker/docker/testutil"
1618
"github.com/docker/docker/testutil/daemon"
1719
"github.com/google/go-cmp/cmp/cmpopts"
@@ -828,3 +830,36 @@ func TestReadOnlySlashProc(t *testing.T) {
828830
})
829831
}
830832
}
833+
834+
// Test that it's possible to set a sysctl on an interface in the container
835+
// using DriverOpts.
836+
func TestSetEndpointSysctl(t *testing.T) {
837+
skip.If(t, testEnv.DaemonInfo.OSType == "windows", "no sysctl on Windows")
838+
839+
ctx := setupTest(t)
840+
d := daemon.New(t)
841+
d.StartWithBusybox(ctx, t)
842+
defer d.Stop(t)
843+
844+
c := d.NewClientT(t)
845+
defer c.Close()
846+
847+
const scName = "net.ipv4.conf.eth0.forwarding"
848+
for _, val := range []string{"0", "1"} {
849+
t.Run("set to "+val, func(t *testing.T) {
850+
ctx := testutil.StartSpan(ctx, t)
851+
runRes := container.RunAttach(ctx, t, c,
852+
container.WithCmd("sysctl", "-qn", scName),
853+
container.WithEndpointSettings(apinetwork.NetworkBridge, &apinetwork.EndpointSettings{
854+
DriverOpts: map[string]string{
855+
netlabel.EndpointSysctls: "ipv4.conf.forwarding=" + val,
856+
},
857+
}),
858+
)
859+
defer c.ContainerRemove(ctx, runRes.ContainerID, containertypes.RemoveOptions{Force: true})
860+
861+
stdout := runRes.Stdout.String()
862+
assert.Check(t, is.Equal(strings.TrimSpace(stdout), val))
863+
})
864+
}
865+
}

‎libnetwork/endpoint.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"encoding/json"
99
"fmt"
1010
"net"
11+
"strings"
1112
"sync"
1213

1314
"github.com/containerd/log"
@@ -401,6 +402,18 @@ func (ep *Endpoint) Value() []byte {
401402
return b
402403
}
403404

405+
func (ep *Endpoint) getSysctls() []string {
406+
ep.mu.Lock()
407+
defer ep.mu.Unlock()
408+
409+
if s, ok := ep.generic[netlabel.EndpointSysctls]; ok {
410+
if ss, ok := s.(string); ok {
411+
return strings.Split(ss, ",")
412+
}
413+
}
414+
return nil
415+
}
416+
404417
func (ep *Endpoint) SetValue(value []byte) error {
405418
return json.Unmarshal(value, ep)
406419
}

‎libnetwork/netlabel/labels.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ const (
2626
// DNSServers A list of DNS servers associated with the endpoint
2727
DNSServers = Prefix + ".endpoint.dnsservers"
2828

29+
// EndpointSysctls is a comma separated list of shortened sysctls ('net.X.Y.ethN.Z=V' -> 'X.Y.Z=V').
30+
EndpointSysctls = Prefix + ".endpoint.sysctls"
31+
2932
// EnableIPv6 constant represents enabling IPV6 at network level
3033
EnableIPv6 = Prefix + ".enable_ipv6"
3134

0 commit comments

Comments
 (0)
0