From 8976a2069851c69ee164bd5eab3c196b1a258d2d Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Wed, 23 Dec 2020 17:57:10 -0300 Subject: [PATCH 1/5] Use `container_name` property on service This applies `container_name` when available in the service definition. Signed-off-by: Ulysses Souza --- formatter/logs.go | 2 +- local/compose/attach.go | 2 +- local/compose/convergence.go | 30 +++++++++++++++++++++++++++++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/formatter/logs.go b/formatter/logs.go index b70b9f45..c3542bcd 100644 --- a/formatter/logs.go +++ b/formatter/logs.go @@ -48,7 +48,7 @@ func (l *logConsumer) Log(service, container, message string) { l.colors[service] = cf l.computeWidth() } - prefix := fmt.Sprintf("%-"+strconv.Itoa(l.width)+"s |", service) + prefix := fmt.Sprintf("%-"+strconv.Itoa(l.width)+"s |", container) for _, line := range strings.Split(message, "\n") { buf := bytes.NewBufferString(fmt.Sprintf("%s %s\n", cf(prefix), line)) diff --git a/local/compose/attach.go b/local/compose/attach.go index 3866cf9a..47f0c8cf 100644 --- a/local/compose/attach.go +++ b/local/compose/attach.go @@ -61,7 +61,7 @@ func (s *composeService) attach(ctx context.Context, project *types.Project, con func (s *composeService) attachContainer(ctx context.Context, container moby.Container, consumer compose.LogConsumer, project *types.Project) error { serviceName := container.Labels[serviceLabel] - w := getWriter(serviceName, container.ID, consumer) + w := getWriter(serviceName, getContainerName(container), consumer) service, err := project.GetService(serviceName) if err != nil { diff --git a/local/compose/convergence.go b/local/compose/convergence.go index 4721a8ab..89d46042 100644 --- a/local/compose/convergence.go +++ b/local/compose/convergence.go @@ -26,6 +26,7 @@ import ( moby "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/network" + "github.com/docker/docker/client" "golang.org/x/sync/errgroup" status "github.com/docker/compose-cli/local/moby" @@ -35,11 +36,25 @@ import ( const ( extLifecycle = "x-lifecycle" forceRecreate = "force_recreate" + + doubledContainerNameWarning = "WARNING: The %q service is using the custom container name %q. " + + "Docker requires each container to have a unique name. " + + "Remove the custom name to scale the service.\n" ) +func containerExists(ctx context.Context, c *client.Client, name string) bool { + container, err := c.ContainerInspect(ctx, name) + return err == nil && container.ContainerJSONBase != nil && container.Name == "/"+name +} + func (s *composeService) ensureService(ctx context.Context, observedState Containers, project *types.Project, service types.ServiceConfig) error { scale := getScale(service) actual := observedState.filter(isService(service.Name)) + if scale > 1 && service.ContainerName != "" { + return fmt.Errorf(doubledContainerNameWarning, + service.Name, + service.ContainerName) + } eg, _ := errgroup.WithContext(ctx) if len(actual) < scale { @@ -50,8 +65,13 @@ func (s *composeService) ensureService(ctx context.Context, observedState Contai missing := scale - len(actual) for i := 0; i < missing; i++ { number := next + i - name := fmt.Sprintf("%s_%s_%d", project.Name, service.Name, number) + name := getContainerLogPrefix(project.Name, service, number) eg.Go(func() error { + if containerExists(ctx, s.apiClient, name) { + return fmt.Errorf(doubledContainerNameWarning, + service.Name, + name) + } return s.createContainer(ctx, project, service, name, number, false) }) } @@ -104,6 +124,14 @@ func (s *composeService) ensureService(ctx context.Context, observedState Contai return eg.Wait() } +func getContainerLogPrefix(projectName string, service types.ServiceConfig, number int) string { + name := fmt.Sprintf("%s_%s_%d", projectName, service.Name, number) + if service.ContainerName != "" { + name = service.ContainerName + } + return name +} + func (s *composeService) waitDependencies(ctx context.Context, project *types.Project, service types.ServiceConfig) error { eg, _ := errgroup.WithContext(ctx) for dep, config := range service.DependsOn { From b264e1814bda13f96228201e72fef461e9b7cd32 Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Wed, 6 Jan 2021 13:43:23 -0300 Subject: [PATCH 2/5] Add warning on container_name and scale > 1 Signed-off-by: Ulysses Souza --- local/compose/convergence.go | 62 +++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/local/compose/convergence.go b/local/compose/convergence.go index 89d46042..0da09d8a 100644 --- a/local/compose/convergence.go +++ b/local/compose/convergence.go @@ -47,31 +47,18 @@ func containerExists(ctx context.Context, c *client.Client, name string) bool { return err == nil && container.ContainerJSONBase != nil && container.Name == "/"+name } -func (s *composeService) ensureService(ctx context.Context, observedState Containers, project *types.Project, service types.ServiceConfig) error { - scale := getScale(service) - actual := observedState.filter(isService(service.Name)) - if scale > 1 && service.ContainerName != "" { - return fmt.Errorf(doubledContainerNameWarning, - service.Name, - service.ContainerName) - } - +func (s *composeService) ensureScale(ctx context.Context, actual []moby.Container, scale int, project *types.Project, service types.ServiceConfig) (*errgroup.Group, []moby.Container, error) { eg, _ := errgroup.WithContext(ctx) if len(actual) < scale { next, err := nextContainerNumber(actual) if err != nil { - return err + return nil, actual, err } missing := scale - len(actual) for i := 0; i < missing; i++ { number := next + i name := getContainerLogPrefix(project.Name, service, number) eg.Go(func() error { - if containerExists(ctx, s.apiClient, name) { - return fmt.Errorf(doubledContainerNameWarning, - service.Name, - name) - } return s.createContainer(ctx, project, service, name, number, false) }) } @@ -90,6 +77,30 @@ func (s *composeService) ensureService(ctx context.Context, observedState Contai } actual = actual[:scale] } + return eg, actual, nil +} + +func (s *composeService) ensureService(ctx context.Context, observedState Containers, project *types.Project, service types.ServiceConfig) error { + actual, err := s.apiClient.ContainerList(ctx, moby.ContainerListOptions{ + Filters: filters.NewArgs( + projectFilter(project.Name), + serviceFilter(service.Name), + ), + All: true, + }) + if err != nil { + return err + } + + scale, err := getScale(service) + if err != nil { + return err + } + + eg, actual, err := s.ensureScale(ctx, actual, scale, project, service) + if err != nil { + return err + } expected, err := jsonHash(service) if err != nil { @@ -171,17 +182,30 @@ func nextContainerNumber(containers []moby.Container) (int, error) { } -func getScale(config types.ServiceConfig) int { +func getScale(config types.ServiceConfig) (int, error) { + scale := 1 + var err error if config.Deploy != nil && config.Deploy.Replicas != nil { - return int(*config.Deploy.Replicas) + scale = int(*config.Deploy.Replicas) } if config.Scale != 0 { - return config.Scale + scale = config.Scale } - return 1 + if scale > 1 && config.ContainerName != "" { + scale = -1 + err = fmt.Errorf(doubledContainerNameWarning, + config.Name, + config.ContainerName) + } + return scale, err } func (s *composeService) createContainer(ctx context.Context, project *types.Project, service types.ServiceConfig, name string, number int, autoRemove bool) error { + if containerExists(ctx, s.apiClient, name) { + return fmt.Errorf(doubledContainerNameWarning, + service.Name, + name) + } w := progress.ContextWriter(ctx) w.Event(progress.CreatingEvent(name)) err := s.createMobyContainer(ctx, project, service, name, number, nil, autoRemove) From b3993230d1674787cedc1517ded636fa77f5538f Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Wed, 6 Jan 2021 15:15:15 -0300 Subject: [PATCH 3/5] Add network alias to container name Signed-off-by: Ulysses Souza --- local/compose/attach.go | 4 ++-- local/compose/compose.go | 5 +++-- local/compose/containers.go | 2 +- local/compose/convergence.go | 30 +++++++++++++++--------------- local/compose/create.go | 6 +++--- local/compose/down.go | 2 +- local/compose/ps.go | 5 +++-- 7 files changed, 28 insertions(+), 26 deletions(-) diff --git a/local/compose/attach.go b/local/compose/attach.go index 47f0c8cf..9cccf3c5 100644 --- a/local/compose/attach.go +++ b/local/compose/attach.go @@ -45,7 +45,7 @@ func (s *composeService) attach(ctx context.Context, project *types.Project, con var names []string for _, c := range containers { - names = append(names, getContainerName(c)) + names = append(names, getCanonicalContainerName(c)) } fmt.Printf("Attaching to %s\n", strings.Join(names, ", ")) @@ -61,7 +61,7 @@ func (s *composeService) attach(ctx context.Context, project *types.Project, con func (s *composeService) attachContainer(ctx context.Context, container moby.Container, consumer compose.LogConsumer, project *types.Project) error { serviceName := container.Labels[serviceLabel] - w := getWriter(serviceName, getContainerName(container), consumer) + w := getWriter(serviceName, getCanonicalContainerName(container), consumer) service, err := project.GetService(serviceName) if err != nil { diff --git a/local/compose/compose.go b/local/compose/compose.go index 402f3f0e..88cb6fc4 100644 --- a/local/compose/compose.go +++ b/local/compose/compose.go @@ -25,10 +25,11 @@ import ( "github.com/docker/compose-cli/api/compose" "github.com/compose-spec/compose-go/types" - errdefs2 "github.com/docker/compose-cli/errdefs" moby "github.com/docker/docker/api/types" "github.com/docker/docker/client" "github.com/sanathkr/go-yaml" + + errdefs2 "github.com/docker/compose-cli/errdefs" ) // NewComposeService create a local implementation of the compose.Service API @@ -44,7 +45,7 @@ func (s *composeService) Up(ctx context.Context, project *types.Project, options return errdefs2.ErrNotImplemented } -func getContainerName(c moby.Container) string { +func getCanonicalContainerName(c moby.Container) string { // Names return container canonical name /foo + link aliases /linked_by/foo for _, name := range c.Names { if strings.LastIndex(name, "/") == 0 { diff --git a/local/compose/containers.go b/local/compose/containers.go index 4159045c..300700e2 100644 --- a/local/compose/containers.go +++ b/local/compose/containers.go @@ -66,7 +66,7 @@ func (containers Containers) split(predicate containerPredicate) (Containers, Co func (containers Containers) names() []string { var names []string for _, c := range containers { - names = append(names, getContainerName(c)) + names = append(names, getCanonicalContainerName(c)) } return names } diff --git a/local/compose/convergence.go b/local/compose/convergence.go index 0da09d8a..eb58e620 100644 --- a/local/compose/convergence.go +++ b/local/compose/convergence.go @@ -57,7 +57,7 @@ func (s *composeService) ensureScale(ctx context.Context, actual []moby.Containe missing := scale - len(actual) for i := 0; i < missing; i++ { number := next + i - name := getContainerLogPrefix(project.Name, service, number) + name := getContainerName(project.Name, service, number) eg.Go(func() error { return s.createContainer(ctx, project, service, name, number, false) }) @@ -109,7 +109,7 @@ func (s *composeService) ensureService(ctx context.Context, observedState Contai for _, container := range actual { container := container - name := getContainerName(container) + name := getCanonicalContainerName(container) diverged := container.Labels[configHashLabel] != expected if diverged || service.Extensions[extLifecycle] == forceRecreate { @@ -135,7 +135,7 @@ func (s *composeService) ensureService(ctx context.Context, observedState Contai return eg.Wait() } -func getContainerLogPrefix(projectName string, service types.ServiceConfig, number int) string { +func getContainerName(projectName string, service types.ServiceConfig, number int) string { name := fmt.Sprintf("%s_%s_%d", projectName, service.Name, number) if service.ContainerName != "" { name = service.ContainerName @@ -218,12 +218,12 @@ func (s *composeService) createContainer(ctx context.Context, project *types.Pro func (s *composeService) recreateContainer(ctx context.Context, project *types.Project, service types.ServiceConfig, container moby.Container) error { w := progress.ContextWriter(ctx) - w.Event(progress.NewEvent(getContainerName(container), progress.Working, "Recreate")) + w.Event(progress.NewEvent(getCanonicalContainerName(container), progress.Working, "Recreate")) err := s.apiClient.ContainerStop(ctx, container.ID, nil) if err != nil { return err } - name := getContainerName(container) + name := getCanonicalContainerName(container) tmpName := fmt.Sprintf("%s_%s", container.ID[:12], name) err = s.apiClient.ContainerRename(ctx, container.ID, tmpName) if err != nil { @@ -241,7 +241,7 @@ func (s *composeService) recreateContainer(ctx context.Context, project *types.P if err != nil { return err } - w.Event(progress.NewEvent(getContainerName(container), progress.Done, "Recreated")) + w.Event(progress.NewEvent(getCanonicalContainerName(container), progress.Done, "Recreated")) setDependentLifecycle(project, service.Name, forceRecreate) return nil } @@ -261,12 +261,12 @@ func setDependentLifecycle(project *types.Project, service string, strategy stri func (s *composeService) restartContainer(ctx context.Context, container moby.Container) error { w := progress.ContextWriter(ctx) - w.Event(progress.NewEvent(getContainerName(container), progress.Working, "Restart")) + w.Event(progress.NewEvent(getCanonicalContainerName(container), progress.Working, "Restart")) err := s.apiClient.ContainerStart(ctx, container.ID, moby.ContainerStartOptions{}) if err != nil { return err } - w.Event(progress.NewEvent(getContainerName(container), progress.Done, "Restarted")) + w.Event(progress.NewEvent(getCanonicalContainerName(container), progress.Done, "Restarted")) return nil } @@ -281,8 +281,8 @@ func (s *composeService) createMobyContainer(ctx context.Context, project *types } id := created.ID for netName := range service.Networks { - network := project.Networks[netName] - err = s.connectContainerToNetwork(ctx, id, service.Name, network.Name) + netwrk := project.Networks[netName] + err = s.connectContainerToNetwork(ctx, id, netwrk.Name, service.Name, getContainerName(project.Name, service, number)) if err != nil { return err } @@ -290,9 +290,9 @@ func (s *composeService) createMobyContainer(ctx context.Context, project *types return nil } -func (s *composeService) connectContainerToNetwork(ctx context.Context, id string, service string, n string) error { - err := s.apiClient.NetworkConnect(ctx, n, id, &network.EndpointSettings{ - Aliases: []string{service}, +func (s *composeService) connectContainerToNetwork(ctx context.Context, id string, netwrk string, aliases ...string) error { + err := s.apiClient.NetworkConnect(ctx, netwrk, id, &network.EndpointSettings{ + Aliases: aliases, }) if err != nil { return err @@ -352,10 +352,10 @@ func (s *composeService) startService(ctx context.Context, project *types.Projec } eg.Go(func() error { w := progress.ContextWriter(ctx) - w.Event(progress.StartingEvent(getContainerName(container))) + w.Event(progress.StartingEvent(getCanonicalContainerName(container))) err := s.apiClient.ContainerStart(ctx, container.ID, moby.ContainerStartOptions{}) if err == nil { - w.Event(progress.StartedEvent(getContainerName(container))) + w.Event(progress.StartedEvent(getCanonicalContainerName(container))) } return err }) diff --git a/local/compose/create.go b/local/compose/create.go index 8855b2ef..6e445ce7 100644 --- a/local/compose/create.go +++ b/local/compose/create.go @@ -213,7 +213,7 @@ func getCreateOptions(p *types.Project, s types.ServiceConfig, number int, inher Resources: resources, } - networkConfig := buildDefaultNetworkConfig(s, networkMode) + networkConfig := buildDefaultNetworkConfig(s, networkMode, getContainerName(p.Name, s, number)) return &containerConfig, &hostConfig, networkConfig, nil } @@ -359,11 +359,11 @@ func buildTmpfsOptions(tmpfs *types.ServiceVolumeTmpfs) *mount.TmpfsOptions { } } -func buildDefaultNetworkConfig(s types.ServiceConfig, networkMode container.NetworkMode) *network.NetworkingConfig { +func buildDefaultNetworkConfig(s types.ServiceConfig, networkMode container.NetworkMode, containerName string) *network.NetworkingConfig { config := map[string]*network.EndpointSettings{} net := string(networkMode) config[net] = &network.EndpointSettings{ - Aliases: getAliases(s, s.Networks[net]), + Aliases: append(getAliases(s, s.Networks[net]), containerName), } return &network.NetworkingConfig{ diff --git a/local/compose/down.go b/local/compose/down.go index 2d2d203d..bd699ae6 100644 --- a/local/compose/down.go +++ b/local/compose/down.go @@ -94,7 +94,7 @@ func (s *composeService) removeContainers(ctx context.Context, w progress.Writer for _, container := range containers { toDelete := container eg.Go(func() error { - eventName := "Container " + getContainerName(toDelete) + eventName := "Container " + getCanonicalContainerName(toDelete) w.Event(progress.StoppingEvent(eventName)) err := s.apiClient.ContainerStop(ctx, toDelete.ID, nil) if err != nil { diff --git a/local/compose/ps.go b/local/compose/ps.go index 7c6ca1b5..9dfcdd7e 100644 --- a/local/compose/ps.go +++ b/local/compose/ps.go @@ -21,9 +21,10 @@ import ( "fmt" "sort" - "github.com/docker/compose-cli/api/compose" moby "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" + + "github.com/docker/compose-cli/api/compose" ) func (s *composeService) Ps(ctx context.Context, projectName string) ([]compose.ContainerSummary, error) { @@ -54,7 +55,7 @@ func (s *composeService) Ps(ctx context.Context, projectName string) ([]compose. summary = append(summary, compose.ContainerSummary{ ID: c.ID, - Name: getContainerName(c), + Name: getCanonicalContainerName(c), Project: c.Labels[projectLabel], Service: c.Labels[serviceLabel], State: c.State, From 0021b14de81c8ec566309deb5641adc311fceb94 Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Wed, 6 Jan 2021 19:54:46 -0300 Subject: [PATCH 4/5] Add unit tests to container_name Signed-off-by: Ulysses Souza --- local/compose/convergence_test.go | 55 +++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 local/compose/convergence_test.go diff --git a/local/compose/convergence_test.go b/local/compose/convergence_test.go new file mode 100644 index 00000000..3a5009a0 --- /dev/null +++ b/local/compose/convergence_test.go @@ -0,0 +1,55 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package compose + +import ( + "fmt" + "testing" + + "github.com/compose-spec/compose-go/types" + "gotest.tools/assert" +) + +func TestContainerName(t *testing.T) { + var replicas uint64 = 1 + s := types.ServiceConfig{ + Name: "testservicename", + ContainerName: "testcontainername", + Scale: 1, + Deploy: &types.DeployConfig{}, + } + ret, err := getScale(s) + assert.NilError(t, err) + assert.Equal(t, ret, s.Scale) + + s.Scale = 0 + s.Deploy.Replicas = &replicas + ret, err = getScale(s) + assert.NilError(t, err) + assert.Equal(t, ret, int(*s.Deploy.Replicas)) + + s.Deploy.Replicas = nil + s.Scale = 2 + _, err = getScale(s) + assert.Error(t, err, fmt.Sprintf(doubledContainerNameWarning, s.Name, s.ContainerName)) + + replicas = 2 + s.Deploy.Replicas = &replicas + s.Scale = 0 + _, err = getScale(s) + assert.Error(t, err, fmt.Sprintf(doubledContainerNameWarning, s.Name, s.ContainerName)) +} From 7a6712afdb402c6b9e8621e66535e556a9419a55 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 8 Jan 2021 10:59:58 +0100 Subject: [PATCH 5/5] Do not inspect for double container name at creation time, this has already been checked, let moby error bubble up if there is a name clash at this stage Signed-off-by: Guillaume Tardif --- local/compose/convergence.go | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/local/compose/convergence.go b/local/compose/convergence.go index eb58e620..0402cc5f 100644 --- a/local/compose/convergence.go +++ b/local/compose/convergence.go @@ -26,7 +26,6 @@ import ( moby "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/network" - "github.com/docker/docker/client" "golang.org/x/sync/errgroup" status "github.com/docker/compose-cli/local/moby" @@ -42,11 +41,6 @@ const ( "Remove the custom name to scale the service.\n" ) -func containerExists(ctx context.Context, c *client.Client, name string) bool { - container, err := c.ContainerInspect(ctx, name) - return err == nil && container.ContainerJSONBase != nil && container.Name == "/"+name -} - func (s *composeService) ensureScale(ctx context.Context, actual []moby.Container, scale int, project *types.Project, service types.ServiceConfig) (*errgroup.Group, []moby.Container, error) { eg, _ := errgroup.WithContext(ctx) if len(actual) < scale { @@ -81,16 +75,7 @@ func (s *composeService) ensureScale(ctx context.Context, actual []moby.Containe } func (s *composeService) ensureService(ctx context.Context, observedState Containers, project *types.Project, service types.ServiceConfig) error { - actual, err := s.apiClient.ContainerList(ctx, moby.ContainerListOptions{ - Filters: filters.NewArgs( - projectFilter(project.Name), - serviceFilter(service.Name), - ), - All: true, - }) - if err != nil { - return err - } + actual := observedState.filter(isService(service.Name)) scale, err := getScale(service) if err != nil { @@ -201,11 +186,6 @@ func getScale(config types.ServiceConfig) (int, error) { } func (s *composeService) createContainer(ctx context.Context, project *types.Project, service types.ServiceConfig, name string, number int, autoRemove bool) error { - if containerExists(ctx, s.apiClient, name) { - return fmt.Errorf(doubledContainerNameWarning, - service.Name, - name) - } w := progress.ContextWriter(ctx) w.Event(progress.CreatingEvent(name)) err := s.createMobyContainer(ctx, project, service, name, number, nil, autoRemove)