From 6b6c9b99d8a871f6b9b571cb2029516289b48a9b Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Tue, 2 Jun 2026 00:54:52 +0400 Subject: [PATCH] fix(k8s): default PodDisruptionBudget to maxUnavailable:1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The default PDB was minAvailable:1, which on a single-replica Deployment resolves to disruptionsAllowed:0. That forbids the eviction API, so the cluster autoscaler can never drain the pod's node — on GKE Autopilot the node can never be consolidated and stays pinned regardless of utilization, billing an underutilized node per single-replica service. Default to maxUnavailable:1 instead: disruptionsAllowed:1 lets the pod be evicted and rescheduled for consolidation, while still bounding voluntary disruptions to one pod at a time for multi-replica deployments. Explicitly configured budgets are honored unchanged. Default extracted into a testable helper with coverage. Signed-off-by: Dmitrii Creed --- pkg/clouds/pulumi/kubernetes/deployment.go | 89 +++++++++++-------- .../kubernetes/deployment_disruption_test.go | 56 ++++++++++++ 2 files changed, 108 insertions(+), 37 deletions(-) create mode 100644 pkg/clouds/pulumi/kubernetes/deployment_disruption_test.go diff --git a/pkg/clouds/pulumi/kubernetes/deployment.go b/pkg/clouds/pulumi/kubernetes/deployment.go index 6d1d0487..24c7d908 100644 --- a/pkg/clouds/pulumi/kubernetes/deployment.go +++ b/pkg/clouds/pulumi/kubernetes/deployment.go @@ -220,43 +220,41 @@ func DeploySimpleContainer(ctx *sdk.Context, args Args, opts ...sdk.ResourceOpti args.Params.Log.Warn(ctx.Context(), "configure simple container deployment for %q in %q", stackName, stackEnv) sc, err := NewSimpleContainer(ctx, &SimpleContainerArgs{ - KubeProvider: args.KubeProvider, - ComputeContext: args.ComputeContext, - ServiceType: args.ServiceType, - ExternalTrafficPolicy: args.ExternalTrafficPolicy, - UseSSL: args.UseSSL, - ProvisionIngress: args.ProvisionIngress, - Namespace: namespace, - Service: deploymentName, - Deployment: deploymentName, - ScEnv: stackEnv, - IngressContainer: args.Deployment.IngressContainer, - Domain: lo.FromPtr(args.Deployment.StackConfig).Domain, - Prefix: lo.FromPtr(args.Deployment.StackConfig).Prefix, - ProxyKeepPrefix: lo.FromPtr(args.Deployment.StackConfig).ProxyKeepPrefix, - ParentStack: lo.If(args.Params.ParentStack != nil, lo.ToPtr(lo.FromPtr(args.Params.ParentStack).FullReference)).Else(nil), - ParentEnv: lo.If(parentEnv != "", lo.ToPtr(parentEnv)).Else(nil), - Replicas: replicas, - Headers: args.Deployment.Headers, - SecretEnvs: mergedSecretEnvs, - LbConfig: args.Deployment.StackConfig.LBConfig, - Volumes: args.Deployment.TextVolumes, - PersistentVolumes: pvs, - EphemeralVolumes: args.Deployment.EphemeralVolumes, // Pass generic ephemeral volumes configuration - PriorityClassName: args.Deployment.PriorityClassName, - Containers: containers, - ServiceAccountName: args.ServiceAccountName, - InitContainers: args.InitContainers, - GenerateCaddyfileEntry: args.GenerateCaddyfileEntry, - Annotations: args.Annotations, - NodeSelector: args.NodeSelector, - Affinity: args.Affinity, - Sidecars: args.Sidecars, - VPA: args.VPA, // Pass VPA configuration to SimpleContainer - Scale: args.Deployment.Scale, // Pass Scale configuration to SimpleContainer - PodDisruption: lo.If(args.Deployment.DisruptionBudget != nil, args.Deployment.DisruptionBudget).Else(&k8s.DisruptionBudget{ - MinAvailable: lo.ToPtr(1), - }), + KubeProvider: args.KubeProvider, + ComputeContext: args.ComputeContext, + ServiceType: args.ServiceType, + ExternalTrafficPolicy: args.ExternalTrafficPolicy, + UseSSL: args.UseSSL, + ProvisionIngress: args.ProvisionIngress, + Namespace: namespace, + Service: deploymentName, + Deployment: deploymentName, + ScEnv: stackEnv, + IngressContainer: args.Deployment.IngressContainer, + Domain: lo.FromPtr(args.Deployment.StackConfig).Domain, + Prefix: lo.FromPtr(args.Deployment.StackConfig).Prefix, + ProxyKeepPrefix: lo.FromPtr(args.Deployment.StackConfig).ProxyKeepPrefix, + ParentStack: lo.If(args.Params.ParentStack != nil, lo.ToPtr(lo.FromPtr(args.Params.ParentStack).FullReference)).Else(nil), + ParentEnv: lo.If(parentEnv != "", lo.ToPtr(parentEnv)).Else(nil), + Replicas: replicas, + Headers: args.Deployment.Headers, + SecretEnvs: mergedSecretEnvs, + LbConfig: args.Deployment.StackConfig.LBConfig, + Volumes: args.Deployment.TextVolumes, + PersistentVolumes: pvs, + EphemeralVolumes: args.Deployment.EphemeralVolumes, // Pass generic ephemeral volumes configuration + PriorityClassName: args.Deployment.PriorityClassName, + Containers: containers, + ServiceAccountName: args.ServiceAccountName, + InitContainers: args.InitContainers, + GenerateCaddyfileEntry: args.GenerateCaddyfileEntry, + Annotations: args.Annotations, + NodeSelector: args.NodeSelector, + Affinity: args.Affinity, + Sidecars: args.Sidecars, + VPA: args.VPA, // Pass VPA configuration to SimpleContainer + Scale: args.Deployment.Scale, // Pass Scale configuration to SimpleContainer + PodDisruption: defaultDisruptionBudget(args.Deployment.DisruptionBudget), RollingUpdate: lo.If(args.Deployment.RollingUpdate != nil, toRollingUpdateArgs(args.Deployment.RollingUpdate)).Else(nil), SecurityContext: nil, // TODO Log: args.Params.Log, @@ -304,6 +302,23 @@ func buildPreStopLifecycle(preStopSleepSeconds *int) *corev1.LifecycleArgs { } } +// defaultDisruptionBudget returns the PodDisruptionBudget to apply to a deployment. +// An explicitly configured budget is used as-is; otherwise we default to +// maxUnavailable:1 instead of minAvailable:1. +// +// minAvailable:1 on a single-replica deployment resolves to disruptionsAllowed:0, +// which forbids the eviction API and so blocks the cluster autoscaler from ever +// draining the pod's node. On GKE Autopilot that means the node can never be +// consolidated and stays pinned regardless of utilization. maxUnavailable:1 yields +// disruptionsAllowed:1 (the single pod can be evicted and rescheduled), while still +// bounding voluntary disruptions to one pod at a time for multi-replica deployments. +func defaultDisruptionBudget(db *k8s.DisruptionBudget) *k8s.DisruptionBudget { + if db != nil { + return db + } + return &k8s.DisruptionBudget{MaxUnavailable: lo.ToPtr(1)} +} + func toRollingUpdateArgs(update *k8s.RollingUpdate) *v1.RollingUpdateDeploymentArgs { return &v1.RollingUpdateDeploymentArgs{ MaxUnavailable: lo.If(lo.FromPtr(update).MaxUnavailable != nil, sdk.IntPtrFromPtr(lo.FromPtr(update).MaxUnavailable)).Else(nil), diff --git a/pkg/clouds/pulumi/kubernetes/deployment_disruption_test.go b/pkg/clouds/pulumi/kubernetes/deployment_disruption_test.go new file mode 100644 index 00000000..809ed731 --- /dev/null +++ b/pkg/clouds/pulumi/kubernetes/deployment_disruption_test.go @@ -0,0 +1,56 @@ +package kubernetes + +import ( + "testing" + + "github.com/samber/lo" + + "github.com/simple-container-com/api/pkg/clouds/k8s" +) + +// TestDefaultDisruptionBudget guards the PodDisruptionBudget default. The default +// must be maxUnavailable:1 (not minAvailable:1): minAvailable:1 on a single-replica +// deployment yields disruptionsAllowed:0, which blocks the cluster autoscaler from +// draining the pod's node and pins underutilized nodes on GKE Autopilot. +func TestDefaultDisruptionBudget(t *testing.T) { + t.Run("nil defaults to maxUnavailable:1 so single-replica nodes stay drainable", func(t *testing.T) { + got := defaultDisruptionBudget(nil) + if got == nil { + t.Fatal("expected a default budget, got nil") + } + if got.MinAvailable != nil { + t.Errorf("default must not set MinAvailable (it pins single-replica nodes); got %d", *got.MinAvailable) + } + if got.MaxUnavailable == nil { + t.Fatal("default must set MaxUnavailable") + } + if *got.MaxUnavailable != 1 { + t.Errorf("default MaxUnavailable = %d, want 1", *got.MaxUnavailable) + } + }) + + t.Run("explicit minAvailable is honored unchanged", func(t *testing.T) { + in := &k8s.DisruptionBudget{MinAvailable: lo.ToPtr(2)} + got := defaultDisruptionBudget(in) + if got != in { + t.Fatal("explicit budget must be returned as-is") + } + if got.MinAvailable == nil || *got.MinAvailable != 2 { + t.Errorf("MinAvailable = %v, want 2", got.MinAvailable) + } + if got.MaxUnavailable != nil { + t.Errorf("explicit minAvailable budget must not gain a MaxUnavailable; got %d", *got.MaxUnavailable) + } + }) + + t.Run("explicit maxUnavailable is honored unchanged", func(t *testing.T) { + in := &k8s.DisruptionBudget{MaxUnavailable: lo.ToPtr(3)} + got := defaultDisruptionBudget(in) + if got != in { + t.Fatal("explicit budget must be returned as-is") + } + if got.MaxUnavailable == nil || *got.MaxUnavailable != 3 { + t.Errorf("MaxUnavailable = %v, want 3", got.MaxUnavailable) + } + }) +}