refactor the parent/dep uninstall scenarios

Signed-off-by: Vaughn Dice <vadice@microsoft.com>

pkg/porter/dependencies.go

@@ -256,29 +256,24 @@ func (e *dependencyExecutioner) executeDependency(dep *queuedDependency, parentA
 	if err != nil {
 		executeErrs = multierror.Append(executeErrs, errors.Wrapf(err, "error executing dependency %s", dep.Alias))
 
-		if depArgs.Action == claim.ActionUninstall {
-			if e.parentOpts.Delete && !e.parentOpts.ForceDelete {
-				executeErrs = multierror.Append(executeErrs, ErrUnsafeInstallationDeleteRetryForceDelete)
-			}
-		}
-
-		if !e.parentOpts.ForceDelete {
+		// Handle errors when/if the action is uninstall
+		executeErrs = e.parentOpts.handleUninstallErrs(e.Out, executeErrs)
+		if executeErrs != nil {
 			return executeErrs
 		}
-		// else, we swallow executeErrs as opts.ForceDelete is true
 	}
 
-	if e.parentOpts.Delete || e.parentOpts.ForceDelete {
+	if e.parentOpts.shouldDelete() {
 		fmt.Fprintf(e.Out, installationDeleteTmpl, depArgs.Installation)
 		return e.Claims.DeleteInstallation(depArgs.Installation)
-	} else {
-		// Collect expected outputs via claim
-		outputs, err := e.Claims.ReadLastOutputs(depArgs.Installation)
-		if err != nil {
-			return err
-		}
-		dep.outputs = outputs
 	}
 
+	// Collect expected outputs via claim
+	outputs, err := e.Claims.ReadLastOutputs(depArgs.Installation)
+	if err != nil {
+		return err
+	}
+	dep.outputs = outputs
+
 	return nil
 }

pkg/porter/uninstall.go

@@ -2,6 +2,7 @@ package porter
 
 import (
 	"fmt"
+	"io"
 	"strings"
 
 	"github.com/cnabio/cnab-go/claim"
@@ -24,6 +25,30 @@ type UninstallDeleteOptions struct {
 	ForceDelete bool
 }
 
+func (opts *UninstallDeleteOptions) shouldDelete() bool {
+	return opts.Delete || opts.ForceDelete
+}
+
+func (opts *UninstallDeleteOptions) unsafeDelete() bool {
+	return opts.Delete && !opts.ForceDelete
+}
+
+func (opts *UninstallDeleteOptions) handleUninstallErrs(out io.Writer, err error) error {
+	if err == nil {
+		return nil
+	}
+
+	if opts.unsafeDelete() {
+		return multierror.Append(err, ErrUnsafeInstallationDeleteRetryForceDelete)
+	}
+
+	if opts.ForceDelete {
+		fmt.Fprintf(out, "ignoring the following errors as --force-delete is true:\n  %s", err.Error())
+		return nil
+	}
+	return err
+}
+
 // UninstallBundle accepts a set of pre-validated UninstallOptions and uses
 // them to uninstall a bundle.
 func (p *Porter) UninstallBundle(opts UninstallOptions) error {
@@ -48,35 +73,30 @@ func (p *Porter) UninstallBundle(opts UninstallOptions) error {
 	err = p.CNAB.Execute(opts.ToActionArgs(deperator))
 	if err != nil {
 		uninstallErrs = multierror.Append(uninstallErrs, err)
-		// If the installation is not found, bail out now and return this error; no further action needed
+
+		// If the installation is not found, no further action is needed
 		if strings.Contains(err.Error(), claim.ErrInstallationNotFound.Error()) {
 			return uninstallErrs
 		}
 
-		if len(deperator.deps) > 0 {
+		if len(deperator.deps) > 0 && !opts.ForceDelete {
 			uninstallErrs = multierror.Append(uninstallErrs,
 				fmt.Errorf("failed to uninstall the %s bundle, the remaining dependencies were not uninstalled", opts.Name))
 		}
 
-		if opts.Delete && !opts.ForceDelete {
-			uninstallErrs = multierror.Append(uninstallErrs, ErrUnsafeInstallationDeleteRetryForceDelete)
-		}
-		if !opts.ForceDelete {
+		uninstallErrs = opts.handleUninstallErrs(p.Out, uninstallErrs)
+		if uninstallErrs != nil {
 			return uninstallErrs
 		}
-		// else, we swallow uninstallErrs as opts.ForceDelete is true
-		// and we wish to pass same option down to deps, if applicable
 	}
 
 	// TODO: See https://github.com/deislabs/porter/issues/465 for flag to allow keeping around the dependencies
-	err = deperator.Execute()
+	err = opts.handleUninstallErrs(p.Out, deperator.Execute())
 	if err != nil {
-		if !opts.ForceDelete {
-			return err
-		}
+		return err
 	}
 
-	if opts.Delete || opts.ForceDelete {
+	if opts.shouldDelete() {
 		fmt.Fprintf(p.Out, installationDeleteTmpl, opts.Name)
 		return p.Claims.DeleteInstallation(opts.Name)
 	}

pkg/porter/uninstall_test.go

@@ -0,0 +1,55 @@
+package porter
+
+import (
+	"bytes"
+	"fmt"
+	"testing"
+
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPorter_HandleUninstallErrs(t *testing.T) {
+	testcases := []struct {
+		name    string
+		opts    UninstallDeleteOptions
+		err     error
+		wantOut string
+		wantErr string
+	}{
+		{
+			name: "no delete; no err",
+			opts: UninstallDeleteOptions{},
+		}, {
+			name:    "no delete",
+			opts:    UninstallDeleteOptions{},
+			err:     errors.New("an error was encountered"),
+			wantErr: "an error was encountered",
+		}, {
+			name:    "--delete; no --force-delete",
+			opts:    UninstallDeleteOptions{Delete: true},
+			err:     errors.New("an error was encountered"),
+			wantErr: fmt.Sprintf("2 errors occurred:\n\t* an error was encountered\n\t* %s\n\n", ErrUnsafeInstallationDeleteRetryForceDelete),
+		}, {
+			name:    "--force-delete",
+			opts:    UninstallDeleteOptions{ForceDelete: true},
+			err:     errors.New("an error was encountered"),
+			wantOut: "ignoring the following errors as --force-delete is true:\n  an error was encountered",
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			out := bytes.NewBufferString("")
+
+			err := tc.opts.handleUninstallErrs(out, tc.err)
+			if tc.wantErr != "" {
+				require.EqualError(t, err, tc.wantErr)
+			} else {
+				require.NoError(t, err)
+			}
+
+			require.Equal(t, tc.wantOut, out.String())
+		})
+	}
+}