fix(backup): retry the scheduled base backup before failing the night

internal/app/names.go

@@ -553,3 +553,11 @@ func (n Names) BackupRunLock(service string) string {
 func (n Names) BackupVerifyScript(service string) string {
 	return path.Join(n.AppDir(), "backup", "verify-"+service+".sh")
 }
+
+// BackupPushScript is the host-side base backup the scheduled backup unit
+// runs. It lives beside the verify script for the same reason: it drives
+// docker from the host, and it is where the unit's retry loop is written down
+// rather than in a unit file that cannot bound one.
+func (n Names) BackupPushScript(service string) string {
+	return path.Join(n.AppDir(), "backup", "backup-"+service+".sh")
+}

internal/engine/backup_schedule.go

@@ -19,9 +19,10 @@ import (
 // Two timers per protected service, taken straight from the policy rather than
 // invented:
 //
-//   - the backup schedule takes a base backup and then applies retention, in
-//     that order, so the repository is never briefly below the number of
-//     generations the policy promises;
+//   - the backup schedule takes a base backup — retrying it a bounded number
+//     of times, because one refused upload part aborts the whole push — and
+//     then applies retention, in that order, so the repository is never
+//     briefly below the number of generations the policy promises;
 //   - the drill schedule verifies the archived WAL forms an unbroken
 //     chain, which is the check a green backup does not imply.
 //
@@ -100,8 +101,11 @@ func (e *Engine) SyncBackupSchedules(ctx context.Context) error {
 			commands  []string
 		}{
 			{"backup", projection.Policy.Schedule, []string{
-				walgExec(container, "backup-push", app.PgDataPath),
-				// Retention after the new generation exists, never before.
+				"/bin/sh " + n.BackupPushScript(service),
+				// Retention after the new generation exists, never before. A
+				// separate ExecStart, so a push that fails every attempt
+				// stops the unit here and never expires a generation to make
+				// room for one that does not exist.
 				prune,
 			}},
 			{"verify", projection.Policy.Drill.Schedule, []string{
@@ -140,11 +144,19 @@ func (e *Engine) SyncBackupSchedules(ctx context.Context) error {
 	// straight through would be marked successful by systemd over a repository
 	// with holes in it, for as long as nobody looked. The interactive command
 	// judges the same report in Go; this is the unattended half of it.
+	//
+	// The backup unit runs a script for a different reason: a single
+	// ExecStart has no way to try again, and a base backup is the one
+	// scheduled job where giving up on the first fault loses a whole night.
 	for _, service := range backedUpServiceNames(e.Spec) {
 		if err := e.writeServiceFile(ctx, n.BackupVerifyScript(service),
 			[]byte(backupVerifyScript(n.ServiceContainer(service)))); err != nil {
 			return fmt.Errorf("cannot install the archive verification for %s: %w", service, err)
 		}
+		if err := e.writeServiceFile(ctx, n.BackupPushScript(service),
+			[]byte(backupPushScript(n.ServiceContainer(service)))); err != nil {
+			return fmt.Errorf("cannot install the scheduled base backup for %s: %w", service, err)
+		}
 	}
 
 	wantedNames := map[string]bool{}
@@ -223,6 +235,66 @@ func backupVerifyScript(container string) string {
 	}, "\n")
 }
 
+// backupPushRetryWaits is the pause, in seconds, before each further attempt
+// at the scheduled base backup; its length is the number of retries.
+//
+// A base backup streams gigabytes through many multipart uploads, and the
+// whole push aborts on the first part the destination refuses — wal-g treats
+// a lost part as fatal, and its S3 client only retries the errors the SDK
+// classes as transient, which a bare HTTP 400 from an edge proxy is not. Seen
+// on a production host against an S3-compatible destination: three of six
+// nights lost to one part, with the same configuration succeeding on the
+// others. Two more attempts, a minute and then five apart, cover a fault
+// that lasts seconds without hiding one that lasts the night.
+var backupPushRetryWaits = []int{60, 300}
+
+// backupPushScript is the scheduled base backup.
+//
+// It is a script rather than a unit directive because systemd's Restart=
+// re-runs every ExecStart, so a retry of the push would also re-run the
+// retention that follows it, and the start rate limiter bounds restarts by
+// time, not count — a push slower than the limiter's window retries forever.
+// A loop in POSIX sh is bounded by construction and exits with the last
+// attempt's status, so the unit still fails, visibly, when every attempt
+// does. It runs under the unit's flock for its whole span, retries included,
+// so an interactive `ob backup` keeps waiting rather than interleaving.
+func backupPushScript(container string) string {
+	return backupPushScriptWith(walgExec(container, "backup-push", app.PgDataPath), backupPushRetryWaits)
+}
+
+func backupPushScriptWith(command string, waits []int) string {
+	attempts := len(waits) + 1
+	schedule := make([]string, 0, len(waits))
+	for _, wait := range waits {
+		schedule = append(schedule, fmt.Sprint(wait))
+	}
+	// Bounded by the attempt count, not by running out of waits: one wait per
+	// retry is what makes `$1` defined every time the loop reaches `shift`.
+	return strings.Join([]string{
+		"#!/bin/sh",
+		"# Written by Onebox. Edits are overwritten on the next apply.",
+		"set -u",
+		fmt.Sprintf("attempts=%d", attempts),
+		"attempt=0",
+		"set -- " + strings.Join(schedule, " "),
+		"while :; do",
+		"  attempt=$((attempt + 1))",
+		"  " + command,
+		"  status=$?",
+		`  [ "$status" -eq 0 ] && exit 0`,
+		`  if [ "$attempt" -ge "$attempts" ]; then`,
+		`    echo "onebox: base backup failed on attempt $attempt of $attempts (exit $status); giving up" >&2`,
+		`    exit "$status"`,
+		"  fi",
+		`  wait=$1`,
+		"  shift",
+		`  echo "onebox: base backup attempt $attempt of $attempts failed (exit $status); retrying in ${wait}s" >&2`,
+		`  sleep "$wait"`,
+		"done",
+		"",
+	}, "\n")
+}
+
 func walgExec(container string, args ...string) string {
 	parts := []string{"/usr/bin/docker", "exec", "-u", "postgres", container, app.WalgBinary}
 	return strings.Join(append(parts, args...), " ")

internal/engine/schedule_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -965,6 +966,161 @@ func TestBackupServiceUnitRecordsEnvironmentOwnership(t *testing.T) {
 	}
 }
 
+// The scheduled base backup has to survive one refused upload part.
+//
+// wal-g aborts the whole push when the destination refuses a single multipart
+// part, and its S3 client does not retry a bare HTTP 400, so a unit with one
+// ExecStart lost three nights in six on a production host while the same
+// configuration succeeded on the others. The loop is bounded, and when every
+// attempt fails the unit still fails with the last attempt's status — a retry
+// that hid that would be a backup that had silently stopped.
+func TestTheScheduledBaseBackupRetriesBeforeFailingTheUnit(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("the scheduled backup is a POSIX shell script")
+	}
+	dir := t.TempDir()
+	counter := filepath.Join(dir, "attempts")
+	for _, tc := range []struct {
+		name        string
+		succeedFrom int // the attempt that succeeds; 0 means none does
+		wantExit    int
+		wantRuns    int
+		wantStderr  []string
+	}{
+		{"first attempt succeeds", 1, 0, 1, nil},
+		{"second attempt succeeds", 2, 0, 2, []string{"attempt 1 of 3 failed (exit 7); retrying in 0s"}},
+		{"every attempt fails", 0, 7, 3, []string{
+			"attempt 1 of 3 failed (exit 7); retrying in 0s",
+			"attempt 2 of 3 failed (exit 7); retrying in 0s",
+			"failed on attempt 3 of 3 (exit 7); giving up",
+		}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			if err := os.WriteFile(counter, nil, 0o600); err != nil {
+				t.Fatal(err)
+			}
+			// The stand-in for `docker exec ... backup-push`: a program of its
+			// own, as the real one is. Each run appends a line, and it succeeds
+			// once the line count reaches the attempt meant to succeed.
+			push := filepath.Join(dir, "push.sh")
+			if err := os.WriteFile(push, []byte(fmt.Sprintf(
+				"echo run >> %s\n[ %d -gt 0 ] && [ \"$(wc -l < %s | tr -d ' ')\" -ge %d ] || exit 7\n",
+				counter, tc.succeedFrom, counter, tc.succeedFrom)), 0o600); err != nil {
+				t.Fatal(err)
+			}
+			script := filepath.Join(dir, "backup.sh")
+			if err := os.WriteFile(script, []byte(backupPushScriptWith("sh "+push, []int{0, 0})), 0o600); err != nil {
+				t.Fatal(err)
+			}
+			var stderr bytes.Buffer
+			cmd := exec.CommandContext(context.Background(), "sh", script)
+			cmd.Stderr = &stderr
+			err := cmd.Run()
+			exit := 0
+			var exitErr *exec.ExitError
+			if errors.As(err, &exitErr) {
+				exit = exitErr.ExitCode()
+			} else if err != nil {
+				t.Fatal(err)
+			}
+			if exit != tc.wantExit {
+				t.Errorf("exit %d, want %d:\n%s", exit, tc.wantExit, stderr.String())
+			}
+			runs, err := os.ReadFile(counter)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if got := strings.Count(string(runs), "run\n"); got != tc.wantRuns {
+				t.Errorf("the push ran %d times, want %d:\n%s", got, tc.wantRuns, stderr.String())
+			}
+			for _, want := range tc.wantStderr {
+				if !strings.Contains(stderr.String(), want) {
+					t.Errorf("stderr does not say %q:\n%s", want, stderr.String())
+				}
+			}
+		})
+	}
+
+	// The installed script: the real push, the real waits, and shell that does
+	// not bail out of the loop at the first failed attempt.
+	script := backupPushScript("shop-database-1")
+	for _, want := range []string{
+		"/usr/bin/docker exec -u postgres shop-database-1 " + app.WalgBinary + " backup-push " + app.PgDataPath,
+		"attempts=3",
+		"set -- 60 300",
+		`exit "$status"`,
+	} {
+		if !strings.Contains(script, want) {
+			t.Errorf("the scheduled base backup does not contain %q:\n%s", want, script)
+		}
+	}
+	if strings.Contains(script, "set -e") {
+		t.Errorf("set -e exits at the first failed attempt before the loop can retry:\n%s", script)
+	}
+	check := exec.CommandContext(context.Background(), "sh", "-n")
+	check.Stdin = strings.NewReader(script)
+	if output, err := check.CombinedOutput(); err != nil {
+		t.Fatalf("the scheduled base backup is not valid POSIX shell: %v: %s\n%s", err, output, script)
+	}
+}
+
+// The unit retries the push and only the push.
+//
+// Retention stays a separate ExecStart on purpose: systemd stops a oneshot at
+// the first ExecStart that fails, so a push that failed every attempt never
+// reaches `delete retain`. A retry wired into the unit with Restart= would
+// instead re-run every ExecStart, retention included, on each attempt.
+func TestTheScheduledBackupUnitRetriesThePushAndNotTheRetention(t *testing.T) {
+	f := &transport.Fake{Dynamic: func(cmd string) (transport.Result, bool) {
+		if strings.HasSuffix(cmd, "echo ok") {
+			return transport.Result{Stdout: "ok\n"}, true
+		}
+		return transport.Result{}, false
+	}}
+	resolved := protectedPostgresResolved(t, "7513211627332151223")
+	e := New(resolved, nil, f, Options{Out: &bytes.Buffer{}, Sleep: noSleep, Environment: "production"})
+	if err := e.SyncBackupSchedules(context.Background()); err != nil {
+		t.Fatal(err)
+	}
+	n := resolved.Spec.NamesFor("production")
+	var unit, script string
+	for _, body := range f.Inputs {
+		switch {
+		case strings.Contains(body, "Description=Onebox backup backup for postgres"):
+			unit = body
+		case strings.HasPrefix(body, "#!/bin/sh") && strings.Contains(body, "backup-push"):
+			script = body
+		}
+	}
+	if unit == "" {
+		t.Fatalf("no backup unit was written:\n%s", strings.Join(f.Commands, "\n"))
+	}
+	if script == "" || !strings.Contains(strings.Join(f.Commands, "\n"), n.BackupPushScript("postgres")) {
+		t.Fatalf("the retrying base backup was not installed at %s:\n%s", n.BackupPushScript("postgres"), strings.Join(f.Commands, "\n"))
+	}
+	lock := n.BackupRunLock("postgres")
+	push := "ExecStart=/usr/bin/flock -w 3600 " + lock + " /bin/sh " + n.BackupPushScript("postgres")
+	prune := "ExecStart=/usr/bin/flock -w 3600 " + lock + " /usr/bin/docker exec -u postgres " +
+		n.ServiceContainer("postgres") + " " + app.WalgBinary + " delete retain FULL"
+	for _, want := range []string{push, prune} {
+		if !strings.Contains(unit, want) {
+			t.Errorf("the backup unit does not contain %q:\n%s", want, unit)
+		}
+	}
+	if strings.Index(unit, push) > strings.Index(unit, prune) {
+		t.Errorf("retention runs before the push:\n%s", unit)
+	}
+	if strings.Contains(unit, "backup-push") {
+		t.Errorf("the unit runs the push outside the retrying script:\n%s", unit)
+	}
+	if strings.Contains(unit, "Restart=") {
+		t.Errorf("a systemd restart would re-run retention with every attempt:\n%s", unit)
+	}
+	if !strings.Contains(script, "/usr/bin/docker exec -u postgres "+n.ServiceContainer("postgres")+" "+app.WalgBinary+" backup-push "+app.PgDataPath) {
+		t.Errorf("the installed script does not push this service's cluster:\n%s", script)
+	}
+}
+
 func TestScheduledJobRunnersRecordRunStateForTheNotifier(t *testing.T) {
 	names := app.Names{App: "sample", BasePath: "/var/lib/onebox"}
 	for _, tc := range []struct {

site/src/content/docs/guides/back-up-a-database.mdx

@@ -67,6 +67,14 @@ be telling you the database is protected at the moment it is not.
 The restart is a real restart. Enabling is a maintenance action, not a
 configuration change.
 
+The scheduled base backup does not give up on its first fault. A push streams
+gigabytes through many multipart uploads and aborts on the first part the
+destination refuses, so the unit tries up to three times, a minute and then
+five minutes apart, under the same lock an interactive `ob backup` waits on.
+Retention runs only after a push that succeeded. A night on which every attempt
+fails is still a failed unit: `systemctl status onebox-backup-database-backup`
+says so, and the journal has each attempt's reason.
+
 ## The credential file
 
 The encrypted file your target names needs three entries — the two you named,