Skip to content

fix(guest): decouple Gateway outage from app boot - #948

Merged
kvinwang merged 8 commits into
masterfrom
codex/fix-guest-gateway-outage-boot
Aug 5, 2026
Merged

fix(guest): decouple Gateway outage from app boot#948
kvinwang merged 8 commits into
masterfrom
codex/fix-guest-gateway-outage-boot

Conversation

@kvinwang

@kvinwang kvinwang commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Problem

Guest boot treated temporary Gateway unavailability as fatal even when the application and local services could start without it. A control-plane outage therefore prevented otherwise healthy workloads from booting.

Root cause and fix

Separate Gateway registration/refresh failure from the application boot critical path and retry it after boot.

Implementation

The branch records the following focused implementation work:

  • fix(guest): decouple Gateway outage from app boot

Changed paths:

  • dstack/dstack-util/src/system_setup.rs
  • os/common/rootfs/wg-checker.sh

Scope

This PR addresses one logical guest finding. It intentionally excludes the acceptance-test infrastructure from #841 and unrelated product fixes from #840.

Dependency and merge order

This PR is based directly on master and does not require another split product PR to merge first.

Verification

  • git diff --check origin/master..origin/codex/fix-guest-gateway-outage-boot: passed.
  • The declared base was verified as an ancestor of the PR head.
  • Focused compile/check verification was run for the changed component where applicable; non-Rust packaging or configuration changes were reviewed against their exact branch delta.

Copilot AI review requested due to automatic review settings July 31, 2026 03:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Follow-up to the boot decoupling. Three problems with retrying gateway
registration after boot:

1. `GatewayKeyStore::save()` ran only after `register_cvm` succeeded, so
   during a gateway outage the cache in /run was never written and every
   retry re-minted the key store: a KMS round-trip, two cert signing
   requests and a TDX quote. A gateway outage is fleet-wide, so a fixed
   30s retry had the whole fleet hammering the KMS. Persist the key store
   as soon as it is minted, and back the retry off 30s -> 60s -> 120s.

2. `setup()` returns early when the app never enabled dstack-gateway, so
   those CVMs never have a WireGuard config and the missing-config retry
   spun forever on them. The checker now reads `gateway_enabled()`
   directly and exits 0; the unit switches to Restart=on-failure.

3. A missing gateway app id or gateway URL is a deployment mistake, not
   an outage. `GatewayRefresher::check_config` separates those so the
   checker fails loudly instead of retrying something that can never
   succeed.

wg-checker.sh is replaced by `dstack-util gateway-checker`. The refresh
decision is a pure function of an observation, so the three trigger
conditions, the backoff and the handshake timing are unit tested without
a gateway, a KMS or a WireGuard interface.
…unit

Boot no longer fails when gateway registration fails, so a guest log line
was the only trace of it: the VM reported a clean boot while having no
ingress route at all. Report `boot.error` to the host instead, which the
VMM already surfaces via VmInfo and vmm-cli.

The report is retracted once the route comes back. The checker seeds its
notion of "already reported" from whether a WireGuard config exists at
startup rather than assuming health, so a boot-time failure that the
checker later recovers from does not leave a stale error on screen
forever. Reporting only on transitions keeps the event ring buffer clean.

The unit is renamed wg-checker.service -> dstack-gateway-checker.service
now that it supervises gateway registration rather than poking WireGuard.
The rootfs is read-only and rebuilt per image, so no upgrade path is
needed; acceptance asserts the old names are gone.
Conflict: master fixed wg-checker.sh in two commits while this branch
deleted it in favour of `dstack-util gateway-checker`. Resolved by
keeping the deletion and porting both fixes into the Rust checker --
see the follow-up commit, which is where the actual behaviour change is.

Master also carried two changes this branch's base predates and that
apply directly to the code here: gateway-cache.json and the WireGuard
config are now written through safe_write_with_mode(0o600) rather than
fs::write plus a follow-up chmod. Both merged cleanly and are kept.
Master fixed two bugs in wg-checker.sh after this branch was cut, and the
Rust rewrite reintroduced both:

1. Staleness was checked after the periodic refresh instead of before it.
2. Every refresh cleared the missing-handshake timer.

Either one alone makes the forced branch unreachable for a peer that
never handshakes: HANDSHAKE_TIMEOUT equals REFRESH_INTERVAL, so the
periodic refresh at last_attempt+180 reset a timer that had only reached
170. That matters because gateway setup short-circuits on an unchanged
config unless force is set, so a CVM whose WireGuard config is correct
but whose tunnel is dead could only ever recover through the forced path
it could not reach.

handshake_missing_since now means exactly one thing -- when we first
observed a peer with no handshake -- and is cleared by an observed
handshake and by nothing else. Staleness is evaluated first and returns
either way, since a periodic refresh cannot rebuild a dead tunnel.

Reaching the forced branch exposes the third thing master added and this
branch lacked: nothing rate limited it, so a gateway that stays down
would be bounced and re-certified on every 10s poll. last_force is
tracked separately from last_attempt (a cheap periodic refresh must not
re-arm the limit that guards the expensive forced one) and allows one
forced attempt per HANDSHAKE_TIMEOUT.

The tests now replay master's verification table over 900s of simulated
time at the real poll interval, so a future refactor that breaks these
semantics fails against the same numbers that commit was verified with:

  scenario                     forced
  healthy handshakes                0
  never handshakes                  4
  handshake frozen, gw down         4
  handshake then peer gone          3
  no handshake, gw recovers         2, then periodic

The old `gives_a_new_interface_a_full_timeout_to_handshake` asserted only
that *some* refresh happened at t=189 and passed against the broken
ordering, because the periodic refresh satisfied it. It now pins which
refresh fires at which poll.
Three findings from re-reading the poll loop end to end.

1. `observe()` forked `wg show` on every poll even with no WireGuard
   config, where `decide()` never looks at the handshake. That is exactly
   the state a gateway outage parks the CVM in, so the useless fork ran
   every 10s for the whole outage. Probe only when a config exists.

2. The checker re-registered immediately at startup, seconds after boot
   had just registered the same CVM. /etc is a volatile overlay, so a
   config on disk can only mean this boot wrote it; treat that as the
   start of the periodic clock instead. A fleet rebooting together no
   longer serves the gateway a second full round of registrations right
   after the first. With no config, boot's registration failed and fast
   recovery is the point, so the first poll still acts immediately.

3. `notify_q` swallows its error, so a host-API blip while reporting or
   retracting the gateway error flipped `reported_degraded` anyway and
   the state was never re-sent. A stale error could sit on the VMM for
   the life of the VM, or an outage could go unreported. Use `notify` and
   only move the flag once the host has actually taken the message.

Considered and rejected: replacing the fixed 10s poll with a sleep until
the next deadline. It would cut wakeups from ~8.6k/day to a few hundred,
but the loop now costs one stat() per poll in the outage case, the CPU
saving is on the order of 0.03%, and it would mean encoding the same
deadlines in both `decide()` and a `next_wake()` that must never drift
apart. Not worth the second source of truth.
Checking notify's result meant the loop had to carry a third state --
degraded, and the host may or may not know -- so the two flag flips grew
into two nested matches and doubled the body of the refresh arm. That is
a lot of machinery for a host-API blip that the next transition into or
out of the degraded state reports again anyway.

Back to notify_q, with the tradeoff written down where the flag is
seeded. The observe() and startup-registration changes from the previous
commit stand.
Two changes, one dropping state and one closing the last failure mode
that had no automatic recovery.

Drop reported_degraded. Mirroring gateway state to the host meant the
loop had to remember what the host had been told, and that memory bought
nothing the loop needs: recovery does not depend on it. Boot still
reports the one signal that matters -- this CVM came up without a route.
The cost is that the reported error stays on the VMM after the checker
recovers, until the VM restarts, which is the intended trade.

Add a systemd watchdog. Every recovery path here only runs while the
loop runs, and nothing below the loop could guarantee that: a refresh
that never returns leaves a healthy-looking process that Restart= would
never fire on, and the CVM sits without a route indefinitely. That is
not hypothetical -- a refresh spends most of its time in blocking cmd!
shell-outs, and wg-quick resolves peer endpoints, so a hung DNS lookup
is enough.

tokio::time::timeout cannot fix this: those calls block a runtime worker
with no await point to cancel at. Neither can a watchdog task on another
worker, which would keep pinging while this loop is wedged. The ping has
to come from the loop itself, before the work rather than after, so that
a refresh which never returns stops the pings. systemd then kills and
restarts the service, covering a hang whatever its cause.

Follows the Type=notify + WatchdogSec + sd_notify pattern already used
by dstack-guest-agent. Readiness is reported before the checker decides
whether it has anything to do, so the exit-0 (gateway disabled) and
exit-3 (misconfigured) paths remain a started service that then stopped.
WatchdogSec=600 clears the worst legitimate refresh, which can span KMS
certificate requests plus every configured gateway URL at 60s each.

Verified end to end against a stub notify socket: the process emits
READY=1 at startup and WATCHDOG=1 on each iteration, and stays inert
when the unit does not arm the watchdog, so manual runs still work.
@kvinwang
kvinwang merged commit d50b75a into master Aug 5, 2026
18 checks passed
@kvinwang
kvinwang deleted the codex/fix-guest-gateway-outage-boot branch August 7, 2026 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants