Extend e2e-tests: Add resources tests #1342

elikatsis · 2019-05-16T10:16:17Z

I think we should start moving to v2.3 already by using the latest pre-release (v2.3.0-rc3). It comes with a lot of features and bug fixes.

This way:

Users will be able to fully exploit Extend the DSL to implement the design of #801 #926 and volume usage in general. See for example the open issue The volumes PVC name is unreasonable after mount a pvc for pipeline #1303, and how volume create sample fails with "{{inputs.parameters.create-pvc-name}}" not found #1327 was fulfilled once the images were updated. People seem to be looking forward to use it. Also,
we can develop artifacts even further using Alexey's addition (Added support for artifact path references argoproj/argo-workflows#1300),
we can introduce more parameters (e.g. again Alexey's Allow output parameters with .value, not only .valueFrom argoproj/argo-workflows#1336), which is something you have already expressed you could make use of, as I recall.

I'm sure there are more stuff we can take advantage of. But also, we can start deprecating stuff such as the global volumes attribute, to use template-local volumes instead.

I have been using this release extensively and it seems to be working fine.

I'm making this PR to run the E2E tests. I set the deployment scripts accordingly to deploy Argo v2.3.0-rc3.
Then, if all tests are successful, I suggest to update the kubeflow repo's argo deployment, since 0.1.20 release totally covers #1250.

WDYT?

/cc @Ark-kun @vicaire @hongye-sun

If everything works this PR closes #1250.

This change is

elikatsis · 2019-05-16T10:16:37Z

/ok-to-test

elikatsis · 2019-05-16T10:21:01Z

/test kubeflow-pipelines-e2e-test

elikatsis · 2019-05-16T10:27:34Z

/cc @IronPan @gaoning777

elikatsis · 2019-05-16T12:54:50Z

Due to pipeline-runner permissions, testing the samples fails.
I need to make some changes to the samples.
I.e. instead of creating a Secret in resourceop_basic.py I'm thinking of making it create a Pod, so that I don't change any other permissions.
[Edit: I'll keep the sample as is because it offers extra value for pvolumes usage.
I opened kubeflow/kubeflow#3294 to extend pipeline-runner's permissions]

Also, I have to cherry-pick kubeflow/kubeflow#2556 into pipelines branch (I made the PR against master back then).

elikatsis · 2019-05-16T13:22:16Z

Could you take a look at kubeflow/kubeflow#3293 ?

hongye-sun · 2019-05-16T18:15:29Z

@elikatsis, I'd suggest to change resourceop basic sample to demonstrate a more realistic use case like what argo sample does: create a k8s job. It doesn't seem right to expose cluster wide secret create/delete permissions for pipeline runner.

You may optionally demo pvolume usage by orchestrating it with a job spec, which will be very useful sample to answer question like #1345.

WDYT?

elikatsis · 2019-05-16T19:57:51Z

@hongye-sun
[I've continued the discussion on Secret permissions in kubeflow/kubeflow#3294.]

I can add a sample with some ContainerOps and a ResourceOp creating a Job, all of them using a PipelineVolume, but it would have to be a little complex and not really an example of "basic usage".
I believe the existing sample is just right to introduce users to the concept.

* Change Argo version deployed from v2.2.0 to v2.3.0-rc3 * Add resourceop test: Executes the resourceop_basic.py Extend run_basic_test.py with '--params' argument. * Add volumeop test: Executes the volumeop_sequential.py pipeline * Change modes from RWM to RWO for VolumeOp in volumeop_sequential.py * We cannot use some other sample with VolumeOps because GCP does not support the ReadWriteMany access mode. We also cannot use a VolumeSnapshotOp sample, because VolumeSnapshots only exist in Alpha GCP clusters. Signed-off-by: Ilias Katsakioris <[email protected]>

Signed-off-by: Ilias Katsakioris <[email protected]>

IronPan · 2019-05-22T17:03:40Z

test/deploy-kubeflow.sh

@@ -56,4 +56,11 @@ ${KUBEFLOW_SRC}/scripts/kfctl.sh apply platform
 ${KUBEFLOW_SRC}/scripts/kfctl.sh generate k8s
 ${KUBEFLOW_SRC}/scripts/kfctl.sh apply k8s

+pushd ks_app


do you think we can bump the version in kfctl instead?
https://github.com/kubeflow/kubeflow/blob/master/kubeflow/argo/prototypes/argo.jsonnet

Sure, I saw kubeflow/kubeflow#3335 so I pushed the removal of these lines, along with the corresponding change in test/install-argo.sh.

Let's merge this upon the aforementioned PR's approval. Would that be ok?

Signed-off-by: Ilias Katsakioris <[email protected]>

k8s-ci-robot · 2019-05-22T21:19:12Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hongye-sun
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: ark-kun

If they are not already assigned, you can assign the PR to them by writing /assign @ark-kun in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~samples/OWNERS~~ [hongye-sun]
test/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

elikatsis · 2019-05-23T07:14:48Z

/test kubeflow-pipeline-e2e-test

elikatsis · 2019-05-23T09:20:32Z

Would you mind checking the errors? It must be something regarding the deletion of the deployment.

hongye-sun · 2019-05-24T02:23:42Z

/retest

hongye-sun · 2019-05-24T02:27:35Z

@IronPan, do you know any known test issue recently?

Ark-kun · 2019-05-24T22:40:28Z

Would you mind checking the errors? It must be something regarding the deletion of the deployment.

Looks like leaking resources have used up all the quota.

/test kubeflow-pipeline-e2e-test

elikatsis · 2019-05-25T09:59:57Z

Cool, it seems it got fixed and the tests succeeded

elikatsis · 2019-05-25T12:37:13Z

@Ark-kun you said leaking resources, is there something we could do to the tests to prevent it?

Ark-kun · 2019-06-01T02:11:30Z

/test kubeflow-pipeline-sample-test

elikatsis · 2019-06-04T01:31:45Z

/hold
Until #1433 is merged

Ark-kun · 2019-08-01T01:04:15Z

/retest

Ark-kun · 2019-08-01T01:14:33Z

Generally LGTM, but the test probably belongs to sample tests. We've recently upgraded sample_test.yaml and it's now easier to add more tests.
/lgtm
/cc @gaoning777

k8s-ci-robot · 2019-08-01T01:14:34Z

@Ark-kun: GitHub didn't allow me to request PR reviews from the following users: gaoning777.

Note that only kubeflow members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Generally LGTM, but the test probably belongs to sample tests. We've recently upgraded sample_test.yaml and it's now easier to add more tests.
/lgtm
/cc @gaoning777

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2019-08-01T01:53:35Z

@elikatsis: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
kubeflow-pipeline-e2e-test	`105da34`	link	`/test kubeflow-pipeline-e2e-test`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

elikatsis · 2019-09-02T15:26:52Z

Closing this since testing has changed a lot.

I opened #2019 to cover a part of it and more PRs will follow.

k8s-ci-robot added the do-not-merge/work-in-progress label May 16, 2019

k8s-ci-robot requested a review from Ark-kun May 16, 2019 10:16

k8s-ci-robot added the do-not-merge/work-in-progress label May 16, 2019

k8s-ci-robot requested review from hongye-sun, vicaire and Ark-kun May 16, 2019 10:16

k8s-ci-robot added the size/M label May 16, 2019

k8s-ci-robot added the ok-to-test label May 16, 2019

elikatsis mentioned this pull request May 16, 2019

Issues on Argo 2.3 #1250

Closed

k8s-ci-robot requested review from gaoning777 and IronPan May 16, 2019 10:27

elikatsis force-pushed the resource-e2e-test branch 4 times, most recently from b324cd1 to c47e3a3 Compare May 16, 2019 11:42

elikatsis mentioned this pull request May 16, 2019

Add verb "create" for Secrets to ClusterRole "pipeline-runner" kubeflow/kubeflow#3294

Closed

elikatsis added 2 commits May 16, 2019 23:12

Comment out ResourceOp E2E test

c621a50

Signed-off-by: Ilias Katsakioris <[email protected]>

elikatsis force-pushed the resource-e2e-test branch from c47e3a3 to c621a50 Compare May 16, 2019 20:36

Ark-kun assigned IronPan May 18, 2019

IronPan reviewed May 22, 2019

View reviewed changes

Use Argo v2.3.0

cfd25af

Signed-off-by: Ilias Katsakioris <[email protected]>

k8s-ci-robot removed the lgtm label May 22, 2019

Set the sample paths according to kubeflow#1433

105da34

k8s-ci-robot added the do-not-merge/hold label Jun 4, 2019

Ark-kun mentioned this pull request Jul 31, 2019

Restructure samples #1710

Merged

Ark-kun unassigned vicaire Aug 1, 2019

k8s-ci-robot added the lgtm label Aug 1, 2019

Ark-kun assigned gaoning777 and numerology and unassigned Ark-kun Aug 3, 2019

elikatsis closed this Sep 2, 2019

elikatsis deleted the resource-e2e-test branch March 9, 2020 14:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend e2e-tests: Add resources tests #1342

Extend e2e-tests: Add resources tests #1342

elikatsis commented May 16, 2019 •

edited by jlewi

Loading

elikatsis commented May 16, 2019

elikatsis commented May 16, 2019

elikatsis commented May 16, 2019

elikatsis commented May 16, 2019 •

edited

Loading

elikatsis commented May 16, 2019

hongye-sun commented May 16, 2019

elikatsis commented May 16, 2019

IronPan May 22, 2019

elikatsis May 22, 2019

k8s-ci-robot commented May 22, 2019

elikatsis commented May 23, 2019

elikatsis commented May 23, 2019

hongye-sun commented May 24, 2019

hongye-sun commented May 24, 2019

Ark-kun commented May 24, 2019

elikatsis commented May 25, 2019

elikatsis commented May 25, 2019

Ark-kun commented Jun 1, 2019

elikatsis commented Jun 4, 2019

Ark-kun commented Aug 1, 2019

Ark-kun commented Aug 1, 2019

k8s-ci-robot commented Aug 1, 2019

k8s-ci-robot commented Aug 1, 2019 •

edited

Loading

elikatsis commented Sep 2, 2019

Extend e2e-tests: Add resources tests #1342

Extend e2e-tests: Add resources tests #1342

Conversation

elikatsis commented May 16, 2019 • edited by jlewi Loading

elikatsis commented May 16, 2019

elikatsis commented May 16, 2019

elikatsis commented May 16, 2019

elikatsis commented May 16, 2019 • edited Loading

elikatsis commented May 16, 2019

hongye-sun commented May 16, 2019

elikatsis commented May 16, 2019

IronPan May 22, 2019

Choose a reason for hiding this comment

elikatsis May 22, 2019

Choose a reason for hiding this comment

k8s-ci-robot commented May 22, 2019

elikatsis commented May 23, 2019

elikatsis commented May 23, 2019

hongye-sun commented May 24, 2019

hongye-sun commented May 24, 2019

Ark-kun commented May 24, 2019

elikatsis commented May 25, 2019

elikatsis commented May 25, 2019

Ark-kun commented Jun 1, 2019

elikatsis commented Jun 4, 2019

Ark-kun commented Aug 1, 2019

Ark-kun commented Aug 1, 2019

k8s-ci-robot commented Aug 1, 2019

k8s-ci-robot commented Aug 1, 2019 • edited Loading

elikatsis commented Sep 2, 2019

elikatsis commented May 16, 2019 •

edited by jlewi

Loading

elikatsis commented May 16, 2019 •

edited

Loading

k8s-ci-robot commented Aug 1, 2019 •

edited

Loading