VolumeOp doesn't support GC after workflow deletion #1779

hongye-sun · 2019-08-08T23:42:41Z

The PVC created from VolumeOp is not automatically recycled after workflow is deleted.

I wonder if owner reference can work with PVC when it is set with workflow's reference. If it works, we can provide an option in volumeop to set it when creating the PVC.

If it doesn't work, we should provide a way to either use Argo's volume API or support a delete PVC op in exit handler.

prcastro · 2019-12-30T17:58:20Z

What is the practical consequence of this? I'm having an issue with bound PVCs from pipelines that ran long ago.

elikatsis · 2020-01-02T11:23:41Z

Hi all,
I'm working on a solution using owner references that seems to be working just fine!
I'll follow up with a PR soon.

/assign elikatsis

elikatsis · 2020-01-08T11:07:19Z

As I mentioned in the previous comment, I've been working on solving this issue using OwnerReferences.

An OwnerReference requires two basic fields that refer to the actual Kubernetes resource: name and uid.
Argo enables access to those using Argo (v2.3.0) variables: {{workflow.name}}, {{workflow.uid}}.

PR #1720, however, changed {{workflow.uid}} so that it points to runID instead, making it impossible to add OwnerReferences.

I'm not sure I understand the rationale behind this. [ cc @IronPan ]
Why don't we use {{workflow.labels.pipeline/runid}}?
[ cc @Ark-kun since it's used in the DSL in RUN_ID_PLACEHOLDER ]
It feels very odd having {{workflow.uid}} modified and not function as it's supposed to (i.e., be replaced with workflow's UUID), especially when there's a valid way to do what we desire.

karlschriek · 2020-01-09T14:52:05Z

I would be very interested in this. We have workflows where we sometimes need to dump out fairly large files to disk, but they are only relevant while the component is executing. Currently we have to always manually delete the PVCs afterwards

elikatsis · 2020-01-15T11:15:55Z

@Ark-kun @IronPan friendly ping 😄

PatrickXYS · 2020-02-29T04:17:49Z

Willing to help, and will send a PR for adding support a delete PVC op.

elikatsis · 2020-04-22T16:29:20Z

@Ark-kun @IronPan

Hi there! Pinging again for an answer to this comment: #1779 (comment)

Since it's been some time (KFP has had many releases, KF ships with a KFP version containing #1720), can we move to using a different Argo magic string and not override the existing {{workflow.uid}}?
Is it OK if I submit a PR fixing that behavior?

If we go with {{workflow.labels.pipeline/runid}} rather than a new {{kfp.run.id}}, there will also be no problem with incompatibilities between SDK and apiserver.

Ark-kun · 2020-04-28T20:04:08Z

I generally agree with the suggestion.

stale · 2020-07-27T20:06:04Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

PatrickXYS · 2020-07-27T20:31:40Z

I don't think it's fixed now, please keep open until we fix.

marrrcin · 2020-08-14T13:52:57Z

Is there any workaround to remove the actual volume (disk) after the pipeline finishes?
Using

my_volume = dsl.VolumeOp(
        name="create-volume",
        resource_name="asd",
        size="10Gi",
        modes=dsl.VOLUME_MODE_RWO,
    )
dsl.ResourceOp(
        name="delete-volume",
        k8s_resource=my_volume.k8s_resource,
        action="delete"
    )

still leaves the unused disk in GCE.

stale · 2020-11-15T17:39:48Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

jackwhelpton · 2020-11-15T17:45:41Z

This is still a sorely-needed feature for us, would be good to keep the ticket open until it is resolved.

Gsantomaggio · 2020-11-15T17:46:55Z

Agree with @jackwhelpton
I would reopen it.
Thank you

talhairfanbentley · 2020-11-20T12:25:06Z

Hey, did anyone found any workaround for this?

PatrickXYS · 2020-11-20T17:05:55Z

@kubeflow/wg-pipeline-leads I think this is a well-known issue and all the cloud-provider should get the same issue.

As I can see, we met similar issue here. Can you put some efforts on this issue and I can help as well?

/cc @Bobgy

Bobgy · 2020-11-21T00:15:35Z

/assign @chensun
Can you take a look?
Looks like this is requested by a lot of people

elikatsis · 2020-11-23T18:09:41Z

Since we have now upgraded Argo client version used by the backend (apart from the deployment) we can take advantage of the setOwnerReference field of the ResourceTemplate (source).

Using this we will be able to set an owner reference of the workflow to PVCs created via the SDK, without having to deal with the [odd] design described above (#1779 (comment)).

@Bobgy I'm preparing a PR for this and will open it pretty soon :)

PatrickXYS · 2020-11-23T18:14:23Z

@elikatsis Feel free to cc me the PR

Would like to help as well

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>

…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>

RoyerRamirez · 2021-01-29T19:11:45Z

Hi, I'm following along. Have we found a fix for this issue? We currently run many jobs in parallel, and having to go back to delete the disks after they are used can be a pain. Not to mention, it can get costly over time if users don't clean up their environment.

elikatsis · 2021-02-03T17:53:17Z

@RoyerRamirez we've got #4831 targeting this but it's not merged yet.

…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>

paveldournov assigned jbottum Aug 9, 2019

paveldournov added area/sdk/dsl area/pipelines kind/investigate labels Aug 9, 2019

paveldournov added this to the M9 milestone Aug 9, 2019

paveldournov added priority/p2 help wanted The community is welcome to contribute. labels Aug 9, 2019

rmgogogo removed the kind/investigate label Nov 18, 2019

k8s-ci-robot assigned elikatsis Jan 2, 2020

PatrickXYS mentioned this issue Feb 29, 2020

Handle delete action for VolumeOp #3194

Closed

elikatsis mentioned this issue Mar 4, 2020

SDK/DSL: Enable the deletion of a resource via ResourceOp method #3213

Merged

elikatsis mentioned this issue May 7, 2020

Fix workflow.uid global variable usage #3709

Closed

elikatsis mentioned this issue May 18, 2020

Delete marshal PVC after run kubeflow-kale/kale#129

Closed

stale bot added the lifecycle/stale The issue / pull request is stale, any activities remove this label. label Jul 27, 2020

stale bot removed the lifecycle/stale The issue / pull request is stale, any activities remove this label. label Jul 27, 2020

Saurav-D mentioned this issue Nov 5, 2020

Cleanup Kubeflow Pipeline PVC after training completes Unity-Technologies/datasetinsights#120

Closed

stale bot added the lifecycle/stale The issue / pull request is stale, any activities remove this label. label Nov 15, 2020

stale bot removed the lifecycle/stale The issue / pull request is stale, any activities remove this label. label Nov 15, 2020

k8s-ci-robot assigned chensun Nov 21, 2020

elikatsis mentioned this issue Nov 26, 2020

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

Merged

2 tasks

kim-sardine mentioned this issue Feb 3, 2021

Kubeflow 1.2 PVC volume is not getting deleted #5046

Closed

google-oss-robot closed this as completed in #4831 Mar 12, 2021

magdalenakuhn17 pushed a commit to magdalenakuhn17/pipelines that referenced this issue Oct 22, 2023

Fix parallel inference example with Ray 1.5 (kubeflow#1779)

521e10c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VolumeOp doesn't support GC after workflow deletion #1779

VolumeOp doesn't support GC after workflow deletion #1779

hongye-sun commented Aug 8, 2019

prcastro commented Dec 30, 2019

elikatsis commented Jan 2, 2020

elikatsis commented Jan 8, 2020

karlschriek commented Jan 9, 2020

elikatsis commented Jan 15, 2020

PatrickXYS commented Feb 29, 2020

elikatsis commented Apr 22, 2020 •

edited

Loading

Ark-kun commented Apr 28, 2020

stale bot commented Jul 27, 2020

PatrickXYS commented Jul 27, 2020

marrrcin commented Aug 14, 2020

stale bot commented Nov 15, 2020

jackwhelpton commented Nov 15, 2020

Gsantomaggio commented Nov 15, 2020

talhairfanbentley commented Nov 20, 2020

PatrickXYS commented Nov 20, 2020

Bobgy commented Nov 21, 2020

elikatsis commented Nov 23, 2020

PatrickXYS commented Nov 23, 2020 •

edited

Loading

RoyerRamirez commented Jan 29, 2021

elikatsis commented Feb 3, 2021

VolumeOp doesn't support GC after workflow deletion #1779

VolumeOp doesn't support GC after workflow deletion #1779

Comments

hongye-sun commented Aug 8, 2019

prcastro commented Dec 30, 2019

elikatsis commented Jan 2, 2020

elikatsis commented Jan 8, 2020

karlschriek commented Jan 9, 2020

elikatsis commented Jan 15, 2020

PatrickXYS commented Feb 29, 2020

elikatsis commented Apr 22, 2020 • edited Loading

Ark-kun commented Apr 28, 2020

stale bot commented Jul 27, 2020

PatrickXYS commented Jul 27, 2020

marrrcin commented Aug 14, 2020

stale bot commented Nov 15, 2020

jackwhelpton commented Nov 15, 2020

Gsantomaggio commented Nov 15, 2020

talhairfanbentley commented Nov 20, 2020

PatrickXYS commented Nov 20, 2020

Bobgy commented Nov 21, 2020

elikatsis commented Nov 23, 2020

PatrickXYS commented Nov 23, 2020 • edited Loading

RoyerRamirez commented Jan 29, 2021

elikatsis commented Feb 3, 2021

elikatsis commented Apr 22, 2020 •

edited

Loading

PatrickXYS commented Nov 23, 2020 •

edited

Loading