-
Notifications
You must be signed in to change notification settings - Fork 57
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
Garbage collection for Update objects #810
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #810 +/- ##
==========================================
+ Coverage 51.16% 51.61% +0.44%
==========================================
Files 31 31
Lines 4325 4340 +15
==========================================
+ Hits 2213 2240 +27
+ Misses 1923 1909 -14
- Partials 189 191 +2 ☔ View full report in Codecov by Sentry. |
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.
LGTM as someone who isn't familiar with PKO.
obj = &autov1alpha1.Update{} | ||
_ = k8sClient.Get(ctx, objName, obj) | ||
|
||
// progressing = meta.FindStatusCondition(obj.Status.Conditions, autov1alpha1.UpdateConditionTypeProgressing) |
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] cleanup? Also line 53
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.
I deliberately kept this here because it is useful scaffolding for more tests. This controller simply doesn't have any tests yet that would use it. Keep/drop?
Proposed changes
Implements a retention policy for completed Update objects, based on a simple TTL with the default value of 24h. Update objects that are in a completed state and are older than the TTL are summarily deleted.
New API:
Update.spec.ttlAfterCompleted
A new field to enable garbage collection of this update, once completed. Optional; no default value.
Update Controller
Updated to implement the cleanup logic. When a TTL is set on a completed update, the controller looks to the last transition time of the Completed status condition. If there's time remaining, the controller requeues the object for later, otherwise it deletes the update object.
Stack Controller
Updated to assign a 24-hour TTL on the update objects that it creates. A future enhancement would be to provide an
updateTemplate
field to allow for further control. A user may edit an existing Update to change the TTL.Update Controller Tests
A new unit test was added to exercise the retention logic.
Related issues (optional)
Closes #733