Run backup command for cloud-based repos in the backup job.

Add/adjust tests for cloud repo backup job changes.

Author: dsessler7

internal/controller/postgrescluster/instance.go

@@ -1173,7 +1173,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)
@@ -1398,10 +1398,8 @@ func addPGBackRestToInstancePodSpec(
 ctx context.Context, cluster *v1beta1.PostgresCluster,
 instanceCertificates *corev1.Secret, instancePod *corev1.PodSpec,
 ) {
-if pgbackrest.RepoHostVolumeDefined(cluster) {
-pgbackrest.AddServerToInstancePod(ctx, cluster, instancePod,
-instanceCertificates.Name)
-}
+pgbackrest.AddServerToInstancePod(ctx, cluster, instancePod,
+instanceCertificates.Name)
 
 pgbackrest.AddConfigToInstancePod(cluster, instancePod)
 }
@@ -1470,7 +1468,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(
@@ -1513,7 +1511,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

@@ -23,7 +23,6 @@ import (
 "k8s.io/apimachinery/pkg/api/meta"
 metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-"k8s.io/apimachinery/pkg/labels"
 utilerrors "k8s.io/apimachinery/pkg/util/errors"
 "sigs.k8s.io/controller-runtime/pkg/client"
 "sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -774,12 +773,7 @@ func (r *Reconciler) generateRepoVolumeIntent(postgresCluster *v1beta1.PostgresC
 // generateBackupJobSpecIntent generates a JobSpec for a pgBackRest backup job
 func generateBackupJobSpecIntent(ctx context.Context, postgresCluster *v1beta1.PostgresCluster,
 repo v1beta1.PGBackRestRepo, serviceAccountName string,
-labels, annotations map[string]string, opts ...string) (*batchv1.JobSpec, error) {
-
-selector, containerName, err := getPGBackRestExecSelector(postgresCluster, repo)
-if err != nil {
-return nil, errors.WithStack(err)
-}
+labels, annotations map[string]string, opts ...string) *batchv1.JobSpec {
 
 repoIndex := regexRepoIndex.FindString(repo.Name)
 cmdOpts := []string{
@@ -794,21 +788,31 @@ func generateBackupJobSpecIntent(ctx context.Context, postgresCluster *v1beta1.P
 cmdOpts = append(cmdOpts, opts...)
 
 container := corev1.Container{
-Command: []string{"/opt/crunchy/bin/pgbackrest"},
-Env: []corev1.EnvVar{
-{Name: "COMMAND", Value: "backup"},
-{Name: "COMMAND_OPTS", Value: strings.Join(cmdOpts, " ")},
-{Name: "COMPARE_HASH", Value: "true"},
-{Name: "CONTAINER", Value: containerName},
-{Name: "NAMESPACE", Value: postgresCluster.GetNamespace()},
-{Name: "SELECTOR", Value: selector.String()},
-},
 Image: config.PGBackRestContainerImage(postgresCluster),
 ImagePullPolicy: postgresCluster.Spec.ImagePullPolicy,
 Name: naming.PGBackRestRepoContainerName,
 SecurityContext: initialize.RestrictedSecurityContext(),
 }
 
+// If the repo that we are backing up to is a local volume, we will configure
+// the job to use the pgbackrest go binary to exec into the repo host and run
+// the backup. If the repo is a cloud-based repo, we will run the pgbackrest
+// backup command directly in the job pod.
+if repo.Volume != nil {
+container.Command = []string{"/opt/crunchy/bin/pgbackrest"}
+container.Env = []corev1.EnvVar{
+{Name: "COMMAND", Value: "backup"},
+{Name: "COMMAND_OPTS", Value: strings.Join(cmdOpts, " ")},
+{Name: "COMPARE_HASH", Value: "true"},
+{Name: "CONTAINER", Value: naming.PGBackRestRepoContainerName},
+{Name: "NAMESPACE", Value: postgresCluster.GetNamespace()},
+{Name: "SELECTOR", Value: naming.PGBackRestDedicatedSelector(postgresCluster.GetName()).String()},
+}
+} else {
+container.Command = []string{"/bin/pgbackrest", "backup"}
+container.Command = append(container.Command, cmdOpts...)
+}
+
 if postgresCluster.Spec.Backups.PGBackRest.Jobs != nil {
 container.Resources = postgresCluster.Spec.Backups.PGBackRest.Jobs.Resources
 }
@@ -862,13 +866,16 @@ func generateBackupJobSpecIntent(ctx context.Context, postgresCluster *v1beta1.P
 jobSpec.Template.Spec.ImagePullSecrets = postgresCluster.Spec.ImagePullSecrets
 
 // add pgBackRest configs to template
-if containerName == naming.PGBackRestRepoContainerName {
+if repo.Volume != nil {
 pgbackrest.AddConfigToRepoPod(postgresCluster, &jobSpec.Template.Spec)
 } else {
-pgbackrest.AddConfigToInstancePod(postgresCluster, &jobSpec.Template.Spec)
+// If we are doing a cloud repo backup, we need to give pgbackrest proper permissions
+// to read certificate files
+jobSpec.Template.Spec.SecurityContext = postgres.PodSecurityContext(postgresCluster)
+pgbackrest.AddConfigToCloudBackupJob(postgresCluster, &jobSpec.Template)
 }
 
-return jobSpec, nil
+return jobSpec
 }
 
 // +kubebuilder:rbac:groups="",resources="configmaps",verbs={delete,list}
@@ -2027,14 +2034,12 @@ 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,
 instanceNames []string) error {
 
-log := logging.FromContext(ctx).WithValues("reconcileResource", "repoConfig")
-
 backrestConfig, err := pgbackrest.CreatePGBackRestConfigMapIntent(ctx, postgresCluster, repoHostName,
 configHash, serviceName, serviceNamespace, instanceNames)
 if err != nil {
@@ -2048,12 +2053,6 @@ func (r *Reconciler) reconcilePGBackRestConfig(ctx context.Context,
 return errors.WithStack(err)
 }
 
-repoHostConfigured := pgbackrest.RepoHostVolumeDefined(postgresCluster)
-if !repoHostConfigured {
-log.V(1).Info("skipping SSH reconciliation, no repo hosts configured")
-return nil
-}
-
 return nil
 }
 
@@ -2455,11 +2454,8 @@ func (r *Reconciler) reconcileManualBackup(ctx context.Context,
 backupJob.Labels = labels
 backupJob.Annotations = annotations
 
-spec, err := generateBackupJobSpecIntent(ctx, postgresCluster, repo,
+spec := generateBackupJobSpecIntent(ctx, postgresCluster, repo,
 serviceAccount.GetName(), labels, annotations, backupOpts...)
-if err != nil {
-return errors.WithStack(err)
-}
 
 backupJob.Spec = *spec
 
@@ -2547,11 +2543,15 @@ func (r *Reconciler) reconcileReplicaCreateBackup(ctx context.Context,
 replicaRepoReady = (condition.Status == metav1.ConditionTrue)
 }
 
-// get pod name and container name as needed to exec into the proper pod and create
-// the pgBackRest backup
-_, containerName, err := getPGBackRestExecSelector(postgresCluster, replicaCreateRepo)
-if err != nil {
-return errors.WithStack(err)
+// TODO: Since we now only exec into the repo host when backing up to a local volume and
+// run the backup in the job pod when backing up to a cloud-based repo, we should consider
+// using a different value than the container name for the "pgbackrest-config" annotation
+// that we attach to these backups
+var containerName string
+if replicaCreateRepo.Volume != nil {
+containerName = naming.PGBackRestRepoContainerName
+} else {
+containerName = naming.ContainerDatabase
 }
 
 // determine if the dedicated repository host is ready using the repo host ready status
@@ -2603,10 +2603,10 @@ func (r *Reconciler) reconcileReplicaCreateBackup(ctx context.Context,
 }
 }
 
-dedicatedEnabled := pgbackrest.RepoHostVolumeDefined(postgresCluster)
 // return if no job has been created and the replica repo or the dedicated
 // repo host is not ready
-if job == nil && ((dedicatedEnabled && !dedicatedRepoReady) || !replicaRepoReady) {
+if job == nil && ((pgbackrest.RepoHostVolumeDefined(postgresCluster) && !dedicatedRepoReady) ||
+!replicaRepoReady) {
 return nil
 }
 
@@ -2631,11 +2631,8 @@ func (r *Reconciler) reconcileReplicaCreateBackup(ctx context.Context,
 backupJob.Labels = labels
 backupJob.Annotations = annotations
 
-spec, err := generateBackupJobSpecIntent(ctx, postgresCluster, replicaCreateRepo,
+spec := generateBackupJobSpecIntent(ctx, postgresCluster, replicaCreateRepo,
 serviceAccount.GetName(), labels, annotations)
-if err != nil {
-return errors.WithStack(err)
-}
 
 backupJob.Spec = *spec
 
@@ -2817,27 +2814,6 @@ func (r *Reconciler) reconcileStanzaCreate(ctx context.Context,
 return false, nil
 }
 
-// getPGBackRestExecSelector returns a selector and container name that allows the proper
-// Pod (along with a specific container within it) to be found within the Kubernetes
-// cluster as needed to exec into the container and run a pgBackRest command.
-func getPGBackRestExecSelector(postgresCluster *v1beta1.PostgresCluster,
-repo v1beta1.PGBackRestRepo) (labels.Selector, string, error) {
-
-var err error
-var podSelector labels.Selector
-var containerName string
-
-if repo.Volume != nil {
-podSelector = naming.PGBackRestDedicatedSelector(postgresCluster.GetName())
-containerName = naming.PGBackRestRepoContainerName
-} else {
-podSelector, err = naming.AsSelector(naming.ClusterPrimary(postgresCluster.GetName()))
-containerName = naming.ContainerDatabase
-}
-
-return podSelector, containerName, err
-}
-
 // getRepoHostStatus is responsible for returning the pgBackRest status for the
 // provided pgBackRest repository host
 func getRepoHostStatus(repoHost *appsv1.StatefulSet) *v1beta1.RepoHostStatus {
@@ -3082,11 +3058,8 @@ func (r *Reconciler) reconcilePGBackRestCronJob(
 // set backup type (i.e. "full", "diff", "incr")
 backupOpts := []string{"--type=" + backupType}
 
-jobSpec, err := generateBackupJobSpecIntent(ctx, cluster, repo,
+jobSpec := generateBackupJobSpecIntent(ctx, cluster, repo,
 serviceAccount.GetName(), labels, annotations, backupOpts...)
-if err != nil {
-return errors.WithStack(err)
-}
 
 // Suspend cronjobs when shutdown or read-only. Any jobs that have already
 // started will continue.
@@ -3119,7 +3092,7 @@ func (r *Reconciler) reconcilePGBackRestCronJob(
 
 // set metadata
 pgBackRestCronJob.SetGroupVersionKind(batchv1.SchemeGroupVersion.WithKind("CronJob"))
-err = errors.WithStack(r.setControllerReference(cluster, pgBackRestCronJob))
+err := errors.WithStack(r.setControllerReference(cluster, pgBackRestCronJob))
 
 if err == nil {
 err = r.apply(ctx, pgBackRestCronJob)