controller: add shim to garbage collect old resources

This patch adds a shim layer that is responsible for cleaning
up the old resources so that we do not have two CronJobs created
for the same underlying PVC.

Signed-off-by: Niraj Yadav <niryadav@redhat.com>

Author: black-dragon74

internal/controller/csiaddons/pvc_new_controller.go

@@ -108,29 +108,53 @@ func (r *PVCReconiler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Re
 
 	// Reconcile for dependent features
 	// Reconcile - Key rotation
+	keyRotationName := fmt.Sprintf("%s-keyrotation", pvc.Name)
 	keyRotationSched := sc.Annotations[utils.KrcJobScheduleTimeAnnotation]
 	keyRotationEnabled := sc.Annotations[utils.KrEnableAnnotation]
 	keyRotationChild := &csiaddonsv1alpha1.EncryptionKeyRotationCronJob{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      fmt.Sprintf("%s-keyrotation", pvc.Name),
+			Name:      keyRotationName,
 			Namespace: pvc.Namespace,
 		},
 	}
 
+	// FIXME: This is a shim and should be removed in later releases
+	// along with the field indexers on child objects
+	if requeue, err := utils.CleanOldJobs(ctx,
+		r.Client,
+		logger,
+		req,
+		&csiaddonsv1alpha1.EncryptionKeyRotationCronJobList{},
+		keyRotationName); err != nil || requeue {
+		return ctrl.Result{Requeue: requeue}, err
+	}
+
 	if err := r.reoncileFeature(ctx, logger, pvc, keyRotationChild, keyRotationSched, keyRotationEnabled); err != nil {
 		return ctrl.Result{}, err
 	}
 
 	// Reconcile - Reclaim space
+	reclaimSpaceName := fmt.Sprintf("%s-reclaimspace", pvc.Name)
 	reclaimSpaceSched := sc.Annotations[utils.RsCronJobScheduleTimeAnnotation]
-	relciamSpaceChild := &csiaddonsv1alpha1.ReclaimSpaceCronJob{
+	reclaimSpaceChild := &csiaddonsv1alpha1.ReclaimSpaceCronJob{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      fmt.Sprintf("%s-reclaimspace", pvc.Name),
+			Name:      reclaimSpaceName,
 			Namespace: pvc.Namespace,
 		},
 	}
 
-	if err := r.reoncileFeature(ctx, logger, pvc, relciamSpaceChild, reclaimSpaceSched, "true"); err != nil {
+	// FIXME: This is a shim and should be removed in later releases
+	// along with the field indexers on child objects
+	if requeue, err := utils.CleanOldJobs(ctx,
+		r.Client,
+		logger,
+		req,
+		&csiaddonsv1alpha1.ReclaimSpaceCronJobList{},
+		reclaimSpaceName); err != nil || requeue {
+		return ctrl.Result{Requeue: requeue}, err
+	}
+
+	if err := r.reoncileFeature(ctx, logger, pvc, reclaimSpaceChild, reclaimSpaceSched, "true"); err != nil {
 		return ctrl.Result{}, err
 	}
 

internal/controller/utils/predicates.go

@@ -3,6 +3,8 @@ package utils
 import (
 	"context"
 
+	csiaddonsv1alpha1 "github.com/csi-addons/kubernetes-csi-addons/api/csiaddons/v1alpha1"
+
 	corev1 "k8s.io/api/core/v1"
 	storagev1 "k8s.io/api/storage/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -90,6 +92,18 @@ func SetupPVCControllerIndexers(mgr ctrl.Manager) error {
 				return []string{*pvc.Spec.StorageClassName}
 			},
 		},
+		// FIXME: Remove this shim in later releases
+		{
+			obj:     &csiaddonsv1alpha1.ReclaimSpaceCronJob{},
+			field:   JobOwnerKey,
+			indexFn: ExtractOwnerNameFromPVCObj[*csiaddonsv1alpha1.ReclaimSpaceCronJob],
+		},
+		// FIXME: Remove this shim in later releases
+		{
+			obj:     &csiaddonsv1alpha1.EncryptionKeyRotationCronJob{},
+			field:   JobOwnerKey,
+			indexFn: ExtractOwnerNameFromPVCObj[*csiaddonsv1alpha1.EncryptionKeyRotationCronJob],
+		},
 	}
 
 	for _, index := range indices {

internal/controller/utils/reclaimspace_keyrotation_utils.go

@@ -1,11 +1,15 @@
 package utils
 
 import (
+	"context"
 	"errors"
 
 	csiaddonsv1alpha1 "github.com/csi-addons/kubernetes-csi-addons/api/csiaddons/v1alpha1"
 
+	"github.com/go-logr/logr"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -105,3 +109,56 @@ func ExtractOwnerNameFromPVCObj[T client.Object](rawObj client.Object) []string
 
 	return []string{owner.Name}
 }
+
+func CleanOldJobs(
+	ctx context.Context,
+	c client.Client,
+	log logr.Logger,
+	req ctrl.Request,
+	objList client.ObjectList,
+	expectedName string,
+) (bool, error) {
+	// We need to find all the EKRCronJobs in the namespace of the PVC which
+	// are owned by this controller and remove it if it doesn't match the expected name
+	shouldRequeue := false
+
+	if err := c.List(ctx, objList, client.InNamespace(req.Namespace), client.MatchingFields{JobOwnerKey: req.Name}); client.IgnoreNotFound(err) != nil {
+		return shouldRequeue, err
+	}
+
+	items, err := meta.ExtractList(objList)
+	if err != nil {
+		return shouldRequeue, err
+	}
+
+	for _, item := range items {
+		obj, ok := item.(client.Object)
+		if !ok {
+			// As long as objList is a k8s object
+			// we will never hit this
+			continue
+		}
+		objName := obj.GetName()
+
+		// Only delete what we might have created
+		if owner := metav1.GetControllerOf(obj); owner == nil ||
+			owner.Kind != "PersistentVolumeClaim" ||
+			owner.Name != req.Name {
+			log.Info("Found an object without any owner", "jobName", objName)
+
+			continue
+		}
+
+		// If the name does not match, delete the resource
+		if obj.GetName() != expectedName {
+			if err := c.Delete(ctx, obj); client.IgnoreNotFound(err) != nil {
+				return shouldRequeue, err
+			}
+
+			shouldRequeue = true
+			log.Info("Deleted old job", "jobName", objName)
+		}
+	}
+
+	return shouldRequeue, nil
+}