Merge pull request #77 from PDOK/wr/pdb-fix

Added check and tests that PDB gets removed/not created if replicas == 1

Author: Shalucik

internal/controller/shared_controller.go

@@ -6,6 +6,8 @@ import (
 	"strconv"
 	"time"
 
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
 	"github.com/pdok/smooth-operator/model"
 
 	"github.com/pdok/mapserver-operator/internal/controller/constants"
@@ -105,12 +107,9 @@ func createOrUpdateAllForWMSWFS[R Reconciler, O pdoknlv3.WMSWFS](ctx context.Con
 
 	// region PodDisruptionBudget
 	{
-		podDisruptionBudget := getBarePodDisruptionBudget(obj)
-		operationResults[smoothoperatorutils.GetObjectFullName(reconcilerClient, podDisruptionBudget)], err = controllerutil.CreateOrUpdate(ctx, reconcilerClient, podDisruptionBudget, func() error {
-			return mutatePodDisruptionBudget(r, obj, podDisruptionBudget)
-		})
+		err = createOrUpdateOrDeletePodDisruptionBudget(ctx, r, obj, operationResults)
 		if err != nil {
-			return operationResults, fmt.Errorf("unable to create/update resource %s: %w", smoothoperatorutils.GetObjectFullName(reconcilerClient, podDisruptionBudget), err)
+			return operationResults, err
 		}
 	}
 	// end region PodDisruptionBudget
@@ -227,3 +226,26 @@ func createOrUpdateConfigMap[O pdoknlv3.WMSWFS, R Reconciler](ctx context.Contex
 	}
 	return cm, &or, nil
 }
+
+func createOrUpdateOrDeletePodDisruptionBudget[O pdoknlv3.WMSWFS, R Reconciler](ctx context.Context, reconciler R, obj O, operationResults map[string]controllerutil.OperationResult) (err error) {
+	reconcilerClient := getReconcilerClient(reconciler)
+	podDisruptionBudget := getBarePodDisruptionBudget(obj)
+	if obj.HorizontalPodAutoscalerPatch().MinReplicas != nil && obj.HorizontalPodAutoscalerPatch().MaxReplicas != nil &&
+		*obj.HorizontalPodAutoscalerPatch().MinReplicas == 1 && *obj.HorizontalPodAutoscalerPatch().MaxReplicas == 1 {
+		err = reconcilerClient.Delete(ctx, podDisruptionBudget)
+		if err == nil {
+			operationResults[smoothoperatorutils.GetObjectFullName(reconcilerClient, podDisruptionBudget)] = "deleted"
+		}
+		if client.IgnoreNotFound(err) != nil {
+			return fmt.Errorf("unable to delete resource %s: %w", smoothoperatorutils.GetObjectFullName(reconcilerClient, podDisruptionBudget), err)
+		}
+	} else {
+		operationResults[smoothoperatorutils.GetObjectFullName(reconcilerClient, podDisruptionBudget)], err = controllerutil.CreateOrUpdate(ctx, reconcilerClient, podDisruptionBudget, func() error {
+			return mutatePodDisruptionBudget(reconciler, obj, podDisruptionBudget)
+		})
+		if err != nil {
+			return fmt.Errorf("unable to create/update resource %s: %w", smoothoperatorutils.GetObjectFullName(reconcilerClient, podDisruptionBudget), err)
+		}
+	}
+	return nil
+}

internal/controller/wfs_controller_test.go

@@ -33,6 +33,8 @@ import (
 	"github.com/pdok/smooth-operator/model"
 	smoothoperatorutils "github.com/pdok/smooth-operator/pkg/util"
 	k8stypes "k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/ptr"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	. "github.com/onsi/ginkgo/v2" //nolint:revive // ginkgo bdd
 	. "github.com/onsi/gomega"    //nolint:revive // ginkgo bdd
@@ -179,6 +181,58 @@ var _ = Describe("Testing WFS Controller", func() {
 			}, "10s", "1s").Should(BeTrue())
 		})
 
+		It("Should delete PodDisruptionBudget if Min and Max replicas == 1 ", func() {
+			controllerReconciler := getWFSReconciler()
+
+			By("Setting Min and Max replicas to 1")
+
+			Expect(k8sClient.Get(ctx, objectKeyWfs, clusterWfs)).To(Succeed())
+
+			resource := clusterWfs.DeepCopy()
+
+			resource.Spec.HorizontalPodAutoscalerPatch.MinReplicas = ptr.To(int32(1))
+			resource.Spec.HorizontalPodAutoscalerPatch.MaxReplicas = ptr.To(int32(1))
+
+			Expect(k8sClient.Update(ctx, resource)).To(Succeed())
+
+			podDisruptionBudget := getBarePodDisruptionBudget(resource)
+
+			By("Reconciling the WFS")
+			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: objectKeyWfs})
+			Expect(err).NotTo(HaveOccurred())
+
+			By("Getting the PodDisruptionBudget")
+			err = k8sClient.Get(ctx, client.ObjectKeyFromObject(podDisruptionBudget), podDisruptionBudget)
+			Expect(apierrors.IsNotFound(err)).To(BeTrue())
+
+			Expect(k8sClient.Get(ctx, objectKeyWfs, clusterWfs)).To(Succeed())
+			Expect(clusterWfs.Status.OperationResults[smoothoperatorutils.GetObjectFullName(k8sClient, podDisruptionBudget)]).To(Equal(controllerutil.OperationResult("deleted")))
+		})
+
+		It("Should not Create PodDisruptionBudget if Min and Max replicas == 1 ", func() {
+			controllerReconciler := getWFSReconciler()
+
+			By("Getting Cluster WFS Min and Max replicas to 1")
+			Expect(k8sClient.Get(ctx, objectKeyWfs, clusterWfs)).To(Succeed())
+
+			Expect(clusterWfs.HorizontalPodAutoscalerPatch().MaxReplicas).To(Equal(ptr.To(int32(1))))
+			Expect(clusterWfs.HorizontalPodAutoscalerPatch().MinReplicas).To(Equal(ptr.To(int32(1))))
+
+			podDisruptionBudget := getBarePodDisruptionBudget(clusterWfs)
+
+			By("Reconciling the WFS")
+			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: objectKeyWfs})
+			Expect(err).NotTo(HaveOccurred())
+
+			By("Getting the PodDisruptionBudget")
+			err = k8sClient.Get(ctx, client.ObjectKeyFromObject(podDisruptionBudget), podDisruptionBudget)
+			Expect(apierrors.IsNotFound(err)).To(BeTrue())
+
+			Expect(k8sClient.Get(ctx, objectKeyWfs, clusterWfs)).To(Succeed())
+			_, ok := clusterWfs.Status.OperationResults[smoothoperatorutils.GetObjectFullName(k8sClient, podDisruptionBudget)]
+			Expect(ok).To(BeFalse())
+		})
+
 		It("Respects the TTL of the WFS", func() {
 			By("Creating a new resource for the Kind WFS")