Skip to content

Commit b5b111f

Browse files
committed
fix(guest): make the gateway checker loop self-recovering
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.
1 parent 17d3368 commit b5b111f

6 files changed

Lines changed: 79 additions & 45 deletions

File tree

dstack/Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dstack/dstack-util/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ serde.workspace = true
2525
serde-human-bytes.workspace = true
2626
semver.workspace = true
2727
serde_json.workspace = true
28+
sd-notify.workspace = true
2829
sha2.workspace = true
2930
tokio = { workspace = true, features = ["full"] }
3031
tracing.workspace = true

dstack/dstack-util/src/gateway_checker.rs

Lines changed: 56 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,10 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH};
3333

3434
use anyhow::{Context, Result};
3535
use cmd_lib::run_fun as cmd;
36+
use sd_notify::NotifyState;
3637
use tracing::{error, info, warn};
3738

38-
use crate::system_setup::{
39-
gateway_unavailable_message, GatewayRefresher, WG_CONFIG_PATH, WG_INTERFACE,
40-
};
39+
use crate::system_setup::{GatewayRefresher, WG_CONFIG_PATH, WG_INTERFACE};
4140

4241
/// How often the loop samples the world.
4342
const POLL_INTERVAL: Duration = Duration::from_secs(10);
@@ -279,7 +278,45 @@ fn observe(now: i64) -> Observation {
279278
}
280279
}
281280

281+
/// systemd liveness reporting, inert when the unit has no `WatchdogSec`.
282+
///
283+
/// The recovery paths in this loop only work while the loop runs, and nothing
284+
/// below it can guarantee that: a wedged refresh leaves a healthy-looking
285+
/// process that systemd will never restart. Handing liveness to systemd covers
286+
/// a hang wherever it comes from, including causes not anticipated here.
287+
struct Watchdog {
288+
enabled: bool,
289+
}
290+
291+
impl Watchdog {
292+
/// Report readiness and arm the watchdog. Readiness is sent before the
293+
/// checker decides whether it has anything to do, so the paths that exit
294+
/// straight away are still a started service that then stopped, not a
295+
/// service that failed to start.
296+
fn arm() -> Self {
297+
let mut usec = 0;
298+
let enabled = sd_notify::watchdog_enabled(false, &mut usec);
299+
if let Err(error) = sd_notify::notify(false, &[NotifyState::Ready]) {
300+
warn!("failed to report readiness to systemd: {error}");
301+
}
302+
if enabled {
303+
info!("systemd watchdog armed, timeout={usec}us");
304+
}
305+
Self { enabled }
306+
}
307+
308+
fn ping(&self) {
309+
if !self.enabled {
310+
return;
311+
}
312+
if let Err(error) = sd_notify::notify(false, &[NotifyState::Watchdog]) {
313+
warn!("failed to ping the systemd watchdog: {error}");
314+
}
315+
}
316+
}
317+
282318
pub async fn cmd_gateway_checker(args: GatewayCheckerArgs) -> Result<()> {
319+
let watchdog = Watchdog::arm();
283320
let refresher =
284321
GatewayRefresher::load(&args.work_dir).context("failed to load gateway configuration")?;
285322

@@ -306,41 +343,33 @@ pub async fn cmd_gateway_checker(args: GatewayCheckerArgs) -> Result<()> {
306343
}
307344

308345
info!("watching dstack-gateway registration");
309-
let vmm = refresher.host_api();
310-
// Seed from the observable world rather than assuming health: no WireGuard
311-
// config here means boot-time registration failed and already reported it,
312-
// so the first success owes the host a retraction. Assuming health instead
313-
// would leave that boot error on screen forever after we recover.
314-
//
315-
// Reporting is deliberately fire-and-forget. Tracking delivery would mean
316-
// carrying a third state ("degraded, and the host may or may not know")
317-
// through the loop to cover a host-API blip that the next refresh cycle
318-
// already re-reports on the way in or out of the degraded state.
319-
let config_present = wg_config_present();
320-
let mut reported_degraded = !config_present;
321-
let mut checker = Checker::starting(now_secs(), config_present);
346+
// The checker does not report gateway state to the host. Boot already
347+
// reports the one signal that matters -- this CVM came up without a route
348+
// -- and mirroring every later transition would mean tracking what the host
349+
// has been told, which is state this loop should not have to carry. The
350+
// consequence is that a boot error stays on the VMM after the checker
351+
// recovers, until the VM restarts.
352+
let mut checker = Checker::starting(now_secs(), wg_config_present());
322353
loop {
354+
// Ping before the work, not after, so a refresh that never returns
355+
// stops the pings. Nothing else can do this for us: a refresh spends
356+
// most of its time in blocking `cmd!` shell-outs (`wg-quick up` alone
357+
// resolves peer endpoints), which occupy a runtime worker with no await
358+
// point. tokio::time::timeout cannot cancel that, and a watchdog task
359+
// on another worker would happily keep pinging while this loop is
360+
// wedged. Only the loop itself can prove the loop is alive.
361+
watchdog.ping();
362+
323363
let now = now_secs();
324364
if let Some(refresh) = checker.decide(observe(now)) {
325365
info!("refreshing dstack-gateway: {}", refresh.reason);
326366
let succeeded = match refresher.refresh(refresh.force).await {
327367
Ok(()) => {
328368
info!("dstack-gateway refresh succeeded");
329-
if reported_degraded {
330-
info!("dstack-gateway route restored; clearing the reported error");
331-
// Empty body resets the host's boot_error field.
332-
vmm.notify_q("boot.error", "").await;
333-
reported_degraded = false;
334-
}
335369
true
336370
}
337371
Err(error) => {
338372
warn!("dstack-gateway refresh failed: {error:#}");
339-
if !reported_degraded {
340-
vmm.notify_q("boot.error", &gateway_unavailable_message(&error))
341-
.await;
342-
reported_degraded = true;
343-
}
344373
false
345374
}
346375
};

dstack/dstack-util/src/system_setup.rs

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1936,14 +1936,6 @@ impl Stage0<'_> {
19361936
}
19371937
}
19381938

1939-
/// The message reported to the host while this CVM has no gateway route.
1940-
///
1941-
/// Boot and the gateway checker share it so the operator sees one consistent
1942-
/// string no matter which of the two noticed the outage.
1943-
pub fn gateway_unavailable_message(error: &anyhow::Error) -> String {
1944-
format!("dstack-gateway registration failed, the app has no ingress route: {error:#}")
1945-
}
1946-
19471939
/// Owns the inputs needed to (re)register this CVM with dstack-gateway.
19481940
///
19491941
/// Loading is separated from refreshing so a long-running caller (the gateway
@@ -1974,14 +1966,6 @@ impl GatewayRefresher {
19741966
self.shared.app_compose.gateway_enabled()
19751967
}
19761968

1977-
/// Client for reporting guest state back to the host.
1978-
pub fn host_api(&self) -> HostApi {
1979-
HostApi::new(
1980-
self.shared.sys_config.host_api_url.clone(),
1981-
self.shared.sys_config.collateral_urls().pccs,
1982-
)
1983-
}
1984-
19851969
/// Validate the parts of the gateway config that can never become valid by
19861970
/// waiting. These are deployment mistakes, not outages, so callers that
19871971
/// retry should give up instead of looping forever.
@@ -2923,7 +2907,12 @@ impl Stage1<'_> {
29232907
// visible from the VMM. The gateway checker clears this once it
29242908
// manages to register.
29252909
self.vmm
2926-
.notify_q("boot.error", &gateway_unavailable_message(&error))
2910+
.notify_q(
2911+
"boot.error",
2912+
&format!(
2913+
"dstack-gateway registration failed, the app has no ingress route: {error:#}"
2914+
),
2915+
)
29272916
.await;
29282917
}
29292918
self.vmm

os/common/rootfs/dstack-gateway-checker.service

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,16 @@ After=network-online.target dstack-prepare.service
44
Wants=network-online.target
55

66
[Service]
7-
Type=simple
7+
Type=notify
88
ExecStart=/bin/dstack-util gateway-checker --work-dir /dstack
9+
# Every recovery path in the checker only runs while its loop runs, and a loop
10+
# that wedges leaves a healthy-looking process systemd would never restart. The
11+
# loop pings the watchdog itself, so a hang anywhere -- including the blocking
12+
# wg-quick/iptables shell-outs a refresh performs, which no in-process timeout
13+
# can cancel -- gets the service killed and restarted. The timeout is generous
14+
# because one refresh may legitimately spend minutes across KMS certificate
15+
# requests and every configured gateway URL.
16+
WatchdogSec=600
917
# The checker exits 0 when the app never enabled dstack-gateway, because there
1018
# is then nothing to supervise. Restart=always would respawn that exit forever.
1119
Restart=on-failure

os/mkosi/tests/acceptance.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,12 @@ exit_code=$(sed -n 's/^const EXIT_MISCONFIGURED: i32 = \([0-9]\+\);$/\1/p' "$che
8686
[[ -n $exit_code ]] || { echo "cannot read EXIT_MISCONFIGURED from $checker_src"; exit 1; }
8787
grep -q "^RestartPreventExitStatus=${exit_code}\$" "$gw_unit" || {
8888
echo "dstack-gateway-checker.service must set RestartPreventExitStatus=$exit_code"; exit 1; }
89+
# The loop's recovery paths only run while the loop runs, so systemd has to be
90+
# the thing that notices a wedge. WatchdogSec is useless without Type=notify.
91+
grep -q '^Type=notify$' "$gw_unit" || {
92+
echo 'dstack-gateway-checker.service must use Type=notify to arm the watchdog'; exit 1; }
93+
grep -q '^WatchdogSec=' "$gw_unit" || {
94+
echo 'dstack-gateway-checker.service must set WatchdogSec'; exit 1; }
8995
test ! -e "$D/../common/rootfs/wg-checker.sh"
9096
test ! -e "$D/../common/rootfs/wg-checker.service"
9197
# systemd enables any unit that matches no preset rule, so the enable list is

0 commit comments

Comments
 (0)