Browse Source

avoid use of service.Name when iterating on project.Services

Signed-off-by: Nicolas De Loof <[email protected]>
Nicolas De Loof 1 year ago
parent
commit
26aca867d8

+ 3 - 3
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()

+ 1 - 1
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
 }

+ 4 - 3
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
 }

+ 1 - 1
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{

+ 7 - 7
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

+ 5 - 11
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})

+ 6 - 16
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
 }

+ 1 - 1
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

+ 2 - 2
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=

+ 1 - 4
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() {

+ 5 - 5
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
 }

+ 5 - 4
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
 }

+ 2 - 1
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]

+ 12 - 18
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)

+ 8 - 8
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),
 						})

+ 7 - 6
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)
 				}
 			}
 		}

+ 1 - 1
pkg/e2e/e2e_config_plugin.go

@@ -18,4 +18,4 @@
 
 package e2e
 
-const composeStandaloneMode = false
+const composeStandaloneMode = true