diff --git a/api/types/network/endpoint.go b/api/types/network/endpoint.go index 4b3c06a52b583..98ad8ff11a698 100644 --- a/api/types/network/endpoint.go +++ b/api/types/network/endpoint.go @@ -14,7 +14,11 @@ type EndpointSettings struct { IPAMConfig *EndpointIPAMConfig Links []string Aliases []string // Aliases holds the list of extra, user-specified DNS names for this endpoint. - MacAddress string + // DesiredMacAddress is the configured value, it's copied from MacAddress (the + // API param field) when the container is created, and cleared in the inspect + // output so that only MacAddress is returned to clients. DesiredMacAddress is + // used to set the value for netlabel.MacAddress passed to the driver. + DesiredMacAddress string `json:",omitempty"` // Operational data NetworkID string EndpointID string @@ -24,6 +28,7 @@ type EndpointSettings struct { IPv6Gateway string GlobalIPv6Address string GlobalIPv6PrefixLen int + MacAddress string DriverOpts map[string]string // DNSNames holds all the (non fully qualified) DNS names associated to this endpoint. First entry is used to // generate PTR records. diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 5df9640bb9a22..7e1449277fbde 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -634,12 +634,10 @@ func cleanOperationalData(es *network.EndpointSettings) { es.IPv6Gateway = "" es.GlobalIPv6Address = "" es.GlobalIPv6PrefixLen = 0 + es.MacAddress = "" if es.IPAMOperational { es.IPAMConfig = nil } - if es.MACOperational { - es.MacAddress = "" - } } func (daemon *Daemon) updateNetworkConfig(container *container.Container, n *libnetwork.Network, endpointConfig *networktypes.EndpointSettings, updateSettings bool) error { @@ -711,7 +709,6 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container. } var operIPAM bool - operMAC := true if nwCfg != nil { if epConfig, ok := nwCfg.EndpointsConfig[nwName]; ok { if endpointConfig.IPAMConfig == nil || (endpointConfig.IPAMConfig.IPv4Address == "" && endpointConfig.IPAMConfig.IPv6Address == "" && len(endpointConfig.IPAMConfig.LinkLocalIPs) == 0) { @@ -721,11 +718,6 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container. // copy IPAMConfig and NetworkID from epConfig via AttachNetwork endpointConfig.IPAMConfig = epConfig.IPAMConfig endpointConfig.NetworkID = epConfig.NetworkID - - // Work out whether the MAC address is user-configured. - operMAC = endpointConfig.MacAddress == "" - // Copy the configured MAC address (which may be empty). - endpointConfig.MacAddress = epConfig.MacAddress } } @@ -755,7 +747,6 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container. container.NetworkSettings.Networks[nwName] = &network.EndpointSettings{ EndpointSettings: endpointConfig, IPAMOperational: operIPAM, - MACOperational: operMAC, } delete(container.NetworkSettings.Networks, n.ID()) diff --git a/daemon/create.go b/daemon/create.go index aa063b60aa5ef..163680760d035 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -105,6 +105,18 @@ func (daemon *Daemon) containerCreate(ctx context.Context, daemonCfg *configStor return containertypes.CreateResponse{Warnings: warnings}, errdefs.InvalidParameter(err) } + // If a MAC address has been configured, it's in the MacAddress field of the + // network's EndpointConfig. But, MacAddress is also used to store a generated + // MAC address, and we must not restore a generated MAC address when a container + // is restarted. So, copy the configured value to DesiredMacAddress here. It will + // be stored as part of the container's config, and used to pass the configured + // value to the driver, but it is removed from 'inspect' output. + if opts.params.NetworkingConfig != nil { + for _, ep := range opts.params.NetworkingConfig.EndpointsConfig { + ep.DesiredMacAddress = ep.MacAddress + } + } + if opts.params.HostConfig == nil { opts.params.HostConfig = &containertypes.HostConfig{} } diff --git a/daemon/inspect.go b/daemon/inspect.go index 915b9d810b183..6d75fdd11ba55 100644 --- a/daemon/inspect.go +++ b/daemon/inspect.go @@ -70,6 +70,7 @@ func (daemon *Daemon) ContainerInspectCurrent(ctx context.Context, name string, if epConf.EndpointSettings != nil { // We must make a copy of this pointer object otherwise it can race with other operations apiNetworks[nwName] = epConf.EndpointSettings.Copy() + apiNetworks[nwName].DesiredMacAddress = "" } } diff --git a/daemon/network.go b/daemon/network.go index c4b0c93c1dcda..797fc180b0714 100644 --- a/daemon/network.go +++ b/daemon/network.go @@ -824,8 +824,8 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e createOptions = append(createOptions, libnetwork.EndpointOptionGeneric(options.Generic{k: v})) } - if epConfig.MacAddress != "" { - mac, err := net.ParseMAC(epConfig.MacAddress) + if epConfig.DesiredMacAddress != "" { + mac, err := net.ParseMAC(epConfig.DesiredMacAddress) if err != nil { return nil, err } diff --git a/daemon/network/settings.go b/daemon/network/settings.go index ec44ba0e31370..c31b139ae8dcf 100644 --- a/daemon/network/settings.go +++ b/daemon/network/settings.go @@ -33,8 +33,6 @@ type Settings struct { type EndpointSettings struct { *networktypes.EndpointSettings IPAMOperational bool - // MACOperational is false if EndpointSettings.MacAddress is a user-configured value. - MACOperational bool } // AttachmentStore stores the load balancer IP address for a network id. diff --git a/integration/networking/mac_addr_test.go b/integration/networking/mac_addr_test.go index 497b4f96dc7f3..1c515aeb16b58 100644 --- a/integration/networking/mac_addr_test.go +++ b/integration/networking/mac_addr_test.go @@ -77,3 +77,60 @@ func TestMACAddrOnRestart(t *testing.T) { assert.Check(t, ctr1MAC != ctr2MAC, "expected containers to have different MAC addresses; got %q for both", ctr1MAC) } + +// Check that a configured MAC address is restored after a container restart, +// and after a daemon restart. +func TestCfgdMACAddrOnRestart(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType == "windows") + + ctx := setupTest(t) + + d := daemon.New(t) + d.StartWithBusybox(ctx, t) + defer d.Stop(t) + + c := d.NewClientT(t) + defer c.Close() + + const netName = "testcfgmacaddr" + network.CreateNoError(ctx, t, c, netName, + network.WithDriver("bridge"), + network.WithOption("com.docker.network.bridge.name", netName)) + defer network.RemoveNoError(ctx, t, c, netName) + + const wantMAC = "02:42:ac:11:00:42" + const ctr1Name = "ctr1" + id1 := container.Run(ctx, t, c, + container.WithName(ctr1Name), + container.WithImage("busybox:latest"), + container.WithCmd("top"), + container.WithNetworkMode(netName), + container.WithMacAddress(netName, wantMAC)) + defer c.ContainerRemove(ctx, id1, containertypes.RemoveOptions{ + Force: true, + }) + + inspect := container.Inspect(ctx, t, c, ctr1Name) + gotMAC := inspect.NetworkSettings.Networks[netName].MacAddress + assert.Check(t, is.Equal(wantMAC, gotMAC)) + + startAndCheck := func() { + t.Helper() + err := c.ContainerStart(ctx, ctr1Name, containertypes.StartOptions{}) + assert.Assert(t, is.Nil(err)) + inspect = container.Inspect(ctx, t, c, ctr1Name) + gotMAC = inspect.NetworkSettings.Networks[netName].MacAddress + assert.Check(t, is.Equal(wantMAC, gotMAC)) + } + + // Restart the container, check that the MAC address is restored. + err := c.ContainerStop(ctx, ctr1Name, containertypes.StopOptions{}) + assert.Assert(t, is.Nil(err)) + startAndCheck() + + // Restart the daemon, check that the MAC address is restored. + err = c.ContainerStop(ctx, ctr1Name, containertypes.StopOptions{}) + assert.Assert(t, is.Nil(err)) + d.Restart(t) + startAndCheck() +}