detect network config changes and recreate if needed

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/Microsoft/go-winio v0.6.2
 	github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d
 	github.com/buger/goterm v1.0.4
-	github.com/compose-spec/compose-go/v2 v2.4.4
+	github.com/compose-spec/compose-go/v2 v2.4.5-0.20241111154218-9d02caaf8465
 	github.com/containerd/containerd v1.7.23
 	github.com/containerd/platforms v0.2.1
 	github.com/davecgh/go-spew v1.1.1

go.sum

@@ -85,8 +85,8 @@ github.com/cncf/xds/go v0.0.0-20240723142845-024c85f92f20 h1:N+3sFI5GUjRKBi+i0Tx
 github.com/cncf/xds/go v0.0.0-20240723142845-024c85f92f20/go.mod h1:W+zGtBO5Y1IgJhy4+A9GOqVhqLpfZi+vwmdNXUehLA8=
 github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb h1:EDmT6Q9Zs+SbUoc7Ik9EfrFqcylYqgPZ9ANSbTAntnE=
 github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb/go.mod h1:ZjrT6AXHbDs86ZSdt/osfBi5qfexBrKUdONk989Wnk4=
-github.com/compose-spec/compose-go/v2 v2.4.4 h1:cvHBl5Jf1iNBmRrZCICmHvaoskYc1etTPEMLKVwokAY=
-github.com/compose-spec/compose-go/v2 v2.4.4/go.mod h1:lFN0DrMxIncJGYAXTfWuajfwj5haBJqrBkarHcnjJKc=
+github.com/compose-spec/compose-go/v2 v2.4.5-0.20241111154218-9d02caaf8465 h1:1PRX/3a/n4W2DrMJu4CV9OS8Z2eauOBLe0zOuSlrWDY=
+github.com/compose-spec/compose-go/v2 v2.4.5-0.20241111154218-9d02caaf8465/go.mod h1:lFN0DrMxIncJGYAXTfWuajfwj5haBJqrBkarHcnjJKc=
 github.com/containerd/cgroups v1.1.0 h1:v8rEWFl6EoqHB+swVNjVoCJE8o3jX7e8nqBGPLaDFBM=
 github.com/containerd/cgroups/v3 v3.0.2 h1:f5WFqIVSgo5IZmtTT3qVBo6TzI1ON6sycSBKkymb9L0=
 github.com/containerd/cgroups/v3 v3.0.2/go.mod h1:JUgITrzdFqp42uI2ryGA+ge0ap/nxzYgkGmIcetmErE=

pkg/compose/convergence.go

@@ -57,24 +57,25 @@ const (
 // Cross services dependencies are managed by creating services in expected order and updating `service:xx` reference
 // when a service has converged, so dependent ones can be managed with resolved containers references.
 type convergence struct {
-	service       *composeService
-	observedState map[string]Containers
-	stateMutex    sync.Mutex
+	service    *composeService
+	services   map[string]Containers
+	networks   map[string]string
+	stateMutex sync.Mutex
 }
 
 func (c *convergence) getObservedState(serviceName string) Containers {
 	c.stateMutex.Lock()
 	defer c.stateMutex.Unlock()
-	return c.observedState[serviceName]
+	return c.services[serviceName]
 }
 
 func (c *convergence) setObservedState(serviceName string, containers Containers) {
 	c.stateMutex.Lock()
 	defer c.stateMutex.Unlock()
-	c.observedState[serviceName] = containers
+	c.services[serviceName] = containers
 }
 
-func newConvergence(services []string, state Containers, s *composeService) *convergence {
+func newConvergence(services []string, state Containers, networks map[string]string, s *composeService) *convergence {
 	observedState := map[string]Containers{}
 	for _, s := range services {
 		observedState[s] = Containers{}
@@ -84,8 +85,9 @@ func newConvergence(services []string, state Containers, s *composeService) *con
 		observedState[service] = append(observedState[service], c)
 	}
 	return &convergence{
-		service:       s,
-		observedState: observedState,
+		service:  s,
+		services: observedState,
+		networks: networks,
 	}
 }
 
@@ -124,11 +126,11 @@ func (c *convergence) ensureService(ctx context.Context, project *types.Project,
 
 	sort.Slice(containers, func(i, j int) bool {
 		// select obsolete containers first, so they get removed as we scale down
-		if obsolete, _ := mustRecreate(service, containers[i], recreate); obsolete {
+		if obsolete, _ := c.mustRecreate(service, containers[i], recreate); obsolete {
 			// i is obsolete, so must be first in the list
 			return true
 		}
-		if obsolete, _ := mustRecreate(service, containers[j], recreate); obsolete {
+		if obsolete, _ := c.mustRecreate(service, containers[j], recreate); obsolete {
 			// j is obsolete, so must be first in the list
 			return false
 		}
@@ -157,7 +159,7 @@ func (c *convergence) ensureService(ctx context.Context, project *types.Project,
 			continue
 		}
 
-		mustRecreate, err := mustRecreate(service, container, recreate)
+		mustRecreate, err := c.mustRecreate(service, container, recreate)
 		if err != nil {
 			return err
 		}
@@ -315,7 +317,7 @@ func (c *convergence) resolveSharedNamespaces(service *types.ServiceConfig) erro
 	return nil
 }
 
-func mustRecreate(expected types.ServiceConfig, actual moby.Container, policy string) (bool, error) {
+func (c *convergence) mustRecreate(expected types.ServiceConfig, actual moby.Container, policy string) (bool, error) {
 	if policy == api.RecreateNever {
 		return false, nil
 	}
@@ -328,7 +330,33 @@ func mustRecreate(expected types.ServiceConfig, actual moby.Container, policy st
 	}
 	configChanged := actual.Labels[api.ConfigHashLabel] != configHash
 	imageUpdated := actual.Labels[api.ImageDigestLabel] != expected.CustomLabels[api.ImageDigestLabel]
-	return configChanged || imageUpdated, nil
+	if configChanged || imageUpdated {
+		return true, nil
+	}
+
+	if c.networks != nil {
+		// check the networks container is connected to are the expected ones
+		for net := range expected.Networks {
+			id := c.networks[net]
+			if id == "swarm" {
+				// corner-case : swarm overlay network isn't visible until a container is attached
+				continue
+			}
+			found := false
+			for _, settings := range actual.NetworkSettings.Networks {
+				if settings.NetworkID == id {
+					found = true
+					break
+				}
+			}
+			if !found {
+				// config is up-t-date but container is not connected to network - maybe recreated ?
+				return true, nil
+			}
+		}
+	}
+
+	return false, nil
 }
 
 func getContainerName(projectName string, service types.ServiceConfig, number int) string {

pkg/compose/create.go

@@ -80,12 +80,6 @@ func (s *composeService) create(ctx context.Context, project *types.Project, opt
 		return err
 	}
 
-	var observedState Containers
-	observedState, err = s.getContainers(ctx, project.Name, oneOffInclude, true)
-	if err != nil {
-		return err
-	}
-
 	err = s.ensureImagesExists(ctx, project, options.Build, options.QuietPull)
 	if err != nil {
 		return err
@@ -93,7 +87,8 @@ func (s *composeService) create(ctx context.Context, project *types.Project, opt
 
 	prepareNetworks(project)
 
-	if err := s.ensureNetworks(ctx, project.Networks); err != nil {
+	networks, err := s.ensureNetworks(ctx, project)
+	if err != nil {
 		return err
 	}
 
@@ -101,6 +96,11 @@ func (s *composeService) create(ctx context.Context, project *types.Project, opt
 		return err
 	}
 
+	var observedState Containers
+	observedState, err = s.getContainers(ctx, project.Name, oneOffInclude, true)
+	if err != nil {
+		return err
+	}
 	orphans := observedState.filter(isOrphaned(project))
 	if len(orphans) > 0 && !options.IgnoreOrphans {
 		if options.RemoveOrphans {
@@ -115,27 +115,30 @@ func (s *composeService) create(ctx context.Context, project *types.Project, opt
 				"--remove-orphans flag to clean it up.", orphans.names())
 		}
 	}
-	return newConvergence(options.Services, observedState, s).apply(ctx, project, options)
+	return newConvergence(options.Services, observedState, networks, s).apply(ctx, project, options)
 }
 
 func prepareNetworks(project *types.Project) {
 	for k, nw := range project.Networks {
-		nw.Labels = nw.Labels.Add(api.NetworkLabel, k)
-		nw.Labels = nw.Labels.Add(api.ProjectLabel, project.Name)
-		nw.Labels = nw.Labels.Add(api.VersionLabel, api.ComposeVersion)
+		nw.CustomLabels = nw.CustomLabels.
+			Add(api.NetworkLabel, k).
+			Add(api.ProjectLabel, project.Name).
+			Add(api.VersionLabel, api.ComposeVersion)
 		project.Networks[k] = nw
 	}
 }
 
-func (s *composeService) ensureNetworks(ctx context.Context, networks types.Networks) error {
-	for i, nw := range networks {
-		err := s.ensureNetwork(ctx, &nw)
+func (s *composeService) ensureNetworks(ctx context.Context, project *types.Project) (map[string]string, error) {
+	networks := map[string]string{}
+	for name, nw := range project.Networks {
+		id, err := s.ensureNetwork(ctx, project, name, &nw)
 		if err != nil {
-			return err
+			return nil, err
 		}
-		networks[i] = nw
+		networks[name] = id
+		project.Networks[name] = nw
 	}
-	return nil
+	return networks, nil
 }
 
 func (s *composeService) ensureProjectVolumes(ctx context.Context, project *types.Project) error {
@@ -1200,24 +1203,21 @@ func buildTmpfsOptions(tmpfs *types.ServiceVolumeTmpfs) *mount.TmpfsOptions {
 	}
 }
 
-func (s *composeService) ensureNetwork(ctx context.Context, n *types.NetworkConfig) error {
+func (s *composeService) ensureNetwork(ctx context.Context, project *types.Project, name string, n *types.NetworkConfig) (string, error) {
 	if n.External {
 		return s.resolveExternalNetwork(ctx, n)
 	}
 
-	err := s.resolveOrCreateNetwork(ctx, n)
+	id, err := s.resolveOrCreateNetwork(ctx, project, name, n)
 	if errdefs.IsConflict(err) {
 		// Maybe another execution of `docker compose up|run` created same network
 		// let's retry once
-		return s.resolveOrCreateNetwork(ctx, n)
+		return s.resolveOrCreateNetwork(ctx, project, "", n)
 	}
-	return err
+	return id, err
 }
 
-func (s *composeService) resolveOrCreateNetwork(ctx context.Context, n *types.NetworkConfig) error { //nolint:gocyclo
-	expectedNetworkLabel := n.Labels[api.NetworkLabel]
-	expectedProjectLabel := n.Labels[api.ProjectLabel]
-
+func (s *composeService) resolveOrCreateNetwork(ctx context.Context, project *types.Project, name string, n *types.NetworkConfig) (string, error) { //nolint:gocyclo
 	// First, try to find a unique network matching by name or ID
 	inspect, err := s.apiClient().NetworkInspect(ctx, n.Name, network.InspectOptions{})
 	if err == nil {
@@ -1228,20 +1228,33 @@ func (s *composeService) resolveOrCreateNetwork(ctx context.Context, n *types.Ne
 			if !ok {
 				logrus.Warnf("a network with name %s exists but was not created by compose.\n"+
 					"Set `external: true` to use an existing network", n.Name)
-			} else if p != expectedProjectLabel {
+			} else if p != project.Name {
 				logrus.Warnf("a network with name %s exists but was not created for project %q.\n"+
-					"Set `external: true` to use an existing network", n.Name, expectedProjectLabel)
+					"Set `external: true` to use an existing network", n.Name, project.Name)
 			}
-			if inspect.Labels[api.NetworkLabel] != expectedNetworkLabel {
-				return fmt.Errorf(
+			if inspect.Labels[api.NetworkLabel] != name {
+				return "", fmt.Errorf(
 					"network %s was found but has incorrect label %s set to %q (expected: %q)",
 					n.Name,
 					api.NetworkLabel,
 					inspect.Labels[api.NetworkLabel],
-					expectedNetworkLabel,
+					name,
 				)
 			}
-			return nil
+
+			hash := inspect.Labels[api.ConfigHashLabel]
+			expected, err := NetworkHash(n)
+			if err != nil {
+				return "", err
+			}
+			if hash == "" || hash == expected {
+				return inspect.ID, nil
+			}
+
+			err = s.removeDivergedNetwork(ctx, project, name, n)
+			if err != nil {
+				return "", err
+			}
 		}
 	}
 	// ignore other errors. Typically, an ambiguous request by name results in some generic `invalidParameter` error
@@ -1251,7 +1264,7 @@ func (s *composeService) resolveOrCreateNetwork(ctx context.Context, n *types.Ne
 		Filters: filters.NewArgs(filters.Arg("name", n.Name)),
 	})
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	// NetworkList Matches all or part of a network name, so we have to filter for a strict match
@@ -1260,9 +1273,9 @@ func (s *composeService) resolveOrCreateNetwork(ctx context.Context, n *types.Ne
 	})
 
 	for _, net := range networks {
-		if net.Labels[api.ProjectLabel] == expectedProjectLabel &&
-			net.Labels[api.NetworkLabel] == expectedNetworkLabel {
-			return nil
+		if net.Labels[api.ProjectLabel] == project.Name &&
+			net.Labels[api.NetworkLabel] == name {
+			return net.ID, nil
 		}
 	}
 
@@ -1272,7 +1285,7 @@ func (s *composeService) resolveOrCreateNetwork(ctx context.Context, n *types.Ne
 	if len(networks) > 0 {
 		logrus.Warnf("a network with name %s exists but was not created by compose.\n"+
 			"Set `external: true` to use an existing network", n.Name)
-		return nil
+		return networks[0].ID, nil
 	}
 
 	var ipam *network.IPAM
@@ -1291,8 +1304,13 @@ func (s *composeService) resolveOrCreateNetwork(ctx context.Context, n *types.Ne
 			Config: config,
 		}
 	}
+	hash, err := NetworkHash(n)
+	if err != nil {
+		return "", err
+	}
+	n.CustomLabels = n.CustomLabels.Add(api.ConfigHashLabel, hash)
 	createOpts := network.CreateOptions{
-		Labels:     n.Labels,
+		Labels:     mergeLabels(n.Labels, n.CustomLabels),
 		Driver:     n.Driver,
 		Options:    n.DriverOpts,
 		Internal:   n.Internal,
@@ -1322,16 +1340,42 @@ func (s *composeService) resolveOrCreateNetwork(ctx context.Context, n *types.Ne
 	w := progress.ContextWriter(ctx)
 	w.Event(progress.CreatingEvent(networkEventName))
 
-	_, err = s.apiClient().NetworkCreate(ctx, n.Name, createOpts)
+	resp, err := s.apiClient().NetworkCreate(ctx, n.Name, createOpts)
 	if err != nil {
 		w.Event(progress.ErrorEvent(networkEventName))
-		return fmt.Errorf("failed to create network %s: %w", n.Name, err)
+		return "", fmt.Errorf("failed to create network %s: %w", n.Name, err)
 	}
 	w.Event(progress.CreatedEvent(networkEventName))
-	return nil
+	return resp.ID, nil
 }
 
-func (s *composeService) resolveExternalNetwork(ctx context.Context, n *types.NetworkConfig) error {
+func (s *composeService) removeDivergedNetwork(ctx context.Context, project *types.Project, name string, n *types.NetworkConfig) error {
+	// Remove services attached to this network to force recreation
+	var services []string
+	for _, service := range project.Services.Filter(func(config types.ServiceConfig) bool {
+		_, ok := config.Networks[name]
+		return ok
+	}) {
+		services = append(services, service.Name)
+	}
+
+	// Stop containers so we can remove network
+	// They will be restarted (actually: recreated) with the updated network
+	err := s.stop(ctx, project.Name, api.StopOptions{
+		Services: services,
+		Project:  project,
+	})
+	if err != nil {
+		return err
+	}
+
+	err = s.apiClient().NetworkRemove(ctx, n.Name)
+	eventName := fmt.Sprintf("Network %s", n.Name)
+	progress.ContextWriter(ctx).Event(progress.RemovedEvent(eventName))
+	return err
+}
+
+func (s *composeService) resolveExternalNetwork(ctx context.Context, n *types.NetworkConfig) (string, error) {
 	// NetworkInspect will match on ID prefix, so NetworkList with a name
 	// filter is used to look for an exact match to prevent e.g. a network
 	// named `db` from getting erroneously matched to a network with an ID
@@ -1341,14 +1385,14 @@ func (s *composeService) resolveExternalNetwork(ctx context.Context, n *types.Ne
 	})
 
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	if len(networks) == 0 {
 		// in this instance, n.Name is really an ID
 		sn, err := s.apiClient().NetworkInspect(ctx, n.Name, network.InspectOptions{})
 		if err != nil && !errdefs.IsNotFound(err) {
-			return err
+			return "", err
 		}
 		networks = append(networks, sn)
 	}
@@ -1363,22 +1407,22 @@ func (s *composeService) resolveExternalNetwork(ctx context.Context, n *types.Ne
 
 	switch len(networks) {
 	case 1:
-		return nil
+		return networks[0].ID, nil
 	case 0:
 		enabled, err := s.isSWarmEnabled(ctx)
 		if err != nil {
-			return err
+			return "", err
 		}
 		if enabled {
 			// Swarm nodes do not register overlay networks that were
 			// created on a different node unless they're in use.
 			// So we can't preemptively check network exists, but
 			// networkAttach will later fail anyway if network actually doesn't exist
-			return nil
+			return "swarm", nil
 		}
-		return fmt.Errorf("network %s declared as external, but could not be found", n.Name)
+		return "", fmt.Errorf("network %s declared as external, but could not be found", n.Name)
 	default:
-		return fmt.Errorf("multiple networks with name %q were found. Use network ID as `name` to avoid ambiguity", n.Name)
+		return "", fmt.Errorf("multiple networks with name %q were found. Use network ID as `name` to avoid ambiguity", n.Name)
 	}
 }
 

pkg/compose/create_test.go

@@ -92,7 +92,7 @@ func TestPrepareNetworkLabels(t *testing.T) {
 		Networks: composetypes.Networks(map[string]composetypes.NetworkConfig{"skynet": {}}),
 	}
 	prepareNetworks(&project)
-	assert.DeepEqual(t, project.Networks["skynet"].Labels, composetypes.Labels(map[string]string{
+	assert.DeepEqual(t, project.Networks["skynet"].CustomLabels, composetypes.Labels(map[string]string{
 		"com.docker.compose.network": "skynet",
 		"com.docker.compose.project": "myProject",
 		"com.docker.compose.version": api.ComposeVersion,

pkg/compose/hash.go

@@ -41,3 +41,11 @@ func ServiceHash(o types.ServiceConfig) (string, error) {
 	}
 	return digest.SHA256.FromBytes(bytes).Encoded(), nil
 }
+
+func NetworkHash(o *types.NetworkConfig) (string, error) {
+	bytes, err := json.Marshal(o)
+	if err != nil {
+		return "", err
+	}
+	return digest.SHA256.FromBytes(bytes).Encoded(), nil
+}

pkg/compose/remove.go

@@ -81,6 +81,7 @@ func (s *composeService) Remove(ctx context.Context, projectName string, options
 		_, _ = fmt.Fprintln(s.stdinfo(), "No stopped containers")
 		return nil
 	}
+
 	msg := fmt.Sprintf("Going to remove %s", strings.Join(names, ", "))
 	if options.Force {
 		_, _ = fmt.Fprintln(s.stdout(), msg)

pkg/compose/run.go

@@ -104,7 +104,7 @@ func (s *composeService) prepareRun(ctx context.Context, project *types.Project,
 		Labels:            mergeLabels(service.Labels, service.CustomLabels),
 	}
 
-	err = newConvergence(project.ServiceNames(), observedState, s).resolveServiceReferences(&service)
+	err = newConvergence(project.ServiceNames(), observedState, nil, s).resolveServiceReferences(&service)
 	if err != nil {
 		return "", err
 	}

pkg/e2e/fixtures/network-test/compose.subnet.yaml

@@ -0,0 +1,12 @@
+services:
+  test:
+    image: nginx:alpine
+    networks:
+      - test
+
+networks:
+  test:
+    ipam:
+      config:
+        - subnet: ${SUBNET-172.28.0.0/16}
+

pkg/e2e/networks_test.go

@@ -147,3 +147,28 @@ func TestNetworkModes(t *testing.T) {
 		_ = c.RunDockerComposeCmd(t, "--project-name", projectName, "down")
 	})
 }
+
+func TestNetworkConfigChanged(t *testing.T) {
+	// fixture is shared with TestNetworks and is not safe to run concurrently
+	c := NewCLI(t)
+	const projectName = "network_config_change"
+
+	c.RunDockerComposeCmd(t, "-f", "./fixtures/network-test/compose.subnet.yaml", "--project-name", projectName, "up", "-d")
+	t.Cleanup(func() {
+		c.RunDockerComposeCmd(t, "--project-name", projectName, "down")
+	})
+
+	res := c.RunDockerComposeCmd(t, "--project-name", projectName, "exec", "test", "hostname", "-i")
+	res.Assert(t, icmd.Expected{Out: "172.28.0."})
+	res.Combined()
+
+	cmd := c.NewCmdWithEnv([]string{"SUBNET=192.168.0.0/16"},
+		"docker", "compose", "-f", "./fixtures/network-test/compose.subnet.yaml", "--project-name", projectName, "up", "-d")
+	res = icmd.RunCmd(cmd)
+	res.Assert(t, icmd.Success)
+	out := res.Combined()
+	fmt.Println(out)
+
+	res = c.RunDockerComposeCmd(t, "--project-name", projectName, "exec", "test", "hostname", "-i")
+	res.Assert(t, icmd.Expected{Out: "192.168.0."})
+}