Address review comments

docs/administrator.md

@@ -229,10 +229,10 @@ The operator logs reasons for a rolling update with the `info` level and a diff
 
 ## Logical backups
 
-The operator can launch k8s cron jobs to do periodic logical backups of all PG clusters under its control. The cron job spawns a separate pod with a single container that connects to one of the Postgres replicas for a backup. The operator updates cron jobs during Sync if a schedule or a docker image of a job changes. Notes:
+The operator can manage k8s cron jobs to do periodic logical backups of all PG clusters under its control. The cron job spawns a separate pod with a single container that connects to one of the Postgres replicas for a backup. The operator updates cron jobs during Sync if a schedule or a docker image of a job changes. Notes:
 
 1. The provided  `registry.opensource.zalan.do/acid/logical-backup` image implements the backup via `pg_dumpall` and upload of (compressed) results to an S3 bucket; `pg_dumpall` requires a `superuser` access to a DB.
 
 2. Due to the [limitation of Kubernetes cron jobs](https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/#cron-job-limitations) it is highly advisable to set up additional monitoring for this feature; such monitoring is outside of the scope of operator responsibilities. 
 
-3. The operator does not remove old backups: it is the responsibility of a k8s cluster administrator to define and implement a retention policy for backups.
+3. The operator does not remove old backups.

docs/reference/cluster_manifest.md

@@ -39,7 +39,7 @@ Those parameters are grouped under the `metadata` top-level key.
 
 ## Top-level parameters
 
-These are parameters are grouped directly under  the `spec` key in the manifest.
+These parameters are grouped directly under  the `spec` key in the manifest.
 
 * **teamId**
   name of the team the cluster belongs to. Changing it after the cluster
@@ -111,6 +111,12 @@ These are parameters are grouped directly under  the `spec` key in the manifest.
   is `false`, then no volume will be mounted no matter how operator was
   configured (so you can override the operator configuration).
 
+* **enableLogicalBackup**
+  Determines if the logical backup of this cluster should be taken and uploaded to S3. Default: false.
+
+* **logicalBackupSchedule**
+  Backup schedule in the Cron format. Default: "30 00 \* \* \*"
+
 ## Postgres parameters
 
 Those parameters are grouped under the `postgresql` top-level key.
@@ -256,11 +262,3 @@ defined in the sidecar dictionary:
   a dictionary of environment variables. Use usual Kubernetes definition
   (https://kubernetes.io/docs/tasks/inject-data-application/environment-variable-expose-pod-information/)
   for environment variables. Optional.
-
-### Logical backup
-
- Those parameters are defined under the `logical_backup` key:
-  *   **enable_logical_backup**
-     determines if the logical backup of this cluster should be taken and uploaded to S3. Default: false.
-  *   **logical_backup_schedule**
-     backup schedule in Cron format. Default: "30 00 * * *"

docs/user.md

@@ -316,4 +316,4 @@ If you add
 logical_backup:
   enable_logical_backup: true
 ```
-to the cluster manifest, the operator will start a k8s cron job that will periodically execute `pg_dumpall` on the target PG cluster and upload results to an S3 bucket. Note that due to the [limitation of Kubernetes cron jobs](https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/#cron-job-limitations) it is highly advisable to set up additional monitoring for this feature; such monitoring is outside of the scope of operator responsibilities. See [configuration reference](reference/cluster_manifest.md) for details.
+to the cluster manifest, the operator will create and sync the configuration of a k8s cron job that periodically executes `pg_dumpall` on the target PG cluster and uploads results to an S3 bucket. Note that due to the [limitation of Kubernetes cron jobs](https://kubernetes.io/docs/concepts/workloads/controllers/cron-jobs/#cron-job-limitations) it is highly advisable to set up additional monitoring for this feature; such monitoring is outside of the scope of operator responsibilities. See [configuration reference](reference/cluster_manifest.md) for details.

manifests/complete-postgres-manifest.yaml

@@ -61,9 +61,8 @@ spec:
   #  timestamp: "2017-12-19T12:40:33+01:00" # timezone required (offset relative to UTC, see RFC 3339 section 5.6)
   
   # run periodic backups with k8s cron jobs and pg_dumpall
-  # logical_backup:
+  # enableLogicalBackup: true
-  #   enable_logical_backup: true
+  # logicalBackupSchedule: "30 00 * * *"
-  #   logical_backup_schedule: "30 00 * * *"
   maintenanceWindows:
   - 01:00-06:00 #UTC
   - Sat:00:00-04:00

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

@@ -152,8 +152,8 @@ type OperatorConfigurationUsers struct {
 type Duration time.Duration
 
 type OperatorLogicalBackupConfiguration struct {
-	EnableLogicalBackup      bool   `json:"enable_logical_backup,omitempty"`
+	Enable      bool   `json:"enable_logical_backup,omitempty"`
-	LogicalBackupSchedule    string `json:"logical_backup_schedule,omitempty"`
+	Schedule    string `json:"logical_backup_schedule,omitempty"`
-	LogicalBackupDockerImage string `json:"logical_backup_docker_image,omitempty"`
+	DockerImage string `json:"logical_backup_docker_image,omitempty"`
-	LogicalBackupS3Bucket    string `json:"logical_backup_s3_bucket,omitempty"`
+	S3Bucket    string `json:"logical_backup_s3_bucket,omitempty"`
 }

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

@@ -25,7 +25,6 @@ type PostgresSpec struct {
 	Volume          `json:"volume,omitempty"`
 	Patroni         `json:"patroni,omitempty"`
 	Resources       `json:"resources,omitempty"`
-	LogicalBackup   `json:"logical_backup,omitempty"`
 
 	TeamID      string `json:"teamId"`
 	DockerImage string `json:"dockerImage,omitempty"`
@@ -53,6 +52,10 @@ type PostgresSpec struct {
 	Sidecars             []Sidecar            `json:"sidecars,omitempty"`
 	PodPriorityClassName string               `json:"pod_priority_class_name,omitempty"`
 	ShmVolume            *bool                `json:"enableShmVolume,omitempty"`
+
+	// LogicalBackup contains config of a k8s cron job responsible for running pg_dumpall
+	EnableLogicalBackup   bool   `json:"enableLogicalBackup,omitempty"`
+	LogicalBackupSchedule string `json:"logicalBackupSchedule,omitempty"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
@@ -128,9 +131,3 @@ type UserFlags []string
 
 // PostgresStatus contains status of the PostgreSQL cluster (running, creation failed etc.)
 type PostgresStatus string
-
-// LogicalBackup contains config of a k8s cron job responsible for running pg_dumpall
-type LogicalBackup struct {
-	EnableLogicalBackup   bool   `json:"enable_logical_backup,omitempty"`
-	LogicalBackupSchedule string `json:"logical_backup_schedule,omitempty"`
-}

pkg/controller/operator_config.go

@@ -97,10 +97,10 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur
 	result.ScalyrCPULimit = fromCRD.Scalyr.ScalyrCPULimit
 	result.ScalyrMemoryLimit = fromCRD.Scalyr.ScalyrMemoryLimit
 
-	result.EnableLogicalBackup = fromCRD.LogicalBackup.EnableLogicalBackup
+	result.EnableLogicalBackup = fromCRD.LogicalBackup.Enable
-	result.LogicalBackupSchedule = fromCRD.LogicalBackup.LogicalBackupSchedule
+	result.LogicalBackupSchedule = fromCRD.LogicalBackup.Schedule
-	result.LogicalBackupDockerImage = fromCRD.LogicalBackup.LogicalBackupDockerImage
+	result.LogicalBackupDockerImage = fromCRD.LogicalBackup.DockerImage
-	result.LogicalBackupS3Bucket = fromCRD.LogicalBackup.LogicalBackupS3Bucket
+	result.LogicalBackupS3Bucket = fromCRD.LogicalBackup.S3Bucket
 
 	return result
 }