Merge pull request docker#10756 from milas/network-race

up: fix race condition on network connect

Author: glours

pkg/compose/convergence.go

@@ -29,7 +29,6 @@ import (
 	"github.com/containerd/containerd/platforms"
 	moby "github.com/docker/docker/api/types"
 	containerType "github.com/docker/docker/api/types/container"
-	"github.com/docker/docker/api/types/network"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -233,7 +232,13 @@ func (c *convergence) ensureService(ctx context.Context, project *types.Project,
 		name := getContainerName(project.Name, service, number)
 		i := i
 		eg.Go(func() error {
-			container, err := c.service.createContainer(ctx, project, service, name, number, false, true, false)
+			opts := createOptions{
+				AutoRemove:        false,
+				AttachStdin:       false,
+				UseNetworkAliases: true,
+				Labels:            mergeLabels(service.Labels, service.CustomLabels),
+			}
+			container, err := c.service.createContainer(ctx, project, service, name, number, opts)
 			updated[actual+i] = container
 			return err
 		})
@@ -399,12 +404,11 @@ func getScale(config types.ServiceConfig) (int, error) {
 }
 
 func (s *composeService) createContainer(ctx context.Context, project *types.Project, service types.ServiceConfig,
-	name string, number int, autoRemove bool, useNetworkAliases bool, attachStdin bool) (container moby.Container, err error) {
+	name string, number int, opts createOptions) (container moby.Container, err error) {
 	w := progress.ContextWriter(ctx)
 	eventName := "Container " + name
 	w.Event(progress.CreatingEvent(eventName))
-	container, err = s.createMobyContainer(ctx, project, service, name, number, nil,
-		autoRemove, useNetworkAliases, attachStdin, w, mergeLabels(service.Labels, service.CustomLabels))
+	container, err = s.createMobyContainer(ctx, project, service, name, number, nil, opts, w)
 	if err != nil {
 		return
 	}
@@ -429,9 +433,13 @@ func (s *composeService) recreateContainer(ctx context.Context, project *types.P
 	}
 	name := getContainerName(project.Name, service, number)
 	tmpName := fmt.Sprintf("%s_%s", replaced.ID[:12], name)
-	created, err = s.createMobyContainer(ctx, project, service, tmpName, number, inherited,
-		false, true, false, w,
-		mergeLabels(service.Labels, service.CustomLabels).Add(api.ContainerReplaceLabel, replaced.ID))
+	opts := createOptions{
+		AutoRemove:        false,
+		AttachStdin:       false,
+		UseNetworkAliases: true,
+		Labels:            mergeLabels(service.Labels, service.CustomLabels).Add(api.ContainerReplaceLabel, replaced.ID),
+	}
+	created, err = s.createMobyContainer(ctx, project, service, tmpName, number, inherited, opts, w)
 	if err != nil {
 		return created, err
 	}
@@ -484,19 +492,18 @@ func (s *composeService) startContainer(ctx context.Context, container moby.Cont
 	return nil
 }
 
-func (s *composeService) createMobyContainer(ctx context.Context, //nolint:gocyclo
+func (s *composeService) createMobyContainer(ctx context.Context,
 	project *types.Project,
 	service types.ServiceConfig,
 	name string,
 	number int,
 	inherit *moby.Container,
-	autoRemove, useNetworkAliases, attachStdin bool,
+	opts createOptions,
 	w progress.Writer,
-	labels types.Labels,
 ) (moby.Container, error) {
 	var created moby.Container
-	containerConfig, hostConfig, networkingConfig, err := s.getCreateOptions(ctx, project, service, number, inherit,
-		autoRemove, attachStdin, labels)
+	cfgs, err := s.getCreateConfigs(ctx, project, service, number, inherit, opts)
+
 	if err != nil {
 		return created, err
 	}
@@ -514,18 +521,7 @@ func (s *composeService) createMobyContainer(ctx context.Context, //nolint:gocyc
 		plat = &p
 	}
 
-	links, err := s.getLinks(ctx, project.Name, service, number)
-	if err != nil {
-		return created, err
-	}
-	if networkingConfig != nil {
-		for k, s := range networkingConfig.EndpointsConfig {
-			s.Links = links
-			networkingConfig.EndpointsConfig[k] = s
-		}
-	}
-
-	response, err := s.apiClient().ContainerCreate(ctx, containerConfig, hostConfig, networkingConfig, plat, name)
+	response, err := s.apiClient().ContainerCreate(ctx, cfgs.Container, cfgs.Host, cfgs.Network, plat, name)
 	if err != nil {
 		return created, err
 	}
@@ -548,29 +544,19 @@ func (s *composeService) createMobyContainer(ctx context.Context, //nolint:gocyc
 			Networks: inspectedContainer.NetworkSettings.Networks,
 		},
 	}
-	for _, netName := range service.NetworksByPriority() {
-		netwrk := project.Networks[netName]
-		cfg := service.Networks[netName]
-		aliases := []string{getContainerName(project.Name, service, number)}
-		if useNetworkAliases {
-			aliases = append(aliases, service.Name)
-			if cfg != nil {
-				aliases = append(aliases, cfg.Aliases...)
-			}
-		}
-		if val, ok := created.NetworkSettings.Networks[netwrk.Name]; ok {
-			if shortIDAliasExists(created.ID, val.Aliases...) {
-				continue
-			}
-			err = s.apiClient().NetworkDisconnect(ctx, netwrk.Name, created.ID, false)
-			if err != nil {
+
+	// the highest-priority network is the primary and is included in the ContainerCreate API
+	// call via container.NetworkMode & network.NetworkingConfig
+	// any remaining networks are connected one-by-one here after creation (but before start)
+	serviceNetworks := service.NetworksByPriority()
+	if len(serviceNetworks) > 1 {
+		for _, networkKey := range serviceNetworks[1:] {
+			mobyNetworkName := project.Networks[networkKey].Name
+			epSettings := createEndpointSettings(project, service, number, networkKey, cfgs.Links, opts.UseNetworkAliases)
+			if err := s.apiClient().NetworkConnect(ctx, mobyNetworkName, created.ID, epSettings); err != nil {
 				return created, err
 			}
 		}
-		err = s.connectContainerToNetwork(ctx, created.ID, netwrk.Name, cfg, links, aliases...)
-		if err != nil {
-			return created, err
-		}
 	}
 
 	err = s.injectSecrets(ctx, project, service, created.ID)
@@ -635,43 +621,6 @@ func (s *composeService) getLinks(ctx context.Context, projectName string, servi
 	return links, nil
 }
 
-func shortIDAliasExists(containerID string, aliases ...string) bool {
-	for _, alias := range aliases {
-		if alias == containerID[:12] {
-			return true
-		}
-	}
-	return false
-}
-
-func (s *composeService) connectContainerToNetwork(ctx context.Context, id string, netwrk string, cfg *types.ServiceNetworkConfig, links []string, aliases ...string) error {
-	var (
-		ipv4Address string
-		ipv6Address string
-		ipam        *network.EndpointIPAMConfig
-	)
-	if cfg != nil {
-		ipv4Address = cfg.Ipv4Address
-		ipv6Address = cfg.Ipv6Address
-		ipam = &network.EndpointIPAMConfig{
-			IPv4Address:  ipv4Address,
-			IPv6Address:  ipv6Address,
-			LinkLocalIPs: cfg.LinkLocalIPs,
-		}
-	}
-	err := s.apiClient().NetworkConnect(ctx, netwrk, id, &network.EndpointSettings{
-		Aliases:           aliases,
-		IPAddress:         ipv4Address,
-		GlobalIPv6Address: ipv6Address,
-		Links:             links,
-		IPAMConfig:        ipam,
-	})
-	if err != nil {
-		return err
-	}
-	return nil
-}
-
 func (s *composeService) isServiceHealthy(ctx context.Context, containers Containers, fallbackRunning bool) (bool, error) {
 	for _, c := range containers {
 		container, err := s.apiClient().ContainerInspect(ctx, c.ID)