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 <[email protected]>

Author: b1zzu

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.
 

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{

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"`

pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.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,
 			},
 		},
 	}

pkg/cluster/k8sres.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 {

pkg/cluster/k8sres_test.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())

pkg/controller/operator_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"`