From 0e48f1bdca96f8263837063d453144c9d93edaa3 Mon Sep 17 00:00:00 2001 From: Mlanawo MBECHEZI Date: Sat, 25 Jul 2026 10:18:20 +0300 Subject: [PATCH] fix(microvm): name both causes when a command health check cannot reach the guest --- documentation/concepts/runtimes.md | 6 + documentation/runtimes/cloud-hypervisor.md | 4 + src/hypervisor/vsock_client.rs | 133 +++++++++++++++++++++ src/runtime/cloud_hypervisor/lifecycle.rs | 47 ++++---- src/runtime/firecracker/lifecycle.rs | 34 +++--- 5 files changed, 189 insertions(+), 35 deletions(-) diff --git a/documentation/concepts/runtimes.md b/documentation/concepts/runtimes.md index 7d1b070..5db3b48 100644 --- a/documentation/concepts/runtimes.md +++ b/documentation/concepts/runtimes.md @@ -157,6 +157,12 @@ checks via the in-guest `ring-agent` over vsock, `kind: job` run-to-completion, **Current limitations (experimental):** - Crash detection is tick-bound (no event stream), and `labels`, while stored and filterable as Ring metadata, are not applied to the VM itself +- Like Cloud Hypervisor, the vsock device backing `command` health checks is + attached at boot and only when the deployment already declares such a check; + one added to a running deployment cannot reach its VM until that VM restarts + (neither hypervisor can hot-plug it). The probe names this cause alongside a + missing agent instead of reporting only the latter, and `on_failure: restart` + heals itself — `alert` and `stop` need a manual restart A `ring-server` restart is transparent: running microVMs (and their persistent host taps) survive it, and the reconciler re-adopts them (re-deriving each instance's network from its id and re-spawning the host port-forwarders the old process took down) so a deployment keeps its guest state and its published ports across a restart. diff --git a/documentation/runtimes/cloud-hypervisor.md b/documentation/runtimes/cloud-hypervisor.md index 0c265fe..d01dd6d 100644 --- a/documentation/runtimes/cloud-hypervisor.md +++ b/documentation/runtimes/cloud-hypervisor.md @@ -208,6 +208,10 @@ Note that the guest network is only allocated when the deployment publishes at l `tcp`, `http`, `command` all work. `tcp` and `http` probe from the host against the guest IP (no agent required). `command` goes through the in-guest `ring-agent` over AF_VSOCK port 2375, so install the agent in the guest image. If the agent isn't reachable (missing from the image, or not started yet), the `command` probe fails with an explicit message naming ring-agent rather than a bare connection error. +> **A `command` check added to a running deployment needs a VM restart.** The vsock device is attached at boot, and only when the deployment already declares a `command` check — cloud-hypervisor has no hot-plug path for it. Until the VM restarts, the probe cannot reach the guest at all. The failure message names this cause alongside a missing agent, so it isn't mistaken for one. +> +> Whether the VM ever restarts on its own is up to the check's `on_failure`: `restart` heals itself once the failure threshold is reached, but `alert` and `stop` never reboot the VM, so the check stays red until you restart the deployment yourself. Declaring the `command` check before the first boot avoids the whole situation. + The readiness gate (`readiness: true`) works exactly as on Docker, since the scheduler-side drain logic is runtime-agnostic. **But there is no CH equivalent of the native Docker `HEALTHCHECK` translation**, so a `readiness: true` check gates the Ring drain but is not exposed to external proxies. ## Logs diff --git a/src/hypervisor/vsock_client.rs b/src/hypervisor/vsock_client.rs index f46f197..a883957 100644 --- a/src/hypervisor/vsock_client.rs +++ b/src/hypervisor/vsock_client.rs @@ -245,3 +245,136 @@ async fn exchange( Response::Error { message } => Err(VsockError::Agent(message)), } } + +/// Explain a vsock connect failure for a `command` health check. +/// +/// A bare io error leaves the operator with nothing to act on, and there are two +/// quite different causes: +/// +/// * the guest side is at fault — `ring-agent` isn't installed, isn't running, +/// or isn't listening on [`VSOCK_PORT`] yet; +/// * the VM has no vsock device at all. It is attached at boot, and only when +/// the deployment already declares a `command` check, so one added to a +/// running deployment cannot work until that VM restarts — neither hypervisor +/// can hot-plug it. +/// +/// `host_socket_present` reports whether the host-side vsock socket is on disk. +/// It is a *hint*, not proof: a crashed VMM can leave the socket behind until +/// the reconciler reaps it, and a live VM can have its socket unlinked while +/// keeping the device. So it only orders the two causes — the message names both +/// either way, rather than asserting one and sending the operator to the wrong +/// place. +pub(crate) fn connect_failure_message( + runtime: &str, + cid: u32, + source: &str, + host_socket_present: bool, +) -> String { + let (first, second) = if host_socket_present { + ( + format!( + "ring-agent may not be running in the guest on AF_VSOCK port {VSOCK_PORT} \ + (install it in the image and start it at boot — see the {runtime} runtime docs)" + ), + "or this VM may have been booted without a vsock device".to_string(), + ) + } else { + ( + "this VM appears to have been booted without a vsock device".to_string(), + format!( + "or ring-agent may not be running in the guest on AF_VSOCK port {VSOCK_PORT} \ + (install it in the image and start it at boot — see the {runtime} runtime docs)" + ), + ) + }; + + format!( + "cannot reach ring-agent in the guest (CID {cid}): {source}. {first}, {second}. \ + The device is attached at boot only when the deployment already declares a \ + `command` check, so one added to a running deployment takes effect on the next \ + VM restart — neither {runtime} nor Ring can hot-plug it" + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Both causes must appear whichever way the hint points: the host socket is + /// not proof (a crashed VMM leaves it behind, a live VM can have it + /// unlinked), so asserting one cause would send the operator to the wrong + /// place half the time. + #[test] + fn both_causes_are_always_named() { + for present in [true, false] { + let msg = connect_failure_message("firecracker", 42, "connection refused", present); + assert!(msg.contains("ring-agent"), "names the agent: {msg}"); + assert!(msg.contains("vsock device"), "names the device: {msg}"); + assert!(msg.contains("CID 42"), "names the CID: {msg}"); + assert!( + msg.contains("connection refused"), + "keeps the source: {msg}" + ); + // The agent-side remedy must survive whichever way the hint points: + // it is what an operator acts on when the image really is at fault. + assert!(msg.contains("2375"), "names the port: {msg}"); + assert!( + msg.contains("install it in the image"), + "keeps the install remedy: {msg}" + ); + assert!(msg.contains("runtime docs"), "points at the docs: {msg}"); + } + } + + /// The hint decides which cause is stated first — that ordering is the whole + /// value the host-socket check adds. + #[test] + fn the_hint_orders_the_two_causes() { + // Unwrap both positions before comparing: `Option` ordering would make + // `None < Some(_)` pass, so a vanished phrase would look like correct + // ordering instead of failing. + let with_socket = connect_failure_message("firecracker", 1, "e", true); + let agent_first = with_socket + .find("ring-agent may not be running") + .unwrap_or_else(|| panic!("agent cause missing: {with_socket}")); + let device_second = with_socket + .find("may have been booted without") + .unwrap_or_else(|| panic!("device cause missing: {with_socket}")); + assert!( + agent_first < device_second, + "socket on disk → guest side first: {with_socket}" + ); + + let without_socket = connect_failure_message("firecracker", 1, "e", false); + let device_first = without_socket + .find("appears to have been booted without") + .unwrap_or_else(|| panic!("device cause missing: {without_socket}")); + let agent_second = without_socket + .find("or ring-agent may not be running") + .unwrap_or_else(|| panic!("agent cause missing: {without_socket}")); + assert!( + device_first < agent_second, + "no socket → missing device first: {without_socket}" + ); + } + + /// The boot-time constraint is the actionable part: it tells the operator a + /// restart is needed, which no amount of guest-side debugging would reveal. + #[test] + fn the_boot_time_constraint_is_always_explained() { + for present in [true, false] { + let msg = connect_failure_message("cloud-hypervisor", 7, "no such file", present); + assert!(msg.contains("attached at boot only"), "{msg}"); + assert!(msg.contains("next VM restart"), "gives the remedy: {msg}"); + } + } + + /// The runtime name is interpolated so the message points at the right docs. + #[test] + fn message_names_the_runtime() { + for runtime in ["firecracker", "cloud-hypervisor"] { + assert!(connect_failure_message(runtime, 1, "e", true).contains(runtime)); + assert!(connect_failure_message(runtime, 1, "e", false).contains(runtime)); + } + } +} diff --git a/src/runtime/cloud_hypervisor/lifecycle.rs b/src/runtime/cloud_hypervisor/lifecycle.rs index 52236b4..014e4fe 100644 --- a/src/runtime/cloud_hypervisor/lifecycle.rs +++ b/src/runtime/cloud_hypervisor/lifecycle.rs @@ -586,13 +586,15 @@ impl CloudHypervisorLifecycle { // vsock for every CH VM would mean an extra device per VM for no // benefit; gate it on demand instead. // - // Known limitation: `needs_vsock` is evaluated only at boot. If an - // operator adds a `command` health check to an already-running - // deployment, the existing VM has no vsock device; the next probe - // will fail at connect, the scheduler will increment failures and - // eventually restart the VM, at which point this path runs again - // and the vsock is provisioned. The transient failures are - // unavoidable without a hot-attach path through the CH API. + // Known limitation: `needs_vsock` is evaluated only at boot, and CH has + // no hot-attach path for vhost-vsock. A `command` check added to an + // already-running deployment therefore cannot reach its VM until that + // VM restarts. Whether it ever does is up to the check's `on_failure`: + // `restart` heals itself after the failure threshold, but `alert` and + // `stop` never re-run this path, so the check stays red until the + // operator restarts the deployment. `execute_command_probe` detects + // this case (no host-side vsock socket) and says so, rather than + // blaming a missing `ring-agent` in the guest. let needs_vsock = deployment .health_checks .iter() @@ -1373,20 +1375,23 @@ impl RuntimeLifecycle for CloudHypervisorLifecycle { resp.stderr.trim() )), ), - // A connect failure almost always means the guest image doesn't - // ship `ring-agent` listening on AF_VSOCK port 2375 (or it hasn't - // started yet) — the single most common `command` health-check - // pitfall on this runtime. Point the operator straight at it - // instead of leaving them with a bare io error. - Err(crate::hypervisor::vsock_client::VsockError::Connect { cid, source }) => ( - HealthCheckStatus::Failed, - Some(format!( - "cannot reach ring-agent in the guest (CID {cid}): {source}. \ - `command` health checks on cloud-hypervisor require ring-agent \ - running in the guest image on AF_VSOCK port 2375 — see the \ - cloud-hypervisor runtime docs" - )), - ), + // A connect failure has two very different causes, and blaming the + // guest image for both sends the operator hunting in the wrong + // place. The host-side socket only exists when the VM was booted + // with a vsock device, so its absence pinpoints the other cause. + Err(crate::hypervisor::vsock_client::VsockError::Connect { cid, source }) => { + let vsock_path = + PathBuf::from(&self.config.socket_dir).join(format!("{}.vsock", instance_id)); + ( + HealthCheckStatus::Failed, + Some(crate::hypervisor::vsock_client::connect_failure_message( + "cloud-hypervisor", + cid, + &source.to_string(), + vsock_path.exists(), + )), + ) + } Err(e) => ( HealthCheckStatus::Failed, Some(format!("vsock probe failed: {}", e)), diff --git a/src/runtime/firecracker/lifecycle.rs b/src/runtime/firecracker/lifecycle.rs index e6982c2..8bd0886 100644 --- a/src/runtime/firecracker/lifecycle.rs +++ b/src/runtime/firecracker/lifecycle.rs @@ -713,7 +713,10 @@ impl FirecrackerLifecycle { // runtime. The guest reaches `ring-agent` over AF_VSOCK; the host // reaches it through the multiplexing Unix socket at `vsock_path`. // Same boot-time limitation as CH: adding a `command` check to a - // running deployment only takes effect after the next restart. + // running deployment only takes effect after the next VM restart, and + // whether one ever happens depends on the check's `on_failure` + // (`restart` heals itself; `alert`/`stop` do not). `execute_command_probe` + // detects a VM booted without the device and says so. if needs_vsock(deployment) { client .put_vsock(&Vsock { @@ -1721,19 +1724,22 @@ impl RuntimeLifecycle for FirecrackerLifecycle { resp.stderr.trim() )), ), - // A connect failure almost always means the guest image doesn't - // ship `ring-agent` listening on AF_VSOCK port 2375 (or it hasn't - // started yet) — the most common `command` health-check pitfall on - // this runtime. Point the operator straight at it. - Err(VsockError::Connect { cid, source }) => ( - HealthCheckStatus::Failed, - Some(format!( - "cannot reach ring-agent in the guest (CID {cid}): {source}. \ - `command` health checks on firecracker require ring-agent \ - running in the guest image on AF_VSOCK port 2375 — see the \ - firecracker runtime docs" - )), - ), + // A connect failure has two very different causes, and blaming the + // guest image for both sends the operator hunting in the wrong + // place. The host-side socket only exists when the VM was booted + // with a vsock device, so its absence pinpoints the other cause. + Err(VsockError::Connect { cid, source }) => { + let host_socket_present = Path::new(&self.vsock_path(instance_id)).exists(); + ( + HealthCheckStatus::Failed, + Some(vsock_client::connect_failure_message( + "firecracker", + cid, + &source.to_string(), + host_socket_present, + )), + ) + } Err(e) => ( HealthCheckStatus::Failed, Some(format!("vsock probe failed: {}", e)),