Skip to content

Commit dae3303

Browse files
robmryrobmry
committed
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. So, this change removes that code, and adds DesiredMacAddress to the wrapped version of EndpointSettings, where it is persisted but does not appear in 'inspect' results. Its value is copied from MacAddress (the API field) when a container is created. Signed-off-by: Rob Murray <[email protected]>
1 parent 8619881 commit dae3303

6 files changed

Lines changed: 90 additions & 29 deletions

File tree

‎api/types/network/endpoint.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ type EndpointSettings struct {
1414
IPAMConfig *EndpointIPAMConfig
1515
Links []string
1616
Aliases []string // Aliases holds the list of extra, user-specified DNS names for this endpoint.
17+
// MacAddress may be used to specify a MAC address when the container is created.
18+
// Once the container is running, it becomes operational data (it may contain a
19+
// generated address).
1720
MacAddress string
1821
// Operational data
1922
NetworkID string

‎daemon/container_operations.go‎

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,11 @@ func (daemon *Daemon) updateContainerNetworkSettings(container *container.Contai
442442
for name, epConfig := range endpointsConfig {
443443
container.NetworkSettings.Networks[name] = &network.EndpointSettings{
444444
EndpointSettings: epConfig,
445+
// At this point, during container creation, epConfig.MacAddress is the
446+
// configured value from the API. If there is no configured value, the
447+
// same field will later be used to store a generated MAC address. So,
448+
// remember the requested address now.
449+
DesiredMacAddress: epConfig.MacAddress,
445450
}
446451
}
447452
}
@@ -508,7 +513,7 @@ func (daemon *Daemon) allocateNetwork(cfg *config.Config, container *container.C
508513
defaultNetName := runconfig.DefaultDaemonNetworkMode().NetworkName()
509514
if nConf, ok := container.NetworkSettings.Networks[defaultNetName]; ok {
510515
cleanOperationalData(nConf)
511-
if err := daemon.connectToNetwork(cfg, container, defaultNetName, nConf.EndpointSettings, updateSettings); err != nil {
516+
if err := daemon.connectToNetwork(cfg, container, defaultNetName, nConf, updateSettings); err != nil {
512517
return err
513518
}
514519
}
@@ -525,7 +530,7 @@ func (daemon *Daemon) allocateNetwork(cfg *config.Config, container *container.C
525530

526531
for netName, epConf := range networks {
527532
cleanOperationalData(epConf)
528-
if err := daemon.connectToNetwork(cfg, container, netName, epConf.EndpointSettings, updateSettings); err != nil {
533+
if err := daemon.connectToNetwork(cfg, container, netName, epConf, updateSettings); err != nil {
529534
return err
530535
}
531536
}
@@ -634,12 +639,10 @@ func cleanOperationalData(es *network.EndpointSettings) {
634639
es.IPv6Gateway = ""
635640
es.GlobalIPv6Address = ""
636641
es.GlobalIPv6PrefixLen = 0
642+
es.MacAddress = ""
637643
if es.IPAMOperational {
638644
es.IPAMConfig = nil
639645
}
640-
if es.MACOperational {
641-
es.MacAddress = ""
642-
}
643646
}
644647

645648
func (daemon *Daemon) updateNetworkConfig(container *container.Container, n *libnetwork.Network, endpointConfig *networktypes.EndpointSettings, updateSettings bool) error {
@@ -682,7 +685,7 @@ func buildEndpointDNSNames(ctr *container.Container, aliases []string) []string
682685
return sliceutil.Dedup(dnsNames)
683686
}
684687

685-
func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container.Container, idOrName string, endpointConfig *networktypes.EndpointSettings, updateSettings bool) (retErr error) {
688+
func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container.Container, idOrName string, endpointConfig *network.EndpointSettings, updateSettings bool) (retErr error) {
686689
start := time.Now()
687690
if container.HostConfig.NetworkMode.IsContainer() {
688691
return runconfig.ErrConflictSharedNetwork
@@ -692,10 +695,12 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container.
692695
return nil
693696
}
694697
if endpointConfig == nil {
695-
endpointConfig = &networktypes.EndpointSettings{}
698+
endpointConfig = &network.EndpointSettings{
699+
EndpointSettings: &networktypes.EndpointSettings{},
700+
}
696701
}
697702

698-
n, nwCfg, err := daemon.findAndAttachNetwork(container, idOrName, endpointConfig)
703+
n, nwCfg, err := daemon.findAndAttachNetwork(container, idOrName, endpointConfig.EndpointSettings)
699704
if err != nil {
700705
return err
701706
}
@@ -710,26 +715,20 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container.
710715
}
711716
}
712717

713-
var operIPAM bool
714-
operMAC := true
718+
endpointConfig.IPAMOperational = false
715719
if nwCfg != nil {
716720
if epConfig, ok := nwCfg.EndpointsConfig[nwName]; ok {
717721
if endpointConfig.IPAMConfig == nil || (endpointConfig.IPAMConfig.IPv4Address == "" && endpointConfig.IPAMConfig.IPv6Address == "" && len(endpointConfig.IPAMConfig.LinkLocalIPs) == 0) {
718-
operIPAM = true
722+
endpointConfig.IPAMOperational = true
719723
}
720724

721725
// copy IPAMConfig and NetworkID from epConfig via AttachNetwork
722726
endpointConfig.IPAMConfig = epConfig.IPAMConfig
723727
endpointConfig.NetworkID = epConfig.NetworkID
724-
725-
// Work out whether the MAC address is user-configured.
726-
operMAC = endpointConfig.MacAddress == ""
727-
// Copy the configured MAC address (which may be empty).
728-
endpointConfig.MacAddress = epConfig.MacAddress
729728
}
730729
}
731730

732-
if err := daemon.updateNetworkConfig(container, n, endpointConfig, updateSettings); err != nil {
731+
if err := daemon.updateNetworkConfig(container, n, endpointConfig.EndpointSettings, updateSettings); err != nil {
733732
return err
734733
}
735734

@@ -752,11 +751,7 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container.
752751
}
753752
}
754753
}()
755-
container.NetworkSettings.Networks[nwName] = &network.EndpointSettings{
756-
EndpointSettings: endpointConfig,
757-
IPAMOperational: operIPAM,
758-
MACOperational: operMAC,
759-
}
754+
container.NetworkSettings.Networks[nwName] = endpointConfig
760755

761756
delete(container.NetworkSettings.Networks, n.ID())
762757

@@ -1060,7 +1055,10 @@ func (daemon *Daemon) ConnectToNetwork(container *container.Container, idOrName
10601055
}
10611056
}
10621057
} else {
1063-
if err := daemon.connectToNetwork(&daemon.config().Config, container, idOrName, endpointConfig, true); err != nil {
1058+
epc := &network.EndpointSettings{
1059+
EndpointSettings: endpointConfig,
1060+
}
1061+
if err := daemon.connectToNetwork(&daemon.config().Config, container, idOrName, epc, true); err != nil {
10641062
return err
10651063
}
10661064
}

‎daemon/inspect.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ func (daemon *Daemon) ContainerInspectCurrent(ctx context.Context, name string,
7070
if epConf.EndpointSettings != nil {
7171
// We must make a copy of this pointer object otherwise it can race with other operations
7272
apiNetworks[nwName] = epConf.EndpointSettings.Copy()
73+
apiNetworks[nwName].DesiredMacAddress = ""
7374
}
7475
}
7576

‎daemon/network.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,7 @@ func (daemon *Daemon) clearAttachableNetworks() {
788788
}
789789

790790
// buildCreateEndpointOptions builds endpoint options from a given network.
791-
func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, epConfig *network.EndpointSettings, sb *libnetwork.Sandbox, daemonDNS []string) ([]libnetwork.EndpointOption, error) {
791+
func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, epConfig *internalnetwork.EndpointSettings, sb *libnetwork.Sandbox, daemonDNS []string) ([]libnetwork.EndpointOption, error) {
792792
var createOptions []libnetwork.EndpointOption
793793
var genericOptions = make(options.Generic)
794794

@@ -824,8 +824,8 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e
824824
createOptions = append(createOptions, libnetwork.EndpointOptionGeneric(options.Generic{k: v}))
825825
}
826826

827-
if epConfig.MacAddress != "" {
828-
mac, err := net.ParseMAC(epConfig.MacAddress)
827+
if epConfig.DesiredMacAddress != "" {
828+
mac, err := net.ParseMAC(epConfig.DesiredMacAddress)
829829
if err != nil {
830830
return nil, err
831831
}

‎daemon/network/settings.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,9 @@ type Settings struct {
3333
type EndpointSettings struct {
3434
*networktypes.EndpointSettings
3535
IPAMOperational bool
36-
// MACOperational is false if EndpointSettings.MacAddress is a user-configured value.
37-
MACOperational bool
36+
// DesiredMacAddress is the configured value, it's copied from MacAddress (the
37+
// API param field) when the container is created.
38+
DesiredMacAddress string
3839
}
3940

4041
// AttachmentStore stores the load balancer IP address for a network id.

‎integration/networking/mac_addr_test.go‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
containertypes "github.com/docker/docker/api/types/container"
77
"github.com/docker/docker/integration/internal/container"
88
"github.com/docker/docker/integration/internal/network"
9+
"github.com/docker/docker/libnetwork/drivers/bridge"
910
"github.com/docker/docker/testutil/daemon"
1011
"gotest.tools/v3/assert"
1112
is "gotest.tools/v3/assert/cmp"
@@ -35,7 +36,7 @@ func TestMACAddrOnRestart(t *testing.T) {
3536
const netName = "testmacaddrs"
3637
network.CreateNoError(ctx, t, c, netName,
3738
network.WithDriver("bridge"),
38-
network.WithOption("com.docker.network.bridge.name", netName))
39+
network.WithOption(bridge.BridgeName, netName))
3940
defer network.RemoveNoError(ctx, t, c, netName)
4041

4142
const ctr1Name = "ctr1"
@@ -77,3 +78,60 @@ func TestMACAddrOnRestart(t *testing.T) {
7778
assert.Check(t, ctr1MAC != ctr2MAC,
7879
"expected containers to have different MAC addresses; got %q for both", ctr1MAC)
7980
}
81+
82+
// Check that a configured MAC address is restored after a container restart,
83+
// and after a daemon restart.
84+
func TestCfgdMACAddrOnRestart(t *testing.T) {
85+
skip.If(t, testEnv.DaemonInfo.OSType == "windows")
86+
87+
ctx := setupTest(t)
88+
89+
d := daemon.New(t)
90+
d.StartWithBusybox(ctx, t)
91+
defer d.Stop(t)
92+
93+
c := d.NewClientT(t)
94+
defer c.Close()
95+
96+
const netName = "testcfgmacaddr"
97+
network.CreateNoError(ctx, t, c, netName,
98+
network.WithDriver("bridge"),
99+
network.WithOption(bridge.BridgeName, netName))
100+
defer network.RemoveNoError(ctx, t, c, netName)
101+
102+
const wantMAC = "02:42:ac:11:00:42"
103+
const ctr1Name = "ctr1"
104+
id1 := container.Run(ctx, t, c,
105+
container.WithName(ctr1Name),
106+
container.WithImage("busybox:latest"),
107+
container.WithCmd("top"),
108+
container.WithNetworkMode(netName),
109+
container.WithMacAddress(netName, wantMAC))
110+
defer c.ContainerRemove(ctx, id1, containertypes.RemoveOptions{
111+
Force: true,
112+
})
113+
114+
inspect := container.Inspect(ctx, t, c, ctr1Name)
115+
gotMAC := inspect.NetworkSettings.Networks[netName].MacAddress
116+
assert.Check(t, is.Equal(wantMAC, gotMAC))
117+
118+
startAndCheck := func() {
119+
t.Helper()
120+
err := c.ContainerStart(ctx, ctr1Name, containertypes.StartOptions{})
121+
assert.Assert(t, is.Nil(err))
122+
inspect = container.Inspect(ctx, t, c, ctr1Name)
123+
gotMAC = inspect.NetworkSettings.Networks[netName].MacAddress
124+
assert.Check(t, is.Equal(wantMAC, gotMAC))
125+
}
126+
127+
// Restart the container, check that the MAC address is restored.
128+
err := c.ContainerStop(ctx, ctr1Name, containertypes.StopOptions{})
129+
assert.Assert(t, is.Nil(err))
130+
startAndCheck()
131+
132+
// Restart the daemon, check that the MAC address is restored.
133+
err = c.ContainerStop(ctx, ctr1Name, containertypes.StopOptions{})
134+
assert.Assert(t, is.Nil(err))
135+
d.Restart(t)
136+
startAndCheck()
137+
}

0 commit comments

Comments
 (0)