Uh oh!
There was an error while loading. Please reload this page.
ci: reserve the Supabase ports before supabase start (both lanes) - #588
Merged
Conversation
Run 32624263094 (push to main, 9f34454) died at `Start Supabase` with `failed to bind host port for 0.0.0.0:54322: address already in use`, and the identical commit re-run 40 minutes later passed. The runner is a fresh VM, so nothing is left over. The mechanism is that Supabase's ports (54320-54329) sit inside Linux's default ephemeral source-port range (32768-60999): the attempt-1 log shows dozens of image pulls finishing and the bind error landing in the same second, so an outbound connection had been handed 54322 as its source port. Reserve the ports out of automatic assignment (explicit bind() is unaffected, so Docker can still publish them), derived from supabase/config.toml so moving a port cannot un-protect it. Then diagnose what reservation cannot undo: remove leftover containers publishing them, fail by name on a foreign LISTENer, and wait out a draining socket. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The leftover-container cleanup was the one pipeline in the step without a
guard. Under the runner's `bash -e` (confirmed in run 32627212767's log:
`shell: /usr/bin/bash -e {0}`), a `docker rm -f` failure — permission, or a
race with another remover — aborted the step on the spot: no `preflight:`
diagnostic, no LISTEN check, no drain wait, no summary line. That is exactly
the opaque-CI-failure class this step exists to remove.
Warn and continue instead, consistent with the sysctl and drain branches;
the LISTEN check immediately after already names anything that survived.
Reproduced against the pre-fix commit: exit 123 with no output after the
docker error. With the guard: WARNING, then the step runs to completion.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 5399744 | Commit Preview URL Branch Preview URL | Aug 26 2026, 05:31 AM |
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds a CI-only Supabase port preflight script. E2E and integration workflows run it after checkout. The integration workflow also retries ChangesSupabase port preflight
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk:⚪ Minimal · up to This PR adds preflight protection for Supabase port collisions in CI and reports successful validation; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant preflight-ports.sh
participant Linux kernel
participant ss
participant Supabase CLI
GitHub Actions->>preflight-ports.sh: Run port preflight after checkout
preflight-ports.sh->>Linux kernel: Reserve configured ports
preflight-ports.sh->>ss: Probe configured bind ports
ss-->>preflight-ports.sh: Return socket status
preflight-ports.sh-->>GitHub Actions: Return preflight status
GitHub Actions->>Supabase CLI: Run supabase start
Supabase CLI-->>GitHub Actions: Return startup status
GitHub Actions->>Supabase CLI: Run supabase stop after failure
GitHub Actions->>Supabase CLI: Retry supabase start
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (2 skipped: 2 unsupported.) Full details: Description checkExplanation The description is detailed and on-topic. It explains the failure, mechanism, implementation, affected workflows, testing evidence, issue reference, and review considerations. It does not use the repository template headings exactly, but it provides the required information. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
…sh (#574) Merge-gate review of #588 found 15 issues in the inline step. The whole body moves to scripts/preflight-ports.sh — testable, shellcheckable, and called by BOTH lanes (integration.yml, and e2e.yml, which reaches `supabase start` through `make e2e-up` and had the identical exposure). What changed beyond the move: - Two port sets, not one. RESERVE = every config.toml *port inside the LIVE ephemeral range (read from /proc, not hardcoded). CHECK = the ports `supabase start` actually binds — ENABLED sections only, minus shadow_port, unfiltered by range. Only CHECK can fail or stall the job, so a holder on the disabled pooler's 54329 no longer fails an unfiltered PR gate; enabled- ness is derived per section, so flipping `enabled = true` is picked up. - Parser: `[a-z_]*port` could not match the digit-bearing `pop3_port`; values are now matched as bare integers (TOML underscores accepted) and anything else is reported by line and key instead of being coerced to 0 and dropped. - No more fail-open probes: the `ss` call and the awk filters branch on their own exit status, so a probe that could not run never reads as "free". - The sysctl is verified. A failed read of the current value skips the write rather than clobbering a reservation it cannot see; the merge keeps existing RANGES intact; the value is read back and checked by membership (the kernel normalises to ranges, so a string compare would always mismatch); and the summary line says NOT reserved when any of that fails. - Drain: budget 30s -> 75s, because a TIME-WAIT entry lives a fixed 60s and SO_REUSEADDR does NOT let a bind step over it (Linux needs the option on both sockets, and an outbound connection never set it). ESTABLISHED is no longer described as transient. - The docker-container sweep is deleted — it was wrong in five ways where it could fire. `supabase start` gets e2e-up.sh's stop-and-retry instead, which also covers exited/stale projects the sweep could not see. Refs #574 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
supabase startsupabase start (both lanes)…guarded substitutions, validated timeout Three non-blocking findings from the merge-gate re-review of #588: - Every sudo call now uses `-n`. The socket probe already gated on `sudo -n true`, but the sysctl write did not, so a hand-run on a box without passwordless sudo would have blocked on a password prompt instead of failing into the diagnostic half the header promises. The probe command itself is `sudo -n ss` too, closing the window where the gate passes and the sudo timestamp expires before the probe. - The record-splitting command substitutions and the readback membership check branch on their status like the probes do. They are in-memory string ops that realistically cannot fail, but "realistically cannot fail" is the reasoning that produced the fail-open probe this rework had to fix: a failed `check_csv` pipeline would have printed "no ENABLED service binds a port" and exited 0 having checked nothing, and a failed membership check would have claimed the ports were reserved. The merged-list build is guarded for a sharper reason still — an empty value would CLEAR the bitmap it extends. - PREFLIGHT_DRAIN_TIMEOUT is validated as a whole number. A non-numeric value made every `-ge` test fail and spun the drain loop to the job timeout — a worse failure than the one being guarded. It warns and falls back to 75 rather than failing a lane over a typo. Harness: 20 cases / 69 assertions green under both awk and `gawk --posix`, including the three new ones (bad timeout value, empty override, and a sudo shim that fails any call arriving without `-n`). shellcheck 0.11.0 clean. Refs #574 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What failed
Run 32624263094 (push to
main,9f34454) died at
Start Supabase:The identical commit, re-run 40 minutes later, passed. No code cause — a day of signal went to
chasing it.
The mechanism
The runner is a fresh VM per job, so nothing is left over. But Supabase's ports (54320–54329) sit
inside Linux's default ephemeral source-port range, 32768–60999. Any outbound TCP connection the
job makes can be handed 54322 as its source port, and Docker's later
bind(0.0.0.0:54322)thenfails.
The attempt-1 log times it exactly: the last of dozens of
ghcr.ioimage pulls completes at06:58:38.6156,Starting database...is logged 0.03s later, and the bind fails. Those pulls arehundreds of outbound connections. One of them took the port.
Note the pulls happen inside
supabase start— so a pre-flight check would have passed. The fixhas to be pre-emptive.
What the guard does
scripts/preflight-ports.sh, called right aftercheckoutin both lanes that boot Supabase —integration.ymlande2e.yml(which reachessupabase startthroughmake e2e-up→scripts/e2e-up.sh). Right after checkout is the earliest pointsupabase/config.tomlexists, andit puts every later download —
setup-python,setup-cli,npm ci,playwright install— behindthe reservation. Bash only; no new actions or dependencies.
net.ipv4.ip_local_reserved_ports, which excludes them fromautomatic source-port assignment while leaving explicit
bind()untouched — so Docker can stillpublish them. Merges with any existing value (idempotent), then reads the value back and
verifies membership before claiming anything is reserved.
supabase startwill actually bind,printing its state, address, pid and program. It never kills what isn't ours.
what it's waiting on: the one case reservation can't fix retroactively, because it was allocated
before the step ran. On timeout it warns and proceeds rather than failing on a guess.
preflight: ports 54321,54322,54323,54324 free (enabled services; reserved from the ephemeral range)— and the word free only ever appears when the ports wereobserved free.
Two different port sets, both derived from
supabase/config.tomlrather than hardcoded:*portkey inside the live ephemeral range (read from/proc, so thepremise is re-verified in every run's log). Enabled or not: reserving a port nothing binds costs
nothing, and it keeps the protection in place the day someone flips
enabled = true.supabase startwill actually bind:*portkeys of enabled sections,minus
shadow_port(onlydb diffbinds that), and not range-filtered. Only this set can failor stall the job, so a holder on the disabled pooler's 54329 can't red an unfiltered PR gate.
supabase startalso picks upe2e-up.sh's stop-and-retry, which covers stale/exited projects andthe preflight's admitted give-up path.
Merge-gate review rework (2026-08-26) — commit
d992c12fA
/code-reviewat the merge gate returned 15 findings against the original inline step. All ofthem are addressed here; the whole body moved out of the workflow into
scripts/preflight-ports.shso it is testable and shellcheckable, and so one implementation serves both lanes.
[a-z_]*portcan't matchpop3_port(digit in the key)port/[a-z0-9_]+_port, anchored so a key merely ending in "port" can't slip inv = $2 + 0silently coerced54_322→54 and"54322"→0_separators accepted); anything else is reported by line and key as unprotectedbusy=$(sudo ss … || true)was fail-open/proc/sys/net/ipv4/ip_local_port_range; it filters the RESERVE set only, the CHECK set stays unfilteredsupabase startgetse2e-up.sh's stop-and-retry instead, which also covers the exited/stale projectsdocker pscouldn't seeenabled = trueon the pooler moves 54329 into the fail set with no edit54320-54324,54327,54329, so a string compare would mismatch on every success)docker rmscripts/preflight-ports.sh, call from both lanessupabase startfailure recovery in this lanescripts/e2e-up.sh:219-226|| trues, duplicated regex build, repeated indent pipeline,waited -eq 0sentinelindent()helper, one regex build, a namedannouncedflag, no unguarded|| trueTest evidence
Branch-coverage harness — 20 cases, 69 assertions, all green, run twice: under the default awk
and under
gawk --posix(stand-in for the runner's mawk). It builds a throwaway repo root per case,copies the real script in with only its two
/procconstants repointed at fixtures (the patch isgrep-verified — a silently-failed patch produced a false GREEN in the previous round), and drives it
with
sudo/ssshims. Thesudoshim emulates the kernel: it expands ranges, stores a set, andprints it back collapsed into ranges.
Covered: clean path · digit-bearing
pop3_portprotected ·54_322parsed as 54322 · non-integervalue diagnosed by line+key · probe failure → WARNING, never "free" · sysctl fails → summary says NOT
reserved · sysctl "succeeds" but readback lacks the ports → WARNING · foreign LISTENer on an enabled
port → exit 1 naming the pid · LISTENer on the disabled pooler's port → no fail · same holder after
flipping the pooler to
enabled = true→ now fails (the derivation is live) ·shadow_portreservedbut never checked · TIME-WAIT drain waits, announces once, proceeds clean · drain timeout → WARNING +
continue · no port keys → exit 1 naming both branches · ports outside the ephemeral range → nothing
to reserve, checks still run and still fail · unreadable reservation → write skipped · pre-existing
1024-1030range survives the merge · no passwordless sudo → unprivileged probe · noss→"unverified", not "free" · non-numeric drain timeout → loud fallback · a
sudoshim that fails any call arriving without-n.shellcheck 0.11.0 -x— clean, zero findings (the previous round had no shellcheck available).Re-review residuals — commit
53997446The re-review passed all 15 findings and raised three non-blocking residuals, closed in one commit:
sudocall now uses-n. The probe gated onsudo -n true, but the sysctl write didnot — a hand-run on a box without passwordless sudo would have blocked on a password prompt
instead of degrading to the diagnostic half the header promises. The probe command is
sudo -n sstoo, closing the window where the gate passes and the timestamp expires before the probe.
readback membership) — in-memory string ops that realistically cannot fail, but that is the
reasoning which produced the fail-open probe F4 condemned: a failed
check_csvwould have printed"no ENABLED service binds a port" and exited 0 having checked nothing, and an empty merged list
would have cleared the bitmap it extends.
PREFLIGHT_DRAIN_TIMEOUTis validated as a whole number — a non-numeric value made every-getest fail and spun the drain loop to the job timeout. It warns and falls back to 75.Lane runs on this branch @
53997446— both greenintegration.yml(dispatch)71 passed, 2231 deselected(real work, zero skipped)e2e.yml(dispatch)73 passed (3.8m)PlaywrightPlus the PR-triggered CI and
integration runs, both green.
(The previous SHA
d992c12fwas green on the same four lanes:32932741569 /
32932743439.)
Both lanes printed the same three lines on the runner:
Read that middle line closely: the kernel handed the value back range-collapsed, which is
exactly why F8's readback is a membership check and not a string compare — and the absence of a
mismatch WARNING is the membership check passing on real hardware.
54322is inside32768-60999on GitHub's own runner, so the diagnosis holds where it matters, and it is re-verified in every run's
log forever.
Cannot prove a negative
One green run per lane does not prove the collision is gone — the failure rate was already low. What
the change guarantees is (a) the kernel will not hand out these ports as source ports any more,
which removes the mechanism, and (b) if a lane ever goes red on ports again, the log names the
culprit instead of leaving a bare
address already in use.Refs #574