From 3ca26d0dc8687286c85ebbf463c8736f8b6124b7 Mon Sep 17 00:00:00 2001 From: Davide Bizzarri Date: Thu, 4 Jan 2024 15:58:24 +0100 Subject: [PATCH] Make PodDisruptionBudget master label selector optional (#2364) * Make PDB master label selector optional * Update pkg/apis/acid.zalan.do/v1/crds.go --------- Co-authored-by: Felix Kunde --- docs/reference/operator_parameters.md | 13 ++++++---- pkg/apis/acid.zalan.do/v1/crds.go | 3 +++ .../v1/operator_configuration_type.go | 1 + .../acid.zalan.do/v1/zz_generated.deepcopy.go | 5 ++++ pkg/cluster/k8sres.go | 9 ++++++- pkg/cluster/k8sres_test.go | 24 +++++++++++++++++++ pkg/controller/operator_config.go | 1 + pkg/util/config/config.go | 1 + 8 files changed, 52 insertions(+), 5 deletions(-) diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index 70135ea69..428a7f730 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -323,6 +323,11 @@ configuration they are grouped under the `kubernetes` key. replaced by the cluster name. Only the `{cluster}` placeholders is allowed in the template. +* **pdb_master_label_selector** + By default the PDB will match the master role hence preventing nodes to be + drained if the node_readiness_label is not used. This option if set to `false` + will not add the `spilo-role=master` selector to the PDB. + * **enable_pod_disruption_budget** PDB is enabled by default to protect the cluster from voluntarily disruptions and hence unwanted DB downtime. However, on some cloud providers it could be @@ -431,7 +436,7 @@ configuration they are grouped under the `kubernetes` key. environment if they not if conflict with the environment variables generated by the operator. The WAL location (bucket path) can be overridden, though. The default is empty. - + * **pod_environment_secret** similar to pod_environment_configmap but referencing a secret with custom environment variables. Because the secret is not allowed to exist in a @@ -577,7 +582,7 @@ effect, and the parameters are grouped under the `timeouts` key in the CRD-based configuration. * **PatroniAPICheckInterval** - the interval between consecutive attempts waiting for the return of + the interval between consecutive attempts waiting for the return of Patroni Api. The default is `1s`. * **PatroniAPICheckTimeout** @@ -651,7 +656,7 @@ In the CRD-based configuration they are grouped under the `load_balancer` key. balancers. Allowed values are `Cluster` (default) and `Local`. * **master_dns_name_format** - defines the DNS name string template for the master load balancer cluster. + defines the DNS name string template for the master load balancer cluster. The default is `{cluster}.{namespace}.{hostedzone}`, where `{cluster}` is replaced by the cluster name, `{namespace}` is replaced with the namespace and `{hostedzone}` is replaced with the hosted zone (the value of the @@ -816,7 +821,7 @@ grouped under the `logical_backup` key. is specified, no argument will be passed to `aws s3` command. Default: "AES256". * **logical_backup_s3_retention_time** - Specify a retention time for logical backups stored in S3. Backups older than the specified retention + Specify a retention time for logical backups stored in S3. Backups older than the specified retention time will be deleted after a new backup was uploaded. If empty, all backups will be kept. Example values are "3 days", "2 weeks", or "1 month". The default is empty. diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index 354d1933a..4dd30fc84 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -1394,6 +1394,9 @@ var OperatorConfigCRDResourceValidation = apiextv1.CustomResourceValidation{ "pdb_name_format": { Type: "string", }, + "pdb_master_label_selector": { + Type: "boolean", + }, "persistent_volume_claim_retention_policy": { Type: "object", Properties: map[string]apiextv1.JSONSchemaProps{ diff --git a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go index ee44df9a4..e5a089978 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -68,6 +68,7 @@ type KubernetesMetaConfiguration struct { AdditionalPodCapabilities []string `json:"additional_pod_capabilities,omitempty"` WatchedNamespace string `json:"watched_namespace,omitempty"` PDBNameFormat config.StringTemplate `json:"pdb_name_format,omitempty"` + PDBMasterLabelSelector *bool `json:"pdb_master_label_selector,omitempty"` EnablePodDisruptionBudget *bool `json:"enable_pod_disruption_budget,omitempty"` StorageResizeMode string `json:"storage_resize_mode,omitempty"` EnableInitContainers *bool `json:"enable_init_containers,omitempty"` diff --git a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go index 536feec73..b26edcd62 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -178,6 +178,11 @@ func (in *KubernetesMetaConfiguration) DeepCopyInto(out *KubernetesMetaConfigura *out = make([]string, len(*in)) copy(*out, *in) } + if in.PDBMasterLabelSelector != nil { + in, out := &in.PDBMasterLabelSelector, &out.PDBMasterLabelSelector + *out = new(bool) + **out = **in + } if in.EnablePodDisruptionBudget != nil { in, out := &in.EnablePodDisruptionBudget, &out.EnablePodDisruptionBudget *out = new(bool) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 654df1ac9..56f9c0314 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -2150,12 +2150,19 @@ func (c *Cluster) generateStandbyEnvironment(description *acidv1.StandbyDescript func (c *Cluster) generatePodDisruptionBudget() *policyv1.PodDisruptionBudget { minAvailable := intstr.FromInt(1) pdbEnabled := c.OpConfig.EnablePodDisruptionBudget + pdbMasterLabelSelector := c.OpConfig.PDBMasterLabelSelector // if PodDisruptionBudget is disabled or if there are no DB pods, set the budget to 0. if (pdbEnabled != nil && !(*pdbEnabled)) || c.Spec.NumberOfInstances <= 0 { minAvailable = intstr.FromInt(0) } + // define label selector and add the master role selector if enabled + labels := c.labelsSet(false) + if pdbMasterLabelSelector == nil || *c.OpConfig.PDBMasterLabelSelector { + labels[c.OpConfig.PodRoleLabel] = string(Master) + } + return &policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ Name: c.podDisruptionBudgetName(), @@ -2166,7 +2173,7 @@ func (c *Cluster) generatePodDisruptionBudget() *policyv1.PodDisruptionBudget { Spec: policyv1.PodDisruptionBudgetSpec{ MinAvailable: &minAvailable, Selector: &metav1.LabelSelector{ - MatchLabels: c.roleLabelsSet(false, Master), + MatchLabels: labels, }, }, } diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index 700b75f14..bf394fe86 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -2379,6 +2379,30 @@ func TestGeneratePodDisruptionBudget(t *testing.T) { }, }, }, + // With PDBMasterLabelSelector disabled. + { + New( + Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-pdb", PDBMasterLabelSelector: util.False()}}, + k8sutil.KubernetesClient{}, + acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "myapp-database", Namespace: "myapp"}, + Spec: acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 3}}, + logger, + eventRecorder), + policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "postgres-myapp-database-pdb", + Namespace: "myapp", + Labels: map[string]string{"team": "myapp", "cluster-name": "myapp-database"}, + }, + Spec: policyv1.PodDisruptionBudgetSpec{ + MinAvailable: util.ToIntStr(1), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"cluster-name": "myapp-database"}, + }, + }, + }, + }, } for _, tt := range tests { diff --git a/pkg/controller/operator_config.go b/pkg/controller/operator_config.go index 7636b5b5a..346bf27dd 100644 --- a/pkg/controller/operator_config.go +++ b/pkg/controller/operator_config.go @@ -82,6 +82,7 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur result.ClusterDomain = util.Coalesce(fromCRD.Kubernetes.ClusterDomain, "cluster.local") result.WatchedNamespace = fromCRD.Kubernetes.WatchedNamespace result.PDBNameFormat = fromCRD.Kubernetes.PDBNameFormat + result.PDBMasterLabelSelector = util.CoalesceBool(fromCRD.Kubernetes.PDBMasterLabelSelector, util.True()) result.EnablePodDisruptionBudget = util.CoalesceBool(fromCRD.Kubernetes.EnablePodDisruptionBudget, util.True()) result.StorageResizeMode = util.Coalesce(fromCRD.Kubernetes.StorageResizeMode, "pvc") result.EnableInitContainers = util.CoalesceBool(fromCRD.Kubernetes.EnableInitContainers, util.True()) diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 940b1a86b..82df74e9c 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -220,6 +220,7 @@ type Config struct { ReplicaDNSNameFormat StringTemplate `name:"replica_dns_name_format" default:"{cluster}-repl.{namespace}.{hostedzone}"` ReplicaLegacyDNSNameFormat StringTemplate `name:"replica_legacy_dns_name_format" default:"{cluster}-repl.{team}.{hostedzone}"` PDBNameFormat StringTemplate `name:"pdb_name_format" default:"postgres-{cluster}-pdb"` + PDBMasterLabelSelector *bool `name:"pdb_master_label_selector" default:"true"` EnablePodDisruptionBudget *bool `name:"enable_pod_disruption_budget" default:"true"` EnableInitContainers *bool `name:"enable_init_containers" default:"true"` EnableSidecars *bool `name:"enable_sidecars" default:"true"`