From 1183a31cd26e6dfc04d5a807ab30234e53304c98 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Fri, 21 Aug 2026 20:33:34 -0700 Subject: [PATCH 1/7] test(e2e): drive the CLI against a real server over SSH MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Docker suite is a good deterministic gate for deployment choreography, but every engine in it is built with transport.NewLocal() — transport.go records that local docker is standing in for the deploy host. So the transport every operator uses had never run end to end, and neither had anything that needs a machine: bootstrap on a host with no container runtime, systemd timers, the host trust store, or what teardown leaves behind. This adds a second tier that makes the substitution nowhere. The test process runs on the workstation and reaches a disposable Lima guest over real SSH as root, driving the compiled binary rather than internal engine APIs, so the approval artifacts, the confirmations and the exit codes are part of what is tested. The guest arrives bare; `ob bootstrap` is on the critical path rather than assumed. Both gates keep their shape: OB_E2E=1 for the Docker suite, OB_SERVER_E2E=1 plus OB_E2E_SERVER for this one. The server cases compile on every PR and skip when there is no machine, so they cannot rot unnoticed — a build tag would have hidden them from the fast job entirely. TestServerLifecycle walks bootstrap, plan, approve, deploy, status, exec, logs, audit, a second release, rollback, service apply, secrets list, job plan and run, doctor, the eight backup subcommands and destroy, on one machine in order: 17 of 28 commands, from none. TestServerProbes asks what is left behind when things go wrong — a failed enablement, a repeated one, a redeployed database, teardown with and without volumes. Two of those found bugs on their first run (#91, #92). The guest is pinned by digest per architecture, so a Mac boots aarch64 and CI boots x86_64 with no emulation and no floating image. There is no shared mount: payloads travel ob's own tar-over-SSH upload, which is the code path under test, and that removes the one place vz and qemu genuinely differ. The object store runs as a systemd unit rather than a container, because ob owns the container runtime on a host it manages and a foreign container would be either flaky or a test that passes for the wrong reason. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DF9dqmoT7gsSk7TfzL2WQS --- .github/workflows/e2e-server.yml | 148 ++++++++ Justfile | 24 ++ e2e/lima.yaml | 75 +++++ e2e/server_harness_test.go | 409 +++++++++++++++++++++++ e2e/server_objectstore_test.go | 107 ++++++ e2e/server_probe_test.go | 272 +++++++++++++++ e2e/server_test.go | 344 +++++++++++++++++++ e2e/testdata/postgres/README.md | 12 + e2e/testdata/postgres/ob.yml.tmpl | 70 ++++ e2e/testdata/postgres/secrets/age.key | 3 + e2e/testdata/postgres/secrets/backup.env | 9 + scripts/lima.sh | 93 ++++++ 12 files changed, 1566 insertions(+) create mode 100644 .github/workflows/e2e-server.yml create mode 100644 e2e/lima.yaml create mode 100644 e2e/server_harness_test.go create mode 100644 e2e/server_objectstore_test.go create mode 100644 e2e/server_probe_test.go create mode 100644 e2e/server_test.go create mode 100644 e2e/testdata/postgres/README.md create mode 100644 e2e/testdata/postgres/ob.yml.tmpl create mode 100644 e2e/testdata/postgres/secrets/age.key create mode 100644 e2e/testdata/postgres/secrets/backup.env create mode 100755 scripts/lima.sh diff --git a/.github/workflows/e2e-server.yml b/.github/workflows/e2e-server.yml new file mode 100644 index 00000000..fbcdcd72 --- /dev/null +++ b/.github/workflows/e2e-server.yml @@ -0,0 +1,148 @@ +# The server end-to-end suite. +# +# It is its own workflow rather than a job in ci.yml because `paths:` filters +# apply to a workflow, not to one job beside unrelated ones: adding it there +# would have gated Check and the Docker suite on the same paths. +name: E2E (Server) + +on: + push: + branches: [main] + paths: + - "internal/engine/**" + - "internal/transport/**" + - "internal/onebox/**" + - "internal/app/backup_*" + - "cmd/ob/**" + - "e2e/**" + - "scripts/lima.sh" + - ".github/workflows/e2e-server.yml" + pull_request: + paths: + - "internal/engine/**" + - "internal/transport/**" + - "internal/onebox/**" + - "internal/app/backup_*" + - "cmd/ob/**" + - "e2e/**" + - "scripts/lima.sh" + - ".github/workflows/e2e-server.yml" + # Nightly, because most of what this suite depends on is outside the + # repository and rots on its own schedule: the cloud image, the wal-g + # release, the postgres tag, and the certificates in between. + schedule: + - cron: "0 4 * * *" + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: e2e-server-${{ github.event.pull_request.number || github.sha }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +jobs: + server: + name: End-to-end (Server) + runs-on: ubuntu-24.04 + timeout-minutes: 45 + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 + with: + go-version-file: go.mod + cache-dependency-path: go.sum + + - name: Enable KVM for the runner + run: | + set -euo pipefail + echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", OPTIONS+="static_node=kvm"' \ + | sudo tee /etc/udev/rules.d/99-kvm4all.rules + sudo udevadm control --reload-rules + sudo udevadm trigger --name-match=kvm + sudo apt-get update -qq + sudo apt-get install -y -qq qemu-system-x86 + # Without KVM, qemu falls back to software emulation and this job + # takes tens of minutes instead of a few. That is a finding about the + # runner, not a reason to wait: fail here rather than let a job that + # silently degraded read as a slow but healthy one. + test -e /dev/kvm + + - name: Install Lima + run: | + set -euo pipefail + version=2.2.0 + archive=lima-${version}-Linux-x86_64.tar.gz + sha=a0ea1ccf6b7335a900adb5f8d2b8384457965fecb1ba72f09b4e3e46d12f424a + # Pinned and checksummed for the same reason `just` is in ci.yml: + # what runs is exactly the artifact this line names, or the job stops. + # `releases/latest` would resolve at run time, which is the failure + # mode that pinning exists to remove. + curl -fsSL --retry 3 -o "$archive" \ + "https://github.com/lima-vm/lima/releases/download/v${version}/${archive}" + echo "${sha} ${archive}" | sha256sum --check --strict - + sudo tar Cxzf /usr/local "$archive" + rm -f "$archive" + limactl --version + + - name: Install sops + run: | + set -euo pipefail + version=3.13.3 + sha=e5bec3346a873ae91d871550f3e698c1aad962aff462a080e40f25fde17fef6b + # The fixture's backup credentials are sops-encrypted, and ob shells + # out to `sops -d` to read them: without this the suite fails at the + # credential check rather than at anything it means to test. + curl -fsSL --retry 3 -o sops \ + "https://github.com/getsops/sops/releases/download/v${version}/sops-v${version}.linux.amd64" + echo "${sha} sops" | sha256sum --check --strict - + sudo install -m 0755 sops /usr/local/bin/sops + rm -f sops + sops --version --disable-version-check + + # The guest image is ~600MB and is pinned by digest in e2e/lima.yaml, so + # the cache key can be the file itself: a new image means a new key. + - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ~/.cache/lima + key: lima-image-${{ hashFiles('e2e/lima.yaml') }} + + - name: Boot the server + run: timeout 300 bash scripts/lima.sh up + + - name: Run the server end-to-end suite + run: bash scripts/lima.sh test + + # Curated diagnostics only. + # + # /var/lib/ob holds the mode-0600 backup credential file and decrypted + # environment files, so archiving that tree would publish the repository + # key and the object-store keys as a downloadable artifact on a public + # run. Names and permissions answer the questions a failure asks; + # contents answer questions nobody asked. + - name: Collect diagnostics + if: always() + run: | + set -uo pipefail + mkdir -p diagnostics + ssh_guest() { + limactl shell onebox-e2e -- sudo "$@" 2>&1 || true + } + ssh_guest systemctl list-units --type=timer --all --no-pager > diagnostics/timers.txt + ssh_guest journalctl -u 'ob-backup-*' --no-pager --lines=500 > diagnostics/journal.txt + ssh_guest docker ps -a > diagnostics/containers.txt + ssh_guest docker logs --tail=200 minio > diagnostics/minio.txt + # Layout and permissions, never contents. + ssh_guest find /var/lib/ob -maxdepth 4 -printf '%M %u:%g %10s %p\n' > diagnostics/ob-tree.txt + cp ~/.lima/onebox-e2e/serial.log diagnostics/ 2>/dev/null || true + cp ~/.lima/onebox-e2e/ha.stderr.log diagnostics/ 2>/dev/null || true + + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: always() + with: + name: server-diagnostics + path: diagnostics/ + retention-days: 7 diff --git a/Justfile b/Justfile index da1b8663..bdad817f 100644 --- a/Justfile +++ b/Justfile @@ -157,6 +157,30 @@ workflow-check: e2e: OB_E2E=1 go test ./e2e/ -count=1 -timeout 20m +# Boot the throwaway server the `server-e2e` suite deploys to. +# +# Separate from the suite because the guest outlives a test run: booting takes +# about a minute, and iterating on a failing case should not pay for it again. +lima-up: + bash scripts/lima.sh up + +lima-down: + bash scripts/lima.sh down + +# The server end-to-end suite: the tests run here and reach a real machine over +# SSH, which is the transport every operator uses and the one the Docker suite +# substitutes local docker for. +# +# It boots the guest first rather than failing on a missing one, because the +# common case is not having booted it, and `lima-up` reuses a running instance. +server-e2e: lima-up + bash scripts/lima.sh test + +# Print the connection the suite uses, for running a single case by hand: +# eval "$(just server-env | sed 's/^/export /')" +server-env: + @bash scripts/lima.sh env + # Regenerate the parts of the documentation site that are derived from Go. # # The project-file field reference, the error-code catalogue and the CLI diff --git a/e2e/lima.yaml b/e2e/lima.yaml new file mode 100644 index 00000000..5d37b51e --- /dev/null +++ b/e2e/lima.yaml @@ -0,0 +1,75 @@ +# The machine the server suite deploys to. +# +# It stands in for the box an operator rents: a bare Ubuntu with sshd and +# nothing else. Nothing is provisioned here beyond root's key, because what +# ob does to an unprepared machine is the thing under test — a guest that +# arrives with a container runtime already installed would answer the +# bootstrap contract by assumption instead of by observation. +# +# Lima chooses the hypervisor: vz on Apple Silicon, qemu on the CI runner. +# vmType is deliberately unset, and rosetta deliberately absent — nothing in +# this repository is amd64-only (internal/app/backup_walg.go pins wal-g for +# both architectures and postgres:18 is multi-arch), so translating x86_64 +# binaries on an arm64 guest would only make the Mac run something CI never +# runs. +minimumLimaVersion: 2.2.0 + +# Pinned to an immutable dated release, by digest, per architecture. The +# undated `release/` directory is a moving symlink: pinning to it would let +# the guest change under a run that changed nothing, and floating URLs rot on +# each architecture independently, which breaks exactly one of the two +# environments this suite exists to keep identical. +images: + - location: "https://cloud-images.ubuntu.com/releases/noble/release-20260814/ubuntu-24.04-server-cloudimg-amd64.img" + arch: "x86_64" + digest: "sha256:6e40c07ae715f744f84af0bec76415cc1987dd115b4b8de437818561f01a3733" + - location: "https://cloud-images.ubuntu.com/releases/noble/release-20260814/ubuntu-24.04-server-cloudimg-arm64.img" + arch: "aarch64" + digest: "sha256:4a281a921b8d7db952895ab619736f10efe9f63e111fa5b5779ed18f023818aa" + +cpus: 2 +memory: "4GiB" +disk: "20GiB" + +# Lima installs containerd by default. A guest that arrives with a container +# runtime cannot test the contract ce7e41b established, which is that Docker +# is the operator's to provide through the bootstrap hook. +containerd: + system: false + user: false + +# No shared filesystem, for two reasons. It is the one place the two +# hypervisors genuinely differ — virtiofs under vz, 9p under qemu — and +# removing it removes that divergence from the harness. And ob's own +# tar-over-SSH upload is what should be carrying payloads to a server: it is +# the code path under test, and a mount would route around it. +mounts: [] + +# Only Lima's generated key reaches the guest. The developer's personal keys +# are irrelevant to a throwaway machine and would make the harness behave +# differently on a laptop than on a runner. +ssh: + loadDotSSHPubKeys: false + +provision: + # ob connects as root and never elevates: there is no `sudo` anywhere in + # internal/engine, internal/transport or internal/onebox, and it writes unit + # files directly under /etc/systemd/system. A guest reachable only as an + # unprivileged user would fail at the first schedule sync. + # + # Overwriting rather than appending is deliberate: Ubuntu's cloud images + # install a forced-command in root's authorized_keys that prints a refusal + # and exits. + - mode: system + script: | + #!/bin/sh + set -eu + install -d -m 0700 /root/.ssh + cat /home/*/.ssh/authorized_keys > /root/.ssh/authorized_keys + chmod 0600 /root/.ssh/authorized_keys + # A bare machine is the premise of this guest. If an image ever ships + # one, fail here rather than let the bootstrap tests pass vacuously. + if command -v docker >/dev/null 2>&1; then + echo "guest arrived with docker installed; bootstrap coverage would be vacuous" >&2 + exit 1 + fi diff --git a/e2e/server_harness_test.go b/e2e/server_harness_test.go new file mode 100644 index 00000000..f85e7a95 --- /dev/null +++ b/e2e/server_harness_test.go @@ -0,0 +1,409 @@ +// The server end-to-end harness. +// +// These tests run here and reach a real machine over SSH. That is the shape +// the product actually has — ob runs on a workstation and the box is somewhere +// else — and internal/transport/transport.go records that the Docker suite +// deliberately substitutes local docker for that box. This is the one harness +// that does not make the substitution, so it is the only place the SSH +// transport, systemd timers, a bare machine, and the host trust store are +// exercised at all. +// +// The machine is supplied by `just lima-up`; anything reachable as root over +// SSH will do, including a rented one. +package e2e + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "math/big" + "net" + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" + "sync" + "testing" + "text/template" + "time" +) + +type server struct { + target string // user@host[:port], the same shape ob.yml's `server:` takes + user string + host string + port string + key string // private key that reaches it + guest string // the guest's own address, as a container on it sees the host + + // First contact happens once per server, like it does for an operator. + bootstrapped map[string]bool +} + +func requireServer(t *testing.T) *server { + t.Helper() + if os.Getenv("OB_SERVER_E2E") != "1" { + t.Skip("set OB_SERVER_E2E=1 (see `just server-e2e`)") + } + // Opting in is a promise the machine is there. Skipping past an + // unreachable one turns a gate into a green tick for work nobody did. + target := os.Getenv("OB_E2E_SERVER") + key := os.Getenv("OB_E2E_SERVER_KEY") + if target == "" || key == "" { + t.Fatal("OB_SERVER_E2E=1 without OB_E2E_SERVER and OB_E2E_SERVER_KEY") + } + user, rest, ok := strings.Cut(target, "@") + if !ok { + t.Fatalf("OB_E2E_SERVER %q is not user@host[:port]", target) + } + host, port, ok := strings.Cut(rest, ":") + if !ok { + port = "22" + } + // ob never elevates — there is no sudo anywhere in internal/engine, + // internal/transport or internal/onebox, and it writes unit files directly + // under /etc/systemd/system. A server it cannot reach as root fails later, + // in a place that looks like a deploy bug. + if user != "root" { + t.Fatalf("OB_E2E_SERVER is %q; ob writes to /etc/systemd/system and does not elevate, so it must be root", target) + } + s := &server{target: target, user: user, host: host, port: port, key: key, + bootstrapped: map[string]bool{}} + if err := s.try("true"); err != nil { + t.Fatalf("OB_SERVER_E2E=1 but %s is not reachable: %v", target, err) + } + s.guest = strings.Fields(s.run(t, "hostname -I"))[0] + return s +} + +// run executes a command on the server and fails the test if it does not +// succeed. This is fixture plumbing, not the transport under test, so it uses +// the system ssh client directly. +func (s *server) run(t *testing.T, command string) string { + t.Helper() + out, err := s.output(command) + if err != nil { + t.Fatalf("server command failed: %s\n%v\n%s", command, err, out) + } + return out +} + +func (s *server) try(command string) error { + _, err := s.output(command) + return err +} + +func (s *server) output(command string) (string, error) { + cmd := exec.Command("ssh", + "-i", s.key, "-p", s.port, + "-o", "BatchMode=yes", + "-o", "StrictHostKeyChecking=no", + "-o", "UserKnownHostsFile=/dev/null", + "-o", "LogLevel=ERROR", + "-o", "ConnectTimeout=10", + s.user+"@"+s.host, command) + out, err := cmd.CombinedOutput() + return string(out), err +} + +// write places a file on the server without an intermediate shell quoting +// problem: the body travels on stdin. +func (s *server) write(t *testing.T, path string, mode string, body []byte) { + t.Helper() + cmd := exec.Command("ssh", + "-i", s.key, "-p", s.port, + "-o", "BatchMode=yes", + "-o", "StrictHostKeyChecking=no", + "-o", "UserKnownHostsFile=/dev/null", + "-o", "LogLevel=ERROR", + s.user+"@"+s.host, + "install -D -m "+mode+" /dev/stdin "+path) + cmd.Stdin = strings.NewReader(string(body)) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("writing %s: %v\n%s", path, err, out) + } +} + +var ( + obOnce sync.Once + obPath string + obErr error +) + +// obBinary builds the real CLI once per run. +// +// The acceptance path is the binary, not the engine: `ob backup enable` is +// orchestrated in internal/onebox above the engine, and driving the engine +// directly would skip the part where a failed enablement has to put the +// service back. +func obBinary(t *testing.T) string { + t.Helper() + obOnce.Do(func() { + dir, err := os.MkdirTemp("", "ob-e2e-bin") + if err != nil { + obErr = err + return + } + obPath = filepath.Join(dir, "ob") + build := exec.Command("go", "build", "-o", obPath, "./cmd/ob") + build.Dir = repoRoot(t) + if out, err := build.CombinedOutput(); err != nil { + obErr = fmt.Errorf("building ob: %v\n%s", err, out) + } + }) + if obErr != nil { + t.Fatal(obErr) + } + return obPath +} + +func repoRoot(t *testing.T) string { + t.Helper() + root, err := filepath.Abs("..") + if err != nil { + t.Fatal(err) + } + return root +} + +// project renders the fixture against this server and returns its directory. +// +// The version is what the workload serves, so a second render into the same +// directory is a second release of the same project — which is what rollback +// and resume need in order to have something to move between. +func (s *server) project(t *testing.T, endpoint, version string) string { + t.Helper() + dir := t.TempDir() + s.render(t, dir, endpoint, version) + return dir +} + +// render writes ob.yml into an existing project directory, replacing what is +// there. Used to produce the next version without disturbing the credentials +// or the artifacts already beside it. +func (s *server) render(t *testing.T, dir, endpoint, version string) { + t.Helper() + tmpl, err := template.ParseFiles(filepath.Join("testdata", "postgres", "ob.yml.tmpl")) + if err != nil { + t.Fatal(err) + } + out, err := os.Create(filepath.Join(dir, "ob.yml")) + if err != nil { + t.Fatal(err) + } + defer out.Close() + if err := tmpl.Execute(out, struct{ Server, Endpoint, Version string }{s.target, endpoint, version}); err != nil { + t.Fatal(err) + } + out.Close() + // The credential file is resolved relative to the project file, so it + // travels with it. + secrets := filepath.Join(dir, "secrets") + if err := os.MkdirAll(secrets, 0o755); err != nil { + t.Fatal(err) + } + for _, name := range []string{"backup.env", "age.key"} { + body, err := os.ReadFile(filepath.Join("testdata", "postgres", "secrets", name)) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(secrets, name), body, 0o600); err != nil { + t.Fatal(err) + } + } +} + +// obHome is the HOME ob runs with: the identity that reaches the server, and a +// known_hosts holding its key. +// +// ob refuses an unknown host by design, so pinning it here is what lets the +// happy path run at all — and it means the absent and mismatched cases are +// their own tests rather than an accident of the environment. +func (s *server) obHome(t *testing.T) string { + t.Helper() + home := t.TempDir() + ssh := filepath.Join(home, ".ssh") + if err := os.MkdirAll(ssh, 0o700); err != nil { + t.Fatal(err) + } + key, err := os.ReadFile(s.key) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(ssh, "id_ed25519"), key, 0o600); err != nil { + t.Fatal(err) + } + scan := exec.Command("ssh-keyscan", "-p", s.port, s.host) + scanned, err := scan.Output() + if err != nil { + t.Fatalf("ssh-keyscan %s:%s: %v", s.host, s.port, err) + } + if err := os.WriteFile(filepath.Join(ssh, "known_hosts"), scanned, 0o600); err != nil { + t.Fatal(err) + } + return home +} + +// ob runs the CLI against this server and returns its combined output. +func (s *server) ob(t *testing.T, dir string, args ...string) (string, error) { + t.Helper() + return s.obWithHome(t, dir, s.obHome(t), args...) +} + +// obWithHome is the same, with the HOME chosen by the caller — which is how +// the tests about SSH itself deny ob the identity or the host key. +func (s *server) obWithHome(t *testing.T, dir, home string, args ...string) (string, error) { + t.Helper() + return s.obInput(t, dir, home, "", args...) +} + +// obInput is the same again, with something on standard input — which is the +// only way to answer the confirmations ob asks for deliberately and offers no +// flag to skip. +func (s *server) obInput(t *testing.T, dir, home, stdin string, args ...string) (string, error) { + t.Helper() + cmd := exec.Command(obBinary(t), append([]string{"-c", filepath.Join(dir, "ob.yml")}, args...)...) + cmd.Dir = dir + if stdin != "" { + cmd.Stdin = strings.NewReader(stdin) + } + cmd.Env = append(os.Environ(), + "HOME="+home, + // Nothing may fall back to the developer's agent: these tests are + // about which credentials ob finds, so an agent in the environment + // would make them pass for a reason the CI runner does not share. + "SSH_AUTH_SOCK=", + "SOPS_AGE_KEY_FILE="+filepath.Join(dir, "secrets", "age.key"), + ) + out, err := cmd.CombinedOutput() + t.Logf("ob %s\n%s", strings.Join(args, " "), out) + return string(out), err +} + +func (s *server) mustOb(t *testing.T, dir string, args ...string) string { + t.Helper() + out, err := s.ob(t, dir, args...) + if err != nil { + t.Fatalf("ob %s failed: %v", strings.Join(args, " "), err) + } + return out +} + +// authority is a certificate authority that exists only on this guest. +type authority struct { + certPEM []byte + keyPEM []byte + caPEM []byte +} + +// newAuthority issues a CA and a server certificate for an IP address. +// +// A private authority is the point of the exercise: it can only be trusted by +// installing it on the host, which is exactly the arrangement that fails when +// nothing carries the host's trust store into the container wal-g runs in. +func newAuthority(t *testing.T, ip string) authority { + t.Helper() + caKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + caTemplate := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "onebox e2e authority"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + IsCA: true, + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature, + BasicConstraintsValid: true, + } + caDER, err := x509.CreateCertificate(rand.Reader, caTemplate, caTemplate, &caKey.PublicKey, caKey) + if err != nil { + t.Fatal(err) + } + caCert, err := x509.ParseCertificate(caDER) + if err != nil { + t.Fatal(err) + } + + leafKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + leaf := &x509.Certificate{ + SerialNumber: big.NewInt(2), + Subject: pkix.Name{CommonName: ip}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + IPAddresses: []net.IP{net.ParseIP(ip)}, + } + leafDER, err := x509.CreateCertificate(rand.Reader, leaf, caCert, &leafKey.PublicKey, caKey) + if err != nil { + t.Fatal(err) + } + leafKeyDER, err := x509.MarshalECPrivateKey(leafKey) + if err != nil { + t.Fatal(err) + } + return authority{ + certPEM: pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: leafDER}), + keyPEM: pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: leafKeyDER}), + caPEM: pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: caDER}), + } +} + +// deploy runs the release the way the product documents it for a plan that has +// never been approved: plan, approve, apply. +// +// ob refuses a bare `deploy` against a host that has not seen this exact plan +// — "one_time approval is required for this exact deployment plan" — so a test +// that only called deploy would be testing the gate, not the release. Going +// through the artifacts also puts `plan` and `approve` under acceptance +// coverage, which they have never had. +func (s *server) deploy(t *testing.T, dir string) { + t.Helper() + // First contact. ob refuses to deploy to a machine that has no owner + // record for this application — "host has no Onebox application owner" — + // so bootstrap is part of the path, not a precondition a test may assume. + if app := filepath.Base(dir); !s.bootstrapped[app] { + s.mustOb(t, dir, "bootstrap") + s.bootstrapped[app] = true + } + + plan := filepath.Join(dir, "ob-plan.json") + approval := filepath.Join(dir, "ob-approval.json") + s.mustOb(t, dir, "plan", "-o", plan) + + // `approve` records a *human* confirmation and deliberately has no flag to + // skip it, so the answer arrives on standard input. A strong approval wants + // the release ID typed back; anything weaker is a yes-or-no. + body, err := os.ReadFile(plan) + if err != nil { + t.Fatal(err) + } + answer := "y\n" + if strings.Contains(string(body), `"approval":"strong"`) || + strings.Contains(string(body), `"approval": "strong"`) { + match := releaseIDRe.FindSubmatch(body) + if match == nil { + t.Fatalf("plan wants a strong approval but names no release id:\n%s", body) + } + answer = string(match[1]) + "\n" + } + out, err := s.obInput(t, dir, s.obHome(t), answer, "approve", "--plan", plan, "-o", approval) + if err != nil { + t.Fatalf("ob approve failed: %v\n%s", err, out) + } + s.mustOb(t, dir, "deploy", "--plan", plan, "--approval", approval, "-y") +} + +// releaseIDRe pulls the release the plan is bound to out of the artifact. The +// plan is JSON with a stable field name; a full decode would need the internal +// types, which the acceptance path deliberately does not import. +var releaseIDRe = regexp.MustCompile(`"release_id"\s*:\s*"([^"]+)"`) diff --git a/e2e/server_objectstore_test.go b/e2e/server_objectstore_test.go new file mode 100644 index 00000000..d00547ed --- /dev/null +++ b/e2e/server_objectstore_test.go @@ -0,0 +1,107 @@ +package e2e + +import ( + "fmt" + "strings" + "testing" + "time" +) + +// The object store the backup target points at. +// +// It runs from its own binary under its own systemd unit, deliberately not as +// a container. ob owns the container runtime on a server it manages — it +// creates and prunes networks by ownership, and `ob destroy` is meant to leave +// the machine clean — so a foreign container in that daemon is either flaky or +// a test that passes for the wrong reason. Outside it, the object store +// survives everything the suite does to ob's side of the machine. +const ( + minioRelease = "RELEASE.2025-09-07T16-13-09Z" + minioAMD64SHA = "7c5bd8512c6e966455b1d198209358b2d191c77a83ab377c4073281065fb855f" + minioARM64SHA = "5c83cd2cf151717ba0243f73e1c7802ff36e272b67144bdd7f1f7d684fd6f03d" + minioPort = "9443" + minioBucket = "observer-backups" + minioAccessKey = "obe2eaccesskey" + minioSecretKey = "obe2esecretkey0123456789" +) + +// objectStore starts MinIO over HTTPS with a certificate signed by an +// authority that exists only on this server, and installs that authority into +// the server's trust store. +// +// That arrangement is the whole point. Nothing publicly trusted signs this +// endpoint, so wal-g can only reach it if ob carries the host's trust store +// into the container — which is what issue #88 found it does not do. +func (s *server) objectStore(t *testing.T) string { + t.Helper() + ca := newAuthority(t, s.guest) + + s.write(t, "/etc/minio/public.crt", "0644", ca.certPEM) + s.write(t, "/etc/minio/private.key", "0600", ca.keyPEM) + // The trust store the container must inherit. update-ca-certificates + // rebuilds /etc/ssl/certs/ca-certificates.crt, which is the bundle ob + // stages beside the wal-g binary. + s.write(t, "/usr/local/share/ca-certificates/onebox-e2e.crt", "0644", ca.caPEM) + s.run(t, "update-ca-certificates >/dev/null 2>&1") + + arch, sha := "amd64", minioAMD64SHA + if machine := strings.TrimSpace(s.run(t, "uname -m")); machine == "aarch64" || machine == "arm64" { + arch, sha = "arm64", minioARM64SHA + } + // Pinned and checksummed rather than resolved at run time, for the same + // reason the wal-g binary is: what runs is the artifact this line names, or + // the setup stops. + url := fmt.Sprintf("https://github.com/minio/minio/releases/download/%s/minio.linux-%s.%s", + minioRelease, arch, minioRelease) + s.run(t, strings.Join([]string{ + "set -e", + "if ! sha256sum /usr/local/bin/minio 2>/dev/null | grep -q " + sha + "; then", + " curl -fsSL --retry 3 -o /tmp/minio " + url, + " echo '" + sha + " /tmp/minio' | sha256sum --check --strict -", + " install -m 0755 /tmp/minio /usr/local/bin/minio", + " rm -f /tmp/minio", + "fi", + // A bucket the target declares as already existing. With the + // filesystem backend a bucket is a directory. + "mkdir -p /var/lib/minio/" + minioBucket, + }, "\n")) + + s.write(t, "/etc/systemd/system/minio.service", "0644", []byte(`[Unit] +Description=Object store for the onebox server end-to-end suite +After=network-online.target + +[Service] +Environment=MINIO_ROOT_USER=`+minioAccessKey+` +Environment=MINIO_ROOT_PASSWORD=`+minioSecretKey+` +ExecStart=/usr/local/bin/minio server /var/lib/minio \ + --address :`+minioPort+` \ + --certs-dir /etc/minio-certs +Restart=on-failure + +[Install] +WantedBy=multi-user.target +`)) + // MinIO expects public.crt and private.key inside its certs directory. + s.run(t, "mkdir -p /etc/minio-certs && cp /etc/minio/public.crt /etc/minio/private.key /etc/minio-certs/") + s.run(t, "systemctl daemon-reload && systemctl enable --now minio") + + endpoint := "https://" + s.guest + ":" + minioPort + deadline := time.Now().Add(60 * time.Second) + for { + // Verified against the freshly installed authority, so this also + // proves the trust store was actually rebuilt. + if err := s.try("curl -fsS --max-time 5 " + endpoint + "/minio/health/live"); err == nil { + break + } + if time.Now().After(deadline) { + t.Fatalf("object store never became ready at %s:\n%s", endpoint, + s.run(t, "journalctl -u minio --no-pager --lines=40 || true")) + } + time.Sleep(time.Second) + } + // Deliberately left running when the test ends. The guest is disposable, + // and stopping the repository as a cleanup destroys the evidence for + // whatever failed — a wal-push run afterwards reports "connection refused" + // and looks like the bug, when it is only the teardown. + return endpoint +} diff --git a/e2e/server_probe_test.go b/e2e/server_probe_test.go new file mode 100644 index 00000000..75053f3d --- /dev/null +++ b/e2e/server_probe_test.go @@ -0,0 +1,272 @@ +package e2e + +import ( + "os" + "strings" + "testing" + "time" +) + +// Adversarial probes. +// +// The lifecycle suite walks the path an operator takes when everything works. +// These take the same machine and ask what ob leaves behind when it is +// interrupted, repeated, or pointed at something that is not there — which is +// where a tool that manages one box tends to be wrong, because the state it +// leaves is on a machine no unit test can see. +func TestServerProbes(t *testing.T) { + s := requireServer(t) + s.requireDocker(t) + s.primeDriverImage(t) + endpoint := s.objectStore(t) + + // An enablement that cannot reach the repository must leave the service + // exactly as it found it. The failure path is the one that matters: it runs + // after archiving has been turned on, and a service left archiving into + // nothing retains every WAL segment it cannot ship. + t.Run("an unreachable repository leaves archiving off", func(t *testing.T) { + dir := s.project(t, endpoint, "v1") + s.deploy(t, dir) + + s.run(t, "systemctl stop minio") + defer s.run(t, "systemctl start minio") + + out, err := s.ob(t, dir, "backup", "enable", "postgres") + if err == nil { + t.Fatalf("enable succeeded against a repository that is not running:\n%s", out) + } + if mode := s.psql(t, "show archive_mode"); mode == "on" { + t.Error("the service is archiving into a repository it cannot reach") + } + timers := strings.TrimSpace(s.run(t, + "systemctl list-units --type=timer --all --no-pager | grep -c ob-backup || true")) + if timers != "0" { + t.Errorf("a failed enablement left %s backup timer(s) installed", timers) + } + s.teardown(t, dir) + }) + + // The lifecycle suite enables backup on a database that has been running + // for a while. This asks the same question of a cluster that has just been + // created, which is what an operator gets when they enable backup on a + // service they have only now deployed — the far more common case. + t.Run("enable on a fresh cluster archives", func(t *testing.T) { + dir := s.project(t, endpoint, "v1") + s.deploy(t, dir) + s.mustOb(t, dir, "backup", "enable", "postgres") + + before, _ := s.archiverCounts(t) + s.rotateWAL(t) + deadline := time.Now().Add(60 * time.Second) + for { + archived, failed := s.archiverCounts(t) + if archived > before { + break + } + if time.Now().After(deadline) { + t.Fatalf("a freshly created cluster never archived: %d archived, %d failed, last failed %q", + archived, failed, s.psql(t, "select last_failed_wal from pg_stat_archiver")) + } + time.Sleep(2 * time.Second) + } + s.teardown(t, dir) + }) + + // Re-running enable is the documented way to move a service to an edited + // policy, so it has to be safe to run twice. It has been wrong before: an + // earlier version deleted the credential file the same run had installed. + t.Run("enable is safe to repeat", func(t *testing.T) { + dir := s.project(t, endpoint, "v1") + s.deploy(t, dir) + s.mustOb(t, dir, "backup", "enable", "postgres") + s.mustOb(t, dir, "backup", "enable", "postgres") + + // Names.BackupCredentialFile: /backup/secrets/-.env + credentials := strings.TrimSpace(s.run(t, + "ls /var/lib/ob/observer/backup/secrets 2>/dev/null | wc -l")) + if credentials == "0" { + t.Error("re-enabling removed the credential file it had just installed") + } + // Progress, not the cumulative failure counter. pg_stat_archiver counts + // since the last stats reset and survives a restart, so an enablement + // that restarts the server can leave failures in the history without + // anything being wrong now — PostgreSQL retries what it could not ship. + // What must be true is that the repository keeps advancing afterwards. + before, _ := s.archiverCounts(t) + s.rotateWAL(t) + deadline := time.Now().Add(60 * time.Second) + for { + archived, _ := s.archiverCounts(t) + if archived > before { + break + } + if time.Now().After(deadline) { + _, failed := s.archiverCounts(t) + t.Fatalf("nothing reached the repository after a second enable: still %d archived, %d failed", + archived, failed) + } + time.Sleep(2 * time.Second) + } + s.teardown(t, dir) + }) + + // The same repetition, with the archiver drained first. + // + // This distinguishes two explanations for #91. If the collision is between + // `wal-g backup-push` and a segment PostgreSQL has not shipped yet, letting + // the archiver catch up before the second enable removes it. If it survives + // a drained archiver, the two writers collide over the segment being + // written during the backup itself, and the fix has to be elsewhere. + t.Run("enable repeats cleanly once the archiver has drained", func(t *testing.T) { + dir := s.project(t, endpoint, "v1") + s.deploy(t, dir) + s.mustOb(t, dir, "backup", "enable", "postgres") + s.rotateWAL(t) + s.drainArchiver(t) + + s.mustOb(t, dir, "backup", "enable", "postgres") + + before, _ := s.archiverCounts(t) + s.rotateWAL(t) + deadline := time.Now().Add(60 * time.Second) + for { + archived, failed := s.archiverCounts(t) + if archived > before { + break + } + if time.Now().After(deadline) { + t.Fatalf("draining first did not help: %d archived, %d failed, last failed %q", + archived, failed, s.psql(t, "select last_failed_wal from pg_stat_archiver")) + } + time.Sleep(2 * time.Second) + } + s.teardown(t, dir) + }) + + // A new database against a repository that still holds the last one's + // history. + // + // `ob destroy --volumes` removes the cluster; the repository is off-host + // and survives on purpose. Deploying the same application again creates a + // different PostgreSQL cluster that starts numbering its write-ahead log + // from 000000010000000000000001 — the same object names the previous + // cluster already wrote, with different bytes. The repository generation must + // therefore carry the PostgreSQL system identifier: it changes with the data + // volume while the application and service names remain the same. + t.Run("a redeployed database does not collide with the old history", func(t *testing.T) { + first := s.project(t, endpoint, "v1") + s.deploy(t, first) + s.mustOb(t, first, "backup", "enable", "postgres") + s.rotateWAL(t) + s.teardown(t, first) + + second := s.project(t, endpoint, "v1") + s.deploy(t, second) + s.mustOb(t, second, "backup", "enable", "postgres") + + before, _ := s.archiverCounts(t) + s.rotateWAL(t) + deadline := time.Now().Add(60 * time.Second) + for { + archived, failed := s.archiverCounts(t) + if archived > before { + break + } + if time.Now().After(deadline) { + t.Fatalf("the new cluster cannot archive over the old history: %d archived, %d failed, last failed %q", + archived, failed, s.psql(t, "select last_failed_wal from pg_stat_archiver")) + } + time.Sleep(2 * time.Second) + } + s.teardown(t, second) + }) + + // `destroy` is what an operator runs to get the machine back. Containers + // are the visible part; the host state ob installed is the part that gets + // left behind, and a machine that cannot be re-used is a machine that was + // not destroyed. + t.Run("destroy leaves no host state", func(t *testing.T) { + dir := s.project(t, endpoint, "v1") + s.deploy(t, dir) + s.mustOb(t, dir, "backup", "enable", "postgres") + s.teardown(t, dir) + + for _, probe := range []struct{ name, command, want string }{ + {"release tree", "ls -d /var/lib/ob/observer 2>/dev/null | wc -l", "0"}, + {"backup timers", "systemctl list-units --type=timer --all --no-pager | grep -c ob-backup || true", "0"}, + {"unit files", "ls /etc/systemd/system/ob-backup-* 2>/dev/null | wc -l", "0"}, + {"containers", "docker ps -aq --filter label=ob.app=observer | wc -l", "0"}, + {"networks", "docker network ls --filter label=ob.app=observer -q | wc -l", "0"}, + {"volumes", "docker volume ls -q | grep -c observer || true", "0"}, + } { + if got := strings.TrimSpace(s.run(t, probe.command)); got != probe.want { + t.Errorf("destroy left %s behind: %s (want %s)", probe.name, got, probe.want) + } + } + }) + + // Without --volumes the data must survive, because that is the difference + // between tearing an application down and losing the database. + t.Run("destroy without volumes keeps the data", func(t *testing.T) { + dir := s.project(t, endpoint, "v1") + s.deploy(t, dir) + s.psql(t, "create table if not exists kept(note text)") + s.psql(t, "insert into kept values (concat('still', ' ', 'here'))") + + out, err := s.obInput(t, dir, s.obHome(t), "observer\ny\n", "destroy") + if err != nil { + t.Fatalf("destroy failed: %v\n%s", err, out) + } + volumes := strings.TrimSpace(s.run(t, "docker volume ls -q | grep -c observer || true")) + if volumes == "0" { + t.Fatal("destroy without --volumes removed the data volumes") + } + + s.bootstrapped = map[string]bool{} + s.deploy(t, dir) + if note := s.psql(t, "select note from kept limit 1"); note != "still here" { + t.Fatalf("the redeployed service does not hold the row: %q", note) + } + s.teardown(t, dir) + }) +} + +// teardown removes the application and everything it owns, so the next probe +// starts from a machine that has only what a fresh one would. +func (s *server) teardown(t *testing.T, dir string) { + t.Helper() + // Kept deliberately when asked. A probe that fails is a probe whose + // machine is worth looking at, and tearing it down is how the evidence for + // the last three wrong theories disappeared before it could be read. + if os.Getenv("OB_E2E_KEEP") == "1" { + t.Log("OB_E2E_KEEP=1: leaving the application in place") + return + } + if out, err := s.obInput(t, dir, s.obHome(t), "observer\ny\n", "destroy", "--volumes"); err != nil { + t.Fatalf("teardown failed: %v\n%s", err, out) + } + s.bootstrapped = map[string]bool{} +} + +// drainArchiver waits until PostgreSQL has shipped everything it is holding. +// +// A segment is pending while a .ready marker exists beside it in +// archive_status. Waiting for that set to empty is the only way to know the +// archiver and a base backup are not about to write the same object name. +func (s *server) drainArchiver(t *testing.T) { + t.Helper() + container := strings.TrimSpace(s.run(t, + `docker ps -q --filter label=com.docker.compose.service=postgres | head -1`)) + deadline := time.Now().Add(60 * time.Second) + for { + pending := strings.TrimSpace(s.run(t, `docker exec `+container+ + ` sh -c 'ls /var/lib/postgresql/data/pgdata/pg_wal/archive_status/*.ready 2>/dev/null | wc -l'`)) + if pending == "0" { + return + } + if time.Now().After(deadline) { + t.Fatalf("the archiver never drained: %s segment(s) still pending", pending) + } + time.Sleep(2 * time.Second) + } +} diff --git a/e2e/server_test.go b/e2e/server_test.go new file mode 100644 index 00000000..e270791e --- /dev/null +++ b/e2e/server_test.go @@ -0,0 +1,344 @@ +package e2e + +import ( + "encoding/base64" + "path/filepath" + "strconv" + "strings" + "testing" + "time" +) + +// requireDocker puts a container runtime on the server. +// +// ce7e41b made this the operator's job, done through the bootstrap hook, and +// the permutations that belong to that contract — no hook on a bare machine, +// a hook that fails — are their own tests. This is the happy path standing in +// for them so the backup coverage below can run. +func (s *server) requireDocker(t *testing.T) { + t.Helper() + if err := s.try("command -v docker && docker compose version && docker buildx version"); err == nil { + return + } + // Docker's own packages, not Ubuntu's. + // + // `apt install docker.io docker-buildx` looks equivalent and is not: that + // buildx (0.30.1-0ubuntu1) accepts `--format` on `imagetools inspect` and + // ignores it, printing the human-readable manifest and exiting 0. ob pins + // every workload image through exactly that command + // (internal/engine/plan.go), so on such a host no deploy can succeed — and + // the error it produces blames the registry. + // + // This installs what an operator following Docker's instructions gets, + // which is the configuration the product is really used in. The Ubuntu + // packaging is a real gap and belongs in an issue, not in a fixture that + // quietly avoids it. + s.run(t, strings.Join([]string{ + "set -e", + "export DEBIAN_FRONTEND=noninteractive", + "apt-get remove -y -qq docker.io docker-buildx containerd runc >/dev/null 2>&1 || true", + "apt-get update -qq", + "apt-get install -y -qq ca-certificates curl >/dev/null", + "install -m 0755 -d /etc/apt/keyrings", + "curl -fsSL https://download.docker.com/linux/ubuntu/gpg -o /etc/apt/keyrings/docker.asc", + "chmod a+r /etc/apt/keyrings/docker.asc", + `echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/docker.asc] ` + + `https://download.docker.com/linux/ubuntu $(. /etc/os-release && echo "$VERSION_CODENAME") stable" ` + + `> /etc/apt/sources.list.d/docker.list`, + "apt-get update -qq", + "apt-get install -y -qq docker-ce docker-ce-cli containerd.io docker-buildx-plugin docker-compose-plugin >/dev/null", + // A pull-through cache for Docker Hub. + // + // ob resolves a tag through the registry on purpose — a tag can move, + // and internal/engine/backup_postgres.go says so where it skips the + // pull for a digest. But Hub rate-limits anonymous requests per source + // address, and a CI runner shares its address with every other job on + // it, so without a mirror this suite fails with 429s that are nobody's + // bug. Pointing the daemon at a cache is what an operator behind one + // does, and it leaves ob's resolution behaviour untouched. + "mkdir -p /etc/docker", + `printf '{\n "registry-mirrors": ["https://mirror.gcr.io"]\n}\n' > /etc/docker/daemon.json`, + "systemctl enable --now docker.socket docker", + // Restarted explicitly: installing the package already started the + // daemon, so the configuration written a moment ago is not the one it + // is running under until it is told to read it again. + "systemctl restart docker", + "docker info --format '{{json .RegistryConfig.Mirrors}}' | grep -q mirror.gcr.io", + }, "\n")) +} + +// primeDriverImage puts the PostgreSQL image on the server from a mirror. +// +// The driver names `postgres:18` and the tag is not overridable, so every run +// would otherwise pull it from Docker Hub — which rate-limits anonymous +// requests per source address, and a CI runner shares its address with +// everyone else on it. The image content is identical across registries, so +// retagging it locally is the same bytes by the same digest: `docker image +// inspect` reports postgres@sha256:… first, which is what ob reads when it +// pins the protected image. +func (s *server) primeDriverImage(t *testing.T) { + t.Helper() + if err := s.try("docker image inspect postgres:18 >/dev/null 2>&1"); err == nil { + return + } + s.run(t, "docker pull -q public.ecr.aws/docker/library/postgres:18 >/dev/null && "+ + "docker tag public.ecr.aws/docker/library/postgres:18 postgres:18") +} + +// psql runs a query in the protected server and returns the single value. +func (s *server) psql(t *testing.T, query string) string { + t.Helper() + container := strings.TrimSpace(s.run(t, + `docker ps -q --filter label=com.docker.compose.service=postgres | head -1`)) + if container == "" { + t.Fatal("no postgres container is running") + } + // The query travels base64-encoded so that quoting it through ssh, then a + // shell, then docker exec, then psql cannot change what PostgreSQL runs. + // SQL is full of quotes and this test has three shells between it and the + // server. + // + // The superuser and database are ob's, not PostgreSQL's defaults: it + // generates the credentials and passes them to the image, so the container + // itself is the only authority on what they are. + encoded := base64.StdEncoding.EncodeToString([]byte(query)) + return strings.TrimSpace(s.run(t, + `docker exec `+container+` sh -c 'echo `+encoded+` | base64 -d | psql -U "$POSTGRES_USER" -d "$POSTGRES_DB" -tA'`)) +} + +// rotateWAL closes the current segment so there is something to archive. +// +// pg_switch_wal() alone is not enough: PostgreSQL skips the switch when +// nothing has been written since the last one, so on a quiet database it +// returns without producing a segment and a test waiting for the count to move +// waits forever against a perfectly healthy archiver. +func (s *server) rotateWAL(t *testing.T) { + t.Helper() + s.psql(t, "create table if not exists churn(n int)") + s.psql(t, "insert into churn select generate_series(1, 20000)") + s.psql(t, "select pg_switch_wal()") +} + +// archiverCounts reads what the archiver has actually shipped. This is the +// assertion the backup tests turn on: it is a fact about the repository rather +// than about how ob arranged the container, so it stays true whatever shape +// the fix for a broken upload path takes. +func (s *server) archiverCounts(t *testing.T) (archived, failed int) { + t.Helper() + row := s.psql(t, "select archived_count || ' ' || failed_count from pg_stat_archiver") + fields := strings.Fields(row) + if len(fields) != 2 { + t.Fatalf("unreadable pg_stat_archiver row: %q", row) + } + archived, _ = strconv.Atoi(fields[0]) + failed, _ = strconv.Atoi(fields[1]) + return archived, failed +} + +// serves is what the application workload is currently answering with. +// +// Read from inside the container rather than through a published port: the +// fixture declares no proxy, and what matters is which release is running, not +// how the machine exposes it. +func (s *server) serves(t *testing.T) string { + t.Helper() + container := strings.TrimSpace(s.run(t, + `docker ps -q --filter label=com.docker.compose.service=app | head -1`)) + if container == "" { + t.Fatal("no application container is running") + } + return strings.TrimSpace(s.run(t, `docker exec `+container+` wget -qO- http://localhost:8080/`)) +} + +// TestServerLifecycle walks the product the way an operator does: one machine, +// first contact to teardown, in order. +// +// The subtests are steps rather than independent cases, and they share the +// server and the release deliberately — a lifecycle is the thing under test, +// and rebuilding it per case would cost minutes per run and would still not +// prove that the steps compose. Ordering is Go's: subtests run in the order +// they are declared. +func TestServerLifecycle(t *testing.T) { + s := requireServer(t) + s.requireDocker(t) + s.primeDriverImage(t) + endpoint := s.objectStore(t) + dir := s.project(t, endpoint, "v1") + + // preflight changes nothing, so it reports rather than refuses — and what + // it reports about an unclaimed machine is the thing worth asserting: an + // operator runs this before bootstrap precisely to be told what is missing. + t.Run("preflight reports an unclaimed host", func(t *testing.T) { + out := s.mustOb(t, dir, "preflight") + if !strings.Contains(out, "unclaimed") { + t.Fatalf("preflight did not report the host as unclaimed before bootstrap:\n%s", out) + } + }) + + t.Run("bootstrap and first release", func(t *testing.T) { + s.deploy(t, dir) + if body := s.serves(t); body != "v1" { + t.Fatalf("the workload serves %q, want v1", body) + } + }) + + t.Run("preflight", func(t *testing.T) { + s.mustOb(t, dir, "preflight") + }) + + t.Run("status reports the release", func(t *testing.T) { + out := s.mustOb(t, dir, "status") + for _, want := range []string{"app", "postgres"} { + if !strings.Contains(out, want) { + t.Errorf("status never mentions %q:\n%s", want, out) + } + } + }) + + t.Run("exec reaches the workload", func(t *testing.T) { + out := s.mustOb(t, dir, "exec", "app", "--reason", "server e2e", "--", "cat", "/srv/index.html") + if !strings.Contains(out, "v1") { + t.Fatalf("exec did not run in the release:\n%s", out) + } + }) + + t.Run("logs", func(t *testing.T) { + s.mustOb(t, dir, "logs", "app") + }) + + t.Run("audit records the release", func(t *testing.T) { + out := s.mustOb(t, dir, "audit") + if strings.TrimSpace(out) == "" { + t.Fatal("audit is empty after a successful deploy") + } + }) + + t.Run("second release", func(t *testing.T) { + s.render(t, dir, endpoint, "v2") + s.deploy(t, dir) + if body := s.serves(t); body != "v2" { + t.Fatalf("the workload serves %q after the second release, want v2", body) + } + }) + + t.Run("rollback returns the previous release", func(t *testing.T) { + s.mustOb(t, dir, "rollback") + if body := s.serves(t); body != "v1" { + t.Fatalf("the workload serves %q after rollback, want v1", body) + } + }) + + t.Run("service apply converges the data service", func(t *testing.T) { + s.mustOb(t, dir, "service", "apply") + }) + + t.Run("secrets list", func(t *testing.T) { + s.mustOb(t, dir, "secrets", "list") + }) + + t.Run("job plan and run", func(t *testing.T) { + plan := filepath.Join(dir, "ob-job-plan.json") + s.mustOb(t, dir, "job", "plan", "chore", "-o", plan) + out, err := s.obInput(t, dir, s.obHome(t), "y\n", "job", "run", "--plan", plan) + if err != nil { + t.Fatalf("job run failed: %v\n%s", err, out) + } + }) + + t.Run("doctor", func(t *testing.T) { + s.mustOb(t, dir, "doctor") + }) + + // Issue #88: enable established archiving and then failed every upload + // with "x509: certificate signed by unknown authority", because wal-g runs + // inside the driver's image and postgres:18 carries no certificate + // authorities. It failed after the base backup, leaving archiving on. + // + // This endpoint is signed by an authority that exists only on this server, + // so it can only verify if ob carries the host trust store into the + // container the way it already carries the binary. + t.Run("backup enable archives to a privately trusted endpoint", func(t *testing.T) { + s.mustOb(t, dir, "backup", "enable", "postgres") + s.rotateWAL(t) + + deadline := time.Now().Add(90 * time.Second) + for { + archived, failed := s.archiverCounts(t) + if failed > 0 { + t.Fatalf("the archiver is failing (%d archived, %d failed); wal-g cannot reach %s", + archived, failed, endpoint) + } + if archived >= 1 { + return + } + if time.Now().After(deadline) { + t.Fatalf("nothing reached the repository: %d archived, %d failed", archived, failed) + } + time.Sleep(2 * time.Second) + } + }) + + t.Run("backup status reads the repository", func(t *testing.T) { + out := s.mustOb(t, dir, "backup", "status", "postgres") + if strings.TrimSpace(out) == "" { + t.Fatal("backup status says nothing about a protected service") + } + }) + + t.Run("backup create takes another base backup", func(t *testing.T) { + s.mustOb(t, dir, "backup", "create", "postgres") + }) + + t.Run("backup verify proves the chain", func(t *testing.T) { + s.mustOb(t, dir, "backup", "verify", "postgres") + }) + + t.Run("backup drill recovers without touching the database", func(t *testing.T) { + s.mustOb(t, dir, "backup", "drill", "postgres") + if body := s.serves(t); body != "v1" { + t.Fatalf("a drill disturbed the live release: serving %q", body) + } + }) + + t.Run("backup restore returns the data", func(t *testing.T) { + s.psql(t, "create table if not exists survivors(note text)") + s.psql(t, "insert into survivors values (concat('written', ' ', 'before'))") + s.mustOb(t, dir, "backup", "create", "postgres") + s.mustOb(t, dir, "backup", "restore", "postgres", "--confirm", "postgres") + if note := s.psql(t, "select note from survivors limit 1"); note != "written before" { + t.Fatalf("the recovered cluster does not hold the row: %q", note) + } + }) + + t.Run("backup prune", func(t *testing.T) { + s.mustOb(t, dir, "backup", "prune", "postgres") + }) + + t.Run("backup disable stops archiving and removes its timers", func(t *testing.T) { + s.mustOb(t, dir, "backup", "disable", "postgres", "--confirm", "postgres") + if mode := s.psql(t, "show archive_mode"); mode == "on" { + t.Error("archiving is still on after disable") + } + units := s.run(t, "systemctl list-units --type=timer --all --no-pager | grep -c ob-backup || true") + if strings.TrimSpace(units) != "0" { + t.Errorf("backup timers survived disable: %s", units) + } + }) + + // Last, because it removes what every step above built. + t.Run("destroy leaves the machine clean", func(t *testing.T) { + out, err := s.obInput(t, dir, s.obHome(t), "observer\ny\n", "destroy", "--volumes") + if err != nil { + t.Fatalf("destroy failed: %v\n%s", err, out) + } + if left := strings.TrimSpace(s.run(t, + `docker ps -aq --filter label=ob.app=observer | wc -l`)); left != "0" { + t.Errorf("%s containers survived destroy", left) + } + // The object store is not ob's and must be untouched by a teardown of + // the application beside it. + if err := s.try("systemctl is-active --quiet minio"); err != nil { + t.Error("destroy stopped a service that does not belong to ob") + } + }) +} diff --git a/e2e/testdata/postgres/README.md b/e2e/testdata/postgres/README.md new file mode 100644 index 00000000..f807ed83 --- /dev/null +++ b/e2e/testdata/postgres/README.md @@ -0,0 +1,12 @@ +# postgres fixture + +The project the server end-to-end suite deploys. + +`secrets/age.key` is a committed private key, and that is deliberate. It +decrypts `secrets/backup.env`, which holds the credentials for a MinIO the test +starts on a throwaway guest and deletes afterwards. Both are generated for this +fixture and are valid nowhere else. A real project keeps its age key out of the +repository. + +`ob.yml.tmpl` is rendered at test time: the server address and the object-store +endpoint are only known once the guest is running. diff --git a/e2e/testdata/postgres/ob.yml.tmpl b/e2e/testdata/postgres/ob.yml.tmpl new file mode 100644 index 00000000..fe690d43 --- /dev/null +++ b/e2e/testdata/postgres/ob.yml.tmpl @@ -0,0 +1,70 @@ +# The project the server suite deploys: a managed PostgreSQL with a backup +# policy pointing at an object store the guest runs itself. +# +# Rendered rather than checked in whole, because two values are only known once +# the guest is up: the address ob connects to, and the endpoint the object +# store is reachable at from inside the container. +api_version: onebox.run/v1 +app: observer +environments: + production: + server: {{ .Server }} +# A workload is required, and this one exists only to satisfy that: the suite +# is about the database beside it. Kept to a sleeping busybox so the deploy +# costs a pull of a few megabytes rather than a real application image. +# One workload that actually serves something, because a sleeping container +# cannot answer whether a release replaced what was running. It writes its +# version into a file and serves it, so `exec` can read the version back and +# `rollback` has something observable to undo. +workloads: + app: + role: application + # Digest-pinned and taken from a mirror, so a deploy never asks a registry + # to resolve a tag. That resolution is `docker buildx imagetools inspect` + # (internal/engine/plan.go), which walks every child manifest in the index + # — sixteen requests for this image — and exhausts the anonymous quota of + # any public registry within a few runs. Tag resolution has its own test. + image: public.ecr.aws/docker/library/busybox@sha256:9db7b59979c38555a39def84a31fb98b5296952f9e3afd4f6f11f05b07adfab0 + command: + - sh + - -c + - mkdir -p /srv && echo {{ .Version }} > /srv/index.html && httpd -f -p 8080 -h /srv + strategy: rolling + health: { exec: "wget -qO- http://localhost:8080/", interval: 1s, start_period: 1s, within: 30s } + # A one-shot workload so `job plan` and `job run` have something sealed to + # run. Jobs are workloads with the job role, which is why the loader reports + # them separately from applications. + chore: + role: job + image: public.ecr.aws/docker/library/busybox@sha256:9db7b59979c38555a39def84a31fb98b5296952f9e3afd4f6f11f05b07adfab0 + command: ["sh", "-c", "echo chore-ran"] + data_effect: none +deployment: + order: [app] +services: + postgres: + driver: postgres + version: "18" + persistence: { mode: durable } + backup: + target: offsite + recovery_kind: pitr + max_data_loss: 15m + schedule: { cron: "0 2 * * *", timezone: UTC } +backup_targets: + offsite: + kind: s3-compatible + # HTTPS, with a certificate signed by an authority that exists only on this + # guest. That is the whole point: nothing about this endpoint is publicly + # trusted, so it can only verify if ob carries the host's trust store into + # the container the way it carries the binary. + endpoint: {{ .Endpoint }} + bucket: observer-backups + failure_domain: { identity: e2e/guest } + credentials: + file: secrets/backup.env + access_key_entry: BACKUP_ACCESS_KEY_ID + secret_key_entry: BACKUP_SECRET_ACCESS_KEY + encryption: { pitr: client-side } +proxy: + managed: false diff --git a/e2e/testdata/postgres/secrets/age.key b/e2e/testdata/postgres/secrets/age.key new file mode 100644 index 00000000..52c9a062 --- /dev/null +++ b/e2e/testdata/postgres/secrets/age.key @@ -0,0 +1,3 @@ +# created: 2026-08-21T16:47:21-07:00 +# public key: age1xtc0fzq29crqzk98r59zdetgyxdhqwqm7qrvkal4n7zr3cu5wgassh3u3x +AGE-SECRET-KEY-19EKADLZ4Q08XTW3XQ8RQ443PFQXX7Z4D2463ZYD5KQDN975AM9ASCNN38Q diff --git a/e2e/testdata/postgres/secrets/backup.env b/e2e/testdata/postgres/secrets/backup.env new file mode 100644 index 00000000..a60e4796 --- /dev/null +++ b/e2e/testdata/postgres/secrets/backup.env @@ -0,0 +1,9 @@ +BACKUP_ACCESS_KEY_ID=ENC[AES256_GCM,data:lSmmCEHir7dioHfLwuo=,iv:Bt8xUhRZBGG9JP20IVxc2O1LnE9h2iPn0YRuxUs1udg=,tag:IB2q8mLvrknktl/qtjwVBw==,type:str] +BACKUP_SECRET_ACCESS_KEY=ENC[AES256_GCM,data:gzaMGkamkHevSz1ABtJfoi0K+OwtyFqW,iv:9hUVpTJt3I53lx3aKAgeCfJS1KAhR9MAgJGgi2gAsus=,tag:m8RxxbIaIoiff7fjRTvWDw==,type:str] +OB_REPOSITORY_KEY=ENC[AES256_GCM,data:SG1JshUdQ7bWjupdNZmIod+trFELZn8HKoyLO7DLchi22UjUtNNcOBo0qXaPzlLzlk4cY/C62yBsxTUtXYaXbQ==,iv:retfIEsNiwlQW27S2s5ENxRDpdPZj70KtmUrH12poRY=,tag:Tw/aoKBM4zHtsmu8r/w9ew==,type:str] +sops_age__list_0__map_enc=-----BEGIN AGE ENCRYPTED FILE-----\nYWdlLWVuY3J5cHRpb24ub3JnL3YxCi0+IFgyNTUxOSBsWUZYeHhnTnBXT0UweEpn\nTlJRQnV0by9yNlRPOTZhcFlQVHd5VkIvRjJBClViZXRiR2JOWjJLazRadU54M0VB\nZExmS0RqeWh3MlQ5ZVVPZnZCS2NnNGsKLS0tIFkxSktyc3lMN2srdkNwNkhIWUFD\nUStCbDh3MTBDUmV1Y2tzeWdTR01SMFUKh6PXLS/J02QYax8z/T35Sz24KxwHSH1A\nm1Fxpb4n3ue/lEsKRPbUlwwRpzAMBHhmccfSrNAyBwu2RQ6VyZ17jQ==\n-----END AGE ENCRYPTED FILE-----\n +sops_age__list_0__map_recipient=age1xtc0fzq29crqzk98r59zdetgyxdhqwqm7qrvkal4n7zr3cu5wgassh3u3x +sops_lastmodified=2026-08-21T23:47:21Z +sops_mac=ENC[AES256_GCM,data:bNfd63Xkxu6BQNWP0/Go5h1Brtcg6wDVvqmZDRSUDlyu1JhEWVbIBDsgmXf+v4Dy6wlkGpKWT97CCmScJKfP9LY9iKvZBa0uSlpESKgKgzFY7nHUnkiXPjM5U7v5lmWyBuBuGooW1D0kXG71l88mcnfXe0ij9zEQEfYTSAboCZ4=,iv:53tToUkZi3aQ47RX0Su+i9qqreTDUaTo34hFDyYaSfo=,tag:KCfqsNjBhshEsKZH0qItyQ==,type:str] +sops_unencrypted_suffix=_unencrypted +sops_version=3.13.3 diff --git a/scripts/lima.sh b/scripts/lima.sh new file mode 100755 index 00000000..8da0516b --- /dev/null +++ b/scripts/lima.sh @@ -0,0 +1,93 @@ +#!/usr/bin/env bash +# +# Boots the throwaway server the `server-e2e` suite deploys to, and derives the +# connection details from the running instance. +# +# The suite itself runs on this machine, not in the guest. ob's user position +# is a workstation with SSH to a rented box — internal/transport/transport.go +# records that the docker suite substitutes local docker for that server — and +# this is the one harness that does not make the substitution. Running the +# tests inside the guest would relocate the blind spot rather than close it. +set -euo pipefail + +instance="onebox-e2e" +config="e2e/lima.yaml" +repo="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" + +# Lima's own ssh.config is the supported source for an instance's connection +# details. `limactl show-ssh` reads the same thing and is deprecated as of +# Lima 2.2, so it is not used here. +instance_ssh_config() { + local path="${LIMA_HOME:-$HOME/.lima}/${instance}/ssh.config" + [[ -r "$path" ]] || { + echo "no ssh.config for instance ${instance}; run 'just lima-up' first" >&2 + return 1 + } + printf '%s\n' "$path" +} + +ssh_field() { + local field="$1" path + path="$(instance_ssh_config)" + awk -v field="$field" ' + $1 == field { gsub(/"/, "", $2); print $2; exit } + ' "$path" +} + +up() { + if limactl list --quiet 2>/dev/null | grep -qx "$instance"; then + echo "instance ${instance} already exists; reusing it" + else + limactl start --name="$instance" --tty=false "${repo}/${config}" + fi + + local port key + port="$(ssh_field Port)" + key="$(ssh_field IdentityFile)" + + # ob connects as root. Proving that here, at boot, keeps a permissions + # problem from surfacing later as an unrelated-looking deploy failure. + if ! ssh -q -o BatchMode=yes -o StrictHostKeyChecking=no \ + -o UserKnownHostsFile=/dev/null -o ConnectTimeout=10 \ + -i "$key" -p "$port" root@127.0.0.1 true; then + echo "the guest is up but refuses root over SSH; ob does not elevate, so the suite cannot run" >&2 + return 1 + fi + echo "server ready: root@127.0.0.1:${port}" +} + +# Prints the environment the suite reads. Kept separate from `test` so it can +# be eval'd into a shell for running individual cases by hand. +env_lines() { + local port key + port="$(ssh_field Port)" + key="$(ssh_field IdentityFile)" + # The same string shape ob.yml's `server:` field takes, parsed by the same + # code: internal/target.Address is [user@]host[:port]. + printf 'OB_SERVER_E2E=1\n' + printf 'OB_E2E_SERVER=root@127.0.0.1:%s\n' "$port" + printf 'OB_E2E_SERVER_KEY=%s\n' "$key" +} + +run_tests() { + local -a env=() + while IFS= read -r line; do env+=("$line"); done < <(env_lines) + # -count=1 because a cached pass against a guest that has since changed is + # a green tick for work nobody did. + env OB_E2E=1 "${env[@]}" \ + go test "${repo}/e2e/" -count=1 -timeout 40m -run Server "$@" +} + +case "${1:-}" in +up) up ;; +env) env_lines ;; +test) + shift + run_tests "$@" + ;; +down) limactl delete -f "$instance" ;; +*) + echo "usage: $0 {up|env|test|down}" >&2 + exit 2 + ;; +esac From 94da6a6525b4c0ba6b068cd0894bb279c0e15b92 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Fri, 21 Aug 2026 20:33:34 -0700 Subject: [PATCH 2/7] fix(destroy): take backup timers down with the application MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RemoveSchedules matched "ob--" and backup timers are named "ob-backup----", so teardown swept the job namespace and left the backup one behind. Two timers and four unit files survived `ob destroy --volumes`, loaded and firing on schedule against a release directory the same command had just deleted. The split is deliberate and stays: BackupTimerForEnvironment explains that the job scheduler treats every "ob--*" unit as its own and removes what is no longer declared, which once deleted every scheduled backup on the next deploy. Teardown is the case that needs both namespaces, and its own comment already claimed it took down every timer the app installed. A later operation against the same application does sweep the orphans up, which is why this was easy to miss: the units only persist when destroy is the last thing you do — the normal case. Closes #92 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DF9dqmoT7gsSk7TfzL2WQS --- internal/engine/schedule.go | 24 +++++++++++++++--- internal/engine/schedule_test.go | 43 ++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/internal/engine/schedule.go b/internal/engine/schedule.go index 4344209b..05ac656e 100644 --- a/internal/engine/schedule.go +++ b/internal/engine/schedule.go @@ -192,7 +192,19 @@ func setOf(in []string) map[string]struct{} { // destroyed app's timer keeps firing against a release directory that has been // deleted, failing every minute forever and explaining itself to nobody. func (e *Engine) RemoveSchedules(ctx context.Context) error { - prefix := "ob-" + e.Spec.Name + "-" + // Both namespaces this application installs into. + // + // Backup timers are deliberately named outside the job scheduler's + // namespace — app.BackupTimerForEnvironment explains why: a deploy used to + // treat them as "no longer declared" and delete every scheduled backup. + // Teardown is the opposite case and needs both, and matching only the job + // prefix meant `ob destroy` left ob-backup---- + // timers loaded and firing against a release directory it had just + // deleted. They belong to this application and they go with it. + prefixes := []string{ + "ob-" + e.Spec.Name + "-", + app.BackupUnitPrefix + e.Spec.Name + "-", + } res, err := e.T.Run(ctx, "systemctl list-unit-files --no-legend --type=timer 2>/dev/null | awk '{print $1}'") if err != nil { return err @@ -200,8 +212,14 @@ func (e *Engine) RemoveSchedules(ctx context.Context) error { var units []string for _, line := range strings.Split(res.Stdout, "\n") { unit := strings.TrimSpace(line) - if strings.HasPrefix(unit, prefix) && strings.HasSuffix(unit, ".timer") && unitName.MatchString(unit) { - units = append(units, strings.TrimSuffix(unit, ".timer")) + if !strings.HasSuffix(unit, ".timer") || !unitName.MatchString(unit) { + continue + } + for _, prefix := range prefixes { + if strings.HasPrefix(unit, prefix) { + units = append(units, strings.TrimSuffix(unit, ".timer")) + break + } } } if len(units) == 0 { diff --git a/internal/engine/schedule_test.go b/internal/engine/schedule_test.go index c9482489..01cd3a14 100644 --- a/internal/engine/schedule_test.go +++ b/internal/engine/schedule_test.go @@ -79,3 +79,46 @@ func TestSyncSchedulesLeavesBackupTimersAlone(t *testing.T) { t.Fatalf("backup timer %q is inside the job scheduler's namespace and a deploy would delete it", backupTimer) } } + +// Teardown has to take both namespaces with it. +// +// Backup timers are named outside the job scheduler's namespace on purpose — +// a deploy used to treat them as "no longer declared" and delete every +// scheduled backup. Teardown is the opposite case: matching only the job +// prefix left `ob destroy` with ob-backup--… timers still loaded, firing +// against a release directory the same command had just deleted. +func TestRemoveSchedulesTakesBackupTimersToo(t *testing.T) { + f := &transport.Fake{Dynamic: func(cmd string) (transport.Result, bool) { + if strings.Contains(cmd, "list-unit-files") { + return transport.Result{Stdout: strings.Join([]string{ + "ob-sample-nightly.timer", + "ob-backup-sample-production-postgres-backup.timer", + "ob-backup-sample-production-postgres-verify.timer", + // Another application's, and a stranger's. Neither is ours. + "ob-backup-other-production-postgres-backup.timer", + "logrotate.timer", + "", + }, "\n")}, true + } + return transport.Result{}, false + }} + e := New(testConfig(), testProject(t), f, Options{Out: &bytes.Buffer{}, Sleep: noSleep}) + if err := e.RemoveSchedules(context.Background()); err != nil { + t.Fatalf("remove schedules: %v", err) + } + seq := strings.Join(f.Commands, "\n") + for _, want := range []string{ + "ob-sample-nightly", + "ob-backup-sample-production-postgres-backup", + "ob-backup-sample-production-postgres-verify", + } { + if !strings.Contains(seq, "rm -f /etc/systemd/system/"+want+".timer") { + t.Errorf("teardown left %s installed:\n%s", want, seq) + } + } + for _, never := range []string{"ob-backup-other-production", "logrotate"} { + if strings.Contains(seq, never) { + t.Errorf("teardown removed a unit that is not this application's (%s):\n%s", never, seq) + } + } +} From fdc7ab26e17c909e6e5516aa0f7a70cbaad34dce Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Fri, 21 Aug 2026 20:33:34 -0700 Subject: [PATCH 3/7] fix(backup): carry the host trust store, revert failed enablements, scope repositories per cluster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three faults on the same path, found against a real server. The trust store never reached the container. wal-g executes inside the driver's image and postgres:18 carries no certificate authorities, so every upload to the HTTPS endpoint an s3-compatible target must declare failed with "x509: certificate signed by unknown authority". It failed late — the base backup ran first, so the error arrived a quarter of an hour in, with archiving already on. The bundle now travels the way the binary does, through the directory already mounted read-only into the container, and the wrapper exports it under the three names that can do the verifying: wal-g's own S3 setting, the AWS SDK beneath it, and crypto/tls beneath that. A host with no bundle anywhere is refused before archiving is turned on rather than discovered afterwards. A failed enablement left archiving on. PostgreSQL retains every segment it cannot ship, so a command that reported failure established an unbounded disk commitment. Enablement now walks the same pending-then-disabled transitions `ob backup disable` walks when the base backup fails — but only when that run is what turned archiving on. A service that was already protected is left as it was, because taking it to unprotected over a failed re-enable would break a working setup to tidy a failed change. Repositories were shared across clusters. WalgPrefix was _ with no cluster identity, and every fresh cluster starts its write-ahead log at 000000010000000000000001 — so a database redeployed against a target that still held the previous one's history wrote object names that already existed with different bytes. wal-g refuses the overwrite, PostgreSQL archives strictly in order, and the chain stopped at the first collision while enable reported success. `ob destroy --volumes` removes the cluster and leaves the repository, which is the point of a backup, so the sequence that reaches this is ordinary. The prefix now carries PostgreSQL's system identifier, recorded at enablement and used by rendering, status, backup and restore alike. An empty generation names the legacy layout and stays readable; a repository that moves is reported rather than silently rebased. Enablement also quiesces the archiver before taking the base backup: a forced WAL switch, then a wait for archive_status to empty. Nothing at enable time can undo a collision written by a cluster that no longer exists, but the condition is now detected and refused instead of establishing a chain that is already broken — and that failure routes through the revert above, so the service is left unprotected rather than falsely protected. Verified on a disposable Linux guest over SSH: red before each fix, green after. TestServerLifecycle covers the trust store and restore; TestServerProbes covers the failed enablement, the repeated one and the redeployed database. Closes #88 Closes #91 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DF9dqmoT7gsSk7TfzL2WQS --- TRADEMARK.md | 58 +++++++ internal/app/backup_walg.go | 69 ++++++-- internal/app/backup_walg_test.go | 59 ++++++- internal/app/names.go | 15 ++ internal/app/service_image_state.go | 13 ++ internal/engine/backup_postgres.go | 140 ++++++++++++++++ internal/engine/backup_postgres_ops.go | 6 +- internal/engine/backup_restore.go | 6 +- .../engine/backup_system_identifier_test.go | 44 +++++ internal/engine/backup_trust_store_test.go | 59 +++++++ internal/onebox/backup_enable.go | 152 +++++++++++++++++- internal/onebox/backup_enable_test.go | 86 ++++++++++ internal/onebox/backup_state.go | 44 +++-- internal/onebox/backup_state_test.go | 4 +- 14 files changed, 720 insertions(+), 35 deletions(-) create mode 100644 TRADEMARK.md create mode 100644 internal/engine/backup_system_identifier_test.go create mode 100644 internal/engine/backup_trust_store_test.go diff --git a/TRADEMARK.md b/TRADEMARK.md new file mode 100644 index 00000000..d326ea85 --- /dev/null +++ b/TRADEMARK.md @@ -0,0 +1,58 @@ +# Onebox trademark policy + +The Onebox source code is licensed under the Apache License, Version 2.0. That +license covers copyright and patents. It does **not** grant rights to the +Onebox name, logo, or related marks — Apache-2.0 section 6 says so explicitly. +This document states what LabStack LLC permits and does not permit, so that +everyone knows where the line is. + +## What this covers + +- The name "Onebox" and the mark "ob". +- The Onebox logo and wordmark. +- The container image name `ghcr.io/labstack/onebox`. + +## You do not need permission to + +- Say truthfully that your project, product, or service works with, integrates + with, is built on, or is compatible with Onebox. +- Redistribute unmodified Onebox releases under the name Onebox. +- Write about Onebox — articles, talks, tutorials, comparisons, reviews, + including critical ones. +- Use the name in a user group, meetup, conference talk title, or community + channel that is clearly not operated by LabStack. +- Fork the source and build on it, as Apache-2.0 permits, provided the result + is not distributed **under the Onebox name**. + +Nothing in this policy restricts any right Apache-2.0 grants you over the code. + +## You need written permission to + +- Distribute a **modified** version of Onebox under the name Onebox. +- Offer a hosted or managed service named Onebox, or named in a way that leads a + reasonable person to think LabStack operates it. +- Use Onebox, or a confusingly similar term, in your company name, product name, + domain name, app-store listing, or social account handle. +- Use the Onebox logo or wordmark as your own branding, or in a modified form. +- Sell merchandise bearing the marks. + +## Never permitted + +- Implying sponsorship, endorsement, affiliation, or partnership with LabStack + where none exists. +- Using the marks in a way that suggests your modified build is the official + Onebox distribution. +- Registering the marks, or confusingly similar marks, as trademarks or domain + names. + +## When in doubt + +The test is whether an ordinary reader would be misled about who made or who +operates the thing. Descriptive, truthful references are fine. Branding is not. + +## Requests + +Ask LabStack LLC in writing. Reasonable requests are usually granted, and +LabStack may revoke permission if a use becomes misleading. + +This policy may change. The current version is the one in this repository. diff --git a/internal/app/backup_walg.go b/internal/app/backup_walg.go index ca0243f0..6ccee01d 100644 --- a/internal/app/backup_walg.go +++ b/internal/app/backup_walg.go @@ -47,24 +47,36 @@ const WalgMountPath = "/opt/onebox/backup" // wal-g's out of the operator's encrypted file. const WalgBinary = WalgMountPath + "/ob-wal-g" +// WalgTrustStore is the host trust store as the container sees it. The staged +// copy lands beside the binary, inside the directory already mounted read-only +// at WalgMountPath. +const WalgTrustStore = WalgMountPath + "/ca-certificates.crt" + // WalgRepositoryKeyEntry is the credential entry holding the repository // encryption key. Unlike the destination keys it has a fixed name: the key is // Onebox's own requirement rather than a property of the destination, so there // is no backup_targets field to indirect through. const WalgRepositoryKeyEntry = "OB_REPOSITORY_KEY" -// WalgPrefix is the repository location for one protected service. +// WalgPrefix is the repository location for one protected database generation. // // The application and service are joined with the injective rule the rest of // the derived names use, not a hyphen. Hyphens are legal in both, so `a-b`/`c` -// and `a`/`b-c` would land on the same prefix and interleave two clusters' base -// backups and WAL — and this string is unversioned by design, so it could not -// be corrected later without orphaning every backup taken before the fix. -func WalgPrefix(target BackupTarget, app, service string) string { +// and `a`/`b-c` must not land on the same prefix. A PostgreSQL system identifier +// then separates successive clusters for the same service. Every fresh cluster +// starts WAL numbering at the same name, so sharing that namespace would make +// the first new segment collide with the old cluster's history. +// +// An empty generation names the legacy layout. It remains readable so a cluster +// protected before generations existed can keep its established history. +func WalgPrefix(target BackupTarget, app, service, generation string) string { segments := []string{strings.Trim(target.Prefix, "/"), Join(app, service)} if segments[0] == "" { segments = segments[1:] } + if generation != "" { + segments = append(segments, "clusters", generation) + } return "s3://" + target.Bucket + "/" + strings.Join(segments, "/") } @@ -79,7 +91,7 @@ func WalgArchiveCommand() string { return WalgBinary + " wal-push %p" } // WalgEnvironment is the non-secret configuration a protected service runs // with. Every value here is derived from the project and safe to read: the // destination's location, not its keys. -func WalgEnvironment(target BackupTarget, app, service string) (map[string]any, error) { +func WalgEnvironment(target BackupTarget, repository, database, service string) (map[string]any, error) { if target.Kind != "s3-compatible" { return nil, fmt.Errorf("backup for %q: unsupported target kind %q", service, target.Kind) } @@ -97,7 +109,7 @@ func WalgEnvironment(target BackupTarget, app, service string) (map[string]any, service) } env := map[string]any{ - "WALG_S3_PREFIX": WalgPrefix(target, app, service), + "WALG_S3_PREFIX": repository, "AWS_ENDPOINT": target.Endpoint, // Path-style addressing, because an S3-compatible endpoint is usually // not the one provider whose virtual-host naming works everywhere. A @@ -123,7 +135,7 @@ func WalgEnvironment(target BackupTarget, app, service string) (map[string]any, // missing variable. "PGHOST": "/var/run/postgresql", "PGUSER": PgSuperuser, - "PGDATABASE": app, + "PGDATABASE": database, "PGDATA": PgDataPath, } if target.Region != "" { @@ -194,6 +206,25 @@ func RenderWalgWrapper(target BackupTarget) []byte { assign("AWS_SECRET_ACCESS_KEY", target.Credentials.SecretKeyEntry) assign("AWS_SESSION_TOKEN", target.Credentials.SessionTokenEntry) assign("WALG_LIBSODIUM_KEY", WalgRepositoryKeyEntry) + // The trust store, if one was staged. + // + // wal-g executes inside the driver's image, and the official PostgreSQL + // images ship no certificate authorities, so an HTTPS endpoint — which + // every s3-compatible target is required to be — fails verification with + // "certificate signed by unknown authority". Three names because three + // layers can do the verifying: wal-g's own S3 setting, the AWS SDK + // underneath it, and Go's crypto/tls under that. + // + // Guarded on the file being readable rather than exported unconditionally: + // a host with no bundle should leave the image's own store in play, and + // pointing these at a path that does not exist makes the SDK fail with a + // worse error than the one it replaces. + b.WriteString("if [ -r " + WalgTrustStore + " ]; then\n") + for _, name := range []string{"WALG_S3_CA_CERT_FILE", "AWS_CA_BUNDLE", "SSL_CERT_FILE"} { + b.WriteString(" " + name + "=" + WalgTrustStore + "\n") + b.WriteString(" export " + name + "\n") + } + b.WriteString("fi\n") b.WriteString("exec " + WalgMountPath + "/wal-g \"$@\"\n") return []byte(b.String()) } @@ -295,7 +326,11 @@ func (r *Resolved) backupForRender(n Names, serviceName string) (*serviceBackup, if err != nil { return nil, err } - environment, err := WalgEnvironment(projection.Target, r.Spec.Name, serviceName) + repository, err := r.BackupRepository(serviceName) + if err != nil { + return nil, err + } + environment, err := WalgEnvironment(projection.Target, repository, r.Spec.Name, serviceName) if err != nil { return nil, err } @@ -363,6 +398,22 @@ func (r *Resolved) ServiceIsProtected(serviceName string) bool { return observed && (state.BackupState == "enabled" || state.BackupState == "disable-pending") } +// BackupRepository returns the exact repository generation recorded for a +// protected service. The empty generation is the pre-generation layout and is +// intentionally retained for compatibility with an established history. +func (r *Resolved) BackupRepository(serviceName string) (string, error) { + state, observed := r.serviceRuntime[serviceName] + if !observed || (state.BackupState != "enabled" && state.BackupState != "disable-pending") { + return "", errf("backup_state_incomplete", "services."+serviceName+".backup", "ob backup status "+serviceName, + "service %s has no established backup repository", serviceName) + } + projection, err := r.EffectiveBackupProjection(serviceName) + if err != nil { + return "", err + } + return WalgPrefix(projection.Target, r.Spec.Name, serviceName, state.BackupRepositoryGeneration), nil +} + // normalizeMachine folds the spellings of one architecture onto a single name. // // `uname -m` is not standardised: Linux says aarch64 where Darwin says arm64, diff --git a/internal/app/backup_walg_test.go b/internal/app/backup_walg_test.go index cdb26f7a..3fdbbd14 100644 --- a/internal/app/backup_walg_test.go +++ b/internal/app/backup_walg_test.go @@ -61,7 +61,8 @@ func TestRecordedProjectionWinsOverEditedIntent(t *testing.T) { } bound, err := edited.WithServiceRuntimeStates(map[string]ServiceRuntimeState{ "db": {BackupState: "enabled", ServiceImage: "postgres@sha256:" + strings.Repeat("a", 64), - PublicationVerified: true, DigestAvailable: true, LastEffective: &recorded}, + PublicationVerified: true, DigestAvailable: true, LastEffective: &recorded, + DatabaseSystemIdentifier: "7513211627332151223", BackupRepositoryGeneration: "7513211627332151223"}, }) if err != nil { t.Fatal(err) @@ -73,6 +74,14 @@ func TestRecordedProjectionWinsOverEditedIntent(t *testing.T) { if got.Target.Bucket != "recorded-bucket" || got.Policy.Target != "original" { t.Fatalf("projection = %#v, want the recorded one — an edited target must not redirect a restore", got) } + backup, err := bound.backupForRender(bound.Spec.NamesFor("production"), "db") + if err != nil { + t.Fatal(err) + } + wantRepository := "s3://recorded-bucket/shop_db/clusters/7513211627332151223" + if repository := backup.Environment["WALG_S3_PREFIX"]; repository != wantRepository { + t.Fatalf("rendered repository = %q, want recorded generation %q", repository, wantRepository) + } } // The repository prefix must be injective. Hyphens are legal in both an app and @@ -81,13 +90,29 @@ func TestRecordedProjectionWinsOverEditedIntent(t *testing.T) { // prefix is unversioned, so this cannot be corrected later. func TestWalgPrefixCannotCollideAcrossHyphenatedNames(t *testing.T) { target := BackupTarget{Bucket: "backups", Prefix: "production"} - first := WalgPrefix(target, "a-b", "c") - second := WalgPrefix(target, "a", "b-c") + first := WalgPrefix(target, "a-b", "c", "1") + second := WalgPrefix(target, "a", "b-c", "1") if first == second { t.Fatalf("two distinct services share the repository prefix %q", first) } } +func TestWalgPrefixSeparatesSuccessiveDatabaseClusters(t *testing.T) { + target := BackupTarget{Bucket: "backups", Prefix: "production"} + first := WalgPrefix(target, "shop", "database", "7513211627332151223") + second := WalgPrefix(target, "shop", "database", "7513211627332151224") + if first == second { + t.Fatalf("successive PostgreSQL clusters share repository %q", first) + } + if !strings.HasSuffix(first, "/clusters/7513211627332151223") { + t.Fatalf("cluster-scoped prefix = %q", first) + } + legacy := WalgPrefix(target, "shop", "database", "") + if strings.Contains(legacy, "/clusters/") { + t.Fatalf("legacy repository was silently relocated: %q", legacy) + } +} + // A quoted value is ordinary in a shell-sourced dotenv and is stripped when the // file is installed, so validation must judge the same form. Judging the quoted // text rejected a perfectly good key for being 66 characters, with a message @@ -101,3 +126,31 @@ func TestQuotedCredentialValuesAreAccepted(t *testing.T) { t.Fatalf("quoted credential file rejected: %v", err) } } + +// The wrapper is the only place that can put a trust store in wal-g's +// environment: it runs inside the driver's image, which ships none. +func TestTheWrapperPointsWalgAtTheStagedTrustStore(t *testing.T) { + wrapper := string(RenderWalgWrapper(BackupTarget{ + Credentials: CredentialReference{ + AccessKeyEntry: "BACKUP_ACCESS_KEY_ID", + SecretKeyEntry: "BACKUP_SECRET_ACCESS_KEY", + }, + })) + + // Three layers can do the verifying, and which one does depends on the + // endpoint: wal-g's own S3 setting, the AWS SDK beneath it, and crypto/tls + // beneath that. Naming only one leaves the other two on an empty store. + for _, name := range []string{"WALG_S3_CA_CERT_FILE", "AWS_CA_BUNDLE", "SSL_CERT_FILE"} { + if !strings.Contains(wrapper, "export "+name) { + t.Errorf("wrapper never exports %s, so an HTTPS endpoint cannot be verified:\n%s", name, wrapper) + } + if !strings.Contains(wrapper, name+"="+WalgTrustStore) { + t.Errorf("%s does not point at the staged bundle %s", name, WalgTrustStore) + } + } + // Guarded, not unconditional: a host with no bundle should leave the + // image's own store in play rather than name a path that is not there. + if !strings.Contains(wrapper, "if [ -r "+WalgTrustStore+" ]; then") { + t.Errorf("the trust store is exported without checking it was staged:\n%s", wrapper) + } +} diff --git a/internal/app/names.go b/internal/app/names.go index fcdbff09..5b62b295 100644 --- a/internal/app/names.go +++ b/internal/app/names.go @@ -122,6 +122,21 @@ func (n Names) BackupWrapperFile(service string) string { return path.Join(n.BackupRuntimeDir(service), "ob-wal-g") } +// BackupTrustStoreFile is the host's certificate authority bundle, copied in +// beside the binary. +// +// wal-g runs inside the driver's image, and the official PostgreSQL images +// carry no trust store: `postgres:18` has no /etc/ssl/certs/ca-certificates.crt +// at all. Since every S3-compatible target is required to be HTTPS, a wal-g +// with nothing to verify against cannot upload anywhere — it fails the +// handshake with "x509: certificate signed by unknown authority" after the +// base backup has already been written, and archiving has already been turned +// on. The trust store therefore travels the same way the binary does, through +// the directory that is already mounted read-only into the container. +func (n Names) BackupTrustStoreFile(service string) string { + return path.Join(n.BackupRuntimeDir(service), "ca-certificates.crt") +} + // ServiceSecretFile holds the credential Onebox generates on the target. It is // written once and never travels: not in the project, not in the rendered // runtime, not in the digest. diff --git a/internal/app/service_image_state.go b/internal/app/service_image_state.go index b6f0c92b..02882192 100644 --- a/internal/app/service_image_state.go +++ b/internal/app/service_image_state.go @@ -30,6 +30,11 @@ type ServiceRuntimeState struct { TagObservedDigest string RefreshCandidate *ServiceImageCandidate LastEffective *BackupEffectiveProjection + // DatabaseSystemIdentifier is PostgreSQL's stable identity for the data + // volume. BackupRepositoryGeneration is the generation segment bound at + // enablement; it is empty only for repositories using the legacy layout. + DatabaseSystemIdentifier string + BackupRepositoryGeneration string } type ServiceImageSelection struct { @@ -74,6 +79,12 @@ func (state ServiceRuntimeState) validate(service string) error { return errf("service_image_digest_unavailable", "services."+service, "ob service status --output json", "%v", err) } } + if state.DatabaseSystemIdentifier != "" && !postgresSystemIdentifier.MatchString(state.DatabaseSystemIdentifier) { + return errf("project_invalid", "services."+service, "ob backup status --output json", "service runtime state has an invalid PostgreSQL system identifier") + } + if state.BackupRepositoryGeneration != "" && state.BackupRepositoryGeneration != state.DatabaseSystemIdentifier { + return errf("project_invalid", "services."+service, "ob backup status --output json", "backup repository generation does not match the PostgreSQL system identifier") + } previous := "" for _, image := range state.ManifestRootImages { if err := validatePinnedServiceImage(image); err != nil { @@ -92,6 +103,8 @@ func (state ServiceRuntimeState) validate(service string) error { return nil } +var postgresSystemIdentifier = regexp.MustCompile(`^[0-9]{1,20}$`) + func (r *Resolved) selectServiceImage(serviceName, tagImage string) (ServiceImageSelection, error) { state, observed := r.serviceRuntime[serviceName] if !observed || state.BackupState == "never-enabled" || state.BackupState == "disabled" { diff --git a/internal/engine/backup_postgres.go b/internal/engine/backup_postgres.go index 84217f22..39091b72 100644 --- a/internal/engine/backup_postgres.go +++ b/internal/engine/backup_postgres.go @@ -10,6 +10,7 @@ import ( "os" "path" "path/filepath" + "strconv" "strings" "time" @@ -88,9 +89,148 @@ func (e *Engine) StageBackupRuntime(ctx context.Context, service string, wrapper if err := e.chmodPath(ctx, wrapperPath, "0755"); err != nil { return err } + if err := e.stageTrustStore(ctx, service); err != nil { + return err + } return e.chmodPath(ctx, n.BackupRuntimeDir(service), "0755") } +// trustStoreCandidates are the certificate authority bundles a Linux host is +// likely to keep, most common first. Debian and Ubuntu — what the qualified +// PostgreSQL images are built from — use the first. +var trustStoreCandidates = []string{ + "/etc/ssl/certs/ca-certificates.crt", + "/etc/pki/tls/certs/ca-bundle.crt", + "/etc/ssl/ca-bundle.pem", + "/etc/ssl/cert.pem", +} + +// stageTrustStore copies the host's certificate authorities in beside the +// binary, because wal-g runs in the driver's image and that image has none. +// +// `postgres:18` ships two entries under /etc/ssl/certs and no bundle among +// them, so every upload to the HTTPS endpoint an s3-compatible target is +// required to declare fails verification. It failed *late*: the base backup +// completed first, so the error arrived a quarter of an hour in, against a +// server whose archiving was already on. +// +// A host with no bundle is refused here rather than discovered there. The +// alternative is staging nothing, letting the wrapper fall back to the image's +// empty store, and reproducing exactly the failure this exists to prevent — +// only later, and with the database already archiving. +func (e *Engine) stageTrustStore(ctx context.Context, service string) error { + destination := e.names().BackupTrustStoreFile(service) + // Copied on the target rather than uploaded from here: the bundle that + // matters is the one the operator's machine trusts, and this machine may + // not be the same operating system. + // + // Written to a temporary name and renamed, like every other generated file. + // Safe for a bind mount because what is mounted is the directory, so the + // replaced file's new inode is still found through it. + var probe strings.Builder + probe.WriteString("set -e\n") + for _, candidate := range trustStoreCandidates { + probe.WriteString("if [ -r " + q(candidate) + " ]; then\n") + probe.WriteString(" cp " + q(candidate) + " " + q(destination+".tmp") + "\n") + probe.WriteString(" chmod 0644 " + q(destination+".tmp") + "\n") + probe.WriteString(" mv " + q(destination+".tmp") + " " + q(destination) + "\n") + probe.WriteString(" echo " + q(candidate) + "\n") + probe.WriteString(" exit 0\n") + probe.WriteString("fi\n") + } + probe.WriteString("exit 1\n") + + res, err := e.T.Run(ctx, probe.String()) + if err != nil { + return err + } + if res.ExitCode != 0 { + return fmt.Errorf( + "service %s: the target holds no certificate authority bundle at any of %s, "+ + "so wal-g cannot verify the backup endpoint from inside the container; "+ + "install the host's CA certificates (on Debian and Ubuntu: apt-get install ca-certificates)", + service, strings.Join(trustStoreCandidates, ", ")) + } + return nil +} + +// QuiesceArchiver waits until PostgreSQL has shipped everything it is holding, +// so the base backup that follows cannot write an object the archiver is about +// to write differently. +// +// Both wal-g and the archive_command upload into the same WAL namespace. When +// `backup-push` lands a segment PostgreSQL has not archived yet, the copy +// PostgreSQL archives afterwards has different bytes and wal-g refuses it — +// "already archived, contents differ". PostgreSQL archives strictly in order, +// so one refused segment stops the chain for good: archived_count never moves +// again, failed_count climbs, and the recovery window quietly stops advancing +// while the command that caused it reported success. +// +// A forced switch first, because the pending set is what matters and a segment +// still being written is never in it. Waiting for the set to empty is then the +// only statement that both writers cannot be about to touch the same name. +// +// A target that cannot drain is refused rather than backed up anyway: the +// alternative is establishing a backup whose chain is already broken, and +// enablement's caller reverts a failure to unprotected, which leaves the +// service as it was found. +func (e *Engine) QuiesceArchiver(ctx context.Context, service string) error { + n := e.names() + exec := "docker exec -u postgres " + q(n.ServiceContainer(service)) + + " psql -U " + q(app.PgSuperuser) + " -d postgres -Atc " + + if res, err := e.T.Run(ctx, exec+q("select pg_switch_wal();")); err != nil { + return err + } else if res.ExitCode != 0 { + return fmt.Errorf("service %s: cannot close the current write-ahead log segment: %s", + service, strings.TrimSpace(res.Stderr)) + } + + pending := exec + q("select count(*) from pg_ls_dir('pg_wal/archive_status') f where f like '%.ready';") + deadline := time.Now().Add(2 * time.Minute) + for { + res, err := e.T.Run(ctx, pending) + if err != nil { + return err + } + if res.ExitCode == 0 && strings.TrimSpace(res.Stdout) == "0" { + return nil + } + if time.Now().After(deadline) { + return fmt.Errorf( + "service %s: the archiver still holds %s write-ahead log segment(s) it has not shipped; "+ + "taking a base backup now would write objects it is about to write differently and stop the chain — "+ + "check `ob backup status %s` and the archive_command before retrying", + service, strings.TrimSpace(res.Stdout), service) + } + e.Opts.Sleep(2 * time.Second) + } +} + +// PostgresSystemIdentifier reads the identity initdb assigned to this data +// directory. It is stable for the life of the cluster and changes when the +// volume is recreated, which is exactly the boundary a WAL repository must +// separate: fresh clusters reuse the same WAL filenames. +func (e *Engine) PostgresSystemIdentifier(ctx context.Context, service string) (string, error) { + n := e.names() + command := "docker exec -u postgres " + q(n.ServiceContainer(service)) + + " psql -U " + q(app.PgSuperuser) + " -d postgres -Atc " + + q("select system_identifier::text from pg_control_system();") + res, err := e.T.Run(ctx, command) + if err != nil { + return "", err + } + if res.ExitCode != 0 { + return "", fmt.Errorf("service %s: cannot read the PostgreSQL system identifier: %s", + service, strings.TrimSpace(res.Stderr)) + } + identifier := strings.TrimSpace(res.Stdout) + if _, err := strconv.ParseUint(identifier, 10, 64); err != nil || identifier == "" { + return "", fmt.Errorf("service %s: PostgreSQL returned an invalid system identifier %q", service, identifier) + } + return identifier, nil +} + func (e *Engine) targetMachine(ctx context.Context) (string, error) { res, err := e.T.Run(ctx, "uname -m") if err != nil { diff --git a/internal/engine/backup_postgres_ops.go b/internal/engine/backup_postgres_ops.go index d969f3da..1a552b49 100644 --- a/internal/engine/backup_postgres_ops.go +++ b/internal/engine/backup_postgres_ops.go @@ -329,9 +329,13 @@ func (e *Engine) BackupStatusFor(ctx context.Context, service string) (BackupSta if err != nil { return BackupStatus{}, err } + repository, err := e.Spec.BackupRepository(service) + if err != nil { + return BackupStatus{}, err + } status := BackupStatus{ Service: service, - Repository: app.WalgPrefix(projection.Target, e.Spec.Spec.Name, service), + Repository: repository, DeclaredWindow: projection.Policy.Retention.Window, DeclaredMaxDataLoss: projection.Policy.MaxDataLoss, } diff --git a/internal/engine/backup_restore.go b/internal/engine/backup_restore.go index ac395ba8..e56c1bf7 100644 --- a/internal/engine/backup_restore.go +++ b/internal/engine/backup_restore.go @@ -110,7 +110,11 @@ func (e *Engine) RecoverService(ctx context.Context, service, targetTime string, if err != nil { return outcome, err } - environment, err := app.WalgEnvironment(target, e.Spec.Spec.Name, service) + repository, err := e.Spec.BackupRepository(service) + if err != nil { + return outcome, err + } + environment, err := app.WalgEnvironment(target, repository, e.Spec.Spec.Name, service) if err != nil { return outcome, err } diff --git a/internal/engine/backup_system_identifier_test.go b/internal/engine/backup_system_identifier_test.go new file mode 100644 index 00000000..0c0f0923 --- /dev/null +++ b/internal/engine/backup_system_identifier_test.go @@ -0,0 +1,44 @@ +package engine + +import ( + "context" + "strings" + "testing" + + "github.com/labstack/onebox/internal/transport" +) + +func TestPostgresSystemIdentifierReadsTheLiveClusterIdentity(t *testing.T) { + fake := &transport.Fake{Dynamic: func(command string) (transport.Result, bool) { + if strings.Contains(command, "pg_control_system()") { + return transport.Result{Stdout: "7513211627332151223\n"}, true + } + return transport.Result{}, false + }} + engine := New(testConfig(), testProject(t), fake, Options{}) + + got, err := engine.PostgresSystemIdentifier(context.Background(), "postgres") + if err != nil { + t.Fatal(err) + } + if got != "7513211627332151223" { + t.Fatalf("system identifier = %q", got) + } + if command := strings.Join(fake.Commands, "\n"); !strings.Contains(command, "docker exec -u postgres") || !strings.Contains(command, "pg_control_system()") { + t.Fatalf("identity was not read from the live PostgreSQL cluster:\n%s", command) + } +} + +func TestPostgresSystemIdentifierRejectsUnreadableOutput(t *testing.T) { + fake := &transport.Fake{Dynamic: func(command string) (transport.Result, bool) { + if strings.Contains(command, "pg_control_system()") { + return transport.Result{Stdout: "not-an-identifier\n"}, true + } + return transport.Result{}, false + }} + engine := New(testConfig(), testProject(t), fake, Options{}) + + if _, err := engine.PostgresSystemIdentifier(context.Background(), "postgres"); err == nil { + t.Fatal("invalid PostgreSQL system identifier was accepted") + } +} diff --git a/internal/engine/backup_trust_store_test.go b/internal/engine/backup_trust_store_test.go new file mode 100644 index 00000000..ed970fa7 --- /dev/null +++ b/internal/engine/backup_trust_store_test.go @@ -0,0 +1,59 @@ +package engine + +import ( + "context" + "regexp" + "strings" + "testing" + + "github.com/labstack/onebox/internal/transport" +) + +// wal-g executes inside the driver's image. `postgres:18` carries no +// certificate authorities, so unless the host's bundle travels with the binary +// every upload to the HTTPS endpoint an s3-compatible target must declare +// fails with "certificate signed by unknown authority" — after the base backup +// has been written and archiving is already on. +func TestStagingTheRuntimeCopiesTheHostTrustStoreInBesideTheBinary(t *testing.T) { + fake := &transport.Fake{} + engine := backupLockTestEngine(fake) + + if err := engine.stageTrustStore(context.Background(), "database"); err != nil { + t.Fatalf("staging the trust store: %v", err) + } + + staged := engine.names().BackupTrustStoreFile("database") + probe := strings.Join(fake.Commands, "\n") + if !strings.Contains(probe, "/etc/ssl/certs/ca-certificates.crt") { + t.Errorf("the Debian and Ubuntu bundle was never looked for:\n%s", probe) + } + if !strings.Contains(probe, staged) { + t.Errorf("nothing was copied to %s:\n%s", staged, probe) + } + // Written to a temporary name and renamed, like every other generated + // file. The mount is of the directory, so the new inode is still reachable. + if !strings.Contains(probe, staged+".tmp") { + t.Errorf("the bundle was written in place rather than renamed over:\n%s", probe) + } +} + +// A target with no bundle anywhere is refused while the service is still +// exactly as it was. Staging nothing and letting the wrapper fall back to the +// image's empty store reproduces the original failure, only a quarter of an +// hour later and with the database already archiving. +func TestATargetWithNoTrustStoreIsRefusedBeforeArchivingIsTurnedOn(t *testing.T) { + fake := &transport.Fake{Script: []transport.Rule{ + {Match: regexp.MustCompile("ca-certificates|ca-bundle|cert.pem"), Result: transport.Result{ExitCode: 1}}, + }} + engine := backupLockTestEngine(fake) + + err := engine.stageTrustStore(context.Background(), "database") + if err == nil { + t.Fatal("a target with no certificate authorities was accepted") + } + for _, want := range []string{"/etc/ssl/certs/ca-certificates.crt", "ca-certificates"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("the error does not tell the operator what to install (%q): %v", want, err) + } + } +} diff --git a/internal/onebox/backup_enable.go b/internal/onebox/backup_enable.go index 80600d02..3f7b41b7 100644 --- a/internal/onebox/backup_enable.go +++ b/internal/onebox/backup_enable.go @@ -3,8 +3,10 @@ package onebox import ( "context" "encoding/json" + "errors" "fmt" "path/filepath" + "time" "github.com/labstack/onebox/internal/app" "github.com/labstack/onebox/internal/engine" @@ -161,6 +163,11 @@ func executeBackupEnable(ctx context.Context, e *engine.Engine, resolved *app.Re if err != nil { return err } + systemIdentifier, err := e.PostgresSystemIdentifier(ctx, service) + if err != nil { + return err + } + repositoryGeneration := backupRepositoryGeneration(current, projection, resolved.Spec.Name, service, systemIdentifier) // Rebound every time, including when the service is already enabled. // // `ob backup enable` is the one command that binds a service to a @@ -169,7 +176,7 @@ func executeBackupEnable(ctx context.Context, e *engine.Engine, resolved *app.Re // discarded the freshly pinned image and the new projection, so the service // went on archiving to the original repository with no command able to // change it — while the edited project sat there looking applied. - next, err := rebindBackup(current, projection, image, declaredImage, operationID) + next, err := rebindBackup(current, projection, image, declaredImage, operationID, systemIdentifier, repositoryGeneration) if err != nil { return err } @@ -202,9 +209,10 @@ func executeBackupEnable(ctx context.Context, e *engine.Engine, resolved *app.Re // endpoint inside a target called "offsite" moves the history just as // surely as pointing the service at a target called something else, and the // name would not have changed. + nextRepository := app.WalgPrefix(projection.Target, resolved.Spec.Name, service, next.BackupRepositoryGeneration) if previous := previousBackupRepository(current, resolved.Spec.Name, service); previous != "" && - previous != app.WalgPrefix(projection.Target, resolved.Spec.Name, service) { - e.ReportTargetMoved(service, previous, app.WalgPrefix(projection.Target, resolved.Spec.Name, service)) + previous != nextRepository { + e.ReportTargetMoved(service, previous, nextRepository) // The credential file is named for the target it belongs to, so a move // to a *differently named* target leaves the old file holding keys // nothing uses. Editing the bucket inside a target keeps the name, and @@ -217,7 +225,89 @@ func executeBackupEnable(ctx context.Context, e *engine.Engine, resolved *app.Re } } } - return e.BackupService(ctx, service) + // Everything PostgreSQL is holding goes to the repository before the base + // backup writes into the same WAL namespace. Without this, `backup-push` + // can land a segment the archiver has not shipped yet, the archiver's own + // copy is then refused as "already archived, contents differ", and because + // PostgreSQL archives in order the chain stops there permanently — while + // this command reports success. + if err := e.QuiesceArchiver(ctx, service); err != nil { + if current.State != BackupEnabled { + if revertErr := revertFailedEnablement(ctx, e, service, next, operationID); revertErr != nil { + return errors.Join(err, revertErr) + } + } + return err + } + + // The base backup is the last step, and until it exists the service is + // archiving with nothing to replay onto. + // + // A failure here used to be returned as-is, which left `archive_mode=on` + // and an archive_command that could not reach the repository — so + // PostgreSQL retained every WAL segment it could not ship, indefinitely, + // established by a command that reported failure. On a busy database that + // is an unbounded disk commitment nobody agreed to. + if err := e.BackupService(ctx, service); err != nil { + // Only when this run is what turned archiving on. A service that was + // already protected was archiving before this command ran and to its + // own repository; taking it back to unprotected because a re-enable + // could not take a fresh base backup would break a working setup to + // tidy up a failed change. Either way the service is left as it was + // found. + if current.State == BackupEnabled { + return err + } + if revertErr := revertFailedEnablement(ctx, e, service, next, operationID); revertErr != nil { + return errors.Join(err, fmt.Errorf( + "service %s is archiving with no base backup and could not be returned to unprotected — "+ + "write-ahead log will accumulate until `ob backup disable %s` succeeds: %w", + service, service, revertErr)) + } + return err + } + return nil +} + +// revertFailedEnablement takes a service back to unprotected after enablement +// turned archiving on but could not complete a base backup. +// +// It walks the same two transitions `ob backup disable` walks, for the same +// reason: pending is written first, so a run interrupted during the restart +// leaves a record saying the decision was made and the work is unfinished, +// rather than one claiming the service stopped archiving while it still is. +// +// The credential file installed by this run is deliberately left in place. The +// operator's next move is to fix the cause and re-run enable, which reinstalls +// it anyway, and removing it makes the failure harder to diagnose than the +// unused keys are worth. +func revertFailedEnablement(ctx context.Context, e *engine.Engine, service string, enabled BackupLifecycleState, operationID string) error { + pending, next, err := disablementAfterFailedEnablement(enabled, operationID, time.Now()) + if err != nil { + return err + } + body, err := encodeBackupLifecycleState(pending) + if err != nil { + return err + } + if err := e.WriteBackupLifecycleState(ctx, service, body); err != nil { + return err + } + if err := e.RebindServiceRuntimeStates(map[string]app.ServiceRuntimeState{ + service: next.RuntimeState(), + }); err != nil { + return err + } + // Restarts the server without archive_mode and removes the timers this + // enablement installed, because SyncBackupSchedules removes what is no + // longer protected. + if err := e.ApplyServices(ctx); err != nil { + return fmt.Errorf("service %s could not restart without backup: %w", service, err) + } + if body, err = encodeBackupLifecycleState(next); err != nil { + return err + } + return e.WriteBackupLifecycleState(ctx, service, body) } // encodeBackupLifecycleState renders a validated record as the single JSON @@ -263,7 +353,7 @@ func currentBackupLifecycleState(ctx context.Context, e *engine.Engine, applicat // never-enabled first, because EnableBackup is the single place that // decides what an enabled record contains and it refuses to transition from // enabled — the alternative is a second, divergent copy of that logic. -func rebindBackup(current BackupLifecycleState, projection app.BackupEffectiveProjection, image, imageReference, operationID string) (BackupLifecycleState, error) { +func rebindBackup(current BackupLifecycleState, projection app.BackupEffectiveProjection, image, imageReference, operationID, systemIdentifier, repositoryGeneration string) (BackupLifecycleState, error) { source := current if current.State == BackupEnabled { source.State = BackupDisabled @@ -278,7 +368,35 @@ func rebindBackup(current BackupLifecycleState, projection app.BackupEffectivePr return BackupLifecycleState{}, err } } - return EnableBackup(source, projection, image, imageReference, operationID, true, current.Epoch+1) + next, err := EnableBackup(source, projection, image, imageReference, operationID, true, current.Epoch+1) + if err != nil { + return BackupLifecycleState{}, err + } + next.DatabaseSystemIdentifier = systemIdentifier + next.BackupRepositoryGeneration = repositoryGeneration + if err := next.Seal(); err != nil { + return BackupLifecycleState{}, err + } + return next, nil +} + +// backupRepositoryGeneration keeps an established binding only while both the +// database and target repository root are provably unchanged. A legacy record +// has no database identity, so it cannot make that proof: its next explicit +// enable starts a cluster-scoped generation and leaves the old repository +// untouched. Guessing that it is the same database is exactly how a lifecycle +// record surviving volume replacement would recreate the collision this fixes. +func backupRepositoryGeneration(current BackupLifecycleState, projection app.BackupEffectiveProjection, application, service, systemIdentifier string) string { + if current.LastEffective == nil { + return systemIdentifier + } + previousRoot := app.WalgPrefix(current.LastEffective.Target, application, service, "") + nextRoot := app.WalgPrefix(projection.Target, application, service, "") + sameDatabase := current.DatabaseSystemIdentifier != "" && current.DatabaseSystemIdentifier == systemIdentifier + if previousRoot == nextRoot && sameDatabase { + return current.BackupRepositoryGeneration + } + return systemIdentifier } // previousBackupRepository is the repository a service was last archiving to, @@ -287,7 +405,7 @@ func previousBackupRepository(state BackupLifecycleState, application, service s if state.LastEffective == nil { return "" } - return app.WalgPrefix(state.LastEffective.Target, application, service) + return app.WalgPrefix(state.LastEffective.Target, application, service, state.BackupRepositoryGeneration) } // retiresCredentialFile reports whether the previous binding left a credential @@ -301,3 +419,23 @@ func previousBackupRepository(state BackupLifecycleState, application, service s func retiresCredentialFile(previous *app.BackupEffectiveProjection, next app.BackupEffectiveProjection) bool { return previous != nil && previous.Policy.Target != next.Policy.Target } + +// disablementAfterFailedEnablement computes the two records the revert writes: +// the pending one that covers the restart, and the disabled one that replaces +// it once the restart has happened. +// +// Separated from the writing so the transition can be judged without a target: +// what matters is that the epoch never repeats or goes backwards — it is the +// fence, and an operation launched against the state this run is leaving must +// not still be accepted against the state it is leaving it in. +func disablementAfterFailedEnablement(enabled BackupLifecycleState, operationID string, now time.Time) (pending, disabled BackupLifecycleState, err error) { + pending, err = BeginBackupDisable(enabled, operationID, now, enabled.Epoch+1) + if err != nil { + return BackupLifecycleState{}, BackupLifecycleState{}, err + } + disabled, err = DisableBackup(pending, operationID, pending.Epoch+1) + if err != nil { + return BackupLifecycleState{}, BackupLifecycleState{}, err + } + return pending, disabled, nil +} diff --git a/internal/onebox/backup_enable_test.go b/internal/onebox/backup_enable_test.go index 0a3414c5..d9464fbd 100644 --- a/internal/onebox/backup_enable_test.go +++ b/internal/onebox/backup_enable_test.go @@ -1,7 +1,9 @@ package onebox import ( + "strings" "testing" + "time" "github.com/labstack/onebox/internal/app" ) @@ -21,3 +23,87 @@ func TestOnlyARenamedTargetRetiresItsCredentialFile(t *testing.T) { t.Fatal("a first enablement retired a credential file that never existed") } } + +// An enablement that turns archiving on and then cannot take a base backup +// must leave the service unprotected, not archiving into a repository it never +// reached. PostgreSQL retains every segment an archive_command cannot ship, so +// the alternative is unbounded disk growth established by a command that +// reported failure. +func TestAFailedEnablementIsRevertedToUnprotected(t *testing.T) { + fresh, err := NewBackupLifecycleState("shop", "production", "postgres", 1) + if err != nil { + t.Fatal(err) + } + enabled, err := EnableBackup(fresh, backupStateProjection(), + "postgres@sha256:"+strings.Repeat("a", 64), "postgres:18", "op-1", true, 2) + if err != nil { + t.Fatal(err) + } + + pending, disabled, err := disablementAfterFailedEnablement(enabled, "op-1", time.Now()) + if err != nil { + t.Fatalf("a freshly enabled service could not be taken back: %v", err) + } + + if pending.State != BackupDisablePending { + t.Errorf("the restart is not covered by a pending record: state is %v", pending.State) + } + if disabled.State != BackupDisabled { + t.Errorf("the service was left protected: state is %v", disabled.State) + } + // The epoch is the fence. Repeating or lowering it would let an operation + // launched against the enabled state still be accepted afterwards. + if !(enabled.Epoch < pending.Epoch && pending.Epoch < disabled.Epoch) { + t.Errorf("epochs do not strictly increase: enabled=%d pending=%d disabled=%d", + enabled.Epoch, pending.Epoch, disabled.Epoch) + } + // What the next render binds. An unprotected runtime is what makes + // ApplyServices restart the server without archive_mode. + if state := disabled.RuntimeState().BackupState; state == string(BackupEnabled) { + t.Errorf("the reverted runtime still renders a server with archiving on: %q", state) + } +} + +func TestRepositoryGenerationFollowsTheDatabaseCluster(t *testing.T) { + projection := backupStateProjection() + firstID := "7513211627332151223" + secondID := "7513211627332151224" + + fresh, err := NewBackupLifecycleState("shop", "production", "postgres", 1) + if err != nil { + t.Fatal(err) + } + if got := backupRepositoryGeneration(fresh, projection, "shop", "postgres", firstID); got != firstID { + t.Fatalf("new cluster generation = %q, want %q", got, firstID) + } + + // An old record has no database identity and used the unversioned layout. + // Its first explicit enable cannot prove the data volume is the old one, so + // it starts a scoped generation and leaves the legacy history untouched. + legacy := fresh + legacy.LastEffective = &projection + if err := legacy.Seal(); err != nil { + t.Fatal(err) + } + if got := backupRepositoryGeneration(legacy, projection, "shop", "postgres", firstID); got != firstID { + t.Fatalf("legacy repository generation = %q, want current cluster %q", got, firstID) + } + + bound := legacy + bound.DatabaseSystemIdentifier = firstID + if err := bound.Seal(); err != nil { + t.Fatal(err) + } + if got := backupRepositoryGeneration(bound, projection, "shop", "postgres", firstID); got != "" { + t.Fatalf("unchanged legacy cluster moved to generation %q", got) + } + if got := backupRepositoryGeneration(bound, projection, "shop", "postgres", secondID); got != secondID { + t.Fatalf("replacement cluster generation = %q, want %q", got, secondID) + } + + moved := projection + moved.Target.Bucket = "different-bucket" + if got := backupRepositoryGeneration(bound, moved, "shop", "postgres", firstID); got != firstID { + t.Fatalf("moved target generation = %q, want current cluster %q", got, firstID) + } +} diff --git a/internal/onebox/backup_state.go b/internal/onebox/backup_state.go index 3f73c5cd..64cc33a6 100644 --- a/internal/onebox/backup_state.go +++ b/internal/onebox/backup_state.go @@ -70,20 +70,24 @@ type BackupLifecycleState struct { PrerequisiteEffective bool `json:"prerequisite_effective"` LocalSupportInstalled bool `json:"local_support_installed"` LastEffective *app.BackupEffectiveProjection `json:"last_effective,omitempty"` + DatabaseSystemIdentifier string `json:"database_system_identifier,omitempty"` + BackupRepositoryGeneration string `json:"backup_repository_generation,omitempty"` Schedules []BackupScheduleState `json:"schedules,omitempty"` StateDigest string `json:"state_digest"` } type BackupLifecycleStatus struct { - State BackupState `json:"state"` - Phase BackupDisablePhase `json:"phase"` - RequestedAt string `json:"requested_at,omitempty"` - ActionDeadline string `json:"action_deadline,omitempty"` - Elapsed string `json:"elapsed,omitempty"` - Schedules []BackupScheduleState `json:"schedules,omitempty"` - StorageContinues bool `json:"storage_continues"` - ResolvingCommand string `json:"resolving_command,omitempty"` - Failure *LifecycleFailure `json:"failure,omitempty"` + State BackupState `json:"state"` + Phase BackupDisablePhase `json:"phase"` + RequestedAt string `json:"requested_at,omitempty"` + ActionDeadline string `json:"action_deadline,omitempty"` + Elapsed string `json:"elapsed,omitempty"` + Schedules []BackupScheduleState `json:"schedules,omitempty"` + StorageContinues bool `json:"storage_continues"` + ResolvingCommand string `json:"resolving_command,omitempty"` + Repository string `json:"repository,omitempty"` + DatabaseSystemIdentifier string `json:"database_system_identifier,omitempty"` + Failure *LifecycleFailure `json:"failure,omitempty"` } func NewBackupLifecycleState(application, environment, service string, epoch int) (BackupLifecycleState, error) { @@ -131,8 +135,10 @@ func EnableBackup(current BackupLifecycleState, projection app.BackupEffectivePr func (state BackupLifecycleState) RuntimeState() app.ServiceRuntimeState { return app.ServiceRuntimeState{ BackupState: string(state.State), ServiceImage: state.ServiceImage, - PublicationVerified: state.ServiceImagePublicationVerified, - LastEffective: cloneBackupProjection(state.LastEffective), + PublicationVerified: state.ServiceImagePublicationVerified, + LastEffective: cloneBackupProjection(state.LastEffective), + DatabaseSystemIdentifier: state.DatabaseSystemIdentifier, + BackupRepositoryGeneration: state.BackupRepositoryGeneration, } } @@ -140,7 +146,13 @@ func (state BackupLifecycleState) Status(now time.Time) (BackupLifecycleStatus, if err := state.Validate(); err != nil { return BackupLifecycleStatus{}, err } - status := BackupLifecycleStatus{State: state.State, Phase: state.Phase, Schedules: append([]BackupScheduleState(nil), state.Schedules...)} + status := BackupLifecycleStatus{ + State: state.State, Phase: state.Phase, Schedules: append([]BackupScheduleState(nil), state.Schedules...), + DatabaseSystemIdentifier: state.DatabaseSystemIdentifier, + } + if state.LastEffective != nil { + status.Repository = app.WalgPrefix(state.LastEffective.Target, state.Application, state.Service, state.BackupRepositoryGeneration) + } for _, schedule := range state.Schedules { if schedule.Active { status.StorageContinues = true @@ -216,6 +228,12 @@ func (state BackupLifecycleState) validateContent() error { if state.ServiceImage != "" && !backedUpRuntimeImage.MatchString(state.ServiceImage) { return errors.New("protected service image must be digest-pinned") } + if state.DatabaseSystemIdentifier != "" && !databaseSystemIdentifier.MatchString(state.DatabaseSystemIdentifier) { + return errors.New("database system identifier is invalid") + } + if state.BackupRepositoryGeneration != "" && state.BackupRepositoryGeneration != state.DatabaseSystemIdentifier { + return errors.New("backup repository generation must match the database system identifier") + } if state.State == BackupEnabled || state.State == BackupDisablePending { if state.LastEffective == nil || state.ServiceImage == "" || !state.ServiceImagePublicationVerified { return errors.New("active backup state requires last-effective intent and a provenance-verified service image") @@ -244,6 +262,8 @@ func (state BackupLifecycleState) validateContent() error { return nil } +var databaseSystemIdentifier = regexp.MustCompile(`^[0-9]{1,20}$`) + func (state BackupLifecycleState) computeDigest() (string, error) { copy := state copy.StateDigest = "" diff --git a/internal/onebox/backup_state_test.go b/internal/onebox/backup_state_test.go index f7671aa6..53ecd403 100644 --- a/internal/onebox/backup_state_test.go +++ b/internal/onebox/backup_state_test.go @@ -178,7 +178,7 @@ func TestReEnablingAnEnabledServiceIsNotRefusedAsCorruptState(t *testing.T) { t.Fatal(err) } - again, err := rebindBackup(enabled, backupStateProjection(), pin, "postgres:18", "op-2") + again, err := rebindBackup(enabled, backupStateProjection(), pin, "postgres:18", "op-2", "7513211627332151223", "7513211627332151223") if err != nil { t.Fatalf("re-enabling an enabled service: %v", err) } @@ -263,7 +263,7 @@ func TestADisablementLeftPendingCanBeFinishedOrAbandoned(t *testing.T) { t.Fatalf("finished state = %q, want disabled", finished.State) } - abandoned, err := rebindBackup(pending, backupStateProjection(), pin, "postgres:18", "op-3") + abandoned, err := rebindBackup(pending, backupStateProjection(), pin, "postgres:18", "op-3", "7513211627332151223", "7513211627332151223") if err != nil { t.Fatalf("re-enabling out of a pending disablement: %v", err) } From 261cd18bc9e2b39b2cc9d0b711d8e93b5ffcd6c0 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Fri, 21 Aug 2026 22:02:14 -0700 Subject: [PATCH 4/7] docs(backup): explain cluster-scoped repositories and recovery ordering The repository path now carries the cluster's system identifier, so a rebuilt database begins its own history instead of colliding with the old one. That is a visible change in where backups live, and it makes the order of a recovery matter: recovering first keeps the history, because a recovered cluster keeps the identifier it was backed up with, while enabling first binds the service to a new cluster and leaves the old backups in the bucket but out of reach of `ob backup restore`. The stopping section said re-enabling is how you reach a repository again, which stays true for the same database and is the wrong instruction after the volume has been replaced. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DF9dqmoT7gsSk7TfzL2WQS --- .../docs/guides/back-up-a-database.mdx | 41 +++++++++++++++++++ site/src/content/docs/reference/cli.mdx | 17 ++++---- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/site/src/content/docs/guides/back-up-a-database.mdx b/site/src/content/docs/guides/back-up-a-database.mdx index a7a87191..0cbbf601 100644 --- a/site/src/content/docs/guides/back-up-a-database.mdx +++ b/site/src/content/docs/guides/back-up-a-database.mdx @@ -160,6 +160,25 @@ Everything else — a restore, a drill, status, retention — keeps reading the repository the service was actually bound to, so editing a target cannot silently point a recovery at a repository the history is not in. +A repository also moves when the *database* is replaced, without anything in the +project changing. Each repository is scoped to the cluster that wrote it, using +PostgreSQL's own system identifier: + +``` +s3://onebox-backups/shop_database/clusters/7513211627332151223 +``` + +That is not decoration. Every fresh cluster starts its write-ahead log at +`000000010000000000000001`, so a new database archiving where an old one left +off would write object names that already exist with different contents. The +archiver refuses to overwrite them, and because PostgreSQL archives strictly in +order, one refused segment stops the chain permanently. Scoping by cluster means +a rebuilt database simply begins its own history, and the previous one stays +exactly where it is. + +`ob backup status` prints both the repository and the system identifier, which is +how you tell two histories apart when a service has been rebuilt. + ## Prove it restores ```console @@ -204,6 +223,23 @@ replay only moves forward, so a later base can never reach an earlier moment. A point older than everything in the repository is refused, and the refusal tells you when the oldest backup finished. +:::caution[After losing the volume, recover before you re-enable] +A recovery reads the repository the service is *recorded* as bound to, and a +recovered cluster keeps the system identifier it was backed up with — so +recovering first and enabling afterwards keeps the same history, and the service +carries on where it left off. + +Enabling first does not. `ob backup enable` binds the service to whatever cluster +is running, and a database rebuilt from an empty volume is a new cluster with a +new identifier, so enablement starts a new repository. The old backups are +untouched and still in the bucket, but the service is no longer pointed at them +and `ob backup restore` will not find them. + +If that has already happened, the history is not lost: `ob backup status` gives +the identifier the service is bound to now, and the previous generation is beside +it under `clusters/`. +::: + ## Stopping ```console @@ -215,6 +251,11 @@ destination credentials are removed from the host. **The repository is not touched** — but reading or recovering from it needs backup enabled again, because the tooling and credentials that reach it live in the protected service. +Re-enabling the *same* database returns to the same repository: the cluster's +identifier has not changed, so neither has its generation. Re-enabling after the +volume has been replaced does not, for the reason in the caution above — recover +first if what you want is the old history. + ## What is not covered Only the `postgres` driver has an executable contract, on versions 17 and 18. diff --git a/site/src/content/docs/reference/cli.mdx b/site/src/content/docs/reference/cli.mdx index 35229a49..052b9b12 100644 --- a/site/src/content/docs/reference/cli.mdx +++ b/site/src/content/docs/reference/cli.mdx @@ -273,8 +273,9 @@ Usage: ob backup drill [flags] Flags: - -h, --help help for drill - --to string RFC 3339 point in time to prove recoverable (default: the newest recoverable point) + --generation string repository generation to prove (PostgreSQL system identifier or legacy; default: current) + -h, --help help for drill + --to string RFC 3339 point in time to prove recoverable (default: the newest recoverable point) Global Flags: -c, --config string path to the project YAML file (default "ob.yml") @@ -361,10 +362,11 @@ Usage: ob backup restore [flags] Flags: - --break-lock break a stale operation lock after inspecting its holder - --confirm string name of the service whose live data may be replaced - -h, --help help for restore - --to string RFC 3339 point in time to recover to (default: the newest recoverable point) + --break-lock break a stale operation lock after inspecting its holder + --confirm string name of the service whose live data may be replaced + --generation string repository generation to recover (PostgreSQL system identifier or legacy; default: current) + -h, --help help for restore + --to string RFC 3339 point in time to recover to (default: the newest recoverable point) Global Flags: -c, --config string path to the project YAML file (default "ob.yml") @@ -386,7 +388,8 @@ Usage: ob backup status [flags] Flags: - -h, --help help for status + --generation string repository generation to inspect (PostgreSQL system identifier or legacy; default: current) + -h, --help help for status Global Flags: -c, --config string path to the project YAML file (default "ob.yml") From ae09c993be67d373a939ed945544992cf64abfda Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Fri, 21 Aug 2026 22:33:14 -0700 Subject: [PATCH 5/7] fix(backup): preserve repository generations Discover generations in the backup target after local state loss. Reject stale live bindings and restore the exact prior binding when re-enable fails. Also make archiver polling fail promptly and harden the server E2E harness. --- .github/workflows/e2e-server.yml | 4 + cmd/ob/backup.go | 17 ++- e2e/server_probe_test.go | 10 ++ internal/app/service_image_state.go | 8 ++ internal/engine/backup_identity_test.go | 132 ++++++++++++++++++++++++ internal/engine/backup_postgres.go | 81 ++++++++++++++- internal/engine/backup_postgres_ops.go | 91 +++++++++++++--- internal/engine/backup_restore.go | 49 +++++++-- internal/engine/services.go | 23 +++++ internal/onebox/backup_enable.go | 88 ++++++++-------- internal/onebox/backup_read.go | 8 +- internal/onebox/backup_restore.go | 63 ++++++++++- internal/onebox/execute.go | 2 +- internal/onebox/execution_types.go | 8 +- scripts/lima.sh | 8 +- 15 files changed, 520 insertions(+), 72 deletions(-) create mode 100644 internal/engine/backup_identity_test.go diff --git a/.github/workflows/e2e-server.yml b/.github/workflows/e2e-server.yml index fbcdcd72..a089ee72 100644 --- a/.github/workflows/e2e-server.yml +++ b/.github/workflows/e2e-server.yml @@ -13,6 +13,8 @@ on: - "internal/transport/**" - "internal/onebox/**" - "internal/app/backup_*" + - "internal/app/names.go" + - "internal/app/service_image_state.go" - "cmd/ob/**" - "e2e/**" - "scripts/lima.sh" @@ -23,6 +25,8 @@ on: - "internal/transport/**" - "internal/onebox/**" - "internal/app/backup_*" + - "internal/app/names.go" + - "internal/app/service_image_state.go" - "cmd/ob/**" - "e2e/**" - "scripts/lima.sh" diff --git a/cmd/ob/backup.go b/cmd/ob/backup.go index 944b5822..5bb99de5 100644 --- a/cmd/ob/backup.go +++ b/cmd/ob/backup.go @@ -148,7 +148,7 @@ func addBackupCommands(root *cobra.Command, g *globalFlags) { } backupCmd.AddCommand(verifyCmd) - var restoreTo, restoreConfirm string + var restoreTo, restoreConfirm, restoreGeneration string var restoreBreakLock bool restoreCmd := &cobra.Command{ Use: "restore ", @@ -177,16 +177,17 @@ func addBackupCommands(root *cobra.Command, g *globalFlags) { } return runMutation(cmd, g, onebox.ExecuteRequest{ Kind: onebox.KindRestoreCutover, Service: args[0], - RecoveryTarget: restoreTo, BreakLock: restoreBreakLock, + RecoveryTarget: restoreTo, RecoveryGeneration: restoreGeneration, BreakLock: restoreBreakLock, }, "backup restore") }, } restoreCmd.Flags().StringVar(&restoreTo, "to", "", "RFC 3339 point in time to recover to (default: the newest recoverable point)") + restoreCmd.Flags().StringVar(&restoreGeneration, "generation", "", "repository generation to recover (PostgreSQL system identifier or legacy; default: current)") restoreCmd.Flags().StringVar(&restoreConfirm, "confirm", "", "name of the service whose live data may be replaced") restoreCmd.Flags().BoolVar(&restoreBreakLock, "break-lock", false, "break a stale operation lock after inspecting its holder") backupCmd.AddCommand(restoreCmd) - var drillTo string + var drillTo, drillGeneration string drillCmd := &cobra.Command{ Use: "drill ", Short: "prove the repository recovers, without touching anything", @@ -199,13 +200,15 @@ func addBackupCommands(root *cobra.Command, g *globalFlags) { Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { return runMutation(cmd, g, onebox.ExecuteRequest{ - Kind: onebox.KindRestoreTest, Service: args[0], RecoveryTarget: drillTo, + Kind: onebox.KindRestoreTest, Service: args[0], RecoveryTarget: drillTo, RecoveryGeneration: drillGeneration, }, "backup drill") }, } drillCmd.Flags().StringVar(&drillTo, "to", "", "RFC 3339 point in time to prove recoverable (default: the newest recoverable point)") + drillCmd.Flags().StringVar(&drillGeneration, "generation", "", "repository generation to prove (PostgreSQL system identifier or legacy; default: current)") backupCmd.AddCommand(drillCmd) + var statusGeneration string statusCmd := &cobra.Command{ Use: "status ", Short: "what the repository can recover, read from the repository", @@ -215,7 +218,7 @@ func addBackupCommands(root *cobra.Command, g *globalFlags) { "policy is declared but never enabled has no repository to ask, and says so.", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - status, err := operationsService(cmd, g).BackupStatus(cmd.Context(), args[0]) + status, err := operationsService(cmd, g).BackupStatusGeneration(cmd.Context(), args[0], statusGeneration) if err != nil { return err } @@ -226,6 +229,9 @@ func addBackupCommands(root *cobra.Command, g *globalFlags) { } out := cmd.OutOrStdout() fmt.Fprintf(out, "service %s\nrepository %s\n", status.Service, status.Repository) + for _, generation := range status.AvailableRepositoryGenerations { + fmt.Fprintf(out, "generation %s\n", generation) + } for _, issue := range status.RuntimeIssues { fmt.Fprintf(out, "drift %s\n", issue) } @@ -260,6 +266,7 @@ func addBackupCommands(root *cobra.Command, g *globalFlags) { return w.Flush() }, } + statusCmd.Flags().StringVar(&statusGeneration, "generation", "", "repository generation to inspect (PostgreSQL system identifier or legacy; default: current)") backupCmd.AddCommand(statusCmd) root.AddCommand(backupCmd) diff --git a/e2e/server_probe_test.go b/e2e/server_probe_test.go index 75053f3d..8435b211 100644 --- a/e2e/server_probe_test.go +++ b/e2e/server_probe_test.go @@ -157,12 +157,22 @@ func TestServerProbes(t *testing.T) { first := s.project(t, endpoint, "v1") s.deploy(t, first) s.mustOb(t, first, "backup", "enable", "postgres") + firstID := s.psql(t, "select system_identifier from pg_control_system()") s.rotateWAL(t) s.teardown(t, first) second := s.project(t, endpoint, "v1") s.deploy(t, second) s.mustOb(t, second, "backup", "enable", "postgres") + secondID := s.psql(t, "select system_identifier from pg_control_system()") + if firstID == secondID { + t.Fatalf("replacement database kept system identifier %s", firstID) + } + status := s.mustOb(t, second, "backup", "status", "postgres") + if !strings.Contains(status, firstID) || !strings.Contains(status, secondID) { + t.Fatalf("repository generations are not discoverable after local state loss:\n%s", status) + } + s.mustOb(t, second, "backup", "drill", "postgres", "--generation", firstID) before, _ := s.archiverCounts(t) s.rotateWAL(t) diff --git a/internal/app/service_image_state.go b/internal/app/service_image_state.go index 02882192..3675bb91 100644 --- a/internal/app/service_image_state.go +++ b/internal/app/service_image_state.go @@ -43,6 +43,14 @@ type ServiceImageSelection struct { Origin Origin } +// ServiceRuntimeState returns the observed lifecycle binding for one service. +// Engine safety checks need the recorded database identity before they allow +// Compose to touch its volume. +func (r *Resolved) ServiceRuntimeState(service string) (ServiceRuntimeState, bool) { + state, ok := r.serviceRuntime[service] + return state, ok +} + func (r *Resolved) WithServiceRuntimeStates(states map[string]ServiceRuntimeState) (*Resolved, error) { if r == nil { return nil, errors.New("resolved project is nil") diff --git a/internal/engine/backup_identity_test.go b/internal/engine/backup_identity_test.go new file mode 100644 index 00000000..3ade3012 --- /dev/null +++ b/internal/engine/backup_identity_test.go @@ -0,0 +1,132 @@ +package engine + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/labstack/onebox/internal/app" + "github.com/labstack/onebox/internal/transport" +) + +const protectedPostgresProject = `api_version: onebox.run/v1 +app: shop +environments: + production: {server: deploy@example.net} +workloads: + web: {image: nginx:1} +backup_targets: + offsite: + kind: s3-compatible + endpoint: https://objects.example.net + bucket: backups + failure_domain: {identity: remote, host: objects.example.net} + credentials: + file: backup.env + provider: sops + access_key_entry: ACCESS_KEY + secret_key_entry: SECRET_KEY + encryption: {pitr: client-side} +services: + postgres: + version: 17 + backup: {target: offsite, recovery_kind: pitr, max_data_loss: 15m} +` + +func protectedPostgresResolved(t *testing.T, identifier string) *app.Resolved { + t.Helper() + spec, err := app.LoadBytes([]byte(protectedPostgresProject), "ob.yml") + if err != nil { + t.Fatal(err) + } + resolved, err := spec.Resolve("production") + if err != nil { + t.Fatal(err) + } + projection, err := resolved.DeclaredBackupProjection("postgres") + if err != nil { + t.Fatal(err) + } + return mustRuntimeState(t, resolved, app.ServiceRuntimeState{ + BackupState: "enabled", ServiceImage: "postgres@sha256:" + strings.Repeat("a", 64), + PublicationVerified: true, DigestAvailable: true, LastEffective: &projection, + DatabaseSystemIdentifier: identifier, BackupRepositoryGeneration: identifier, + }) +} + +func mustRuntimeState(t *testing.T, resolved *app.Resolved, state app.ServiceRuntimeState) *app.Resolved { + t.Helper() + bound, err := resolved.WithServiceRuntimeStates(map[string]app.ServiceRuntimeState{"postgres": state}) + if err != nil { + t.Fatal(err) + } + return bound +} + +func TestProtectedDatabaseIdentityRejectsMissingOrReplacedVolume(t *testing.T) { + const recorded = "7513211627332151223" + for _, tt := range []struct { + name, actual, want string + missing bool + }{ + {name: "missing", missing: true, want: "data volume ob_shop_postgres_data is missing"}, + {name: "replaced", actual: "7513211627332151224", want: "belongs to PostgreSQL cluster 7513211627332151224"}, + {name: "same", actual: recorded}, + } { + t.Run(tt.name, func(t *testing.T) { + fake := &transport.Fake{Dynamic: func(command string) (transport.Result, bool) { + switch { + case strings.Contains(command, "docker volume inspect"): + if tt.missing { + return transport.Result{ExitCode: 1, Stderr: "not found"}, true + } + return transport.Result{}, true + case strings.Contains(command, "pg_controldata"): + return transport.Result{Stdout: "Database system identifier: " + tt.actual + "\n"}, true + } + return transport.Result{}, false + }} + e := New(protectedPostgresResolved(t, recorded), testProject(t), fake, Options{Environment: "production"}) + err := e.ValidateProtectedDatabaseIdentities(context.Background()) + if tt.want == "" && err != nil { + t.Fatalf("validate: %v", err) + } + if tt.want != "" && (err == nil || !strings.Contains(err.Error(), tt.want)) { + t.Fatalf("error = %v, want %q", err, tt.want) + } + }) + } +} + +func TestQuiesceArchiverReturnsPollingFailureImmediately(t *testing.T) { + fake := &transport.Fake{Dynamic: func(command string) (transport.Result, bool) { + if strings.Contains(command, "pg_switch_wal") { + return transport.Result{Stdout: "0/1"}, true + } + if strings.Contains(command, "archive_status") { + return transport.Result{ExitCode: 2, Stderr: "connection lost"}, true + } + return transport.Result{}, false + }} + sleeps := 0 + e := New(testConfig(), testProject(t), fake, Options{Sleep: func(_ time.Duration) { sleeps++ }, Environment: "production"}) + err := e.QuiesceArchiver(context.Background(), "postgres") + if err == nil || !strings.Contains(err.Error(), "connection lost") { + t.Fatalf("error = %v", err) + } + if sleeps != 0 { + t.Fatalf("poll failure slept %d times", sleeps) + } +} + +func TestRepositoryGenerationDiscovery(t *testing.T) { + listing := "dir 0 0001-01-01 00:00:00 +0000 UTC clusters/\n" + + "dir 0 0001-01-01 00:00:00 +0000 UTC 7513211627332151224/\n" + + "dir 0 0001-01-01 00:00:00 +0000 UTC 7513211627332151223/\n" + + "dir 0 0001-01-01 00:00:00 +0000 UTC basebackups_005/\n" + got := strings.Join(parseRepositoryGenerations(listing), ",") + if want := "7513211627332151223,7513211627332151224,legacy"; got != want { + t.Fatalf("generations = %q, want %q", got, want) + } +} diff --git a/internal/engine/backup_postgres.go b/internal/engine/backup_postgres.go index 39091b72..34127be8 100644 --- a/internal/engine/backup_postgres.go +++ b/internal/engine/backup_postgres.go @@ -10,6 +10,7 @@ import ( "os" "path" "path/filepath" + "regexp" "strconv" "strings" "time" @@ -193,7 +194,11 @@ func (e *Engine) QuiesceArchiver(ctx context.Context, service string) error { if err != nil { return err } - if res.ExitCode == 0 && strings.TrimSpace(res.Stdout) == "0" { + if res.ExitCode != 0 { + return fmt.Errorf("service %s: cannot inspect the PostgreSQL archive queue: %s", + service, strings.TrimSpace(res.Stderr)) + } + if strings.TrimSpace(res.Stdout) == "0" { return nil } if time.Now().After(deadline) { @@ -231,6 +236,80 @@ func (e *Engine) PostgresSystemIdentifier(ctx context.Context, service string) ( return identifier, nil } +// ValidateProtectedDatabaseIdentities proves that each protected PostgreSQL +// volume is the cluster recorded in lifecycle state. It deliberately reads the +// volume without relying on the service container: Compose may be about to +// recreate that container, and a missing volume must be detected before +// Compose silently creates an empty one. +func (e *Engine) ValidateProtectedDatabaseIdentities(ctx context.Context) error { + n := e.names() + for _, service := range e.Spec.ServiceNames() { + if !e.Spec.ServiceIsProtected(service) { + continue + } + state, ok := e.Spec.ServiceRuntimeState(service) + if !ok { + continue + } + recorded := state.DatabaseSystemIdentifier + if recorded == "" { + return fmt.Errorf("service %s is protected by a legacy repository binding with no PostgreSQL system identifier; run `ob backup enable %s` before applying services", service, service) + } + declared := e.Spec.Services[service] + driver := declared.Driver + if driver == "" { + driver = service + } + if driver != "postgres" { + continue + } + volume := n.ServiceVolume(service, app.DataVolumeFor(declared)) + inspect, err := e.T.Run(ctx, "docker volume inspect "+q(volume)) + if err != nil { + return err + } + if inspect.ExitCode != 0 { + return fmt.Errorf("service %s is recorded as PostgreSQL cluster %s, but data volume %s is missing; restore that generation or disable backup before applying services", service, recorded, volume) + } + image, err := e.Spec.ServiceImageForRuntime(service) + if err != nil { + return err + } + command := "docker run --rm --entrypoint pg_controldata -v " + q(volume+":/var/lib/postgresql/data:ro") + + " " + q(image.Image) + " " + q(app.PgDataPath) + res, err := e.T.Run(ctx, command) + if err != nil { + return err + } + if res.ExitCode != 0 { + return fmt.Errorf("service %s: cannot verify the PostgreSQL identity in volume %s: %s", service, volume, strings.TrimSpace(res.Stderr)) + } + actual := postgresControlSystemIdentifier(res.Stdout) + if actual == "" { + return fmt.Errorf("service %s: pg_controldata did not report a PostgreSQL system identifier for volume %s", service, volume) + } + if actual != recorded { + return fmt.Errorf("service %s data volume belongs to PostgreSQL cluster %s, but backup lifecycle state is bound to cluster %s; run `ob backup enable %s` to establish a new repository generation before applying services", service, actual, recorded, service) + } + } + return nil +} + +func postgresControlSystemIdentifier(output string) string { + for _, line := range strings.Split(output, "\n") { + key, value, ok := strings.Cut(line, ":") + if ok && strings.TrimSpace(key) == "Database system identifier" { + identifier := strings.TrimSpace(value) + if databaseSystemIdentifier.MatchString(identifier) { + return identifier + } + } + } + return "" +} + +var databaseSystemIdentifier = regexp.MustCompile(`^[0-9]{1,20}$`) + func (e *Engine) targetMachine(ctx context.Context) (string, error) { res, err := e.T.Run(ctx, "uname -m") if err != nil { diff --git a/internal/engine/backup_postgres_ops.go b/internal/engine/backup_postgres_ops.go index 1a552b49..c05e72a5 100644 --- a/internal/engine/backup_postgres_ops.go +++ b/internal/engine/backup_postgres_ops.go @@ -26,12 +26,13 @@ type BackupGeneration struct { // repository rather than from the project. The distinction is the whole point: // a policy says what should be true, and only the repository says what is. type BackupStatus struct { - Service string `json:"service"` - Repository string `json:"repository"` - Generations []BackupGeneration `json:"generations"` - RuntimeIssues []string `json:"runtime_issues,omitempty"` - LatestBackup *BackupGeneration `json:"latest_backup,omitempty"` - RecoverableTo string `json:"recoverable_to,omitempty"` + Service string `json:"service"` + Repository string `json:"repository"` + AvailableRepositoryGenerations []string `json:"available_repository_generations,omitempty"` + Generations []BackupGeneration `json:"generations"` + RuntimeIssues []string `json:"runtime_issues,omitempty"` + LatestBackup *BackupGeneration `json:"latest_backup,omitempty"` + RecoverableTo string `json:"recoverable_to,omitempty"` // The declared promise, carried alongside the facts so the report can be // read against it. Reporting only what the repository holds left the // operator to work out whether it satisfies the policy they wrote — which @@ -90,11 +91,21 @@ func (e *Engine) runWalg(ctx context.Context, service string, args ...string) (s return e.runWalgLocked(ctx, service, e.walgLockPrefix(ctx, service), args...) } -// runWalgRead is for commands that only read the repository. It waits briefly -// on a shared lock instead of an hour on an exclusive one, so status answers -// while a backup is running rather than blocking behind it. -func (e *Engine) runWalgRead(ctx context.Context, service string, args ...string) (string, error) { - return e.runWalgLocked(ctx, service, e.walgReadLockPrefix(ctx, service), args...) +func (e *Engine) runWalgReadAtRepository(ctx context.Context, service, repository string, args ...string) (string, error) { + n := e.names() + command := []string{strings.TrimSpace(e.walgReadLockPrefix(ctx, service)), "docker", "exec", "-u", "postgres", + "-e", q("WALG_S3_PREFIX=" + repository), q(n.ServiceContainer(service)), app.WalgBinary} + for _, arg := range args { + command = append(command, q(arg)) + } + res, err := e.T.Run(ctx, strings.TrimSpace(strings.Join(command, " "))) + if err != nil { + return "", err + } + if res.ExitCode != 0 { + return "", fmt.Errorf("wal-g %s: %s", strings.Join(args, " "), lastLines(res.Stderr+res.Stdout, 6)) + } + return res.Stdout, nil } func (e *Engine) runWalgLocked(ctx context.Context, service, lockPrefix string, args ...string) (string, error) { @@ -320,6 +331,13 @@ func parseWalVerifyRows(out string) []walVerifyStatus { // comes from wal-g, not from the project: the project's claim about retention // and recovery window is exactly the claim this is here to check. func (e *Engine) BackupStatusFor(ctx context.Context, service string) (BackupStatus, error) { + return e.BackupStatusForGeneration(ctx, service, "") +} + +// BackupStatusForGeneration inspects an explicitly selected physical cluster +// generation. An empty selection means the generation currently bound to the +// service. +func (e *Engine) BackupStatusForGeneration(ctx context.Context, service, generation string) (BackupStatus, error) { if _, _, err := e.backedUpService(service); err != nil { return BackupStatus{}, err } @@ -333,12 +351,32 @@ func (e *Engine) BackupStatusFor(ctx context.Context, service string) (BackupSta if err != nil { return BackupStatus{}, err } + root := app.WalgPrefix(projection.Target, e.Spec.Spec.Name, service, "") + if generation != "" { + if generation != "legacy" && !repositoryGeneration.MatchString(generation) { + return BackupStatus{}, fmt.Errorf("repository generation %q is not a PostgreSQL system identifier", generation) + } + selected := generation + if selected == "legacy" { + selected = "" + } + repository = app.WalgPrefix(projection.Target, e.Spec.Spec.Name, service, selected) + } status := BackupStatus{ Service: service, Repository: repository, DeclaredWindow: projection.Policy.Retention.Window, DeclaredMaxDataLoss: projection.Policy.MaxDataLoss, } + rootListing, err := e.runWalgReadAtRepository(ctx, service, root, "st", "ls") + if err != nil { + return status, fmt.Errorf("discover repository generations: %w", err) + } + clusterListing, err := e.runWalgReadAtRepository(ctx, service, root, "st", "ls", "clusters") + if err != nil { + return status, fmt.Errorf("discover repository generations: %w", err) + } + status.AvailableRepositoryGenerations = parseRepositoryGenerations(rootListing + "\n" + clusterListing) issues, err := e.VerifyBackupRuntime(ctx, service) if err != nil { @@ -346,7 +384,7 @@ func (e *Engine) BackupStatusFor(ctx context.Context, service string) (BackupSta } status.RuntimeIssues = issues - out, err := e.runWalgRead(ctx, service, "backup-list", "--detail", "--json") + out, err := e.runWalgReadAtRepository(ctx, service, repository, "backup-list", "--detail", "--json") if err != nil { return status, err } @@ -408,6 +446,35 @@ func (e *Engine) BackupStatusFor(ctx context.Context, service string) (BackupSta return status, nil } +var repositoryGeneration = regexp.MustCompile(`^[0-9]{1,20}$`) +var repositoryGenerationPath = regexp.MustCompile(`clusters/([0-9]{1,20})(?:/|$)`) + +func parseRepositoryGenerations(listing string) []string { + seen := map[string]bool{} + for _, line := range strings.Split(listing, "\n") { + matches := repositoryGenerationPath.FindAllStringSubmatch(line, -1) + for _, match := range matches { + seen[match[1]] = true + } + fields := strings.Fields(line) + if len(fields) > 0 { + name := strings.TrimSuffix(fields[len(fields)-1], "/") + if repositoryGeneration.MatchString(name) { + seen[name] = true + } + } + if len(matches) == 0 && (strings.Contains(line, "basebackups_005/") || strings.Contains(line, "wal_005/")) { + seen["legacy"] = true + } + } + out := make([]string, 0, len(seen)) + for generation := range seen { + out = append(out, generation) + } + sort.Strings(out) + return out +} + func lastLines(text string, count int) string { lines := strings.Split(strings.TrimSpace(text), "\n") if len(lines) > count { diff --git a/internal/engine/backup_restore.go b/internal/engine/backup_restore.go index e56c1bf7..f22e80e5 100644 --- a/internal/engine/backup_restore.go +++ b/internal/engine/backup_restore.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "strconv" "strings" "time" @@ -31,14 +32,15 @@ const ( // RestoreOutcome is what a recovery produced, whether or not it was kept. type RestoreOutcome struct { - Service string `json:"service"` - Target string `json:"target"` - Backup string `json:"backup"` - RecoveredTo string `json:"recovered_to"` - Rows string `json:"sanity_check"` - StagingVolume string `json:"staging_volume,omitempty"` - PreviousData string `json:"previous_data_volume,omitempty"` - Promoted bool `json:"promoted"` + Service string `json:"service"` + Target string `json:"target"` + Backup string `json:"backup"` + RecoveredTo string `json:"recovered_to"` + Rows string `json:"sanity_check"` + StagingVolume string `json:"staging_volume,omitempty"` + PreviousData string `json:"previous_data_volume,omitempty"` + Promoted bool `json:"promoted"` + DatabaseSystemIdentifier string `json:"database_system_identifier,omitempty"` // RetainStaging is set the moment promotion starts modifying the live // volume. From then on the staging volume is the only complete copy of the // recovered data and must survive a failure, so the operator has something @@ -154,11 +156,25 @@ func (e *Engine) RecoverService(ctx context.Context, service, targetTime string, return outcome, err } outcome.RecoveredTo, outcome.Rows = recoveredTo, rows + identifier, err := e.recoveredSystemIdentifier(ctx, container, service) + if err != nil { + st(err) + return outcome, err + } + outcome.DatabaseSystemIdentifier = identifier + if state, ok := e.Spec.ServiceRuntimeState(service); ok && state.DatabaseSystemIdentifier != "" && state.DatabaseSystemIdentifier != identifier { + err := fmt.Errorf("recovered PostgreSQL cluster %s does not match selected repository generation %s", identifier, state.DatabaseSystemIdentifier) + st(err) + return outcome, err + } st(nil) if !promote { return outcome, nil } + if err := e.StageServiceCompose(ctx, service); err != nil { + return outcome, fmt.Errorf("cannot stage the recovered cluster's repository binding: %w", err) + } previous, err := e.promoteRecoveredVolume(ctx, service, container, staging, &outcome) outcome.PreviousData = previous if err != nil { @@ -168,6 +184,23 @@ func (e *Engine) RecoverService(ctx context.Context, service, targetTime string, return outcome, nil } +func (e *Engine) recoveredSystemIdentifier(ctx context.Context, container, service string) (string, error) { + command := "docker exec -u postgres " + q(container) + " psql -U " + q(app.PgSuperuser) + + " -d postgres -Atc " + q("select system_identifier::text from pg_control_system();") + res, err := e.T.Run(ctx, command) + if err != nil { + return "", err + } + if res.ExitCode != 0 { + return "", fmt.Errorf("service %s: cannot read the recovered PostgreSQL system identifier: %s", service, strings.TrimSpace(res.Stderr)) + } + identifier := strings.TrimSpace(res.Stdout) + if _, err := strconv.ParseUint(identifier, 10, 64); err != nil || identifier == "" { + return "", fmt.Errorf("service %s: recovered PostgreSQL returned an invalid system identifier %q", service, identifier) + } + return identifier, nil +} + func recoveryTargetLabel(targetTime string) string { if targetTime == "" { return "the newest recoverable point" diff --git a/internal/engine/services.go b/internal/engine/services.go index 136bc549..23090b1d 100644 --- a/internal/engine/services.go +++ b/internal/engine/services.go @@ -70,6 +70,13 @@ func (e *Engine) ApplyServices(ctx context.Context) error { if len(names) == 0 { return nil } + // A protected repository generation belongs to one physical PostgreSQL + // cluster. Check the volume before Compose is allowed to create or start + // anything: a missing/replaced volume otherwise starts at WAL segment 1 and + // archives it into the old cluster's namespace. + if err := e.ValidateProtectedDatabaseIdentities(ctx); err != nil { + return err + } // Rendered here rather than handed in. A caller that forgot would produce // a host missing a database, and the engine already holds everything the // documents are derived from. @@ -147,6 +154,22 @@ func (e *Engine) ApplyServices(ctx context.Context) error { return nil } +// StageServiceCompose writes one service's current rendered runtime without +// starting it. Recovery uses this after proving a historical generation and +// before swapping volumes, so the recovered cluster can never start with the +// generation binding of the database it replaces. +func (e *Engine) StageServiceCompose(ctx context.Context, service string) error { + rendered, err := e.Spec.RenderServices(e.Opts.Environment) + if err != nil { + return err + } + doc, ok := rendered[service] + if !ok { + return fmt.Errorf("service %s was declared but not rendered — this is an Onebox bug", service) + } + return e.writeServiceFile(ctx, e.names().ServiceFile(service), doc) +} + // serviceIsHealthy waits briefly for a just-started service to report health. // A driver with no health check reports "none", which is as strong a statement // as it can make and is treated as running. diff --git a/internal/onebox/backup_enable.go b/internal/onebox/backup_enable.go index 3f7b41b7..09093e29 100644 --- a/internal/onebox/backup_enable.go +++ b/internal/onebox/backup_enable.go @@ -199,31 +199,11 @@ func executeBackupEnable(ctx context.Context, e *engine.Engine, resolved *app.Re // before starting anything that mounts them, then restarts the server with // archive_mode on. if err := e.ApplyServices(ctx); err != nil { - return fmt.Errorf("service %s could not restart under backup: %w", service, err) - } - // A target that moved is a new history, and the operator is told rather than - // left to notice: the base backup below is the first one in the new - // repository, so the declared window starts from now. What the old - // repository holds is untouched and stays where it is. - // Compared as repositories, not as target names. Editing the bucket or the - // endpoint inside a target called "offsite" moves the history just as - // surely as pointing the service at a target called something else, and the - // name would not have changed. - nextRepository := app.WalgPrefix(projection.Target, resolved.Spec.Name, service, next.BackupRepositoryGeneration) - if previous := previousBackupRepository(current, resolved.Spec.Name, service); previous != "" && - previous != nextRepository { - e.ReportTargetMoved(service, previous, nextRepository) - // The credential file is named for the target it belongs to, so a move - // to a *differently named* target leaves the old file holding keys - // nothing uses. Editing the bucket inside a target keeps the name, and - // therefore the path — and removing it then would delete the file this - // very run just installed. It did: the next command that needed the - // repository failed with "--env-file: no such file or directory". - if retiresCredentialFile(current.LastEffective, projection) { - if err := e.RemoveBackupCredentials(ctx, service, current.LastEffective); err != nil { - return err - } + failure := fmt.Errorf("service %s could not restart under backup: %w", service, err) + if rollbackErr := rollbackFailedEnablement(ctx, e, service, current, next, operationID); rollbackErr != nil { + return errors.Join(failure, rollbackErr) } + return failure } // Everything PostgreSQL is holding goes to the repository before the base // backup writes into the same WAL namespace. Without this, `backup-push` @@ -232,10 +212,8 @@ func executeBackupEnable(ctx context.Context, e *engine.Engine, resolved *app.Re // PostgreSQL archives in order the chain stops there permanently — while // this command reports success. if err := e.QuiesceArchiver(ctx, service); err != nil { - if current.State != BackupEnabled { - if revertErr := revertFailedEnablement(ctx, e, service, next, operationID); revertErr != nil { - return errors.Join(err, revertErr) - } + if rollbackErr := rollbackFailedEnablement(ctx, e, service, current, next, operationID); rollbackErr != nil { + return errors.Join(err, rollbackErr) } return err } @@ -249,23 +227,51 @@ func executeBackupEnable(ctx context.Context, e *engine.Engine, resolved *app.Re // established by a command that reported failure. On a busy database that // is an unbounded disk commitment nobody agreed to. if err := e.BackupService(ctx, service); err != nil { - // Only when this run is what turned archiving on. A service that was - // already protected was archiving before this command ran and to its - // own repository; taking it back to unprotected because a re-enable - // could not take a fresh base backup would break a working setup to - // tidy up a failed change. Either way the service is left as it was - // found. - if current.State == BackupEnabled { - return err + if rollbackErr := rollbackFailedEnablement(ctx, e, service, current, next, operationID); rollbackErr != nil { + return errors.Join(err, rollbackErr) } - if revertErr := revertFailedEnablement(ctx, e, service, next, operationID); revertErr != nil { - return errors.Join(err, fmt.Errorf( - "service %s is archiving with no base backup and could not be returned to unprotected — "+ - "write-ahead log will accumulate until `ob backup disable %s` succeeds: %w", - service, service, revertErr)) + return err + } + + // A target that moved is reported and its old credential retired only after + // the new repository has accepted a complete base backup. Until this point a + // failure must be able to restore the exact previous binding, including the + // credential file that opens it. + nextRepository := app.WalgPrefix(projection.Target, resolved.Spec.Name, service, next.BackupRepositoryGeneration) + if previous := previousBackupRepository(current, resolved.Spec.Name, service); previous != "" && previous != nextRepository { + e.ReportTargetMoved(service, previous, nextRepository) + if retiresCredentialFile(current.LastEffective, projection) { + if err := e.RemoveBackupCredentials(ctx, service, current.LastEffective); err != nil { + return err + } } + } + return nil +} + +// rollbackFailedEnablement restores the state in which this enablement found +// the service. An enabled or disable-pending service was already archiving, so +// its exact repository binding is reinstated. A previously unprotected service +// instead walks the fail-closed disable transition used by first enablement. +func rollbackFailedEnablement(ctx context.Context, e *engine.Engine, service string, current, attempted BackupLifecycleState, operationID string) error { + if current.State != BackupEnabled && current.State != BackupDisablePending { + return revertFailedEnablement(ctx, e, service, attempted, operationID) + } + body, err := encodeBackupLifecycleState(current) + if err != nil { return err } + if err := e.WriteBackupLifecycleState(ctx, service, body); err != nil { + return err + } + runtime := current.RuntimeState() + runtime.DigestAvailable = true + if err := e.RebindServiceRuntimeStates(map[string]app.ServiceRuntimeState{service: runtime}); err != nil { + return err + } + if err := e.ApplyServices(ctx); err != nil { + return fmt.Errorf("service %s could not restore its previous backup binding: %w", service, err) + } return nil } diff --git a/internal/onebox/backup_read.go b/internal/onebox/backup_read.go index 302a1a46..b0076b1a 100644 --- a/internal/onebox/backup_read.go +++ b/internal/onebox/backup_read.go @@ -16,6 +16,12 @@ import ( // asking. The wal-g listing underneath takes a *shared* repository lock with a // short timeout, so it reads consistently without queueing behind a backup. func (s *Service) BackupStatus(ctx context.Context, service string) (engine.BackupStatus, error) { + return s.BackupStatusGeneration(ctx, service, "") +} + +// BackupStatusGeneration reads either the currently bound repository +// generation or an explicitly selected historical generation. +func (s *Service) BackupStatusGeneration(ctx context.Context, service, generation string) (engine.BackupStatus, error) { lp, err := s.loadProject(ctx, true) if err != nil { return engine.BackupStatus{}, fmt.Errorf("load project: %w", err) @@ -42,5 +48,5 @@ func (s *Service) BackupStatus(ctx context.Context, service string) (engine.Back } return engine.BackupStatus{}, failure } - return e.BackupStatusFor(ctx, service) + return e.BackupStatusForGeneration(ctx, service, generation) } diff --git a/internal/onebox/backup_restore.go b/internal/onebox/backup_restore.go index 5abecdc2..54c7a303 100644 --- a/internal/onebox/backup_restore.go +++ b/internal/onebox/backup_restore.go @@ -2,8 +2,11 @@ package onebox import ( "context" + "errors" "fmt" + "regexp" + "github.com/labstack/onebox/internal/app" "github.com/labstack/onebox/internal/engine" ) @@ -13,7 +16,7 @@ import ( // The locking is the same as enablement's and for the same reason: a restore // replaces a database's data, so it must not interleave with a deploy, another // recovery, or a scheduled backup. -func executeRecovery(ctx context.Context, e *engine.Engine, service, target string, promote bool, operationID string) error { +func executeRecovery(ctx context.Context, e *engine.Engine, service, target, generation string, promote bool, operationID string) error { if service == "" { return fmt.Errorf("recovery requires a service name") } @@ -62,11 +65,69 @@ func executeRecovery(ctx context.Context, e *engine.Engine, service, target stri } return failure } + selected := current + if generation != "" { + selectedGeneration := generation + if generation == "legacy" { + selectedGeneration = "" + } else if !recoveryGeneration.MatchString(generation) { + return fmt.Errorf("repository generation %q is not a PostgreSQL system identifier or legacy", generation) + } + selected.DatabaseSystemIdentifier = selectedGeneration + selected.BackupRepositoryGeneration = selectedGeneration + if err := selected.Seal(); err != nil { + return err + } + runtime := selected.RuntimeState() + runtime.DigestAvailable = true + if err := e.RebindServiceRuntimeStates(map[string]app.ServiceRuntimeState{service: runtime}); err != nil { + return err + } + } outcome, err := e.RecoverService(ctx, service, target, promote) if err != nil { + if generation != "" { + if outcome.RetainStaging && outcome.DatabaseSystemIdentifier != "" { + selected.DatabaseSystemIdentifier = outcome.DatabaseSystemIdentifier + if generation != "legacy" { + selected.BackupRepositoryGeneration = outcome.DatabaseSystemIdentifier + } + if persistErr := writeRecoveryBinding(context.WithoutCancel(ctx), e, service, selected); persistErr != nil { + return errors.Join(err, fmt.Errorf("cannot record the binding required by the partially promoted recovered service: %w", persistErr)) + } + } else { + runtime := current.RuntimeState() + runtime.DigestAvailable = true + if rebindErr := e.RebindServiceRuntimeStates(map[string]app.ServiceRuntimeState{service: runtime}); rebindErr == nil { + _ = e.StageServiceCompose(context.WithoutCancel(ctx), service) + } + } + } return err } + if promote && generation != "" { + selected.DatabaseSystemIdentifier = outcome.DatabaseSystemIdentifier + if generation != "legacy" { + selected.BackupRepositoryGeneration = outcome.DatabaseSystemIdentifier + } + if err := writeRecoveryBinding(ctx, e, service, selected); err != nil { + return fmt.Errorf("recovered service is running but its selected repository binding could not be recorded: %w", err) + } + } e.ReportRecovery(outcome) return nil } + +var recoveryGeneration = regexp.MustCompile(`^[0-9]{1,20}$`) + +func writeRecoveryBinding(ctx context.Context, e *engine.Engine, service string, state BackupLifecycleState) error { + if err := state.Seal(); err != nil { + return err + } + body, err := encodeBackupLifecycleState(state) + if err != nil { + return err + } + return e.WriteBackupLifecycleState(ctx, service, body) +} diff --git a/internal/onebox/execute.go b/internal/onebox/execute.go index 27577488..09912781 100644 --- a/internal/onebox/execute.go +++ b/internal/onebox/execute.go @@ -199,7 +199,7 @@ func (s *Service) Execute(ctx context.Context, request ExecuteRequest) (Operatio // One path, two endings. A drill stops after proving the recovered // cluster answers; a restore goes on to put it in service. result.EvidenceID = operationID - err = executeRecovery(ctx, e, request.Service, request.RecoveryTarget, + err = executeRecovery(ctx, e, request.Service, request.RecoveryTarget, request.RecoveryGeneration, request.Kind == KindRestoreCutover, operationID) case KindProxyApply: result.EvidenceID = operationID diff --git a/internal/onebox/execution_types.go b/internal/onebox/execution_types.go index 8701533f..e2b918c7 100644 --- a/internal/onebox/execution_types.go +++ b/internal/onebox/execution_types.go @@ -300,7 +300,10 @@ type ExecuteRequest struct { Service string // RecoveryTarget is the RFC 3339 point in time a recovery aims at. Empty // means the newest recoverable point. - RecoveryTarget string + RecoveryTarget string + // RecoveryGeneration selects a historical physical PostgreSQL cluster by + // system identifier. "legacy" names the pre-generation repository layout. + RecoveryGeneration string BreakMigrationGate bool NoRollback bool Redeploy bool @@ -342,6 +345,9 @@ func (request ExecuteRequest) Validate() error { if (request.RemoveVolumes || request.RemoveProxy) && request.Kind != KindDestroy { return errors.New("remove_volumes and remove_proxy are valid only for destroy") } + if request.RecoveryGeneration != "" && request.Kind != KindRestoreTest && request.Kind != KindRestoreCutover { + return errors.New("recovery_generation is valid only for restore and drill") + } if (request.Approval != nil || request.BackupReport != nil || request.MigrationBackupOverride != nil) && request.Kind != KindDeploy && request.Kind != KindJobRun { return errors.New("approval and migration backup authorization are valid only for deploy and job run") } diff --git a/scripts/lima.sh b/scripts/lima.sh index 8da0516b..10d18a67 100755 --- a/scripts/lima.sh +++ b/scripts/lima.sh @@ -35,8 +35,14 @@ ssh_field() { } up() { + local status if limactl list --quiet 2>/dev/null | grep -qx "$instance"; then - echo "instance ${instance} already exists; reusing it" + status="$(limactl list --format '{{.Status}}' "$instance" 2>/dev/null | tr '[:upper:]' '[:lower:]')" + if [[ "$status" != "running" ]]; then + limactl start --tty=false "$instance" + else + echo "instance ${instance} already exists; reusing it" + fi else limactl start --name="$instance" --tty=false "${repo}/${config}" fi From cb3bfe5ca9b767752ad017783ee5e0e741fa8317 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Fri, 21 Aug 2026 22:41:52 -0700 Subject: [PATCH 6/7] docs(backup): document generation recovery --- docs/product.md | 12 ++--- .../docs/guides/back-up-a-database.mdx | 53 ++++++++++++------- 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/docs/product.md b/docs/product.md index 61b79465..a390debb 100644 --- a/docs/product.md +++ b/docs/product.md @@ -38,12 +38,12 @@ identity, not user configuration. The broader managed-operations goal is direction, not an inventory. Owned today: host bootstrap, the container runtime check, the proxy and its TLS, the host -ingress network, release -staging and retention, the supporting data services and their credentials, and -scheduled jobs. **Not owned today: backups, restore proof, and log rotation.** -Onebox says so rather than implying otherwise — `ob doctor` reports the absence -of backups for every workload and service holding durable data, because silence -there would read as approval. +ingress network, release staging and retention, the supporting data services and +their credentials, scheduled jobs, and PostgreSQL backup, point-in-time recovery, +and on-demand restore proof. **Not owned today: workload-volume backups, +unattended full restore drills, and log rotation.** Onebox says so rather than +implying otherwise — `ob doctor` reports every durable workload or service that +has no executable backup contract, because silence there would read as approval. The distinction matters more than it looks. A product direction that reads as a capability list is how an operator ends up believing their database is backed diff --git a/site/src/content/docs/guides/back-up-a-database.mdx b/site/src/content/docs/guides/back-up-a-database.mdx index 0cbbf601..6aaf56c9 100644 --- a/site/src/content/docs/guides/back-up-a-database.mdx +++ b/site/src/content/docs/guides/back-up-a-database.mdx @@ -156,9 +156,10 @@ means: → database now archives to backup target "s3://elsewhere/shop_database". What "s3://onebox-backups/shop_database" holds is untouched, but this repository starts from the backup being taken now, so the declared recovery window begins here ``` -Everything else — a restore, a drill, status, retention — keeps reading the -repository the service was actually bound to, so editing a target cannot -silently point a recovery at a repository the history is not in. +By default, everything else — a restore, a drill, status, retention — keeps +reading the repository the service was actually bound to, so editing a target +cannot silently point a recovery at a repository the history is not in. Status, +drill, and restore can also select an older cluster generation explicitly. A repository also moves when the *database* is replaced, without anything in the project changing. Each repository is scoped to the cluster that wrote it, using @@ -176,8 +177,8 @@ order, one refused segment stops the chain permanently. Scoping by cluster means a rebuilt database simply begins its own history, and the previous one stays exactly where it is. -`ob backup status` prints both the repository and the system identifier, which is -how you tell two histories apart when a service has been rebuilt. +`ob backup status` prints the repository and every cluster generation it finds +there, which is how you tell two histories apart when a service has been rebuilt. ## Prove it restores @@ -223,21 +224,33 @@ replay only moves forward, so a later base can never reach an earlier moment. A point older than everything in the repository is refused, and the refusal tells you when the oldest backup finished. -:::caution[After losing the volume, recover before you re-enable] -A recovery reads the repository the service is *recorded* as bound to, and a -recovered cluster keeps the system identifier it was backed up with — so -recovering first and enabling afterwards keeps the same history, and the service -carries on where it left off. - -Enabling first does not. `ob backup enable` binds the service to whatever cluster -is running, and a database rebuilt from an empty volume is a new cluster with a -new identifier, so enablement starts a new repository. The old backups are -untouched and still in the bucket, but the service is no longer pointed at them -and `ob backup restore` will not find them. - -If that has already happened, the history is not lost: `ob backup status` gives -the identifier the service is bound to now, and the previous generation is beside -it under `clusters/`. +:::caution[After losing the volume or host, select the old generation] +A database rebuilt from an empty volume is a new cluster with a new identifier. +`ob backup enable` therefore starts a new generation instead of risking a WAL +collision with the old one. It also restores the runtime and credentials needed +to inspect the off-host repository when the original host state is gone. + +Status discovers every generation still present in that repository: + +```console +$ ob backup status database +service database +repository s3://onebox-backups/shop_database/clusters/7520844209557584421 +generation 7513211627332151223 +generation 7520844209557584421 +``` + +Prove and restore the old history by selecting its identifier explicitly: + +```console +$ ob backup drill database --generation 7513211627332151223 +$ ob backup restore database --generation 7513211627332151223 --confirm database +``` + +A successful restore records that recovered cluster as the service's current +generation, so subsequent backup commands continue on the restored history. Use +`--generation legacy` for a repository created before cluster-scoped generations +were introduced. ::: ## Stopping From 6e68aa9c5f6587176c4ff2e3bdd3c8b6137c096c Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Fri, 21 Aug 2026 23:01:54 -0700 Subject: [PATCH 7/7] ci(release): parallelize server e2e gate Run lifecycle and failure probes on isolated guests in parallel before publishing. Keep the slow suite out of PR CI while retaining nightly and manual coverage. --- .github/workflows/e2e-server.yml | 50 +++++++++++--------------------- .github/workflows/release.yml | 8 ++++- 2 files changed, 24 insertions(+), 34 deletions(-) diff --git a/.github/workflows/e2e-server.yml b/.github/workflows/e2e-server.yml index a089ee72..da93036a 100644 --- a/.github/workflows/e2e-server.yml +++ b/.github/workflows/e2e-server.yml @@ -1,36 +1,12 @@ # The server end-to-end suite. # -# It is its own workflow rather than a job in ci.yml because `paths:` filters -# apply to a workflow, not to one job beside unrelated ones: adding it there -# would have gated Check and the Docker suite on the same paths. +# It is deliberately outside PR CI: the Lima/SSH run takes long enough to make +# review feedback drag. Release calls it before publishing, while nightly and +# manual runs keep the external dependencies exercised between releases. name: E2E (Server) on: - push: - branches: [main] - paths: - - "internal/engine/**" - - "internal/transport/**" - - "internal/onebox/**" - - "internal/app/backup_*" - - "internal/app/names.go" - - "internal/app/service_image_state.go" - - "cmd/ob/**" - - "e2e/**" - - "scripts/lima.sh" - - ".github/workflows/e2e-server.yml" - pull_request: - paths: - - "internal/engine/**" - - "internal/transport/**" - - "internal/onebox/**" - - "internal/app/backup_*" - - "internal/app/names.go" - - "internal/app/service_image_state.go" - - "cmd/ob/**" - - "e2e/**" - - "scripts/lima.sh" - - ".github/workflows/e2e-server.yml" + workflow_call: # Nightly, because most of what this suite depends on is outside the # repository and rots on its own schedule: the cloud image, the wal-g # release, the postgres tag, and the certificates in between. @@ -42,14 +18,22 @@ permissions: contents: read concurrency: - group: e2e-server-${{ github.event.pull_request.number || github.sha }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} + group: e2e-server-${{ github.sha }} + cancel-in-progress: false jobs: server: - name: End-to-end (Server) + name: End-to-end (Server / ${{ matrix.suite.name }}) runs-on: ubuntu-24.04 timeout-minutes: 45 + strategy: + fail-fast: false + matrix: + suite: + - name: lifecycle + test: TestServerLifecycle + - name: probes + test: TestServerProbes steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: @@ -118,7 +102,7 @@ jobs: run: timeout 300 bash scripts/lima.sh up - name: Run the server end-to-end suite - run: bash scripts/lima.sh test + run: bash scripts/lima.sh test -run '^${{ matrix.suite.test }}$' # Curated diagnostics only. # @@ -147,6 +131,6 @@ jobs: - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 if: always() with: - name: server-diagnostics + name: server-diagnostics-${{ matrix.suite.name }} path: diagnostics/ retention-days: 7 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f83b10fc..2fda323f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -23,9 +23,15 @@ jobs: contents: read uses: ./.github/workflows/ci.yml + server-e2e: + name: Verify release on a real server + permissions: + contents: read + uses: ./.github/workflows/e2e-server.yml + release: name: Publish signed release, Homebrew, and Scoop - needs: verify + needs: [verify, server-e2e] runs-on: ubuntu-24.04 timeout-minutes: 40 permissions: