fix(microvm): name both causes when a command health check cannot reach the guest

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.
 

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

src/hypervisor/vsock_client.rs

@@ -245,3 +245,136 @@ async fn exchange<S: AsyncRead + AsyncWrite + Unpin>(
         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));
+        }
+    }
+}

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)),

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)),