Add/adjust tests for cloud repo backup job changes.

Author: dsessler7

internal/controller/postgrescluster/instance.go

@@ -1172,7 +1172,7 @@ func (r *Reconciler) reconcileInstance(
 	}
 	if err == nil {
 		instanceCertificates, err = r.reconcileInstanceCertificates(
-			ctx, cluster, spec, instance, rootCA)
+			ctx, cluster, spec, instance, rootCA, backupsSpecFound)
 	}
 	if err == nil {
 		postgresDataVolume, err = r.reconcilePostgresDataVolume(ctx, cluster, spec, instance, clusterVolumes, nil)
@@ -1465,7 +1465,7 @@ func (r *Reconciler) reconcileInstanceConfigMap(
 func (r *Reconciler) reconcileInstanceCertificates(
 	ctx context.Context, cluster *v1beta1.PostgresCluster,
 	spec *v1beta1.PostgresInstanceSetSpec, instance *appsv1.StatefulSet,
-	root *pki.RootCertificateAuthority,
+	root *pki.RootCertificateAuthority, backupsSpecFound bool,
 ) (*corev1.Secret, error) {
 	existing := &corev1.Secret{ObjectMeta: naming.InstanceCertificates(instance)}
 	err := errors.WithStack(client.IgnoreNotFound(
@@ -1508,7 +1508,7 @@ func (r *Reconciler) reconcileInstanceCertificates(
 			root.Certificate, leafCert.Certificate,
 			leafCert.PrivateKey, instanceCerts)
 	}
-	if err == nil {
+	if err == nil && backupsSpecFound {
 		err = pgbackrest.InstanceCertificates(ctx, cluster,
 			root.Certificate, leafCert.Certificate, leafCert.PrivateKey,
 			instanceCerts)

internal/controller/postgrescluster/instance_test.go

@@ -544,49 +544,7 @@ func TestAddPGBackRestToInstancePodSpec(t *testing.T) {
 		},
 	}
 
-	t.Run("NoVolumeRepo", func(t *testing.T) {
-		cluster := cluster.DeepCopy()
-		cluster.Spec.Backups.PGBackRest.Repos = nil
-
-		out := pod.DeepCopy()
-		addPGBackRestToInstancePodSpec(ctx, cluster, &certificates, out)
-
-		// Only Containers and Volumes fields have changed.
-		assert.DeepEqual(t, pod, *out, cmpopts.IgnoreFields(pod, "Containers", "Volumes"))
-
-		// Only database container has mounts.
-		// Other containers are ignored.
-		assert.Assert(t, cmp.MarshalMatches(out.Containers, `
-- name: database
-  resources: {}
-  volumeMounts:
-  - mountPath: /etc/pgbackrest/conf.d
-    name: pgbackrest-config
-    readOnly: true
-- name: other
-  resources: {}
-		`))
-
-		// Instance configuration files but no certificates.
-		// Other volumes are ignored.
-		assert.Assert(t, cmp.MarshalMatches(out.Volumes, `
-- name: other
-- name: postgres-data
-- name: postgres-wal
-- name: pgbackrest-config
-  projected:
-    sources:
-    - configMap:
-        items:
-        - key: pgbackrest_instance.conf
-          path: pgbackrest_instance.conf
-        - key: config-hash
-          path: config-hash
-        name: hippo-pgbackrest-config
-		`))
-	})
-
-	t.Run("OneVolumeRepo", func(t *testing.T) {
+	t.Run("CloudOrVolumeSameBehavior", func(t *testing.T) {
 		alwaysExpect := func(t testing.TB, result *corev1.PodSpec) {
 			// Only Containers and Volumes fields have changed.
 			assert.DeepEqual(t, pod, *result, cmpopts.IgnoreFields(pod, "Containers", "Volumes"))
@@ -635,21 +593,31 @@ func TestAddPGBackRestToInstancePodSpec(t *testing.T) {
 			`))
 		}
 
-		cluster := cluster.DeepCopy()
-		cluster.Spec.Backups.PGBackRest.Repos = []v1beta1.PGBackRestRepo{
+		clusterWithVolume := cluster.DeepCopy()
+		clusterWithVolume.Spec.Backups.PGBackRest.Repos = []v1beta1.PGBackRestRepo{
 			{
 				Name:   "repo1",
 				Volume: new(v1beta1.RepoPVC),
 			},
 		}
 
-		out := pod.DeepCopy()
-		addPGBackRestToInstancePodSpec(ctx, cluster, &certificates, out)
-		alwaysExpect(t, out)
+		clusterWithCloudRepo := cluster.DeepCopy()
+		clusterWithCloudRepo.Spec.Backups.PGBackRest.Repos = []v1beta1.PGBackRestRepo{
+			{
+				Name: "repo1",
+				GCS:  new(v1beta1.RepoGCS),
+			},
+		}
+
+		outWithVolume := pod.DeepCopy()
+		addPGBackRestToInstancePodSpec(ctx, clusterWithVolume, &certificates, outWithVolume)
+		alwaysExpect(t, outWithVolume)
 
-		// The TLS server is added and configuration mounted.
-		// It has PostgreSQL volumes mounted while other volumes are ignored.
-		assert.Assert(t, cmp.MarshalMatches(out.Containers, `
+		outWithCloudRepo := pod.DeepCopy()
+		addPGBackRestToInstancePodSpec(ctx, clusterWithCloudRepo, &certificates, outWithCloudRepo)
+		alwaysExpect(t, outWithCloudRepo)
+
+		outContainers := `
 - name: database
   resources: {}
   volumeMounts:
@@ -737,7 +705,12 @@ func TestAddPGBackRestToInstancePodSpec(t *testing.T) {
   - mountPath: /etc/pgbackrest/conf.d
     name: pgbackrest-config
     readOnly: true
-		`))
+		`
+
+		// The TLS server is added and configuration mounted.
+		// It has PostgreSQL volumes mounted while other volumes are ignored.
+		assert.Assert(t, cmp.MarshalMatches(outWithVolume.Containers, outContainers))
+		assert.Assert(t, cmp.MarshalMatches(outWithCloudRepo.Containers, outContainers))
 
 		t.Run("CustomResources", func(t *testing.T) {
 			cluster := cluster.DeepCopy()
@@ -754,7 +727,7 @@ func TestAddPGBackRestToInstancePodSpec(t *testing.T) {
 				},
 			}
 
-			before := out.DeepCopy()
+			before := outWithVolume.DeepCopy()
 			out := pod.DeepCopy()
 			addPGBackRestToInstancePodSpec(ctx, cluster, &certificates, out)
 			alwaysExpect(t, out)

internal/controller/postgrescluster/pgbackrest.go

@@ -2034,7 +2034,7 @@ func (r *Reconciler) copyConfigurationResources(ctx context.Context, cluster,
 	return nil
 }
 
-// reconcilePGBackRestConfig is responsible for reconciling the pgBackRest ConfigMaps and Secrets.
+// reconcilePGBackRestConfig is responsible for reconciling the pgBackRest ConfigMaps.
 func (r *Reconciler) reconcilePGBackRestConfig(ctx context.Context,
 	postgresCluster *v1beta1.PostgresCluster,
 	repoHostName, configHash, serviceName, serviceNamespace string,

internal/controller/postgrescluster/pgbackrest_test.go

@@ -2602,9 +2602,81 @@ func TestCopyConfigurationResources(t *testing.T) {
 
 func TestGenerateBackupJobIntent(t *testing.T) {
 	ctx := context.Background()
+	cluster := v1beta1.PostgresCluster{}
+	cluster.Name = "hippo-test"
+	cluster.Default()
+
+	// The code assumes that if repo.Volume is nil, then it is a cloud repo backup,
+	// therefore, an "empty" input results in a job spec for a cloud repo backup
 	t.Run("empty", func(t *testing.T) {
 		spec, err := generateBackupJobSpecIntent(ctx,
-			&v1beta1.PostgresCluster{}, v1beta1.PGBackRestRepo{},
+			&cluster, v1beta1.PGBackRestRepo{},
+			"",
+			nil, nil,
+		)
+		assert.NilError(t, err)
+		assert.Assert(t, cmp.MarshalMatches(spec.Template.Spec, `
+containers:
+- command:
+  - /bin/pgbackrest
+  - backup
+  - --stanza=db
+  - --repo=
+  name: pgbackrest
+  resources: {}
+  securityContext:
+    allowPrivilegeEscalation: false
+    capabilities:
+      drop:
+      - ALL
+    privileged: false
+    readOnlyRootFilesystem: true
+    runAsNonRoot: true
+    seccompProfile:
+      type: RuntimeDefault
+  volumeMounts:
+  - mountPath: /etc/pgbackrest/conf.d
+    name: pgbackrest-config
+    readOnly: true
+  - mountPath: /tmp
+    name: tmp
+enableServiceLinks: false
+restartPolicy: Never
+securityContext:
+  fsGroup: 26
+  fsGroupChangePolicy: OnRootMismatch
+volumes:
+- name: pgbackrest-config
+  projected:
+    sources:
+    - configMap:
+        items:
+        - key: pgbackrest_cloud.conf
+          path: pgbackrest_cloud.conf
+        name: hippo-test-pgbackrest-config
+    - secret:
+        items:
+        - key: pgbackrest.ca-roots
+          path: ~postgres-operator/tls-ca.crt
+        - key: pgbackrest-client.crt
+          path: ~postgres-operator/client-tls.crt
+        - key: pgbackrest-client.key
+          mode: 384
+          path: ~postgres-operator/client-tls.key
+        name: hippo-test-pgbackrest
+- emptyDir:
+    sizeLimit: 16Mi
+  name: tmp
+		`))
+	})
+
+	t.Run("volumeRepo", func(t *testing.T) {
+		spec, err := generateBackupJobSpecIntent(ctx,
+			&cluster, v1beta1.PGBackRestRepo{
+				Volume: &v1beta1.RepoPVC{
+					VolumeClaimSpec: v1beta1.VolumeClaimSpec{},
+				},
+			},
 			"",
 			nil, nil,
 		)
@@ -2621,10 +2693,10 @@ containers:
   - name: COMPARE_HASH
     value: "true"
   - name: CONTAINER
-    value: database
+    value: pgbackrest
   - name: NAMESPACE
   - name: SELECTOR
-    value: postgres-operator.crunchydata.com/cluster=,postgres-operator.crunchydata.com/instance,postgres-operator.crunchydata.com/role=master
+    value: postgres-operator.crunchydata.com/cluster=hippo-test,postgres-operator.crunchydata.com/pgbackrest=,postgres-operator.crunchydata.com/pgbackrest-dedicated=
   name: pgbackrest
   resources: {}
   securityContext:
@@ -2651,11 +2723,23 @@ volumes:
     sources:
     - configMap:
         items:
-        - key: pgbackrest_instance.conf
-          path: pgbackrest_instance.conf
+        - key: pgbackrest_repo.conf
+          path: pgbackrest_repo.conf
         - key: config-hash
           path: config-hash
-        name: -pgbackrest-config
+        - key: pgbackrest-server.conf
+          path: ~postgres-operator_server.conf
+        name: hippo-test-pgbackrest-config
+    - secret:
+        items:
+        - key: pgbackrest.ca-roots
+          path: ~postgres-operator/tls-ca.crt
+        - key: pgbackrest-client.crt
+          path: ~postgres-operator/client-tls.crt
+        - key: pgbackrest-client.key
+          mode: 384
+          path: ~postgres-operator/client-tls.key
+        name: hippo-test-pgbackrest
 		`))
 	})