Skip to content

Commit

Permalink
Add missing opts to --network advanced syntax
Browse files Browse the repository at this point in the history 
The new advanced --network syntax introduced in #1767 is
lacking support for `link-local-ip` and `mac-address` fields. This
commit adds both.

Signed-off-by: Albin Kerouanton <[email protected]>
  • Loading branch information
@akerouanton
akerouanton committed Sep 10, 2023
1 parent 58f4260 commit 9e1b42e
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 9 deletions.
26 changes: 22 additions & 4 deletions cli/command/container/opts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"))
Expand All @@ -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())
Expand All @@ -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())
Expand Down Expand Up @@ -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
}

Expand Down
44 changes: 41 additions & 3 deletions cli/command/container/opts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}{
Expand Down Expand Up @@ -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": {
Expand All @@ -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{
Expand All @@ -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"},
},
{
Expand All @@ -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"},
Expand All @@ -638,18 +660,34 @@ 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)
return
}

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)
})
Expand Down
11 changes: 9 additions & 2 deletions opts/network.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ const (
networkOptAlias = "alias"
networkOptIPv4Address = "ip"
networkOptIPv6Address = "ip6"
networkOptMacAddress = "mac-address"
networkOptLinkLocalIP = "link-local-ip"
driverOpt = "driver-opt"
)

Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
20 changes: 20 additions & 0 deletions opts/network_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9e1b42e

Please sign in to comment.