Merge branch 'master' of https://github.com/zalando/postgres-operator into standby

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

@@ -342,11 +342,12 @@ func isBootstrapOnlyParameter(param string) bool {
 		param == "track_commit_timestamp"
 }
 
-func generateVolumeMounts() []v1.VolumeMount {
+func generateVolumeMounts(volume acidv1.Volume) []v1.VolumeMount {
 	return []v1.VolumeMount{
 		{
 			Name:      constants.DataVolumeName,
 			MountPath: constants.PostgresDataMount, //TODO: fetch from manifest
+			SubPath:   volume.SubPath,
 		},
 	}
 }
@@ -359,6 +360,8 @@ func generateContainer(
 	volumeMounts []v1.VolumeMount,
 	privilegedMode bool,
 ) *v1.Container {
+	falseBool := false
+
 	return &v1.Container{
 		Name:            name,
 		Image:           *dockerImage,
@@ -381,7 +384,8 @@ func generateContainer(
 		VolumeMounts: volumeMounts,
 		Env:          envVars,
 		SecurityContext: &v1.SecurityContext{
-			Privileged: &privilegedMode,
+			Privileged:             &privilegedMode,
+			ReadOnlyRootFilesystem: &falseBool,
 		},
 	}
 }
@@ -441,6 +445,8 @@ func generatePodTemplate(
 	shmVolume bool,
 	podAntiAffinity bool,
 	podAntiAffinityTopologyKey string,
+	additionalSecretMount string,
+	additionalSecretMountPath string,
 ) (*v1.PodTemplateSpec, error) {
 
 	terminateGracePeriodSeconds := terminateGracePeriod
@@ -475,6 +481,10 @@ func generatePodTemplate(
 		podSpec.PriorityClassName = priorityClassName
 	}
 
+	if additionalSecretMount != "" {
+		addSecretVolume(&podSpec, additionalSecretMount, additionalSecretMountPath)
+	}
+
 	template := v1.PodTemplateSpec{
 		ObjectMeta: metav1.ObjectMeta{
 			Labels:    labels,
@@ -804,7 +814,7 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*v1beta1.State
 	// pickup the docker image for the spilo container
 	effectiveDockerImage := util.Coalesce(spec.DockerImage, c.OpConfig.DockerImage)
 
-	volumeMounts := generateVolumeMounts()
+	volumeMounts := generateVolumeMounts(spec.Volume)
 
 	// generate the spilo container
 	c.logger.Debugf("Generating Spilo container, environment variables: %v", spiloEnvVars)
@@ -867,7 +877,9 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*v1beta1.State
 		effectivePodPriorityClassName,
 		mountShmVolumeNeeded(c.OpConfig, spec),
 		c.OpConfig.EnablePodAntiAffinity,
-		c.OpConfig.PodAntiAffinityTopologyKey); err != nil {
+		c.OpConfig.PodAntiAffinityTopologyKey,
+		c.OpConfig.AdditionalSecretMount,
+		c.OpConfig.AdditionalSecretMountPath); err != nil {
 		return nil, fmt.Errorf("could not generate pod template: %v", err)
 	}
 
@@ -1021,6 +1033,28 @@ func addShmVolume(podSpec *v1.PodSpec) {
 	podSpec.Volumes = volumes
 }
 
+func addSecretVolume(podSpec *v1.PodSpec, additionalSecretMount string, additionalSecretMountPath string) {
+	volumes := append(podSpec.Volumes, v1.Volume{
+		Name: additionalSecretMount,
+		VolumeSource: v1.VolumeSource{
+			Secret: &v1.SecretVolumeSource{
+				SecretName: additionalSecretMount,
+			},
+		},
+	})
+
+	for i := range podSpec.Containers {
+		mounts := append(podSpec.Containers[i].VolumeMounts,
+			v1.VolumeMount{
+				Name:      additionalSecretMount,
+				MountPath: additionalSecretMountPath,
+			})
+		podSpec.Containers[i].VolumeMounts = mounts
+	}
+
+	podSpec.Volumes = volumes
+}
+
 func generatePersistentVolumeClaimTemplate(volumeSize, volumeStorageClass string) (*v1.PersistentVolumeClaim, error) {
 
 	var storageClassName *string
@@ -1329,6 +1363,12 @@ func (c *Cluster) generateStandbyEnvironment(description *acidv1.StandbyDescript
 
 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{
@@ -1418,6 +1458,8 @@ func (c *Cluster) generateLogicalBackupJob() (*batchv1beta1.CronJob, error) {
 		"",
 		false,
 		false,
+		"",
+		"",
 		""); err != nil {
 		return nil, fmt.Errorf("could not generate pod template for logical backup pod: %v", err)
 	}

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,76 @@ func TestCloneEnv(t *testing.T) {
 			t.Errorf("%s %s: Expected env value %s, have %s instead",
 				testName, tt.subTest, tt.env.Value, env.Value)
 		}
+	}
+}
 
+func TestSecretVolume(t *testing.T) {
+	testName := "TestSecretVolume"
+	tests := []struct {
+		subTest   string
+		podSpec   *v1.PodSpec
+		secretPos int
+	}{
+		{
+			subTest: "empty PodSpec",
+			podSpec: &v1.PodSpec{
+				Volumes: []v1.Volume{},
+				Containers: []v1.Container{
+					{
+						VolumeMounts: []v1.VolumeMount{},
+					},
+				},
+			},
+			secretPos: 0,
+		},
+		{
+			subTest: "non empty PodSpec",
+			podSpec: &v1.PodSpec{
+				Volumes: []v1.Volume{{}},
+				Containers: []v1.Container{
+					{
+						VolumeMounts: []v1.VolumeMount{
+							{
+								Name:      "data",
+								ReadOnly:  false,
+								MountPath: "/data",
+							},
+						},
+					},
+				},
+			},
+			secretPos: 1,
+		},
+	}
+	for _, tt := range tests {
+		additionalSecretMount := "aws-iam-s3-role"
+		additionalSecretMountPath := "/meta/credentials"
+
+		numMounts := len(tt.podSpec.Containers[0].VolumeMounts)
+
+		addSecretVolume(tt.podSpec, additionalSecretMount, additionalSecretMountPath)
+
+		volumeName := tt.podSpec.Volumes[tt.secretPos].Name
+
+		if volumeName != additionalSecretMount {
+			t.Errorf("%s %s: Expected volume %s was not created, have %s instead",
+				testName, tt.subTest, additionalSecretMount, volumeName)
+		}
+
+		for i := range tt.podSpec.Containers {
+			volumeMountName := tt.podSpec.Containers[i].VolumeMounts[tt.secretPos].Name
+
+			if volumeMountName != additionalSecretMount {
+				t.Errorf("%s %s: Expected mount %s was not created, have %s instead",
+					testName, tt.subTest, additionalSecretMount, volumeMountName)
+			}
+		}
+
+		numMountsCheck := len(tt.podSpec.Containers[0].VolumeMounts)
+
+		if numMountsCheck != numMounts+1 {
+			t.Errorf("Unexpected number of VolumeMounts: got %v instead of %v",
+				numMountsCheck, numMounts+1)
+		}
 	}
 }

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
 }