Delete partially fetched packages on failure. (#2410)

* Fetch pkg into tmp dir before moving on success.

* Delete directory on failed fetches.

* Fix typos in comment.

* Move cleaning up dir to helper.

* Hardcode op.

Author: etefera

internal/util/get/get.go

@@ -86,20 +86,20 @@ func (c Command) Run(ctx context.Context) error {
 
 	err = kptfileutil.WriteFile(c.Destination, kf)
 	if err != nil {
-		return errors.E(op, types.UniquePath(c.Destination), err)
+		return cleanUpDirAndError(c.Destination, err)
 	}
 
 	p, err := pkg.New(c.Destination)
 	if err != nil {
-		return errors.E(op, types.UniquePath(c.Destination), err)
+		return cleanUpDirAndError(c.Destination, err)
 	}
 
 	if err = c.fetchPackages(ctx, p); err != nil {
-		return errors.E(op, types.UniquePath(c.Destination), err)
+		return cleanUpDirAndError(c.Destination, err)
 	}
 
 	if err := addmergecomment.Process(c.Destination); err != nil {
-		return errors.E(op, types.UniquePath(c.Destination), err)
+		return cleanUpDirAndError(c.Destination, err)
 	}
 	return nil
 }
@@ -183,3 +183,12 @@ func (c *Command) DefaultValues() error {
 
 	return nil
 }
+
+func cleanUpDirAndError(destination string, err error) error {
+	const op errors.Op = "get.Run"
+	rmErr := os.RemoveAll(destination)
+	if rmErr != nil {
+		return errors.E(op, types.UniquePath(destination), err, rmErr)
+	}
+	return errors.E(op, types.UniquePath(destination), err)
+}

internal/util/get/get_test.go

@@ -739,6 +739,12 @@ func TestCommand_Run_failInvalidRepo(t *testing.T) {
 	if !assert.Contains(t, err.Error(), "'foo' does not appear to be a git repository") {
 		t.FailNow()
 	}
+
+	// Confirm destination directory no longer exists.
+	_, err = os.Stat(absPath)
+	if !assert.Error(t, err) {
+		t.FailNow()
+	}
 }
 
 func TestCommand_Run_failInvalidBranch(t *testing.T) {
@@ -766,6 +772,12 @@ func TestCommand_Run_failInvalidBranch(t *testing.T) {
 	if !assert.Contains(t, err.Error(), "exit status 128") {
 		t.FailNow()
 	}
+
+	// Confirm destination directory no longer exists.
+	_, err = os.Stat(absPath)
+	if !assert.Error(t, err) {
+		t.FailNow()
+	}
 }
 
 func TestCommand_Run_failInvalidTag(t *testing.T) {
@@ -775,13 +787,14 @@ func TestCommand_Run_failInvalidTag(t *testing.T) {
 	})
 	defer clean()
 
+	absPath := filepath.Join(w.WorkspaceDirectory, g.RepoDirectory)
 	err := Command{
 		Git: &kptfilev1.Git{
 			Repo:      g.RepoDirectory,
 			Directory: "/",
 			Ref:       "refs/tags/foo",
 		},
-		Destination: filepath.Join(w.WorkspaceDirectory, g.RepoDirectory),
+		Destination: absPath,
 	}.Run(fake.CtxWithDefaultPrinter())
 	if !assert.Error(t, err) {
 		t.FailNow()
@@ -792,6 +805,12 @@ func TestCommand_Run_failInvalidTag(t *testing.T) {
 	if !assert.Contains(t, err.Error(), "exit status 128") {
 		t.FailNow()
 	}
+
+	// Confirm destination directory no longer exists.
+	_, err = os.Stat(absPath)
+	if !assert.Error(t, err) {
+		t.FailNow()
+	}
 }
 
 func TestCommand_Run_subpackages(t *testing.T) {