network: fix random missing network when service has more than one

As part of the fix for docker#10668, the logic was adjusted so that the
default (highest-priority) network is used in the `ContainerCreate`,
and then the remaining networks are connected via calls to
`NetworkConnect` before starting the container.

Unfortunately, `ServiceConfig::NetworksByPriority` is neither
deterministic nor stable when networks have the same priority.

It's non-deterministic because the order of networks from parsing
YAML is random, since they are loaded into a Go map (which have
random iteration order). Additionally, it's not using a `SortStable`
in `compose-go`, so even if the load order was predictable, it
still might produce different results.

While I look at improving `compose-go` here to prevent this from
tripping us up in the future, this fix looks at _all_ networks for
a service and ignores the "default" one now. Before, it would
always skip the first one in the slice since that _should_ have
been the "default".

Signed-off-by: Milas Bowman <[email protected]>

Author: milas

pkg/compose/convergence.go

@@ -549,13 +549,15 @@ func (s *composeService) createMobyContainer(ctx context.Context,
 	// 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
-			}
+	for _, networkKey := range serviceNetworks {
+		mobyNetworkName := project.Networks[networkKey].Name
+		if string(cfgs.Host.NetworkMode) == mobyNetworkName {
+			// primary network already configured as part of ContainerCreate
+			continue
+		}
+		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
 		}
 	}
 

pkg/e2e/networks_test.go

@@ -29,47 +29,34 @@ import (
 
 func TestNetworks(t *testing.T) {
 	// fixture is shared with TestNetworkModes and is not safe to run concurrently
-	c := NewCLI(t)
-
 	const projectName = "network-e2e"
+	c := NewCLI(t, WithEnv(
+		"COMPOSE_PROJECT_NAME="+projectName,
+		"COMPOSE_FILE=./fixtures/network-test/compose.yaml",
+	))
 
-	t.Run("ensure we do not reuse previous networks", func(t *testing.T) {
-		c.RunDockerOrExitError(t, "network", "rm", projectName+"-dbnet")
-		c.RunDockerOrExitError(t, "network", "rm", "microservices")
-	})
-
-	t.Run("up", func(t *testing.T) {
-		c.RunDockerComposeCmd(t, "-f", "./fixtures/network-test/compose.yaml", "--project-name", projectName, "up",
-			"-d")
-	})
+	c.RunDockerComposeCmd(t, "down", "-t0", "-v")
 
-	t.Run("check running project", func(t *testing.T) {
-		res := c.RunDockerComposeCmd(t, "-p", projectName, "ps")
-		res.Assert(t, icmd.Expected{Out: `web`})
+	c.RunDockerComposeCmd(t, "up", "-d")
 
-		endpoint := "http://localhost:80"
-		output := HTTPGetWithRetry(t, endpoint+"/words/noun", http.StatusOK, 2*time.Second, 20*time.Second)
-		assert.Assert(t, strings.Contains(output, `"word":`))
+	res := c.RunDockerComposeCmd(t, "ps")
+	res.Assert(t, icmd.Expected{Out: `web`})
 
-		res = c.RunDockerCmd(t, "network", "ls")
-		res.Assert(t, icmd.Expected{Out: projectName + "_dbnet"})
-		res.Assert(t, icmd.Expected{Out: "microservices"})
-	})
+	endpoint := "http://localhost:80"
+	output := HTTPGetWithRetry(t, endpoint+"/words/noun", http.StatusOK, 2*time.Second, 20*time.Second)
+	assert.Assert(t, strings.Contains(output, `"word":`))
 
-	t.Run("port", func(t *testing.T) {
-		res := c.RunDockerComposeCmd(t, "--project-name", projectName, "port", "words", "8080")
-		res.Assert(t, icmd.Expected{Out: `0.0.0.0:8080`})
-	})
+	res = c.RunDockerCmd(t, "network", "ls")
+	res.Assert(t, icmd.Expected{Out: projectName + "_dbnet"})
+	res.Assert(t, icmd.Expected{Out: "microservices"})
 
-	t.Run("down", func(t *testing.T) {
-		_ = c.RunDockerComposeCmd(t, "--project-name", projectName, "down")
-	})
+	res = c.RunDockerComposeCmd(t, "port", "words", "8080")
+	res.Assert(t, icmd.Expected{Out: `0.0.0.0:8080`})
 
-	t.Run("check networks after down", func(t *testing.T) {
-		res := c.RunDockerCmd(t, "network", "ls")
-		assert.Assert(t, !strings.Contains(res.Combined(), projectName), res.Combined())
-		assert.Assert(t, !strings.Contains(res.Combined(), "microservices"), res.Combined())
-	})
+	c.RunDockerComposeCmd(t, "down", "-t0", "-v")
+	res = c.RunDockerCmd(t, "network", "ls")
+	assert.Assert(t, !strings.Contains(res.Combined(), projectName), res.Combined())
+	assert.Assert(t, !strings.Contains(res.Combined(), "microservices"), res.Combined())
 }
 
 func TestNetworkAliases(t *testing.T) {