feat(dp): handle PodDisruptionBudget reconciliation for deployment (#464

) * feat(dp): handle PodDisruptionBudget reconciliation for deployment * Apply suggestions from code review Co-authored-by: Patryk Małek <[email protected]> * address review comments --------- Co-authored-by: Patryk Małek <[email protected]>
Kong · Aug 9, 2024 · ec80e42 · ec80e42
1 parent c43cb8c
commit ec80e42
Show file tree

Hide file tree

Showing 21 changed files with 507 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -32,6 +32,9 @@
 - Introduce `KongPluginInstallation` CRD to allow installing custom Kong
   plugins distributed as container images.
   [400](https://github.com/Kong/gateway-operator/pull/400)
+- Extended `DataPlane` API with a possibility to specify `PodDisruptionBudget` to be
+  created for the `DataPlane` deployments via `spec.resources.podDisruptionBudget`.
+  [#464](https://github.com/Kong/gateway-operator/pull/464)
 
 ### Fixed
 

diff --git a/api/v1beta1/dataplane_types.go b/api/v1beta1/dataplane_types.go
@@ -280,7 +280,7 @@ type DataPlaneStatus struct {
 
 	// Selector contains a unique DataPlane identifier used as a deterministic
 	// label selector that is used throughout its dependent resources.
-	// This is used e.g. as a label selector for DataPlane's Services and Deployments.
+	// This is used e.g. as a label selector for DataPlane's Services, Deployments and PodDisruptionBudgets.
 	//
 	// +kubebuilder:validation:MaxLength=512
 	// +kubebuilder:validation:MinLength=8

diff --git a/config/crd/bases/gateway-operator.konghq.com_dataplanes.yaml b/config/crd/bases/gateway-operator.konghq.com_dataplanes.yaml
@@ -9322,7 +9322,7 @@ spec:
                 description: |-
                   Selector contains a unique DataPlane identifier used as a deterministic
                   label selector that is used throughout its dependent resources.
-                  This is used e.g. as a label selector for DataPlane's Services and Deployments.
+                  This is used e.g. as a label selector for DataPlane's Services, Deployments and PodDisruptionBudgets.
                 maxLength: 512
                 minLength: 8
                 type: string

diff --git a/config/crd/dataplane/gateway-operator.konghq.com_dataplanes.yaml b/config/crd/dataplane/gateway-operator.konghq.com_dataplanes.yaml
@@ -9322,7 +9322,7 @@ spec:
                 description: |-
                   Selector contains a unique DataPlane identifier used as a deterministic
                   label selector that is used throughout its dependent resources.
-                  This is used e.g. as a label selector for DataPlane's Services and Deployments.
+                  This is used e.g. as a label selector for DataPlane's Services, Deployments and PodDisruptionBudgets.
                 maxLength: 512
                 minLength: 8
                 type: string

diff --git a/config/rbac/role/role.yaml b/config/rbac/role/role.yaml
@@ -657,6 +657,18 @@ rules:
   - patch
   - update
   - watch
+- apiGroups:
+  - policy
+  resources:
+  - poddisruptionbudgets
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
 - apiGroups:
   - rbac.authorization.k8s.io
   resources:

diff --git a/controller/dataplane/controller.go b/controller/dataplane/controller.go
@@ -64,8 +64,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	logger := log.GetLogger(ctx, "dataplane", r.DevelopmentMode)
 
 	log.Trace(logger, "reconciling DataPlane resource", req)
+	dpNn := req.NamespacedName
 	dataplane := new(operatorv1beta1.DataPlane)
-	if err := r.Client.Get(ctx, req.NamespacedName, dataplane); err != nil {
+	if err := r.Client.Get(ctx, dpNn, dataplane); err != nil {
 		if k8serrors.IsNotFound(err) {
 			return ctrl.Result{}, nil
 		}
@@ -189,8 +190,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 	deployment, res, err := deploymentBuilder.BuildAndDeploy(ctx, dataplane, r.DevelopmentMode)
 	if err != nil {
-		return ctrl.Result{}, fmt.Errorf("could not build Deployment for DataPlane %s/%s: %w",
-			dataplane.Namespace, dataplane.Name, err)
+		return ctrl.Result{}, fmt.Errorf("could not build Deployment for DataPlane %s: %w",
+			dpNn, err)
 	}
 	if res != op.Noop {
 		return ctrl.Result{}, nil
@@ -204,6 +205,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{}, nil
 	}
 
+	res, _, err = ensurePodDisruptionBudgetForDataPlane(ctx, r.Client, logger, dataplane)
+	if err != nil {
+		return ctrl.Result{}, fmt.Errorf("could not ensure PodDisruptionBudget for DataPlane %s: %w", dpNn, err)
+	}
+	if res != op.Noop {
+		log.Debug(logger, "PodDisruptionBudget created/updated", dataplane)
+		return ctrl.Result{}, nil
+	}
+
 	if res, err := ensureDataPlaneReadyStatus(ctx, r.Client, logger, dataplane, dataplane.Generation); err != nil {
 		return ctrl.Result{}, err
 	} else if res.Requeue {

diff --git a/controller/dataplane/controller_rbac.go b/controller/dataplane/controller_rbac.go
@@ -4,14 +4,15 @@ package dataplane
 // DataPlaneReconciler - RBAC
 // -----------------------------------------------------------------------------
 
-//+kubebuilder:rbac:groups=gateway-operator.konghq.com,resources=dataplanes,verbs=get;list;watch;update;patch
-//+kubebuilder:rbac:groups=gateway-operator.konghq.com,resources=dataplanes/status,verbs=get;update;patch
-//+kubebuilder:rbac:groups=gateway-operator.konghq.com,resources=dataplanes/finalizers,verbs=update
-//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;get;list;watch;update;patch;delete
-//+kubebuilder:rbac:groups=apps,resources=deployments/status,verbs=get
-//+kubebuilder:rbac:groups=core,resources=services,verbs=create;get;list;watch;update;patch;delete
-//+kubebuilder:rbac:groups=core,resources=services/status,verbs=get
-//+kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;watch
-//+kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;delete
-//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch
-//+kubebuilder:rbac:groups=autoscaling,resources=horizontalpodautoscalers,verbs=create;get;list;patch;watch
+// +kubebuilder:rbac:groups=gateway-operator.konghq.com,resources=dataplanes,verbs=get;list;watch;update;patch
+// +kubebuilder:rbac:groups=gateway-operator.konghq.com,resources=dataplanes/status,verbs=get;update;patch
+// +kubebuilder:rbac:groups=gateway-operator.konghq.com,resources=dataplanes/finalizers,verbs=update
+// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;get;list;watch;update;patch;delete
+// +kubebuilder:rbac:groups=apps,resources=deployments/status,verbs=get
+// +kubebuilder:rbac:groups=core,resources=services,verbs=create;get;list;watch;update;patch;delete
+// +kubebuilder:rbac:groups=core,resources=services/status,verbs=get
+// +kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;watch
+// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;delete
+// +kubebuilder:rbac:groups="",resources=events,verbs=create;patch
+// +kubebuilder:rbac:groups=autoscaling,resources=horizontalpodautoscalers,verbs=create;get;list;patch;watch
+// +kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=create;get;list;watch;update;patch
diff --git a/controller/dataplane/owned_resources.go b/controller/dataplane/owned_resources.go
@@ -11,6 +11,7 @@ import (
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	certificatesv1 "k8s.io/api/certificates/v1"
 	corev1 "k8s.io/api/core/v1"
+	policyv1 "k8s.io/api/policy/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/selection"
@@ -113,6 +114,62 @@ func ensureHPAForDataPlane(
 	return op.Created, nil, nil
 }
 
+func ensurePodDisruptionBudgetForDataPlane(
+	ctx context.Context,
+	cl client.Client,
+	log logr.Logger,
+	dataplane *operatorv1beta1.DataPlane,
+) (res op.Result, pdb *policyv1.PodDisruptionBudget, err error) {
+	dpNn := client.ObjectKeyFromObject(dataplane)
+	matchingLabels := k8sresources.GetManagedLabelForOwner(dataplane)
+	pdbs, err := k8sutils.ListPodDisruptionBudgetsForOwner(ctx, cl, dataplane.Namespace, dataplane.UID, matchingLabels)
+	if err != nil {
+		return op.Noop, nil, fmt.Errorf("failed listing PodDisruptionBudgets for DataPlane %s: %w", dpNn, err)
+	}
+
+	if dataplane.Spec.Resources.PodDisruptionBudget == nil {
+		if err := k8sreduce.ReducePodDisruptionBudgets(ctx, cl, pdbs, k8sreduce.FilterNone); err != nil {
+			return op.Noop, nil, fmt.Errorf("failed reducing PodDisruptionBudgets for DataPlane %s: %w", dpNn, err)
+		}
+		return op.Noop, nil, nil
+	}
+
+	if len(pdbs) > 1 {
+		if err := k8sreduce.ReducePodDisruptionBudgets(ctx, cl, pdbs, k8sreduce.FilterPodDisruptionBudgets); err != nil {
+			return op.Noop, nil, fmt.Errorf("failed reducing PodDisruptionBudgets for DataPlane %s: %w", dpNn, err)
+		}
+		return op.Noop, nil, nil
+	}
+
+	generatedPDB, err := k8sresources.GeneratePodDisruptionBudgetForDataPlane(dataplane)
+	if err != nil {
+		return op.Noop, nil, fmt.Errorf("failed generating PodDisruptionBudget for DataPlane %s: %w", dpNn, err)
+	}
+
+	if len(pdbs) == 1 {
+		var updated bool
+		existingPDB := &pdbs[0]
+		oldExistingPDB := existingPDB.DeepCopy()
+
+		// Ensure that PDB's metadata is up-to-date.
+		updated, existingPDB.ObjectMeta = k8sutils.EnsureObjectMetaIsUpdated(existingPDB.ObjectMeta, generatedPDB.ObjectMeta)
+
+		// Ensure that PDB's spec is up-to-date.
+		if !cmp.Equal(existingPDB.Spec, generatedPDB.Spec) {
+			existingPDB.Spec = generatedPDB.Spec
+			updated = true
+		}
+
+		return patch.ApplyPatchIfNonEmpty(ctx, cl, log, existingPDB, oldExistingPDB, dataplane, updated)
+	}
+
+	if err := cl.Create(ctx, generatedPDB); err != nil {
+		return op.Noop, nil, fmt.Errorf("failed creating PodDisruptionBudget for DataPlane %s: %w", dpNn, err)
+	}
+
+	return op.Created, generatedPDB, nil
+}
+
 func matchingLabelsToServiceOpt(ml client.MatchingLabels) k8sresources.ServiceOpt {
 	return func(s *corev1.Service) {
 		if s.Labels == nil {

diff --git a/controller/dataplane/watch.go b/controller/dataplane/watch.go
@@ -4,6 +4,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	corev1 "k8s.io/api/core/v1"
+	policyv1 "k8s.io/api/policy/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 
@@ -15,14 +16,16 @@ import (
 // the operator.
 func DataPlaneWatchBuilder(mgr ctrl.Manager) *builder.Builder {
 	return ctrl.NewControllerManagedBy(mgr).
-		// watch DataPlane objects
+		// Watch DataPlane objects.
 		For(&operatorv1beta1.DataPlane{}).
-		// watch for changes in Secrets created by the dataplane controller
+		// Watch for changes in Secrets created by the dataplane controller.
 		Owns(&corev1.Secret{}).
-		// watch for changes in Services created by the dataplane controller
+		// Watch for changes in Services created by the dataplane controller.
 		Owns(&corev1.Service{}).
-		// watch for changes in Deployments created by the dataplane controller
+		// Watch for changes in Deployments created by the dataplane controller.
 		Owns(&appsv1.Deployment{}).
-		// watch for changes in HPA created by the dataplane controller
-		Owns(&autoscalingv2.HorizontalPodAutoscaler{})
+		// Watch for changes in HPA created by the dataplane controller.
+		Owns(&autoscalingv2.HorizontalPodAutoscaler{}).
+		// Watch for changes in PodDisruptionBudgets created by the dataplane controller.
+		Owns(&policyv1.PodDisruptionBudget{})
 }
diff --git a/controller/konnect/constraints.go b/controller/konnect/constraints.go
@@ -24,11 +24,8 @@ type SupportedKonnectEntityType interface {
 // Separating this from SupportedKonnectEntityType allows us to use EntityType
 // where client.Object is required, since it embeds client.Object and uses pointer
 // to refer to the SupportedKonnectEntityType.
-type EntityType[
-	T SupportedKonnectEntityType,
-] interface {
+type EntityType[T SupportedKonnectEntityType] interface {
 	*T
-
 	// Kubernetes Object methods
 	GetObjectMeta() metav1.Object
 	client.Object

diff --git a/controller/pkg/patch/patch.go b/controller/pkg/patch/patch.go
@@ -9,6 +9,7 @@ import (
 	"github.com/go-logr/logr"
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
+	policyv1 "k8s.io/api/policy/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
 
@@ -23,7 +24,7 @@ import (
 func ApplyPatchIfNonEmpty[
 	OwnerT *operatorv1beta1.DataPlane | *operatorv1beta1.ControlPlane,
 	ResourceT interface {
-		*appsv1.Deployment | *autoscalingv2.HorizontalPodAutoscaler | *certmanagerv1.Certificate
+		*appsv1.Deployment | *autoscalingv2.HorizontalPodAutoscaler | *certmanagerv1.Certificate | *policyv1.PodDisruptionBudget
 		client.Object
 	},
 ](

diff --git a/docs/api-reference.md b/docs/api-reference.md
@@ -894,7 +894,7 @@ DataPlaneStatus defines the observed state of DataPlane
 | `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#condition-v1-meta) array_ | Conditions describe the status of the DataPlane. |
 | `service` _string_ | Service indicates the Service that exposes the DataPlane's configured routes |
 | `addresses` _[Address](#address) array_ | Addresses lists the addresses that have actually been bound to the DataPlane. |
-| `selector` _string_ | Selector contains a unique DataPlane identifier used as a deterministic label selector that is used throughout its dependent resources. This is used e.g. as a label selector for DataPlane's Services and Deployments. |
+| `selector` _string_ | Selector contains a unique DataPlane identifier used as a deterministic label selector that is used throughout its dependent resources. This is used e.g. as a label selector for DataPlane's Services, Deployments and PodDisruptionBudgets. |
 | `readyReplicas` _integer_ | ReadyReplicas indicates how many replicas have reported to be ready. |
 | `replicas` _integer_ | Replicas indicates how many replicas have been set for the DataPlane. |
 | `rollout` _[DataPlaneRolloutStatus](#dataplanerolloutstatus)_ | RolloutStatus contains information about the rollout. It is set only if a rollout strategy was configured in the spec. |

diff --git a/pkg/utils/kubernetes/lists.go b/pkg/utils/kubernetes/lists.go
@@ -8,6 +8,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	corev1 "k8s.io/api/core/v1"
+	policyv1 "k8s.io/api/policy/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -83,6 +84,41 @@ func ListHPAsForOwner(
 	return hpas, nil
 }
 
+// ListPodDisruptionBudgetsForOwner is a helper function which gets a list of PodDisruptionBudget
+// using the provided list options and reduce by OwnerReference UID and namespace to efficiently
+// list only the objects owned by the provided UID.
+func ListPodDisruptionBudgetsForOwner(
+	ctx context.Context,
+	c client.Client,
+	namespace string,
+	uid types.UID,
+	listOpts ...client.ListOption,
+) ([]policyv1.PodDisruptionBudget, error) {
+	pdbList := &policyv1.PodDisruptionBudgetList{}
+
+	err := c.List(
+		ctx,
+		pdbList,
+		append(
+			[]client.ListOption{client.InNamespace(namespace)},
+			listOpts...,
+		)...,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	var pdbs []policyv1.PodDisruptionBudget
+	for _, pdb := range pdbList.Items {
+		pdb := pdb
+		if IsOwnedByRefUID(&pdb, uid) {
+			pdbs = append(pdbs, pdb)
+		}
+	}
+
+	return pdbs, nil
+}
+
 // ListServicesForOwner is a helper function which gets a list of Services
 // using the provided list options and reduce by OwnerReference UID and namespace to efficiently
 // list only the objects owned by the provided UID.

diff --git a/pkg/utils/kubernetes/reduce/filters.go b/pkg/utils/kubernetes/reduce/filters.go
@@ -8,6 +8,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	discoveryv1 "k8s.io/api/discovery/v1"
 	networkingv1 "k8s.io/api/networking/v1"
+	policyv1 "k8s.io/api/policy/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 
 	operatorv1beta1 "github.com/kong/gateway-operator/api/v1beta1"
@@ -367,6 +368,29 @@ func FilterHPAs(hpas []autoscalingv2.HorizontalPodAutoscaler) []autoscalingv2.Ho
 	return append(hpas[:best], hpas[best+1:]...)
 }
 
+// -----------------------------------------------------------------------------
+// Filter functions - PodDisruptionBudgets
+// -----------------------------------------------------------------------------
+
+// FilterPodDisruptionBudgets filters out the PodDisruptionBudgets to be kept and returns all
+// the PodDisruptionBudgets to be deleted.
+// The filtered-out PodDisruptionBudget is decided as follows:
+// 1. creationTimestamp (older is better)
+func FilterPodDisruptionBudgets(pdbs []policyv1.PodDisruptionBudget) []policyv1.PodDisruptionBudget {
+	if len(pdbs) < 2 {
+		return nil
+	}
+
+	best := 0
+	for i, hpa := range pdbs {
+		if hpa.CreationTimestamp.Before(&pdbs[best].CreationTimestamp) {
+			best = i
+		}
+	}
+
+	return append(pdbs[:best], pdbs[best+1:]...)
+}
+
 // -----------------------------------------------------------------------------
 // Filter functions - ValidatingWebhookConfigurations
 // -----------------------------------------------------------------------------