-
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
Merged
fbm3307
merged 27 commits into
codeready-toolchain:master
from
fbm3307:ks144ttr_cleanup_controller
Mar 20, 2025
Merged
Changes from 8 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
83de6a7
KUBESAW-144: Implement TierTemplateRevision cleanup controller
fbm3307 b5534de
KUBESAW-264: Implement TierTemplateRevision cleanup controller
fbm3307 f44718b
Merge branch 'master' into ks144ttr_cleanup_controller
fbm3307 7612388
golint
fbm3307 01dbe62
some cleanup
fbm3307 726947e
golint spelling
fbm3307 ffc4f83
adding it to main
fbm3307 20bae4e
Merge branch 'master' into ks144ttr_cleanup_controller
fbm3307 81d0768
review comments
fbm3307 87707b0
review comments part 2
fbm3307 e898e6c
golanglint ci
fbm3307 7307ee7
nstemplatetier not found rc
fbm3307 1be1970
requeu bool true
fbm3307 467980e
deleteing ttr if tier-name not there
fbm3307 6059ce5
coverage
fbm3307 ab49a42
reque bool
fbm3307 879809a
adding test case and rc
fbm3307 8d33377
removing extra
fbm3307 588ef62
loggers and comments
fbm3307 c53009d
changes to test cases
fbm3307 cfc17cf
Merge branch 'master' into ks144ttr_cleanup_controller
mfrancisc 1268cbd
Merge branch 'master' into ks144ttr_cleanup_controller
MatousJobanek 0eaa6cf
Merge branch 'master' into ks144ttr_cleanup_controller
metlos a48d2d1
review comments
fbm3307 7e79da5
golint
fbm3307 433f758
Merge branch 'master' into ks144ttr_cleanup_controller
fbm3307 f9ebadc
adding deletion stamp check
fbm3307 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
128 changes: 128 additions & 0 deletions
128
controllers/nstemplatetierrevisioncleanup/nstemplatetier_revision_cleanup_controller.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
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/runtime" | ||
"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) | ||
} | ||
|
||
type Reconciler struct { | ||
Client runtimeclient.Client | ||
Scheme *runtime.Scheme | ||
} | ||
|
||
func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { | ||
logger := log.FromContext(ctx) | ||
|
||
// fetch the NSTemplateTier tier | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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. | ||
logger.Error(err, "unable to get the current TierTemplateRevision") | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return reconcile.Result{}, fmt.Errorf("unable to get the current TierTemplateRevision: %w", err) | ||
} | ||
tierName, ok := ttr.GetLabels()[toolchainv1alpha1.TierLabelKey] | ||
if !ok { | ||
return reconcile.Result{}, fmt.Errorf("tier-name label not found in tiertemplaterevision") | ||
|
||
} | ||
// 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) { | ||
logger.Info("NSTemplateTier not found") | ||
return reconcile.Result{}, nil | ||
} | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Error reading the object - requeue the request. | ||
logger.Error(err, "unable to get the current NSTemplateTier") | ||
return reconcile.Result{}, fmt.Errorf("unable to get the current NSTemplateTier: %w", err) | ||
} | ||
|
||
// verify that the tier template revision which is unsused(not referenced by any tiers and whose creation date is older that 30secs | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// and all the spaces are up-to-date) | ||
UnusedRevisionBool, requeue, reqAft, err := r.VerifyUnusedTTR(ctx, tier, ttr, deletionTimeThreshold) | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
return reconcile.Result{RequeueAfter: reqAft, Requeue: requeue}, fmt.Errorf("cannot delete the current TTR,requing :%w", err) | ||
} | ||
|
||
// Delete the unused revision | ||
if UnusedRevisionBool { | ||
if err := r.Client.Delete(ctx, ttr); err != nil { | ||
return reconcile.Result{}, fmt.Errorf("unable to delete the current Tier Template Revision %s: %w", ttr.Name, err) | ||
} | ||
} | ||
|
||
return reconcile.Result{}, nil | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// VerifyUnusedTTR function verifies that the tier template revision's creation time stamp is older than 30sec, | ||
// it is not present/refernced in status.revisions field, and there are no outdated spaces. | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func (r *Reconciler) VerifyUnusedTTR(ctx context.Context, nsTmplTier *toolchainv1alpha1.NSTemplateTier, | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
rev *toolchainv1alpha1.TierTemplateRevision, deletionTimeThreshold time.Duration) (bool, bool, time.Duration, error) { | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
//get the tier template revision creation time stamp and the duration | ||
timeSinceCreation := time.Since(rev.GetCreationTimestamp().Time) | ||
|
||
//the ttr age should be greater than 30 seconds | ||
if timeSinceCreation < deletionTimeThreshold { | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
requeAfter := deletionTimeThreshold - timeSinceCreation | ||
return false, true, requeAfter, fmt.Errorf("the revision creation timestamp is less than 30 sec(new revision)") | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
//check if the ttr name is present status.revisions | ||
for _, ttstatusrev := range nsTmplTier.Status.Revisions { | ||
if ttstatusrev == rev.Name { | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return false, false, 0, fmt.Errorf("the revision is still being referenced in status.revisions") | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
// get the outdated matchig label to list outdated spaces | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
matchOutdated, err := nstemplatetier.OutdatedTierSelector(nsTmplTier) | ||
if err != nil { | ||
return false, false, 0, err | ||
|
||
} | ||
// 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, false, 0, err | ||
} | ||
|
||
//there should not be any outdated spaces | ||
if len(spaces.Items) > 0 { | ||
return false, false, 0, fmt.Errorf("there are still some spaces which are outdated") | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
return true, false, 0, nil | ||
|
||
} |
218 changes: 218 additions & 0 deletions
218
controllers/nstemplatetierrevisioncleanup/nstemplatetier_revision_cleanup_controller_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,218 @@ | ||
package nstemplatetierrevisioncleanup_test | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"os" | ||
"testing" | ||
"time" | ||
|
||
toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" | ||
"github.com/codeready-toolchain/host-operator/controllers/nstemplatetierrevisioncleanup" | ||
"github.com/codeready-toolchain/host-operator/pkg/apis" | ||
tiertest "github.com/codeready-toolchain/host-operator/test/nstemplatetier" | ||
"github.com/codeready-toolchain/host-operator/test/tiertemplaterevision" | ||
"github.com/codeready-toolchain/toolchain-common/pkg/test" | ||
spacetest "github.com/codeready-toolchain/toolchain-common/pkg/test/space" | ||
"github.com/stretchr/testify/require" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"k8s.io/apimachinery/pkg/types" | ||
"k8s.io/kubectl/pkg/scheme" | ||
controllerruntime "sigs.k8s.io/controller-runtime" | ||
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" | ||
"sigs.k8s.io/controller-runtime/pkg/reconcile" | ||
) | ||
|
||
const ( | ||
operatorNamespace = "toolchain-host-operator" | ||
) | ||
|
||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func TestTTRDeletionReconcile(t *testing.T) { | ||
t.Run("TTR Deleted Successfully", func(t *testing.T) { | ||
//given | ||
nsTemplateTier := tiertest.Base1nsTier(t, tiertest.CurrentBase1nsTemplates, tiertest.WithStatusRevisions()) | ||
ttr := createttr(*nsTemplateTier, (nsTemplateTier.Spec.ClusterResources.TemplateRef + "-ttrcr"), metav1.NewTime(time.Now().Add(-time.Minute))) | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
s := createSpace(nsTemplateTier) | ||
r, req, cl := prepareReconcile(t, ttr.Name, ttr, s, nsTemplateTier) | ||
//when | ||
res, err := r.Reconcile(context.TODO(), req) | ||
//then | ||
require.NoError(t, err) | ||
require.Equal(t, controllerruntime.Result{}, res) | ||
tiertemplaterevision.AssertThatTTRs(t, cl, nsTemplateTier.GetNamespace()).DoNotExist() | ||
}) | ||
t.Run("Failure", func(t *testing.T) { | ||
nsTemplateTier := tiertest.Base1nsTier(t, tiertest.CurrentBase1nsTemplates, tiertest.WithStatusRevisions()) | ||
ttr := createttr(*nsTemplateTier, (nsTemplateTier.Spec.ClusterResources.TemplateRef + "-ttrcr"), metav1.NewTime(time.Now().Add(-time.Minute))) | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
s := createSpace(nsTemplateTier) | ||
t.Run("the creation timestamp is less than 30 sec", func(t *testing.T) { | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// given | ||
ttr := createttr(*nsTemplateTier, (nsTemplateTier.Spec.ClusterResources.TemplateRef + "-ttr"), metav1.NewTime(time.Now())) | ||
r, req, cl := prepareReconcile(t, ttr.Name, ttr, s, nsTemplateTier) | ||
|
||
// when | ||
res, err := r.Reconcile(context.TODO(), req) | ||
|
||
// then | ||
require.EqualError(t, err, "cannot delete the current TTR,requing :the revision creation timestamp is less than 30 sec(new revision)") | ||
require.True(t, res.Requeue) | ||
tiertemplaterevision.AssertThatTTRs(t, cl, nsTemplateTier.GetNamespace()).ExistFor(nsTemplateTier.Name) | ||
|
||
}) | ||
t.Run("ttr is still being referenced in status.revisions", func(t *testing.T) { | ||
// given | ||
ttr := createttr(*nsTemplateTier, (nsTemplateTier.Spec.ClusterResources.TemplateRef + "-ttr"), metav1.NewTime(time.Now().Add(-time.Minute))) | ||
r, req, cl := prepareReconcile(t, ttr.Name, ttr, s, nsTemplateTier) | ||
|
||
// when | ||
res, err := r.Reconcile(context.TODO(), req) | ||
|
||
// then | ||
require.EqualError(t, err, "cannot delete the current TTR,requing :the revision is still being referenced in status.revisions") | ||
require.False(t, res.Requeue) | ||
tiertemplaterevision.AssertThatTTRs(t, cl, nsTemplateTier.GetNamespace()).ExistFor(nsTemplateTier.Name) | ||
|
||
}) | ||
|
||
t.Run("spaces are still being updated", func(t *testing.T) { | ||
// given | ||
nsTemplateTier.Status.Revisions = map[string]string{ | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"base1ns-code-123456new": "base1ns-code-123456new-ttr", | ||
"base1ns-clusterresources-123456new": "base1ns-clusterresources-123456new-ttr", | ||
} | ||
ttr := createttr(*nsTemplateTier, (nsTemplateTier.Spec.ClusterResources.TemplateRef + "-ttrcr"), metav1.NewTime(time.Now().Add(-time.Minute))) | ||
r, req, cl := prepareReconcile(t, ttr.Name, ttr, s, nsTemplateTier) | ||
|
||
// when | ||
res, err := r.Reconcile(context.TODO(), req) | ||
|
||
// then | ||
require.EqualError(t, err, "cannot delete the current TTR,requing :there are still some spaces which are outdated") | ||
require.False(t, res.Requeue) | ||
tiertemplaterevision.AssertThatTTRs(t, cl, nsTemplateTier.GetNamespace()).ExistFor(nsTemplateTier.Name) | ||
}) | ||
|
||
t.Run("Error while deleting the TTR", func(t *testing.T) { | ||
// given | ||
nsTemplateTier := tiertest.Base1nsTier(t, tiertest.CurrentBase1nsTemplates, tiertest.WithStatusRevisions()) | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ttr := createttr(*nsTemplateTier, (nsTemplateTier.Spec.ClusterResources.TemplateRef + "-ttrcr"), metav1.NewTime(time.Now().Add(-time.Minute))) | ||
s := createSpace(nsTemplateTier) | ||
r, req, cl := prepareReconcile(t, ttr.Name, ttr, s, nsTemplateTier) | ||
cl.MockDelete = func(ctx context.Context, obj runtimeclient.Object, opts ...runtimeclient.DeleteOption) error { | ||
return fmt.Errorf("some error cannot delete") | ||
} | ||
// when | ||
res, err := r.Reconcile(context.TODO(), req) | ||
|
||
// then | ||
require.EqualError(t, err, "unable to delete the current Tier Template Revision base1ns-clusterresources-123456new-ttrcr: some error cannot delete") | ||
require.False(t, res.Requeue) | ||
tiertemplaterevision.AssertThatTTRs(t, cl, nsTemplateTier.GetNamespace()).ExistFor(nsTemplateTier.Name) | ||
}) | ||
|
||
t.Run("error while getting revision", func(t *testing.T) { | ||
r, req, cl := prepareReconcile(t, ttr.Name, ttr, s, nsTemplateTier) | ||
cl.MockGet = func(ctx context.Context, key runtimeclient.ObjectKey, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error { | ||
return fmt.Errorf("some error cannot get") | ||
} | ||
// when | ||
res, err := r.Reconcile(context.TODO(), req) | ||
|
||
// then | ||
require.EqualError(t, err, "unable to get the current TierTemplateRevision: some error cannot get") | ||
require.False(t, res.Requeue) | ||
tiertemplaterevision.AssertThatTTRs(t, cl, nsTemplateTier.GetNamespace()).ExistFor(nsTemplateTier.Name) | ||
}) | ||
|
||
t.Run("error while listing outdated spaces", func(t *testing.T) { | ||
r, req, cl := prepareReconcile(t, ttr.Name, ttr, s, nsTemplateTier) | ||
cl.MockList = func(ctx context.Context, list runtimeclient.ObjectList, opts ...runtimeclient.ListOption) error { | ||
return fmt.Errorf("some error") | ||
} | ||
// when | ||
res, err := r.Reconcile(context.TODO(), req) | ||
|
||
// then | ||
require.EqualError(t, err, "cannot delete the current TTR,requing :some error") | ||
require.False(t, res.Requeue) | ||
tiertemplaterevision.AssertThatTTRs(t, cl, nsTemplateTier.GetNamespace()).ExistFor(nsTemplateTier.Name) | ||
}) | ||
|
||
t.Run("revision not found", func(t *testing.T) { | ||
r, req, _ := prepareReconcile(t, "base") | ||
//when | ||
res, err := r.Reconcile(context.TODO(), req) | ||
//then | ||
require.NoError(t, err) | ||
require.False(t, res.Requeue) | ||
|
||
}) | ||
|
||
t.Run("NsTemplate Tier not found", func(t *testing.T) { | ||
r, req, _ := prepareReconcile(t, ttr.Name, ttr) | ||
//when | ||
res, err := r.Reconcile(context.TODO(), req) | ||
//then | ||
require.NoError(t, err) | ||
require.False(t, res.Requeue) | ||
|
||
}) | ||
|
||
t.Run("tier label not found", func(t *testing.T) { | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ttr.Labels = map[string]string{} | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
r, req, _ := prepareReconcile(t, ttr.Name, ttr) | ||
//when | ||
res, err := r.Reconcile(context.TODO(), req) | ||
//then | ||
require.EqualError(t, err, "tier-name label not found in tiertemplaterevision") | ||
require.False(t, res.Requeue) | ||
}) | ||
|
||
}) | ||
|
||
} | ||
|
||
func prepareReconcile(t *testing.T, name string, initObjs ...runtimeclient.Object) (*nstemplatetierrevisioncleanup.Reconciler, reconcile.Request, *test.FakeClient) { | ||
os.Setenv("WATCH_NAMESPACE", test.HostOperatorNs) | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
s := scheme.Scheme | ||
err := apis.AddToScheme(s) | ||
require.NoError(t, err) | ||
cl := test.NewFakeClient(t, initObjs...) | ||
r := &nstemplatetierrevisioncleanup.Reconciler{ | ||
Client: cl, | ||
Scheme: s, | ||
} | ||
return r, reconcile.Request{ | ||
NamespacedName: types.NamespacedName{ | ||
Name: name, | ||
Namespace: operatorNamespace, | ||
}, | ||
}, cl | ||
} | ||
func createttr(nsTTier toolchainv1alpha1.NSTemplateTier, name string, crtime metav1.Time) *toolchainv1alpha1.TierTemplateRevision { | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
labels := map[string]string{ | ||
toolchainv1alpha1.TierLabelKey: nsTTier.Name, | ||
} | ||
ttr := &toolchainv1alpha1.TierTemplateRevision{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Namespace: operatorNamespace, | ||
Name: name, | ||
Labels: labels, | ||
CreationTimestamp: crtime, | ||
}, | ||
Spec: toolchainv1alpha1.TierTemplateRevisionSpec{ | ||
Parameters: nsTTier.Spec.Parameters, | ||
}, | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
return ttr | ||
} | ||
|
||
func createSpace(nsTTier *toolchainv1alpha1.NSTemplateTier) *toolchainv1alpha1.Space { | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
testSpace := spacetest.NewSpace(test.HostOperatorNs, "oddity1", | ||
spacetest.WithTierNameAndHashLabelFor(nsTTier), | ||
spacetest.WithSpecTargetCluster("member-1"), | ||
spacetest.WithStatusTargetCluster("member-1"), | ||
spacetest.WithFinalizer(), | ||
spacetest.WithCondition(spacetest.Ready())) | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return testSpace | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.