Set ActionSetStatus to Failed when RenderArtifacts fails (#4171)

* Set ActionSetStatus to Failed when rendering fails

* Minor log improvement

pkg/controller/controller.go

@@ -358,13 +358,13 @@ func (c *Controller) runAction(ctx context.Context, as *crv1alpha1.ActionSet, aI
 		for i, p := range phases {
 			c.logAndSuccessEvent(fmt.Sprintf("Executing phase %s", p.Name()), "Started Phase", as)
 			err = param.InitPhaseParams(ctx, c.clientset, tp, p.Name(), p.Objects())
-			if err != nil {
-				reason := fmt.Sprintf("ActionSetFailed Action: %s", as.Spec.Actions[aIDX].Name)
-				msg := fmt.Sprintf("Failed to init phase params: %#v:", as.Status.Actions[aIDX].Phases[i])
-				c.logAndErrorEvent(msg, reason, err, as, bp)
-				return
+			var output map[string]interface{}
+			var msg string
+			if err == nil {
+				output, err = p.Exec(ctx, *bp, action.Name, *tp)
+			} else {
+				msg = fmt.Sprintf("Failed to init phase params: %#v:", as.Status.Actions[aIDX].Phases[i])
 			}
-			output, err := p.Exec(ctx, *bp, action.Name, *tp)
 			var rf func(*crv1alpha1.ActionSet) error
 			if err != nil {
 				rf = func(ras *crv1alpha1.ActionSet) error {
@@ -387,7 +387,9 @@ func (c *Controller) runAction(ctx context.Context, as *crv1alpha1.ActionSet, aI
 			}
 			if err != nil {
 				reason := fmt.Sprintf("ActionSetFailed Action: %s", as.Spec.Actions[aIDX].Name)
-				msg := fmt.Sprintf("Failed to execute phase: %#v:", as.Status.Actions[aIDX].Phases[i])
+				if msg == "" {
+					msg = fmt.Sprintf("Failed to execute phase: %#v:", as.Status.Actions[aIDX].Phases[i])
+				}
 				c.logAndErrorEvent(msg, reason, err, as, bp)
 				return
 			}
@@ -410,24 +412,32 @@ func (c *Controller) runAction(ctx context.Context, as *crv1alpha1.ActionSet, aI
 		}
 		// Render the artifacts
 		arts, err := param.RenderArtifacts(artTpls, *tp)
+		var af func(*crv1alpha1.ActionSet) error
 		if err != nil {
-			reason := fmt.Sprintf("ActionSetFailed Action: %s", action.Name)
-			msg := fmt.Sprintf("Failed to render Output Artifacts: %#v:", artTpls)
-			c.logAndErrorEvent(msg, reason, err, as, bp)
-			return
-		}
-		af := func(ras *crv1alpha1.ActionSet) error {
-			ras.Status.Actions[aIDX].Artifacts = arts
-			ras.Status.State = crv1alpha1.StateComplete
-			return nil
+			af = func(ras *crv1alpha1.ActionSet) error {
+				ras.Status.State = crv1alpha1.StateFailed
+				return nil
+			}
+		} else {
+			af = func(ras *crv1alpha1.ActionSet) error {
+				ras.Status.Actions[aIDX].Artifacts = arts
+				ras.Status.State = crv1alpha1.StateComplete
+				return nil
+			}
 		}
-		// Update ActionSet with artifacts
+		// Update ActionSet
 		if aErr := reconcile.ActionSet(ctx, c.crClient.CrV1alpha1(), ns, name, af); aErr != nil {
 			reason := fmt.Sprintf("ActionSetFailed Action: %s", action.Name)
 			msg := fmt.Sprintf("Failed to update Output Artifacts: %#v:", artTpls)
 			c.logAndErrorEvent(msg, reason, aErr, as, bp)
 			return
 		}
+		if err != nil {
+			reason := fmt.Sprintf("ActionSetFailed Action: %s", action.Name)
+			msg := "Failed to render output artifacts"
+			c.logAndErrorEvent(msg, reason, err, as, bp)
+			return
+		}
 	}()
 	return nil
 }

pkg/controller/controller_test.go

@@ -172,6 +172,32 @@ func newBPWithOutputArtifact() *crv1alpha1.Blueprint {
 	}
 }
 
+func newBPWithFakeOutputArtifact() *crv1alpha1.Blueprint {
+	return &crv1alpha1.Blueprint{
+		ObjectMeta: metav1.ObjectMeta{
+			GenerateName: "test-blueprint-",
+		},
+		Actions: map[string]*crv1alpha1.BlueprintAction{
+			"myAction": &crv1alpha1.BlueprintAction{
+				OutputArtifacts: map[string]crv1alpha1.Artifact{
+					"myArt": crv1alpha1.Artifact{
+						KeyValue: map[string]string{
+							"key": "{{ .Phases.myPhase0.Output.myKey }}",
+						},
+					},
+				},
+				Kind: "Deployment",
+				Phases: []crv1alpha1.BlueprintPhase{
+					{
+						Name: "myPhase0",
+						Func: testutil.OutputFuncName,
+					},
+				},
+			},
+		},
+	}
+}
+
 func (s *ControllerSuite) TestEmptyActionSetStatus(c *C) {
 	as := &crv1alpha1.ActionSet{
 		ObjectMeta: metav1.ObjectMeta{
@@ -395,3 +421,24 @@ func (s *ControllerSuite) TestPhaseOutputAsArtifact(c *C) {
 	keyVal := arts["myArt"].KeyValue
 	c.Assert(keyVal, DeepEquals, map[string]string{"key": "myValue"})
 }
+
+func (s *ControllerSuite) TestRenderArtifactsFailure(c *C) {
+	bp := newBPWithFakeOutputArtifact()
+	bp = testutil.BlueprintWithConfigMap(bp)
+	bp, err := s.crCli.Blueprints(s.namespace).Create(bp)
+	c.Assert(err, IsNil)
+
+	// Add an actionset that references that blueprint.
+	as := testutil.NewTestActionSet(s.namespace, bp.GetName(), "Deployment", s.deployment.GetName(), s.namespace)
+	as = testutil.ActionSetWithConfigMap(as, s.confimap.GetName())
+	as, err = s.crCli.ActionSets(s.namespace).Create(as)
+	c.Assert(err, IsNil)
+
+	err = s.waitOnActionSetState(c, as, crv1alpha1.StateRunning)
+	c.Assert(err, IsNil)
+
+	c.Assert(testutil.OutputFuncOut(), DeepEquals, map[string]interface{}{"key": "myValue"})
+
+	err = s.waitOnActionSetState(c, as, crv1alpha1.StateFailed)
+	c.Assert(err, IsNil)
+}

pkg/validate/validate.go

@@ -91,9 +91,6 @@ func actionSetStatus(as *crv1alpha1.ActionSetStatus) error {
 			return errorf("ActionSet cannot be complete if any actions are not complete")
 		}
 	}
-	if saw[crv1alpha1.StateFailed] != (as.State == crv1alpha1.StateFailed) {
-		return errorf("Iff any action is failed, the whole ActionSet must be failed")
-	}
 	return nil
 }
 

pkg/validate/validate_test.go

@@ -421,7 +421,7 @@ func (s *ValidateSuite) TestActionSetStatus(c *C) {
 					},
 				},
 			},
-			checker: NotNil,
+			checker: IsNil,
 		},
 		{
 			as: &crv1alpha1.ActionSetStatus{