From 58f4260872b3531af9ca4bef4b1bc3747a1bb026 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Sun, 10 Sep 2023 18:52:46 +0200 Subject: [PATCH 1/2] Rename expectedCfg into expectedHostCfg Signed-off-by: Albin Kerouanton --- cli/command/container/opts_test.go | 42 +++++++++++++++--------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/cli/command/container/opts_test.go b/cli/command/container/opts_test.go index 1cd68046b67c..ceef26cb5759 100644 --- a/cli/command/container/opts_test.go +++ b/cli/command/container/opts_test.go @@ -507,23 +507,23 @@ func TestParseDevice(t *testing.T) { func TestParseNetworkConfig(t *testing.T) { tests := []struct { - name string - flags []string - expected map[string]*networktypes.EndpointSettings - expectedCfg container.HostConfig - expectedErr string + name string + flags []string + expected map[string]*networktypes.EndpointSettings + expectedHostCfg container.HostConfig + expectedErr string }{ { - name: "single-network-legacy", - flags: []string{"--network", "net1"}, - expected: map[string]*networktypes.EndpointSettings{}, - expectedCfg: container.HostConfig{NetworkMode: "net1"}, + name: "single-network-legacy", + flags: []string{"--network", "net1"}, + expected: map[string]*networktypes.EndpointSettings{}, + expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, { - name: "single-network-advanced", - flags: []string{"--network", "name=net1"}, - expected: map[string]*networktypes.EndpointSettings{}, - expectedCfg: container.HostConfig{NetworkMode: "net1"}, + name: "single-network-advanced", + flags: []string{"--network", "name=net1"}, + expected: map[string]*networktypes.EndpointSettings{}, + expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, { name: "single-network-legacy-with-options", @@ -549,7 +549,7 @@ func TestParseNetworkConfig(t *testing.T) { Aliases: []string{"web1", "web2"}, }, }, - expectedCfg: container.HostConfig{NetworkMode: "net1"}, + expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, { name: "multiple-network-advanced-mixed", @@ -587,7 +587,7 @@ func TestParseNetworkConfig(t *testing.T) { Aliases: []string{"web3"}, }, }, - expectedCfg: container.HostConfig{NetworkMode: "net1"}, + expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, { name: "single-network-advanced-with-options", @@ -605,13 +605,13 @@ func TestParseNetworkConfig(t *testing.T) { Aliases: []string{"web1", "web2"}, }, }, - expectedCfg: container.HostConfig{NetworkMode: "net1"}, + expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, { - name: "multiple-networks", - flags: []string{"--network", "net1", "--network", "name=net2"}, - expected: map[string]*networktypes.EndpointSettings{"net1": {}, "net2": {}}, - expectedCfg: container.HostConfig{NetworkMode: "net1"}, + name: "multiple-networks", + flags: []string{"--network", "net1", "--network", "name=net2"}, + expected: map[string]*networktypes.EndpointSettings{"net1": {}, "net2": {}}, + expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, { name: "conflict-network", @@ -650,7 +650,7 @@ func TestParseNetworkConfig(t *testing.T) { } assert.NilError(t, err) - assert.DeepEqual(t, hConfig.NetworkMode, tc.expectedCfg.NetworkMode) + assert.DeepEqual(t, hConfig.NetworkMode, tc.expectedHostCfg.NetworkMode) assert.DeepEqual(t, nwConfig.EndpointsConfig, tc.expected) }) } From 9e1b42e642dac97de474f20377858db6a18dc766 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Thu, 13 Jul 2023 11:50:01 +0200 Subject: [PATCH 2/2] Add missing opts to --network advanced syntax The new advanced --network syntax introduced in docker/cli#1767 is lacking support for `link-local-ip` and `mac-address` fields. This commit adds both. Signed-off-by: Albin Kerouanton --- cli/command/container/opts.go | 26 +++++++++++++++--- cli/command/container/opts_test.go | 44 ++++++++++++++++++++++++++++-- opts/network.go | 11 ++++++-- opts/network_test.go | 20 ++++++++++++++ 4 files changed, 92 insertions(+), 9 deletions(-) diff --git a/cli/command/container/opts.go b/cli/command/container/opts.go index cd5d69f7e791..7b338c25ce06 100644 --- a/cli/command/container/opts.go +++ b/cli/command/container/opts.go @@ -712,6 +712,12 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con return nil, err } + // Put the endpoint-specific MacAddress of the "main" network attachment into the container Config for backward + // compatibility with older daemons. + if nw, ok := networkingConfig.EndpointsConfig[hostConfig.NetworkMode.NetworkName()]; ok { + config.MacAddress = nw.MacAddress + } + return &containerConfig{ Config: config, HostConfig: hostConfig, @@ -773,8 +779,7 @@ func parseNetworkOpts(copts *containerOptions) (map[string]*networktypes.Endpoin return endpoints, nil } -func applyContainerOptions(n *opts.NetworkAttachmentOpts, copts *containerOptions) error { - // TODO should copts.MacAddress actually be set on the first network? (currently it's not) +func applyContainerOptions(n *opts.NetworkAttachmentOpts, copts *containerOptions) error { //nolint:gocyclo // TODO should we error if _any_ advanced option is used? (i.e. forbid to combine advanced notation with the "old" flags (`--network-alias`, `--link`, `--ip`, `--ip6`)? if len(n.Aliases) > 0 && copts.aliases.Len() > 0 { return errdefs.InvalidParameter(errors.New("conflicting options: cannot specify both --network-alias and per-network alias")) @@ -788,6 +793,12 @@ func applyContainerOptions(n *opts.NetworkAttachmentOpts, copts *containerOption if n.IPv6Address != "" && copts.ipv6Address != "" { return errdefs.InvalidParameter(errors.New("conflicting options: cannot specify both --ip6 and per-network IPv6 address")) } + if n.MacAddress != "" && copts.macAddress != "" { + return errdefs.InvalidParameter(errors.New("conflicting options: cannot specify both --mac-address and per-network MAC address")) + } + if len(n.LinkLocalIPs) > 0 && copts.linkLocalIPs.Len() > 0 { + return errdefs.InvalidParameter(errors.New("conflicting options: cannot specify both --link-local-ip and per-network link-local IP addresses")) + } if copts.aliases.Len() > 0 { n.Aliases = make([]string, copts.aliases.Len()) copy(n.Aliases, copts.aliases.GetAll()) @@ -802,8 +813,9 @@ func applyContainerOptions(n *opts.NetworkAttachmentOpts, copts *containerOption if copts.ipv6Address != "" { n.IPv6Address = copts.ipv6Address } - - // TODO should linkLocalIPs be added to the _first_ network only, or to _all_ networks? (should this be a per-network option as well?) + if copts.macAddress != "" { + n.MacAddress = copts.macAddress + } if copts.linkLocalIPs.Len() > 0 { n.LinkLocalIPs = make([]string, copts.linkLocalIPs.Len()) copy(n.LinkLocalIPs, copts.linkLocalIPs.GetAll()) @@ -840,6 +852,12 @@ func parseNetworkAttachmentOpt(ep opts.NetworkAttachmentOpts) (*networktypes.End LinkLocalIPs: ep.LinkLocalIPs, } } + if ep.MacAddress != "" { + if _, err := opts.ValidateMACAddress(ep.MacAddress); err != nil { + return nil, errors.Errorf("%s is not a valid mac address", ep.MacAddress) + } + epConfig.MacAddress = ep.MacAddress + } return epConfig, nil } diff --git a/cli/command/container/opts_test.go b/cli/command/container/opts_test.go index ceef26cb5759..2fccddf083fc 100644 --- a/cli/command/container/opts_test.go +++ b/cli/command/container/opts_test.go @@ -510,6 +510,7 @@ func TestParseNetworkConfig(t *testing.T) { name string flags []string expected map[string]*networktypes.EndpointSettings + expectedCfg container.Config expectedHostCfg container.HostConfig expectedErr string }{ @@ -565,6 +566,7 @@ func TestParseNetworkConfig(t *testing.T) { "--network-alias", "web2", "--network", "net2", "--network", "name=net3,alias=web3,driver-opt=field3=value3,ip=172.20.88.22,ip6=2001:db8::8822", + "--network", "name=net4,mac-address=02:32:1c:23:00:04,link-local-ip=169.254.169.254", }, expected: map[string]*networktypes.EndpointSettings{ "net1": { @@ -586,12 +588,18 @@ func TestParseNetworkConfig(t *testing.T) { }, Aliases: []string{"web3"}, }, + "net4": { + MacAddress: "02:32:1c:23:00:04", + IPAMConfig: &networktypes.EndpointIPAMConfig{ + LinkLocalIPs: []string{"169.254.169.254"}, + }, + }, }, expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, { name: "single-network-advanced-with-options", - flags: []string{"--network", "name=net1,alias=web1,alias=web2,driver-opt=field1=value1,driver-opt=field2=value2,ip=172.20.88.22,ip6=2001:db8::8822"}, + flags: []string{"--network", "name=net1,alias=web1,alias=web2,driver-opt=field1=value1,driver-opt=field2=value2,ip=172.20.88.22,ip6=2001:db8::8822,mac-address=02:32:1c:23:00:04"}, expected: map[string]*networktypes.EndpointSettings{ "net1": { DriverOpts: map[string]string{ @@ -602,9 +610,11 @@ func TestParseNetworkConfig(t *testing.T) { IPv4Address: "172.20.88.22", IPv6Address: "2001:db8::8822", }, - Aliases: []string{"web1", "web2"}, + Aliases: []string{"web1", "web2"}, + MacAddress: "02:32:1c:23:00:04", }, }, + expectedCfg: container.Config{MacAddress: "02:32:1c:23:00:04"}, expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, { @@ -613,6 +623,18 @@ func TestParseNetworkConfig(t *testing.T) { expected: map[string]*networktypes.EndpointSettings{"net1": {}, "net2": {}}, expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, }, + { + name: "advanced-options-with-standalone-mac-address-flag", + flags: []string{"--network=name=net1,alias=foobar", "--mac-address", "52:0f:f3:dc:50:10"}, + expected: map[string]*networktypes.EndpointSettings{ + "net1": { + Aliases: []string{"foobar"}, + MacAddress: "52:0f:f3:dc:50:10", + }, + }, + expectedCfg: container.Config{MacAddress: "52:0f:f3:dc:50:10"}, + expectedHostCfg: container.HostConfig{NetworkMode: "net1"}, + }, { name: "conflict-network", flags: []string{"--network", "duplicate", "--network", "name=duplicate"}, @@ -638,11 +660,26 @@ func TestParseNetworkConfig(t *testing.T) { flags: []string{"--network", "name=host", "--network", "net1"}, expectedErr: `conflicting options: cannot attach both user-defined and non-user-defined network-modes`, }, + { + name: "conflict-options-link-local-ip", + flags: []string{"--network", "name=net1,link-local-ip=169.254.169.254", "--link-local-ip", "169.254.10.8"}, + expectedErr: `conflicting options: cannot specify both --link-local-ip and per-network link-local IP addresses`, + }, + { + name: "conflict-options-mac-address", + flags: []string{"--network", "name=net1,mac-address=02:32:1c:23:00:04", "--mac-address", "02:32:1c:23:00:04"}, + expectedErr: `conflicting options: cannot specify both --mac-address and per-network MAC address`, + }, + { + name: "invalid-mac-address", + flags: []string{"--network", "name=net1,mac-address=foobar"}, + expectedErr: "foobar is not a valid mac address", + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - _, hConfig, nwConfig, err := parseRun(tc.flags) + config, hConfig, nwConfig, err := parseRun(tc.flags) if tc.expectedErr != "" { assert.Error(t, err, tc.expectedErr) @@ -650,6 +687,7 @@ func TestParseNetworkConfig(t *testing.T) { } assert.NilError(t, err) + assert.DeepEqual(t, config.MacAddress, tc.expectedCfg.MacAddress) assert.DeepEqual(t, hConfig.NetworkMode, tc.expectedHostCfg.NetworkMode) assert.DeepEqual(t, nwConfig.EndpointsConfig, tc.expected) }) diff --git a/opts/network.go b/opts/network.go index 12c3977b1bc3..e36ef405d121 100644 --- a/opts/network.go +++ b/opts/network.go @@ -12,6 +12,8 @@ const ( networkOptAlias = "alias" networkOptIPv4Address = "ip" networkOptIPv6Address = "ip6" + networkOptMacAddress = "mac-address" + networkOptLinkLocalIP = "link-local-ip" driverOpt = "driver-opt" ) @@ -23,7 +25,8 @@ type NetworkAttachmentOpts struct { Links []string // TODO add support for links in the csv notation of `--network` IPv4Address string IPv6Address string - LinkLocalIPs []string // TODO add support for LinkLocalIPs in the csv notation of `--network` ? + LinkLocalIPs []string + MacAddress string } // NetworkOpt represents a network config in swarm mode. @@ -32,7 +35,7 @@ type NetworkOpt struct { } // Set networkopts value -func (n *NetworkOpt) Set(value string) error { +func (n *NetworkOpt) Set(value string) error { //nolint:gocyclo longSyntax, err := regexp.MatchString(`\w+=\w+(,\w+=\w+)*`, value) if err != nil { return err @@ -66,6 +69,10 @@ func (n *NetworkOpt) Set(value string) error { netOpt.IPv4Address = val case networkOptIPv6Address: netOpt.IPv6Address = val + case networkOptMacAddress: + netOpt.MacAddress = val + case networkOptLinkLocalIP: + netOpt.LinkLocalIPs = append(netOpt.LinkLocalIPs, val) case driverOpt: key, val, err = parseDriverOpt(val) if err != nil { diff --git a/opts/network_test.go b/opts/network_test.go index ca595927e523..8a5b66fb117c 100644 --- a/opts/network_test.go +++ b/opts/network_test.go @@ -78,6 +78,26 @@ func TestNetworkOptAdvancedSyntax(t *testing.T) { }, }, }, + { + value: "name=docknet1,mac-address=52:0f:f3:dc:50:10", + expected: []NetworkAttachmentOpts{ + { + Target: "docknet1", + Aliases: []string{}, + MacAddress: "52:0f:f3:dc:50:10", + }, + }, + }, + { + value: "name=docknet1,link-local-ip=169.254.169.254,link-local-ip=169.254.10.10", + expected: []NetworkAttachmentOpts{ + { + Target: "docknet1", + Aliases: []string{}, + LinkLocalIPs: []string{"169.254.169.254", "169.254.10.10"}, + }, + }, + }, } for _, tc := range testCases { tc := tc