From 26aca867d8a225e61206650272dafec970e74239 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Mon, 27 Nov 2023 11:02:19 +0100 Subject: [PATCH] avoid use of service.Name when iterating on project.Services Signed-off-by: Nicolas De Loof --- cmd/compose/compose.go | 6 +++--- cmd/compose/config.go | 2 +- cmd/compose/options.go | 7 ++++--- cmd/compose/options_test.go | 2 +- cmd/compose/run.go | 14 +++++++------- cmd/compose/scale.go | 16 +++++----------- cmd/compose/up.go | 22 ++++++---------------- go.mod | 2 +- go.sum | 4 ++-- internal/sync/docker_cp.go | 5 +---- pkg/api/api.go | 10 +++++----- pkg/compose/build.go | 9 +++++---- pkg/compose/compose.go | 3 ++- pkg/compose/convergence.go | 30 ++++++++++++------------------ pkg/compose/pull.go | 16 ++++++++-------- pkg/compose/viz_test.go | 13 +++++++------ pkg/e2e/e2e_config_plugin.go | 2 +- 17 files changed, 71 insertions(+), 92 deletions(-) diff --git a/cmd/compose/compose.go b/cmd/compose/compose.go index 76ae4ab092c..d2f8e98e9e2 100644 --- a/cmd/compose/compose.go +++ b/cmd/compose/compose.go @@ -230,10 +230,10 @@ func (o *ProjectOptions) ToProject(dockerCli command.Cli, services []string, po return nil, err } - for i, s := range project.Services { + for name, s := range project.Services { s.CustomLabels = map[string]string{ api.ProjectLabel: project.Name, - api.ServiceLabel: s.Name, + api.ServiceLabel: name, api.VersionLabel: api.ComposeVersion, api.WorkingDirLabel: project.WorkingDir, api.ConfigFilesLabel: strings.Join(project.ComposeFiles, ","), @@ -242,7 +242,7 @@ func (o *ProjectOptions) ToProject(dockerCli command.Cli, services []string, po if len(o.EnvFiles) != 0 { s.CustomLabels[api.EnvironmentFileLabel] = strings.Join(o.EnvFiles, ",") } - project.Services[i] = s + project.Services[name] = s } project.WithoutUnnecessaryResources() diff --git a/cmd/compose/config.go b/cmd/compose/config.go index c86d86a38a6..2128c6cea97 100644 --- a/cmd/compose/config.go +++ b/cmd/compose/config.go @@ -210,7 +210,7 @@ func runHash(ctx context.Context, dockerCli command.Cli, opts configOptions) err if err != nil { return err } - fmt.Fprintf(dockerCli.Out(), "%s %s\n", s.Name, hash) + fmt.Fprintf(dockerCli.Out(), "%s %s\n", name, hash) } return nil } diff --git a/cmd/compose/options.go b/cmd/compose/options.go index 3552841a194..948b6640099 100644 --- a/cmd/compose/options.go +++ b/cmd/compose/options.go @@ -25,7 +25,7 @@ import ( func applyPlatforms(project *types.Project, buildForSinglePlatform bool) error { defaultPlatform := project.Environment["DOCKER_DEFAULT_PLATFORM"] - for _, service := range project.Services { + for name, service := range project.Services { if service.Build == nil { continue } @@ -33,7 +33,7 @@ func applyPlatforms(project *types.Project, buildForSinglePlatform bool) error { // default platform only applies if the service doesn't specify if defaultPlatform != "" && service.Platform == "" { if len(service.Build.Platforms) > 0 && !utils.StringContains(service.Build.Platforms, defaultPlatform) { - return fmt.Errorf("service %q build.platforms does not support value set by DOCKER_DEFAULT_PLATFORM: %s", service.Name, defaultPlatform) + return fmt.Errorf("service %q build.platforms does not support value set by DOCKER_DEFAULT_PLATFORM: %s", name, defaultPlatform) } service.Platform = defaultPlatform } @@ -41,7 +41,7 @@ func applyPlatforms(project *types.Project, buildForSinglePlatform bool) error { if service.Platform != "" { if len(service.Build.Platforms) > 0 { if !utils.StringContains(service.Build.Platforms, service.Platform) { - return fmt.Errorf("service %q build configuration does not support platform: %s", service.Name, service.Platform) + return fmt.Errorf("service %q build configuration does not support platform: %s", name, service.Platform) } } @@ -68,6 +68,7 @@ func applyPlatforms(project *types.Project, buildForSinglePlatform bool) error { // empty indicates that the builder gets to decide service.Build.Platforms = nil } + project.Services[name] = service } return nil } diff --git a/cmd/compose/options_test.go b/cmd/compose/options_test.go index 28b34298ac5..866306e6c8b 100644 --- a/cmd/compose/options_test.go +++ b/cmd/compose/options_test.go @@ -101,7 +101,7 @@ func TestApplyPlatforms_UnsupportedPlatform(t *testing.T) { "DOCKER_DEFAULT_PLATFORM": "commodore/64", }, Services: types.Services{ - "foo": { + "test": { Name: "test", Image: "foo", Build: &types.BuildConfig{ diff --git a/cmd/compose/run.go b/cmd/compose/run.go index e252315fdab..de996778f7a 100644 --- a/cmd/compose/run.go +++ b/cmd/compose/run.go @@ -105,9 +105,9 @@ func (options runOptions) apply(project *types.Project) error { target.Volumes = append(target.Volumes, volume) } - for i, s := range project.Services { - if s.Name == options.Service { - project.Services[i] = target + for name := range project.Services { + if name == options.Service { + project.Services[name] = target break } } @@ -279,10 +279,10 @@ func runRun(ctx context.Context, backend api.Service, project *types.Project, op QuietPull: options.quietPull, } - for i, service := range project.Services { - if service.Name == options.Service { + for name, service := range project.Services { + if name == options.Service { service.StdinOpen = options.interactive - project.Services[i] = service + project.Services[name] = service } } @@ -301,7 +301,7 @@ func startDependencies(ctx context.Context, backend api.Service, project types.P dependencies := types.Services{} var requestedService types.ServiceConfig for name, service := range project.Services { - if service.Name != requestedServiceName { + if name != requestedServiceName { dependencies[name] = service } else { requestedService = service diff --git a/cmd/compose/scale.go b/cmd/compose/scale.go index c547293b8a7..78b23ae6f67 100644 --- a/cmd/compose/scale.go +++ b/cmd/compose/scale.go @@ -73,18 +73,12 @@ func runScale(ctx context.Context, dockerCli command.Cli, backend api.Service, o } for key, value := range serviceReplicaTuples { - for i, service := range project.Services { - if service.Name != key { - continue - } - value := value - service.Scale = &value - if service.Deploy != nil { - service.Deploy.Replicas = &value - } - project.Services[i] = service - break + service, err := project.GetService(key) + if err != nil { + return err } + service.SetScale(value) + project.Services[key] = service } return backend.Scale(ctx, project, api.ScaleOptions{Services: services}) diff --git a/cmd/compose/up.go b/cmd/compose/up.go index 1f0b3a0c8bd..f68c794030e 100644 --- a/cmd/compose/up.go +++ b/cmd/compose/up.go @@ -260,21 +260,11 @@ func runUp( } func setServiceScale(project *types.Project, name string, replicas int) error { - for i, s := range project.Services { - if s.Name != name { - continue - } - - service, err := project.GetService(name) - if err != nil { - return err - } - service.Scale = &replicas - if service.Deploy != nil { - service.Deploy.Replicas = &replicas - } - project.Services[i] = service - return nil + service, err := project.GetService(name) + if err != nil { + return err } - return fmt.Errorf("unknown service %q", name) + service.SetScale(replicas) + project.Services[name] = service + return nil } diff --git a/go.mod b/go.mod index 5e30d7e443b..b6c7ba0e3f0 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/AlecAivazis/survey/v2 v2.3.7 github.com/Microsoft/go-winio v0.6.1 github.com/buger/goterm v1.0.4 - github.com/compose-spec/compose-go/v2 v2.0.0-20231123162526-11ef9572f1a4 + github.com/compose-spec/compose-go/v2 v2.0.0-beta.1 github.com/containerd/console v1.0.3 github.com/containerd/containerd v1.7.7 github.com/davecgh/go-spew v1.1.1 diff --git a/go.sum b/go.sum index b539794ea2d..94b2e9197da 100644 --- a/go.sum +++ b/go.sum @@ -132,8 +132,8 @@ github.com/cncf/xds/go v0.0.0-20230607035331-e9ce68804cb4 h1:/inchEIKaYC1Akx+H+g github.com/cncf/xds/go v0.0.0-20230607035331-e9ce68804cb4/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= 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.0.0-20231123162526-11ef9572f1a4 h1:Lr78By808iuG+2gTyxIDslRpKQCk/lcRqElKsrhzp+U= -github.com/compose-spec/compose-go/v2 v2.0.0-20231123162526-11ef9572f1a4/go.mod h1:PWCgeD8cxiI/DmdpBM407CuLDrZ2W4xuS6/Z9jAi0YQ= +github.com/compose-spec/compose-go/v2 v2.0.0-beta.1 h1:/A+2QMQVSsAmr9Gn5fm6YwaufjRZmWBnHYjr0oCyGiw= +github.com/compose-spec/compose-go/v2 v2.0.0-beta.1/go.mod h1:PWCgeD8cxiI/DmdpBM407CuLDrZ2W4xuS6/Z9jAi0YQ= 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.3 h1:lIr7SlA5PxZyMV30bDW0MGbiOPXwc63yRuCP0ARubLw= diff --git a/internal/sync/docker_cp.go b/internal/sync/docker_cp.go index f11364cc253..47077b404e5 100644 --- a/internal/sync/docker_cp.go +++ b/internal/sync/docker_cp.go @@ -62,10 +62,7 @@ func (d *DockerCopy) Sync(ctx context.Context, service types.ServiceConfig, path } func (d *DockerCopy) sync(ctx context.Context, service types.ServiceConfig, pathMapping PathMapping) error { - scale := 1 - if service.Scale != nil { - scale = *service.Scale - } + scale := service.GetScale() if fi, statErr := os.Stat(pathMapping.HostPath); statErr == nil { if fi.IsDir() { diff --git a/pkg/api/api.go b/pkg/api/api.go index 589713d29b1..83176381aed 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -146,9 +146,9 @@ type BuildOptions struct { // Apply mutates project according to build options func (o BuildOptions) Apply(project *types.Project) error { platform := project.Environment["DOCKER_DEFAULT_PLATFORM"] - for i, service := range project.Services { + for name, service := range project.Services { if service.Image == "" && service.Build == nil { - return fmt.Errorf("invalid service %q. Must specify either image or build", service.Name) + return fmt.Errorf("invalid service %q. Must specify either image or build", name) } if service.Build == nil { @@ -156,20 +156,20 @@ func (o BuildOptions) Apply(project *types.Project) error { } if platform != "" { if len(service.Build.Platforms) > 0 && !utils.StringContains(service.Build.Platforms, platform) { - return fmt.Errorf("service %q build.platforms does not support value set by DOCKER_DEFAULT_PLATFORM: %s", service.Name, platform) + return fmt.Errorf("service %q build.platforms does not support value set by DOCKER_DEFAULT_PLATFORM: %s", name, platform) } service.Platform = platform } if service.Platform != "" { if len(service.Build.Platforms) > 0 && !utils.StringContains(service.Build.Platforms, service.Platform) { - return fmt.Errorf("service %q build configuration does not support platform: %s", service.Name, service.Platform) + return fmt.Errorf("service %q build configuration does not support platform: %s", name, service.Platform) } } service.Build.Pull = service.Build.Pull || o.Pull service.Build.NoCache = service.Build.NoCache || o.NoCache - project.Services[i] = service + project.Services[name] = service } return nil } diff --git a/pkg/compose/build.go b/pkg/compose/build.go index 1b356007bc8..d24a8fcbf46 100644 --- a/pkg/compose/build.go +++ b/pkg/compose/build.go @@ -189,7 +189,7 @@ func (s *composeService) build(ctx context.Context, project *types.Project, opti return err } - digest, err := s.doBuildBuildkit(ctx, service.Name, buildOptions, w, nodes) + digest, err := s.doBuildBuildkit(ctx, name, buildOptions, w, nodes) if err != nil { return err } @@ -221,9 +221,9 @@ func (s *composeService) build(ctx context.Context, project *types.Project, opti } func (s *composeService) ensureImagesExists(ctx context.Context, project *types.Project, buildOpts *api.BuildOptions, quietPull bool) error { - for _, service := range project.Services { + for name, service := range project.Services { if service.Image == "" && service.Build == nil { - return fmt.Errorf("invalid service %q. Must specify either image or build", service.Name) + return fmt.Errorf("invalid service %q. Must specify either image or build", name) } } @@ -261,7 +261,7 @@ func (s *composeService) ensureImagesExists(ctx context.Context, project *types. } // set digest as com.docker.compose.image label so we can detect outdated containers - for _, service := range project.Services { + for name, service := range project.Services { image := api.GetImageNameOrDefault(service, project.Name) digest, ok := images[image] if ok { @@ -270,6 +270,7 @@ func (s *composeService) ensureImagesExists(ctx context.Context, project *types. } service.CustomLabels.Add(api.ImageDigestLabel, digest) } + project.Services[name] = service } return nil } diff --git a/pkg/compose/compose.go b/pkg/compose/compose.go index 624d2f4992b..18dc65a1c65 100644 --- a/pkg/compose/compose.go +++ b/pkg/compose/compose.go @@ -193,9 +193,10 @@ func (s *composeService) projectFromName(containers Containers, projectName stri Image: c.Image, Labels: c.Labels, } - set[serviceLabel] = service + } service.Scale = increment(service.Scale) + set[serviceLabel] = service } for name, service := range set { dependencies := service.Labels[api.DependenciesLabel] diff --git a/pkg/compose/convergence.go b/pkg/compose/convergence.go index d2c0fe603f4..4ba31ff09ac 100644 --- a/pkg/compose/convergence.go +++ b/pkg/compose/convergence.go @@ -205,6 +205,16 @@ func (c *convergence) ensureService(ctx context.Context, project *types.Project, return err } +func getScale(config types.ServiceConfig) (int, error) { + scale := config.GetScale() + if scale > 1 && config.ContainerName != "" { + return 0, fmt.Errorf(doubledContainerNameWarning, + config.Name, + config.ContainerName) + } + return scale, nil +} + // resolveServiceReferences replaces reference to another service with reference to an actual container func (c *convergence) resolveServiceReferences(service *types.ServiceConfig) error { err := c.resolveVolumeFrom(service) @@ -428,7 +438,7 @@ func shouldWaitForDependency(serviceName string, dependencyConfig types.ServiceD } } return false, err - } else if service.Scale != nil && *service.Scale == 0 { + } else if service.GetScale() == 0 { // don't wait for the dependency which configured to have 0 containers running return false, nil } @@ -455,22 +465,6 @@ func nextContainerNumber(containers []moby.Container) int { } -func getScale(config types.ServiceConfig) (int, error) { - scale := 1 - if config.Scale != nil { - scale = *config.Scale - } else if config.Deploy != nil && config.Deploy.Replicas != nil { - // this should not be required as compose-go enforce consistency between scale anr replicas - scale = *config.Deploy.Replicas - } - if scale > 1 && config.ContainerName != "" { - return 0, fmt.Errorf(doubledContainerNameWarning, - config.Name, - config.ContainerName) - } - return scale, nil -} - func (s *composeService) createContainer(ctx context.Context, project *types.Project, service types.ServiceConfig, name string, number int, opts createOptions) (container moby.Container, err error) { w := progress.ContextWriter(ctx) @@ -754,7 +748,7 @@ func (s *composeService) startService(ctx context.Context, project *types.Projec } if len(containers) == 0 { - if scale, err := getScale(service); err != nil && scale == 0 { + if service.GetScale() == 0 { return nil } return fmt.Errorf("service %q has no container to start", service.Name) diff --git a/pkg/compose/pull.go b/pkg/compose/pull.go index 4b1f912d7b0..977397bd096 100644 --- a/pkg/compose/pull.go +++ b/pkg/compose/pull.go @@ -64,10 +64,10 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts ) i := 0 - for _, service := range project.Services { + for name, service := range project.Services { if service.Image == "" { w.Event(progress.Event{ - ID: service.Name, + ID: name, Status: progress.Done, Text: "Skipped - No image to be pulled", }) @@ -77,7 +77,7 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts switch service.PullPolicy { case types.PullPolicyNever, types.PullPolicyBuild: w.Event(progress.Event{ - ID: service.Name, + ID: name, Status: progress.Done, Text: "Skipped", }) @@ -85,7 +85,7 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts case types.PullPolicyMissing, types.PullPolicyIfNotPresent: if imageAlreadyPresent(service.Image, images) { w.Event(progress.Event{ - ID: service.Name, + ID: name, Status: progress.Done, Text: "Skipped - Image is already present locally", }) @@ -95,7 +95,7 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts if service.Build != nil && opts.IgnoreBuildable { w.Event(progress.Event{ - ID: service.Name, + ID: name, Status: progress.Done, Text: "Skipped - Image can be built", }) @@ -104,7 +104,7 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts if s, ok := imagesBeingPulled[service.Image]; ok { w.Event(progress.Event{ - ID: service.Name, + ID: name, Status: progress.Done, Text: fmt.Sprintf("Skipped - Image is already being pulled by %v", s), }) @@ -113,7 +113,7 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts imagesBeingPulled[service.Image] = service.Name - idx, service := i, service + idx, name, service := i, name, service eg.Go(func() error { _, err := s.pullServiceImage(ctx, service, s.configFile(), w, false, project.Environment["DOCKER_DEFAULT_PLATFORM"]) if err != nil { @@ -124,7 +124,7 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts if !opts.IgnoreFailures && service.Build == nil { if s.dryRun { w.Event(progress.Event{ - ID: service.Name, + ID: name, Status: progress.Error, Text: fmt.Sprintf(" - Pull error for image: %s", service.Image), }) diff --git a/pkg/compose/viz_test.go b/pkg/compose/viz_test.go index 78cbd3458eb..5591996fb59 100644 --- a/pkg/compose/viz_test.go +++ b/pkg/compose/viz_test.go @@ -149,11 +149,12 @@ func TestViz(t *testing.T) { // check edges that SHOULD exist in the generated graph allowedEdges := make(map[string][]string) - for _, service := range project.Services { - allowedEdges[service.Name] = make([]string, 0, len(service.DependsOn)) + for name, service := range project.Services { + allowed := make([]string, 0, len(service.DependsOn)) for depName := range service.DependsOn { - allowedEdges[service.Name] = append(allowedEdges[service.Name], depName) + allowed = append(allowed, depName) } + allowedEdges[name] = allowed } for serviceName, dependencies := range allowedEdges { for _, dependencyName := range dependencies { @@ -163,12 +164,12 @@ func TestViz(t *testing.T) { // check edges that SHOULD NOT exist in the generated graph forbiddenEdges := make(map[string][]string) - for _, service := range project.Services { - forbiddenEdges[service.Name] = make([]string, 0, len(project.ServiceNames())-len(service.DependsOn)) + for name, service := range project.Services { + forbiddenEdges[name] = make([]string, 0, len(project.ServiceNames())-len(service.DependsOn)) for _, serviceName := range project.ServiceNames() { _, edgeExists := service.DependsOn[serviceName] if !edgeExists { - forbiddenEdges[service.Name] = append(forbiddenEdges[service.Name], serviceName) + forbiddenEdges[name] = append(forbiddenEdges[name], serviceName) } } } diff --git a/pkg/e2e/e2e_config_plugin.go b/pkg/e2e/e2e_config_plugin.go index 84ca6eabcd9..5de99481f1a 100644 --- a/pkg/e2e/e2e_config_plugin.go +++ b/pkg/e2e/e2e_config_plugin.go @@ -18,4 +18,4 @@ package e2e -const composeStandaloneMode = false +const composeStandaloneMode = true