From d20f9f359b6f4a6d3bc906d2630ffafbdb61f388 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Fri, 26 Jan 2024 18:00:32 +0000 Subject: [PATCH] Only restore a configured MAC addr on restart. The API's EndpointConfig struct has a MacAddress field that's used for both the configured address, and the current address (which may be generated). A configured address must be restored when a container is restarted, but a generated address must not. The previous attempt to differentiate between the two, without adding a field to the API's EndpointConfig that would show up in 'inspect' output, was a field in the daemon's version of EndpointSettings, MACOperational. It did not work, MACOperational was set to true when a configured address was used. So, while it ensured addresses were regenerated, it failed to preserve a configured address. Also, because the daemon's EndpointSettings.MACOperational field was not visible when generating inspect output, it could not be used to avoid adding the generated address to Config.MacAddress. So, this change removes that code, and adds DesiredMacAddress to the API's version of EndpointSettings. Its value is copied from MacAddress (the API field) when a container is created, it is stored as part of the container's config, and used to inform the network driver of the configured value. But, it is removed from 'inspect' output. Signed-off-by: Rob Murray --- api/types/network/endpoint.go | 7 ++++++- daemon/container_operations.go | 11 +---------- daemon/create.go | 12 ++++++++++++ daemon/inspect.go | 1 + daemon/network.go | 4 ++-- daemon/network/settings.go | 2 -- 6 files changed, 22 insertions(+), 15 deletions(-) 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.