Create stanza after repohost is added (#3965)

Given a cloud host is already in place, the user should be able to add a repohost.

Author: tony-landreth

.gitignore

@@ -1,3 +1,4 @@
 .DS_Store
 /vendor/
 /testing/kuttl/e2e-generated*/
+gke_gcloud_auth_plugin_cache

internal/controller/postgrescluster/pgbackrest.go

@@ -2595,7 +2595,7 @@ func (r *Reconciler) reconcileStanzaCreate(ctx context.Context,
 
 	// Always attempt to create pgBackRest stanza first
 	configHashMismatch, err := pgbackrest.Executor(exec).StanzaCreateOrUpgrade(ctx, configHash,
-		false)
+		false, postgresCluster)
 	if err != nil {
 		// record and log any errors resulting from running the stanza-create command
 		r.Recorder.Event(postgresCluster, corev1.EventTypeWarning, EventUnableToCreateStanzas,

internal/pgbackrest/pgbackrest.go

@@ -23,13 +23,19 @@ import (
 	"strings"
 
 	"github.com/pkg/errors"
+
+	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
 )
 
 const (
 	// errMsgConfigHashMismatch is the error message displayed when a configuration hash mismatch
 	// is detected while attempting stanza creation
 	errMsgConfigHashMismatch = "postgres operator error: pgBackRest config hash mismatch"
 
+	// errMsgStaleReposWithVolumesConfig is the error message displayed when a volume-backed repo has been
+	// configured, but the configuration has not yet propagated into the container.
+	errMsgStaleReposWithVolumesConfig = "postgres operator error: pgBackRest stale volume-backed repo configuration"
+
 	// errMsgBackupDbMismatch is the error message returned from pgBackRest when PG versions
 	// or PG system identifiers do not match between the PG instance and the existing stanza
 	errMsgBackupDbMismatch = "backup and archive info files exist but do not match the database"
@@ -51,7 +57,7 @@ type Executor func(
 // from running (with a config mismatch indicating that the pgBackRest configuration as stored in
 // the cluster's pgBackRest ConfigMap has not yet propagated to the Pod).
 func (exec Executor) StanzaCreateOrUpgrade(ctx context.Context, configHash string,
-	upgrade bool) (bool, error) {
+	upgrade bool, postgresCluster *v1beta1.PostgresCluster) (bool, error) {
 
 	var stdout, stderr bytes.Buffer
 
@@ -60,22 +66,46 @@ func (exec Executor) StanzaCreateOrUpgrade(ctx context.Context, configHash strin
 		stanzaCmd = "upgrade"
 	}
 
+	var reposWithVolumes []v1beta1.PGBackRestRepo
+	for _, repo := range postgresCluster.Spec.Backups.PGBackRest.Repos {
+		if repo.Volume != nil {
+			reposWithVolumes = append(reposWithVolumes, repo)
+		}
+	}
+
+	grep := "grep %s-path /etc/pgbackrest/conf.d/pgbackrest_instance.conf"
+
+	var checkRepoCmd string
+	if len(reposWithVolumes) > 0 {
+		repo := reposWithVolumes[0]
+		checkRepoCmd = checkRepoCmd + fmt.Sprintf(grep, repo.Name)
+
+		reposWithVolumes = reposWithVolumes[1:]
+		for _, repo := range reposWithVolumes {
+			checkRepoCmd = checkRepoCmd + fmt.Sprintf(" && "+grep, repo.Name)
+		}
+	}
+
 	// this is the script that is run to create a stanza.  First it checks the
 	// "config-hash" file to ensure all configuration changes (e.g. from ConfigMaps) have
 	// propagated to the container, and if not, it prints an error and returns with exit code 1).
+	// Next, it checks that any volume-backed repo added to the config has propagated into
+	// the container, and if not, prints an error and exits with code 1.
 	// Otherwise, it runs the pgbackrest command, which will either be "stanza-create" or
 	// "stanza-upgrade", depending on the value of the boolean "upgrade" parameter.
 	const script = `
-declare -r hash="$1" stanza="$2" message="$3" cmd="$4"
+declare -r hash="$1" stanza="$2" hash_msg="$3" vol_msg="$4" cmd="$5" check_repo_cmd="$6"
 if [[ "$(< /etc/pgbackrest/conf.d/config-hash)" != "${hash}" ]]; then
-    printf >&2 "%s" "${message}"; exit 1;
+    printf >&2 "%s" "${hash_msg}"; exit 1;
+elif ! bash -c "${check_repo_cmd}"; then
+ 	 printf >&2 "%s" "${vol_msg}"; exit 1;
 else
     pgbackrest "${cmd}" --stanza="${stanza}"
 fi
 `
 	if err := exec(ctx, nil, &stdout, &stderr, "bash", "-ceu", "--",
-		script, "-", configHash, DefaultStanzaName, errMsgConfigHashMismatch,
-		fmt.Sprintf("stanza-%s", stanzaCmd)); err != nil {
+		script, "-", configHash, DefaultStanzaName, errMsgConfigHashMismatch, errMsgStaleReposWithVolumesConfig,
+		fmt.Sprintf("stanza-%s", stanzaCmd), checkRepoCmd); err != nil {
 
 		errReturn := stderr.String()
 
@@ -86,10 +116,16 @@ fi
 			return true, nil
 		}
 
+		// if the configuration for volume-backed repositories is stale, return true and don't return an error since this
+		// is expected while waiting for config changes in ConfigMaps to make it to the container
+		if errReturn == errMsgStaleReposWithVolumesConfig {
+			return true, nil
+		}
+
 		// if the err returned from pgbackrest command is about a version mismatch
 		// then we should run upgrade rather than create
 		if strings.Contains(errReturn, errMsgBackupDbMismatch) {
-			return exec.StanzaCreateOrUpgrade(ctx, configHash, true)
+			return exec.StanzaCreateOrUpgrade(ctx, configHash, true, postgresCluster)
 		}
 
 		// if none of the above errors, return the err

internal/pgbackrest/pgbackrest_test.go

@@ -24,8 +24,13 @@ import (
 	"testing"
 
 	"gotest.tools/v3/assert"
+	"k8s.io/apimachinery/pkg/api/resource"
+
+	corev1 "k8s.io/api/core/v1"
 
 	"github.com/crunchydata/postgres-operator/internal/testing/require"
+
+	"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
 )
 
 func TestStanzaCreateOrUpgrade(t *testing.T) {
@@ -34,15 +39,20 @@ func TestStanzaCreateOrUpgrade(t *testing.T) {
 	ctx := context.Background()
 	configHash := "7f5d4d5bdc"
 	expectedCommand := []string{"bash", "-ceu", "--", `
-declare -r hash="$1" stanza="$2" message="$3" cmd="$4"
+declare -r hash="$1" stanza="$2" hash_msg="$3" vol_msg="$4" cmd="$5" check_repo_cmd="$6"
 if [[ "$(< /etc/pgbackrest/conf.d/config-hash)" != "${hash}" ]]; then
-    printf >&2 "%s" "${message}"; exit 1;
+    printf >&2 "%s" "${hash_msg}"; exit 1;
+elif ! bash -c "${check_repo_cmd}"; then
+ 	 printf >&2 "%s" "${vol_msg}"; exit 1;
 else
     pgbackrest "${cmd}" --stanza="${stanza}"
 fi
 `,
 		"-", "7f5d4d5bdc", "db", "postgres operator error: pgBackRest config hash mismatch",
-		"stanza-create"}
+		"postgres operator error: pgBackRest stale volume-backed repo configuration",
+		"stanza-create",
+		"grep repo1-path /etc/pgbackrest/conf.d/pgbackrest_instance.conf",
+	}
 
 	var shellCheckScript string
 	stanzaExec := func(ctx context.Context, stdin io.Reader, stdout, stderr io.Writer,
@@ -56,8 +66,36 @@ fi
 
 		return nil
 	}
+	postgresCluster := &v1beta1.PostgresCluster{
+		Spec: v1beta1.PostgresClusterSpec{
+			Backups: v1beta1.Backups{
+				PGBackRest: v1beta1.PGBackRestArchive{
+					Repos: []v1beta1.PGBackRestRepo{{
+						Name: "repo1",
+						Volume: &v1beta1.RepoPVC{
+							VolumeClaimSpec: corev1.PersistentVolumeClaimSpec{
+								AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany},
+								Resources: corev1.VolumeResourceRequirements{
+									Requests: map[corev1.ResourceName]resource.Quantity{
+										corev1.ResourceStorage: resource.MustParse("1Gi"),
+									},
+								},
+							},
+						},
+					}, {
+						Name: "repo2",
+						S3: &v1beta1.RepoS3{
+							Bucket:   "bucket",
+							Endpoint: "endpoint",
+							Region:   "region",
+						},
+					}},
+				},
+			},
+		},
+	}
 
-	configHashMismatch, err := Executor(stanzaExec).StanzaCreateOrUpgrade(ctx, configHash, false)
+	configHashMismatch, err := Executor(stanzaExec).StanzaCreateOrUpgrade(ctx, configHash, false, postgresCluster)
 	assert.NilError(t, err)
 	assert.Assert(t, !configHashMismatch)