Skip to content

Commit

Permalink
Extend the DSL to implement the design of #801 (#926)
Browse files Browse the repository at this point in the history
* SDK: Create BaseOp class

* BaseOp class is the base class for any Argo Template type
* ContainerOp derives from BaseOp
* Rename dependent_names to deps

Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>

* SDK: In preparation for the new feature ResourceOps (#801)

* Add cops attributes to Pipeline. This is a dict having all the
  ContainerOps of the pipeline.
* Set some processing in _op_to_template as ContainerOp specific

Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>

* SDK: Simplify the consumption of Volumes by ContainerOps

Add `pvolumes` argument and attribute to ContainerOp. It is a dict
having mount paths as keys and V1Volumes as values. These are added to
the pipeline and mounted by the container of the ContainerOp.

Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>

* SDK: Add ResourceOp

* ResourceOp is the SDK's equivalent for Argo's resource template
* Add rops attribute to Pipeline: Dictionary containing ResourceOps
* Extend _op_to_template to produce the template for ResourceOps
* Use processed_op instead of op everywhere in _op_to_template()
* Add samples/resourceop/resourceop_basic.py
* Add tests/dsl/resource_op_tests.py
* Extend tests/compiler/compiler_tests.py

Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>

* SDK: Simplify the creation of PersistentVolumeClaim instances

* Add VolumeOp: A specified ResourceOp for PVC creation
* Add samples/resourceops/volumeop_basic.py
* Add tests/dsl/volume_op_tests.py
* Extend tests/compiler/compiler_tests.py

Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>

* SDK: Emit a V1Volume as `.volume` from dsl.VolumeOp

* Extend VolumeOp so it outputs a `.volume` attribute ready to be
  consumed by the `pvolumes` argument to ContainerOp's constructor
* Update samples/resourceop/volumeop_basic.py
* Extend tests/dsl/volume_op_tests.py
* Update tests/compiler/compiler_tests.py

Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>

* SDK: Add PipelineVolume

* PipelineVolume inherits from V1Volume and it comes with its own set of
  KFP-specific dependencies. It is aligned with how PipelineParam
  instances are used. I.e. consuming a PipelineVolume leads to implicit
  dependencies without the user having to call the `.after()` method on
  a ContainerOp.
* PipelineVolume comes with its own `.after()` method, which can be used
  to append extra dependencies to the instance.
* Extend ContainerOp to handle PipelineVolume deps
* Set `.volume` attribute of VolumeOp to be a PipelineVolume instead
* Add samples/resourceops/volumeop_{parallel,dag,sequential}.py
* Fix tests/dsl/volume_op_tests.py
* Add tests/dsl/pipeline_volume_tests.py
* Extend tests/compiler/compiler_tests.py

Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>

* SDK: Simplify the creation of VolumeSnapshot instances

* VolumeSnapshotOp: A specified ResourceOp for VolumeSnapshot creation
* Add samples/resourceops/volume_snapshotop_{sequential,rokurl}.py
* Add tests/dsl/volume_snapshotop_tests.py
* Extend tests/compiler/compiler_tests.py

NOTE: VolumeSnapshots is an Alpha feature at the time of this commit.

Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>

* Extend UI for the ResourceOp and Volumes feature of the Compiler

* Add VolumeMounts tab/entry (Run/Pipeline view)
* Add Manifest tab/entry (Run/Pipeline view)
* Add & Extend tests
* Update tests snapshot files

Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>

* Cleaning up the diff (before moving things back)

* Renamed op.deps back to op.dependent_names

* Moved Container, Sidecar and BaseOp classed back to _container_op.py
This way the diff is much smaller and more understandable. We can always split or refactor the file later. Refactorings should not be mixed with genuine changes.
  • Loading branch information
elikatsis authored and k8s-ci-robot committed Apr 25, 2019
1 parent c9d3d39 commit 07cb50e
Show file tree
Hide file tree
Showing 47 changed files with 4,494 additions and 267 deletions.
38 changes: 27 additions & 11 deletions frontend/src/components/StaticNodeDetails.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018 Google LLC
* Copyright 2018-2019 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -20,7 +20,7 @@ import { classes, stylesheet } from 'typestyle';
import { commonCss, fontsize } from '../Css';
import { SelectedNodeInfo } from '../lib/StaticGraphParser';

export type nodeType = 'container' | 'dag' | 'unknown';
export type nodeType = 'container' | 'resource' | 'dag' | 'unknown';

const css = stylesheet({
fontSizeTitle: {
Expand All @@ -42,19 +42,35 @@ class StaticNodeDetails extends React.Component<StaticNodeDetailsProps> {
const nodeInfo = this.props.nodeInfo;

return <div>
<DetailsTable title='Input parameters' fields={nodeInfo.inputs} />
{(nodeInfo.nodeType === 'container') && (
<div>
<DetailsTable title='Input parameters' fields={nodeInfo.inputs} />

<DetailsTable title='Output parameters' fields={nodeInfo.outputs} />
<DetailsTable title='Output parameters' fields={nodeInfo.outputs} />

<div className={classes(commonCss.header, css.fontSizeTitle)}>Arguments</div>
{nodeInfo.args.map((arg, i) =>
<div key={i} style={{ fontFamily: 'monospace' }}>{arg}</div>)}
<div className={classes(commonCss.header, css.fontSizeTitle)}>Arguments</div>
{nodeInfo.args.map((arg, i) =>
<div key={i} style={{ fontFamily: 'monospace' }}>{arg}</div>)}

<div className={classes(commonCss.header, css.fontSizeTitle)}>Command</div>
{nodeInfo.command.map((c, i) => <div key={i} style={{ fontFamily: 'monospace' }}>{c}</div>)}
<div className={classes(commonCss.header, css.fontSizeTitle)}>Command</div>
{nodeInfo.command.map((c, i) => <div key={i} style={{ fontFamily: 'monospace' }}>{c}</div>)}

<div className={classes(commonCss.header, css.fontSizeTitle)}>Image</div>
<div style={{ fontFamily: 'monospace' }}>{nodeInfo.image}</div>
<div className={classes(commonCss.header, css.fontSizeTitle)}>Image</div>
<div style={{ fontFamily: 'monospace' }}>{nodeInfo.image}</div>

<DetailsTable title='Volume Mounts' fields={nodeInfo.volumeMounts} />
</div>
)}

{(nodeInfo.nodeType === 'resource') && (
<div>
<DetailsTable title='Input parameters' fields={nodeInfo.inputs} />

<DetailsTable title='Output parameters' fields={nodeInfo.outputs} />

<DetailsTable title='Manifest' fields={nodeInfo.resource} />
</div>
)}

{!!nodeInfo.condition && (
<div>
Expand Down
196 changes: 190 additions & 6 deletions frontend/src/lib/StaticGraphParser.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018 Google LLC
* Copyright 2018-2019 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -76,6 +76,52 @@ describe('StaticGraphParser', () => {
};
}

function newResourceCreatingWorkflow(): any {
return {
spec: {
entrypoint: 'template-1',
templates: [
{
dag: {
tasks: [
{ name: 'create-pvc-task', template: 'create-pvc' },
{
dependencies: ['create-pvc-task'],
name: 'container-1',
template: 'container-1',
},
{
dependencies: ['container-1'],
name: 'create-snapshot-task',
template: 'create-snapshot',
},
]
},
name: 'template-1',
},
{
name: 'create-pvc',
resource: {
action: 'create',
manifest: 'apiVersion: v1\nkind: PersistentVolumeClaim',
},
},
{
container: {},
name: 'container-1',
},
{
name: 'create-snapshot',
resource: {
action: 'create',
manifest: 'apiVersion: snapshot.storage.k8s.io/v1alpha1\nkind: VolumeSnapshot',
},
},
]
}
};
}

describe('createGraph', () => {
it('creates a single node with no edges for a workflow with one step.', () => {
const workflow = newWorkflow();
Expand Down Expand Up @@ -198,6 +244,18 @@ describe('StaticGraphParser', () => {
});
});

it('includes the resource\'s action and manifest itself in the info of resource nodes', () => {
const g = createGraph(newResourceCreatingWorkflow());
g.nodes().forEach((nodeName) => {
const node = g.node(nodeName);
if (nodeName.startsWith('create-pvc')) {
expect(node.info.resource).toEqual([['create', 'apiVersion: v1\nkind: PersistentVolumeClaim']]);
} else if (nodeName.startsWith('create-snapshot')) {
expect(node.info.resource).toEqual([['create', 'apiVersion: snapshot.storage.k8s.io\nkind: VolumeSnapshot']]);
}
});
});

it('renders extremely simple workflows with no steps or DAGs', () => {
const simpleWorkflow = {
spec: {
Expand Down Expand Up @@ -392,12 +450,13 @@ describe('StaticGraphParser', () => {
expect(nodeInfo).toEqual(defaultSelectedNodeInfo);
});

it('returns nodeInfo with empty values for args, command, and/or image if container does not have them', () => {
it('returns nodeInfo of a container with empty values for args, command, image and/or volumeMounts if container does not have them', () => {
const template = {
container: {
// No args
// No command
// No image
// No volumeMounts
},
dag: [],
name: 'template-1',
Expand All @@ -407,6 +466,7 @@ describe('StaticGraphParser', () => {
expect(nodeInfo.args).toEqual([]);
expect(nodeInfo.command).toEqual([]);
expect(nodeInfo.image).toEqual('');
expect(nodeInfo.volumeMounts).toEqual([]);
});


Expand Down Expand Up @@ -449,7 +509,48 @@ describe('StaticGraphParser', () => {
expect(nodeInfo.image).toEqual('some-image');
});

it('returns nodeInfo with empty values if template does not have inputs and/or outputs', () => {
it('returns nodeInfo containing container volumeMounts', () => {
const template = {
container: {
volumeMounts: [{'mountPath': '/some/path', 'name': 'some-vol'}]
},
dag: [],
name: 'template-1',
} as any;
const nodeInfo = _populateInfoFromTemplate(new SelectedNodeInfo(), template);
expect(nodeInfo.nodeType).toEqual('container');
expect(nodeInfo.volumeMounts).toEqual([['/some/path', 'some-vol']]);
});

it('returns nodeInfo of a resource with empty values for action and manifest', () => {
const template = {
dag: [],
name: 'template-1',
resource: {
// No action
// No manifest
},
} as any;
const nodeInfo = _populateInfoFromTemplate(new SelectedNodeInfo(), template);
expect(nodeInfo.nodeType).toEqual('resource');
expect(nodeInfo.resource).toEqual([[]]);
});

it('returns nodeInfo containing resource action and manifest', () => {
const template = {
dag: [],
name: 'template-1',
resource: {
action: 'create',
manifest: 'manifest'
},
} as any;
const nodeInfo = _populateInfoFromTemplate(new SelectedNodeInfo(), template);
expect(nodeInfo.nodeType).toEqual('resource');
expect(nodeInfo.resource).toEqual([['create', 'manifest']]);
});

it('returns nodeInfo of a container with empty values if template does not have inputs and/or outputs', () => {
const template = {
container: {},
dag: [],
Expand All @@ -463,7 +564,7 @@ describe('StaticGraphParser', () => {
expect(nodeInfo.outputs).toEqual([[]]);
});

it('returns nodeInfo containing template inputs params as list of name/value tuples', () => {
it('returns nodeInfo of a container containing template inputs params as list of name/value tuples', () => {
const template = {
container: {},
dag: [],
Expand All @@ -477,7 +578,7 @@ describe('StaticGraphParser', () => {
expect(nodeInfo.inputs).toEqual([['param1', 'val1'], ['param2', 'val2']]);
});

it('returns empty strings for inputs with no specified value', () => {
it('returns nodeInfo of a container with empty strings for inputs with no specified value', () => {
const template = {
container: {},
dag: [],
Expand Down Expand Up @@ -516,7 +617,7 @@ describe('StaticGraphParser', () => {
]);
});

it('returns empty strings for outputs with no specified value', () => {
it('returns nodeInfo of a container with empty strings for outputs with no specified value', () => {
const template = {
container: {},
name: 'template-1',
Expand All @@ -532,6 +633,89 @@ describe('StaticGraphParser', () => {
expect(nodeInfo.outputs).toEqual([['param1', ''], ['param2', '']]);
});

it('returns nodeInfo of a resource with empty values if template does not have inputs and/or outputs', () => {
const template = {
dag: [],
// No inputs
// No outputs
name: 'template-1',
resource: {},
} as any;
const nodeInfo = _populateInfoFromTemplate(new SelectedNodeInfo(), template);
expect(nodeInfo.nodeType).toEqual('resource');
expect(nodeInfo.inputs).toEqual([[]]);
expect(nodeInfo.outputs).toEqual([[]]);
});

it('returns nodeInfo of a resource containing template inputs params as list of name/value tuples', () => {
const template = {
dag: [],
inputs: {
parameters: [{ name: 'param1', value: 'val1' }, { name: 'param2', value: 'val2' }]
},
name: 'template-1',
resource: {},
} as any;
const nodeInfo = _populateInfoFromTemplate(new SelectedNodeInfo(), template);
expect(nodeInfo.nodeType).toEqual('resource');
expect(nodeInfo.inputs).toEqual([['param1', 'val1'], ['param2', 'val2']]);
});

it('returns nodeInfo of a resource with empty strings for inputs with no specified value', () => {
const template = {
dag: [],
inputs: {
parameters: [{ name: 'param1' }, { name: 'param2' }]
},
name: 'template-1',
resource: {},
} as any;
const nodeInfo = _populateInfoFromTemplate(new SelectedNodeInfo(), template);
expect(nodeInfo.nodeType).toEqual('resource');
expect(nodeInfo.inputs).toEqual([['param1', ''], ['param2', '']]);
});

it('returns nodeInfo containing resource outputs as list of name/value tuples, pulling from valueFrom if necessary', () => {
const template = {
name: 'template-1',
outputs: {
parameters: [
{ name: 'param1', value: 'val1' },
{ name: 'param2', valueFrom: { jsonPath: 'jsonPath' } },
{ name: 'param3', valueFrom: { path: 'path' } },
{ name: 'param4', valueFrom: { parameter: 'parameterReference' } },
{ name: 'param5', valueFrom: { jqFilter: 'jqFilter' } },
],
},
resource: {},
} as any;
const nodeInfo = _populateInfoFromTemplate(new SelectedNodeInfo(), template);
expect(nodeInfo.nodeType).toEqual('resource');
expect(nodeInfo.outputs).toEqual([
['param1', 'val1'],
['param2', 'jsonPath'],
['param3', 'path'],
['param4', 'parameterReference'],
['param5', 'jqFilter'],
]);
});

it('returns nodeInfo of a resource with empty strings for outputs with no specified value', () => {
const template = {
name: 'template-1',
outputs: {
parameters: [
{ name: 'param1' },
{ name: 'param2' },
],
},
resource: {},
} as any;
const nodeInfo = _populateInfoFromTemplate(new SelectedNodeInfo(), template);
expect(nodeInfo.nodeType).toEqual('resource');
expect(nodeInfo.outputs).toEqual([['param1', ''], ['param2', '']]);
});

});
});

Loading

0 comments on commit 07cb50e

Please sign in to comment.