Skip to content

Commit 1bd8a77

Browse files
ndeloofndeloof
authored
detect network conflict as name is not guaranteed to be unique (#10612)
Signed-off-by: Nicolas De Loof <[email protected]>
1 parent fed8ef6 commit 1bd8a77

6 files changed

Lines changed: 198 additions & 98 deletions

File tree

‎pkg/compose/convergence.go‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ func (s *composeService) startContainer(ctx context.Context, container moby.Cont
484484
return nil
485485
}
486486

487-
func (s *composeService) createMobyContainer(ctx context.Context,
487+
func (s *composeService) createMobyContainer(ctx context.Context, //nolint:gocyclo
488488
project *types.Project,
489489
service types.ServiceConfig,
490490
name string,
@@ -513,6 +513,18 @@ func (s *composeService) createMobyContainer(ctx context.Context,
513513
}
514514
plat = &p
515515
}
516+
517+
links, err := s.getLinks(ctx, project.Name, service, number)
518+
if err != nil {
519+
return created, err
520+
}
521+
if networkingConfig != nil {
522+
for k, s := range networkingConfig.EndpointsConfig {
523+
s.Links = links
524+
networkingConfig.EndpointsConfig[k] = s
525+
}
526+
}
527+
516528
response, err := s.apiClient().ContainerCreate(ctx, containerConfig, hostConfig, networkingConfig, plat, name)
517529
if err != nil {
518530
return created, err
@@ -536,10 +548,6 @@ func (s *composeService) createMobyContainer(ctx context.Context,
536548
Networks: inspectedContainer.NetworkSettings.Networks,
537549
},
538550
}
539-
links, err := s.getLinks(ctx, project.Name, service, number)
540-
if err != nil {
541-
return created, err
542-
}
543551
for _, netName := range service.NetworksByPriority() {
544552
netwrk := project.Networks[netName]
545553
cfg := service.Networks[netName]

‎pkg/compose/create.go‎

Lines changed: 147 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,12 @@ func prepareNetworks(project *types.Project) {
143143
}
144144

145145
func (s *composeService) ensureNetworks(ctx context.Context, networks types.Networks) error {
146-
for _, network := range networks {
147-
err := s.ensureNetwork(ctx, network)
146+
for i, network := range networks {
147+
err := s.ensureNetwork(ctx, &network)
148148
if err != nil {
149149
return err
150150
}
151+
networks[i] = network
151152
}
152153
return nil
153154
}
@@ -1009,98 +1010,168 @@ func getAliases(s types.ServiceConfig, c *types.ServiceNetworkConfig) []string {
10091010
return aliases
10101011
}
10111012

1012-
func (s *composeService) ensureNetwork(ctx context.Context, n types.NetworkConfig) error {
1013-
// NetworkInspect will match on ID prefix, so NetworkList with a name
1014-
// filter is used to look for an exact match to prevent e.g. a network
1015-
// named `db` from getting erroneously matched to a network with an ID
1016-
// like `db9086999caf`
1013+
func (s *composeService) ensureNetwork(ctx context.Context, n *types.NetworkConfig) error {
1014+
if n.External.External {
1015+
return s.resolveExternalNetwork(ctx, n)
1016+
}
1017+
1018+
err := s.resolveOrCreateNetwork(ctx, n)
1019+
if errdefs.IsConflict(err) {
1020+
// Maybe another execution of `docker compose up|run` created same network
1021+
// let's retry once
1022+
return s.resolveOrCreateNetwork(ctx, n)
1023+
}
1024+
return err
1025+
}
1026+
1027+
func (s *composeService) resolveOrCreateNetwork(ctx context.Context, n *types.NetworkConfig) error { //nolint:gocyclo
1028+
expectedNetworkLabel := n.Labels[api.NetworkLabel]
1029+
expectedProjectLabel := n.Labels[api.ProjectLabel]
1030+
1031+
// First, try to find a unique network matching by name or ID
1032+
inspect, err := s.apiClient().NetworkInspect(ctx, n.Name, moby.NetworkInspectOptions{})
1033+
if err == nil {
1034+
// NetworkInspect will match on ID prefix, so double check we get the expected one
1035+
// as looking for network named `db` we could erroneously matched network ID `db9086999caf`
1036+
if inspect.Name == n.Name || inspect.ID == n.Name {
1037+
if inspect.Labels[api.ProjectLabel] != expectedProjectLabel {
1038+
logrus.Warnf("a network with name %s exists but was not created by compose.\n"+
1039+
"Set `external: true` to use an existing network", n.Name)
1040+
}
1041+
if inspect.Labels[api.NetworkLabel] != expectedNetworkLabel {
1042+
return fmt.Errorf("network %s was found but has incorrect label %s set to %q", n.Name, api.NetworkLabel, inspect.Labels[api.NetworkLabel])
1043+
}
1044+
return nil
1045+
}
1046+
}
1047+
// ignore other errors. Typically, an ambiguous request by name results in some generic `invalidParameter` error
1048+
1049+
// Either not found, or name is ambiguous - use NetworkList to list by name
10171050
networks, err := s.apiClient().NetworkList(ctx, moby.NetworkListOptions{
10181051
Filters: filters.NewArgs(filters.Arg("name", n.Name)),
10191052
})
10201053
if err != nil {
10211054
return err
10221055
}
1023-
networkNotFound := true
1056+
1057+
// NetworkList Matches all or part of a network name, so we have to filter for a strict match
1058+
networks = utils.Filter(networks, func(net moby.NetworkResource) bool {
1059+
return net.Name == n.Name
1060+
})
1061+
10241062
for _, net := range networks {
1025-
if net.Name == n.Name {
1026-
networkNotFound = false
1027-
break
1063+
if net.Labels[api.ProjectLabel] == expectedProjectLabel &&
1064+
net.Labels[api.NetworkLabel] == expectedNetworkLabel {
1065+
return nil
10281066
}
10291067
}
1030-
if networkNotFound {
1031-
if n.External.External {
1032-
if n.Driver == "overlay" {
1033-
// Swarm nodes do not register overlay networks that were
1034-
// created on a different node unless they're in use.
1035-
// Here we assume `driver` is relevant for a network we don't manage
1036-
// which is a non-sense, but this is our legacy ¯\(ツ)/¯
1037-
// networkAttach will later fail anyway if network actually doesn't exists
1038-
enabled, err := s.isSWarmEnabled(ctx)
1039-
if err != nil {
1040-
return err
1041-
}
1042-
if enabled {
1043-
return nil
1044-
}
1045-
}
1046-
return fmt.Errorf("network %s declared as external, but could not be found", n.Name)
1047-
}
1048-
var ipam *network.IPAM
1049-
if n.Ipam.Config != nil {
1050-
var config []network.IPAMConfig
1051-
for _, pool := range n.Ipam.Config {
1052-
config = append(config, network.IPAMConfig{
1053-
Subnet: pool.Subnet,
1054-
IPRange: pool.IPRange,
1055-
Gateway: pool.Gateway,
1056-
AuxAddress: pool.AuxiliaryAddresses,
1057-
})
1058-
}
1059-
ipam = &network.IPAM{
1060-
Driver: n.Ipam.Driver,
1061-
Config: config,
1062-
}
1068+
1069+
// we could have set NetworkList with a projectFilter and networkFilter but not doing so allows to catch this
1070+
// scenario were a network with same name exists but doesn't have label, and use of `CheckDuplicate: true`
1071+
// prevents to create another one.
1072+
if len(networks) > 0 {
1073+
logrus.Warnf("a network with name %s exists but was not created by compose.\n"+
1074+
"Set `external: true` to use an existing network", n.Name)
1075+
return nil
1076+
}
1077+
1078+
var ipam *network.IPAM
1079+
if n.Ipam.Config != nil {
1080+
var config []network.IPAMConfig
1081+
for _, pool := range n.Ipam.Config {
1082+
config = append(config, network.IPAMConfig{
1083+
Subnet: pool.Subnet,
1084+
IPRange: pool.IPRange,
1085+
Gateway: pool.Gateway,
1086+
AuxAddress: pool.AuxiliaryAddresses,
1087+
})
10631088
}
1064-
createOpts := moby.NetworkCreate{
1065-
CheckDuplicate: true,
1066-
// TODO NameSpace Labels
1067-
Labels: n.Labels,
1068-
Driver: n.Driver,
1069-
Options: n.DriverOpts,
1070-
Internal: n.Internal,
1071-
Attachable: n.Attachable,
1072-
IPAM: ipam,
1073-
EnableIPv6: n.EnableIPv6,
1089+
ipam = &network.IPAM{
1090+
Driver: n.Ipam.Driver,
1091+
Config: config,
10741092
}
1093+
}
1094+
createOpts := moby.NetworkCreate{
1095+
CheckDuplicate: true,
1096+
Labels: n.Labels,
1097+
Driver: n.Driver,
1098+
Options: n.DriverOpts,
1099+
Internal: n.Internal,
1100+
Attachable: n.Attachable,
1101+
IPAM: ipam,
1102+
EnableIPv6: n.EnableIPv6,
1103+
}
10751104

1076-
if n.Ipam.Driver != "" || len(n.Ipam.Config) > 0 {
1077-
createOpts.IPAM = &network.IPAM{}
1078-
}
1105+
if n.Ipam.Driver != "" || len(n.Ipam.Config) > 0 {
1106+
createOpts.IPAM = &network.IPAM{}
1107+
}
10791108

1080-
if n.Ipam.Driver != "" {
1081-
createOpts.IPAM.Driver = n.Ipam.Driver
1109+
if n.Ipam.Driver != "" {
1110+
createOpts.IPAM.Driver = n.Ipam.Driver
1111+
}
1112+
1113+
for _, ipamConfig := range n.Ipam.Config {
1114+
config := network.IPAMConfig{
1115+
Subnet: ipamConfig.Subnet,
1116+
IPRange: ipamConfig.IPRange,
1117+
Gateway: ipamConfig.Gateway,
1118+
AuxAddress: ipamConfig.AuxiliaryAddresses,
10821119
}
1120+
createOpts.IPAM.Config = append(createOpts.IPAM.Config, config)
1121+
}
1122+
networkEventName := fmt.Sprintf("Network %s", n.Name)
1123+
w := progress.ContextWriter(ctx)
1124+
w.Event(progress.CreatingEvent(networkEventName))
1125+
1126+
_, err = s.apiClient().NetworkCreate(ctx, n.Name, createOpts)
1127+
if err != nil {
1128+
w.Event(progress.ErrorEvent(networkEventName))
1129+
return errors.Wrapf(err, "failed to create network %s", n.Name)
1130+
}
1131+
w.Event(progress.CreatedEvent(networkEventName))
1132+
return nil
1133+
}
1134+
1135+
func (s *composeService) resolveExternalNetwork(ctx context.Context, n *types.NetworkConfig) error {
1136+
// NetworkInspect will match on ID prefix, so NetworkList with a name
1137+
// filter is used to look for an exact match to prevent e.g. a network
1138+
// named `db` from getting erroneously matched to a network with an ID
1139+
// like `db9086999caf`
1140+
networks, err := s.apiClient().NetworkList(ctx, moby.NetworkListOptions{
1141+
Filters: filters.NewArgs(filters.Arg("name", n.Name)),
1142+
})
1143+
if err != nil {
1144+
return err
1145+
}
1146+
1147+
// NetworkList API doesn't return the exact name match, so we can retrieve more than one network with a request
1148+
networks = utils.Filter(networks, func(net moby.NetworkResource) bool {
1149+
return net.Name == n.Name
1150+
})
10831151

1084-
for _, ipamConfig := range n.Ipam.Config {
1085-
config := network.IPAMConfig{
1086-
Subnet: ipamConfig.Subnet,
1087-
IPRange: ipamConfig.IPRange,
1088-
Gateway: ipamConfig.Gateway,
1089-
AuxAddress: ipamConfig.AuxiliaryAddresses,
1152+
switch len(networks) {
1153+
case 1:
1154+
n.Name = networks[0].ID
1155+
return nil
1156+
case 0:
1157+
if n.Driver == "overlay" {
1158+
// Swarm nodes do not register overlay networks that were
1159+
// created on a different node unless they're in use.
1160+
// Here we assume `driver` is relevant for a network we don't manage
1161+
// which is a non-sense, but this is our legacy ¯\(ツ)/¯
1162+
// networkAttach will later fail anyway if network actually doesn't exists
1163+
enabled, err := s.isSWarmEnabled(ctx)
1164+
if err != nil {
1165+
return err
1166+
}
1167+
if enabled {
1168+
return nil
10901169
}
1091-
createOpts.IPAM.Config = append(createOpts.IPAM.Config, config)
1092-
}
1093-
networkEventName := fmt.Sprintf("Network %s", n.Name)
1094-
w := progress.ContextWriter(ctx)
1095-
w.Event(progress.CreatingEvent(networkEventName))
1096-
if _, err := s.apiClient().NetworkCreate(ctx, n.Name, createOpts); err != nil {
1097-
w.Event(progress.ErrorEvent(networkEventName))
1098-
return errors.Wrapf(err, "failed to create network %s", n.Name)
10991170
}
1100-
w.Event(progress.CreatedEvent(networkEventName))
1101-
return nil
1171+
return fmt.Errorf("network %s declared as external, but could not be found", n.Name)
1172+
default:
1173+
return fmt.Errorf("multiple networks with name %q were found. Use network ID as `name` to avoid ambiguity", n.Name)
11021174
}
1103-
return nil
11041175
}
11051176

11061177
func (s *composeService) ensureVolume(ctx context.Context, volume types.VolumeConfig, project string) error {

‎pkg/compose/down.go‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -171,32 +171,30 @@ func (s *composeService) ensureImagesDown(ctx context.Context, project *types.Pr
171171

172172
func (s *composeService) ensureNetworksDown(ctx context.Context, project *types.Project, w progress.Writer) []downOp {
173173
var ops []downOp
174-
for _, n := range project.Networks {
174+
for key, n := range project.Networks {
175175
if n.External.External {
176176
continue
177177
}
178178
// loop capture variable for op closure
179-
networkName := n.Name
179+
networkKey := key
180+
idOrName := n.Name
180181
ops = append(ops, func() error {
181-
return s.removeNetwork(ctx, networkName, w)
182+
return s.removeNetwork(ctx, networkKey, project.Name, idOrName, w)
182183
})
183184
}
184185
return ops
185186
}
186187

187-
func (s *composeService) removeNetwork(ctx context.Context, name string, w progress.Writer) error {
188-
// networks are guaranteed to have unique IDs but NOT names, so it's
189-
// possible to get into a situation where a compose down will fail with
190-
// an error along the lines of:
191-
// failed to remove network test: Error response from daemon: network test is ambiguous (2 matches found based on name)
192-
// as a workaround here, the delete is done by ID after doing a list using
193-
// the name as a filter (99.9% of the time this will return a single result)
188+
func (s *composeService) removeNetwork(ctx context.Context, composeNetworkName string, projectName string, name string, w progress.Writer) error {
194189
networks, err := s.apiClient().NetworkList(ctx, moby.NetworkListOptions{
195-
Filters: filters.NewArgs(filters.Arg("name", name)),
190+
Filters: filters.NewArgs(
191+
projectFilter(projectName),
192+
networkFilter(composeNetworkName)),
196193
})
197194
if err != nil {
198-
return errors.Wrapf(err, fmt.Sprintf("failed to inspect network %s", name))
195+
return errors.Wrapf(err, "failed to list networks")
199196
}
197+
200198
if len(networks) == 0 {
201199
return nil
202200
}

‎pkg/compose/down_test.go‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ func TestDown(t *testing.T) {
6060
// cleanup properly if duplicates are inadvertently created
6161
api.EXPECT().NetworkList(gomock.Any(), moby.NetworkListOptions{Filters: filters.NewArgs(projectFilter(strings.ToLower(testProject)))}).
6262
Return([]moby.NetworkResource{
63-
{ID: "abc123", Name: "myProject_default"},
64-
{ID: "def456", Name: "myProject_default"},
63+
{ID: "abc123", Name: "myProject_default", Labels: map[string]string{compose.NetworkLabel: "default"}},
64+
{ID: "def456", Name: "myProject_default", Labels: map[string]string{compose.NetworkLabel: "default"}},
6565
}, nil)
6666

6767
stopOptions := containerType.StopOptions{}
@@ -74,7 +74,9 @@ func TestDown(t *testing.T) {
7474
api.EXPECT().ContainerRemove(gomock.Any(), "789", moby.ContainerRemoveOptions{Force: true}).Return(nil)
7575

7676
api.EXPECT().NetworkList(gomock.Any(), moby.NetworkListOptions{
77-
Filters: filters.NewArgs(filters.Arg("name", "myProject_default")),
77+
Filters: filters.NewArgs(
78+
projectFilter(strings.ToLower(testProject)),
79+
networkFilter("default")),
7880
}).Return([]moby.NetworkResource{
7981
{ID: "abc123", Name: "myProject_default"},
8082
{ID: "def456", Name: "myProject_default"},
@@ -106,7 +108,11 @@ func TestDownRemoveOrphans(t *testing.T) {
106108
api.EXPECT().VolumeList(gomock.Any(), filters.NewArgs(projectFilter(strings.ToLower(testProject)))).
107109
Return(volume.ListResponse{}, nil)
108110
api.EXPECT().NetworkList(gomock.Any(), moby.NetworkListOptions{Filters: filters.NewArgs(projectFilter(strings.ToLower(testProject)))}).
109-
Return([]moby.NetworkResource{{Name: "myProject_default"}}, nil)
111+
Return([]moby.NetworkResource{
112+
{
113+
Name: "myProject_default",
114+
Labels: map[string]string{compose.NetworkLabel: "default"},
115+
}}, nil)
110116

111117
stopOptions := containerType.StopOptions{}
112118
api.EXPECT().ContainerStop(gomock.Any(), "123", stopOptions).Return(nil)
@@ -118,7 +124,10 @@ func TestDownRemoveOrphans(t *testing.T) {
118124
api.EXPECT().ContainerRemove(gomock.Any(), "321", moby.ContainerRemoveOptions{Force: true}).Return(nil)
119125

120126
api.EXPECT().NetworkList(gomock.Any(), moby.NetworkListOptions{
121-
Filters: filters.NewArgs(filters.Arg("name", "myProject_default")),
127+
Filters: filters.NewArgs(
128+
networkFilter("default"),
129+
projectFilter(strings.ToLower(testProject)),
130+
),
122131
}).Return([]moby.NetworkResource{{ID: "abc123", Name: "myProject_default"}}, nil)
123132
api.EXPECT().NetworkInspect(gomock.Any(), "abc123", gomock.Any()).Return(moby.NetworkResource{ID: "abc123"}, nil)
124133
api.EXPECT().NetworkRemove(gomock.Any(), "abc123").Return(nil)

‎pkg/compose/filters.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ func serviceFilter(serviceName string) filters.KeyValuePair {
3131
return filters.Arg("label", fmt.Sprintf("%s=%s", api.ServiceLabel, serviceName))
3232
}
3333

34+
func networkFilter(name string) filters.KeyValuePair {
35+
return filters.Arg("label", fmt.Sprintf("%s=%s", api.NetworkLabel, name))
36+
}
37+
3438
func oneOffFilter(b bool) filters.KeyValuePair {
3539
v := "False"
3640
if b {

0 commit comments

Comments
 (0)