infra: roll back to the last good commit and wait for every Deployment by devinder-dev · Pull Request #14 · devinder-dev/relay

devinder-dev · 2026-10-29T10:25:33Z

Follow-up to #13, from Abdikhani's review. It changes what relay-deploy does when a deploy fails, before the pipeline starts deploying in step 1.8.

What changes

A real rollback. After every successful deploy, the script writes the commit to /home/deploy/.relay-last-good. If a later rollout fails, it deploys that commit again the same way (checkout, apply, wait). So ConfigMaps, Services and Ingresses go back together with the image. kubectl rollout undo only reverted the pod template.
Every Deployment. The script waits for each Deployment that kubectl apply -o name reports, not only relay-api. That covers the dashboard, and later the worker.
Gate 1 above the lock. An invalid SHA is rejected at once, without waiting for the lock.
One function for deploy and rollback. Steps 3–5 are now deploy_commit. Every step in it ends with || return 1, because set -e is off inside a function that is called from if.
k8s/README.md:
- the dashboard lives in k8s/app/ too;
- every merge builds every image;
- nothing is deleted automatically, so we delete by hand;
- how the rollback works.
infra/server/test-relay-deploy.sh: tests the script's decisions with fake git, kubectl and flock. No server and no cluster needed.

The three endings of a failed deploy

All of them exit 1, so the CI job turns red, because the commit that was asked for isn't live:

no last good commit yet → "no last good commit to roll back to"
the rollback worked → "rolled back to "
the rollback failed too → "rollback to failed"

Tested

bash infra/server/test-relay-deploy.sh infra/server/relay-deploy.sh → 11 passed, 0 failed:

A good deploy → exit 0, and it's remembered as last good.
A broken deploy → exit 1, the last good commit is put back, and the note is unchanged.
A broken first deploy → exit 1, and no note is invented.
The rollback fails too → exit 1, after trying the last good commit.
kubectl apply fails → exit 1, and it's not recorded as good.

Test 5 guards the set -e trap. Without || return 1 on the apply line, it fails: exit 0, and the broken commit is saved as last good.

The real rollback gets tested in 1.8, with a deliberately broken commit.

After merging

Install the script on the server again (see "install or update" in k8s/README.md), then check that the two copies match:
ssh relay 'cat /usr/local/bin/relay-deploy' | diff - infra/server/relay-deploy.sh

Added after an independent review of Phases 0–1

The gates are now tested (test 6 and 7):
- whoami, an injection, an uppercase SHA and a SHA with a trailing space all stop at gate 1, with nothing checked out.
- A commit that isn't on main stops at gate 2, with nothing checked out and nothing applied.
A failed rollout shows why. The script prints the pods and the latest 20 events into the CI log (test 8). For that, the deployer Role gets read-only get/list on events (k8s/platform/deployer.yaml), which one of us applies by hand after merging.
The last-good note is checked like gate 1 before a rollback (test 9). Its content goes into git and sed, and GNU sed can even run commands (e), so only a 40-hex value may reach it.
The result: 26 passed, 0 failed, and shellcheck is clean on all three scripts.

After merging, also apply deployer.yaml by hand, before reinstalling the script.

…st-good note

…15) Step 1.7: the API's Kubernetes manifests in `k8s/app/`. This PR doesn't deploy anything. The pipeline's deploy job comes in 1.8, once #12 (the image) and #14 (rollback) are merged. ## What's in it - **`deployment.yaml`:** - 2 pods (VG1). - Rolling update with `maxSurge: 1, maxUnavailable: 0`, so there are never fewer than 2 ready pods (VG2). - Image `relay-api:__GIT_SHA__`, filled in by `relay-deploy`. - Probes, and a `preStop` sleep of 5 s. - Requests 100m CPU and 128Mi memory, limit 256Mi (the 1.3 load test peaked at 93 MB). - Runs as uid 1000 (not root), with no privilege escalation, all capabilities dropped, and the default seccomp profile. - **`service.yaml`:** port 80 → the pods' port named `http`. - **`ingress.yaml`:** `api.myrelay.tech`, with a `letsencrypt-prod` certificate in namespace `relay`. ## The probes - **Readiness:** `/health/ready` every 5 s. After 3 failures (about 15 s), the pod is taken out of the Service but keeps running. - **Liveness:** `/health/live` every 20 s, starting after 15 s. After 3 failures (about 60 s), the container is restarted. Liveness is the more patient one because a restart is more drastic than losing traffic. It also never checks the database: a database outage must not restart every pod. ## Why `runAsUser: 1000` The image says `USER bun`, which is a name. Kubernetes can only verify `runAsNonRoot` against a number. With just the name, the pod would fail with `CreateContainerConfigError`. ## Tested - **A server-side dry run as the `deployer` account itself** (`sudo -u deploy kubectl … --dry-run=server`): all three files were accepted. A dry run is authorized like a real request, so this proves the pipeline may apply exactly these files. - **The probe paths against the real image.** A loop reads every `path:` from `deployment.yaml` and curls the running container: `200 /health/ready`, `200 /health/live`. The dry run alone can't catch a wrong path: it also passed with `/ready` and `/health`, which both answer 404. ## At the first deploy (1.8) The hello app in `default` also claims `api.myrelay.tech`, so we delete it by hand at that moment, because two Ingresses for one host would compete. cert-manager then issues a new certificate for namespace `relay`. ## Added after an independent review of Phases 0–1 - **`automountServiceAccountToken: false`:** the API never talks to Kubernetes, so its pods get no token at all. - **`readOnlyRootFilesystem: true`:** tested with the real image (`--read-only --cap-drop ALL --user 1000:1000`). `/health/ready`, `/health/live` and `/version` answer 200, and `docker stop` exits 0 in 0.15 s. - **A comment on why there's no CPU limit:** a limit would throttle the API even when the server has CPU to spare. The request is what scheduling, and later the HPA, use. - The server-side dry run still passes.

AbdikhaniAbdirashid

Approving. The rollback is a real improvement over rollout undo, and the test script is the part I liked most — being able to check the script's decisions with no server and no cluster is what made this reviewable at all. Line comments on four things, none blocking.

What I checked myself

Ran bash infra/server/test-relay-deploy.sh infra/server/relay-deploy.sh: 26 passed, 0 failed, matching your description.
Gate 1 before the lock, gate 2 inside it. The git fetch sits after flock, which is what matters — two deploys can't move the shared checkout under each other.
The injection case genuinely stops: aaa…; touch /tmp/pwned is refused and nothing is checked out.
Test 9 is the one I'd have missed. The last-good note feeds git and sed, GNU sed can run commands, and re-applying gate 1 to the note closes that. Good catch.
The events rule in deployer.yaml matches what show_state actually calls, and the Role is still confined to relay with no cluster-wide verbs.

Not a line comment, because it's about us rather than the code: two manual steps after merge — deployer.yaml applied by hand, then the script reinstalled. I checked the server and it's still on the old version. Can we agree who does both, and when? If the RBAC step is missed, everything looks fine until a rollout fails and the events call is denied — exactly when we need the output.

Questions

When the dashboard's manifests land in k8s/app/, every deploy will wait on both Deployments even when only one changed. Is that intended? I think yes — it's simpler, and it's honest about what "deployed" means — but it doubles the worst case, which feeds into my comment on line 30.
kubectl apply without --prune, and the README says we delete by hand. What happens the first time we rename something in k8s/app/? The old object stays and keeps serving. Worth naming that in the README as the case to watch for?

AbdikhaniAbdirashid · 2026-10-29T14:03:17Z

+# 2. One deploy at a time: a second deploy waits here for up to 5 minutes.
+#    The lock is released by itself when the script ends, even if it crashes.
+exec 9>/home/deploy/.relay-deploy.lock
+flock --wait 300 9 || fail "another deploy is still running"


The one number I'd like changed.

This waits 5 minutes, but a deploy can hold the lock for much longer. One deploy_commit waits up to 180 s per Deployment, and k8s/app/ will hold two (API and dashboard), so 6 minutes. A failed deploy plus its rollback is two of those — up to 12 minutes — while the next deploy gives up after 5 and exits 1.

So merging two PRs close together, with the first one failing, turns the second build red for a reason that has nothing to do with its code. We'll be merging a lot in the week before the demo.

Either raise --wait above the worst case, or make "someone else holds the lock" a distinct, obviously-retryable message rather than a plain failure.

AbdikhaniAbdirashid · 2026-10-29T14:03:17Z

+  # Wait until each Deployment it touched has its new pods ready (the API, the dashboard,
+  # and later the worker), so one that never gets ready can't pass as a success.
+  for deployment in $(echo "$applied" | grep '^deployment'); do
+    if ! kubectl rollout status "$deployment" --timeout=180s; then


Is 180 s enough for a cold image pull? The first deploy of a new image pulls ~84 MB of base layers, and with maxUnavailable: 0 the pods roll one at a time, so it's two sequential pull-and-start cycles.

Probably fine, but a timeout that is occasionally too short is worse than one that's clearly generous: it triggers a rollback of a deploy that would have worked.

AbdikhaniAbdirashid · 2026-10-29T14:03:17Z

+# them must never change the result.
+show_state() {
+  echo "--- pods"
+  kubectl get pods -o wide || true


Not a bug — a note for the next reader. This and rollout status below run without -n relay, so I went looking for a bug. It's fine: setup-deploy-user.sh sets namespace: relay on the kubeconfig context.

A one-line comment saying the namespace comes from the kubeconfig would save the next person that hunt. If that line in setup-deploy-user.sh ever changes, every one of these commands silently targets the wrong namespace.

AbdikhaniAbdirashid · 2026-10-29T14:03:17Z

+      return 1
+    fi
+  done
+}


Small: no closing return 0. The function currently returns the for loop's status, which is 0, so it works today.

But the contract is "0 on success, 1 on any failure", and right now that holds only because the last statement happens to be a loop. Add a line after done and the function can start lying about its result. One return 0 makes it true by construction.

devinder-dev · 2026-10-29T14:10:15Z

Thanks, and good that you ran the harness yourself. All four line comments are fair. They go into one small follow-up PR, opened today:

Lock wait (line 30): you're right that one failed deploy plus its rollback can hold the lock for longer than the next one waits. In CI it can't actually happen: the workflow's concurrency group on main runs each pipeline only after the previous one has finished, deploy included. But an admin deploying by hand at the same moment could hit it. The fix:
- flock --wait 900 (15 min, above the 12-minute worst case);
- a distinct message: "another deploy is still running after 15 minutes, re-run this job";
- the deploy job's timeout goes from 20 to 30 minutes in infra: deploy every merge to main and check that it's live #16.
180 s (line 76): I'd keep it, and measure it. With one node, each image is downloaded once. A new commit only changes our small code layers, because the base layers are already on the server (the node holds relay-api:a8274c1 since today). The two pods roll in sequence, but the second one doesn't download again. The first deploy's log will show the real time, and if it's anywhere near 180 s we raise it.
Namespace (line 43): a comment at the KUBECONFIG line, saying the namespace comes from the context set in setup-deploy-user.sh.
return 0 (line 81): added, so "0 on success" holds by construction.

Questions

Both Deployments on every deploy: intended, as you say. Since the tag is the commit, both images change on every merge, so both always roll, and they roll in parallel. The wait is roughly the slower one, not the sum. It's "one commit = one release" being honest.
Renames without --prune: the old object stays and keeps serving (a renamed Service or Ingress is the dangerous case). I'm adding that to k8s/README.md: a rename = the new name in the PR, plus deleting the old object by hand when it merges.

The two manual steps: I'll do both right after merging, today: deployer.yaml first, then the script. I'll post the check output here (auth can-i list events as the deployer, and the diff of the installed script against git).

devinder-dev · 2026-10-29T14:13:50Z

Both manual steps are done (deployer.yaml first, then the script), checked from outside:

auth can-i as system:serviceaccount:relay:deployer
  list events                 yes
  list events.events.k8s.io   yes
  get secrets                 no
  delete deployments          no

/usr/local/bin/relay-deploy   root:root 755   sha256 0081160b…  ==  origin/main:infra/server/relay-deploy.sh

The follow-up PR with your four line comments comes next.

Follow-up to #14, from Abdikhani's review of it. ## `infra/server/relay-deploy.sh` - **Lock (line 30):** waits 15 minutes instead of 5, which is longer than the worst case of a deploy plus its rollback. If it still can't get the lock, it says "nothing was changed, re-run this job", so the failure is obviously retryable. In CI this rarely waits at all, because the pipeline's `concurrency` group already runs main's deploys one after the other. The lock protects against an admin deploying by hand at the same moment. The deploy job's timeout goes from 20 to 30 minutes in #16. - **Namespace (line 43):** a comment saying every kubectl call gets namespace `relay` from the kubeconfig's context, set by `setup-deploy-user.sh`. - **`return 0` (line 81):** `deploy_commit` now ends with an explicit `return 0`, so "0 on success" holds by construction. - **180 s per rollout:** unchanged on purpose. With one node, an image is downloaded once, and each commit only changes our small code layers. The first deploy's log will show the real time. ## `infra/server/test-relay-deploy.sh` - **Test 10:** when another deploy holds the lock, the script exits 1, tells you to re-run, and changes nothing. Result: `29 passed, 0 failed`, and `shellcheck` is clean. ## `k8s/README.md` - **Renames** are the dangerous case without `--prune`: the old object stays and keeps serving. A rename = the new name in the PR, plus deleting the old object right after it merges. - **The rule from your review:** server and platform changes are applied by the one who merges, right after merging, and the check output is posted on the PR. After merging: install the script again and post the `diff` check here.

Step 1.8: every merge to main is deployed, then checked from outside. Rebased onto main now that #12, #14 and #15 are merged. **Merge only after the switch-over below.** ## What changes - **`api.yml` → `pipeline.yml`.** It now deploys too, and the dashboard's jobs will join it later (`needs:` only works inside one workflow file). - **No path filter on `main`.** Every merge is a release: a deploy needs an image tagged with exactly the merged commit, even when only `k8s/app/` or the docs changed. - **New job `deploy-script-test`** on every PR and merge. It runs `infra/server/test-relay-deploy.sh` (relay-deploy's gates and rollback, with fake git and kubectl) and `shellcheck` on the server scripts. The deploy needs it, so a broken deploy script never reaches the server. - **New job `deploy`**, after `api-image` and `deploy-script-test`, only on a merge to main: - **`environment: production`:** the only way to get `DEPLOY_SSH_KEY`, and only on main. - **`permissions: {}`:** no GitHub access at all. - **`timeout-minutes: 30`:** the worst case is the server's lock (15 min, #19) plus a rollout and a rollback. - **Step 1:** writes the key and our server's `known_hosts`, then `ssh … deploy@<vps> "$GITHUB_SHA"` (`StrictHostKeyChecking=yes`, `BatchMode=yes`). The exit code of `relay-deploy` becomes the step's. - **Step 2:** asks `https://api.myrelay.tech/version` until it answers this commit, for up to 36 × 5 s, because the first deploy also waits for a new certificate. - **`timeout-minutes: 10`** on `api-test` and `api-image` (from the #12 review). - **`k8s/hello/` is deleted**, together with its row in `k8s/README.md`. The API's Ingress takes over `api.myrelay.tech`. ## Tested - `actionlint` 1.7.7 (with shellcheck on the `run:` scripts): no findings. - The new job's two steps, run locally: `26 passed, 0 failed`, and `shellcheck` is clean. - The version check, run on its own with `bash -eo pipefail` against a local container: - the right SHA → passes; - an old SHA → keeps asking, then fails; - nothing answering → "no answer" on every try, then fails. - The rebase was rehearsed in a throwaway clone first: the 3 commits replayed onto main without conflicts. ## Already done before merging - #12, #14 and #15 merged. - `deployer.yaml` applied: the deployer can read events, and still can't read Secrets or delete. - `relay-deploy` installed, byte-identical to main. - The image is pullable without credentials (ADR 0004, #18), and the server has already pulled `relay-api:a8274c1`. ## Switch-over (right before merging) 1. Delete the hello app from `default`: the Ingress, Service and Deployment, plus its leftover TLS Secret `api-myrelay-tech-tls`. `api.myrelay.tech` is down for about 1–2 minutes, once. 2. Merge → the pipeline tests, builds, deploys → `/version` answers the commit. ## After the first deploy - **A real rollback:** deploy #15's merge commit `a78934c` by hand. It has no image, so the rollout fails and `relay-deploy` redeploys the last good commit, while a curl loop shows only 200s. - **Zero downtime:** a load loop during a normal deploy (merging #19) should show no failed requests. - Then make `api-test` and `deploy-script-test` required checks.

infra: roll back to the last good commit and wait for every Deployment

8f42893

devinder-dev requested a review from AbdikhaniAbdirashid October 29, 2026 10:25

This was referenced Oct 29, 2026

infra: add the API manifests (2 pods, probes, zero-downtime rollout) #15

Merged

infra: deploy every merge to main and check that it's live #16

Merged

infra: test the deploy gates, show why a rollout failed, check the la…

2769579

…st-good note

devinder-dev mentioned this pull request Oct 29, 2026

infra: add API pipeline (test, build, push to GHCR) #12

Merged

AbdikhaniAbdirashid approved these changes Oct 29, 2026

View reviewed changes

devinder-dev merged commit 7187e54 into main Oct 29, 2026

devinder-dev deleted the infra/deploy-rollback branch October 29, 2026 14:10

devinder-dev mentioned this pull request Oct 29, 2026

infra: deploy script follow-ups from the #14 review #19

Merged

This was referenced Oct 29, 2026

docs: decide that our container images stay public (ADR 0004) #18

Merged

infra: capture the server's base setup, a rebuild runbook and the firewall fix #17

Merged

Navigation Menu

Repository navigation

infra: roll back to the last good commit and wait for every Deployment - #14

infra: roll back to the last good commit and wait for every Deployment#14
devinder-dev merged 2 commits into
mainfrom
infra/deploy-rollback

devinder-dev commented Oct 29, 2026 •

edited

Loading

Uh oh!

AbdikhaniAbdirashid left a comment

Uh oh!

AbdikhaniAbdirashid Oct 29, 2026

Uh oh!

AbdikhaniAbdirashid Oct 29, 2026

Uh oh!

AbdikhaniAbdirashid Oct 29, 2026

Uh oh!

AbdikhaniAbdirashid Oct 29, 2026

Uh oh!

devinder-dev commented Oct 29, 2026

Uh oh!

devinder-dev commented Oct 29, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

devinder-dev commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes

The three endings of a failed deploy

Tested

After merging

Added after an independent review of Phases 0–1

Uh oh!

AbdikhaniAbdirashid left a comment

Choose a reason for hiding this comment

Uh oh!

AbdikhaniAbdirashid Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

AbdikhaniAbdirashid Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

AbdikhaniAbdirashid Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

AbdikhaniAbdirashid Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

devinder-dev commented Oct 29, 2026

Uh oh!

devinder-dev commented Oct 29, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

devinder-dev commented Oct 29, 2026 •

edited

Loading