diff --git a/internal/pgbackrest/postgres.go b/internal/pgbackrest/postgres.go index a451ab557f..d583672ec4 100644 --- a/internal/pgbackrest/postgres.go +++ b/internal/pgbackrest/postgres.go @@ -82,6 +82,7 @@ func PostgreSQL( if inCluster.CompareVersion("2.9.0") >= 0 { restore = "/opt/crunchy/bin/restore-command-wrapper.sh " + restore } + restoreOverridden := false if inCluster.Spec.Patroni != nil && inCluster.Spec.Patroni.DynamicConfiguration != nil { postgresql, ok := inCluster.Spec.Patroni.DynamicConfiguration["postgresql"].(map[string]any) if ok { @@ -90,11 +91,22 @@ func PostgreSQL( restore_command, ok := params["restore_command"].(string) if ok { restore = restore_command + restoreOverridden = true } } } } - outParameters.Mandatory.Add("restore_command", restore) + + // If backups are disabled, there is no pgBackRest repository to restore WAL + // from. Unlike archive_command, restore_command can't be replaced with a + // no-op placeholder: Postgres treats a zero exit status as "the file was + // placed", so a placeholder would make it think recovery succeeded when it + // didn't. Leave restore_command unset so Postgres relies on streaming + // replication and local WAL only -- unless the user explicitly configured + // their own restore_command. + if backupsEnabled || restoreOverridden { + outParameters.Mandatory.Add("restore_command", restore) + } if inCluster.Spec.Standby != nil && inCluster.Spec.Standby.Enabled && inCluster.Spec.Standby.RepoName != "" { diff --git a/internal/pgbackrest/postgres_test.go b/internal/pgbackrest/postgres_test.go index 7de00d1c49..89346b4f74 100644 --- a/internal/pgbackrest/postgres_test.go +++ b/internal/pgbackrest/postgres_test.go @@ -90,6 +90,59 @@ func TestPostgreSQLParameters(t *testing.T) { }) }) + t.Run("backups disabled", func(t *testing.T) { + cluster := new(v1beta1.PostgresCluster) + parameters := new(postgres.Parameters) + + if cluster.Labels == nil { + cluster.Labels = make(map[string]string) + } + cluster.Labels["pgv2.percona.com/version"] = version.Version() + + // No restore_command override: the key is omitted entirely, since a + // pgBackRest command would fail with no repository configured. + PostgreSQL(cluster, parameters, false) + assert.DeepEqual(t, parameters.Mandatory.AsMap(), map[string]string{ + "archive_mode": "on", + "archive_command": "true", + }) + + // An explicit user override is still respected even with backups disabled. + dynamic := map[string]any{ + "postgresql": map[string]any{ + "parameters": map[string]any{ + "restore_command": "/bin/true", + }, + }, + } + if cluster.Spec.Patroni == nil { + cluster.Spec.Patroni = &v1beta1.PatroniSpec{} + } + cluster.Spec.Patroni.DynamicConfiguration = dynamic + + PostgreSQL(cluster, parameters, false) + assert.DeepEqual(t, parameters.Mandatory.AsMap(), map[string]string{ + "archive_mode": "on", + "archive_command": "true", + "restore_command": "/bin/true", + }) + + // A standby cluster following an external repo needs restore_command + // regardless of this cluster's own backups.enabled setting. + cluster.Spec.Standby = &v1beta1.PostgresStandbySpec{ + Enabled: true, + RepoName: "repo99", + } + cluster.Spec.Patroni.DynamicConfiguration = nil + + PostgreSQL(cluster, parameters, false) + assert.DeepEqual(t, parameters.Mandatory.AsMap(), map[string]string{ + "archive_mode": "on", + "archive_command": "true", + "restore_command": `/opt/crunchy/bin/restore-command-wrapper.sh pgbackrest --stanza=db archive-get %f "%p" --repo=99`, + }) + }) + t.Run("2.8.0< version", func(t *testing.T) { cluster := new(v1beta1.PostgresCluster) parameters := new(postgres.Parameters)