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

As part of the fix for #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 <milas.bowman@docker.com>
This commit is contained in:
Milas Bowman 2023-07-06 16:28:54 -04:00 committed by Nicolas De loof
parent b5f5e27597
commit be22bc735a
2 changed files with 29 additions and 40 deletions

View File

@ -549,15 +549,17 @@ 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:] {
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
}
}
}
err = s.injectSecrets(ctx, project, service, created.ID)
return created, err

View File

@ -29,22 +29,17 @@ 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")
})
c.RunDockerComposeCmd(t, "down", "-t0", "-v")
t.Run("up", func(t *testing.T) {
c.RunDockerComposeCmd(t, "-f", "./fixtures/network-test/compose.yaml", "--project-name", projectName, "up",
"-d")
})
c.RunDockerComposeCmd(t, "up", "-d")
t.Run("check running project", func(t *testing.T) {
res := c.RunDockerComposeCmd(t, "-p", projectName, "ps")
res := c.RunDockerComposeCmd(t, "ps")
res.Assert(t, icmd.Expected{Out: `web`})
endpoint := "http://localhost:80"
@ -54,22 +49,14 @@ func TestNetworks(t *testing.T) {
res = c.RunDockerCmd(t, "network", "ls")
res.Assert(t, icmd.Expected{Out: projectName + "_dbnet"})
res.Assert(t, icmd.Expected{Out: "microservices"})
})
t.Run("port", func(t *testing.T) {
res := c.RunDockerComposeCmd(t, "--project-name", projectName, "port", "words", "8080")
res = c.RunDockerComposeCmd(t, "port", "words", "8080")
res.Assert(t, icmd.Expected{Out: `0.0.0.0:8080`})
})
t.Run("down", func(t *testing.T) {
_ = c.RunDockerComposeCmd(t, "--project-name", projectName, "down")
})
t.Run("check networks after down", func(t *testing.T) {
res := c.RunDockerCmd(t, "network", "ls")
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) {