From ef63c2e8e801ef3fbdfc7913c0ba5b660a0f7a22 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Tue, 5 Nov 2024 15:42:59 +0100 Subject: [PATCH] detect network config changes and recreate if needed Signed-off-by: Nicolas De Loof --- go.mod | 2 +- go.sum | 4 +- pkg/compose/convergence.go | 54 ++++++++++---- pkg/compose/create.go | 140 ++++++++++++++++++++++++------------- pkg/compose/hash.go | 8 +++ pkg/compose/remove.go | 1 + pkg/compose/run.go | 2 +- 7 files changed, 146 insertions(+), 65 deletions(-) diff --git a/go.mod b/go.mod index 540fdb0081..4e52e7505d 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 8e8fd639f4..ec802875f7 100644 --- a/go.sum +++ b/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 v1.1.0/go.mod h1:6ppBcbh/NOOUU+dMKrykgaBnK9lCIBxHqJDGwsa1mIw= github.com/containerd/console v1.0.4 h1:F2g4+oChYvBTsASRTz8NP6iIAi97J3TtSAsLbIFn4ro= diff --git a/pkg/compose/convergence.go b/pkg/compose/convergence.go index d57c13b8b5..404e12ba67 100644 --- a/pkg/compose/convergence.go +++ b/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 { diff --git a/pkg/compose/create.go b/pkg/compose/create.go index 639c1fba33..0b8d92d071 100644 --- a/pkg/compose/create.go +++ b/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) 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) error { +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) } } diff --git a/pkg/compose/hash.go b/pkg/compose/hash.go index f284a84e09..221f439b43 100644 --- a/pkg/compose/hash.go +++ b/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 +} diff --git a/pkg/compose/remove.go b/pkg/compose/remove.go index 3866e5f9a9..993398be84 100644 --- a/pkg/compose/remove.go +++ b/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) diff --git a/pkg/compose/run.go b/pkg/compose/run.go index 7598b68bac..0d15600b09 100644 --- a/pkg/compose/run.go +++ b/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 }