Merge pull request #4419 from akerouanton/missing-nw-advanced-options

Add missing opts to --network advanced syntax

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,
@@ -787,8 +793,7 @@ func parseNetworkOpts(copts *containerOptions) (map[string]*networktypes.Endpoin
 	return endpoints, nil
 }
 
-func applyContainerOptions(n *opts.NetworkAttachmentOpts, copts *containerOptions) error {
+func applyContainerOptions(n *opts.NetworkAttachmentOpts, copts *containerOptions) error { //nolint:gocyclo
-	// TODO should copts.MacAddress actually be set on the first network? (currently it's not)
 	// 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"))
@@ -802,6 +807,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())
@@ -816,8 +827,9 @@ func applyContainerOptions(n *opts.NetworkAttachmentOpts, copts *containerOption
 	if copts.ipv6Address != "" {
 		n.IPv6Address = copts.ipv6Address
 	}
-
+	if copts.macAddress != "" {
-	// TODO should linkLocalIPs be added to the _first_ network only, or to _all_ networks? (should this be a per-network option as well?)
+		n.MacAddress = copts.macAddress
+	}
 	if copts.linkLocalIPs.Len() > 0 {
 		n.LinkLocalIPs = make([]string, copts.linkLocalIPs.Len())
 		copy(n.LinkLocalIPs, copts.linkLocalIPs.GetAll())
@@ -854,6 +866,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
 }
 

cli/command/container/opts_test.go

@@ -510,20 +510,21 @@ func TestParseNetworkConfig(t *testing.T) {
 		name            string
 		flags           []string
 		expected        map[string]*networktypes.EndpointSettings
-		expectedCfg container.HostConfig
+		expectedCfg     container.Config
+		expectedHostCfg container.HostConfig
 		expectedErr     string
 	}{
 		{
 			name:            "single-network-legacy",
 			flags:           []string{"--network", "net1"},
 			expected:        map[string]*networktypes.EndpointSettings{},
-			expectedCfg: container.HostConfig{NetworkMode: "net1"},
+			expectedHostCfg: container.HostConfig{NetworkMode: "net1"},
 		},
 		{
 			name:            "single-network-advanced",
 			flags:           []string{"--network", "name=net1"},
 			expected:        map[string]*networktypes.EndpointSettings{},
-			expectedCfg: container.HostConfig{NetworkMode: "net1"},
+			expectedHostCfg: container.HostConfig{NetworkMode: "net1"},
 		},
 		{
 			name: "single-network-legacy-with-options",
@@ -549,7 +550,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",
@@ -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"},
 					},
-			expectedCfg: container.HostConfig{NetworkMode: "net1"},
+				},
+			},
+			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{
@@ -603,15 +611,29 @@ func TestParseNetworkConfig(t *testing.T) {
 						IPv6Address: "2001:db8::8822",
 					},
 					Aliases:    []string{"web1", "web2"},
+					MacAddress: "02:32:1c:23:00:04",
 				},
 			},
-			expectedCfg: container.HostConfig{NetworkMode: "net1"},
+			expectedCfg:     container.Config{MacAddress: "02:32:1c:23:00:04"},
+			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"},
+			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",
@@ -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,7 +687,8 @@ func TestParseNetworkConfig(t *testing.T) {
 			}
 
 			assert.NilError(t, err)
-			assert.DeepEqual(t, hConfig.NetworkMode, tc.expectedCfg.NetworkMode)
+			assert.DeepEqual(t, config.MacAddress, tc.expectedCfg.MacAddress)
+			assert.DeepEqual(t, hConfig.NetworkMode, tc.expectedHostCfg.NetworkMode)
 			assert.DeepEqual(t, nwConfig.EndpointsConfig, tc.expected)
 		})
 	}

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 {

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