beenull/moby
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge pull request #47233 from robmry/47146-duplicate_mac_addrs2

Browse source 
Only restore a configured MAC addr on restart.
This commit is contained in:
Albin Kerouanton 2024-02-02 09:08:17 +01:00 committed by GitHub
commit ca683c1c77
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 191 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
api/types/network/endpoint.go
View file

@ -14,6 +14,9 @@ type EndpointSettings struct {
IPAMConfig *EndpointIPAMConfig
Links []string
Aliases []string // Aliases holds the list of extra, user-specified DNS names for this endpoint.
// MacAddress may be used to specify a MAC address when the container is created.
// Once the container is running, it becomes operational data (it may contain a
// generated address).
MacAddress string
// Operational data
NetworkID string

44
daemon/container_operations.go
View file

@ -445,6 +445,11 @@ func (daemon *Daemon) updateContainerNetworkSettings(container *container.Contai
for name, epConfig := range endpointsConfig {
container.NetworkSettings.Networks[name] = &network.EndpointSettings{
EndpointSettings: epConfig,
// At this point, during container creation, epConfig.MacAddress is the
// configured value from the API. If there is no configured value, the
// same field will later be used to store a generated MAC address. So,
// remember the requested address now.
DesiredMacAddress: epConfig.MacAddress,
}
}
}
@ -511,7 +516,7 @@ func (daemon *Daemon) allocateNetwork(cfg *config.Config, container *container.C
defaultNetName := runconfig.DefaultDaemonNetworkMode().NetworkName()
if nConf, ok := container.NetworkSettings.Networks[defaultNetName]; ok {
cleanOperationalData(nConf)
if err := daemon.connectToNetwork(cfg, container, defaultNetName, nConf.EndpointSettings, updateSettings); err != nil {
if err := daemon.connectToNetwork(cfg, container, defaultNetName, nConf, updateSettings); err != nil {
return err
}
}
@ -528,7 +533,7 @@ func (daemon *Daemon) allocateNetwork(cfg *config.Config, container *container.C
for netName, epConf := range networks {
cleanOperationalData(epConf)
if err := daemon.connectToNetwork(cfg, container, netName, epConf.EndpointSettings, updateSettings); err != nil {
if err := daemon.connectToNetwork(cfg, container, netName, epConf, updateSettings); err != nil {
return err
}
}
@ -637,12 +642,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 {
@ -685,7 +688,7 @@ func buildEndpointDNSNames(ctr *container.Container, aliases []string) []string
return sliceutil.Dedup(dnsNames)
}
func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container.Container, idOrName string, endpointConfig *networktypes.EndpointSettings, updateSettings bool) (retErr error) {
func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container.Container, idOrName string, endpointConfig *network.EndpointSettings, updateSettings bool) (retErr error) {
start := time.Now()
if container.HostConfig.NetworkMode.IsContainer() {
return runconfig.ErrConflictSharedNetwork
@ -695,10 +698,12 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container.
return nil
}
if endpointConfig == nil {
endpointConfig = &networktypes.EndpointSettings{}
endpointConfig = &network.EndpointSettings{
EndpointSettings: &networktypes.EndpointSettings{},
}
}
n, nwCfg, err := daemon.findAndAttachNetwork(container, idOrName, endpointConfig)
n, nwCfg, err := daemon.findAndAttachNetwork(container, idOrName, endpointConfig.EndpointSettings)
if err != nil {
return err
}
@ -713,26 +718,20 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container.
}
}
var operIPAM bool
operMAC := true
endpointConfig.IPAMOperational = false
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) {
operIPAM = true
endpointConfig.IPAMOperational = true
}
// 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
}
}
if err := daemon.updateNetworkConfig(container, n, endpointConfig, updateSettings); err != nil {
if err := daemon.updateNetworkConfig(container, n, endpointConfig.EndpointSettings, updateSettings); err != nil {
return err
}
@ -755,11 +754,7 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container.
}
}
}()
container.NetworkSettings.Networks[nwName] = &network.EndpointSettings{
EndpointSettings: endpointConfig,
IPAMOperational: operIPAM,
MACOperational: operMAC,
}
container.NetworkSettings.Networks[nwName] = endpointConfig
delete(container.NetworkSettings.Networks, n.ID())
@ -1063,7 +1058,10 @@ func (daemon *Daemon) ConnectToNetwork(container *container.Container, idOrName
}
}
} else {
if err := daemon.connectToNetwork(&daemon.config().Config, container, idOrName, endpointConfig, true); err != nil {
epc := &network.EndpointSettings{
EndpointSettings: endpointConfig,
}
if err := daemon.connectToNetwork(&daemon.config().Config, container, idOrName, epc, true); err != nil {
return err
}
}

8
daemon/inspect.go
View file

@ -165,8 +165,12 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, container *contai
// unversioned API endpoints.
if container.Config != nil && container.Config.MacAddress == "" { //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
if nwm := hostConfig.NetworkMode; nwm.IsDefault() || nwm.IsBridge() || nwm.IsUserDefined() {
if epConf, ok := container.NetworkSettings.Networks[nwm.NetworkName()]; ok {
container.Config.MacAddress = epConf.MacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
name := nwm.NetworkName()
if nwm.IsDefault() {
name = daemon.netController.Config().DefaultNetwork
}
if epConf, ok := container.NetworkSettings.Networks[name]; ok {
container.Config.MacAddress = epConf.DesiredMacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
}
}
}

6
daemon/network.go
View file

@ -788,7 +788,7 @@ func (daemon *Daemon) clearAttachableNetworks() {
}
// buildCreateEndpointOptions builds endpoint options from a given network.
func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, epConfig *network.EndpointSettings, sb *libnetwork.Sandbox, daemonDNS []string) ([]libnetwork.EndpointOption, error) {
func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, epConfig *internalnetwork.EndpointSettings, sb *libnetwork.Sandbox, daemonDNS []string) ([]libnetwork.EndpointOption, error) {
var createOptions []libnetwork.EndpointOption
var genericOptions = make(options.Generic)
@ -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
}

5
daemon/network/settings.go
View file

@ -33,8 +33,9 @@ type Settings struct {
type EndpointSettings struct {
*networktypes.EndpointSettings
IPAMOperational bool
// MACOperational is false if EndpointSettings.MacAddress is a user-configured value.
MACOperational bool
// DesiredMacAddress is the configured value, it's copied from MacAddress (the
// API param field) when the container is created.
DesiredMacAddress string
}
// AttachmentStore stores the load balancer IP address for a network id.

156
integration/networking/mac_addr_test.go
View file

@ -4,8 +4,11 @@ import (
"testing"
containertypes "github.com/docker/docker/api/types/container"
"github.com/docker/docker/client"
"github.com/docker/docker/integration/internal/container"
"github.com/docker/docker/integration/internal/network"
"github.com/docker/docker/libnetwork/drivers/bridge"
"github.com/docker/docker/testutil"
"github.com/docker/docker/testutil/daemon"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
@ -35,7 +38,7 @@ func TestMACAddrOnRestart(t *testing.T) {
const netName = "testmacaddrs"
network.CreateNoError(ctx, t, c, netName,
network.WithDriver("bridge"),
network.WithOption("com.docker.network.bridge.name", netName))
network.WithOption(bridge.BridgeName, netName))
defer network.RemoveNoError(ctx, t, c, netName)
const ctr1Name = "ctr1"
@ -77,3 +80,154 @@ 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(bridge.BridgeName, 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()
}
// Regression test for https://github.com/moby/moby/issues/47228 - check that a
// generated MAC address is not included in the Config section of 'inspect'
// output, but a configured address is.
func TestInspectCfgdMAC(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)
testcases := []struct {
name string
desiredMAC string
netName string
ctrWide bool
}{
{
name: "generated address default bridge",
netName: "bridge",
},
{
name: "configured address default bridge",
desiredMAC: "02:42:ac:11:00:42",
netName: "bridge",
},
{
name: "generated address custom bridge",
netName: "testnet",
},
{
name: "configured address custom bridge",
desiredMAC: "02:42:ac:11:00:42",
netName: "testnet",
},
{
name: "ctr-wide address default bridge",
desiredMAC: "02:42:ac:11:00:42",
netName: "bridge",
ctrWide: true,
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
ctx := testutil.StartSpan(ctx, t)
var copts []client.Opt
if tc.ctrWide {
copts = append(copts, client.WithVersion("1.43"))
}
c := d.NewClientT(t, copts...)
defer c.Close()
if tc.netName != "bridge" {
const netName = "inspectcfgmac"
network.CreateNoError(ctx, t, c, netName,
network.WithDriver("bridge"),
network.WithOption(bridge.BridgeName, netName))
defer network.RemoveNoError(ctx, t, c, netName)
}
const ctrName = "ctr"
opts := []func(*container.TestContainerConfig){
container.WithName(ctrName),
container.WithCmd("top"),
container.WithImage("busybox:latest"),
}
// Don't specify the network name for the bridge network, because that
// exercises a different code path (the network name isn't set until the
// container starts, until then it's "default").
if tc.netName != "bridge" {
opts = append(opts, container.WithNetworkMode(tc.netName))
}
if tc.desiredMAC != "" {
if tc.ctrWide {
opts = append(opts, container.WithContainerWideMacAddress(tc.desiredMAC))
} else {
opts = append(opts, container.WithMacAddress(tc.netName, tc.desiredMAC))
}
}
id := container.Create(ctx, t, c, opts...)
defer c.ContainerRemove(ctx, id, containertypes.RemoveOptions{
Force: true,
})
inspect := container.Inspect(ctx, t, c, ctrName)
configMAC := inspect.Config.MacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44.
assert.Check(t, is.DeepEqual(configMAC, tc.desiredMAC))
})
}
}