fix(sdk): fix nested param logic for pl (#1091)

kubeflow · Nov 29, 2022 · ada23f2 · ada23f2
1 parent f99259d
commit ada23f2
Show file tree

Hide file tree

Showing 15 changed files with 103 additions and 69 deletions.
diff --git a/sdk/python/kfp_tekton/compiler/_tekton_handler.py b/sdk/python/kfp_tekton/compiler/_tekton_handler.py
@@ -442,14 +442,15 @@ def process_inline_cr_field(field_name):
                 for custom_param in custom_task_cr['spec']['pipelineSpec']['params']:
                     all_params.append(''.join(['$(params.', custom_param['name'], ')']))
                 for global_task_value in global_task_values:
-                    if global_task_value not in all_params:
+                    if global_task_value not in all_params and \
+                        re.findall('\$\(params.([^ \t\n.:,;\{\}]+)\)', global_task_value)[0] not in nested_loop_counter_params:
                         all_params.append(global_task_value)
                         custom_task_cr['spec']['pipelineSpec']['params'].append(
                             {'name': re.findall('\$\(params.([^ \t\n.:,;\{\}]+)\)', global_task_value)[0],
                              'type': 'string'}
                         )
                         for task in tasks:
-                            if task['name'] == nested_custom_task['father_ct'] and global_task_value not in global_task_values:
+                            if task['name'] == nested_custom_task['father_ct']:
                                 task['params'].append(
                                     {'name': re.findall('\$\(params.([^ \t\n.:,;\{\}]+)\)', global_task_value)[0],
                                      'value': global_task_value}

diff --git a/sdk/python/tests/compiler/testdata/loop_with_enumerate_withitem_multi_nested.yaml b/sdk/python/tests/compiler/testdata/loop_with_enumerate_withitem_multi_nested.yaml
@@ -50,8 +50,12 @@ spec:
       params:
       - name: loop-item-param-2
         value: '[{"a": 1, "b": 2}, {"a": 10, "b": 20}]'
+      - name: my_pipe_param
+        value: $(params.my_pipe_param)
       - name: my_pipe_param-loop-item
         value: $(params.my_pipe_param)
+      - name: my_pipe_param3
+        value: $(params.my_pipe_param3)
       - name: my_pipe_param3-loop-item
         value: $(params.my_pipe_param3)
       taskSpec:
@@ -140,6 +144,8 @@ spec:
                 value: $(params.loop-item-param-2-subvar-b)
               - name: my_pipe_param-loop-item
                 value: $(params.my_pipe_param-loop-item)
+              - name: my_pipe_param3
+                value: $(params.my_pipe_param3)
               - name: my_pipe_param3-loop-item
                 value: $(params.my_pipe_param3-loop-item)
               taskSpec:
@@ -199,6 +205,8 @@ spec:
                         value: $(params.loop-item-param-2-subvar-b)
                       - name: loop-item-param-7
                         value: '[4, 5]'
+                      - name: my_pipe_param3
+                        value: $(params.my_pipe_param3)
                       - name: my_pipe_param3-loop-item
                         value: $(params.my_pipe_param3-loop-item)
                       taskSpec:

diff --git a/sdk/python/tests/compiler/testdata/loop_with_enumerate_withitem_multi_nested_noninlined.yaml b/sdk/python/tests/compiler/testdata/loop_with_enumerate_withitem_multi_nested_noninlined.yaml
@@ -58,20 +58,20 @@ metadata:
       {"name": "loop-item-param-2-subvar-a", "value": "$(params.loop-item-param-2-subvar-a)"},
       {"name": "loop-item-param-2-subvar-b", "value": "$(params.loop-item-param-2-subvar-b)"},
       {"name": "my_pipe_param-loop-item", "value": "$(params.my_pipe_param-loop-item)"},
-      {"name": "my_pipe_param3-loop-item", "value": "$(params.my_pipe_param3-loop-item)"}],
-      "taskRef": {"apiVersion": "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop",
-      "name": "withitem-multiple-nesting-pipeline-for-loop-5"}}]}}}, {"apiVersion":
-      "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop", "metadata": {"name": "withitem-multiple-nesting-pipeline-for-loop-5"},
-      "spec": {"iterateParam": "my_pipe_param-loop-item", "pipelineSpec": {"params":
-      [{"name": "iteration-number-4", "type": "string"}, {"name": "loop-item-param-2-subvar-a",
-      "type": "string"}, {"name": "loop-item-param-2-subvar-b", "type": "string"},
-      {"name": "my_pipe_param-loop-item", "type": "string"}, {"name": "my_pipe_param3",
-      "type": "string"}, {"name": "my_pipe_param3-loop-item", "type": "string"}],
-      "tasks": [{"name": "my-1st-inner-coop", "params": [{"name": "loop-item-param-2-subvar-a",
-      "value": "$(params.loop-item-param-2-subvar-a)"}, {"name": "my_pipe_param-loop-item",
-      "value": "$(params.my_pipe_param-loop-item)"}], "taskSpec": {"metadata": {"annotations":
-      {"pipelines.kubeflow.org/component_spec_digest": "{\"name\": \"my-1st-inner-coop\",
-      \"outputs\": [], \"version\": \"my-1st-inner-coop@sha256=feaa8a218c3caaa76fa1154bcca9fc485286e72814986c35d08edcd28d2a50b9\"}"},
+      {"name": "my_pipe_param3", "value": "$(params.my_pipe_param3)"}, {"name": "my_pipe_param3-loop-item",
+      "value": "$(params.my_pipe_param3-loop-item)"}], "taskRef": {"apiVersion": "custom.tekton.dev/v1alpha1",
+      "kind": "PipelineLoop", "name": "withitem-multiple-nesting-pipeline-for-loop-5"}}]}}},
+      {"apiVersion": "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop", "metadata":
+      {"name": "withitem-multiple-nesting-pipeline-for-loop-5"}, "spec": {"iterateParam":
+      "my_pipe_param-loop-item", "pipelineSpec": {"params": [{"name": "iteration-number-4",
+      "type": "string"}, {"name": "loop-item-param-2-subvar-a", "type": "string"},
+      {"name": "loop-item-param-2-subvar-b", "type": "string"}, {"name": "my_pipe_param-loop-item",
+      "type": "string"}, {"name": "my_pipe_param3", "type": "string"}, {"name": "my_pipe_param3-loop-item",
+      "type": "string"}], "tasks": [{"name": "my-1st-inner-coop", "params": [{"name":
+      "loop-item-param-2-subvar-a", "value": "$(params.loop-item-param-2-subvar-a)"},
+      {"name": "my_pipe_param-loop-item", "value": "$(params.my_pipe_param-loop-item)"}],
+      "taskSpec": {"metadata": {"annotations": {"pipelines.kubeflow.org/component_spec_digest":
+      "{\"name\": \"my-1st-inner-coop\", \"outputs\": [], \"version\": \"my-1st-inner-coop@sha256=feaa8a218c3caaa76fa1154bcca9fc485286e72814986c35d08edcd28d2a50b9\"}"},
       "labels": {"pipelines.kubeflow.org/cache_enabled": "true"}}, "params": [{"name":
       "loop-item-param-2-subvar-a", "type": "string"}, {"name": "my_pipe_param-loop-item",
       "type": "string"}], "steps": [{"args": ["set -e\necho \"op1-1\" \"$0\" \"$1\"\n",
@@ -80,10 +80,11 @@ metadata:
       {"name": "withitem-multiple-nesting-pipeline-for-loop-8", "params": [{"name":
       "iteration-number-4", "value": "$(params.iteration-number-4)"}, {"name": "loop-item-param-2-subvar-b",
       "value": "$(params.loop-item-param-2-subvar-b)"}, {"name": "loop-item-param-7",
-      "value": "[4, 5]"}, {"name": "my_pipe_param3-loop-item", "value": "$(params.my_pipe_param3-loop-item)"}],
-      "taskRef": {"apiVersion": "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop",
-      "name": "withitem-multiple-nesting-pipeline-for-loop-8"}}]}}}, {"apiVersion":
-      "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop", "metadata": {"name": "withitem-multiple-nesting-pipeline-for-loop-8"},
+      "value": "[4, 5]"}, {"name": "my_pipe_param3-loop-item", "value": "$(params.my_pipe_param3-loop-item)"},
+      {"name": "my_pipe_param3", "value": "$(params.my_pipe_param3)"}], "taskRef":
+      {"apiVersion": "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop", "name":
+      "withitem-multiple-nesting-pipeline-for-loop-8"}}]}}}, {"apiVersion": "custom.tekton.dev/v1alpha1",
+      "kind": "PipelineLoop", "metadata": {"name": "withitem-multiple-nesting-pipeline-for-loop-8"},
       "spec": {"iterateParam": "loop-item-param-7", "iterationNumberParam": "iteration-number-9",
       "pipelineSpec": {"params": [{"name": "iteration-number-4", "type": "string"},
       {"name": "iteration-number-9", "type": "string"}, {"name": "loop-item-param-2-subvar-b",
@@ -144,7 +145,11 @@ spec:
       params:
       - name: loop-item-param-2
         value: '[{"a": 1, "b": 2}, {"a": 10, "b": 20}]'
+      - name: my_pipe_param
+        value: $(params.my_pipe_param)
       - name: my_pipe_param-loop-item
         value: $(params.my_pipe_param)
+      - name: my_pipe_param3
+        value: $(params.my_pipe_param3)
       - name: my_pipe_param3-loop-item
         value: $(params.my_pipe_param3)
diff --git a/sdk/python/tests/compiler/testdata/nested_loop_counter.yaml b/sdk/python/tests/compiler/testdata/nested_loop_counter.yaml
@@ -48,8 +48,6 @@ spec:
             params:
             - name: loop-item-param-1
               type: string
-            - name: loop-item-param-3
-              type: string
             tasks:
             - name: loop-3-range-for-loop-4
               params:

diff --git a/sdk/python/tests/compiler/testdata/nested_loop_counter_noninlined.yaml b/sdk/python/tests/compiler/testdata/nested_loop_counter_noninlined.yaml
@@ -31,18 +31,17 @@ metadata:
     tekton.dev/resource_templates: '[{"apiVersion": "custom.tekton.dev/v1alpha1",
       "kind": "PipelineLoop", "metadata": {"name": "loop-3-range-for-loop-2"}, "spec":
       {"iterateNumeric": "loop-item-param-1", "pipelineSpec": {"params": [{"name":
-      "loop-item-param-1", "type": "string"}, {"name": "loop-item-param-3", "type":
-      "string"}], "tasks": [{"name": "loop-3-range-for-loop-4", "params": [{"name":
-      "from", "value": "1"}, {"name": "loop-item-param-1", "value": "$(params.loop-item-param-1)"},
+      "loop-item-param-1", "type": "string"}], "tasks": [{"name": "loop-3-range-for-loop-4",
+      "params": [{"name": "from", "value": "1"}, {"name": "loop-item-param-1", "value":
+      "$(params.loop-item-param-1)"}, {"name": "to", "value": "2"}], "taskRef": {"apiVersion":
+      "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop", "name": "loop-3-range-for-loop-4"}}]}}},
+      {"apiVersion": "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop", "metadata":
+      {"name": "loop-3-range-for-loop-4"}, "spec": {"iterateNumeric": "loop-item-param-3",
+      "pipelineSpec": {"params": [{"name": "loop-item-param-1", "type": "string"},
+      {"name": "loop-item-param-3", "type": "string"}], "tasks": [{"name": "loop-3-range-for-loop-6",
+      "params": [{"name": "from", "value": "1"}, {"name": "loop-item-param-1", "value":
+      "$(params.loop-item-param-1)"}, {"name": "loop-item-param-3", "value": "$(params.loop-item-param-3)"},
       {"name": "to", "value": "2"}], "taskRef": {"apiVersion": "custom.tekton.dev/v1alpha1",
-      "kind": "PipelineLoop", "name": "loop-3-range-for-loop-4"}}]}}}, {"apiVersion":
-      "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop", "metadata": {"name": "loop-3-range-for-loop-4"},
-      "spec": {"iterateNumeric": "loop-item-param-3", "pipelineSpec": {"params": [{"name":
-      "loop-item-param-1", "type": "string"}, {"name": "loop-item-param-3", "type":
-      "string"}], "tasks": [{"name": "loop-3-range-for-loop-6", "params": [{"name":
-      "from", "value": "1"}, {"name": "loop-item-param-1", "value": "$(params.loop-item-param-1)"},
-      {"name": "loop-item-param-3", "value": "$(params.loop-item-param-3)"}, {"name":
-      "to", "value": "2"}], "taskRef": {"apiVersion": "custom.tekton.dev/v1alpha1",
       "kind": "PipelineLoop", "name": "loop-3-range-for-loop-6"}}]}}}, {"apiVersion":
       "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop", "metadata": {"name": "loop-3-range-for-loop-6"},
       "spec": {"iterateNumeric": "loop-item-param-5", "pipelineSpec": {"params": [{"name":

diff --git a/sdk/python/tests/compiler/testdata/nested_loop_global_param.yaml b/sdk/python/tests/compiler/testdata/nested_loop_global_param.yaml
@@ -119,6 +119,8 @@ spec:
             pipelines.kubeflow.org/cache_enabled: "true"
     - name: empty-loop-for-loop-3
       params:
+      - name: param
+        value: $(params.param)
       - name: param-loop-item
         value: $(params.param)
       taskSpec:

diff --git a/sdk/python/tests/compiler/testdata/nested_loop_global_param_noninlined.yaml b/sdk/python/tests/compiler/testdata/nested_loop_global_param_noninlined.yaml
@@ -99,3 +99,5 @@ spec:
       params:
       - name: param-loop-item
         value: $(params.param)
+      - name: param
+        value: $(params.param)
diff --git a/sdk/python/tests/compiler/testdata/nested_loop_param.yaml b/sdk/python/tests/compiler/testdata/nested_loop_param.yaml
@@ -48,8 +48,6 @@ spec:
             params:
             - name: loop-item-param-2
               type: string
-            - name: loop-item-param-6
-              type: string
             tasks:
             - name: loop-3-range-for-loop-7
               params:

diff --git a/sdk/python/tests/compiler/testdata/nested_loop_param_noninlined.yaml b/sdk/python/tests/compiler/testdata/nested_loop_param_noninlined.yaml
@@ -31,31 +31,30 @@ metadata:
     tekton.dev/resource_templates: '[{"apiVersion": "custom.tekton.dev/v1alpha1",
       "kind": "PipelineLoop", "metadata": {"name": "loop-3-range-for-loop-4"}, "spec":
       {"iterateParam": "loop-item-param-2", "iterateParamStringSeparator": "loop-item-param-3",
-      "pipelineSpec": {"params": [{"name": "loop-item-param-2", "type": "string"},
-      {"name": "loop-item-param-6", "type": "string"}], "tasks": [{"name": "loop-3-range-for-loop-7",
-      "params": [{"name": "loop-item-param-2", "value": "$(params.loop-item-param-2)"},
-      {"name": "loop-item-param-6", "value": "[A,B,C]"}], "taskRef": {"apiVersion":
-      "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop", "name": "loop-3-range-for-loop-7"}}]}}},
-      {"apiVersion": "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop", "metadata":
-      {"name": "loop-3-range-for-loop-7"}, "spec": {"iterateParam": "loop-item-param-6",
-      "pipelineSpec": {"params": [{"name": "loop-item-param-2", "type": "string"},
-      {"name": "loop-item-param-6", "type": "string"}], "tasks": [{"name": "loop-3-range-for-loop-11",
-      "params": [{"name": "loop-item-param-10", "value": ","}, {"name": "loop-item-param-2",
+      "pipelineSpec": {"params": [{"name": "loop-item-param-2", "type": "string"}],
+      "tasks": [{"name": "loop-3-range-for-loop-7", "params": [{"name": "loop-item-param-2",
       "value": "$(params.loop-item-param-2)"}, {"name": "loop-item-param-6", "value":
-      "$(params.loop-item-param-6)"}, {"name": "loop-item-param-9", "value": "1,2,3"}],
-      "taskRef": {"apiVersion": "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop",
-      "name": "loop-3-range-for-loop-11"}}]}}}, {"apiVersion": "custom.tekton.dev/v1alpha1",
-      "kind": "PipelineLoop", "metadata": {"name": "loop-3-range-for-loop-11"}, "spec":
-      {"iterateParam": "loop-item-param-9", "iterateParamStringSeparator": "loop-item-param-10",
-      "pipelineSpec": {"params": [{"name": "loop-item-param-2", "type": "string"},
-      {"name": "loop-item-param-6", "type": "string"}, {"name": "loop-item-param-9",
-      "type": "string"}], "tasks": [{"name": "print", "params": [{"name": "loop-item-param-2",
-      "value": "$(params.loop-item-param-2)"}, {"name": "loop-item-param-6", "value":
-      "$(params.loop-item-param-6)"}, {"name": "loop-item-param-9", "value": "$(params.loop-item-param-9)"}],
-      "taskSpec": {"metadata": {"annotations": {"pipelines.kubeflow.org/component_spec_digest":
-      "{\"name\": \"print\", \"outputs\": [{\"description\": \"Represents an output
-      paramter.\", \"name\": \"output_value\", \"type\": \"String\"}], \"version\":
-      \"print@sha256=c6e88bb19253b3bedeb9912855f4e324700cd80285e6b625b9ebcffb58677766\"}"},
+      "[A,B,C]"}], "taskRef": {"apiVersion": "custom.tekton.dev/v1alpha1", "kind":
+      "PipelineLoop", "name": "loop-3-range-for-loop-7"}}]}}}, {"apiVersion": "custom.tekton.dev/v1alpha1",
+      "kind": "PipelineLoop", "metadata": {"name": "loop-3-range-for-loop-7"}, "spec":
+      {"iterateParam": "loop-item-param-6", "pipelineSpec": {"params": [{"name": "loop-item-param-2",
+      "type": "string"}, {"name": "loop-item-param-6", "type": "string"}], "tasks":
+      [{"name": "loop-3-range-for-loop-11", "params": [{"name": "loop-item-param-10",
+      "value": ","}, {"name": "loop-item-param-2", "value": "$(params.loop-item-param-2)"},
+      {"name": "loop-item-param-6", "value": "$(params.loop-item-param-6)"}, {"name":
+      "loop-item-param-9", "value": "1,2,3"}], "taskRef": {"apiVersion": "custom.tekton.dev/v1alpha1",
+      "kind": "PipelineLoop", "name": "loop-3-range-for-loop-11"}}]}}}, {"apiVersion":
+      "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop", "metadata": {"name": "loop-3-range-for-loop-11"},
+      "spec": {"iterateParam": "loop-item-param-9", "iterateParamStringSeparator":
+      "loop-item-param-10", "pipelineSpec": {"params": [{"name": "loop-item-param-2",
+      "type": "string"}, {"name": "loop-item-param-6", "type": "string"}, {"name":
+      "loop-item-param-9", "type": "string"}], "tasks": [{"name": "print", "params":
+      [{"name": "loop-item-param-2", "value": "$(params.loop-item-param-2)"}, {"name":
+      "loop-item-param-6", "value": "$(params.loop-item-param-6)"}, {"name": "loop-item-param-9",
+      "value": "$(params.loop-item-param-9)"}], "taskSpec": {"metadata": {"annotations":
+      {"pipelines.kubeflow.org/component_spec_digest": "{\"name\": \"print\", \"outputs\":
+      [{\"description\": \"Represents an output paramter.\", \"name\": \"output_value\",
+      \"type\": \"String\"}], \"version\": \"print@sha256=c6e88bb19253b3bedeb9912855f4e324700cd80285e6b625b9ebcffb58677766\"}"},
       "labels": {"pipelines.kubeflow.org/cache_enabled": "true"}}, "params": [{"name":
       "loop-item-param-2", "type": "string"}, {"name": "loop-item-param-6", "type":
       "string"}, {"name": "loop-item-param-9", "type": "string"}], "results": [{"description":

diff --git a/sdk/python/tests/compiler/testdata/nested_loop_same_arg.yaml b/sdk/python/tests/compiler/testdata/nested_loop_same_arg.yaml
@@ -47,6 +47,8 @@ spec:
         value: $(params.param)
       - name: loop-item-param-3
         value: $(params.param)
+      - name: param
+        value: $(params.param)
       taskSpec:
         apiVersion: custom.tekton.dev/v1alpha1
         kind: PipelineLoop

diff --git a/sdk/python/tests/compiler/testdata/nested_loop_same_arg_noninlined.yaml b/sdk/python/tests/compiler/testdata/nested_loop_same_arg_noninlined.yaml
@@ -75,3 +75,5 @@ spec:
         value: $(params.param)
       - name: loop-item-param-3
         value: $(params.param)
+      - name: param
+        value: $(params.param)
diff --git a/sdk/python/tests/compiler/testdata/nested_loop_with_underscore.yaml b/sdk/python/tests/compiler/testdata/nested_loop_with_underscore.yaml
@@ -81,6 +81,8 @@ spec:
       params:
       - name: param_a-loop-item
         value: $(params.param_a)
+      - name: param_b
+        value: $(params.param_b)
       - name: param_b-loop-item
         value: $(params.param_b)
       - name: print-0-output_value

diff --git a/sdk/python/tests/compiler/testdata/nested_loop_with_underscore_noninlined.yaml b/sdk/python/tests/compiler/testdata/nested_loop_with_underscore_noninlined.yaml
@@ -114,3 +114,5 @@ spec:
         value: $(params.param_b)
       - name: print-0-output_value
         value: $(tasks.print-0.results.output-value)
+      - name: param_b
+        value: $(params.param_b)
diff --git a/sdk/python/tests/compiler/testdata/withitem_multi_nested.yaml b/sdk/python/tests/compiler/testdata/withitem_multi_nested.yaml
@@ -50,8 +50,12 @@ spec:
       params:
       - name: loop-item-param-1
         value: '[{"a": 1, "b": 2}, {"a": 10, "b": 20}]'
+      - name: my_pipe_param
+        value: $(params.my_pipe_param)
       - name: my_pipe_param-loop-item
         value: $(params.my_pipe_param)
+      - name: my_pipe_param3
+        value: $(params.my_pipe_param3)
       - name: my_pipe_param3-loop-item
         value: $(params.my_pipe_param3)
       taskSpec:
@@ -131,6 +135,8 @@ spec:
                 value: $(params.loop-item-param-1-subvar-b)
               - name: my_pipe_param-loop-item
                 value: $(params.my_pipe_param-loop-item)
+              - name: my_pipe_param3
+                value: $(params.my_pipe_param3)
               - name: my_pipe_param3-loop-item
                 value: $(params.my_pipe_param3-loop-item)
               taskSpec:
@@ -186,6 +192,8 @@ spec:
                         value: $(params.loop-item-param-1-subvar-b)
                       - name: loop-item-param-4
                         value: '[4, 5]'
+                      - name: my_pipe_param3
+                        value: $(params.my_pipe_param3)
                       - name: my_pipe_param3-loop-item
                         value: $(params.my_pipe_param3-loop-item)
                       taskSpec: