Do not add finalizers automatically.

Author: Gerrit91

controllers/deployment/reconcile.go

@@ -14,6 +14,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
 func (c *controller) Reconcile(r *controllers.Ctx[*v2.FirewallDeployment]) error {
@@ -138,6 +139,8 @@ func (c *controller) createFirewallSet(r *controllers.Ctx[*v2.FirewallDeployment
 		},
 	}
 
+	_ = controllerutil.AddFinalizer(set, v2.FinalizerName)
+
 	err = c.c.GetSeedClient().Create(r.Ctx, set, &client.CreateOptions{})
 	if err != nil {
 		cond := v2.NewCondition(v2.FirewallDeplomentProgressing, v2.ConditionFalse, "FirewallSetCreateError", fmt.Sprintf("Error creating firewall set: %s.", err))

controllers/generic_controller.go

@@ -97,36 +97,31 @@ func (g GenericController[O]) Reconcile(ctx context.Context, req ctrl.Request) (
 	}
 
 	if !o.GetDeletionTimestamp().IsZero() {
-		if controllerutil.ContainsFinalizer(o, v2.FinalizerName) {
-			log.Info("reconciling resource deletion flow")
-			err := g.reconciler.Delete(rctx)
-			if err != nil {
-				var requeueErr *requeueError
-				if errors.As(err, &requeueErr) {
-					log.Info(requeueErr.Error())
-					return ctrl.Result{RequeueAfter: requeueErr.after}, nil //nolint:nilerr we need to return nil such that the requeue works
-				}
-
-				log.Error(err, "error during deletion flow")
-				return ctrl.Result{}, err
-			}
+		if !controllerutil.ContainsFinalizer(o, v2.FinalizerName) {
+			log.Info("resource has no finalizer anymore, nothing further to do for deletion")
+			return ctrl.Result{}, nil
+		}
 
-			log.Info("deletion finished, removing finalizer")
-			controllerutil.RemoveFinalizer(o, v2.FinalizerName)
-			if err := g.c.Update(ctx, o); err != nil {
-				return ctrl.Result{}, err
+		log.Info("reconciling resource deletion flow")
+		err := g.reconciler.Delete(rctx)
+		if err != nil {
+			var requeueErr *requeueError
+			if errors.As(err, &requeueErr) {
+				log.Info(requeueErr.Error())
+				return ctrl.Result{RequeueAfter: requeueErr.after}, nil //nolint:nilerr we need to return nil such that the requeue works
 			}
-		}
 
-		return ctrl.Result{}, nil
-	}
+			log.Error(err, "error during deletion flow")
+			return ctrl.Result{}, err
+		}
 
-	if !controllerutil.ContainsFinalizer(o, v2.FinalizerName) {
-		log.Info("adding finalizer")
-		controllerutil.AddFinalizer(o, v2.FinalizerName)
+		log.Info("deletion finished, removing finalizer")
+		controllerutil.RemoveFinalizer(o, v2.FinalizerName)
 		if err := g.c.Update(ctx, o); err != nil {
-			return ctrl.Result{}, fmt.Errorf("unable to add finalizer: %w", err)
+			return ctrl.Result{}, err
 		}
+
+		return ctrl.Result{}, nil
 	}
 
 	var statusErr error

controllers/monitor/reconcile.go

@@ -23,7 +23,7 @@ func (c *controller) Reconcile(r *controllers.Ctx[*v2.FirewallMonitor]) error {
 	fw, err := c.updateFirewallStatus(r)
 	if err != nil {
 		r.Log.Error(err, "unable to update firewall status")
-		return controllers.RequeueAfter(3*time.Second, "unable to update firewall status, retrying")
+		return fmt.Errorf("unable to update firewall status: %w", err)
 	}
 
 	err = c.offerFirewallControllerMigrationSecret(r, fw)

controllers/set/controller_test.go

@@ -108,6 +108,10 @@ var _ = Context("firewall set controller", Ordered, func() {
 				if !fw.CreationTimestamp.Time.Before(newest.CreationTimestamp.Time) {
 					newest = &fw
 				}
+
+				// as there is no firewall controller running, we need to take out the finalizers
+				fw.Finalizers = nil
+				Expect(k8sClient.Update(ctx, &fw)).To(Succeed())
 			}
 
 			Expect(newest).NotTo(BeNil())

controllers/set/reconcile.go

@@ -9,6 +9,7 @@ import (
 	"github.com/metal-stack/firewall-controller-manager/controllers"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
 func (c *controller) Reconcile(r *controllers.Ctx[*v2.FirewallSet]) error {
@@ -163,6 +164,8 @@ func (c *controller) createFirewall(r *controllers.Ctx[*v2.FirewallSet]) (*v2.Fi
 		Distance:   r.Target.Spec.Distance,
 	}
 
+	_ = controllerutil.AddFinalizer(fw, v2.FinalizerName)
+
 	err = c.c.GetSeedClient().Create(r.Ctx, fw, &client.CreateOptions{})
 	if err != nil {
 		return nil, fmt.Errorf("unable to create firewall resource: %w", err)
@@ -204,6 +207,7 @@ func (c *controller) adoptFirewall(r *controllers.Ctx[*v2.FirewallSet], fw *v2.F
 	}
 
 	fw.OwnerReferences = append(fw.OwnerReferences, *metav1.NewControllerRef(r.Target, v2.GroupVersion.WithKind("FirewallSet")))
+	_ = controllerutil.AddFinalizer(fw, v2.FinalizerName)
 
 	err = c.c.GetSeedClient().Update(r.Ctx, fw)
 	if err != nil {

integration/integration_test.go

@@ -123,8 +123,9 @@ var _ = Context("integration test", Ordered, func() {
 		deployment = func() *v2.FirewallDeployment {
 			return &v2.FirewallDeployment{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "test",
-					Namespace: namespaceName,
+					Name:       "test",
+					Namespace:  namespaceName,
+					Finalizers: []string{v2.FinalizerName},
 				},
 				Spec: v2.FirewallDeploymentSpec{
 					Replicas: 1,
@@ -1551,8 +1552,9 @@ var _ = Context("integration test", Ordered, func() {
 			deployment = func() *v2.FirewallDeployment {
 				return &v2.FirewallDeployment{
 					ObjectMeta: metav1.ObjectMeta{
-						Name:      "test",
-						Namespace: namespaceName,
+						Name:       "test",
+						Namespace:  namespaceName,
+						Finalizers: []string{v2.FinalizerName},
 					},
 					Spec: v2.FirewallDeploymentSpec{
 						Replicas: 1,