From a983cf551d3045fdfd7d3116a2aa3c44585840a4 Mon Sep 17 00:00:00 2001 From: Guillaume Lours Date: Tue, 3 May 2022 21:16:19 +0200 Subject: [PATCH] cp command: copy to all containers of a service as default behaviour Signed-off-by: Guillaume Lours --- cmd/compose/cp.go | 6 ++++-- docs/reference/compose_cp.md | 3 +-- docs/reference/docker_compose_cp.yaml | 8 ++++---- pkg/compose/cp.go | 6 +++++- pkg/e2e/cp_test.go | 23 ++++++----------------- 5 files changed, 20 insertions(+), 26 deletions(-) diff --git a/cmd/compose/cp.go b/cmd/compose/cp.go index e38b9de42..f0919ca76 100644 --- a/cmd/compose/cp.go +++ b/cmd/compose/cp.go @@ -55,7 +55,7 @@ func copyCommand(p *projectOptions, backend api.Service) *cobra.Command { } return nil }), - RunE: Adapt(func(ctx context.Context, args []string) error { + RunE: AdaptCmd(func(ctx context.Context, cmd *cobra.Command, args []string) error { opts.source = args[0] opts.destination = args[1] return runCopy(ctx, backend, opts) @@ -64,8 +64,10 @@ func copyCommand(p *projectOptions, backend api.Service) *cobra.Command { } flags := copyCmd.Flags() - flags.IntVar(&opts.index, "index", 1, "Index of the container if there are multiple instances of a service [default: 1].") + flags.IntVar(&opts.index, "index", 0, "Index of the container if there are multiple instances of a service [default: 1 when copying from a container].") flags.BoolVar(&opts.all, "all", false, "Copy to all the containers of the service.") + flags.MarkHidden("all") //nolint:errcheck + flags.MarkDeprecated("all", "By default all the containers of the service will get the source file/directory to be copied.") //nolint:errcheck flags.BoolVarP(&opts.followLink, "follow-link", "L", false, "Always follow symbol link in SRC_PATH") flags.BoolVarP(&opts.copyUIDGID, "archive", "a", false, "Archive mode (copy all uid/gid information)") diff --git a/docs/reference/compose_cp.md b/docs/reference/compose_cp.md index 2d97b9f2f..f9bc21434 100644 --- a/docs/reference/compose_cp.md +++ b/docs/reference/compose_cp.md @@ -7,10 +7,9 @@ Copy files/folders between a service container and the local filesystem | Name | Type | Default | Description | | --- | --- | --- | --- | -| `--all` | | | Copy to all the containers of the service. | | `-a`, `--archive` | | | Archive mode (copy all uid/gid information) | | `-L`, `--follow-link` | | | Always follow symbol link in SRC_PATH | -| `--index` | `int` | `1` | Index of the container if there are multiple instances of a service [default: 1]. | +| `--index` | `int` | `0` | Index of the container if there are multiple instances of a service [default: 1 when copying from a container]. | diff --git a/docs/reference/docker_compose_cp.yaml b/docs/reference/docker_compose_cp.yaml index 461f9a5dc..fd3ef6427 100644 --- a/docs/reference/docker_compose_cp.yaml +++ b/docs/reference/docker_compose_cp.yaml @@ -10,8 +10,8 @@ options: value_type: bool default_value: "false" description: Copy to all the containers of the service. - deprecated: false - hidden: false + deprecated: true + hidden: true experimental: false experimentalcli: false kubernetes: false @@ -40,9 +40,9 @@ options: swarm: false - option: index value_type: int - default_value: "1" + default_value: "0" description: | - Index of the container if there are multiple instances of a service [default: 1]. + Index of the container if there are multiple instances of a service [default: 1 when copying from a container]. deprecated: false hidden: false experimental: false diff --git a/pkg/compose/cp.go b/pkg/compose/cp.go index b43a98f8b..fe8f82fe3 100644 --- a/pkg/compose/cp.go +++ b/pkg/compose/cp.go @@ -57,6 +57,10 @@ func (s *composeService) Copy(ctx context.Context, projectName string, options a if options.All { return errors.New("cannot use the --all flag when copying from a service") } + // due to remove of the --all flag, restore the default value to 1 when copying from a service container to the host. + if options.Index == 0 { + options.Index = 1 + } } if destService != "" { direction |= toService @@ -72,7 +76,7 @@ func (s *composeService) Copy(ctx context.Context, projectName string, options a return fmt.Errorf("no container found for service %q", serviceName) } - if !options.All { + if direction == fromService || (direction == toService && options.Index > 0) { containers = containers.filter(indexed(options.Index)) } diff --git a/pkg/e2e/cp_test.go b/pkg/e2e/cp_test.go index 661e5bee1..0aa14b3a2 100644 --- a/pkg/e2e/cp_test.go +++ b/pkg/e2e/cp_test.go @@ -47,15 +47,18 @@ func TestCopy(t *testing.T) { res.Assert(t, icmd.Expected{Out: `nginx running`}) }) - t.Run("copy to container copies the file to the first container by default", func(t *testing.T) { + t.Run("copy to container copies the file to the all containers by default", func(t *testing.T) { res := c.RunDockerComposeCmd("-f", "./fixtures/cp-test/compose.yaml", "-p", projectName, "cp", "./fixtures/cp-test/cp-me.txt", "nginx:/tmp/default.txt") res.Assert(t, icmd.Expected{ExitCode: 0}) output := c.RunDockerCmd("exec", projectName+"-nginx-1", "cat", "/tmp/default.txt").Stdout() assert.Assert(t, strings.Contains(output, `hello world`), output) - res = c.RunDockerOrExitError("exec", projectName+"_nginx_2", "cat", "/tmp/default.txt") - res.Assert(t, icmd.Expected{ExitCode: 1}) + output = c.RunDockerCmd("exec", projectName+"-nginx-2", "cat", "/tmp/default.txt").Stdout() + assert.Assert(t, strings.Contains(output, `hello world`), output) + + output = c.RunDockerCmd("exec", projectName+"-nginx-3", "cat", "/tmp/default.txt").Stdout() + assert.Assert(t, strings.Contains(output, `hello world`), output) }) t.Run("copy to container with a given index copies the file to the given container", func(t *testing.T) { @@ -69,20 +72,6 @@ func TestCopy(t *testing.T) { res.Assert(t, icmd.Expected{ExitCode: 1}) }) - t.Run("copy to container with the all flag copies the file to all containers", func(t *testing.T) { - res := c.RunDockerComposeCmd("-f", "./fixtures/cp-test/compose.yaml", "-p", projectName, "cp", "--all", "./fixtures/cp-test/cp-me.txt", "nginx:/tmp/all.txt") - res.Assert(t, icmd.Expected{ExitCode: 0}) - - output := c.RunDockerCmd("exec", projectName+"-nginx-1", "cat", "/tmp/all.txt").Stdout() - assert.Assert(t, strings.Contains(output, `hello world`), output) - - output = c.RunDockerCmd("exec", projectName+"-nginx-2", "cat", "/tmp/all.txt").Stdout() - assert.Assert(t, strings.Contains(output, `hello world`), output) - - output = c.RunDockerCmd("exec", projectName+"-nginx-3", "cat", "/tmp/all.txt").Stdout() - assert.Assert(t, strings.Contains(output, `hello world`), output) - }) - t.Run("copy from a container copies the file to the host from the first container by default", func(t *testing.T) { res := c.RunDockerComposeCmd("-f", "./fixtures/cp-test/compose.yaml", "-p", projectName, "cp", "nginx:/tmp/default.txt", "./fixtures/cp-test/from-default.txt") res.Assert(t, icmd.Expected{ExitCode: 0})