Move The 'wait' Argument Handling Logic To Its Exec Function (#1630)

* Move the 'wait' argument handling logic into its exec function This ensures that 'wait' has more control over the rendering of the conditions arguments, which are made up of a mixed of Go template actions and jsonpath variables. The handling of all other arguments remain unchanged. Signed-off-by: Ivan Sim <ivan.sim@kasten.io> * Fix unit test Signed-off-by: Ivan Sim <ivan.sim@kasten.io> * Fix unit test Signed-off-by: Ivan Sim <ivan.sim@kasten.io> * Fix another unit test issue Signed-off-by: Ivan Sim <ivan.sim@kasten.io> * Switch to go template syntax for Wait conditions (#1635) * Switch to go template syntax for Wait conditions Signed-off-by: Prasad Ghangal <prasad.ghangal@gmail.com> * Update K8ssandra BP as per the new Wait func syntax Signed-off-by: Prasad Ghangal <prasad.ghangal@gmail.com> Signed-off-by: Prasad Ghangal <prasad.ghangal@gmail.com> * Revert "Switch to go template syntax for Wait conditions (#1635)" (#1833) This reverts commit 2be38c8. Signed-off-by: Ivan Sim <ivan.sim@kasten.io> Signed-off-by: Prasad Ghangal <prasad.ghangal@gmail.com> Co-authored-by: Prasad Ghangal <prasad.ghangal@gmail.com>
kanisterio · Jan 9, 2023 · 15157fb · 15157fb
1 parent 1eac779
commit 15157fb
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 28 deletions.
diff --git a/pkg/function/kubeops_test.go b/pkg/function/kubeops_test.go
@@ -276,7 +276,9 @@ func (s *KubeOpsSuite) TestKubeOpsCreateDeleteWithCoreResource(c *C) {
 func (s *KubeOpsSuite) TestKubeOpsCreateWaitDelete(c *C) {
 	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
 	defer cancel()
-	tp := param.TemplateParams{}
+	tp := param.TemplateParams{
+		Time: time.Now().String(),
+	}
 	action := "test"
 	gvr := schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"}
 	deployName := "test-deployment"

diff --git a/pkg/function/wait.go b/pkg/function/wait.go
@@ -69,15 +69,23 @@ func (*waitFunc) Name() string {
 }
 
 func (ktf *waitFunc) Exec(ctx context.Context, tp param.TemplateParams, args map[string]interface{}) (map[string]interface{}, error) {
+	rendered, err := param.RenderArgs(args, tp)
+	if err != nil {
+		return nil, err
+	}
+
 	var timeout string
-	var conditions WaitConditions
-	var err error
-	if err = Arg(args, WaitTimeoutArg, &timeout); err != nil {
+	if err := Arg(rendered, WaitTimeoutArg, &timeout); err != nil {
 		return nil, err
 	}
-	if err = Arg(args, WaitConditionsArg, &conditions); err != nil {
+
+	// get the 'conditions' from the unrendered arguments list.
+	// they will be evaluated in the 'evaluateCondition()` function.
+	var conditions WaitConditions
+	if err := Arg(args, WaitConditionsArg, &conditions); err != nil {
 		return nil, err
 	}
+
 	dynCli, err := kube.NewDynamicClient()
 	if err != nil {
 		return nil, err
@@ -86,7 +94,7 @@ func (ktf *waitFunc) Exec(ctx context.Context, tp param.TemplateParams, args map
 	if err != nil {
 		return nil, errors.Wrap(err, "Failed to parse timeout")
 	}
-	err = waitForCondition(ctx, dynCli, conditions, timeoutDur)
+	err = waitForCondition(ctx, dynCli, conditions, timeoutDur, tp)
 	return nil, err
 }
 
@@ -105,14 +113,14 @@ func (*waitFunc) Arguments() []string {
 }
 
 // waitForCondition wait till the condition satisfies within the timeout duration
-func waitForCondition(ctx context.Context, dynCli dynamic.Interface, waitCond WaitConditions, timeout time.Duration) error {
+func waitForCondition(ctx context.Context, dynCli dynamic.Interface, waitCond WaitConditions, timeout time.Duration, tp param.TemplateParams) error {
 	ctxTimeout, cancel := context.WithTimeout(ctx, timeout)
 	defer cancel()
 	var evalErr error
 	result := false
 	err := poll.Wait(ctxTimeout, func(ctx context.Context) (bool, error) {
 		for _, cond := range waitCond.AnyOf {
-			result, evalErr = evaluateCondition(ctx, dynCli, cond)
+			result, evalErr = evaluateCondition(ctx, dynCli, cond, tp)
 			if evalErr != nil {
 				// TODO: Fail early if the error is due to jsonpath syntax
 				log.Debug().WithError(evalErr).Print("Failed to evaluate the condition", field.M{"result": result})
@@ -123,7 +131,7 @@ func waitForCondition(ctx context.Context, dynCli dynamic.Interface, waitCond Wa
 			}
 		}
 		for _, cond := range waitCond.AllOf {
-			result, evalErr = evaluateCondition(ctx, dynCli, cond)
+			result, evalErr = evaluateCondition(ctx, dynCli, cond, tp)
 			if evalErr != nil {
 				// TODO: Fail early if the error is due to jsonpath syntax
 				log.Debug().WithError(evalErr).Print("Failed to evaluate the condition", field.M{"result": result})
@@ -144,8 +152,17 @@ func waitForCondition(ctx context.Context, dynCli dynamic.Interface, waitCond Wa
 }
 
 // evaluateCondition evaluate the go template condition
-func evaluateCondition(ctx context.Context, dynCli dynamic.Interface, cond Condition) (bool, error) {
-	obj, err := fetchObjectFromRef(ctx, dynCli, cond.ObjectReference)
+func evaluateCondition(ctx context.Context, dynCli dynamic.Interface, cond Condition, tp param.TemplateParams) (bool, error) {
+	objRefRaw := map[string]crv1alpha1.ObjectReference{
+		"objRef": cond.ObjectReference,
+	}
+	rendered, err := param.RenderObjectRefs(objRefRaw, tp)
+	if err != nil {
+		return false, err
+	}
+	objRef := rendered["objRef"]
+
+	obj, err := fetchObjectFromRef(ctx, dynCli, objRef)
 	if err != nil {
 		return false, err
 	}

diff --git a/pkg/function/wait_test.go b/pkg/function/wait_test.go
@@ -16,6 +16,7 @@ package function
 
 import (
 	"context"
+	"time"
 
 	. "gopkg.in/check.v1"
 	v1 "k8s.io/api/core/v1"
@@ -134,13 +135,13 @@ func waitDeployPhase(namespace, deploy string) crv1alpha1.BlueprintPhase {
 			WaitConditionsArg: map[string]interface{}{
 				"anyOf": []interface{}{
 					map[string]interface{}{
-						"condition": `{{ if and (eq {$.spec.replicas} {$.status.availableReplicas} )
-									(and (eq "{$.status.conditions[?(@.type == "Available")].type}" "Available")
-									(eq "{$.status.conditions[?(@.type == "Available")].status}" "True"))}}
-									true
-								{{ else }}
-									false
-								{{ end }}`,
+						"condition": `{{ if and (eq "{$.spec.replicas}" "{$.status.availableReplicas}" )
+							(eq "{$.status.conditions[?(@.type == 'Available')].type}" "Available")
+							(eq "{$.status.conditions[?(@.type == 'Available')].status}" "True") }}
+							true
+							{{ else }}
+							false
+							{{ end }}`,
 						"objectReference": map[string]interface{}{
 							"apiVersion": "v1",
 							"group":      "apps",
@@ -164,7 +165,7 @@ func waitStatefulSetPhase(namespace, sts string) crv1alpha1.BlueprintPhase {
 			WaitConditionsArg: map[string]interface{}{
 				"allOf": []interface{}{
 					map[string]interface{}{
-						"condition": `{{ if (eq {$.spec.replicas} {$.status.availableReplicas})}}
+						"condition": `{{ if (eq "{$.spec.replicas}" "{$.status.availableReplicas}" )}}
 									true
 								{{ else }}
 									false
@@ -178,7 +179,7 @@ func waitStatefulSetPhase(namespace, sts string) crv1alpha1.BlueprintPhase {
 						},
 					},
 					map[string]interface{}{
-						"condition": `{{ if (eq {$.spec.replicas} {$.status.readyReplicas})}}
+						"condition": `{{ if (eq "{$.spec.replicas}" "{$.status.readyReplicas}" )}}
 									true
 								{{ else }}
 									false
@@ -208,7 +209,9 @@ func newWaitBlueprint(phases ...crv1alpha1.BlueprintPhase) *crv1alpha1.Blueprint
 }
 
 func (s *WaitSuite) TestWait(c *C) {
-	tp := param.TemplateParams{}
+	tp := param.TemplateParams{
+		Time: time.Now().String(),
+	}
 	action := "test"
 	for _, tc := range []struct {
 		bp      *crv1alpha1.Blueprint

diff --git a/pkg/param/render.go b/pkg/param/render.go
@@ -25,7 +25,6 @@ import (
 	"github.com/pkg/errors"
 
 	crv1alpha1 "github.com/kanisterio/kanister/pkg/apis/cr/v1alpha1"
-	"github.com/kanisterio/kanister/pkg/jsonpath"
 )
 
 const (
@@ -120,11 +119,6 @@ func RenderArtifacts(arts map[string]crv1alpha1.Artifact, tp TemplateParams) (ma
 }
 
 func renderStringArg(arg string, tp TemplateParams) (string, error) {
-	// Skip render if contains jsonpath arg
-	matched := jsonpath.FindJsonpathArgs(arg)
-	if len(matched) != 0 {
-		return arg, nil
-	}
 	t, err := template.New("config").Option("missingkey=error").Funcs(sprig.TxtFuncMap()).Parse(arg)
 	if err != nil {
 		return "", errors.WithStack(err)

diff --git a/pkg/phase.go b/pkg/phase.go
@@ -16,6 +16,7 @@ package kanister
 
 import (
 	"context"
+	"strings"
 
 	"github.com/Masterminds/semver"
 	"github.com/pkg/errors"
@@ -65,10 +66,12 @@ func (p *Phase) Exec(ctx context.Context, bp crv1alpha1.Blueprint, action string
 			if ap.Name != p.name {
 				continue
 			}
-			args, err := param.RenderArgs(ap.Args, tp)
+
+			args, err := renderFuncArgs(ap.Func, ap.Args, tp)
 			if err != nil {
 				return nil, err
 			}
+
 			if err = checkRequiredArgs(p.f.RequiredArgs(), args); err != nil {
 				return nil, errors.Wrapf(err, "Required args missing for function %s", p.f.Name())
 			}
@@ -84,6 +87,18 @@ func (p *Phase) Exec(ctx context.Context, bp crv1alpha1.Blueprint, action string
 	return p.f.Exec(ctx, tp, p.args)
 }
 
+func renderFuncArgs(
+	funcName string,
+	args map[string]interface{},
+	tp param.TemplateParams) (map[string]interface{}, error) {
+	// let wait handle its own go template + jsonpath mixed arguments
+	if strings.ToLower(funcName) == "wait" {
+		return args, nil
+	}
+
+	return param.RenderArgs(args, tp)
+}
+
 func checkSupportedArgs(supportedArgs []string, args map[string]interface{}) error {
 	for a := range args {
 		if !slices.Contains(supportedArgs, a) {