Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion api/v1beta1/dataplane_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions config/rbac/role/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
16 changes: 13 additions & 3 deletions controller/dataplane/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
23 changes: 12 additions & 11 deletions controller/dataplane/controller_rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
57 changes: 57 additions & 0 deletions controller/dataplane/owned_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 9 additions & 6 deletions controller/dataplane/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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{})
}
5 changes: 1 addition & 4 deletions controller/konnect/constraints.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion controller/pkg/patch/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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
},
](
Expand Down
2 changes: 1 addition & 1 deletion docs/api-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand Down
36 changes: 36 additions & 0 deletions pkg/utils/kubernetes/lists.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down
24 changes: 24 additions & 0 deletions pkg/utils/kubernetes/reduce/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
czeslavo marked this conversation as resolved.
Show resolved Hide resolved
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
// -----------------------------------------------------------------------------
Expand Down
Loading
Loading