fix(backend): updated the argo version too 2.7.7. Fixes #4392 #4498

NikeNano · 2020-09-15T07:35:04Z

Description of your changes:
Fixes #4392

Checklist:

The title for your pull request (PR) should follow our title convention. Learn more about the pull request title convention used in this repository.

Do you want this pull request (PR) cherry-picked into the current release branch?

Learn more about cherry-picking updates into the release branch.

kubeflow-bot · 2020-09-15T07:35:10Z

This change is

Ark-kun · 2020-09-15T07:39:53Z

go.mod

 	github.com/Masterminds/squirrel v0.0.0-20190107164353-fa735ea14f09
 	github.com/VividCortex/mysqlerr v0.0.0-20170204212430-6c6b55f8796f
-	github.com/argoproj/argo v2.3.0+incompatible
+	github.com/argoproj/argo v0.0.0-20200506223611-54154c61eb4f


Is it possible to use a release version here?

Not sure, will check if possible.

The answer seems to be no, @Ark-kun argoproj/argo-workflows#2602 (comment)

I meant to ask whether it' possible to refer to particular GIT tag like github.com/argoproj/argo v2.7.5...

I will try to clean this up after I fixed some of the bugs :)

I don't get it to work to specify the version as you mention @Ark-kun, agree that it would be better to have it with version.

Bobgy · 2020-09-16T04:20:33Z

This would be great!

Can you fix the build failure?
https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubeflow_pipelines/4498/kubeflow-pipeline-e2e-test/1305856181247414272#1:build-log.txt%3A4191

Ark-kun · 2020-09-19T06:48:37Z

backend/src/apiserver/resource/resource_manager_util_test.go

@@ -263,7 +263,8 @@ spec:
  arguments: {}
  entrypoint: rand-fail-dag
  templates:
-  - dag:
+  - arguments: {}


Hmm. I thought this Argo issue was fixed at some point...
Update: Turns out, not fully argoproj/argo-workflows#1686

NikeNano · 2020-09-19T07:25:23Z

Can you fix the build failure?
https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubeflow_pipelines/4498/kubeflow-pipeline-e2e-test/1305856181247414272#1:build-log.txt%3A4191

Working on it :)

NikeNano · 2020-09-21T09:30:52Z

/test kubeflow-pipeline-backend-test

NikeNano · 2020-09-22T12:45:45Z

I have been manage to fix some of the issues but still getting issues with the integration tests according to automatic tests. When runing the tests locally using bazel, I get the following issue:

ERROR: /workspaces/pipelines/backend/src/crd/pkg/client/clientset/versioned/typed/scheduledworkflow/v1beta1/BUILD.bazel:3:1: GoCompile backend/src/crd/pkg/client/clientset/versioned/typed/scheduledworkflow/v1beta1/linux_amd64_stripped/go_default_library%/github.com/kubeflow/pipelines/backend/src/crd/pkg/client/clientset/versioned/typed/scheduledworkflow/v1beta1.a failed (Exit 1) compile failed: error executing command bazel-out/host/bin/external/io_bazel_rules_go/go/tools/builders/linux_amd64_stripped/compile -sdk external/go_sdk -installsuffix linux_amd64 -src ... (remaining 30 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
GoCompile: error running compiler: exit status 2
/root/.cache/bazel/_bazel_root/595e728d121f60f59a14887cb2b2325a/sandbox/processwrapper-sandbox/153/execroot/__main__/backend/src/crd/pkg/client/clientset/versioned/typed/scheduledworkflow/v1beta1/scheduledworkflow_client.go:70:45: scheme.Codecs.WithoutConversion undefined (type serializer.CodecFactory has no field or method WithoutConversion)

As I understand this is related to the CRD client, however when trying to update it seems like the file for updating the API, generate-groups.sh is not available in the repo any longer

pipelines/backend/src/crd/hack/update-codegen.sh

Line 29 in 1802db3

${CODEGEN_PKG}/generate-groups.sh "deepcopy" \

Is this still the supported way to update the client? I am a bit on deep water would be great if you could point me in the right direction @neuromage, @Bobgy

Bobgy · 2020-09-23T12:42:59Z

@NikeNano Great progress!

For

pipelines/backend/src/crd/hack/update-codegen.sh

Line 26 in 1802db3

CODEGEN_PKG=${SCRIPT_ROOT}/../../../../vendor/k8s.io/code-generator

I think you can run go mod vendor to put all go modules in a vendor folder, then the script should be available there. We are calling k8s.io/code-generator this go module to generate code.

However, I believe no one has tried to rerun it again for a while, so it's totally possible something will fail. Feel free to report it back here.

NikeNano · 2020-09-25T08:32:53Z

From the main folder(pipelines) I run:

./hack/update-codegen.sh

Inside vendor/k8s.io/ I have the following folders:

api/                     
apimachinery/            
klog/
kubernetes/              
apiextensions-apiserver/
client-go/
kube-openapi/            
utils/

and thus it seems like the vendor/k8s.io/code-generator/generate-groups.sh script is not generate @Bobgy.

google-cla · 2020-10-15T20:08:34Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

Bobgy · 2020-10-22T10:09:49Z

Thank you so much! All looking good now!
I've left enough time for others to review.
Let's get this merged now.

/lgtm
/approve

Bobgy · 2020-10-22T10:10:17Z

I think you used a different email account to do one of the commits, overriding the cla for you.

Bobgy · 2020-10-22T10:11:49Z

By the way, is this argo client 2.7.5?
Can you update that to the PR title?

Bobgy · 2020-10-22T10:12:03Z

/hold
You can unhold after updating the title

NikeNano · 2020-10-22T13:32:02Z

/unhold

By the way, is this argo client 2.7.5?
Can you update that to the PR title?

The argo package is updated to version 2.7.7 removed client from the name of the PR as well.

Bobgy · 2020-10-22T23:08:06Z

/approve

k8s-ci-robot · 2020-10-22T23:08:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bobgy, NikeNano

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:

~~OWNERS~~ [Bobgy]

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

Argo supports a field in the ResourceTemplate that makes the controller add an owner reference of the workflow to the created resource since v2.4.0 [1]. With the upgrade of Argo client [2] and deployment [3] we are now able to exploit it. We set it to 'false' by default on all ResourceOps (actually, leave it empty) and 'true' for VolumeOps. This allows the garbage collection of PVCs upon workflow cleanup without any further change [4]. [1] https://github.com/argoproj/argo/blob/v2.4.0/pkg/apis/workflow/v1alpha1/workflow_types.go#L1044-L1045 [2] kubeflow#4498 [3] kubeflow#3537 [4] kubeflow#1779 Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>

…ow#1779 Argo supports a field in the ResourceTemplate that makes the controller add an owner reference of the workflow to the created resource since v2.4.0 [1]. With the upgrade of Argo client [2] and deployment [3] we are now able to exploit it. We set it to 'false' by default on all ResourceOps (actually, leave it empty) and 'true' for VolumeOps. This allows the garbage collection of PVCs upon workflow cleanup without any further change [4]. [1] https://github.com/argoproj/argo/blob/v2.4.0/pkg/apis/workflow/v1alpha1/workflow_types.go#L1044-L1045 [2] kubeflow#4498 [3] kubeflow#3537 [4] kubeflow#1779 Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>

…kubeflow#4498) * updated the version * updated the serializer * fixed test * fixed some more changes * tested to update versions of k8 packages * reverted package update * change in API * fixed dependencies, need to fix broken tests now * updated fake client and fixed test due to updates in timestam.timestamp * missed to update fake client pod * fixed issue in controller * tested to update * updated * updated controller viewer * updates to fix go mod vendor * Updated the client * updated the golang versions * missed one docker file update, from 1.11 -> 1.13 * testing to fixe persistinace agent issues * Updated after feedback Co-authored-by: Niklas hansson <niklashansson@Niklass-MacBook-Pro.local>

…ow#1779 Argo supports a field in the ResourceTemplate that makes the controller add an owner reference of the workflow to the created resource since v2.4.0 [1]. With the upgrade of Argo client [2] and deployment [3] we are now able to exploit it. We set it to 'false' by default on all ResourceOps (actually, leave it empty) and 'true' for VolumeOps. This allows the garbage collection of PVCs upon workflow cleanup without any further change [4]. [1] https://github.com/argoproj/argo/blob/v2.4.0/pkg/apis/workflow/v1alpha1/workflow_types.go#L1044-L1045 [2] kubeflow#4498 [3] kubeflow#3537 [4] kubeflow#1779 Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>

…ow#1779 Argo supports a field in the ResourceTemplate that makes the controller add an owner reference of the workflow to the created resource since v2.4.0 [1]. With the upgrade of Argo client [2] and deployment [3] we are now able to exploit it. We set it to 'false' by default on all ResourceOps (actually, leave it empty). Setting the field to 'true' for VolumeOps allows the garbage collection of PVCs upon workflow cleanup [4]. [1] https://github.com/argoproj/argo/blob/v2.4.0/pkg/apis/workflow/v1alpha1/workflow_types.go#L1044-L1045 [2] kubeflow#4498 [3] kubeflow#3537 [4] kubeflow#1779 Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>

…4831) Argo supports a field in the ResourceTemplate that makes the controller add an owner reference of the workflow to the created resource since v2.4.0 [1]. With the upgrade of Argo client [2] and deployment [3] we are now able to exploit it. We set it to 'false' by default on all ResourceOps (actually, leave it empty). Setting the field to 'true' for VolumeOps allows the garbage collection of PVCs upon workflow cleanup [4]. [1] https://github.com/argoproj/argo/blob/v2.4.0/pkg/apis/workflow/v1alpha1/workflow_types.go#L1044-L1045 [2] #4498 [3] #3537 [4] #1779 Signed-off-by: Ilias Katsakioris <elikatsis@arrikto.com>

updated the version

313bde1

k8s-ci-robot requested review from Ark-kun and IronPan September 15, 2020 07:35

google-cla bot added the cla: yes label Sep 15, 2020

k8s-ci-robot added the size/XL label Sep 15, 2020

Ark-kun reviewed Sep 15, 2020

View reviewed changes

Ark-kun assigned rmgogogo and Bobgy Sep 15, 2020

Ark-kun added the area/backend label Sep 15, 2020

Ark-kun requested review from rmgogogo and Bobgy September 15, 2020 07:40

NikeNano changed the title ~~fix(backend): updated the argo version. Fixes #4392~~ fix(backend): updated the argo version. Fixes #4392 WIP Sep 15, 2020

NikeNano added 3 commits September 15, 2020 13:12

updated the serializer

4922b23

fixed test

50ae6b5

fixed some more changes

a6c30da

Ark-kun reviewed Sep 19, 2020

View reviewed changes

NikeNano added 7 commits September 20, 2020 20:37

tested to update versions of k8 packages

9f1adef

reverted package update

7481cb4

change in API

3a02f5d

fixed dependencies, need to fix broken tests now

7a13a00

updated fake client and fixed test due to updates in timestam.timestamp

9641f16

missed to update fake client pod

84ef7be

fixed issue in controller

c2c7647

k8s-ci-robot added lgtm approved labels Oct 22, 2020

Bobgy added cla: yes and removed approved labels Oct 22, 2020

google-cla bot removed the cla: no label Oct 22, 2020

Bobgy changed the title ~~fix(backend): updated the argo version. Fixes #4392 WIP~~ fix(backend): updated the argo client version. Fixes #4392 Oct 22, 2020

k8s-ci-robot added the do-not-merge/hold label Oct 22, 2020

NikeNano changed the title ~~fix(backend): updated the argo client version. Fixes #4392~~ fix(backend): updated the argo version too 2.7.7. Fixes #4392 Oct 22, 2020

k8s-ci-robot removed the do-not-merge/hold label Oct 22, 2020

k8s-ci-robot added the approved label Oct 22, 2020

k8s-ci-robot merged commit d7793af into kubeflow:master Oct 23, 2020

This was referenced Oct 28, 2020

Upgrade Argo go client to v2.7.5 #3770

Closed

feat(backend) Update argo from 2.7.5 to 2.11.6 #4677

Closed

elikatsis mentioned this pull request Nov 26, 2020

feat(sdk): Enable setting OwnerReference on ResourceOps. Fixes #1779 #4831

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(backend): updated the argo version too 2.7.7. Fixes #4392 #4498

fix(backend): updated the argo version too 2.7.7. Fixes #4392 #4498

NikeNano commented Sep 15, 2020 •

edited by Bobgy

Loading

kubeflow-bot commented Sep 15, 2020

Ark-kun Sep 15, 2020

NikeNano Sep 15, 2020

NikeNano Sep 15, 2020

Ark-kun Sep 19, 2020

NikeNano Sep 22, 2020

NikeNano Oct 14, 2020

Bobgy commented Sep 16, 2020

Ark-kun Sep 19, 2020

NikeNano commented Sep 19, 2020 •

edited

Loading

NikeNano commented Sep 21, 2020

NikeNano commented Sep 22, 2020 •

edited

Loading

Bobgy commented Sep 23, 2020 •

edited

Loading

NikeNano commented Sep 25, 2020

google-cla bot commented Oct 15, 2020

Bobgy commented Oct 22, 2020

Bobgy commented Oct 22, 2020

Bobgy commented Oct 22, 2020

Bobgy commented Oct 22, 2020

NikeNano commented Oct 22, 2020

Bobgy commented Oct 22, 2020

k8s-ci-robot commented Oct 22, 2020

fix(backend): updated the argo version too 2.7.7. Fixes #4392 #4498

fix(backend): updated the argo version too 2.7.7. Fixes #4392 #4498

Conversation

NikeNano commented Sep 15, 2020 • edited by Bobgy Loading

kubeflow-bot commented Sep 15, 2020

Ark-kun Sep 15, 2020

Choose a reason for hiding this comment

NikeNano Sep 15, 2020

Choose a reason for hiding this comment

NikeNano Sep 15, 2020

Choose a reason for hiding this comment

Ark-kun Sep 19, 2020

Choose a reason for hiding this comment

NikeNano Sep 22, 2020

Choose a reason for hiding this comment

NikeNano Oct 14, 2020

Choose a reason for hiding this comment

Bobgy commented Sep 16, 2020

Ark-kun Sep 19, 2020

Choose a reason for hiding this comment

NikeNano commented Sep 19, 2020 • edited Loading

NikeNano commented Sep 21, 2020

NikeNano commented Sep 22, 2020 • edited Loading

Bobgy commented Sep 23, 2020 • edited Loading

NikeNano commented Sep 25, 2020

google-cla bot commented Oct 15, 2020

Bobgy commented Oct 22, 2020

Bobgy commented Oct 22, 2020

Bobgy commented Oct 22, 2020

Bobgy commented Oct 22, 2020

NikeNano commented Oct 22, 2020

Bobgy commented Oct 22, 2020

k8s-ci-robot commented Oct 22, 2020

NikeNano commented Sep 15, 2020 •

edited by Bobgy

Loading

NikeNano commented Sep 19, 2020 •

edited

Loading

NikeNano commented Sep 22, 2020 •

edited

Loading

Bobgy commented Sep 23, 2020 •

edited

Loading