Add PDB configuration toggle (#583)

* Don't create an impossible disruption budget for smaller clusters.
* sync PDB also on update

charts/postgres-operator/values.yaml

@@ -114,6 +114,7 @@ configKubernetesCRD:
   cluster_name_label: cluster-name
   enable_pod_antiaffinity: false
   pod_antiaffinity_topology_key: "kubernetes.io/hostname"
+  enable_pod_disruption_budget: true
   secret_name_template: "{username}.{cluster}.credentials.{tprkind}.{tprgroup}"
   # inherited_labels:
   # - application

docs/administrator.md

@@ -154,6 +154,22 @@ data:
   pod_antiaffinity_topology_key: "failure-domain.beta.kubernetes.io/zone"
 ```
 
+### Pod Disruption Budget
+
+By default the operator uses a PodDisruptionBudget (PDB) to protect the cluster
+from voluntarily disruptions and hence unwanted DB downtime. The `MinAvailable`
+parameter of the PDB is set to `1` which prevents killing masters in single-node
+clusters and/or the last remaining running instance in a multi-node cluster.
+
+The PDB is only relaxed in two scenarios:
+* If a cluster is scaled down to `0` instances (e.g. for draining nodes)
+* If the PDB is disabled in the configuration (`enable_pod_disruption_budget`)
+
+The PDB is still in place having `MinAvailable` set to `0`. If enabled it will
+be automatically set to `1` on scale up. Disabling PDBs helps avoiding blocking
+Kubernetes upgrades in managed K8s environments at the cost of prolonged DB
+downtime. See PR #384 for the use case.
+
 ### Add cluster-specific labels
 
 In some cases, you might want to add `labels` that are specific to a given

docs/reference/operator_parameters.md

@@ -161,6 +161,13 @@ configuration they are grouped under the `kubernetes` key.
   replaced by the cluster name. Only the `{cluster}` placeholders is allowed in
   the template.
 
+* **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
+  necessary to temporarily disabled it, e.g. for node updates. See
+  [admin docs](../administrator.md#pod-disruption-budget) for more information.
+  Default is true.
+
 * **secret_name_template**
   a template for the name of the database user secrets generated by the
   operator. `{username}` is replaced with name of the secret, `{cluster}` with

manifests/postgresql-operator-default-configuration.yaml

@@ -20,6 +20,7 @@ configuration:
     pod_service_account_name: operator
     pod_terminate_grace_period: 5m
     pdb_name_format: "postgres-{cluster}-pdb"
+    enable_pod_disruption_budget: true
     secret_name_template: "{username}.{cluster}.credentials.{tprkind}.{tprgroup}"
     cluster_domain: cluster.local
     oauth_token_secret_name: postgresql-operator

pkg/apis/acid.zalan.do/v1/operator_configuration_type.go

@@ -49,6 +49,7 @@ type KubernetesMetaConfiguration struct {
 	SpiloFSGroup                           *int64                `json:"spilo_fsgroup,omitempty"`
 	WatchedNamespace                       string                `json:"watched_namespace,omitempty"`
 	PDBNameFormat                          config.StringTemplate `json:"pdb_name_format,omitempty"`
+	EnablePodDisruptionBudget              *bool                 `json:"enable_pod_disruption_budget,omitempty"`
 	SecretNameTemplate                     config.StringTemplate `json:"secret_name_template,omitempty"`
 	ClusterDomain                          string                `json:"cluster_domain"`
 	OAuthTokenSecretName                   spec.NamespacedName   `json:"oauth_token_secret_name,omitempty"`

pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go

@@ -76,6 +76,11 @@ func (in *KubernetesMetaConfiguration) DeepCopyInto(out *KubernetesMetaConfigura
 		*out = new(int64)
 		**out = **in
 	}
+	if in.EnablePodDisruptionBudget != nil {
+		in, out := &in.EnablePodDisruptionBudget, &out.EnablePodDisruptionBudget
+		*out = new(bool)
+		**out = **in
+	}
 	out.OAuthTokenSecretName = in.OAuthTokenSecretName
 	out.InfrastructureRolesSecretName = in.InfrastructureRolesSecretName
 	if in.ClusterLabels != nil {

pkg/cluster/cluster.go

@@ -579,6 +579,15 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 		}
 	}()
 
+	// pod disruption budget
+	if oldSpec.Spec.NumberOfInstances != newSpec.Spec.NumberOfInstances {
+		c.logger.Debug("syncing pod disruption budgets")
+		if err := c.syncPodDisruptionBudget(true); err != nil {
+			c.logger.Errorf("could not sync pod disruption budget: %v", err)
+			updateFailed = true
+		}
+	}
+
 	// logical backup job
 	func() {
 

pkg/cluster/k8sres.go

@@ -1300,6 +1300,12 @@ func (c *Cluster) generateCloneEnvironment(description *acidv1.CloneDescription)
 
 func (c *Cluster) generatePodDisruptionBudget() *policybeta1.PodDisruptionBudget {
 	minAvailable := intstr.FromInt(1)
+	pdbEnabled := c.OpConfig.EnablePodDisruptionBudget
+
+	// 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)
+	}
 
 	return &policybeta1.PodDisruptionBudget{
 		ObjectMeta: metav1.ObjectMeta{

pkg/cluster/k8sres_test.go

@@ -1,6 +1,8 @@
 package cluster
 
 import (
+	"reflect"
+
 	"k8s.io/api/core/v1"
 
 	"testing"
@@ -9,6 +11,10 @@ import (
 	"github.com/zalando/postgres-operator/pkg/util/config"
 	"github.com/zalando/postgres-operator/pkg/util/constants"
 	"github.com/zalando/postgres-operator/pkg/util/k8sutil"
+
+	policyv1beta1 "k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
 func True() *bool {
@@ -21,6 +27,11 @@ func False() *bool {
 	return &b
 }
 
+func toIntStr(val int) *intstr.IntOrString {
+	b := intstr.FromInt(val)
+	return &b
+}
+
 func TestGenerateSpiloJSONConfiguration(t *testing.T) {
 	var cluster = New(
 		Config{
@@ -143,6 +154,113 @@ func TestCreateLoadBalancerLogic(t *testing.T) {
 	}
 }
 
+func TestGeneratePodDisruptionBudget(t *testing.T) {
+	tests := []struct {
+		c   *Cluster
+		out policyv1beta1.PodDisruptionBudget
+	}{
+		// With multiple instances.
+		{
+			New(
+				Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-pdb"}},
+				k8sutil.KubernetesClient{},
+				acidv1.Postgresql{
+					ObjectMeta: metav1.ObjectMeta{Name: "myapp-database", Namespace: "myapp"},
+					Spec:       acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 3}},
+				logger),
+			policyv1beta1.PodDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "postgres-myapp-database-pdb",
+					Namespace: "myapp",
+					Labels:    map[string]string{"team": "myapp", "cluster-name": "myapp-database"},
+				},
+				Spec: policyv1beta1.PodDisruptionBudgetSpec{
+					MinAvailable: toIntStr(1),
+					Selector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{"spilo-role": "master", "cluster-name": "myapp-database"},
+					},
+				},
+			},
+		},
+		// With zero instances.
+		{
+			New(
+				Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-pdb"}},
+				k8sutil.KubernetesClient{},
+				acidv1.Postgresql{
+					ObjectMeta: metav1.ObjectMeta{Name: "myapp-database", Namespace: "myapp"},
+					Spec:       acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 0}},
+				logger),
+			policyv1beta1.PodDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "postgres-myapp-database-pdb",
+					Namespace: "myapp",
+					Labels:    map[string]string{"team": "myapp", "cluster-name": "myapp-database"},
+				},
+				Spec: policyv1beta1.PodDisruptionBudgetSpec{
+					MinAvailable: toIntStr(0),
+					Selector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{"spilo-role": "master", "cluster-name": "myapp-database"},
+					},
+				},
+			},
+		},
+		// With PodDisruptionBudget disabled.
+		{
+			New(
+				Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-pdb", EnablePodDisruptionBudget: False()}},
+				k8sutil.KubernetesClient{},
+				acidv1.Postgresql{
+					ObjectMeta: metav1.ObjectMeta{Name: "myapp-database", Namespace: "myapp"},
+					Spec:       acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 3}},
+				logger),
+			policyv1beta1.PodDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "postgres-myapp-database-pdb",
+					Namespace: "myapp",
+					Labels:    map[string]string{"team": "myapp", "cluster-name": "myapp-database"},
+				},
+				Spec: policyv1beta1.PodDisruptionBudgetSpec{
+					MinAvailable: toIntStr(0),
+					Selector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{"spilo-role": "master", "cluster-name": "myapp-database"},
+					},
+				},
+			},
+		},
+		// With non-default PDBNameFormat and PodDisruptionBudget explicitly enabled.
+		{
+			New(
+				Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-databass-budget", EnablePodDisruptionBudget: True()}},
+				k8sutil.KubernetesClient{},
+				acidv1.Postgresql{
+					ObjectMeta: metav1.ObjectMeta{Name: "myapp-database", Namespace: "myapp"},
+					Spec:       acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 3}},
+				logger),
+			policyv1beta1.PodDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "postgres-myapp-database-databass-budget",
+					Namespace: "myapp",
+					Labels:    map[string]string{"team": "myapp", "cluster-name": "myapp-database"},
+				},
+				Spec: policyv1beta1.PodDisruptionBudgetSpec{
+					MinAvailable: toIntStr(1),
+					Selector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{"spilo-role": "master", "cluster-name": "myapp-database"},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		result := tt.c.generatePodDisruptionBudget()
+		if !reflect.DeepEqual(*result, tt.out) {
+			t.Errorf("Expected PodDisruptionBudget: %#v, got %#v", tt.out, *result)
+		}
+	}
+}
+
 func TestShmVolume(t *testing.T) {
 	testName := "TestShmVolume"
 	tests := []struct {
@@ -269,6 +387,5 @@ func TestCloneEnv(t *testing.T) {
 			t.Errorf("%s %s: Expected env value %s, have %s instead",
 				testName, tt.subTest, tt.env.Value, env.Value)
 		}
-
 	}
 }

pkg/cluster/sync.go

@@ -73,20 +73,6 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error {
 		}
 	}
 
-	// create database objects unless we are running without pods or disabled that feature explicitly
-	if !(c.databaseAccessDisabled() || c.getNumberOfInstances(&newSpec.Spec) <= 0) {
-		c.logger.Debugf("syncing roles")
-		if err = c.syncRoles(); err != nil {
-			err = fmt.Errorf("could not sync roles: %v", err)
-			return err
-		}
-		c.logger.Debugf("syncing databases")
-		if err = c.syncDatabases(); err != nil {
-			err = fmt.Errorf("could not sync databases: %v", err)
-			return err
-		}
-	}
-
 	c.logger.Debug("syncing pod disruption budgets")
 	if err = c.syncPodDisruptionBudget(false); err != nil {
 		err = fmt.Errorf("could not sync pod disruption budget: %v", err)
@@ -103,6 +89,20 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error {
 		}
 	}
 
+	// create database objects unless we are running without pods or disabled that feature explicitly
+	if !(c.databaseAccessDisabled() || c.getNumberOfInstances(&newSpec.Spec) <= 0) {
+		c.logger.Debugf("syncing roles")
+		if err = c.syncRoles(); err != nil {
+			err = fmt.Errorf("could not sync roles: %v", err)
+			return err
+		}
+		c.logger.Debugf("syncing databases")
+		if err = c.syncDatabases(); err != nil {
+			err = fmt.Errorf("could not sync databases: %v", err)
+			return err
+		}
+	}
+
 	return err
 }
 

pkg/controller/operator_config.go

@@ -46,6 +46,7 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur
 	result.ClusterDomain = fromCRD.Kubernetes.ClusterDomain
 	result.WatchedNamespace = fromCRD.Kubernetes.WatchedNamespace
 	result.PDBNameFormat = fromCRD.Kubernetes.PDBNameFormat
+	result.EnablePodDisruptionBudget = fromCRD.Kubernetes.EnablePodDisruptionBudget
 	result.SecretNameTemplate = fromCRD.Kubernetes.SecretNameTemplate
 	result.OAuthTokenSecretName = fromCRD.Kubernetes.OAuthTokenSecretName
 	result.InfrastructureRolesSecretName = fromCRD.Kubernetes.InfrastructureRolesSecretName

pkg/util/config/config.go

@@ -114,6 +114,7 @@ type Config struct {
 	MasterDNSNameFormat       StringTemplate    `name:"master_dns_name_format" default:"{cluster}.{team}.{hostedzone}"`
 	ReplicaDNSNameFormat      StringTemplate    `name:"replica_dns_name_format" default:"{cluster}-repl.{team}.{hostedzone}"`
 	PDBNameFormat             StringTemplate    `name:"pdb_name_format" default:"postgres-{cluster}-pdb"`
+	EnablePodDisruptionBudget *bool             `name:"enable_pod_disruption_budget" default:"true"`
 	Workers                   uint32            `name:"workers" default:"4"`
 	APIPort                   int               `name:"api_port" default:"8080"`
 	RingLogLines              int               `name:"ring_log_lines" default:"100"`
@@ -123,7 +124,7 @@ type Config struct {
 	PodManagementPolicy       string            `name:"pod_management_policy" default:"ordered_ready"`
 	ProtectedRoles            []string          `name:"protected_role_names" default:"admin"`
 	PostgresSuperuserTeams    []string          `name:"postgres_superuser_teams" default:""`
-	SetMemoryRequestToLimit  bool              `name:"set_memory_request_to_limit" defaults:"false"`
+	SetMemoryRequestToLimit   bool              `name:"set_memory_request_to_limit" default:"false"`
 }
 
 // MustMarshal marshals the config or panics

pkg/util/k8sutil/k8sutil.go

@@ -158,7 +158,7 @@ func SamePDB(cur, new *policybeta1.PodDisruptionBudget) (match bool, reason stri
 	//TODO: improve comparison
 	match = reflect.DeepEqual(new.Spec, cur.Spec)
 	if !match {
-		reason = "new service spec doesn't match the current one"
+		reason = "new PDB spec doesn't match the current one"
 	}
 
 	return