Skip to content

Commit 61b9728

Browse files
cbandytjmoore4
authored andcommitted
Always try "stanza-upgrade" after a failed "stanza-create"
Error messages could be on either stderr or stdout depending on logging options. Error messages and exit codes could change unexpectedly, so use a shell list to run "stanza-upgrade" any time "stanza-create" exits non-zero. Issue: PGO-1558
1 parent 91398e4 commit 61b9728

File tree

3 files changed

+8
-26
lines changed

3 files changed

+8
-26
lines changed

internal/controller/postgrescluster/pgbackrest.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -2678,8 +2678,7 @@ func (r *Reconciler) reconcileStanzaCreate(ctx context.Context,
26782678
}
26792679

26802680
// Always attempt to create pgBackRest stanza first
2681-
configHashMismatch, err := pgbackrest.Executor(exec).StanzaCreateOrUpgrade(ctx, configHash,
2682-
false, postgresCluster)
2681+
configHashMismatch, err := pgbackrest.Executor(exec).StanzaCreateOrUpgrade(ctx, configHash, postgresCluster)
26832682
if err != nil {
26842683
// record and log any errors resulting from running the stanza-create command
26852684
r.Recorder.Event(postgresCluster, corev1.EventTypeWarning, EventUnableToCreateStanzas,

internal/pgbackrest/pgbackrest.go

+4-20
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"context"
1010
"fmt"
1111
"io"
12-
"strings"
1312

1413
"github.com/pkg/errors"
1514

@@ -24,10 +23,6 @@ const (
2423
// errMsgStaleReposWithVolumesConfig is the error message displayed when a volume-backed repo has been
2524
// configured, but the configuration has not yet propagated into the container.
2625
errMsgStaleReposWithVolumesConfig = "postgres operator error: pgBackRest stale volume-backed repo configuration"
27-
28-
// errMsgBackupDbMismatch is the error message returned from pgBackRest when PG versions
29-
// or PG system identifiers do not match between the PG instance and the existing stanza
30-
errMsgBackupDbMismatch = "backup and archive info files exist but do not match the database"
3126
)
3227

3328
// Executor calls "pgbackrest" commands
@@ -46,15 +41,10 @@ type Executor func(
4641
// from running (with a config mismatch indicating that the pgBackRest configuration as stored in
4742
// the cluster's pgBackRest ConfigMap has not yet propagated to the Pod).
4843
func (exec Executor) StanzaCreateOrUpgrade(ctx context.Context, configHash string,
49-
upgrade bool, postgresCluster *v1beta1.PostgresCluster) (bool, error) {
44+
postgresCluster *v1beta1.PostgresCluster) (bool, error) {
5045

5146
var stdout, stderr bytes.Buffer
5247

53-
stanzaCmd := "create"
54-
if upgrade {
55-
stanzaCmd = "upgrade"
56-
}
57-
5848
var reposWithVolumes []v1beta1.PGBackRestRepo
5949
for _, repo := range postgresCluster.Spec.Backups.PGBackRest.Repos {
6050
if repo.Volume != nil {
@@ -83,18 +73,18 @@ func (exec Executor) StanzaCreateOrUpgrade(ctx context.Context, configHash strin
8373
// Otherwise, it runs the pgbackrest command, which will either be "stanza-create" or
8474
// "stanza-upgrade", depending on the value of the boolean "upgrade" parameter.
8575
const script = `
86-
declare -r hash="$1" stanza="$2" hash_msg="$3" vol_msg="$4" cmd="$5" check_repo_cmd="$6"
76+
declare -r hash="$1" stanza="$2" hash_msg="$3" vol_msg="$4" check_repo_cmd="$5"
8777
if [[ "$(< /etc/pgbackrest/conf.d/config-hash)" != "${hash}" ]]; then
8878
printf >&2 "%s" "${hash_msg}"; exit 1;
8979
elif ! bash -c "${check_repo_cmd}"; then
9080
printf >&2 "%s" "${vol_msg}"; exit 1;
9181
else
92-
pgbackrest "${cmd}" --stanza="${stanza}"
82+
pgbackrest stanza-create --stanza="${stanza}" || pgbackrest stanza-upgrade --stanza="${stanza}"
9383
fi
9484
`
9585
if err := exec(ctx, nil, &stdout, &stderr, "bash", "-ceu", "--",
9686
script, "-", configHash, DefaultStanzaName, errMsgConfigHashMismatch, errMsgStaleReposWithVolumesConfig,
97-
fmt.Sprintf("stanza-%s", stanzaCmd), checkRepoCmd); err != nil {
87+
checkRepoCmd); err != nil {
9888

9989
errReturn := stderr.String()
10090

@@ -111,12 +101,6 @@ fi
111101
return true, nil
112102
}
113103

114-
// if the err returned from pgbackrest command is about a version mismatch
115-
// then we should run upgrade rather than create
116-
if strings.Contains(errReturn, errMsgBackupDbMismatch) {
117-
return exec.StanzaCreateOrUpgrade(ctx, configHash, true, postgresCluster)
118-
}
119-
120104
// if none of the above errors, return the err
121105
return false, errors.WithStack(fmt.Errorf("%w: %v", err, errReturn))
122106
}

internal/pgbackrest/pgbackrest_test.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,17 @@ func TestStanzaCreateOrUpgrade(t *testing.T) {
2828
ctx := context.Background()
2929
configHash := "7f5d4d5bdc"
3030
expectedCommand := []string{"bash", "-ceu", "--", `
31-
declare -r hash="$1" stanza="$2" hash_msg="$3" vol_msg="$4" cmd="$5" check_repo_cmd="$6"
31+
declare -r hash="$1" stanza="$2" hash_msg="$3" vol_msg="$4" check_repo_cmd="$5"
3232
if [[ "$(< /etc/pgbackrest/conf.d/config-hash)" != "${hash}" ]]; then
3333
printf >&2 "%s" "${hash_msg}"; exit 1;
3434
elif ! bash -c "${check_repo_cmd}"; then
3535
printf >&2 "%s" "${vol_msg}"; exit 1;
3636
else
37-
pgbackrest "${cmd}" --stanza="${stanza}"
37+
pgbackrest stanza-create --stanza="${stanza}" || pgbackrest stanza-upgrade --stanza="${stanza}"
3838
fi
3939
`,
4040
"-", "7f5d4d5bdc", "db", "postgres operator error: pgBackRest config hash mismatch",
4141
"postgres operator error: pgBackRest stale volume-backed repo configuration",
42-
"stanza-create",
4342
"grep repo1-path /etc/pgbackrest/conf.d/pgbackrest_instance.conf",
4443
}
4544

@@ -84,7 +83,7 @@ fi
8483
},
8584
}
8685

87-
configHashMismatch, err := Executor(stanzaExec).StanzaCreateOrUpgrade(ctx, configHash, false, postgresCluster)
86+
configHashMismatch, err := Executor(stanzaExec).StanzaCreateOrUpgrade(ctx, configHash, postgresCluster)
8887
assert.NilError(t, err)
8988
assert.Assert(t, !configHashMismatch)
9089

0 commit comments

Comments
 (0)