fix: set finalizer in Works (#167)

- set finalizer in works to clean up its workplacements
before deleting work

controllers/destination_controller.go

@@ -49,10 +49,6 @@ type DestinationReconciler struct {
 //+kubebuilder:rbac:groups=platform.kratix.io,resources=destinations/status,verbs=get;update;patch
 //+kubebuilder:rbac:groups=platform.kratix.io,resources=destinations/finalizers,verbs=update
 
-// Reconcile is part of the main kubernetes reconciliation loop which aims to
-// move the current state of the destination closer to the desired state.
-// For more details, check Reconcile and its Result here:
-// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.11.0/pkg/reconcile
 func (r *DestinationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	logger := r.Log.WithValues(
 		"destination", req.NamespacedName,

controllers/promise_controller.go

@@ -170,7 +170,7 @@ func (r *PromiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		return *requeue, nil
 	}
 
-	//TODO add workflowFinalizzer if deletes exist (currently we only add it if we have a configure pipeline)
+	//TODO add workflowFinalizer if deletes exist (currently we only add it if we have a configure pipeline)
 
 	var rrCRD *apiextensionsv1.CustomResourceDefinition
 	var rrGVK schema.GroupVersionKind

controllers/work_controller.go

@@ -18,14 +18,17 @@ package controllers
 
 import (
 	"context"
-
 	"github.com/go-logr/logr"
 	"github.com/syntasso/kratix/api/v1alpha1"
 	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
+const workCleanUpFinalizer = v1alpha1.KratixPrefix + "work-cleanup"
+
 // WorkReconciler reconciles a Work object
 type WorkReconciler struct {
 	Client    client.Client
@@ -64,6 +67,17 @@ func (r *WorkReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 		return ctrl.Result{Requeue: false}, err
 	}
 
+	if !work.DeletionTimestamp.IsZero() {
+		return r.deleteWork(ctx, work)
+	}
+
+	if !controllerutil.ContainsFinalizer(work, workCleanUpFinalizer) {
+		return addFinalizers(opts{
+			client: r.Client,
+			logger: r.Log,
+			ctx:    ctx}, work, []string{workFinalizer})
+	}
+
 	logger.Info("Requesting scheduling for Work")
 	unscheduledWorkloadGroupIDs, err := r.Scheduler.ReconcileWork(work)
 	if err != nil {
@@ -82,6 +96,29 @@ func (r *WorkReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 
 }
 
+func (r *WorkReconciler) deleteWork(ctx context.Context, work *v1alpha1.Work) (ctrl.Result, error) {
+	workplacementGVK := schema.GroupVersionKind{
+		Group:   v1alpha1.GroupVersion.Group,
+		Version: v1alpha1.GroupVersion.Version,
+		Kind:    "WorkPlacement",
+	}
+
+	resourcesRemaining, err := deleteAllResourcesWithKindMatchingLabel(opts{client: r.Client, logger: r.Log, ctx: ctx},
+		workplacementGVK, map[string]string{workLabelKey: work.Name})
+	if err != nil {
+		return defaultRequeue, err
+	}
+
+	if !resourcesRemaining {
+		controllerutil.RemoveFinalizer(work, workCleanUpFinalizer)
+		err = r.Client.Update(ctx, work)
+		if err != nil {
+			return defaultRequeue, err
+		}
+	}
+	return defaultRequeue, nil
+}
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *WorkReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).

controllers/work_controller_test.go

@@ -19,17 +19,15 @@ package controllers_test
 import (
 	"context"
 	"errors"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/syntasso/kratix/api/v1alpha1"
 	"github.com/syntasso/kratix/controllers"
 	"github.com/syntasso/kratix/controllers/controllersfakes"
 	"github.com/syntasso/kratix/lib/hash"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
-
-	"github.com/syntasso/kratix/api/v1alpha1"
 	//+kubebuilder:scaffold:imports
 )
 
@@ -165,4 +163,45 @@ var _ = Describe("WorkReconciler", func() {
 			})
 		})
 	})
+
+	When("work is deleted", func() {
+		BeforeEach(func() {
+			fakeScheduler.ReconcileWorkReturns([]string{}, nil)
+			Expect(fakeK8sClient.Create(ctx, work)).To(Succeed())
+			Expect(fakeK8sClient.Get(ctx, workName, work)).To(Succeed())
+		})
+
+		It("succeeds", func() {
+			result, err := t.reconcileUntilCompletion(reconciler, work)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(result).To(Equal(ctrl.Result{}))
+
+			By("setting the finalizer on work on creation")
+			work := &v1alpha1.Work{}
+			Expect(fakeK8sClient.Get(ctx, workName, work)).
+				To(Succeed())
+			Expect(work.GetFinalizers()).To(ContainElement("kratix.io/work-cleanup"))
+
+			By("cleaning up workplacement with matching label on deletion")
+			workPlacement := v1alpha1.WorkPlacement{
+				ObjectMeta: v1.ObjectMeta{
+					Name:      work.Name,
+					Namespace: "default",
+					Labels:    map[string]string{v1alpha1.KratixPrefix + "work": work.Name},
+				},
+				Spec: v1alpha1.WorkPlacementSpec{TargetDestinationName: "test-destination"},
+			}
+			Expect(fakeK8sClient.Create(ctx, &workPlacement)).To(Succeed())
+			Expect(fakeK8sClient.Get(ctx, types.NamespacedName{Name: workPlacement.Name, Namespace: workPlacement.Namespace},
+				&v1alpha1.WorkPlacement{})).To(Succeed())
+
+			Expect(fakeK8sClient.Delete(ctx, work)).To(Succeed())
+			_, err = t.reconcileUntilCompletion(reconciler, work)
+			Expect(err).NotTo(HaveOccurred())
+
+			Expect(fakeK8sClient.Get(ctx, workName, work)).To(MatchError(ContainSubstring("not found")))
+			Expect(fakeK8sClient.Get(ctx, types.NamespacedName{Name: workPlacement.Name, Namespace: workPlacement.Namespace},
+				&v1alpha1.WorkPlacement{})).To(MatchError(ContainSubstring("not found")))
+		})
+	})
 })

controllers/workplacement_controller.go

@@ -31,7 +31,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	"github.com/syntasso/kratix/api/v1alpha1"
-	"github.com/syntasso/kratix/lib/resourceutil"
 	"github.com/syntasso/kratix/lib/writers"
 )
 
@@ -54,16 +53,10 @@ type WorkPlacementReconciler struct {
 
 const repoCleanupWorkPlacementFinalizer = "finalizers.workplacement.kratix.io/repo-cleanup"
 
-var workPlacementFinalizers = []string{repoCleanupWorkPlacementFinalizer}
-
 //+kubebuilder:rbac:groups=platform.kratix.io,resources=workplacements,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=platform.kratix.io,resources=workplacements/status,verbs=get;update;patch
 //+kubebuilder:rbac:groups=platform.kratix.io,resources=workplacements/finalizers,verbs=update
 
-// Reconcile is part of the main kubernetes reconciliation loop which aims to
-// move the current state of the destination closer to the desired state.
-// For more details, check Reconcile and its Result here:
-// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.11.0/pkg/reconcile
 func (r *WorkPlacementReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	logger := r.Log.WithValues("work-placement-controller", req.NamespacedName)
 
@@ -106,8 +99,8 @@ func (r *WorkPlacementReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return r.deleteWorkPlacement(ctx, writer, workPlacement, destination.GetFilepathMode(), logger)
 	}
 
-	if resourceutil.FinalizersAreMissing(workPlacement, workPlacementFinalizers) {
-		return addFinalizers(opts, workPlacement, workPlacementFinalizers)
+	if !controllerutil.ContainsFinalizer(workPlacement, repoCleanupWorkPlacementFinalizer) {
+		return addFinalizers(opts, workPlacement, []string{repoCleanupWorkPlacementFinalizer})
 	}
 
 	versionID, err := r.writeWorkloadsToStateStore(writer, *workPlacement, *destination, logger)