TEP: Add optional flag to enable PVCs auto-removal in "workspaces" coschedule mode

[fambelic]

This TEP proposes adding a new optional enable-pvc-auto-removal flag to the feature-flags ConfigMap, allowing automatic deletion of workspace PVCs when using the workspaces coschedule mode. The change maintains backward compatibility while improving resource cleanup and usability.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: fambelic / name: fambelic (92ae4cf)

[afrittoli]

Thanks @fambelic for this TEP.

One existing alternative today is to use tektoncd/results and configure it so that it deletes PipelineRuns once they are ingested into permanent storage. I appreciate that it does not do the exact same thing, I just wanted to make sure you were aware of it.

Generally speaking it seems fine for me to add this feature. I assume it would only apply to PVC created by Tekton, correct?

It would be good to get some eyes from the chains and results teams, as they add annotations to PipelineRuns after their execution is finished, and I just wanted to make sure that a missing PVC wouldn't break anything - although I don't think it would.

@vdemeester @lcarva @adambkaplan WDYT?

[afrittoli]

NIT: The extra line here is breaking the lint job

[fambelic]

Thanks! I'll fix it asap

[adambkaplan]

One existing alternative today is to use tektoncd/results and configure it so that it deletes PipelineRuns once they are ingested into permanent storage.

Agree that Results pruning of PipelineRuns should be listed as an alternative. The mechanism is different (Kubernetes cascading deletion), and has the downside of removing the PipelineRun + related objects - most notably the TaskRun containers and their logs.

It would be good to get some eyes from the chains and results teams, as they add annotations to PipelineRuns after their execution is finished, and I just wanted to make sure that a missing PVC wouldn't break anything - although I don't think it would.

@khrm @enarha @jkhelil may be in a better position to answer this. I don't think Results adds annotations to PVCs that are owned by a TaskRun or PipelineRun.

[afrittoli]

@pritidesai FYI

[afrittoli]

@fambelic this seems somewhat related tektoncd/pipeline#6635 but it has been abandoned, perhaps you could reuse some of that work too

[fambelic]

Hi @afrittoli! Thanks all for the responses, really appreciate it! :)

One existing alternative today is to use tektoncd/results and configure it so that it deletes PipelineRuns once they are ingested into permanent storage. I appreciate that it does not do the exact same thing, I just wanted to make sure you were aware of it.

Yes, I’m aware of that, but my proposal is more focused on PVCs. It extends a logic that’s already implemented for the two newer coscheduling modes. In short, it aims to handle the case where someone wants to clean up PVCs immediately after a PipelineRun completes. This would bring the default coschedule mode in line with the behavior already present in the other two modes.

Generally speaking it seems fine for me to add this feature. I assume it would only apply to PVC created by Tekton, correct?

Yes!

It would be good to get some eyes from the chains and results teams, as they add annotations to PipelineRuns after their execution is finished, and I just wanted to make sure that a missing PVC wouldn't break anything - although I don't think it would.

Just out of curiosity and to understand better: since the other two coscheduling modes (pipelineruns and isolate-pipelineruns) already automatically remove PVCs, wouldn’t any issues related to missing PVCs and their effect on Chains or Results already be present in those modes?

[afrittoli]

Just out of curiosity and to understand better: since the other two coscheduling modes (pipelineruns and isolate-pipelineruns) already automatically remove PVCs, wouldn’t any issues related to missing PVCs and their effect on Chains or Results already be present in those modes?

That's a fair point; it most likely will be fine.
I'm not sure how extensively those combinations have been tested, but in any case, it's not likely that Chains or Results will be affected by the missing PVC. Results may be looking for logs, so as long as the Pod stays healthy it should be ok.

[tekton-robot]

The following Tekton test failed:

Test name	Commit	Details	Required	Rerun command
pull-community-teps-lint	92ae4cf	link	true	/test pull-community-teps-lint

[fambelic]

Hi @vdemeester! Sorry for the delay! If this feature is considered valuable, I’d be happy to work on its implementation. 😊

[tekton-robot]

@fambelic: PR needs rebase.

Details

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.

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• teps/OWNERS [vdemeester]

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