From 7a7d33bc9920aded22d43e2c63cf5f7c464187dd Mon Sep 17 00:00:00 2001 From: Philipp Eberle Date: Fri, 8 Aug 2025 09:10:04 +0200 Subject: [PATCH 01/12] Pod Anti Affinity POC --- pkg/webhooks/fsGroupChangePolicySetter.go | 34 ++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/pkg/webhooks/fsGroupChangePolicySetter.go b/pkg/webhooks/fsGroupChangePolicySetter.go index 032e2167..b4bf2f15 100644 --- a/pkg/webhooks/fsGroupChangePolicySetter.go +++ b/pkg/webhooks/fsGroupChangePolicySetter.go @@ -7,12 +7,13 @@ import ( "github.com/go-logr/logr" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -// +kubebuilder:webhook:path=/mutate-apps-v1-statefulset,mutating=true,failurePolicy=ignore,groups=apps,resources=statefulsets,verbs=create;update,versions=v1,name=fsgroupchangepolicy.postgres.fits.cloud +// +kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,failurePolicy=ignore,groups=core,resources=pods,verbs=create;update,versions=v1,name=fsgroupchangepolicy.postgres.fits.cloud // FsGroupChangePolicySetter Adds securityContext.fsGroupChangePolicy=OnRootMismatch when the securityContext.fsGroup field is set type FsGroupChangePolicySetter struct { @@ -39,6 +40,37 @@ func (a *FsGroupChangePolicySetter) Handle(ctx context.Context, req admission.Re log.V(1).Info("Mutating Pod", "pod", pod) } + // + // PodAntiAffinity + // + paa := v1.PodAntiAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: []v1.PodAffinityTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: pod.ObjectMeta.Labels, + }, + TopologyKey: "kubernetes.io/hostname", + }, + }, + PreferredDuringSchedulingIgnoredDuringExecution: []v1.WeightedPodAffinityTerm{ + { + PodAffinityTerm: v1.PodAffinityTerm{ + LabelSelector: &metav1.LabelSelector{ + MatchLabels: pod.ObjectMeta.Labels, + }, + TopologyKey: "machine.metal-stack.io/rack", + }, + Weight: 1, + }, + }, + } + // initialize if necessary + if pod.Spec.Affinity == nil { + pod.Spec.Affinity = &v1.Affinity{} + } + // force our podantiaffinity + pod.Spec.Affinity.PodAntiAffinity = &paa + marshaledSts, err := json.Marshal(pod) if err != nil { log.Error(err, "failed to marshal response") From 81402f73f76a11b0de0f125eadd75d415c100cf4 Mon Sep 17 00:00:00 2001 From: Philipp Eberle Date: Fri, 8 Aug 2025 10:10:45 +0200 Subject: [PATCH 02/12] Do not just use *all* labels from the pod in the selector --- pkg/webhooks/fsGroupChangePolicySetter.go | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/pkg/webhooks/fsGroupChangePolicySetter.go b/pkg/webhooks/fsGroupChangePolicySetter.go index b4bf2f15..bbcffede 100644 --- a/pkg/webhooks/fsGroupChangePolicySetter.go +++ b/pkg/webhooks/fsGroupChangePolicySetter.go @@ -11,6 +11,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + pg "github.com/fi-ts/postgreslet/api/v1" ) // +kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,failurePolicy=ignore,groups=core,resources=pods,verbs=create;update,versions=v1,name=fsgroupchangepolicy.postgres.fits.cloud @@ -43,22 +45,27 @@ func (a *FsGroupChangePolicySetter) Handle(ctx context.Context, req admission.Re // // PodAntiAffinity // + //pod.ObjectMeta.Labels + ls := &metav1.LabelSelector{ + MatchLabels: map[string]string{ + pg.UIDLabelName: pod.ObjectMeta.Labels[pg.UIDLabelName], + pg.NameLabelName: pod.ObjectMeta.Labels[pg.NameLabelName], + pg.ProjectIDLabelName: pod.ObjectMeta.Labels[pg.ProjectIDLabelName], + pg.TenantLabelName: pod.ObjectMeta.Labels[pg.TenantLabelName], + }, + } paa := v1.PodAntiAffinity{ RequiredDuringSchedulingIgnoredDuringExecution: []v1.PodAffinityTerm{ { - LabelSelector: &metav1.LabelSelector{ - MatchLabels: pod.ObjectMeta.Labels, - }, - TopologyKey: "kubernetes.io/hostname", + LabelSelector: ls, + TopologyKey: "kubernetes.io/hostname", }, }, PreferredDuringSchedulingIgnoredDuringExecution: []v1.WeightedPodAffinityTerm{ { PodAffinityTerm: v1.PodAffinityTerm{ - LabelSelector: &metav1.LabelSelector{ - MatchLabels: pod.ObjectMeta.Labels, - }, - TopologyKey: "machine.metal-stack.io/rack", + LabelSelector: ls, + TopologyKey: "machine.metal-stack.io/rack", }, Weight: 1, }, From 2d289faf697f07a1077f2f36570ef279694cba1b Mon Sep 17 00:00:00 2001 From: Philipp Eberle Date: Fri, 8 Aug 2025 11:03:19 +0200 Subject: [PATCH 03/12] More labels --- pkg/webhooks/fsGroupChangePolicySetter.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/webhooks/fsGroupChangePolicySetter.go b/pkg/webhooks/fsGroupChangePolicySetter.go index bbcffede..55797cd2 100644 --- a/pkg/webhooks/fsGroupChangePolicySetter.go +++ b/pkg/webhooks/fsGroupChangePolicySetter.go @@ -45,13 +45,16 @@ func (a *FsGroupChangePolicySetter) Handle(ctx context.Context, req admission.Re // // PodAntiAffinity // - //pod.ObjectMeta.Labels ls := &metav1.LabelSelector{ MatchLabels: map[string]string{ - pg.UIDLabelName: pod.ObjectMeta.Labels[pg.UIDLabelName], - pg.NameLabelName: pod.ObjectMeta.Labels[pg.NameLabelName], - pg.ProjectIDLabelName: pod.ObjectMeta.Labels[pg.ProjectIDLabelName], - pg.TenantLabelName: pod.ObjectMeta.Labels[pg.TenantLabelName], + "application": "spilo", + "cluster-name": pod.ObjectMeta.Labels["cluster-name"], + pg.NameLabelName: pod.ObjectMeta.Labels[pg.NameLabelName], + pg.PartitionIDLabelName: pod.ObjectMeta.Labels[pg.PartitionIDLabelName], + pg.ProjectIDLabelName: pod.ObjectMeta.Labels[pg.ProjectIDLabelName], + pg.TenantLabelName: pod.ObjectMeta.Labels[pg.TenantLabelName], + pg.UIDLabelName: pod.ObjectMeta.Labels[pg.UIDLabelName], + "team": pod.ObjectMeta.Labels["team"], }, } paa := v1.PodAntiAffinity{ From dc8f7d890778b26bb9eeed916543b6df07203afc Mon Sep 17 00:00:00 2001 From: Philipp Eberle Date: Fri, 8 Aug 2025 11:17:56 +0200 Subject: [PATCH 04/12] Add instead of replace --- pkg/webhooks/fsGroupChangePolicySetter.go | 27 +++++++++-------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/pkg/webhooks/fsGroupChangePolicySetter.go b/pkg/webhooks/fsGroupChangePolicySetter.go index 55797cd2..9de3fdb7 100644 --- a/pkg/webhooks/fsGroupChangePolicySetter.go +++ b/pkg/webhooks/fsGroupChangePolicySetter.go @@ -57,29 +57,22 @@ func (a *FsGroupChangePolicySetter) Handle(ctx context.Context, req admission.Re "team": pod.ObjectMeta.Labels["team"], }, } - paa := v1.PodAntiAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: []v1.PodAffinityTerm{ - { - LabelSelector: ls, - TopologyKey: "kubernetes.io/hostname", - }, - }, - PreferredDuringSchedulingIgnoredDuringExecution: []v1.WeightedPodAffinityTerm{ - { - PodAffinityTerm: v1.PodAffinityTerm{ - LabelSelector: ls, - TopologyKey: "machine.metal-stack.io/rack", - }, - Weight: 1, - }, + wpat := v1.WeightedPodAffinityTerm{ + PodAffinityTerm: v1.PodAffinityTerm{ + LabelSelector: ls, + TopologyKey: "machine.metal-stack.io/rack", }, + Weight: 1, } // initialize if necessary if pod.Spec.Affinity == nil { pod.Spec.Affinity = &v1.Affinity{} } - // force our podantiaffinity - pod.Spec.Affinity.PodAntiAffinity = &paa + if pod.Spec.Affinity.PodAntiAffinity == nil { + pod.Spec.Affinity.PodAntiAffinity = &v1.PodAntiAffinity{} + } + // add our podantiaffinity + pod.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution = append(pod.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution, wpat) marshaledSts, err := json.Marshal(pod) if err != nil { From f005559dc9a0c3aca85978d7d14194c1ac7d01e2 Mon Sep 17 00:00:00 2001 From: Philipp Eberle Date: Fri, 8 Aug 2025 14:54:13 +0200 Subject: [PATCH 05/12] Test topology spread constraints --- pkg/webhooks/fsGroupChangePolicySetter.go | 49 ++++++++++------------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/pkg/webhooks/fsGroupChangePolicySetter.go b/pkg/webhooks/fsGroupChangePolicySetter.go index 9de3fdb7..c60eb278 100644 --- a/pkg/webhooks/fsGroupChangePolicySetter.go +++ b/pkg/webhooks/fsGroupChangePolicySetter.go @@ -8,6 +8,7 @@ import ( "github.com/go-logr/logr" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -43,36 +44,30 @@ func (a *FsGroupChangePolicySetter) Handle(ctx context.Context, req admission.Re } // - // PodAntiAffinity + // TopologySpreadConstraint // - ls := &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "application": "spilo", - "cluster-name": pod.ObjectMeta.Labels["cluster-name"], - pg.NameLabelName: pod.ObjectMeta.Labels[pg.NameLabelName], - pg.PartitionIDLabelName: pod.ObjectMeta.Labels[pg.PartitionIDLabelName], - pg.ProjectIDLabelName: pod.ObjectMeta.Labels[pg.ProjectIDLabelName], - pg.TenantLabelName: pod.ObjectMeta.Labels[pg.TenantLabelName], - pg.UIDLabelName: pod.ObjectMeta.Labels[pg.UIDLabelName], - "team": pod.ObjectMeta.Labels["team"], + var maxSkew int32 = 1 + var minDomains int32 = 2 + tsc := v1.TopologySpreadConstraint{ + MaxSkew: maxSkew, + MinDomains: ptr.To(minDomains), + WhenUnsatisfiable: v1.DoNotSchedule, + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "application": "spilo", + "cluster-name": pod.ObjectMeta.Labels["cluster-name"], + pg.NameLabelName: pod.ObjectMeta.Labels[pg.NameLabelName], + pg.PartitionIDLabelName: pod.ObjectMeta.Labels[pg.PartitionIDLabelName], + pg.ProjectIDLabelName: pod.ObjectMeta.Labels[pg.ProjectIDLabelName], + pg.TenantLabelName: pod.ObjectMeta.Labels[pg.TenantLabelName], + pg.UIDLabelName: pod.ObjectMeta.Labels[pg.UIDLabelName], + "team": pod.ObjectMeta.Labels["team"], + }, }, + TopologyKey: "machine.metal-stack.io/rack", } - wpat := v1.WeightedPodAffinityTerm{ - PodAffinityTerm: v1.PodAffinityTerm{ - LabelSelector: ls, - TopologyKey: "machine.metal-stack.io/rack", - }, - Weight: 1, - } - // initialize if necessary - if pod.Spec.Affinity == nil { - pod.Spec.Affinity = &v1.Affinity{} - } - if pod.Spec.Affinity.PodAntiAffinity == nil { - pod.Spec.Affinity.PodAntiAffinity = &v1.PodAntiAffinity{} - } - // add our podantiaffinity - pod.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution = append(pod.Spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution, wpat) + // add our topology spread constraints + pod.Spec.TopologySpreadConstraints = append(pod.Spec.TopologySpreadConstraints, tsc) marshaledSts, err := json.Marshal(pod) if err != nil { From 52bf0fc898f4714c2dc580fe98aa8b6eb925b2a1 Mon Sep 17 00:00:00 2001 From: Philipp Eberle Date: Tue, 12 Aug 2025 17:24:21 +0200 Subject: [PATCH 06/12] Make settings configurable --- main.go | 85 +++++++++++++++-------- pkg/webhooks/fsGroupChangePolicySetter.go | 57 ++++++++------- 2 files changed, 88 insertions(+), 54 deletions(-) diff --git a/main.go b/main.go index 859035f0..93bb123a 100644 --- a/main.go +++ b/main.go @@ -97,6 +97,10 @@ const ( walGExporterMemoryLimitFlg = "walg-exporter-memory-limit" podAntiaffinityPreferredDuringSchedulingFlg = "pod-antiaffinity-preferred-during-scheduling" podAntiaffinityTopologyKeyFlg = "pod-antiaffinity-topology-key" + enablePodTopologySpreadConstraintWebhookFlg = "enable-pod-topology-spread-constraint-webhook" + podTopologySpreadConstraintTopologyKeyFlg = "pod-topology-spread-constraint-topology-key" + podTopologySpreadConstraintMaxSkewFlg = "pod-topology-spread-constraint-max-skew" + podTopologySpreadConstraintMinDomainsFlg = "pod-topology-spread-constraint-min-domains" ) var ( @@ -119,33 +123,34 @@ func init() { func main() { var ( - metricsAddrCtrlMgr string - metricsAddrSvcMgr string - partitionID string - tenant string - ctrlClusterKubeconfig string - pspName string - lbIP string - storageClass string - postgresImage string - etcdHost string - operatorImage string - majorVersionUpgradeMode string - postgresletNamespace string - sidecarsCMName string - controlPlaneNamespace string - etcdImage string - etcdBackupSidecarImage string - etcdBackupSecretName string - etcdPSPName string - postgresletFullname string - initDBJobCMName string - tlsClusterIssuer string - tlsSubDomain string - walGExporterImage string - walGExporterCPULimit string - walGExporterMemoryLimit string - podAntiaffinityTopologyKey string + metricsAddrCtrlMgr string + metricsAddrSvcMgr string + partitionID string + tenant string + ctrlClusterKubeconfig string + pspName string + lbIP string + storageClass string + postgresImage string + etcdHost string + operatorImage string + majorVersionUpgradeMode string + postgresletNamespace string + sidecarsCMName string + controlPlaneNamespace string + etcdImage string + etcdBackupSidecarImage string + etcdBackupSecretName string + etcdPSPName string + postgresletFullname string + initDBJobCMName string + tlsClusterIssuer string + tlsSubDomain string + walGExporterImage string + walGExporterCPULimit string + walGExporterMemoryLimit string + podAntiaffinityTopologyKey string + podTopologySpreadConstraintTopologyKey string enableLeaderElection bool enableCRDRegistration bool @@ -164,10 +169,13 @@ func main() { enableFsGroupChangePolicyWebhook bool enableWalGExporter bool podAntiaffinityPreferredDuringScheduling bool + enablePodTopologySpreadConstraintWebhook bool portRangeStart int32 portRangeSize int32 replicationChangeRequeueTimeInSeconds int + podTopologySpreadConstraintMaxSkew int32 + podTopologySpreadConstraintMinDomains int32 patroniTTL uint32 patroniLoopWait uint32 @@ -355,6 +363,15 @@ func main() { walGExporterMemoryLimit = viper.GetString(walGExporterMemoryLimitFlg) resource.MustParse(walGExporterMemoryLimit) + // TODO disable by default + viper.SetDefault(enablePodTopologySpreadConstraintWebhookFlg, true) + enablePodTopologySpreadConstraintWebhook = viper.GetBool(enablePodTopologySpreadConstraintWebhookFlg) + viper.SetDefault(podTopologySpreadConstraintTopologyKeyFlg, "machine.metal-stack.io/rack") + podTopologySpreadConstraintTopologyKey = viper.GetString(podTopologySpreadConstraintTopologyKeyFlg) + viper.SetDefault(podTopologySpreadConstraintMaxSkewFlg, 1) + podTopologySpreadConstraintMaxSkew = viper.GetInt32(podTopologySpreadConstraintMaxSkewFlg) + podTopologySpreadConstraintMinDomains = viper.GetInt32(podTopologySpreadConstraintMinDomainsFlg) + ctrl.Log.Info("flag", metricsAddrSvcMgrFlg, metricsAddrSvcMgr, metricsAddrCtrlMgrFlg, metricsAddrCtrlMgr, @@ -406,6 +423,10 @@ func main() { walGExporterImageFlg, walGExporterImage, walGExporterCPULimitFlg, walGExporterCPULimit, walGExporterMemoryLimitFlg, walGExporterMemoryLimit, + enablePodTopologySpreadConstraintWebhookFlg, enablePodTopologySpreadConstraintWebhook, + podTopologySpreadConstraintTopologyKeyFlg, podTopologySpreadConstraintTopologyKey, + podTopologySpreadConstraintMaxSkewFlg, podTopologySpreadConstraintMaxSkew, + podTopologySpreadConstraintMinDomainsFlg, podTopologySpreadConstraintMinDomains, ) svcClusterConf := ctrl.GetConfigOrDie() @@ -554,9 +575,13 @@ func main() { "/mutate-v1-pod", &webhook.Admission{ Handler: &webhooks.FsGroupChangePolicySetter{ - SvcClient: svcClusterMgr.GetClient(), - Decoder: admission.NewDecoder(svcClusterMgr.GetScheme()), - Log: ctrl.Log.WithName("webhooks").WithName("FsGroupChangePolicySetter"), + SvcClient: svcClusterMgr.GetClient(), + Decoder: admission.NewDecoder(svcClusterMgr.GetScheme()), + Log: ctrl.Log.WithName("webhooks").WithName("FsGroupChangePolicySetter"), + EnablePodTopologySpreadConstraintWebhook: enablePodTopologySpreadConstraintWebhook, + PodTopologySpreadConstraintTopologyKey: podTopologySpreadConstraintTopologyKey, + PodTopologySpreadConstraintMaxSkew: podTopologySpreadConstraintMaxSkew, + PodTopologySpreadConstraintMinDomains: podTopologySpreadConstraintMinDomains, }, }, ) diff --git a/pkg/webhooks/fsGroupChangePolicySetter.go b/pkg/webhooks/fsGroupChangePolicySetter.go index c60eb278..69593b02 100644 --- a/pkg/webhooks/fsGroupChangePolicySetter.go +++ b/pkg/webhooks/fsGroupChangePolicySetter.go @@ -8,7 +8,6 @@ import ( "github.com/go-logr/logr" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -20,9 +19,13 @@ import ( // FsGroupChangePolicySetter Adds securityContext.fsGroupChangePolicy=OnRootMismatch when the securityContext.fsGroup field is set type FsGroupChangePolicySetter struct { - SvcClient client.Client - Decoder admission.Decoder - Log logr.Logger + SvcClient client.Client + Decoder admission.Decoder + Log logr.Logger + EnablePodTopologySpreadConstraintWebhook bool + PodTopologySpreadConstraintTopologyKey string + PodTopologySpreadConstraintMaxSkew int32 + PodTopologySpreadConstraintMinDomains int32 } func (a *FsGroupChangePolicySetter) Handle(ctx context.Context, req admission.Request) admission.Response { @@ -46,28 +49,34 @@ func (a *FsGroupChangePolicySetter) Handle(ctx context.Context, req admission.Re // // TopologySpreadConstraint // - var maxSkew int32 = 1 - var minDomains int32 = 2 - tsc := v1.TopologySpreadConstraint{ - MaxSkew: maxSkew, - MinDomains: ptr.To(minDomains), - WhenUnsatisfiable: v1.DoNotSchedule, - LabelSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "application": "spilo", - "cluster-name": pod.ObjectMeta.Labels["cluster-name"], - pg.NameLabelName: pod.ObjectMeta.Labels[pg.NameLabelName], - pg.PartitionIDLabelName: pod.ObjectMeta.Labels[pg.PartitionIDLabelName], - pg.ProjectIDLabelName: pod.ObjectMeta.Labels[pg.ProjectIDLabelName], - pg.TenantLabelName: pod.ObjectMeta.Labels[pg.TenantLabelName], - pg.UIDLabelName: pod.ObjectMeta.Labels[pg.UIDLabelName], - "team": pod.ObjectMeta.Labels["team"], + if a.EnablePodTopologySpreadConstraintWebhook { + tsc := v1.TopologySpreadConstraint{ + MaxSkew: a.PodTopologySpreadConstraintMaxSkew, + WhenUnsatisfiable: v1.ScheduleAnyway, + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "application": "spilo", + "cluster-name": pod.ObjectMeta.Labels["cluster-name"], + pg.NameLabelName: pod.ObjectMeta.Labels[pg.NameLabelName], + pg.PartitionIDLabelName: pod.ObjectMeta.Labels[pg.PartitionIDLabelName], + pg.ProjectIDLabelName: pod.ObjectMeta.Labels[pg.ProjectIDLabelName], + pg.TenantLabelName: pod.ObjectMeta.Labels[pg.TenantLabelName], + pg.UIDLabelName: pod.ObjectMeta.Labels[pg.UIDLabelName], + "team": pod.ObjectMeta.Labels["team"], + }, }, - }, - TopologyKey: "machine.metal-stack.io/rack", + TopologyKey: a.PodTopologySpreadConstraintTopologyKey, + } + + // if defined, set the minDomains (and corresponding whenUnsatisfied) field as well + if a.PodTopologySpreadConstraintMinDomains > 0 { + tsc.MinDomains = &a.PodTopologySpreadConstraintMinDomains + tsc.WhenUnsatisfiable = v1.DoNotSchedule + } + + // add our topology spread constraints + pod.Spec.TopologySpreadConstraints = append(pod.Spec.TopologySpreadConstraints, tsc) } - // add our topology spread constraints - pod.Spec.TopologySpreadConstraints = append(pod.Spec.TopologySpreadConstraints, tsc) marshaledSts, err := json.Marshal(pod) if err != nil { From 532440eec792e41323740a7fa9d08aca85cce548 Mon Sep 17 00:00:00 2001 From: Philipp Eberle Date: Thu, 14 Aug 2025 11:27:49 +0200 Subject: [PATCH 07/12] Disable by default --- main.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/main.go b/main.go index 93bb123a..85f30a49 100644 --- a/main.go +++ b/main.go @@ -363,8 +363,7 @@ func main() { walGExporterMemoryLimit = viper.GetString(walGExporterMemoryLimitFlg) resource.MustParse(walGExporterMemoryLimit) - // TODO disable by default - viper.SetDefault(enablePodTopologySpreadConstraintWebhookFlg, true) + viper.SetDefault(enablePodTopologySpreadConstraintWebhookFlg, false) enablePodTopologySpreadConstraintWebhook = viper.GetBool(enablePodTopologySpreadConstraintWebhookFlg) viper.SetDefault(podTopologySpreadConstraintTopologyKeyFlg, "machine.metal-stack.io/rack") podTopologySpreadConstraintTopologyKey = viper.GetString(podTopologySpreadConstraintTopologyKeyFlg) From 43e955bc83136a874eb4394e1444bafcfa47f34e Mon Sep 17 00:00:00 2001 From: Philipp Eberle Date: Thu, 14 Aug 2025 11:28:24 +0200 Subject: [PATCH 08/12] Logging --- pkg/webhooks/fsGroupChangePolicySetter.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/webhooks/fsGroupChangePolicySetter.go b/pkg/webhooks/fsGroupChangePolicySetter.go index 69593b02..89ba057a 100644 --- a/pkg/webhooks/fsGroupChangePolicySetter.go +++ b/pkg/webhooks/fsGroupChangePolicySetter.go @@ -43,7 +43,7 @@ func (a *FsGroupChangePolicySetter) Handle(ctx context.Context, req admission.Re if pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.FSGroup != nil { p := v1.FSGroupChangeOnRootMismatch pod.Spec.SecurityContext.FSGroupChangePolicy = &p - log.V(1).Info("Mutating Pod", "pod", pod) + log.V(1).Info("Mutating Pod securityContext", "pod", pod) } // @@ -76,6 +76,7 @@ func (a *FsGroupChangePolicySetter) Handle(ctx context.Context, req admission.Re // add our topology spread constraints pod.Spec.TopologySpreadConstraints = append(pod.Spec.TopologySpreadConstraints, tsc) + log.V(1).Info("Mutating Pod topologySpreadConstraints", "topologySpreadConstraints", pod.Spec.TopologySpreadConstraints) } marshaledSts, err := json.Marshal(pod) From 5590df63d10e38affa80853881bf61a6e2a39738 Mon Sep 17 00:00:00 2001 From: Philipp Eberle Date: Thu, 14 Aug 2025 11:48:36 +0200 Subject: [PATCH 09/12] Override instead of replace --- pkg/webhooks/fsGroupChangePolicySetter.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/webhooks/fsGroupChangePolicySetter.go b/pkg/webhooks/fsGroupChangePolicySetter.go index 89ba057a..01080b29 100644 --- a/pkg/webhooks/fsGroupChangePolicySetter.go +++ b/pkg/webhooks/fsGroupChangePolicySetter.go @@ -74,8 +74,8 @@ func (a *FsGroupChangePolicySetter) Handle(ctx context.Context, req admission.Re tsc.WhenUnsatisfiable = v1.DoNotSchedule } - // add our topology spread constraints - pod.Spec.TopologySpreadConstraints = append(pod.Spec.TopologySpreadConstraints, tsc) + // override topology spread constraints + pod.Spec.TopologySpreadConstraints = []v1.TopologySpreadConstraint{tsc} log.V(1).Info("Mutating Pod topologySpreadConstraints", "topologySpreadConstraints", pod.Spec.TopologySpreadConstraints) } From 30ddb214b3c2b2185ebc4e1b84e5df8e839ed933 Mon Sep 17 00:00:00 2001 From: Philipp Eberle Date: Fri, 22 Aug 2025 14:40:19 +0200 Subject: [PATCH 10/12] Proper separation of the two features in the same webhook --- main.go | 7 +++--- ...angePolicySetter.go => spiloPodMutator.go} | 22 ++++++++++++------- 2 files changed, 18 insertions(+), 11 deletions(-) rename pkg/webhooks/{fsGroupChangePolicySetter.go => spiloPodMutator.go} (80%) diff --git a/main.go b/main.go index 85f30a49..7dd065ea 100644 --- a/main.go +++ b/main.go @@ -569,14 +569,15 @@ func main() { } // +kubebuilder:scaffold:builder - if enableFsGroupChangePolicyWebhook { + if enableFsGroupChangePolicyWebhook || enablePodTopologySpreadConstraintWebhook { svcClusterMgr.GetWebhookServer().Register( "/mutate-v1-pod", &webhook.Admission{ - Handler: &webhooks.FsGroupChangePolicySetter{ + Handler: &webhooks.SpiloPodMutator{ SvcClient: svcClusterMgr.GetClient(), Decoder: admission.NewDecoder(svcClusterMgr.GetScheme()), - Log: ctrl.Log.WithName("webhooks").WithName("FsGroupChangePolicySetter"), + Log: ctrl.Log.WithName("webhooks").WithName("SpiloPodMutator"), + EnableFsGroupChangePolicyWebhook: enableFsGroupChangePolicyWebhook, EnablePodTopologySpreadConstraintWebhook: enablePodTopologySpreadConstraintWebhook, PodTopologySpreadConstraintTopologyKey: podTopologySpreadConstraintTopologyKey, PodTopologySpreadConstraintMaxSkew: podTopologySpreadConstraintMaxSkew, diff --git a/pkg/webhooks/fsGroupChangePolicySetter.go b/pkg/webhooks/spiloPodMutator.go similarity index 80% rename from pkg/webhooks/fsGroupChangePolicySetter.go rename to pkg/webhooks/spiloPodMutator.go index 01080b29..cd7e0f98 100644 --- a/pkg/webhooks/fsGroupChangePolicySetter.go +++ b/pkg/webhooks/spiloPodMutator.go @@ -17,18 +17,19 @@ import ( // +kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,failurePolicy=ignore,groups=core,resources=pods,verbs=create;update,versions=v1,name=fsgroupchangepolicy.postgres.fits.cloud -// FsGroupChangePolicySetter Adds securityContext.fsGroupChangePolicy=OnRootMismatch when the securityContext.fsGroup field is set -type FsGroupChangePolicySetter struct { +// SpiloPodMutator Adds securityContext.fsGroupChangePolicy=OnRootMismatch when the securityContext.fsGroup field is set +type SpiloPodMutator struct { SvcClient client.Client Decoder admission.Decoder Log logr.Logger + EnableFsGroupChangePolicyWebhook bool EnablePodTopologySpreadConstraintWebhook bool PodTopologySpreadConstraintTopologyKey string PodTopologySpreadConstraintMaxSkew int32 PodTopologySpreadConstraintMinDomains int32 } -func (a *FsGroupChangePolicySetter) Handle(ctx context.Context, req admission.Request) admission.Response { +func (a *SpiloPodMutator) Handle(ctx context.Context, req admission.Request) admission.Response { log := a.Log.WithValues("name", req.Name, "ns", req.Namespace) log.V(1).Info("handling admission request") @@ -39,11 +40,16 @@ func (a *FsGroupChangePolicySetter) Handle(ctx context.Context, req admission.Re return admission.Errored(http.StatusBadRequest, err) } - // when the fsGroup field is set, also set the fsGroupChangePolicy to OnRootMismatch - if pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.FSGroup != nil { - p := v1.FSGroupChangeOnRootMismatch - pod.Spec.SecurityContext.FSGroupChangePolicy = &p - log.V(1).Info("Mutating Pod securityContext", "pod", pod) + // + // FSGroupChangePolicy + // + if a.EnableFsGroupChangePolicyWebhook { + // when the fsGroup field is set, also set the fsGroupChangePolicy to OnRootMismatch + if pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.FSGroup != nil { + p := v1.FSGroupChangeOnRootMismatch + pod.Spec.SecurityContext.FSGroupChangePolicy = &p + log.V(1).Info("Mutating Pod securityContext", "pod", pod) + } } // From 021b2e93c1a4b34b4fe9ec4afdcb834b413eb7ba Mon Sep 17 00:00:00 2001 From: Philipp Eberle Date: Tue, 26 Aug 2025 11:11:22 +0200 Subject: [PATCH 11/12] Proper variable name --- pkg/webhooks/spiloPodMutator.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/webhooks/spiloPodMutator.go b/pkg/webhooks/spiloPodMutator.go index cd7e0f98..16071c23 100644 --- a/pkg/webhooks/spiloPodMutator.go +++ b/pkg/webhooks/spiloPodMutator.go @@ -85,12 +85,12 @@ func (a *SpiloPodMutator) Handle(ctx context.Context, req admission.Request) adm log.V(1).Info("Mutating Pod topologySpreadConstraints", "topologySpreadConstraints", pod.Spec.TopologySpreadConstraints) } - marshaledSts, err := json.Marshal(pod) + marshaledPod, err := json.Marshal(pod) if err != nil { log.Error(err, "failed to marshal response") return admission.Errored(http.StatusInternalServerError, err) } log.V(1).Info("done") - return admission.PatchResponseFromRaw(req.Object.Raw, marshaledSts) + return admission.PatchResponseFromRaw(req.Object.Raw, marshaledPod) } From b0470dd27b6ad5321bf18e31e6fa4190ebe7853e Mon Sep 17 00:00:00 2001 From: Philipp Eberle Date: Tue, 2 Sep 2025 10:44:38 +0200 Subject: [PATCH 12/12] Remove if clause to simplify code, since we check within the webhook anyway --- main.go | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/main.go b/main.go index 7dd065ea..ba5294be 100644 --- a/main.go +++ b/main.go @@ -569,23 +569,21 @@ func main() { } // +kubebuilder:scaffold:builder - if enableFsGroupChangePolicyWebhook || enablePodTopologySpreadConstraintWebhook { - svcClusterMgr.GetWebhookServer().Register( - "/mutate-v1-pod", - &webhook.Admission{ - Handler: &webhooks.SpiloPodMutator{ - SvcClient: svcClusterMgr.GetClient(), - Decoder: admission.NewDecoder(svcClusterMgr.GetScheme()), - Log: ctrl.Log.WithName("webhooks").WithName("SpiloPodMutator"), - EnableFsGroupChangePolicyWebhook: enableFsGroupChangePolicyWebhook, - EnablePodTopologySpreadConstraintWebhook: enablePodTopologySpreadConstraintWebhook, - PodTopologySpreadConstraintTopologyKey: podTopologySpreadConstraintTopologyKey, - PodTopologySpreadConstraintMaxSkew: podTopologySpreadConstraintMaxSkew, - PodTopologySpreadConstraintMinDomains: podTopologySpreadConstraintMinDomains, - }, + svcClusterMgr.GetWebhookServer().Register( + "/mutate-v1-pod", + &webhook.Admission{ + Handler: &webhooks.SpiloPodMutator{ + SvcClient: svcClusterMgr.GetClient(), + Decoder: admission.NewDecoder(svcClusterMgr.GetScheme()), + Log: ctrl.Log.WithName("webhooks").WithName("SpiloPodMutator"), + EnableFsGroupChangePolicyWebhook: enableFsGroupChangePolicyWebhook, + EnablePodTopologySpreadConstraintWebhook: enablePodTopologySpreadConstraintWebhook, + PodTopologySpreadConstraintTopologyKey: podTopologySpreadConstraintTopologyKey, + PodTopologySpreadConstraintMaxSkew: podTopologySpreadConstraintMaxSkew, + PodTopologySpreadConstraintMinDomains: podTopologySpreadConstraintMinDomains, }, - ) - } + }, + ) ctx := context.Background()