Always collect outputs even when bundle fails

pkg/runtime/runtime.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"path/filepath"
 
+	"github.com/hashicorp/go-multierror"
+
 	"get.porter.sh/porter/pkg/config"
 	"get.porter.sh/porter/pkg/context"
 	"get.porter.sh/porter/pkg/manifest"
@@ -66,61 +68,70 @@ func (r *PorterRuntime) Execute(rm *RuntimeManifest) error {
 		return errors.Wrapf(err, "could not create outputs directory %s", context.MixinOutputsDir)
 	}
 
+	var executionErr error
 	for _, step := range r.RuntimeManifest.GetSteps() {
-		if step != nil {
-			err := r.RuntimeManifest.ResolveStep(step)
-			if err != nil {
-				return errors.Wrap(err, "unable to resolve step")
-			}
+		executionErr = r.executeStep(step)
+		if executionErr != nil {
+			break
+		}
+	}
 
-			description, _ := step.GetDescription()
-			if len(description) > 0 {
-				fmt.Fprintln(r.Out, description)
-			}
+	err = r.applyUnboundBundleOutputs()
+	if err != nil {
+		// Log but allow the bundle to gracefully exit
+		fmt.Fprintln(r.Err, err)
+	}
 
-			// Hand over values needing masking in context output streams
-			r.Context.SetSensitiveValues(r.RuntimeManifest.GetSensitiveValues())
+	if executionErr == nil {
+		fmt.Fprintln(r.Out, "execution completed successfully!")
+	}
+	return executionErr // Report the success of the bundle back up the chain
+}
 
-			input := &ActionInput{
-				action: r.RuntimeManifest.Action,
-				Steps:  []*manifest.Step{step},
-			}
-			inputBytes, _ := yaml.Marshal(input)
-			cmd := pkgmgmt.CommandOptions{
-				Command: string(r.RuntimeManifest.Action),
-				Input:   string(inputBytes),
-				Runtime: true,
-			}
-			err = r.mixins.Run(r.Context, step.GetMixinName(), cmd)
-			if err != nil {
-				return errors.Wrap(err, "mixin execution failed")
-			}
+func (r *PorterRuntime) executeStep(step *manifest.Step) error {
+	if step == nil {
+		return nil
+	}
+	err := r.RuntimeManifest.ResolveStep(step)
+	if err != nil {
+		return errors.Wrap(err, "unable to resolve step")
+	}
 
-			outputs, err := r.readMixinOutputs()
-			if err != nil {
-				return errors.Wrap(err, "could not read step outputs")
-			}
+	description, _ := step.GetDescription()
+	if len(description) > 0 {
+		fmt.Fprintln(r.Out, description)
+	}
 
-			err = r.RuntimeManifest.ApplyStepOutputs(outputs)
-			if err != nil {
-				return err
-			}
+	// Hand over values needing masking in context output streams
+	r.Context.SetSensitiveValues(r.RuntimeManifest.GetSensitiveValues())
 
-			// Apply any Bundle Outputs declared in this step
-			err = r.applyStepOutputsToBundle(outputs)
-			if err != nil {
-				return err
-			}
-		}
+	input := &ActionInput{
+		action: r.RuntimeManifest.Action,
+		Steps:  []*manifest.Step{step},
+	}
+	inputBytes, _ := yaml.Marshal(input)
+	cmd := pkgmgmt.CommandOptions{
+		Command: string(r.RuntimeManifest.Action),
+		Input:   string(inputBytes),
+		Runtime: true,
+	}
+	err = r.mixins.Run(r.Context, step.GetMixinName(), cmd)
+	if err != nil {
+		return errors.Wrap(err, "mixin execution failed")
 	}
 
-	err = r.applyUnboundBundleOutputs()
+	outputs, err := r.readMixinOutputs()
+	if err != nil {
+		return errors.Wrap(err, "could not read step outputs")
+	}
+
+	err = r.RuntimeManifest.ApplyStepOutputs(outputs)
 	if err != nil {
 		return err
 	}
 
-	fmt.Fprintln(r.Out, "execution completed successfully!")
-	return nil
+	// Apply any Bundle Outputs declared in this step
+	return r.applyStepOutputsToBundle(outputs)
 }
 
 func (r *PorterRuntime) createOutputsDir() error {
@@ -166,6 +177,11 @@ func (r *PorterRuntime) applyUnboundBundleOutputs() error {
 		return err
 	}
 
+	if len(r.RuntimeManifest.Outputs) > 0 {
+		fmt.Fprintln(r.Out, "Collecting bundle outputs...")
+	}
+
+	var bigErr *multierror.Error
 	outputs := r.RuntimeManifest.GetOutputs()
 	for _, outputDef := range r.RuntimeManifest.Outputs {
 		// Ignore outputs that have already been set
@@ -182,12 +198,13 @@ func (r *PorterRuntime) applyUnboundBundleOutputs() error {
 			outpath := filepath.Join(config.BundleOutputsDir, outputDef.Name)
 			err = r.CopyFile(outputDef.Path, outpath)
 			if err != nil {
-				return errors.Wrapf(err, "unable to copy output file from %s to %s", outputDef.Path, outpath)
+				err = multierror.Append(bigErr, errors.Wrapf(err, "unable to copy output file from %s to %s", outputDef.Path, outpath))
+				continue
 			}
 		}
 	}
 
-	return nil
+	return bigErr.ErrorOrNil()
 }
 
 func (r *PorterRuntime) shouldApplyOutput(output manifest.OutputDefinition) bool {

tests/smoke/hello_test.go

@@ -101,4 +101,11 @@ func TestHelloBundle(t *testing.T) {
 
 	_, err = test.RunPorter("invoke", "--action=status", "missing", "--reference", myBunsRef)
 	test.RequireNotFoundReturned(err)
+
+	// Test that outputs are collected when a bundle fails
+	err = test.Porter("install", "fail-with-outputs", "--reference", myBunsRef, "-c=mybuns", "--param", "chaos_monkey=true").Run()
+	require.Error(t, err, "the chaos monkey should have failed the installation")
+	magicOutput, err := test.Porter("installation", "outputs", "show", "magic_file", "-i=fail-with-outputs").Output()
+	require.NoError(t, err, "the output should have been saved even though the bundle failed")
+	require.Contains(t, magicOutput, "is a unicorn")
 }

tests/testdata/mybuns/helpers.sh

@@ -30,5 +30,13 @@ uninstall() {
   fi
 }
 
+chaos_monkey() {
+  if [[ "$1" == "true" ]]; then
+    echo "a chaos monkey appears. you have died"
+    exit 1
+  fi
+
+}
+
 # Call the requested function and pass the arguments as-is
 "$@"

tests/testdata/mybuns/porter.yaml

@@ -18,6 +18,10 @@ parameters:
     minimum: 1
     maximum: 11
     default: 5
+  - name: chaos_monkey
+    description: "Set to true to make the bundle fail"
+    type: boolean
+    default: false
   - name: magic_file
     description: "Pay no attention to the generated magic file"
     type: file
@@ -61,12 +65,18 @@ install:
       command: ./helpers.sh
       arguments:
         - makeMagic
-        - "{{ bundle.credentials.username }} is a unicorn"
+        - "'{{ bundle.credentials.username }} is a unicorn'"
   - exec:
       description: "install"
       command: ./helpers.sh
       arguments:
         - install
+  - exec:
+      description: "roll the dice with your chaos monkey"
+      command: ./helpers.sh
+      arguments:
+        - chaos_monkey
+        - "{{ bundle.parameters.chaos_monkey }}"
 
 dry-run:
   - exec:
@@ -100,6 +110,12 @@ upgrade:
       command: ./helpers.sh
       arguments:
         - upgrade
+  - exec:
+      description: "roll the dice with your chaos monkey"
+      command: ./helpers.sh
+      arguments:
+        - chaos_monkey
+        - "{{ bundle.parameters.chaos_monkey }}"
 
 uninstall:
   - exec:
@@ -112,3 +128,9 @@ uninstall:
       command: ./helpers.sh
       arguments:
         - uninstall
+  - exec:
+      description: "roll the dice with your chaos monkey"
+      command: ./helpers.sh
+      arguments:
+        - chaos_monkey
+        - "{{ bundle.parameters.chaos_monkey }}"