fix(sdk): Update SDK to pull artifact related information from annota…

…tions (kubeflow#259) * pull artifact related information from annotations * update test cases * update dict.get syntax for the merging
Tomcli · Aug 10, 2020 · ab108d4 · ab108d4
1 parent ece3a8a
commit ab108d4
Show file tree

Hide file tree

Showing 32 changed files with 482 additions and 176 deletions.
diff --git a/sdk/python/kfp_tekton/compiler/_op_to_template.py b/sdk/python/kfp_tekton/compiler/_op_to_template.py
@@ -291,16 +291,10 @@ def _process_output_artifacts(outputs_dict: Dict[Text, Any],
         Dict[Text, Any]
     """
     if outputs_dict.get('artifacts'):
-        # TODO: Pull default values from KFP configmap when integrated with KFP.
-        storage_location = outputs_dict['artifacts'][0].get('s3', {})
-        insecure = storage_location.get("insecure", True)
-        endpoint = storage_location.get("endpoint", "minio-service.$NAMESPACE:9000")
-        # We want to use the insecure flag to figure out whether to use http or https scheme
-        endpoint = re.sub(r"https?://", "", endpoint)
-        endpoint = 'http://' + endpoint if insecure else 'https://' + endpoint
-        access_key = storage_location.get("accessKeySecret", {"name": "mlpipeline-minio-artifact", "key": "accesskey"})
-        secret_access_key = storage_location.get("secretKeySecret", {"name": "mlpipeline-minio-artifact", "key": "secretkey"})
-        bucket = storage_location.get("bucket", "mlpipeline")
+        endpoint = '${ARTIFACT_ENDPOINT_SCHEME}${ARTIFACT_ENDPOINT}'
+        access_key = {"name": "mlpipeline-minio-artifact", "key": "accesskey"}
+        secret_access_key = {"name": "mlpipeline-minio-artifact", "key": "secretkey"}
+        bucket = '$ARTIFACT_BUCKET'
         copy_artifacts_step = {
             'image': 'minio/mc',
             'name': 'copy-artifacts',
@@ -311,7 +305,11 @@ def _process_output_artifacts(outputs_dict: Dict[Text, Any],
             'env': [
                 {'name': 'PIPELINERUN', 'valueFrom': {'fieldRef': {'fieldPath': "metadata.labels['tekton.dev/pipelineRun']"}}},
                 {'name': 'PIPELINETASK', 'valueFrom': {'fieldRef': {'fieldPath': "metadata.labels['tekton.dev/pipelineTask']"}}},
-                {'name': 'NAMESPACE', 'valueFrom': {'fieldRef': {'fieldPath': "metadata.namespace"}}},
+                {'name': 'ARTIFACT_ENDPOINT_SCHEME', 'valueFrom':
+                    {'fieldRef': {'fieldPath': "metadata.annotations['tekton.dev/artifact_endpoint_scheme']"}}},
+                {'name': 'ARTIFACT_ENDPOINT', 'valueFrom':
+                    {'fieldRef': {'fieldPath': "metadata.annotations['tekton.dev/artifact_endpoint']"}}},
+                {'name': 'ARTIFACT_BUCKET', 'valueFrom': {'fieldRef': {'fieldPath': "metadata.annotations['tekton.dev/artifact_bucket']"}}},
                 {'name': 'AWS_ACCESS_KEY_ID', 'valueFrom': {'secretKeyRef': {'name': access_key['name'], 'key': access_key['key']}}},
                 {'name': 'AWS_SECRET_ACCESS_KEY', 'valueFrom': {'secretKeyRef': {'name': secret_access_key['name'],
                                                                                  'key': secret_access_key['key']}}}
@@ -467,8 +465,10 @@ def _op_to_template(op: BaseOp, pipelinerun_output_artifacts={}, enable_artifact
                 output_annotation = pipelinerun_output_artifacts.get(processed_op.name, [])
                 output_annotation.append(
                     {
-                        'name': output_artifact.get('name'),
-                        'path': output_artifact.get('path')
+                        'name': output_artifact.get('name', ''),
+                        'path': output_artifact.get('path', ''),
+                        'key': "artifacts/$PIPELINERUN/%s/%s.tgz" %
+                        (processed_op.name, output_artifact.get('name', '').replace(processed_op.name + '-', ''))
                     }
                 )
                 pipelinerun_output_artifacts[processed_op.name] = output_annotation

diff --git a/sdk/python/kfp_tekton/compiler/compiler.py b/sdk/python/kfp_tekton/compiler/compiler.py
@@ -23,6 +23,7 @@
 import textwrap
 
 from typing import Callable, List, Text, Dict, Any
+from os import environ as env
 
 # Kubeflow Pipeline imports
 from kfp import dsl
@@ -40,6 +41,11 @@
 from kfp_tekton.compiler._op_to_template import _op_to_template
 
 
+DEFAULT_ARTIFACT_BUCKET = env.get('DEFAULT_ARTIFACT_BUCKET', 'mlpipeline')
+DEFAULT_ARTIFACT_ENDPOINT = env.get('DEFAULT_ARTIFACT_ENDPOINT', 'minio-service.kubeflow:9000')
+DEFAULT_ARTIFACT_ENDPOINT_SCHEME = env.get('DEFAULT_ARTIFACT_ENDPOINT_SCHEME', 'http://')
+
+
 def _get_super_condition_template():
 
   python_script = textwrap.dedent('''\
@@ -484,6 +490,9 @@ def _create_pipeline_workflow(self, args, pipeline, op_transformers=None, pipeli
         'annotations': {
           'tekton.dev/output_artifacts': json.dumps(self.output_artifacts, sort_keys=True),
           'tekton.dev/input_artifacts': json.dumps(self.input_artifacts, sort_keys=True),
+          'tekton.dev/artifact_bucket': DEFAULT_ARTIFACT_BUCKET,
+          'tekton.dev/artifact_endpoint': DEFAULT_ARTIFACT_ENDPOINT,
+          'tekton.dev/artifact_endpoint_scheme': DEFAULT_ARTIFACT_ENDPOINT_SCHEME,
           'sidecar.istio.io/inject': 'false'  # disable Istio inject since Tekton cannot run with Istio sidecar
         }
       },

diff --git a/sdk/python/tests/compiler/testdata/affinity.yaml b/sdk/python/tests/compiler/testdata/affinity.yaml
@@ -19,6 +19,9 @@ metadata:
     pipelines.kubeflow.org/pipeline_spec: '{"description": "A pipeline with affinity",
       "name": "affinity"}'
     sidecar.istio.io/inject: 'false'
+    tekton.dev/artifact_bucket: mlpipeline
+    tekton.dev/artifact_endpoint: minio-service.kubeflow:9000
+    tekton.dev/artifact_endpoint_scheme: http://
     tekton.dev/input_artifacts: '{}'
     tekton.dev/output_artifacts: '{}'
   name: affinity

diff --git a/sdk/python/tests/compiler/testdata/basic_no_decorator.yaml b/sdk/python/tests/compiler/testdata/basic_no_decorator.yaml
@@ -21,6 +21,9 @@ metadata:
       "message"}, {"default": "default_output", "name": "outputpath"}], "name": "Save
       Most Frequent Word"}'
     sidecar.istio.io/inject: 'false'
+    tekton.dev/artifact_bucket: mlpipeline
+    tekton.dev/artifact_endpoint: minio-service.kubeflow:9000
+    tekton.dev/artifact_endpoint_scheme: http://
     tekton.dev/input_artifacts: '{}'
     tekton.dev/output_artifacts: '{}'
   name: save-most-frequent-word

diff --git a/sdk/python/tests/compiler/testdata/big_data_passing.yaml b/sdk/python/tests/compiler/testdata/big_data_passing.yaml
@@ -18,6 +18,9 @@ metadata:
   annotations:
     pipelines.kubeflow.org/pipeline_spec: '{"name": "File passing pipelines"}'
     sidecar.istio.io/inject: 'false'
+    tekton.dev/artifact_bucket: mlpipeline
+    tekton.dev/artifact_endpoint: minio-service.kubeflow:9000
+    tekton.dev/artifact_endpoint_scheme: http://
     tekton.dev/input_artifacts: '{"print-params": [{"name": "gen-params-Output", "parent_task":
       "gen-params"}], "print-text": [{"name": "repeat-line-output_text", "parent_task":
       "repeat-line"}], "print-text-2": [{"name": "split-text-lines-odd_lines", "parent_task":
@@ -26,12 +29,15 @@ metadata:
       "parent_task": "write-numbers"}], "print-text-5": [{"name": "sum-numbers-Output",
       "parent_task": "sum-numbers"}], "sum-numbers": [{"name": "write-numbers-numbers",
       "parent_task": "write-numbers"}]}'
-    tekton.dev/output_artifacts: '{"gen-params": [{"name": "gen-params-Output", "path":
-      "/tmp/outputs/Output/data"}], "repeat-line": [{"name": "repeat-line-output_text",
-      "path": "/tmp/outputs/output_text/data"}], "split-text-lines": [{"name": "split-text-lines-even_lines",
-      "path": "/tmp/outputs/even_lines/data"}, {"name": "split-text-lines-odd_lines",
-      "path": "/tmp/outputs/odd_lines/data"}], "sum-numbers": [{"name": "sum-numbers-Output",
-      "path": "/tmp/outputs/Output/data"}], "write-numbers": [{"name": "write-numbers-numbers",
+    tekton.dev/output_artifacts: '{"gen-params": [{"key": "artifacts/$PIPELINERUN/gen-params/Output.tgz",
+      "name": "gen-params-Output", "path": "/tmp/outputs/Output/data"}], "repeat-line":
+      [{"key": "artifacts/$PIPELINERUN/repeat-line/output_text.tgz", "name": "repeat-line-output_text",
+      "path": "/tmp/outputs/output_text/data"}], "split-text-lines": [{"key": "artifacts/$PIPELINERUN/split-text-lines/even_lines.tgz",
+      "name": "split-text-lines-even_lines", "path": "/tmp/outputs/even_lines/data"},
+      {"key": "artifacts/$PIPELINERUN/split-text-lines/odd_lines.tgz", "name": "split-text-lines-odd_lines",
+      "path": "/tmp/outputs/odd_lines/data"}], "sum-numbers": [{"key": "artifacts/$PIPELINERUN/sum-numbers/Output.tgz",
+      "name": "sum-numbers-Output", "path": "/tmp/outputs/Output/data"}], "write-numbers":
+      [{"key": "artifacts/$PIPELINERUN/write-numbers/numbers.tgz", "name": "write-numbers-numbers",
       "path": "/tmp/outputs/numbers/data"}]}'
   name: file-passing-pipelines
 spec:
@@ -75,10 +81,18 @@ spec:
             valueFrom:
               fieldRef:
                 fieldPath: metadata.labels['tekton.dev/pipelineTask']
-          - name: NAMESPACE
+          - name: ARTIFACT_ENDPOINT_SCHEME
             valueFrom:
               fieldRef:
-                fieldPath: metadata.namespace
+                fieldPath: metadata.annotations['tekton.dev/artifact_endpoint_scheme']
+          - name: ARTIFACT_ENDPOINT
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.annotations['tekton.dev/artifact_endpoint']
+          - name: ARTIFACT_BUCKET
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.annotations['tekton.dev/artifact_bucket']
           - name: AWS_ACCESS_KEY_ID
             valueFrom:
               secretKeyRef:
@@ -93,12 +107,12 @@ spec:
           name: copy-artifacts
           script: '#!/usr/bin/env sh
 
-            mc config host add storage http://minio-service.$NAMESPACE:9000 $AWS_ACCESS_KEY_ID
-            $AWS_SECRET_ACCESS_KEY
+            mc config host add storage ${ARTIFACT_ENDPOINT_SCHEME}${ARTIFACT_ENDPOINT}
+            $AWS_ACCESS_KEY_ID $AWS_SECRET_ACCESS_KEY
 
             tar -cvzf output_text.tgz $(workspaces.repeat-line.path)/repeat-line-output_text
 
-            mc cp output_text.tgz storage/mlpipeline/artifacts/$PIPELINERUN/$PIPELINETASK/output_text.tgz
+            mc cp output_text.tgz storage/$ARTIFACT_BUCKET/artifacts/$PIPELINERUN/$PIPELINETASK/output_text.tgz
 
             '
         workspaces:
@@ -207,10 +221,18 @@ spec:
             valueFrom:
               fieldRef:
                 fieldPath: metadata.labels['tekton.dev/pipelineTask']
-          - name: NAMESPACE
+          - name: ARTIFACT_ENDPOINT_SCHEME
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.annotations['tekton.dev/artifact_endpoint_scheme']
+          - name: ARTIFACT_ENDPOINT
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.annotations['tekton.dev/artifact_endpoint']
+          - name: ARTIFACT_BUCKET
             valueFrom:
               fieldRef:
-                fieldPath: metadata.namespace
+                fieldPath: metadata.annotations['tekton.dev/artifact_bucket']
           - name: AWS_ACCESS_KEY_ID
             valueFrom:
               secretKeyRef:
@@ -225,16 +247,16 @@ spec:
           name: copy-artifacts
           script: '#!/usr/bin/env sh
 
-            mc config host add storage http://minio-service.$NAMESPACE:9000 $AWS_ACCESS_KEY_ID
-            $AWS_SECRET_ACCESS_KEY
+            mc config host add storage ${ARTIFACT_ENDPOINT_SCHEME}${ARTIFACT_ENDPOINT}
+            $AWS_ACCESS_KEY_ID $AWS_SECRET_ACCESS_KEY
 
             tar -cvzf even_lines.tgz $(workspaces.split-text-lines.path)/split-text-lines-even_lines
 
-            mc cp even_lines.tgz storage/mlpipeline/artifacts/$PIPELINERUN/$PIPELINETASK/even_lines.tgz
+            mc cp even_lines.tgz storage/$ARTIFACT_BUCKET/artifacts/$PIPELINERUN/$PIPELINETASK/even_lines.tgz
 
             tar -cvzf odd_lines.tgz $(workspaces.split-text-lines.path)/split-text-lines-odd_lines
 
-            mc cp odd_lines.tgz storage/mlpipeline/artifacts/$PIPELINERUN/$PIPELINETASK/odd_lines.tgz
+            mc cp odd_lines.tgz storage/$ARTIFACT_BUCKET/artifacts/$PIPELINERUN/$PIPELINETASK/odd_lines.tgz
 
             '
         volumes:
@@ -332,10 +354,18 @@ spec:
             valueFrom:
               fieldRef:
                 fieldPath: metadata.labels['tekton.dev/pipelineTask']
-          - name: NAMESPACE
+          - name: ARTIFACT_ENDPOINT_SCHEME
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.annotations['tekton.dev/artifact_endpoint_scheme']
+          - name: ARTIFACT_ENDPOINT
             valueFrom:
               fieldRef:
-                fieldPath: metadata.namespace
+                fieldPath: metadata.annotations['tekton.dev/artifact_endpoint']
+          - name: ARTIFACT_BUCKET
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.annotations['tekton.dev/artifact_bucket']
           - name: AWS_ACCESS_KEY_ID
             valueFrom:
               secretKeyRef:
@@ -350,12 +380,12 @@ spec:
           name: copy-artifacts
           script: '#!/usr/bin/env sh
 
-            mc config host add storage http://minio-service.$NAMESPACE:9000 $AWS_ACCESS_KEY_ID
-            $AWS_SECRET_ACCESS_KEY
+            mc config host add storage ${ARTIFACT_ENDPOINT_SCHEME}${ARTIFACT_ENDPOINT}
+            $AWS_ACCESS_KEY_ID $AWS_SECRET_ACCESS_KEY
 
             tar -cvzf numbers.tgz $(workspaces.write-numbers.path)/write-numbers-numbers
 
-            mc cp numbers.tgz storage/mlpipeline/artifacts/$PIPELINERUN/$PIPELINETASK/numbers.tgz
+            mc cp numbers.tgz storage/$ARTIFACT_BUCKET/artifacts/$PIPELINERUN/$PIPELINETASK/numbers.tgz
 
             '
         workspaces:
@@ -431,10 +461,18 @@ spec:
             valueFrom:
               fieldRef:
                 fieldPath: metadata.labels['tekton.dev/pipelineTask']
-          - name: NAMESPACE
+          - name: ARTIFACT_ENDPOINT_SCHEME
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.annotations['tekton.dev/artifact_endpoint_scheme']
+          - name: ARTIFACT_ENDPOINT
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.annotations['tekton.dev/artifact_endpoint']
+          - name: ARTIFACT_BUCKET
             valueFrom:
               fieldRef:
-                fieldPath: metadata.namespace
+                fieldPath: metadata.annotations['tekton.dev/artifact_bucket']
           - name: AWS_ACCESS_KEY_ID
             valueFrom:
               secretKeyRef:
@@ -449,12 +487,12 @@ spec:
           name: copy-artifacts
           script: '#!/usr/bin/env sh
 
-            mc config host add storage http://minio-service.$NAMESPACE:9000 $AWS_ACCESS_KEY_ID
-            $AWS_SECRET_ACCESS_KEY
+            mc config host add storage ${ARTIFACT_ENDPOINT_SCHEME}${ARTIFACT_ENDPOINT}
+            $AWS_ACCESS_KEY_ID $AWS_SECRET_ACCESS_KEY
 
             tar -cvzf Output.tgz $(workspaces.sum-numbers.path)/sum-numbers-Output
 
-            mc cp Output.tgz storage/mlpipeline/artifacts/$PIPELINERUN/$PIPELINETASK/Output.tgz
+            mc cp Output.tgz storage/$ARTIFACT_BUCKET/artifacts/$PIPELINERUN/$PIPELINETASK/Output.tgz
 
             '
         workspaces:
@@ -526,10 +564,18 @@ spec:
             valueFrom:
               fieldRef:
                 fieldPath: metadata.labels['tekton.dev/pipelineTask']
-          - name: NAMESPACE
+          - name: ARTIFACT_ENDPOINT_SCHEME
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.annotations['tekton.dev/artifact_endpoint_scheme']
+          - name: ARTIFACT_ENDPOINT
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.annotations['tekton.dev/artifact_endpoint']
+          - name: ARTIFACT_BUCKET
             valueFrom:
               fieldRef:
-                fieldPath: metadata.namespace
+                fieldPath: metadata.annotations['tekton.dev/artifact_bucket']
           - name: AWS_ACCESS_KEY_ID
             valueFrom:
               secretKeyRef:
@@ -544,12 +590,12 @@ spec:
           name: copy-artifacts
           script: '#!/usr/bin/env sh
 
-            mc config host add storage http://minio-service.$NAMESPACE:9000 $AWS_ACCESS_KEY_ID
-            $AWS_SECRET_ACCESS_KEY
+            mc config host add storage ${ARTIFACT_ENDPOINT_SCHEME}${ARTIFACT_ENDPOINT}
+            $AWS_ACCESS_KEY_ID $AWS_SECRET_ACCESS_KEY
 
             tar -cvzf Output.tgz $(results.output.path)
 
-            mc cp Output.tgz storage/mlpipeline/artifacts/$PIPELINERUN/$PIPELINETASK/Output.tgz
+            mc cp Output.tgz storage/$ARTIFACT_BUCKET/artifacts/$PIPELINERUN/$PIPELINETASK/Output.tgz
 
             '
     - name: print-params