Fix edge cases such as disable istio-inject for Tekton compiler

sdk/python/kfp_tekton/compiler/_op_to_template.py

@@ -26,18 +26,6 @@
 from .. import tekton_api_version
 
 
-class literal_str(str):
-    """Literal string class for pyyaml
-
-    Literal string class is used for converting string with newline into
-    yaml's literal string format with '|'. In pyyaml, literal string
-    conversion is not natively supported in the default dumper.
-    Therefore, we need to define this class as part of the dumper
-    before compiling it into yaml.
-    """
-    pass
-
-
 def _get_base_step(name: str):
     """Base image step for running bash commands.
 
@@ -329,12 +317,11 @@ def _process_output_artifacts(outputs_dict: Dict[Text, Any],
         mounted_artifact_paths = []
         for artifact in outputs_dict['artifacts']:
             if artifact['name'] in replaced_param_list:
-                print(replaced_param_list)
                 copy_artifacts_step['script'] = copy_artifacts_step['script'] + \
-                    'mc cp $(results.%s.path) storage/%s/runs/$PIPELINERUN/$PODNAME/%s' % (artifact_to_result_mapping[artifact['name']], bucket, artifact['path'].rsplit("/", 1)[1])
+                    'mc cp $(results.%s.path) storage/%s/runs/$PIPELINERUN/$PODNAME/%s\n' % (artifact_to_result_mapping[artifact['name']], bucket, artifact['path'].rsplit("/", 1)[1])
             else:
                 copy_artifacts_step['script'] = copy_artifacts_step['script'] + \
-                    'mc cp %s storage/%s/runs/$PIPELINERUN/$PODNAME/%s' % (artifact['path'], bucket, artifact['path'].rsplit("/", 1)[1])
+                    'mc cp %s storage/%s/runs/$PIPELINERUN/$PODNAME/%s\n' % (artifact['path'], bucket, artifact['path'].rsplit("/", 1)[1])
                 if artifact['path'].rsplit("/", 1)[0] not in mounted_artifact_paths:
                     volume_mount_step_template.append({'name': artifact['name'], 'mountPath': artifact['path'].rsplit("/", 1)[0]})
                     volume_template.append({'name': artifact['name'], 'emptyDir': {}})
@@ -445,17 +432,6 @@ def _op_to_template(op: BaseOp, enable_artifacts=False):
     if processed_op.init_containers:
         template['spec']['steps'] = _prepend_steps(processed_op.init_containers, template['spec']['steps'])
 
-    # initial base_step and volume setup
-    base_step = {
-        'image': 'busybox',
-        'name': 'copy-results',
-        'script': '#!/bin/sh\nset -exo pipefail\n'
-    }
-    volume_mount_step_template = []
-    volume_template = []
-    mounted_param_paths = []
-    replaced_param_list = []
-
     # inputs
     input_artifact_paths = processed_op.input_artifact_paths if isinstance(processed_op, dsl.ContainerOp) else None
     artifact_arguments = processed_op.artifact_arguments if isinstance(processed_op, dsl.ContainerOp) else None
@@ -475,7 +451,6 @@ def _op_to_template(op: BaseOp, enable_artifacts=False):
             mount_path = artifact['path'].rsplit("/", 1)[0]
             if mount_path not in mounted_param_paths:
                 _add_mount_path(artifact['name'], artifact['path'], mount_path, volume_mount_step_template, volume_template, mounted_param_paths)
-        copy_inputs_step['script'] = literal_str(copy_inputs_step['script'])
         template['spec']['steps'] = _prepend_steps([copy_inputs_step], template['spec']['steps'])
         _update_volumes(template, volume_mount_step_template, volume_template)
 
@@ -502,11 +477,9 @@ def _op_to_template(op: BaseOp, enable_artifacts=False):
                                                         replaced_param_list,
                                                         artifact_to_result_mapping)
         if mounted_param_paths:
-            copy_results_step['script'] = literal_str(copy_results_step['script'])
             template['spec']['steps'].append(copy_results_step)
         _update_volumes(template, volume_mount_step_template, volume_template)
         if copy_artifacts_step:
-            copy_artifacts_step['script'] = literal_str(copy_artifacts_step['script'])
             template['spec']['steps'].append(copy_artifacts_step)
 
     # **********************************************************

sdk/python/kfp_tekton/compiler/compiler.py

@@ -23,7 +23,7 @@
 import textwrap
 from typing import Callable, Set, List, Text, Dict, Tuple, Any, Union, Optional
 
-from ._op_to_template import _op_to_template, literal_str
+from ._op_to_template import _op_to_template
 
 from kfp import dsl
 from kfp.compiler._default_transformers import add_pod_env
@@ -36,18 +36,6 @@
 from .. import tekton_api_version
 
 
-def _literal_str_representer(dumper, data):
-  """pyyaml representer for literal yaml string dumper
-
-  Create a representer for the literal string class that converts the string
-  object with newline into yaml's literal string '|' style.
-  """
-  return dumper.represent_scalar(u'tag:yaml.org,2002:str', data, style='|')
-
-# Add the _literal_str_representer as part of the yaml dumper.
-yaml.add_representer(literal_str, _literal_str_representer)
-
-
 class TektonCompiler(Compiler) :
   """DSL Compiler to generate Tekton YAML.
 
@@ -296,7 +284,7 @@ def _process_resourceOp(self, task_refs, pipeline):
                       valueFrom: '%s'
               """ % (value[0], value[1]))
               output_values += output_value
-            tp['value'] = literal_str(output_values)
+            tp['value'] = output_values
 
 
   def _workflow_with_pipelinerun(self, task_refs, pipeline, pipeline_template, workflow):
@@ -497,7 +485,11 @@ def _create_pipeline_workflow(self, args, pipeline, op_transformers=None, pipeli
       'apiVersion': tekton_api_version,
       'kind': 'Pipeline',
       'metadata': {
-        'name': pipeline.name or 'Pipeline'
+        'name': pipeline.name or 'Pipeline',
+        'annotations': {
+          # Disable Istio inject since Tekton cannot run with Istio sidecar
+          'sidecar.istio.io/inject': 'false'
+        }
       },
       'spec': {
         'params': params,

sdk/python/tests/compiler/compiler_tests.py

@@ -18,6 +18,7 @@
 import unittest
 import yaml
 import re
+import textwrap
 
 from kfp_tekton import compiler
 
@@ -26,6 +27,24 @@
 # in order to generate new "golden" YAML files
 GENERATE_GOLDEN_YAML = False
 
+# License header for Kubeflow project
+LICENSE_HEADER = textwrap.dedent("""\
+# 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.
+
+""")
+
 
 class TestTektonCompiler(unittest.TestCase):
 
@@ -236,6 +255,8 @@ def _test_pipeline_workflow(self,
         compiled = list(yaml.safe_load_all(f))
       if GENERATE_GOLDEN_YAML:
         with open(golden_yaml_file, 'w') as f:
+          f.write(LICENSE_HEADER)
+        with open(golden_yaml_file, 'a+') as f:
           yaml.dump_all(compiled, f, default_flow_style=False)
       else:
         with open(golden_yaml_file, 'r') as f:

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

@@ -29,6 +29,7 @@ metadata:
   annotations:
     pipelines.kubeflow.org/pipeline_spec: '{"description": "A pipeline with affinity",
       "name": "affinity"}'
+    sidecar.istio.io/inject: 'false'
   name: affinity
 spec:
   params: []

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

@@ -57,10 +57,14 @@ spec:
           name: $(inputs.params.secret_name)
     image: minio/mc
     name: copy-artifacts
-    script: |-
-      #!/usr/bin/env sh
-      mc config host add storage http://minio-service.$(inputs.params.namespace):9000 $AWS_ACCESS_KEY_ID $AWS_SECRET_ACCESS_KEY
+    script: '#!/usr/bin/env sh
+
+      mc config host add storage http://minio-service.$(inputs.params.namespace):9000
+      $AWS_ACCESS_KEY_ID $AWS_SECRET_ACCESS_KEY
+
       mc cp $(results.output.path) storage/$(inputs.params.bucket)/runs/$PIPELINERUN/$PODNAME/output.txt
+
+      '
 ---
 apiVersion: tekton.dev/v1beta1
 kind: Pipeline
@@ -74,6 +78,7 @@ metadata:
       true, "type": "String"}, {"default": "kubeflow", "name": "namespace", "optional":
       true, "type": "String"}, {"default": "mlpipeline", "name": "bucket", "optional":
       true, "type": "String"}], "name": "custom_artifact_location_pipeline"}'
+    sidecar.istio.io/inject: 'false'
   name: custom-artifact-location-pipeline
 spec:
   params:

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

@@ -136,6 +136,7 @@ metadata:
   annotations:
     pipelines.kubeflow.org/pipeline_spec: '{"description": "shows how to use dsl.Condition.",
       "name": "pipeline flip coin"}'
+    sidecar.istio.io/inject: 'false'
   name: pipeline-flip-coin
 spec:
   params: []

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

@@ -34,10 +34,13 @@ spec:
     name: download-file
   - image: busybox
     name: copy-results
-    script: |
-      #!/bin/sh
+    script: '#!/bin/sh
+
       set -exo pipefail
+
       cp /tmp/results.txt $(results.data.path);
+
+      '
   volumes:
   - emptyDir: {}
     name: data
@@ -65,6 +68,7 @@ metadata:
   annotations:
     pipelines.kubeflow.org/pipeline_spec: '{"description": "Run a script that passes
       file to a non configurable path", "name": "Hidden output file pipeline"}'
+    sidecar.istio.io/inject: 'false'
   name: hidden-output-file-pipeline
 spec:
   params: []

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

@@ -39,6 +39,7 @@ metadata:
     pipelines.kubeflow.org/pipeline_spec: '{"description": "Get Most Frequent Word
       and Save to GCS", "inputs": [{"default": "This is testing", "name": "message",
       "optional": true}], "name": "Save Most Frequent"}'
+    sidecar.istio.io/inject: 'false'
   name: save-most-frequent
 spec:
   params:

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

@@ -35,6 +35,7 @@ metadata:
   annotations:
     pipelines.kubeflow.org/pipeline_spec: '{"description": "A pipeline with init container.",
       "name": "InitContainer"}'
+    sidecar.istio.io/inject: 'false'
   name: initcontainer
 spec:
   params: []

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

@@ -24,10 +24,13 @@ spec:
   steps:
   - image: busybox
     name: copy-inputs
-    script: |
-      #!/bin/sh
+    script: '#!/bin/sh
+
       set -exo pipefail
+
       echo -n "Constant artifact value" > /tmp/inputs/text/data
+
+      '
   - command:
     - cat
     - /tmp/inputs/text/data
@@ -49,10 +52,13 @@ spec:
   steps:
   - image: busybox
     name: copy-inputs
-    script: |
-      #!/bin/sh
+    script: '#!/bin/sh
+
       set -exo pipefail
+
       echo -n "Constant artifact value" > /tmp/inputs/text/data
+
+      '
   - command:
     - cat
     - /tmp/inputs/text/data
@@ -74,10 +80,13 @@ spec:
   steps:
   - image: busybox
     name: copy-inputs
-    script: |
-      #!/bin/sh
+    script: '#!/bin/sh
+
       set -exo pipefail
+
       echo -n "hard-coded artifact value" > /tmp/inputs/text/data
+
+      '
   - command:
     - cat
     - /tmp/inputs/text/data
@@ -99,11 +108,15 @@ spec:
   steps:
   - image: busybox
     name: copy-inputs
-    script: |
-      #!/bin/sh
+    script: '#!/bin/sh
+
       set -exo pipefail
+
       echo -n "Text from a file with hard-coded artifact value
+
       " > /tmp/inputs/text/data
+
+      '
   - command:
     - cat
     - /tmp/inputs/text/data
@@ -120,6 +133,7 @@ metadata:
     pipelines.kubeflow.org/pipeline_spec: '{"description": "Pipeline shows how to
       define artifact inputs and pass raw artifacts to them.", "name": "Pipeline with
       artifact input raw argument value."}'
+    sidecar.istio.io/inject: 'false'
   name: pipeline-with-artifact-input-raw-argument-value
 spec:
   params: []

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

@@ -97,6 +97,7 @@ metadata:
       {"default": "60", "name": "experimentTimeoutMinutes", "optional": true}, {"default":
       "True", "name": "deleteAfterDone", "optional": true}], "name": "Launch katib
       experiment"}'
+    sidecar.istio.io/inject: 'false'
   name: launch-katib-experiment
 spec:
   params:

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

@@ -67,6 +67,7 @@ metadata:
   annotations:
     pipelines.kubeflow.org/pipeline_spec: '{"inputs": [{"default": "10", "name": "my_pipe_param",
       "optional": true}], "name": "my-pipeline"}'
+    sidecar.istio.io/inject: 'false'
   name: my-pipeline
 spec:
   params:

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

@@ -29,6 +29,7 @@ metadata:
   annotations:
     pipelines.kubeflow.org/pipeline_spec: '{"description": "A pipeline with Node Selector",
       "name": "node_selector"}'
+    sidecar.istio.io/inject: 'false'
   name: node-selector
 spec:
   params: []

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

@@ -81,6 +81,7 @@ metadata:
       in parallel and prints the concatenated result.", "inputs": [{"default": "gs://ml-pipeline-playground/shakespeare1.txt",
       "name": "url1", "optional": true}, {"default": "gs://ml-pipeline-playground/shakespeare2.txt",
       "name": "url2", "optional": true}], "name": "Parallel pipeline"}'
+    sidecar.istio.io/inject: 'false'
   name: parallel-pipeline
 spec:
   params:

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

@@ -81,6 +81,7 @@ metadata:
       in parallel and prints the concatenated result.", "inputs": [{"default": "gs://ml-pipeline-playground/shakespeare1.txt",
       "name": "url1", "optional": true}, {"default": "gs://ml-pipeline-playground/shakespeare2.txt",
       "name": "url2", "optional": true}], "name": "Parallel pipeline"}'
+    sidecar.istio.io/inject: 'false'
   name: parallel-pipeline
 spec:
   params:

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

@@ -1,3 +1,17 @@
+# 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/v1beta1
 kind: Task
 metadata:
@@ -68,6 +82,7 @@ metadata:
       [{"default": "gs://ml-pipeline-playground/shakespeare1.txt", "name": "url1",
       "optional": true}, {"default": "gs://ml-pipeline-playground/shakespeare2.txt",
       "name": "url2", "optional": true}], "name": "Parallel pipeline with argo vars"}'
+    sidecar.istio.io/inject: 'false'
   name: parallel-pipeline-with-argo-vars
 spec:
   params:

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

@@ -51,6 +51,7 @@ metadata:
     pipelines.kubeflow.org/pipeline_spec: '{"description": "The pipeline shows how
       to apply functions to all ops in the pipeline by pipeline transformers", "name":
       "Pipeline transformer"}'
+    sidecar.istio.io/inject: 'false'
   name: pipeline-transformer
 spec:
   params: []