fix(sdk): fix nested loop underscore param (kubeflow#1076)

sdk/python/kfp_tekton/compiler/_tekton_handler.py

@@ -314,9 +314,17 @@ def _handle_tekton_custom_task(custom_task: dict, workflow: dict, recursive_task
                                     json.dumps(task['params']).replace(task_param['value'],
                                     '$(params.%s)' % param_result[1]))
                             else:
+                                new_param_ref = '%s-%s' % param_result
+                                # Nested task reference name must be the same as the one in the params
+                                # Otherwise, use the default param ref since it will be generated
+                                # by the custom task functions such as condition and loop
+                                for i in task.get('params', []):
+                                    if sanitize_k8s_name(i['name']) == new_param_ref:
+                                        new_param_ref = i['name']
+                                        break
                                 task['params'] = json.loads(
                                     json.dumps(task['params']).replace(task_param['value'],
-                                    '$(params.%s-%s)' % param_result))
+                                    '$(params.%s)' % new_param_ref))
         custom_task_crs.append(custom_task_cr)
         custom_task[custom_task_key]['spec']['params'] = sorted(custom_task[custom_task_key]['spec']['params'],
                                                                           key=lambda k: k['name'])

sdk/python/tests/compiler/compiler_tests.py

@@ -197,6 +197,13 @@ def test_nested_loop_same_arg_workflow(self):
     from .testdata.nested_loop_same_arg import loop_multi
     self._test_pipeline_workflow(loop_multi, 'nested_loop_same_arg.yaml')
 
+  def test_nested_loop_with_underscore_workflow(self):
+    """
+    Test compiling a nested loop with underscore argument workflow.
+    """
+    from .testdata.nested_loop_with_underscore import double_loop_with_underscore
+    self._test_pipeline_workflow(double_loop_with_underscore, 'nested_loop_with_underscore.yaml')
+
   def test_loop_with_numeric_workflow(self):
     """
     Test compiling a loop with numeric inputs in workflow.

sdk/python/tests/compiler/testdata/nested_loop_with_underscore.py

@@ -0,0 +1,63 @@
+# Copyright 2022 kubeflow.org
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from kfp import dsl
+from kfp.components import load_component_from_text
+from kfp_tekton.compiler import TektonCompiler
+
+
+class Coder:
+  def empty(self):
+    return ""
+
+
+TektonCompiler._get_unique_id_code = Coder.empty
+
+
+def PrintOp(name: str, msg: str = None):
+  if msg is None:
+    msg = name
+  print_op = load_component_from_text(
+  """
+  name: %s
+  inputs:
+  - {name: input_text, type: String, description: 'Represents an input parameter.'}
+  outputs:
+  - {name: output_value, type: String, description: 'Represents an output paramter.'}
+  implementation:
+    container:
+      image: alpine:3.6
+      command:
+      - sh
+      - -c
+      - |
+        set -e
+        echo $0 > $1
+      - {inputValue: input_text}
+      - {outputPath: output_value}
+  """ % (name)
+  )
+  return print_op(msg)
+
+
+@dsl.pipeline("double_loop_with_underscore")
+def double_loop_with_underscore(param_a: list = [1, 2, 3], param_b: list = ["a", "b", "c"]):
+  op0 = PrintOp('print-0', f"Hello!")
+  with dsl.ParallelFor(param_a):
+    with dsl.ParallelFor(param_b):
+      op1 = PrintOp('print-1', f"print {op0.output}")
+
+
+if __name__ == '__main__':
+  TektonCompiler().compile(double_loop_with_underscore, __file__.replace('.py', '.yaml'))

sdk/python/tests/compiler/testdata/nested_loop_with_underscore.yaml

@@ -0,0 +1,160 @@
+# Copyright 2021 kubeflow.org
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  name: double-loop-with-underscore
+  annotations:
+    tekton.dev/output_artifacts: '{"print-0": [{"key": "artifacts/$PIPELINERUN/print-0/output_value.tgz",
+      "name": "print-0-output_value", "path": "/tmp/outputs/output_value/data"}],
+      "print-1": [{"key": "artifacts/$PIPELINERUN/print-1/output_value.tgz", "name":
+      "print-1-output_value", "path": "/tmp/outputs/output_value/data"}]}'
+    tekton.dev/input_artifacts: '{"print-1": [{"name": "print-0-output_value", "parent_task":
+      "print-0"}]}'
+    tekton.dev/artifact_bucket: mlpipeline
+    tekton.dev/artifact_endpoint: minio-service.kubeflow:9000
+    tekton.dev/artifact_endpoint_scheme: http://
+    tekton.dev/artifact_items: '{"print-0": [["output_value", "$(results.output-value.path)"]],
+      "print-1": [["output_value", "$(results.output-value.path)"]]}'
+    sidecar.istio.io/inject: "false"
+    tekton.dev/template: ''
+    pipelines.kubeflow.org/big_data_passing_format: $(workspaces.$TASK_NAME.path)/artifacts/$ORIG_PR_NAME/$TASKRUN_NAME/$TASK_PARAM_NAME
+    pipelines.kubeflow.org/pipeline_spec: '{"inputs": [{"default": "[1, 2, 3]", "name":
+      "param_a", "optional": true, "type": "JsonArray"}, {"default": "[\"a\", \"b\",
+      \"c\"]", "name": "param_b", "optional": true, "type": "JsonArray"}], "name":
+      "double_loop_with_underscore"}'
+  labels:
+    pipelines.kubeflow.org/pipelinename: ''
+    pipelines.kubeflow.org/generation: ''
+spec:
+  params:
+  - name: param_a
+    value: '[1, 2, 3]'
+  - name: param_b
+    value: '["a", "b", "c"]'
+  pipelineSpec:
+    params:
+    - name: param_a
+      default: '[1, 2, 3]'
+    - name: param_b
+      default: '["a", "b", "c"]'
+    tasks:
+    - name: print-0
+      taskSpec:
+        steps:
+        - name: main
+          command:
+          - sh
+          - -c
+          - |
+            set -e
+            echo $0 > $1
+          - Hello!
+          - $(results.output-value.path)
+          image: alpine:3.6
+        results:
+        - name: output-value
+          type: string
+          description: /tmp/outputs/output_value/data
+        metadata:
+          labels:
+            pipelines.kubeflow.org/cache_enabled: "true"
+          annotations:
+            pipelines.kubeflow.org/component_spec_digest: '{"name": "print-0", "outputs":
+              [{"description": "Represents an output paramter.", "name": "output_value",
+              "type": "String"}], "version": "print-0@sha256=5dd3c506ec54281b82008ca8ec5d8142834eae18d74ce8a110a31dd6e371b40d"}'
+    - runAfter:
+      - print-0
+      name: double-loop-with-underscore-for-loop-1
+      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
+        value: $(tasks.print-0.results.output-value)
+      taskSpec:
+        apiVersion: custom.tekton.dev/v1alpha1
+        kind: PipelineLoop
+        spec:
+          pipelineSpec:
+            params:
+            - name: param_a-loop-item
+              type: string
+            - name: param_b
+              type: string
+            - name: param_b-loop-item
+              type: string
+            - name: print-0-output_value
+              type: string
+            tasks:
+            - name: double-loop-with-underscore-for-loop-2
+              params:
+              - name: param_b-loop-item
+                value: $(params.param_b-loop-item)
+              - name: print-0-output_value
+                value: $(params.print-0-output_value)
+              taskSpec:
+                apiVersion: custom.tekton.dev/v1alpha1
+                kind: PipelineLoop
+                spec:
+                  pipelineSpec:
+                    params:
+                    - name: param_b-loop-item
+                      type: string
+                    - name: print-0-output_value
+                      type: string
+                    tasks:
+                    - name: print-1
+                      params:
+                      - name: print-0-output_value
+                        value: $(params.print-0-output_value)
+                      taskSpec:
+                        steps:
+                        - name: main
+                          command:
+                          - sh
+                          - -c
+                          - |
+                            set -e
+                            echo $0 > $1
+                          - print $(inputs.params.print-0-output_value)
+                          - $(results.output-value.path)
+                          image: alpine:3.6
+                        params:
+                        - name: print-0-output_value
+                          type: string
+                        results:
+                        - name: output-value
+                          type: string
+                          description: /tmp/outputs/output_value/data
+                        metadata:
+                          labels:
+                            pipelines.kubeflow.org/cache_enabled: "true"
+                          annotations:
+                            pipelines.kubeflow.org/component_spec_digest: '{"name":
+                              "print-1", "outputs": [{"description": "Represents an
+                              output paramter.", "name": "output_value", "type": "String"}],
+                              "version": "print-1@sha256=3b81342bc143f625b58ebdb01e7c83b145880dee807be35c1e16fdb835d46580"}'
+                  iterateParam: param_b-loop-item
+                metadata:
+                  labels:
+                    pipelines.kubeflow.org/cache_enabled: "true"
+          iterateParam: param_a-loop-item
+        metadata:
+          labels:
+            pipelines.kubeflow.org/cache_enabled: "true"

sdk/python/tests/compiler/testdata/nested_loop_with_underscore_noninlined.yaml

@@ -0,0 +1,116 @@
+# Copyright 2021 kubeflow.org
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  name: double-loop-with-underscore
+  annotations:
+    tekton.dev/output_artifacts: '{"print-0": [{"key": "artifacts/$PIPELINERUN/print-0/output_value.tgz",
+      "name": "print-0-output_value", "path": "/tmp/outputs/output_value/data"}],
+      "print-1": [{"key": "artifacts/$PIPELINERUN/print-1/output_value.tgz", "name":
+      "print-1-output_value", "path": "/tmp/outputs/output_value/data"}]}'
+    tekton.dev/input_artifacts: '{"print-1": [{"name": "print-0-output_value", "parent_task":
+      "print-0"}]}'
+    tekton.dev/artifact_bucket: mlpipeline
+    tekton.dev/artifact_endpoint: minio-service.kubeflow:9000
+    tekton.dev/artifact_endpoint_scheme: http://
+    tekton.dev/artifact_items: '{"print-0": [["output_value", "$(results.output-value.path)"]],
+      "print-1": [["output_value", "$(results.output-value.path)"]]}'
+    sidecar.istio.io/inject: "false"
+    tekton.dev/template: ''
+    pipelines.kubeflow.org/big_data_passing_format: $(workspaces.$TASK_NAME.path)/artifacts/$ORIG_PR_NAME/$TASKRUN_NAME/$TASK_PARAM_NAME
+    pipelines.kubeflow.org/pipeline_spec: '{"inputs": [{"default": "[1, 2, 3]", "name":
+      "param_a", "optional": true, "type": "JsonArray"}, {"default": "[\"a\", \"b\",
+      \"c\"]", "name": "param_b", "optional": true, "type": "JsonArray"}], "name":
+      "double_loop_with_underscore"}'
+    tekton.dev/resource_templates: '[{"apiVersion": "custom.tekton.dev/v1alpha1",
+      "kind": "PipelineLoop", "metadata": {"name": "double-loop-with-underscore-for-loop-1"},
+      "spec": {"iterateParam": "param_a-loop-item", "pipelineSpec": {"params": [{"name":
+      "param_a-loop-item", "type": "string"}, {"name": "param_b", "type": "string"},
+      {"name": "param_b-loop-item", "type": "string"}, {"name": "print-0-output_value",
+      "type": "string"}], "tasks": [{"name": "double-loop-with-underscore-for-loop-2",
+      "params": [{"name": "param_b-loop-item", "value": "$(params.param_b-loop-item)"},
+      {"name": "print-0-output_value", "value": "$(params.print-0-output_value)"}],
+      "taskRef": {"apiVersion": "custom.tekton.dev/v1alpha1", "kind": "PipelineLoop",
+      "name": "double-loop-with-underscore-for-loop-2"}}]}}}, {"apiVersion": "custom.tekton.dev/v1alpha1",
+      "kind": "PipelineLoop", "metadata": {"name": "double-loop-with-underscore-for-loop-2"},
+      "spec": {"iterateParam": "param_b-loop-item", "pipelineSpec": {"params": [{"name":
+      "param_b-loop-item", "type": "string"}, {"name": "print-0-output_value", "type":
+      "string"}], "tasks": [{"name": "print-1", "params": [{"name": "print-0-output_value",
+      "value": "$(params.print-0-output_value)"}], "taskSpec": {"metadata": {"annotations":
+      {"pipelines.kubeflow.org/component_spec_digest": "{\"name\": \"print-1\", \"outputs\":
+      [{\"description\": \"Represents an output paramter.\", \"name\": \"output_value\",
+      \"type\": \"String\"}], \"version\": \"print-1@sha256=3b81342bc143f625b58ebdb01e7c83b145880dee807be35c1e16fdb835d46580\"}"},
+      "labels": {"pipelines.kubeflow.org/cache_enabled": "true"}}, "params": [{"name":
+      "print-0-output_value", "type": "string"}], "results": [{"description": "/tmp/outputs/output_value/data",
+      "name": "output-value", "type": "string"}], "steps": [{"command": ["sh", "-c",
+      "set -e\necho $0 > $1\n", "print $(inputs.params.print-0-output_value)", "$(results.output-value.path)"],
+      "image": "alpine:3.6", "name": "main"}]}}]}}}]'
+  labels:
+    pipelines.kubeflow.org/pipelinename: ''
+    pipelines.kubeflow.org/generation: ''
+spec:
+  params:
+  - name: param_a
+    value: '[1, 2, 3]'
+  - name: param_b
+    value: '["a", "b", "c"]'
+  pipelineSpec:
+    params:
+    - name: param_a
+      default: '[1, 2, 3]'
+    - name: param_b
+      default: '["a", "b", "c"]'
+    tasks:
+    - name: print-0
+      taskSpec:
+        steps:
+        - name: main
+          command:
+          - sh
+          - -c
+          - |
+            set -e
+            echo $0 > $1
+          - Hello!
+          - $(results.output-value.path)
+          image: alpine:3.6
+        results:
+        - name: output-value
+          type: string
+          description: /tmp/outputs/output_value/data
+        metadata:
+          labels:
+            pipelines.kubeflow.org/cache_enabled: "true"
+          annotations:
+            pipelines.kubeflow.org/component_spec_digest: '{"name": "print-0", "outputs":
+              [{"description": "Represents an output paramter.", "name": "output_value",
+              "type": "String"}], "version": "print-0@sha256=5dd3c506ec54281b82008ca8ec5d8142834eae18d74ce8a110a31dd6e371b40d"}'
+    - runAfter:
+      - print-0
+      name: double-loop-with-underscore-for-loop-1
+      taskRef:
+        apiVersion: custom.tekton.dev/v1alpha1
+        kind: PipelineLoop
+        name: double-loop-with-underscore-for-loop-1
+      params:
+      - name: param_a-loop-item
+        value: $(params.param_a)
+      - name: param_b-loop-item
+        value: $(params.param_b)
+      - name: print-0-output_value
+        value: $(tasks.print-0.results.output-value)
+      - name: param_b
+        value: $(params.param_b)