cli/app/cp.go

@@ -2,20 +2,16 @@ package app
 
 import (
 	"context"
-	"errors"
 	"fmt"
-	"log"
 	"os"
-	"path"
 	"strings"
 
 	"coopcloud.tech/abra/cli/internal"
 	"coopcloud.tech/abra/pkg/autocomplete"
 	"coopcloud.tech/abra/pkg/client"
-	containerPkg "coopcloud.tech/abra/pkg/container"
+	"coopcloud.tech/abra/pkg/config"
+	"coopcloud.tech/abra/pkg/container"
 	"coopcloud.tech/abra/pkg/formatter"
-	"coopcloud.tech/abra/pkg/upstream/container"
-	"github.com/docker/cli/cli/command"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
 	dockerClient "github.com/docker/docker/client"
@@ -57,14 +53,36 @@ And if you want to copy that file back to your current working directory locally
 			logrus.Fatal("missing <dest> argument")
 		}
 
-		srcPath, dstPath, service, isToContainer, err := parseSrcAndDst(src, dst)
-		if err != nil {
-			log.Fatal(err)
+		parsedSrc := strings.SplitN(src, ":", 2)
+		parsedDst := strings.SplitN(dst, ":", 2)
+		errorMsg := "one of <src>/<dest> arguments must take $SERVICE:$PATH form"
+		if len(parsedSrc) == 2 && len(parsedDst) == 2 {
+			logrus.Fatal(errorMsg)
+		} else if len(parsedSrc) != 2 {
+			if len(parsedDst) != 2 {
+				logrus.Fatal(errorMsg)
+			}
+		} else if len(parsedDst) != 2 {
+			if len(parsedSrc) != 2 {
+				logrus.Fatal(errorMsg)
+			}
 		}
-		if isToContainer {
-			logrus.Debugf("assuming transfer is going TO the container")
-		} else {
+
+		var service string
+		var srcPath string
+		var dstPath string
+		isToContainer := false // <container:src> <dst>
+		if len(parsedSrc) == 2 {
+			service = parsedSrc[0]
+			srcPath = parsedSrc[1]
+			dstPath = dst
 			logrus.Debugf("assuming transfer is coming FROM the container")
+		} else if len(parsedDst) == 2 {
+			service = parsedDst[0]
+			dstPath = parsedDst[1]
+			srcPath = src
+			isToContainer = true // <src> <container:dst>
+			logrus.Debugf("assuming transfer is going TO the container")
 		}
 
 		if isToContainer {
@@ -78,18 +96,7 @@ And if you want to copy that file back to your current working directory locally
 			logrus.Fatal(err)
 		}
 
-		container, err := findContainer(cl, app.StackName(), service)
-		if err != nil {
-			logrus.Fatal(err)
-		}
-		logrus.Debugf("retrieved %s as target container on %s", formatter.ShortenID(container.ID), app.Server)
-
-		if isToContainer {
-			err = copyToContainer(cl, container.ID, srcPath, dstPath)
-		} else {
-			err = copyFromContainer(cl, container.ID, srcPath, dstPath)
-		}
-		if err != nil {
+		if err := configureAndCp(c, cl, app, srcPath, dstPath, service, isToContainer); err != nil {
 			logrus.Fatal(err)
 		}
 
@@ -97,88 +104,46 @@ And if you want to copy that file back to your current working directory locally
 	},
 }
 
-var errServiceMissing = errors.New("one of <src>/<dest> arguments must take $SERVICE:$PATH form")
-
-// parseSrcAndDst parses src and dest string. One of src or dst must be of the form $SERVICE:$PATH
-func parseSrcAndDst(src, dst string) (srcPath string, dstPath string, service string, toContainer bool, err error) {
-	parsedSrc := strings.SplitN(src, ":", 2)
-	parsedDst := strings.SplitN(dst, ":", 2)
-	if len(parsedSrc)+len(parsedDst) != 3 {
-		return "", "", "", false, errServiceMissing
-	}
-	if len(parsedSrc) == 2 {
-		return parsedSrc[1], dst, parsedSrc[0], false, nil
-	}
-	if len(parsedDst) == 2 {
-		return src, parsedDst[1], parsedDst[0], true, nil
-	}
-	return "", "", "", false, errServiceMissing
-}
-
-func findContainer(cl *dockerClient.Client, stack, service string) (types.Container, error) {
+func configureAndCp(
+	c *cli.Context,
+	cl *dockerClient.Client,
+	app config.App,
+	srcPath string,
+	dstPath string,
+	service string,
+	isToContainer bool) error {
 	filters := filters.NewArgs()
-	filters.Add("name", fmt.Sprintf("^%s_%s", stack, service))
+	filters.Add("name", fmt.Sprintf("^%s_%s", app.StackName(), service))
 
-	container, err := containerPkg.GetContainer(context.Background(), cl, filters, internal.NoInput)
+	container, err := container.GetContainer(context.Background(), cl, filters, internal.NoInput)
 	if err != nil {
-		return types.Container{}, err
-	}
-	return container, nil
-}
-
-// copyToContainer works with one of the following:
-//   <src_dir>/<file> -> <dst_dir> = <dst_dir>/<file>
-//   <src_dir>/<file> -> <dst_dir>/<file> = <dst_dir>/<file> (overrides the file)
-//   <src_dir> -> <dst_dir> = <dst_dir>/<contents_of_src_dir> (creates the dst_dir if it does not exist)
-func copyToContainer(cl *dockerClient.Client, containerID, srcPath, dstPath string) error {
-	toTarOpts := &archive.TarOptions{IncludeSourceDir: true, NoOverwriteDirNonDir: true, Compression: archive.Gzip}
-	content, err := archive.TarWithOptions(srcPath, toTarOpts)
-	if err != nil {
-		return err
+		logrus.Fatal(err)
 	}
 
-	srcStat, _ := os.Stat(srcPath)
-	stat, _ := cl.ContainerStatPath(context.Background(), containerID, dstPath)
-	if !stat.Mode.IsDir() {
-		if srcStat.IsDir() {
-			// Make sure the dst directory exits, when copying a directory.
-			dcli, err := command.NewDockerCli()
-			if err != nil {
-				return err
-			}
-			if err := container.RunExec(dcli, cl, containerID, &types.ExecConfig{
-				AttachStderr: true,
-				AttachStdin:  true,
-				AttachStdout: true,
-				Cmd:          []string{"mkdir", "-p", dstPath},
-				Detach:       false,
-				Tty:          true,
-			}); err != nil {
-				return fmt.Errorf("create remote directory: %s", err)
-			}
-		} else {
-			// Remove the file component from the path, since docker can only copy to a directoy.
-			dstPath, _ = path.Split(dstPath)
+	logrus.Debugf("retrieved %s as target container on %s", formatter.ShortenID(container.ID), app.Server)
+
+	if isToContainer {
+		toTarOpts := &archive.TarOptions{NoOverwriteDirNonDir: true, Compression: archive.Gzip}
+		content, err := archive.TarWithOptions(srcPath, toTarOpts)
+		if err != nil {
+			logrus.Fatal(err)
+		}
+
+		copyOpts := types.CopyToContainerOptions{AllowOverwriteDirWithFile: false, CopyUIDGID: false}
+		if err := cl.CopyToContainer(context.Background(), container.ID, dstPath, content, copyOpts); err != nil {
+			logrus.Fatal(err)
+		}
+	} else {
+		content, _, err := cl.CopyFromContainer(context.Background(), container.ID, srcPath)
+		if err != nil {
+			logrus.Fatal(err)
+		}
+		defer content.Close()
+		fromTarOpts := &archive.TarOptions{NoOverwriteDirNonDir: true, Compression: archive.Gzip}
+		if err := archive.Untar(content, dstPath, fromTarOpts); err != nil {
+			logrus.Fatal(err)
 		}
 	}
 
-	copyOpts := types.CopyToContainerOptions{AllowOverwriteDirWithFile: false, CopyUIDGID: false}
-	if err := cl.CopyToContainer(context.Background(), containerID, dstPath, content, copyOpts); err != nil {
-		return err
-	}
-
-	return nil
-}
-
-func copyFromContainer(cl *dockerClient.Client, containerID, srcPath, dstPath string) error {
-	content, _, err := cl.CopyFromContainer(context.Background(), containerID, srcPath)
-	if err != nil {
-		return err
-	}
-	defer content.Close()
-	fromTarOpts := &archive.TarOptions{NoOverwriteDirNonDir: true, Compression: archive.Gzip}
-	if err := archive.Untar(content, dstPath, fromTarOpts); err != nil {
-		return err
-	}
 	return nil
 }

cli/app/cp_test.go

@@ -1,39 +0,0 @@
-package app
-
-import "testing"
-
-func TestParse(t *testing.T) {
-	tests := []struct {
-		src         string
-		dst         string
-		srcPath     string
-		dstPath     string
-		service     string
-		toContainer bool
-		err         error
-	}{
-		{src: "foo", dst: "bar", err: errServiceMissing},
-		{src: "app:foo", dst: "app:bar", err: errServiceMissing},
-		{src: "app:foo", dst: "bar", srcPath: "foo", dstPath: "bar", service: "app", toContainer: false},
-		{src: "foo", dst: "app:bar", srcPath: "foo", dstPath: "bar", service: "app", toContainer: true},
-	}
-
-	for i, tc := range tests {
-		srcPath, dstPath, service, toContainer, err := parseSrcAndDst(tc.src, tc.dst)
-		if srcPath != tc.srcPath {
-			t.Errorf("[%d] srcPath: want (%s), got(%s)", i, tc.srcPath, srcPath)
-		}
-		if dstPath != tc.dstPath {
-			t.Errorf("[%d] dstPath: want (%s), got(%s)", i, tc.dstPath, dstPath)
-		}
-		if service != tc.service {
-			t.Errorf("[%d] service: want (%s), got(%s)", i, tc.service, service)
-		}
-		if toContainer != tc.toContainer {
-			t.Errorf("[%d] toConainer: want (%t), got(%t)", i, tc.toContainer, toContainer)
-		}
-		if err == nil && tc.err != nil && err.Error() != tc.err.Error() {
-			t.Errorf("[%d] err: want (%s), got(%s)", i, tc.err, err)
-		}
-	}
-}

tests/integration/app_cp.bats

@@ -76,7 +76,7 @@ teardown(){
 @test "error if service doesn't exist" {
   _deploy_app
 
-  run bash -c "echo foo > $BATS_TMPDIR/myfile.txt"
+  run bash -c "echo foo >> $BATS_TMPDIR/myfile.txt"
   assert_success
 
   run $ABRA app cp "$TEST_APP_DOMAIN" "$BATS_TMPDIR/myfile.txt" doesnt_exist:/
@@ -90,19 +90,15 @@ teardown(){
 }
 
 # bats test_tags=slow
-@test "copy local file to container directory" {
+@test "copy to container" {
   _deploy_app
 
-  run bash -c "echo foo > $BATS_TMPDIR/myfile.txt"
+  run bash -c "echo foo >> $BATS_TMPDIR/myfile.txt"
   assert_success
 
   run $ABRA app cp "$TEST_APP_DOMAIN" "$BATS_TMPDIR/myfile.txt" app:/etc
   assert_success
 
-  run $ABRA app run "$TEST_APP_DOMAIN" app cat /etc/myfile.txt
-  assert_success
-  assert_output --partial "foo"
-
   run rm -rf "$BATS_TMPDIR/myfile.txt"
   assert_success
 
@@ -110,61 +106,7 @@ teardown(){
 }
 
 # bats test_tags=slow
-@test "copy local file to container file (override on remote)" {
-  _deploy_app
-
-  run bash -c "echo 'foo' > $BATS_TMPDIR/myfile.txt"
-  assert_success
-
-  run $ABRA app cp "$TEST_APP_DOMAIN" "$BATS_TMPDIR/myfile.txt" app:/etc
-  assert_success
-
-  run $ABRA app run "$TEST_APP_DOMAIN" app cat /etc/myfile.txt
-  assert_success
-  assert_output --partial "foo"
-
-  run bash -c "echo 'bar' > $BATS_TMPDIR/myfile.txt"
-  assert_success
-
-  run $ABRA app cp "$TEST_APP_DOMAIN" "$BATS_TMPDIR/myfile.txt" app:/etc/myfile.txt
-  assert_success
-
-  run $ABRA app run "$TEST_APP_DOMAIN" app cat /etc/myfile.txt
-  assert_success
-  assert_output --partial "bar"
-
-  run rm -rf "$BATS_TMPDIR/myfile.txt"
-  assert_success
-
-  _undeploy_app
-}
-
-# bats test_tags=slow
-@test "copy local directory to container directory (and creates missing directory)" {
-  _deploy_app
-
-  run bash -c "mkdir -p $BATS_TMPDIR/mydir"
-  assert_success
-
-  run bash -c "echo 'foo' > $BATS_TMPDIR/mydir/myfile.txt"
-  assert_success
-
-  run $ABRA app cp "$TEST_APP_DOMAIN" "$BATS_TMPDIR/mydir" app:/etc/mydir
-  assert_success
-
-  run $ABRA app run "$TEST_APP_DOMAIN" app ls /etc/mydir
-  assert_success
-  assert_output --partial "myfile.txt"
-
-
-  run rm -rf "$BATS_TMPDIR/mydir"
-  assert_success
-
-  _undeploy_app
-}
-
-# bats test_tags=slow
-@test "copy container file to local directory" {
+@test "copy from container" {
   _deploy_app
 
   run bash -c "echo foo >> $BATS_TMPDIR/myfile.txt"