Browse Source

cp command: copy to all containers of a service as default behaviour

Signed-off-by: Guillaume Lours <[email protected]>
Guillaume Lours 3 years ago
parent
commit
a983cf551d
5 changed files with 20 additions and 26 deletions
  1. 4 2
      cmd/compose/cp.go
  2. 1 2
      docs/reference/compose_cp.md
  3. 4 4
      docs/reference/docker_compose_cp.yaml
  4. 5 1
      pkg/compose/cp.go
  5. 6 17
      pkg/e2e/cp_test.go

+ 4 - 2
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)")
 

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

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

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

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