SDK - Compiler - Move Argo volume specifications to templates (#2229)

* SDK - Compiler - Move volumes to templates

Argo v2.3.0+ supports per-template volume specs similiar to Kubernetes. Prior to version 2.3.0 Argo only supported workflow-level volume specs.
We had several outstanding issues caused by the need to put all volumes in the same place.
There was also the issue with input parameter reference placeholders in volume specifications which were placed outside their home templates declaring the inputs.

 This change fixes those issues.

* Removed dead code line
This commit is contained in:
Alexey Volkov 2019-10-07 16:55:12 -07:00 committed by Kubernetes Prow Robot
parent 4f65528050
commit 181de66cf9
12 changed files with 85 additions and 77 deletions

View File

@ -28,7 +28,6 @@ def fix_big_data_passing(workflow: dict) -> dict:
workflow = copy.deepcopy(workflow)
templates = workflow['spec']['templates']
volume_map = {volume['name']: volume for volume in workflow['spec'].get('volumes', [])}
container_templates = [template for template in workflow['spec']['templates'] if 'container' in template]
dag_templates = [template for template in workflow['spec']['templates'] if 'dag' in template]
@ -157,20 +156,6 @@ def fix_big_data_passing(workflow: dict) -> dict:
# Searching for parameter input consumers in container and resource templates
for template in container_templates + resource_templates:
# Hack for volumes: Prior to Argo 2.3.0, it was only possible to add volumes globally on the workflow level instead of at template level.
# When VolumeOps were added, the generated structures became hacky since the workflow-level volumes cound now contain template-level input references.
# Here we virtually add volumes back to the template to analyze the input references.
# To properly fix this the compiler should move volumes to templates.
# TODO: Fix the compiler and remove this hack
if 'container' in template:
template = copy.deepcopy(template)
template_volumes = template.setdefault('volumes', [])
for volume_mount in template['container'].get('volumeMounts', []):
volume_name = volume_mount['name']
if volume_name in volume_map:
template_volumes.append(volume_map[volume_name])
# End hack
template_name = template['name']
placeholders = extract_all_placeholders(template)
for placeholder in placeholders:

View File

@ -277,6 +277,11 @@ def _op_to_template(op: BaseOp):
if processed_op.sidecars:
template['sidecars'] = processed_op.sidecars
# volumes
if processed_op.volumes:
template['volumes'] = [K8sHelper.convert_k8s_obj_to_json(volume) for volume in processed_op.volumes]
template['volumes'].sort(key=lambda x: x['name'])
# Display name
if processed_op.display_name:
template.setdefault('metadata', {}).setdefault('annotations', {})['pipelines.kubeflow.org/task_display_name'] = processed_op.display_name

View File

@ -594,21 +594,6 @@ class Compiler(object):
return templates
def _create_volumes(self, pipeline):
"""Create volumes required for the templates"""
volumes = []
volume_name_set = set()
for op in pipeline.ops.values():
if op.volumes:
for v in op.volumes:
# Remove volume duplicates which have the same name
#TODO: check for duplicity based on the serialized volumes instead of just name.
if v['name'] not in volume_name_set:
volume_name_set.add(v['name'])
volumes.append(v)
volumes.sort(key=lambda x: x['name'])
return volumes
def _create_pipeline_workflow(self, args, pipeline, op_transformers=None, pipeline_conf=None):
"""Create workflow for the pipeline."""
@ -634,9 +619,6 @@ class Compiler(object):
if first_group.type == 'exit_handler':
exit_handler = first_group.exit_op
# Volumes
volumes = self._create_volumes(pipeline)
# The whole pipeline workflow
pipeline_name = pipeline.name or 'Pipeline'
workflow = {
@ -665,8 +647,6 @@ class Compiler(object):
if exit_handler:
workflow['spec']['onExit'] = exit_handler.name
if volumes:
workflow['spec']['volumes'] = volumes
return workflow
def _validate_exit_handler(self, pipeline):

View File

@ -23,6 +23,10 @@ spec:
parameters:
- name: create-volume-name
name: cop
volumes:
- name: create-volume
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-volume-name}}'
- name: create-volume
outputs:
parameters:
@ -53,7 +57,3 @@ spec:
- name: create-volume
template: create-volume
name: param-substitutions
volumes:
- name: create-volume
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-volume-name}}'

View File

@ -27,6 +27,10 @@ spec:
parameters:
- name: create-my-secret-name
name: cop
volumes:
- name: my-secret
secret:
secretName: '{{inputs.parameters.create-my-secret-name}}'
- inputs:
parameters:
- name: password
@ -68,7 +72,3 @@ spec:
- name: password
- name: username
name: resourceop-basic
volumes:
- name: my-secret
secret:
secretName: '{{inputs.parameters.create-my-secret-name}}'

View File

@ -48,6 +48,10 @@ spec:
- name: download-downloaded
valueFrom:
path: /tmp/results.txt
volumes:
- name: gcp-credentials
secret:
secretName: user-gcp-sa
- container:
args:
- echo {{inputs.parameters.download-downloaded}}
@ -72,7 +76,3 @@ spec:
name: echo
template: echo
name: volume
volumes:
- name: gcp-credentials
secret:
secretName: user-gcp-sa

View File

@ -133,6 +133,10 @@ spec:
parameters:
- name: create-volume-1-name
name: step1-concat
volumes:
- name: create-volume-1
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-volume-1-name}}'
- container:
command:
- gunzip
@ -146,6 +150,10 @@ spec:
parameters:
- name: create-volume-2-name
name: step2-gunzip
volumes:
- name: create-volume-2
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-volume-2-name}}'
- container:
command:
- cat
@ -158,6 +166,10 @@ spec:
parameters:
- name: create-volume-3-name
name: step3-output
volumes:
- name: create-volume-3
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-volume-3-name}}'
- dag:
tasks:
- arguments:
@ -232,13 +244,3 @@ spec:
parameters:
- name: rok-url
name: volumesnapshotop-rokurl
volumes:
- name: create-volume-1
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-volume-1-name}}'
- name: create-volume-2
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-volume-2-name}}'
- name: create-volume-3
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-volume-3-name}}'

View File

@ -44,6 +44,10 @@ spec:
- name: create-volume-name
- name: url
name: step1-ingest
volumes:
- name: create-volume
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-volume-name}}'
- inputs:
parameters:
- name: create-volume-name
@ -79,6 +83,10 @@ spec:
parameters:
- name: create-volume-name
name: step2-gunzip
volumes:
- name: create-volume
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-volume-name}}'
- inputs:
parameters:
- name: create-volume-name
@ -114,6 +122,10 @@ spec:
parameters:
- name: create-volume-name
name: step3-copy
volumes:
- name: create-volume
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-volume-name}}'
- inputs:
parameters:
- name: create-volume-name
@ -148,6 +160,10 @@ spec:
parameters:
- name: create-volume-name
name: step4-output
volumes:
- name: create-volume
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-volume-name}}'
- dag:
tasks:
- name: create-volume
@ -220,7 +236,3 @@ spec:
parameters:
- name: url
name: volumesnapshotop-sequential
volumes:
- name: create-volume
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-volume-name}}'

View File

@ -26,6 +26,10 @@ spec:
parameters:
- name: create-pvc-name
name: cop
volumes:
- name: create-pvc
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-pvc-name}}'
- inputs:
parameters:
- name: size
@ -66,7 +70,3 @@ spec:
parameters:
- name: size
name: volumeop-basic
volumes:
- name: create-pvc
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-pvc-name}}'

View File

@ -42,6 +42,10 @@ spec:
parameters:
- name: create-pvc-name
name: step1
volumes:
- name: create-pvc
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-pvc-name}}'
- container:
args:
- echo 2 | tee /mnt2/file2
@ -56,6 +60,10 @@ spec:
parameters:
- name: create-pvc-name
name: step2
volumes:
- name: create-pvc
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-pvc-name}}'
- container:
args:
- cat /mnt/file1 /mnt/file2
@ -70,6 +78,10 @@ spec:
parameters:
- name: create-pvc-name
name: step3
volumes:
- name: create-pvc
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-pvc-name}}'
- dag:
tasks:
- name: create-pvc
@ -101,7 +113,3 @@ spec:
name: step3
template: step3
name: volume-op-dag
volumes:
- name: create-pvc
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-pvc-name}}'

View File

@ -42,6 +42,10 @@ spec:
parameters:
- name: create-pvc-name
name: step1
volumes:
- name: create-pvc
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-pvc-name}}'
- container:
args:
- echo 2 | tee /common/file2
@ -56,6 +60,10 @@ spec:
parameters:
- name: create-pvc-name
name: step2
volumes:
- name: create-pvc
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-pvc-name}}'
- container:
args:
- echo 3 | tee /mnt3/file3
@ -70,6 +78,10 @@ spec:
parameters:
- name: create-pvc-name
name: step3
volumes:
- name: create-pvc
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-pvc-name}}'
- dag:
tasks:
- name: create-pvc
@ -99,7 +111,3 @@ spec:
name: step3
template: step3
name: volumeop-parallel
volumes:
- name: create-pvc
persistentVolumeClaim:
claimName: '{{inputs.parameters.create-pvc-name}}'

View File

@ -42,6 +42,10 @@ spec:
parameters:
- name: mypvc-name
name: step1
volumes:
- name: mypvc
persistentVolumeClaim:
claimName: '{{inputs.parameters.mypvc-name}}'
- container:
args:
- cp /data/file1 /data/file2
@ -56,6 +60,10 @@ spec:
parameters:
- name: mypvc-name
name: step2
volumes:
- name: mypvc
persistentVolumeClaim:
claimName: '{{inputs.parameters.mypvc-name}}'
- container:
command:
- cat
@ -69,6 +77,10 @@ spec:
parameters:
- name: mypvc-name
name: step3
volumes:
- name: mypvc
persistentVolumeClaim:
claimName: '{{inputs.parameters.mypvc-name}}'
- dag:
tasks:
- name: mypvc
@ -100,7 +112,3 @@ spec:
name: step3
template: step3
name: volumeop-sequential
volumes:
- name: mypvc
persistentVolumeClaim:
claimName: '{{inputs.parameters.mypvc-name}}'