add logical backup retention as manifest option (#2621)

* add logical backup retention as manifest option
* added unit test for logical backup envvar generation

charts/postgres-operator/crds/postgresqls.yaml

@@ -215,6 +215,8 @@ spec:
                 items:
                   type: object
                   x-kubernetes-preserve-unknown-fields: true
+              logicalBackupRetention:
+                type: string
               logicalBackupSchedule:
                 type: string
                 pattern: '^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$'

docs/reference/cluster_manifest.md

@@ -223,10 +223,17 @@ These parameters are grouped directly under  the `spec` key in the manifest.
   Determines if the logical backup of this cluster should be taken and uploaded
   to S3. Default: false. Optional.
 
+* **logicalBackupRetention**
+  You can set a retention time for the logical backup cron job to remove old backup
+  files after a new backup has been uploaded. Example values are "3 days", "2 weeks", or
+  "1 month". It takes precedence over the global `logical_backup_s3_retention_time`
+  configuration. Currently only supported for AWS. Optional.
+
 * **logicalBackupSchedule**
   Schedule for the logical backup K8s cron job. Please take
   [the reference schedule format](https://kubernetes.io/docs/tasks/job/automated-tasks-with-cron-jobs/#schedule)
-  into account. Optional. Default is: "30 00 \* \* \*"
+  into account. It takes precedence over the global `logical_backup_schedule`
+  configuration. Optional.
 
 * **additionalVolumes**
   List of additional volumes to mount in each container of the statefulset pod.

manifests/complete-postgres-manifest.yaml

@@ -151,6 +151,7 @@ spec:
 
 # run periodic backups with k8s cron jobs
 #  enableLogicalBackup: true
+#  logicalBackupRetention: "3 months"
 #  logicalBackupSchedule: "30 00 * * *"
 
 #  maintenanceWindows:

manifests/postgresql.crd.yaml

@@ -213,6 +213,8 @@ spec:
                 items:
                   type: object
                   x-kubernetes-preserve-unknown-fields: true
+              logicalBackupRetention:
+                type: string
               logicalBackupSchedule:
                 type: string
                 pattern: '^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$'

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

@@ -343,6 +343,9 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{
 							},
 						},
 					},
+					"logicalBackupRetention": {
+						Type: "string",
+					},
 					"logicalBackupSchedule": {
 						Type:    "string",
 						Pattern: "^(\\d+|\\*)(/\\d+)?(\\s+(\\d+|\\*)(/\\d+)?){4}$",

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

@@ -63,23 +63,24 @@ type PostgresSpec struct {
 	UsersWithSecretRotation        []string             `json:"usersWithSecretRotation,omitempty"`
 	UsersWithInPlaceSecretRotation []string             `json:"usersWithInPlaceSecretRotation,omitempty"`
 
-	NumberOfInstances     int32                       `json:"numberOfInstances"`
+	NumberOfInstances      int32                       `json:"numberOfInstances"`
-	MaintenanceWindows    []MaintenanceWindow         `json:"maintenanceWindows,omitempty"`
+	MaintenanceWindows     []MaintenanceWindow         `json:"maintenanceWindows,omitempty"`
-	Clone                 *CloneDescription           `json:"clone,omitempty"`
+	Clone                  *CloneDescription           `json:"clone,omitempty"`
-	Databases             map[string]string           `json:"databases,omitempty"`
+	Databases              map[string]string           `json:"databases,omitempty"`
-	PreparedDatabases     map[string]PreparedDatabase `json:"preparedDatabases,omitempty"`
+	PreparedDatabases      map[string]PreparedDatabase `json:"preparedDatabases,omitempty"`
-	SchedulerName         *string                     `json:"schedulerName,omitempty"`
+	SchedulerName          *string                     `json:"schedulerName,omitempty"`
-	NodeAffinity          *v1.NodeAffinity            `json:"nodeAffinity,omitempty"`
+	NodeAffinity           *v1.NodeAffinity            `json:"nodeAffinity,omitempty"`
-	Tolerations           []v1.Toleration             `json:"tolerations,omitempty"`
+	Tolerations            []v1.Toleration             `json:"tolerations,omitempty"`
-	Sidecars              []Sidecar                   `json:"sidecars,omitempty"`
+	Sidecars               []Sidecar                   `json:"sidecars,omitempty"`
-	InitContainers        []v1.Container              `json:"initContainers,omitempty"`
+	InitContainers         []v1.Container              `json:"initContainers,omitempty"`
-	PodPriorityClassName  string                      `json:"podPriorityClassName,omitempty"`
+	PodPriorityClassName   string                      `json:"podPriorityClassName,omitempty"`
-	ShmVolume             *bool                       `json:"enableShmVolume,omitempty"`
+	ShmVolume              *bool                       `json:"enableShmVolume,omitempty"`
-	EnableLogicalBackup   bool                        `json:"enableLogicalBackup,omitempty"`
+	EnableLogicalBackup    bool                        `json:"enableLogicalBackup,omitempty"`
-	LogicalBackupSchedule string                      `json:"logicalBackupSchedule,omitempty"`
+	LogicalBackupRetention string                      `json:"logicalBackupRetention,omitempty"`
-	StandbyCluster        *StandbyDescription         `json:"standby,omitempty"`
+	LogicalBackupSchedule  string                      `json:"logicalBackupSchedule,omitempty"`
-	PodAnnotations        map[string]string           `json:"podAnnotations,omitempty"`
+	StandbyCluster         *StandbyDescription         `json:"standby,omitempty"`
-	ServiceAnnotations    map[string]string           `json:"serviceAnnotations,omitempty"`
+	PodAnnotations         map[string]string           `json:"podAnnotations,omitempty"`
+	ServiceAnnotations     map[string]string           `json:"serviceAnnotations,omitempty"`
 	// MasterServiceAnnotations takes precedence over ServiceAnnotations for master role if not empty
 	MasterServiceAnnotations map[string]string `json:"masterServiceAnnotations,omitempty"`
 	// ReplicaServiceAnnotations takes precedence over ServiceAnnotations for replica role if not empty

pkg/cluster/k8sres.go

@@ -2360,6 +2360,8 @@ func (c *Cluster) generateLogicalBackupJob() (*batchv1.CronJob, error) {
 
 func (c *Cluster) generateLogicalBackupPodEnvVars() []v1.EnvVar {
 
+	backupProvider := c.OpConfig.LogicalBackup.LogicalBackupProvider
+
 	envVars := []v1.EnvVar{
 		{
 			Name:  "SCOPE",
@@ -2378,55 +2380,6 @@ func (c *Cluster) generateLogicalBackupPodEnvVars() []v1.EnvVar {
 				},
 			},
 		},
-		// Bucket env vars
-		{
-			Name:  "LOGICAL_BACKUP_PROVIDER",
-			Value: c.OpConfig.LogicalBackup.LogicalBackupProvider,
-		},
-		{
-			Name:  "LOGICAL_BACKUP_S3_BUCKET",
-			Value: c.OpConfig.LogicalBackup.LogicalBackupS3Bucket,
-		},
-		{
-			Name:  "LOGICAL_BACKUP_S3_REGION",
-			Value: c.OpConfig.LogicalBackup.LogicalBackupS3Region,
-		},
-		{
-			Name:  "LOGICAL_BACKUP_S3_ENDPOINT",
-			Value: c.OpConfig.LogicalBackup.LogicalBackupS3Endpoint,
-		},
-		{
-			Name:  "LOGICAL_BACKUP_S3_SSE",
-			Value: c.OpConfig.LogicalBackup.LogicalBackupS3SSE,
-		},
-		{
-			Name:  "LOGICAL_BACKUP_S3_RETENTION_TIME",
-			Value: c.OpConfig.LogicalBackup.LogicalBackupS3RetentionTime,
-		},
-		{
-			Name:  "LOGICAL_BACKUP_S3_BUCKET_PREFIX",
-			Value: c.OpConfig.LogicalBackup.LogicalBackupS3BucketPrefix,
-		},
-		{
-			Name:  "LOGICAL_BACKUP_S3_BUCKET_SCOPE_SUFFIX",
-			Value: getBucketScopeSuffix(string(c.Postgresql.GetUID())),
-		},
-		{
-			Name:  "LOGICAL_BACKUP_GOOGLE_APPLICATION_CREDENTIALS",
-			Value: c.OpConfig.LogicalBackup.LogicalBackupGoogleApplicationCredentials,
-		},
-		{
-			Name:  "LOGICAL_BACKUP_AZURE_STORAGE_ACCOUNT_NAME",
-			Value: c.OpConfig.LogicalBackup.LogicalBackupAzureStorageAccountName,
-		},
-		{
-			Name:  "LOGICAL_BACKUP_AZURE_STORAGE_CONTAINER",
-			Value: c.OpConfig.LogicalBackup.LogicalBackupAzureStorageContainer,
-		},
-		{
-			Name:  "LOGICAL_BACKUP_AZURE_STORAGE_ACCOUNT_KEY",
-			Value: c.OpConfig.LogicalBackup.LogicalBackupAzureStorageAccountKey,
-		},
 		// Postgres env vars
 		{
 			Name:  "PG_VERSION",
@@ -2459,19 +2412,83 @@ func (c *Cluster) generateLogicalBackupPodEnvVars() []v1.EnvVar {
 				},
 			},
 		},
+		// Bucket env vars
+		{
+			Name:  "LOGICAL_BACKUP_PROVIDER",
+			Value: backupProvider,
+		},
+		{
+			Name:  "LOGICAL_BACKUP_S3_BUCKET",
+			Value: c.OpConfig.LogicalBackup.LogicalBackupS3Bucket,
+		},
+		{
+			Name:  "LOGICAL_BACKUP_S3_BUCKET_PREFIX",
+			Value: c.OpConfig.LogicalBackup.LogicalBackupS3BucketPrefix,
+		},
+		{
+			Name:  "LOGICAL_BACKUP_S3_BUCKET_SCOPE_SUFFIX",
+			Value: getBucketScopeSuffix(string(c.Postgresql.GetUID())),
+		},
 	}
 
-	if c.OpConfig.LogicalBackup.LogicalBackupS3AccessKeyID != "" {
+	switch backupProvider {
-		envVars = append(envVars, v1.EnvVar{Name: "AWS_ACCESS_KEY_ID", Value: c.OpConfig.LogicalBackup.LogicalBackupS3AccessKeyID})
+	case "s3":
-	}
+		envVars = appendEnvVars(envVars, []v1.EnvVar{
+			{
+				Name:  "LOGICAL_BACKUP_S3_REGION",
+				Value: c.OpConfig.LogicalBackup.LogicalBackupS3Region,
+			},
+			{
+				Name:  "LOGICAL_BACKUP_S3_ENDPOINT",
+				Value: c.OpConfig.LogicalBackup.LogicalBackupS3Endpoint,
+			},
+			{
+				Name:  "LOGICAL_BACKUP_S3_SSE",
+				Value: c.OpConfig.LogicalBackup.LogicalBackupS3SSE,
+			},
+			{
+				Name:  "LOGICAL_BACKUP_S3_RETENTION_TIME",
+				Value: c.getLogicalBackupRetentionTime(),
+			}}...)
 
-	if c.OpConfig.LogicalBackup.LogicalBackupS3SecretAccessKey != "" {
+		if c.OpConfig.LogicalBackup.LogicalBackupS3AccessKeyID != "" {
-		envVars = append(envVars, v1.EnvVar{Name: "AWS_SECRET_ACCESS_KEY", Value: c.OpConfig.LogicalBackup.LogicalBackupS3SecretAccessKey})
+			envVars = append(envVars, v1.EnvVar{Name: "AWS_ACCESS_KEY_ID", Value: c.OpConfig.LogicalBackup.LogicalBackupS3AccessKeyID})
+		}
+
+		if c.OpConfig.LogicalBackup.LogicalBackupS3SecretAccessKey != "" {
+			envVars = append(envVars, v1.EnvVar{Name: "AWS_SECRET_ACCESS_KEY", Value: c.OpConfig.LogicalBackup.LogicalBackupS3SecretAccessKey})
+		}
+
+	case "gcs":
+		envVars = append(envVars, v1.EnvVar{Name: "LOGICAL_BACKUP_GOOGLE_APPLICATION_CREDENTIALS", Value: c.OpConfig.LogicalBackup.LogicalBackupGoogleApplicationCredentials})
+
+	case "az":
+		envVars = appendEnvVars(envVars, []v1.EnvVar{
+			{
+				Name:  "LOGICAL_BACKUP_AZURE_STORAGE_ACCOUNT_NAME",
+				Value: c.OpConfig.LogicalBackup.LogicalBackupAzureStorageAccountName,
+			},
+			{
+				Name:  "LOGICAL_BACKUP_AZURE_STORAGE_CONTAINER",
+				Value: c.OpConfig.LogicalBackup.LogicalBackupAzureStorageContainer,
+			},
+			{
+				Name:  "LOGICAL_BACKUP_AZURE_STORAGE_ACCOUNT_KEY",
+				Value: c.OpConfig.LogicalBackup.LogicalBackupAzureStorageAccountKey,
+			}}...)
 	}
 
 	return envVars
 }
 
+func (c *Cluster) getLogicalBackupRetentionTime() (retentionTime string) {
+	if c.Spec.LogicalBackupRetention != "" {
+		return c.Spec.LogicalBackupRetention
+	}
+
+	return c.OpConfig.LogicalBackup.LogicalBackupS3RetentionTime
+}
+
 // getLogicalBackupJobName returns the name; the job itself may not exists
 func (c *Cluster) getLogicalBackupJobName() (jobName string) {
 	return trimCronjobName(fmt.Sprintf("%s%s", c.OpConfig.LogicalBackupJobPrefix, c.clusterName().Name))

pkg/cluster/k8sres_test.go

@@ -3524,6 +3524,191 @@ func TestGenerateLogicalBackupJob(t *testing.T) {
 	}
 }
 
+func TestGenerateLogicalBackupPodEnvVars(t *testing.T) {
+	var (
+		dummyUUID   = "efd12e58-5786-11e8-b5a7-06148230260c"
+		dummyBucket = "dummy-backup-location"
+	)
+
+	expectedLogicalBackupS3Bucket := []ExpectedValue{
+		{
+			envIndex:       9,
+			envVarConstant: "LOGICAL_BACKUP_PROVIDER",
+			envVarValue:    "s3",
+		},
+		{
+			envIndex:       10,
+			envVarConstant: "LOGICAL_BACKUP_S3_BUCKET",
+			envVarValue:    dummyBucket,
+		},
+		{
+			envIndex:       11,
+			envVarConstant: "LOGICAL_BACKUP_S3_BUCKET_PREFIX",
+			envVarValue:    "spilo",
+		},
+		{
+			envIndex:       12,
+			envVarConstant: "LOGICAL_BACKUP_S3_BUCKET_SCOPE_SUFFIX",
+			envVarValue:    "/" + dummyUUID,
+		},
+		{
+			envIndex:       13,
+			envVarConstant: "LOGICAL_BACKUP_S3_REGION",
+			envVarValue:    "eu-central-1",
+		},
+		{
+			envIndex:       14,
+			envVarConstant: "LOGICAL_BACKUP_S3_ENDPOINT",
+			envVarValue:    "",
+		},
+		{
+			envIndex:       15,
+			envVarConstant: "LOGICAL_BACKUP_S3_SSE",
+			envVarValue:    "",
+		},
+		{
+			envIndex:       16,
+			envVarConstant: "LOGICAL_BACKUP_S3_RETENTION_TIME",
+			envVarValue:    "1 month",
+		},
+	}
+
+	expectedLogicalBackupGCPCreds := []ExpectedValue{
+		{
+			envIndex:       9,
+			envVarConstant: "LOGICAL_BACKUP_PROVIDER",
+			envVarValue:    "gcs",
+		},
+		{
+			envIndex:       13,
+			envVarConstant: "LOGICAL_BACKUP_GOOGLE_APPLICATION_CREDENTIALS",
+			envVarValue:    "some-path-to-credentials",
+		},
+	}
+
+	expectedLogicalBackupAzureStorage := []ExpectedValue{
+		{
+			envIndex:       9,
+			envVarConstant: "LOGICAL_BACKUP_PROVIDER",
+			envVarValue:    "az",
+		},
+		{
+			envIndex:       13,
+			envVarConstant: "LOGICAL_BACKUP_AZURE_STORAGE_ACCOUNT_NAME",
+			envVarValue:    "some-azure-storage-account-name",
+		},
+		{
+			envIndex:       14,
+			envVarConstant: "LOGICAL_BACKUP_AZURE_STORAGE_CONTAINER",
+			envVarValue:    "some-azure-storage-container",
+		},
+		{
+			envIndex:       15,
+			envVarConstant: "LOGICAL_BACKUP_AZURE_STORAGE_ACCOUNT_KEY",
+			envVarValue:    "some-azure-storage-account-key",
+		},
+	}
+
+	expectedLogicalBackupRetentionTime := []ExpectedValue{
+		{
+			envIndex:       16,
+			envVarConstant: "LOGICAL_BACKUP_S3_RETENTION_TIME",
+			envVarValue:    "3 months",
+		},
+	}
+
+	tests := []struct {
+		subTest        string
+		opConfig       config.Config
+		expectedValues []ExpectedValue
+		pgsql          acidv1.Postgresql
+	}{
+		{
+			subTest: "logical backup with provider: s3",
+			opConfig: config.Config{
+				LogicalBackup: config.LogicalBackup{
+					LogicalBackupProvider:        "s3",
+					LogicalBackupS3Bucket:        dummyBucket,
+					LogicalBackupS3BucketPrefix:  "spilo",
+					LogicalBackupS3Region:        "eu-central-1",
+					LogicalBackupS3RetentionTime: "1 month",
+				},
+			},
+			expectedValues: expectedLogicalBackupS3Bucket,
+		},
+		{
+			subTest: "logical backup with provider: gcs",
+			opConfig: config.Config{
+				LogicalBackup: config.LogicalBackup{
+					LogicalBackupProvider:                     "gcs",
+					LogicalBackupS3Bucket:                     dummyBucket,
+					LogicalBackupGoogleApplicationCredentials: "some-path-to-credentials",
+				},
+			},
+			expectedValues: expectedLogicalBackupGCPCreds,
+		},
+		{
+			subTest: "logical backup with provider: az",
+			opConfig: config.Config{
+				LogicalBackup: config.LogicalBackup{
+					LogicalBackupProvider:                "az",
+					LogicalBackupS3Bucket:                dummyBucket,
+					LogicalBackupAzureStorageAccountName: "some-azure-storage-account-name",
+					LogicalBackupAzureStorageContainer:   "some-azure-storage-container",
+					LogicalBackupAzureStorageAccountKey:  "some-azure-storage-account-key",
+				},
+			},
+			expectedValues: expectedLogicalBackupAzureStorage,
+		},
+		{
+			subTest: "will override retention time parameter",
+			opConfig: config.Config{
+				LogicalBackup: config.LogicalBackup{
+					LogicalBackupProvider:        "s3",
+					LogicalBackupS3RetentionTime: "1 month",
+				},
+			},
+			expectedValues: expectedLogicalBackupRetentionTime,
+			pgsql: acidv1.Postgresql{
+				Spec: acidv1.PostgresSpec{
+					LogicalBackupRetention: "3 months",
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		c := newMockCluster(tt.opConfig)
+		pgsql := tt.pgsql
+		c.Postgresql = pgsql
+		c.UID = types.UID(dummyUUID)
+
+		actualEnvs := c.generateLogicalBackupPodEnvVars()
+
+		for _, ev := range tt.expectedValues {
+			env := actualEnvs[ev.envIndex]
+
+			if env.Name != ev.envVarConstant {
+				t.Errorf("%s %s: expected env name %s, have %s instead",
+					t.Name(), tt.subTest, ev.envVarConstant, env.Name)
+			}
+
+			if ev.envVarValueRef != nil {
+				if !reflect.DeepEqual(env.ValueFrom, ev.envVarValueRef) {
+					t.Errorf("%s %s: expected env value reference %#v, have %#v instead",
+						t.Name(), tt.subTest, ev.envVarValueRef, env.ValueFrom)
+				}
+				continue
+			}
+
+			if env.Value != ev.envVarValue {
+				t.Errorf("%s %s: expected env value %s, have %s instead",
+					t.Name(), tt.subTest, ev.envVarValue, env.Value)
+			}
+		}
+	}
+}
+
 func TestGenerateCapabilities(t *testing.T) {
 	tests := []struct {
 		subTest      string