-
Notifications
You must be signed in to change notification settings - Fork 234
TEP: Add optional flag to enable PVCs auto-removal in "workspaces" coschedule mode #1203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
a29b35e
to
b59f0e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
teps/README.md
Outdated
@@ -147,3 +147,5 @@ This is the complete list of Tekton TEPs: | |||
|[TEP-0155](0155-store-pipeline-events-in-db.md) | Store Pipeline Events in Tekton Results | proposed | 2024-04-19 | | |||
|[TEP-0156](0156-whenexpressions-in-step.md) | WhenExpressions in Steps | implemented | 2024-07-22 | | |||
|[TEP-0160](0160-enhance-results-cli.md) | Enhance Tekton Results CLI | proposed | 2025-03-13 | | |||
|[TEP-0161](0161-optional-flag-for-enabling-pvcs-auto-removal-behavior.md) | Optional Flag for Enabling PVCs Auto Removal Behavior | proposed | 2025-05-26 | | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: The extra line here is breaking the lint job
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I'll fix it asap
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.
@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. |
@pritidesai FYI |
@fambelic this seems somewhat related tektoncd/pipeline#6635 but it has been abandoned, perhaps you could reuse some of that work too |
Hi @afrittoli! Thanks all for the responses, really appreciate 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.
Yes!
Just out of curiosity and to understand better: since the other two coscheduling modes ( |
That's a fair point; it most likely will be fine. |
1fba8ca
to
5b5981c
Compare
Hi @vdemeester! Sorry for the delay! If this feature is considered valuable, I’d be happy to work on its implementation. 😊 |
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.