-
Notifications
You must be signed in to change notification settings - Fork 69
KUBESAW-264: Implement TierTemplateRevision cleanup controller #1153
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
Changes from 22 commits
83de6a7
b5534de
f44718b
7612388
01dbe62
726947e
ffc4f83
20bae4e
81d0768
87707b0
e898e6c
7307ee7
1be1970
467980e
6059ce5
ab49a42
879809a
8d33377
588ef62
c53009d
cfc17cf
1268cbd
0eaa6cf
a48d2d1
7e79da5
433f758
f9ebadc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,141 @@ | ||
package nstemplatetierrevisioncleanup | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"time" | ||
|
||
"sigs.k8s.io/controller-runtime/pkg/log" | ||
|
||
toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" | ||
"github.com/codeready-toolchain/host-operator/controllers/nstemplatetier" | ||
"k8s.io/apimachinery/pkg/api/errors" | ||
"k8s.io/apimachinery/pkg/types" | ||
"sigs.k8s.io/controller-runtime/pkg/reconcile" | ||
|
||
ctrl "sigs.k8s.io/controller-runtime" | ||
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" | ||
"sigs.k8s.io/controller-runtime/pkg/manager" | ||
) | ||
|
||
const DeletionTimeThreshold = 30 * time.Second | ||
|
||
// SetupWithManager sets up the controller with the Manager. | ||
func (r *Reconciler) SetupWithManager(mgr manager.Manager) error { | ||
return ctrl.NewControllerManagedBy(mgr). | ||
For(&toolchainv1alpha1.TierTemplateRevision{}). | ||
Complete(r) | ||
Check warning on line 27 in controllers/nstemplatetierrevisioncleanup/nstemplatetier_revision_cleanup_controller.go
|
||
} | ||
|
||
type Reconciler struct { | ||
Client runtimeclient.Client | ||
} | ||
|
||
func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { | ||
logger := log.FromContext(ctx) | ||
|
||
// fetch the NSTemplateTier tier | ||
ttr := &toolchainv1alpha1.TierTemplateRevision{} | ||
if err := r.Client.Get(ctx, request.NamespacedName, ttr); err != nil { | ||
if errors.IsNotFound(err) { | ||
logger.Info("TierTemplateRevision not found") | ||
return reconcile.Result{}, nil | ||
} | ||
// Error reading the object - requeue the request. | ||
return reconcile.Result{}, fmt.Errorf("unable to get the current TierTemplateRevision: %w", err) | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should be a check for a non-nil There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch, but there is no finalizer which means that it should be deleted immediately. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and if we want to check if it's not being deleted, then we should use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okay just curious, if we don't have finalizers, then any particular reason on why we should still check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Usually, if you detect the resource is being deleted, you just stop the reconcile by returning early without an error. If there are finalizers, you first do the clean up logic, remove the finalizer and then return, again not performing the "normal" reconciliation logic. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @metlos I have updated it now PTAL. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @metlos the point was that if there is no finalizer added, then as soon as the resource has the deletion timestamp set, then it should be immediately deleted. |
||
//there is no point in fetching the NStemplateTier, if the TTR is just created | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
//get the tier template revision creation time stamp and the duration | ||
timeSinceCreation := time.Since(ttr.GetCreationTimestamp().Time) | ||
|
||
//the ttr age should be greater than 30 seconds | ||
if timeSinceCreation < DeletionTimeThreshold { | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
requeueAfter := DeletionTimeThreshold - timeSinceCreation | ||
logger.Info("the TierTemplateRevision is not old enough", "requeue-after", requeueAfter) | ||
return reconcile.Result{RequeueAfter: requeueAfter}, nil | ||
} | ||
|
||
//check if there is tier-name label available | ||
tierName, ok := ttr.GetLabels()[toolchainv1alpha1.TierLabelKey] | ||
if !ok { | ||
//if tier-name label is not found we should delete the TTR | ||
return reconcile.Result{}, r.deleteTTR(ctx, ttr) | ||
} | ||
// fetch the related NSTemplateTier tier | ||
tier := &toolchainv1alpha1.NSTemplateTier{} | ||
if err := r.Client.Get(ctx, types.NamespacedName{ | ||
Namespace: ttr.GetNamespace(), | ||
Name: tierName, | ||
}, tier); err != nil { | ||
if errors.IsNotFound(err) { | ||
// if there is no related nstemplate tier, we can delete the TTR directly | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return reconcile.Result{}, r.deleteTTR(ctx, ttr) | ||
} | ||
Comment on lines
+80
to
+83
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Btw, if you added a watcher on NSTemplateTier CRD with a predicate that would allow deletion events only, then you could remove the TTR-specific cleanup logic from the e2e tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You meant the watcher in this controller itselt? |
||
// Error reading the object - requeue the request. | ||
return reconcile.Result{}, fmt.Errorf("unable to get the current NSTemplateTier: %w", err) | ||
} | ||
|
||
// let's make sure that we delete the TTR only if it's not used | ||
isTTrUnused, err := r.verifyUnusedTTR(ctx, tier, ttr) | ||
if err != nil { | ||
return reconcile.Result{}, err | ||
} | ||
|
||
// Delete the unused revision | ||
if isTTrUnused { | ||
return reconcile.Result{}, r.deleteTTR(ctx, ttr) | ||
} | ||
|
||
return reconcile.Result{}, nil | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// verifyUnusedTTR function verifies that the TTR is not used (returns true if it's used). | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// this is done by: | ||
// - checking the NSTemplateTier.status.revisions field, if the TTR is referenced there or not | ||
// - checking if all Spaces are up-to-date. In case there are some outdated space, we could risk that the TTR is still being used | ||
func (r *Reconciler) verifyUnusedTTR(ctx context.Context, nsTmplTier *toolchainv1alpha1.NSTemplateTier, | ||
rev *toolchainv1alpha1.TierTemplateRevision) (bool, error) { | ||
|
||
logger := log.FromContext(ctx) | ||
//check if the ttr name is present status.revisions | ||
for _, ttStatusRev := range nsTmplTier.Status.Revisions { | ||
if ttStatusRev == rev.Name { | ||
logger.Info("the TierTemplateRevision is still being referenced in NSTemplateTier.Status.Revisions") | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return false, nil | ||
} | ||
} | ||
|
||
// get the outdated matching label to list outdated spaces | ||
matchOutdated, err := nstemplatetier.OutdatedTierSelector(nsTmplTier) | ||
if err != nil { | ||
return false, err | ||
|
||
} | ||
Check warning on line 114 in controllers/nstemplatetierrevisioncleanup/nstemplatetier_revision_cleanup_controller.go
|
||
// look-up all spaces associated with the NSTemplateTier which are outdated | ||
spaces := &toolchainv1alpha1.SpaceList{} | ||
if err := r.Client.List(ctx, spaces, runtimeclient.InNamespace(nsTmplTier.Namespace), | ||
matchOutdated, runtimeclient.Limit(1)); err != nil { | ||
return false, err | ||
} | ||
|
||
//If there has been an update on nstemplatetier, it might be in a process to update all the spaces. | ||
// so we need to check that there should be no outdated spaces. | ||
if len(spaces.Items) > 0 { | ||
logger.Info("there are still some spaces which are outdated") | ||
return false, nil | ||
} | ||
return true, nil | ||
} | ||
|
||
// deleteTTR function delete the TTR | ||
func (r *Reconciler) deleteTTR(ctx context.Context, ttr *toolchainv1alpha1.TierTemplateRevision) error { | ||
if err := r.Client.Delete(ctx, ttr); err != nil { | ||
if errors.IsNotFound(err) { | ||
return nil // was already deleted | ||
} | ||
return fmt.Errorf("unable to delete the current Tier Template Revision %s: %w", ttr.Name, err) | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
log.FromContext(ctx).Info("The TierTemplateRevision has been deleted") | ||
return nil | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
Uh oh!
There was an error while loading. Please reload this page.