-
Notifications
You must be signed in to change notification settings - Fork 17
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
Cleaner Report resource deletion conditions #62
Comments
Thank you for the detailed description @ctrought I like the timestamp idea as it requires no annotations/labels to be added to any objects:
I understand the TTL as well:
I am not sure I get the use case for Occurrences. Or better there are scenarios where I can think of using (i.e, a deployment whose active replica is 0 while replicas is not, and it is in that state for many days for instance) but I also think those cases can be taken care (every time an object is a match, active replicas == 0 & != replicas, add an annotation or increment it then have another condition that deletes object is annotation value is higher than X). So I am in favour of implementing what you suggested but limiting it to Timestamp and TTL. What do you think? Thank you |
Hi @gianlucam76 , thanks for your feedback! I'm sorry I may not have been clear with my explanation behind the intention behind the proposed conditions for my personal use case. TTL/Occurrences: Duration: The rationale for enabling a condition for duration in report, would be similar to a TTL/occurrences counter in that it will let the resource appear in the report for a period of time before it gets deleted, and the time it lets the object live in the report for is always relative to when it was first added to the report. The way I see it, this lets you use one Cleaner instance that can run forever to handle the cleanup of objects but also gives a grace period to spot and correct any resources that one may not wish to be removed.
Yep makes sense, it does not re-use data from an old report today. The proposal for the TTL/occurrences (counter) and duration/age might require new fields added to the Reports
type ResourceInfo struct {
// Resource identify a Kubernetes resource
Resource corev1.ObjectReference `json:"resource,omitempty"`
// FullResource contains full resources before
// before Cleaner took an action on it
// +optional
FullResource []byte `json:"fullResource,omitempty"`
// Message is an optional field.
// +optional
Message string `json:"message,omitempty"`
// +kubebuilder:validation:Optional
Occurrences int `json:"occurrences,omitempty"`
// +kubebuilder:validation:Optional
FirstOccurrence *metav1.Time`json:"firstOccurrence,omitempty"`
} This would let Cleaner keep a basic history of the resource in the report. It would also require the way the report is generated to be changed slightly (but I don't think too much hopefully). Rather than overwriting the old report entirely, The naming for the conditions is important to remove any ambiguity, so any user would know the different between a TTL counter vs timestamp, etc. Hopefully I am not misinterpreting your own ideas but in summary I think there could probably be 4 different conditions?
Conditions 2 and 4 are dependent on metadata that would need to be tracked in the Cleaner Report. The rough idea would be something like below. The exact name of metadata fields may depend on what the DeleteResourceConditions are named. // pass in a ref to existing report if it exists
func generateReportSpec(resources []ResourceResult, cleaner *appsv1alpha1.Cleaner, existingReport *appsv1alpha1.Report) *appsv1alpha1.ReportSpec {
reportSpec := appsv1alpha1.ReportSpec{}
reportSpec.Action = cleaner.Spec.Action
message := fmt.Sprintf(". time: %v", time.Now())
time := metav1.NewTime(time.Now())
reportSpec.ResourceInfo = make([]appsv1alpha1.ResourceInfo, len(resources))
for i := range resources {
resourceInfo := appsv1alpha1.ResourceInfo{
Resource: corev1.ObjectReference{
Namespace: resources[i].Resource.GetNamespace(),
Name: resources[i].Resource.GetName(),
Kind: resources[i].Resource.GetKind(),
APIVersion: resources[i].Resource.GetAPIVersion(),
},
Message: resources[i].Message + message,
FirstOccurrence: time,
Occurrences: 1,
}
// retain metadata for resource if it exists in an existing report
if len(existingReport.Spec.ResourceInfo) > 0 {
existingResource := getResourceInfoFromReport(resourceInfo.Resource, existingReport)
if existingResource.Resource.Name != "" {
resourceInfo.FirstOccurrence = existingResource.FirstOccurrence
resourceInfo.Occurrences = existingResource.Occurrences + 1
}
}
reportSpec.ResourceInfo[i] = resourceInfo
}
return &reportSpec
} The updated report in-cluster would then have the necessary metadata saved for the DeleteResourceConditions to be handled. This also means these 2 DeleteResourceConditions require a Report to be generated so it can be referred to later on. I can help draft a PR if you'd like, but let me know your thoughts on everything above. |
Thank you @ctrought I get it now. Thanks for the explanation. I am fine now with having this. I actually really like it. And if you can help taking care of it that would be fantastic. I only have one comment. Right now, Report is generated only if notification type asks for it. With this proposal, we would generate a Report irrespective of that if the occurrence limit is set in the Cleaner.Spec for instance. I am not saying we should add an extra CRD for this (it would be duplication). We should probably set a Failure in the Cleaner instance if Report is required because of the Cleaner configuration and NotificationType is not set to generate a Report. We could also generate the Report irrespective when needed. But I am scared that many Report instances might be generated without user knowing and probably that is not OK. |
Is your feature request related to a problem? Please describe.
I'm looking for a way to expire objects based on a set of predefined conditions that can easily be applied to multiple Cleaner objects.
Describe the solution you'd like
Something native, TTL/expiration options exposed in the CRDs that the operator could decide when to delete objects. The options that would be nice (imo) are
TTL/Occurrences Counter: Configure a max counter in the cleaner. Objects that get written to the Report would have their own counter recorded used later to determine when the object can be deleted.
Every subsequent Cleaner run, the counter would be incremented by 1 if the resource still exists in the report. If the specific resources counter > the
resourceDeleteConditions
then the object would be deleted.Duration: Record the firstOccurrence timestamp in the resource info, once the duration has elapsed after subsequent runs the object is eligible for deletion and will be removed.
Timestamp: Absolute, after the given timestamp the objects in the report are deleted.
In the proposed solution it would be possible to combine the conditions as well, where any conditions set must pass for the object in the report to be deleted.
Describe alternatives you've considered
Building in the logic to each Cleaner resource and replicating that to multiple Cleaner objects. There are some related examples here but are based on object age or pre-defined annotations on objects which cover a different set of needs than what I am looking for. Technically you could probably create a transform Cleaner to apply metadata to the object such as firstOccurrence or counter and then use that information in a separate Cleaner to delete resources based on duration or # of occurrences but it would be much more tedious and prone to errors.
I think conditional deletion is probably a pretty common use case, people often want some time to verify whether objects should really be deleted so they might use
Scan
first and when they are confident they may create a new cleaner or modify the existing toDelete
. However when doing so there is no guarantee that the same set of objects as last run would be deleted and different ones may be in the next run. This would give the user an automated way to track & remove resources after specific time related conditions pass.Additional context
I was thinking since the Cleaner Report already tracks objects, it would be relatively simple to store a bit of additional information in the resource list that can be used to determine when the object should be deleted.
The text was updated successfully, but these errors were encountered: