Update conditional feature with the catalog condition approach (kubef…

…low#167) * update conditional feature with the catalog condition approach * fix lint errors * add catalog condition template * change parameter name * Init README * amend parameter reference * Add documentation and increase clarity of code * improve readability * Update catalog
red-hat-data-services · Jun 5, 2020 · d806bc8 · d806bc8
1 parent 2c68de4
commit d806bc8
Show file tree

Hide file tree

Showing 4 changed files with 124 additions and 86 deletions.
diff --git a/sdk/python/kfp_tekton/compiler/compiler.py b/sdk/python/kfp_tekton/compiler/compiler.py
@@ -38,6 +38,42 @@
 from kfp_tekton import tekton_api_version
 
 
+def _get_super_condition_template():
+
+  python_script = textwrap.dedent('''\
+    'import sys
+    input1=str.rstrip(sys.argv[1])
+    input2=str.rstrip(sys.argv[2])
+    try:
+      input1=int(input1)
+      input2=int(input2)
+    except:
+      input1=str(input1)
+    sys.exit(0) if (input1 $(params.operator) input2) else sys.exit(1)' ''')
+
+  # TODO Change to tekton_api_version once Conditions are out of v1alpha1
+  template = {
+    'apiVersion': 'tekton.dev/v1alpha1',
+    'kind': 'Condition',
+    'metadata': {
+      'name': 'super-condition'
+    },
+    'spec': {
+      'params': [
+        {'name': 'operand1'},
+        {'name': 'operand2'},
+        {'name': 'operator'}
+      ],
+      'check': {
+        'script': 'python -c ' + python_script + "'$(params.operand1)' '$(params.operand2)'",
+        'image': 'python:alpine3.6',
+      }
+    }
+  }
+
+  return template
+
+
 class TektonCompiler(Compiler):
   """DSL Compiler to generate Tekton YAML.
 
@@ -132,7 +168,7 @@ def _resolve_value_or_reference(self, value_or_reference, potential_references):
         if task_name is None:
           return '$(params.%s)' % parameter_name
         else:
-          return '$(params.%s)' % task_name
+          return '$(tasks.%s.results.%s)' % (task_name, parameter_name)
       else:
         return '$(params.%s)' % parameter_name
     else:
@@ -153,41 +189,21 @@ def _group_to_dag_template(self, group, inputs, outputs, dependencies):
       'spec': {}
     }
 
-    # Generates template sections unique to conditions
+    # Generates a pseudo-template unique to conditions due to the catalog condition approach
+    # where every condition is an extension of one super-condition
     if isinstance(sub_group, dsl.OpsGroup) and sub_group.type == 'condition':
       subgroup_inputs = inputs.get(sub_group.name, [])
       condition = sub_group.condition
 
       operand1_value = self._resolve_value_or_reference(condition.operand1, subgroup_inputs)
       operand2_value = self._resolve_value_or_reference(condition.operand2, subgroup_inputs)
 
-      # Adds params to condition template
-      params = []
-      if isinstance(condition.operand1, dsl.PipelineParam):
-        params.append({'name': operand1_value[9: len(operand1_value) - 1]})  # substring to unwrap parameter reference
-      if isinstance(condition.operand2, dsl.PipelineParam):
-        params.append({'name': operand2_value[9: len(operand1_value) - 1]})  # substring to unwrap parameter reference
-      if params:
-        template['spec']['params'] = params
-
-      # Surround the operands by quotes to ensure proper parsing on script execution
-      operand1_value = "'" + operand1_value + "'"
-      operand2_value = "'" + operand2_value + "'"
-
-      input_grab = 'EXITCODE=$(python -c \'import sys\ninput1=str.rstrip(sys.argv[1])\ninput2=str.rstrip(sys.argv[2])\n'
-      try_catch = 'try:\n  input1=int(input1)\n  input2=int(input2)\nexcept:\n  input1=str(input1)\n'
-      if_else = 'print(0) if (input1 ' + condition.operator + ' input2) else print(1)\' ' + operand1_value + ' ' + operand2_value + '); '
-      exit_code = 'exit $EXITCODE'
-      shell_script = input_grab + try_catch + if_else + exit_code
-
-      # TODO Change to tekton_api_version once Conditions are out of v1alpha1
-      template['apiVersion'] = 'tekton.dev/v1alpha1'
       template['kind'] = 'Condition'
-      template['spec']['check'] = {
-        'args': [shell_script],
-        'command': ['sh', '-c'],
-        'image': 'python:alpine3.6',
-      }
+      template['spec']['params'] = [
+        {'name': 'operand1', 'value': operand1_value},
+        {'name': 'operand2', 'value': operand2_value},
+        {'name': 'operator', 'value': str(condition.operator)}
+      ]
 
     return template
 
@@ -307,7 +323,7 @@ def _workflow_with_pipelinerun(self, task_refs, pipeline, pipeline_template, wor
         }
       }
     }
-    
+
     # Generate TaskRunSpec PodTemplate:s
     task_run_spec = []
     for task in task_refs:
@@ -363,22 +379,29 @@ def _create_pipeline_workflow(self, args, pipeline, op_transformers=None, pipeli
       params.append(param)
 
     # generate Tekton tasks from pipeline ops
-    templates = self._create_dag_templates(pipeline, op_transformers, params)
+    raw_templates = self._create_dag_templates(pipeline, op_transformers, params)
 
     # generate task and condition reference list for the Tekton Pipeline
     condition_refs = {}
     task_refs = []
-    for template in templates:
+    templates = []
+    condition_added = False
+    for template in raw_templates:
+      # TODO Allow an opt-out for the condition_template
       if template['kind'] == 'Condition':
+        if not condition_added:
+          templates.append(_get_super_condition_template())
+          condition_added = True
         condition_refs[template['metadata']['name']] = [{
-          'conditionRef': template['metadata']['name'],
+          'conditionRef': 'super-condition',
           'params': [{
               'name': param['name'],
-              'value': '$(params.' + param['name'] + ')'
+              'value': param['value']
             } for param in template['spec'].get('params', [])
           ]
         }]
       else:
+        templates.append(template)
         task_refs.append(
           {
             'name': template['metadata']['name'],

diff --git a/sdk/python/tests/compiler/testdata/condition.yaml b/sdk/python/tests/compiler/testdata/condition.yaml
@@ -15,56 +15,18 @@
 apiVersion: tekton.dev/v1alpha1
 kind: Condition
 metadata:
-  name: condition-1
+  name: super-condition
 spec:
   check:
-    args:
-    - "EXITCODE=$(python -c 'import sys\ninput1=str.rstrip(sys.argv[1])\ninput2=str.rstrip(sys.argv[2])\n\
-      try:\n  input1=int(input1)\n  input2=int(input2)\nexcept:\n  input1=str(input1)\n\
-      print(0) if (input1 == input2) else print(1)' '$(params.flip)' 'heads'); exit\
-      \ $EXITCODE"
-    command:
-    - sh
-    - -c
     image: python:alpine3.6
-  params:
-  - name: flip
----
-apiVersion: tekton.dev/v1alpha1
-kind: Condition
-metadata:
-  name: condition-2
-spec:
-  check:
-    args:
-    - "EXITCODE=$(python -c 'import sys\ninput1=str.rstrip(sys.argv[1])\ninput2=str.rstrip(sys.argv[2])\n\
+    script: "python -c 'import sys\ninput1=str.rstrip(sys.argv[1])\ninput2=str.rstrip(sys.argv[2])\n\
       try:\n  input1=int(input1)\n  input2=int(input2)\nexcept:\n  input1=str(input1)\n\
-      print(0) if (input1 == input2) else print(1)' '$(params.flip-again)' 'tails');\
-      \ exit $EXITCODE"
-    command:
-    - sh
-    - -c
-    image: python:alpine3.6
-  params:
-  - name: flip-again
----
-apiVersion: tekton.dev/v1alpha1
-kind: Condition
-metadata:
-  name: condition-3
-spec:
-  check:
-    args:
-    - "EXITCODE=$(python -c 'import sys\ninput1=str.rstrip(sys.argv[1])\ninput2=str.rstrip(sys.argv[2])\n\
-      try:\n  input1=int(input1)\n  input2=int(input2)\nexcept:\n  input1=str(input1)\n\
-      print(0) if (input1 == input2) else print(1)' '$(params.flip)' 'tails'); exit\
-      \ $EXITCODE"
-    command:
-    - sh
-    - -c
-    image: python:alpine3.6
+      sys.exit(0) if (input1 $(params.operator) input2) else sys.exit(1)' '$(params.operand1)'\
+      \ '$(params.operand2)'"
   params:
-  - name: flip
+  - name: operand1
+  - name: operand2
+  - name: operator
 ---
 apiVersion: tekton.dev/v1beta1
 kind: Task
@@ -146,34 +108,50 @@ spec:
     taskRef:
       name: flip
   - conditions:
-    - conditionRef: condition-1
+    - conditionRef: super-condition
       params:
-      - name: flip
+      - name: operand1
         value: $(tasks.flip.results.output)
+      - name: operand2
+        value: heads
+      - name: operator
+        value: ==
     name: flip-again
     params: []
     taskRef:
       name: flip-again
   - conditions:
-    - conditionRef: condition-2
+    - conditionRef: super-condition
       params:
-      - name: flip-again
+      - name: operand1
         value: $(tasks.flip-again.results.output)
-    - conditionRef: condition-1
+      - name: operand2
+        value: tails
+      - name: operator
+        value: ==
+    - conditionRef: super-condition
       params:
-      - name: flip
+      - name: operand1
         value: $(tasks.flip.results.output)
+      - name: operand2
+        value: heads
+      - name: operator
+        value: ==
     name: print1
     params:
     - name: flip-again-output
       value: $(tasks.flip-again.results.output)
     taskRef:
       name: print1
   - conditions:
-    - conditionRef: condition-3
+    - conditionRef: super-condition
       params:
-      - name: flip
+      - name: operand1
         value: $(tasks.flip.results.output)
+      - name: operand2
+        value: tails
+      - name: operator
+        value: ==
     name: print2
     params:
     - name: flip-again-output

diff --git a/tekton-catalog/condition/README.md b/tekton-catalog/condition/README.md
@@ -0,0 +1,8 @@
+# condition
+Conditional statements are frequently used in Kubeflow Pipelines, and as such a Tekton Catalog Task for conditions has been provided as a simple way to 
+invoke conditions. This is pending the support for formal Conditions spec in Tekton V1beta1 API, which is being developed according the [design specs in Tekton community](https://docs.google.com/document/d/1kESrgmFHnirKNS4oDq3mucuB_OycBm6dSCSwRUHccZg/edit).
+
+## Parameters
+- operand1 - The left hand side operand of the condition
+- operand2 - The right hand side operand of the condition
+- operator - The conditional operator used to compare the two operands (e.g. == or >)
diff --git a/tekton-catalog/condition/condition.yaml b/tekton-catalog/condition/condition.yaml
@@ -0,0 +1,29 @@
+# Copyright 2020 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/v1alpha1
+kind: Condition
+metadata:
+  name: super-condition
+spec:
+  check:
+    image: python:alpine3.6
+    script: "python -c 'import sys\ninput1=str.rstrip(sys.argv[1])\ninput2=str.rstrip(sys.argv[2])\n\
+      try:\n  input1=int(input1)\n  input2=int(input2)\nexcept:\n  input1=str(input1)\n\
+      sys.exit(0) if (input1 $(params.operator) input2) else sys.exit(1)' '$(params.operand1)'\
+      \ '$(params.operand2)'"
+  params:
+  - name: operand1
+  - name: operand2
+  - name: operator