Support the comma as default string param separator (kubeflow#921)

* support the default string param separator comma * support the default string param separator comma
red-hat-data-services · Apr 22, 2022 · 892bbaa · 892bbaa
1 parent c7dfca0
commit 892bbaa
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 13 deletions.
diff --git a/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun.go b/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun.go
@@ -85,6 +85,8 @@ const (
 	DefaultNestedStackDepth = 30
 
 	MaxNestedStackDepthKey = "maxNestedStackDepth"
+
+	defaultIterationParamStrSeparator = ","
 )
 
 // Reconciler implements controller.Reconciler for Configuration resources.
@@ -827,20 +829,29 @@ func computeIterations(run *v1alpha1.Run, tls *pipelineloopv1alpha1.PipelineLoop
 				iterationElements = append(iterationElements, v)
 			}
 		} else {
-			var strings []string
+			var stringArr []string
 			var ints []int
 			var dictsString []map[string]string
 			var dictsInt []map[string]int
-			errString := json.Unmarshal([]byte(iterationParamStr), &strings)
+			errString := json.Unmarshal([]byte(iterationParamStr), &stringArr)
 			errInt := json.Unmarshal([]byte(iterationParamStr), &ints)
 			errDictString := json.Unmarshal([]byte(iterationParamStr), &dictsString)
 			errDictInt := json.Unmarshal([]byte(iterationParamStr), &dictsInt)
 			if errString != nil && errInt != nil && errDictString != nil && errDictInt != nil {
-				return 0, iterationElements, fmt.Errorf("The value of the iterate parameter %q can not transfer to array", tls.IterateParam)
+				//try the default separator comma (,) in last
+				if strings.Contains(iterationParamStr, defaultIterationParamStrSeparator) {
+					stringArr := strings.Split(iterationParamStr, defaultIterationParamStrSeparator)
+					numberOfIterations = len(stringArr)
+					for _, v := range stringArr {
+						iterationElements = append(iterationElements, v)
+					}
+				} else {
+					return 0, iterationElements, fmt.Errorf("the value of the iterate parameter %q can not transfer to array", tls.IterateParam)
+				}
 			}
 			if errString == nil {
-				numberOfIterations = len(strings)
-				for _, v := range strings {
+				numberOfIterations = len(stringArr)
+				for _, v := range stringArr {
 					iterationElements = append(iterationElements, v)
 				}
 			} else if errInt == nil {
@@ -910,7 +921,7 @@ func getParameters(run *v1alpha1.Run, tls *pipelineloopv1alpha1.PipelineLoopSpec
 			}
 		}
 		if iterationParam != nil {
-			if iterationParamStrSeparator != nil {
+			if iterationParamStrSeparator != nil && iterationParamStrSeparator.Value.StringVal != "" {
 				iterationParamStr := strings.Trim(iterationParam.Value.StringVal, " ")
 				stringArr := strings.Split(iterationParamStr, iterationParamStrSeparator.Value.StringVal)
 				out = append(out, v1beta1.Param{
@@ -919,18 +930,19 @@ func getParameters(run *v1alpha1.Run, tls *pipelineloopv1alpha1.PipelineLoopSpec
 				})
 
 			} else {
-				var strings []string
+				var stringArr []string
 				var ints []int
 				var dictsString []map[string]string
 				var dictsInt []map[string]int
-				errString := json.Unmarshal([]byte(iterationParam.Value.StringVal), &strings)
-				errInt := json.Unmarshal([]byte(iterationParam.Value.StringVal), &ints)
-				errDictString := json.Unmarshal([]byte(iterationParam.Value.StringVal), &dictsString)
-				errDictInt := json.Unmarshal([]byte(iterationParam.Value.StringVal), &dictsInt)
+				iterationParamStr := iterationParam.Value.StringVal
+				errString := json.Unmarshal([]byte(iterationParamStr), &stringArr)
+				errInt := json.Unmarshal([]byte(iterationParamStr), &ints)
+				errDictString := json.Unmarshal([]byte(iterationParamStr), &dictsString)
+				errDictInt := json.Unmarshal([]byte(iterationParamStr), &dictsInt)
 				if errString == nil {
 					out = append(out, v1beta1.Param{
 						Name:  iterationParam.Name,
-						Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: strings[iteration-1]},
+						Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: stringArr[iteration-1]},
 					})
 				} else if errInt == nil {
 					out = append(out, v1beta1.Param{
@@ -951,6 +963,15 @@ func getParameters(run *v1alpha1.Run, tls *pipelineloopv1alpha1.PipelineLoopSpec
 							Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: strconv.Itoa(dictsInt[iteration-1][dictParam])},
 						})
 					}
+				} else {
+					//try the default separator ","
+					if strings.Contains(iterationParamStr, defaultIterationParamStrSeparator) {
+						stringArr := strings.Split(iterationParamStr, defaultIterationParamStrSeparator)
+						out = append(out, v1beta1.Param{
+							Name:  iterationParam.Name,
+							Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: stringArr[iteration-1]},
+						})
+					}
 				}
 			}
 		}

diff --git a/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun_test.go b/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun_test.go
@@ -855,6 +855,37 @@ var runPipelineLoopWithSpaceSeparatorParams = &v1alpha1.Run{
 	},
 }
 
+var runPipelineLoopWithDefaultSeparatorParams = &v1alpha1.Run{
+	ObjectMeta: metav1.ObjectMeta{
+		Name:      "run-pipelineloop",
+		Namespace: "foo",
+		Labels: map[string]string{
+			"myTestLabel":                    "myTestLabelValue",
+			"custom.tekton.dev/pipelineLoop": "a-pipelineloop",
+			"tekton.dev/pipeline":            "pr-loop-example",
+			"tekton.dev/pipelineRun":         "pr-loop-example",
+			"tekton.dev/pipelineTask":        "loop-task",
+		},
+		Annotations: map[string]string{
+			"myTestAnnotation": "myTestAnnotationValue",
+		},
+	},
+	Spec: v1alpha1.RunSpec{
+		Params: []v1beta1.Param{{
+			Name:  "current-item",
+			Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: "item1,item2"},
+		}, {
+			Name:  "additional-parameter",
+			Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: "stuff"},
+		}},
+		Ref: &v1alpha1.TaskRef{
+			APIVersion: pipelineloopv1alpha1.SchemeGroupVersion.String(),
+			Kind:       pipelineloop.PipelineLoopControllerName,
+			Name:       "a-pipelineloop",
+		},
+	},
+}
+
 func specifyLoopRange(from, to, step string, r *v1alpha1.Run) *v1alpha1.Run {
 	t := r.DeepCopy()
 	for n, i := range r.Spec.Params {
@@ -1874,6 +1905,16 @@ func TestReconcilePipelineLoopRun(t *testing.T) {
 		expectedReason:       pipelineloopv1alpha1.PipelineLoopRunReasonRunning,
 		expectedPipelineruns: []*v1beta1.PipelineRun{expectedPipelineRunWithPodTemplateAndSA},
 		expectedEvents:       []string{"Normal Started", "Normal Running Iterations completed: 0"},
+	}, {
+		name:                 "Reconcile a new run with a pipelineloop and a string params without separator",
+		pipeline:             aPipeline,
+		pipelineloop:         aPipelineLoop,
+		run:                  runPipelineLoopWithDefaultSeparatorParams,
+		pipelineruns:         []*v1beta1.PipelineRun{},
+		expectedStatus:       corev1.ConditionUnknown,
+		expectedReason:       pipelineloopv1alpha1.PipelineLoopRunReasonRunning,
+		expectedPipelineruns: []*v1beta1.PipelineRun{expectedPipelineRunIteration1},
+		expectedEvents:       []string{"Normal Started", "Normal Running Iterations completed: 0"},
 	},
 	}
 
@@ -2036,9 +2077,10 @@ func TestReconcilePipelineLoopRunFailures(t *testing.T) {
 		reason:       pipelineloopv1alpha1.PipelineLoopRunReasonFailedValidation,
 		wantEvents: []string{
 			"Normal Started ",
-			`Warning Failed Cannot determine number of iterations: The value of the iterate parameter "current-item" can not transfer to array`,
+			`Warning Failed Cannot determine number of iterations: the value of the iterate parameter "current-item" can not transfer to array`,
 		},
 	}}
+	testcases = testcases[len(testcases)-1:]
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
 			ctx := context.Background()