Render Artifacts after execution of all phases (#4125)

* WIP Render ArtifactsOut after all the phases

* Render arts inside go routine

* Remove ArtifactsOut from TemplateParams

* Add Unit test to check artifacts update

* Fix ActionSet update

* Restructure actionset update

* Minor log update

pkg/controller/controller.go

@@ -205,6 +205,8 @@ func (c *Controller) onUpdateActionSet(oldAS, newAS *crv1alpha1.ActionSet) error
 	if newAS.Status == nil || newAS.Status.State != crv1alpha1.StateRunning {
 		if newAS.Status == nil {
 			log.Infof("Updated ActionSet '%s' Status->nil", newAS.Name)
+		} else if newAS.Status.State == crv1alpha1.StateComplete {
+			c.logAndSuccessEvent(fmt.Sprintf("Updated ActionSet '%s' Status->%s", newAS.Name, newAS.Status.State), "Update Complete", newAS)
 		} else {
 			log.Infof("Updated ActionSet '%s' Status->%s", newAS.Name, newAS.Status.State)
 		}
@@ -346,12 +348,6 @@ func (c *Controller) runAction(ctx context.Context, as *crv1alpha1.ActionSet, aI
 	if err != nil {
 		return err
 	}
-	artTpls := as.Status.Actions[aIDX].Artifacts
-	arts, err := param.RenderArtifacts(artTpls, *tp)
-	if err != nil {
-		return err
-	}
-	tp.ArtifactsOut = arts
 	phases, err := kanister.GetPhases(*bp, action.Name, *tp)
 	if err != nil {
 		return err
@@ -363,7 +359,7 @@ func (c *Controller) runAction(ctx context.Context, as *crv1alpha1.ActionSet, aI
 			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 execute phase: %#v:", as.Status.Actions[aIDX].Phases[i])
+				msg := fmt.Sprintf("Failed to init phase params: %#v:", as.Status.Actions[aIDX].Phases[i])
 				c.logAndErrorEvent(msg, reason, err, as, bp)
 				return
 			}
@@ -377,7 +373,6 @@ func (c *Controller) runAction(ctx context.Context, as *crv1alpha1.ActionSet, aI
 				}
 			} else {
 				rf = func(ras *crv1alpha1.ActionSet) error {
-					ras.Status.Actions[aIDX].Artifacts = arts
 					ras.Status.Actions[aIDX].Phases[i].State = crv1alpha1.StateComplete
 					ras.Status.Actions[aIDX].Phases[i].Output = output
 					return nil
@@ -398,6 +393,49 @@ func (c *Controller) runAction(ctx context.Context, as *crv1alpha1.ActionSet, aI
 			param.UpdatePhaseParams(ctx, tp, p.Name(), output)
 			c.logAndSuccessEvent(fmt.Sprintf("Completed phase %s", p.Name()), "Ended Phase", as)
 		}
+		// Check if output artifacts are present
+		artTpls := as.Status.Actions[aIDX].Artifacts
+		if len(artTpls) == 0 {
+			// No artifacts, set ActionSetStatus to complete
+			if rErr := reconcile.ActionSet(ctx, c.crClient.CrV1alpha1(), ns, name, func(ras *crv1alpha1.ActionSet) error {
+				ras.Status.State = crv1alpha1.StateComplete
+				return nil
+			}); rErr != nil {
+				reason := fmt.Sprintf("ActionSetFailed Action: %s", action.Name)
+				msg := fmt.Sprintf("Failed to update ActionSet: %s", name)
+				c.logAndErrorEvent(msg, reason, err, as, bp)
+			}
+			return
+		}
+		// Render the artifacts
+		arts, err := param.RenderArtifacts(artTpls, *tp)
+		var af func(*crv1alpha1.ActionSet) error
+		if err != 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
+		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
+		}
+		// Failed to render artifacts
+		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
+		}
 	}()
 	return nil
 }

pkg/controller/controller_test.go

@@ -146,6 +146,32 @@ func (s *ControllerSuite) waitOnActionSetState(c *C, as *crv1alpha1.ActionSet, s
 	return errors.Wrapf(err, "State '%s' never reached", state)
 }
 
+func newBPWithOutputArtifact() *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.key }}",
+						},
+					},
+				},
+				Kind: "Deployment",
+				Phases: []crv1alpha1.BlueprintPhase{
+					{
+						Name: "myPhase0",
+						Func: testutil.OutputFuncName,
+					},
+				},
+			},
+		},
+	}
+}
+
 func (s *ControllerSuite) TestEmptyActionSetStatus(c *C) {
 	as := &crv1alpha1.ActionSet{
 		ObjectMeta: metav1.ObjectMeta{
@@ -338,3 +364,34 @@ func (s *ControllerSuite) TestRuntimeObjEventLogs(c *C) {
 	c.Assert(err, NotNil)
 	c.Assert(len(events.Items), Equals, 0)
 }
+
+func (s *ControllerSuite) TestPhaseOutputAsArtifact(c *C) {
+	// Create a blueprint that uses func output as artifact
+	bp := newBPWithOutputArtifact()
+	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)
+
+	// Check if the func returned expected output
+	c.Assert(testutil.OutputFuncOut(), DeepEquals, map[string]interface{}{"key": "myValue"})
+
+	err = s.waitOnActionSetState(c, as, crv1alpha1.StateComplete)
+	c.Assert(err, IsNil)
+
+	// Check if the artifacts got updated correctly
+	as, err = s.crCli.ActionSets(as.GetNamespace()).Get(as.GetName(), metav1.GetOptions{})
+	arts := as.Status.Actions[0].Artifacts
+	c.Assert(arts, NotNil)
+	c.Assert(arts, HasLen, 1)
+	keyVal := arts["myArt"].KeyValue
+	c.Assert(keyVal, DeepEquals, map[string]string{"key": "myValue"})
+}

pkg/param/param.go

@@ -20,19 +20,18 @@ const timeFormat = time.RFC3339Nano
 
 // TemplateParams are the values that will change between separate runs of Phases.
 type TemplateParams struct {
-	StatefulSet  *StatefulSetParams
-	Deployment   *DeploymentParams
-	PVC          *PVCParams
-	Namespace    *NamespaceParams
-	ArtifactsIn  map[string]crv1alpha1.Artifact
-	ArtifactsOut map[string]crv1alpha1.Artifact
-	ConfigMaps   map[string]v1.ConfigMap
-	Secrets      map[string]v1.Secret
-	Time         string
-	Profile      *Profile
-	Options      map[string]string
-	Object       map[string]interface{}
-	Phases       map[string]*Phase
+	StatefulSet *StatefulSetParams
+	Deployment  *DeploymentParams
+	PVC         *PVCParams
+	Namespace   *NamespaceParams
+	ArtifactsIn map[string]crv1alpha1.Artifact
+	ConfigMaps  map[string]v1.ConfigMap
+	Secrets     map[string]v1.Secret
+	Time        string
+	Profile     *Profile
+	Options     map[string]string
+	Object      map[string]interface{}
+	Phases      map[string]*Phase
 }
 
 // StatefulSetParams are params for stateful sets.

pkg/param/render_test.go

@@ -36,59 +36,43 @@ func (s *RenderSuite) TestRender(c *C) {
 			checker: IsNil,
 		},
 		{
-			arg: "{{ .ArtifactsOut.hello.KeyValue }}",
+			arg: "{{ .Options.hello }}",
 			tp: TemplateParams{
-				ArtifactsOut: map[string]crv1alpha1.Artifact{
-					"hello": crv1alpha1.Artifact{},
+				Options: map[string]string{
+					"hello": "",
 				},
 			},
-			out:     "map[]",
+			out:     "",
 			checker: IsNil,
 		},
 		{
-			arg: "{{ .ArtifactsOut.hello.KeyValue.someKey }}",
+			arg: "{{ .Options.hello }}",
 			tp: TemplateParams{
-				ArtifactsOut: map[string]crv1alpha1.Artifact{
-					"hello": crv1alpha1.Artifact{
-						KeyValue: map[string]string{
-							"someKey": "someValue",
-						},
-					},
+				Options: map[string]string{
+					"hello": "someValue",
 				},
 			},
 			out:     "someValue",
 			checker: IsNil,
 		},
 		{
 			// `-` cannot be used in a template path.
-			arg: "{{ .ArtifactsOut.hello-world.KeyValue.someKey }}",
+			arg: "{{ .Options.hello-world }}",
 			tp: TemplateParams{
-				ArtifactsOut: map[string]crv1alpha1.Artifact{
-					"hello-world": crv1alpha1.Artifact{
-						KeyValue: map[string]string{
-							"someKey": "someValue",
-						},
-					},
+				Options: map[string]string{
+					"hello-world": "someValue",
 				},
 			},
 			out:     "",
 			checker: NotNil,
 		},
 		{
 			// `-` can exist in artifact keys, it just cannot be used in path.
-			arg: "{{ .ArtifactsOut.hello.KeyValue.someKey }}",
+			arg: "{{ .Options.hello }}",
 			tp: TemplateParams{
-				ArtifactsOut: map[string]crv1alpha1.Artifact{
-					"hello": crv1alpha1.Artifact{
-						KeyValue: map[string]string{
-							"someKey": "someValue",
-						},
-					},
-					"hello-world": crv1alpha1.Artifact{
-						KeyValue: map[string]string{
-							"someKey": "someValue",
-						},
-					},
+				Options: map[string]string{
+					"hello":       "someValue",
+					"hello-world": "someValue",
 				},
 			},
 			out:     "someValue",
@@ -120,9 +104,9 @@ func (s *RenderSuite) TestRender(c *C) {
 		},
 		{
 			// Render should fail if referenced key doesn't exist
-			arg: "{{ .ArtifactsOut.hello.KeyValue.someKey }}",
+			arg: "{{ .Options.hello }}",
 			tp: TemplateParams{
-				ArtifactsOut: map[string]crv1alpha1.Artifact{},
+				Options: map[string]string{},
 			},
 			checker: NotNil,
 		},

pkg/phase_test.go

@@ -42,24 +42,20 @@ func (s *PhaseSuite) TestExec(c *C) {
 	}{
 		{
 			artifact: "hello",
-			argument: "{{ .ArtifactsOut.test.KeyValue.in }} world",
+			argument: "{{ .Options.test }} world",
 			expected: "hello world",
 		},
 		{
 			artifact: "HELLO",
-			argument: "{{ .ArtifactsOut.test.KeyValue.in | lower}} world",
+			argument: "{{ .Options.test | lower}} world",
 			expected: "hello world",
 		},
 	} {
 		var output string
 		tf := &testFunc{output: &output}
 		tp := param.TemplateParams{
-			ArtifactsOut: map[string]crv1alpha1.Artifact{
-				"test": crv1alpha1.Artifact{
-					KeyValue: map[string]string{
-						"in": tc.artifact,
-					},
-				},
+			Options: map[string]string{
+				"test": tc.artifact,
 			},
 		}
 		rawArgs := map[string]interface{}{

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
 }