Journal ordering cleanups (#218)

* credentials: Fix local-fs.target references missed by remote-fs.target migration

Commit be88173 ("credentials: Use remote-fs.target for virtiofs mount
ordering") changed the mount unit generator to use Before=remote-fs.target
and updated the libvirt code path dropin, but missed three other locations:

- run_ephemeral.rs SMBIOS dropin: still created a dropin targeting
  local-fs.target~bcvk-mounts instead of remote-fs.target~bcvk-mounts
- run_ephemeral.rs inject_systemd_units(): still created
  local-fs.target.wants/ directory and symlinks there
- bcvk-qemu smbios_creds_for_mount_unit(): still referenced
  local-fs.target in the dropin credential

This inconsistency meant that the ephemeral code path would create mount
units ordered Before=remote-fs.target but activated via a
local-fs.target dropin, which is contradictory: remote-fs.target orders
After=local-fs.target, so a unit that says Before=remote-fs.target
should not be pulled in by local-fs.target.

Assisted-by: OpenCode (Claude claude-opus-4-6)
Signed-off-by: Colin Walters <walters@verbum.org>

* tests: Add systemd health checks using structured journal queries

Add two new integration tests for systemd health:

test_systemd_health_cross_distro (parameterized, runs per image):
Queries the guest journal via SSH for "Failed with result" and
"ordering cycle" messages using journalctl -o json, parsed with
serde. This catches regressions where mount unit ordering changes
(like the local-fs/remote-fs migration) cause units to fail or
create dependency cycles.

test_run_ephemeral_detect_ordering_cycle (negative test):
Injects two units with a deliberate mutual Before= cycle via
--systemd-units, boots the VM, and verifies the "ordering cycle"
diagnostic appears in the journal. This ensures the health check
is not passing vacuously.

Assisted-by: OpenCode (Claude claude-opus-4-6)
Signed-off-by: Colin Walters <walters@verbum.org>

---------

Signed-off-by: Colin Walters <walters@verbum.org>

Author: cgwalters

crates/bcvk-qemu/src/credentials.rs

@@ -80,7 +80,7 @@ pub fn generate_virtiofs_mount_unit(
 ///
 /// Creates systemd credentials for:
 /// 1. The mount unit itself (via systemd.extra-unit)
-/// 2. A dropin for local-fs.target that wants this mount unit
+/// 2. A dropin for remote-fs.target that wants this mount unit
 ///
 /// Returns a vector of SMBIOS credential strings.
 pub fn smbios_creds_for_mount_unit(
@@ -95,14 +95,14 @@ pub fn smbios_creds_for_mount_unit(
     let mount_cred =
         format!("io.systemd.credential.binary:systemd.extra-unit.{unit_name}={encoded_mount}");
 
-    // Create a dropin for local-fs.target that wants this mount
+    // Create a dropin for remote-fs.target that wants this mount
     let dropin_content = format!(
         "[Unit]\n\
          Wants={unit_name}\n"
     );
     let encoded_dropin = data_encoding::BASE64.encode(dropin_content.as_bytes());
     let dropin_cred = format!(
-        "io.systemd.credential.binary:systemd.unit-dropin.local-fs.target~bcvk-mounts={encoded_dropin}"
+        "io.systemd.credential.binary:systemd.unit-dropin.remote-fs.target~bcvk-mounts={encoded_dropin}"
     );
 
     Ok(vec![mount_cred, dropin_cred])

crates/integration-tests/src/tests/run_ephemeral.rs

@@ -18,6 +18,10 @@ use color_eyre::Result;
 use integration_tests::integration_test;
 use xshell::cmd;
 
+use std::fs;
+use tempfile::TempDir;
+
+use camino::Utf8Path;
 use tracing::debug;
 
 use crate::{get_bck_command, get_test_image, shell, INTEGRATION_TEST_LABEL};
@@ -458,3 +462,83 @@ fn test_run_ephemeral_mount_layout() -> Result<()> {
     Ok(())
 }
 integration_test!(test_run_ephemeral_mount_layout);
+
+/// Verify that systemd ordering cycle detection actually works by injecting
+/// a deliberate cycle: unit A Before=B, unit B Before=A.
+///
+/// We inject the units via --systemd-units with default.target.wants/
+/// (which inject_systemd_units() knows how to copy), let the system boot
+/// normally, then use --execute to check the journal for the expected
+/// "ordering cycle" diagnostic.
+fn test_run_ephemeral_detect_ordering_cycle() -> Result<()> {
+    let sh = shell()?;
+    let bck = get_bck_command()?;
+    let label = INTEGRATION_TEST_LABEL;
+    let image = get_test_image();
+
+    let units_dir = TempDir::new()?;
+    let units_dir_path = Utf8Path::from_path(units_dir.path()).expect("temp dir is not utf8");
+    let system_dir = units_dir_path.join("system");
+    fs::create_dir(&system_dir)?;
+
+    // Create a cycle: A wants and orders before B, B wants and orders before A.
+    // Both Wants= ensure both units are in the transaction; the mutual
+    // Before= constraints form the actual ordering cycle.
+    fs::write(
+        system_dir.join("cycle-a.service"),
+        "[Unit]\n\
+         Description=Cycle test unit A\n\
+         Wants=cycle-b.service\n\
+         Before=cycle-b.service\n\
+         [Service]\n\
+         Type=oneshot\n\
+         ExecStart=/bin/true\n\
+         RemainAfterExit=yes\n",
+    )?;
+    fs::write(
+        system_dir.join("cycle-b.service"),
+        "[Unit]\n\
+         Description=Cycle test unit B\n\
+         Wants=cycle-a.service\n\
+         Before=cycle-a.service\n\
+         [Service]\n\
+         Type=oneshot\n\
+         ExecStart=/bin/true\n\
+         RemainAfterExit=yes\n",
+    )?;
+
+    // Enable via default.target.wants/ which inject_systemd_units() copies
+    let wants_dir = system_dir.join("default.target.wants");
+    fs::create_dir(&wants_dir)?;
+    std::os::unix::fs::symlink("../cycle-a.service", wants_dir.join("cycle-a.service"))?;
+
+    // Use --execute to query the journal in JSON format for ordering cycle
+    // messages after the system boots. journalctl -g exits non-zero when no
+    // matches are found, so we ignore the exit status.
+    let check_script = "journalctl -b --no-pager -o json -g 'ordering cycle'";
+
+    let stdout = cmd!(
+        sh,
+        "{bck} ephemeral run --rm --label {label} --execute {check_script} --systemd-units {units_dir_path} {image}"
+    )
+    .ignore_status()
+    .read()?;
+
+    // Parse JSON lines and look for cycle messages
+    let has_cycle = stdout.lines().any(|line| {
+        serde_json::from_str::<serde_json::Value>(line)
+            .ok()
+            .and_then(|v| v.get("MESSAGE")?.as_str().map(String::from))
+            .is_some_and(|msg| msg.contains("ordering cycle"))
+    });
+
+    assert!(
+        has_cycle,
+        "Expected ordering cycle to be detected for deliberately cyclic units. \
+         Output: {}",
+        stdout
+    );
+
+    Ok(())
+}
+integration_test!(test_run_ephemeral_detect_ordering_cycle);

crates/integration-tests/src/tests/run_ephemeral_ssh.rs

@@ -414,3 +414,91 @@ fn test_run_ephemeral_ssh_timeout() -> Result<()> {
     Ok(())
 }
 integration_test!(test_run_ephemeral_ssh_timeout);
+
+/// A journal entry from `journalctl -o json`.
+///
+/// We only deserialize the fields we care about; serde ignores the rest.
+#[derive(serde::Deserialize)]
+struct JournalEntry {
+    #[serde(rename = "MESSAGE", default)]
+    message: String,
+    #[serde(rename = "UNIT", default)]
+    unit: Option<String>,
+}
+
+/// Parse `journalctl -o json` output (one JSON object per line).
+fn parse_journal_entries(output: &str) -> Vec<JournalEntry> {
+    output
+        .lines()
+        .filter_map(|line| serde_json::from_str::<JournalEntry>(line).ok())
+        .collect()
+}
+
+/// Test systemd health across all configured test images
+///
+/// Queries the guest journal directly (via SSH) for well-known systemd
+/// messages that indicate problems:
+/// - "Failed with result" — a unit entered failed state
+/// - "ordering cycle" — conflicting Before=/After= dependencies
+///
+/// Uses `journalctl -o json` for structured output parsed with serde,
+/// avoiding brittle text parsing of human-readable journal formats.
+fn test_systemd_health_cross_distro(image: &str) -> Result<()> {
+    let sh = shell()?;
+    let bck = get_bck_command()?;
+    let label = INTEGRATION_TEST_LABEL;
+
+    // Query journal for unit failures and ordering cycles in a single SSH call.
+    // Using -o json gives us one JSON object per line, which we parse with serde.
+    // journalctl -g exits non-zero when no matches are found, so we ignore the
+    // exit status and just parse whatever stdout we get (empty = no matches = pass).
+    let check_script = "journalctl -b --no-pager -o json -g 'Failed with result|ordering cycle'";
+
+    let stdout = cmd!(
+        sh,
+        "{bck} ephemeral run-ssh --label {label} {image} -- {check_script}"
+    )
+    .ignore_status()
+    .read()?;
+
+    let entries = parse_journal_entries(&stdout);
+
+    let failures: Vec<&JournalEntry> = entries
+        .iter()
+        .filter(|e| e.message.contains("Failed with result"))
+        .collect();
+
+    let cycles: Vec<&JournalEntry> = entries
+        .iter()
+        .filter(|e| e.message.contains("ordering cycle"))
+        .collect();
+
+    assert!(
+        failures.is_empty(),
+        "Found failed systemd unit(s) on image {}:\n{}",
+        image,
+        failures
+            .iter()
+            .map(|e| format!(
+                "  {}: {}",
+                e.unit.as_deref().unwrap_or("<unknown>"),
+                e.message
+            ))
+            .collect::<Vec<_>>()
+            .join("\n")
+    );
+
+    assert!(
+        cycles.is_empty(),
+        "Found systemd ordering cycle(s) on image {}:\n{}",
+        image,
+        cycles
+            .iter()
+            .map(|e| format!("  {}", e.message))
+            .collect::<Vec<_>>()
+            .join("\n")
+    );
+
+    Ok(())
+}
+parameterized_integration_test!(test_systemd_health_cross_distro);

crates/kit/src/run_ephemeral.rs

@@ -728,7 +728,7 @@ pub(crate) fn process_disk_files(
 }
 
 /// Copy systemd units from /run/systemd-units/system/ to container image /etc/systemd/system/.
-/// Auto-enables .mount units in local-fs.target.wants/, preserves default.target.wants/ symlinks.
+/// Auto-enables .mount units in remote-fs.target.wants/, preserves default.target.wants/ symlinks.
 fn inject_systemd_units() -> Result<()> {
     use std::fs;
 
@@ -744,7 +744,7 @@ fn inject_systemd_units() -> Result<()> {
     // Create target directories
     fs::create_dir_all(target_units)?;
     fs::create_dir_all(&format!("{}/default.target.wants", target_units))?;
-    fs::create_dir_all(&format!("{}/local-fs.target.wants", target_units))?;
+    fs::create_dir_all(&format!("{}/remote-fs.target.wants", target_units))?;
 
     // Copy all .service and .mount files
     for entry in fs::read_dir(source_units)? {
@@ -759,7 +759,7 @@ fn inject_systemd_units() -> Result<()> {
 
             // Create symlinks for mount units to enable them
             if extension.as_deref() == Some("mount") {
-                let wants_dir = format!("{}/local-fs.target.wants", target_units);
+                let wants_dir = format!("{}/remote-fs.target.wants", target_units);
                 let symlink_path = format!("{}/{}", wants_dir, filename);
                 let relative_target = format!("../{}", filename);
                 std::os::unix::fs::symlink(&relative_target, &symlink_path).ok();
@@ -1062,7 +1062,7 @@ pub(crate) async fn run_impl(opts: RunEphemeralOpts) -> Result<()> {
             );
             mount_unit_smbios_creds.push(mount_cred);
 
-            // Collect unit name for the local-fs.target dropin
+            // Collect unit name for the remote-fs.target dropin
             mount_unit_names.push(unit_name.clone());
 
             debug!(
@@ -1072,17 +1072,19 @@ pub(crate) async fn run_impl(opts: RunEphemeralOpts) -> Result<()> {
         }
     }
 
-    // If we have mount units, create a single dropin for local-fs.target
+    // If we have mount units, create a single dropin for remote-fs.target.
+    // We use remote-fs.target because virtiofs is conceptually similar to a remote
+    // filesystem - it requires virtio transport infrastructure, like NFS needs network.
     if !mount_unit_names.is_empty() {
         let wants_list = mount_unit_names.join(" ");
         let dropin_content = format!("[Unit]\nWants={}\n", wants_list);
         let encoded_dropin = data_encoding::BASE64.encode(dropin_content.as_bytes());
         let dropin_cred = format!(
-            "io.systemd.credential.binary:systemd.unit-dropin.local-fs.target~bcvk-mounts={encoded_dropin}"
+            "io.systemd.credential.binary:systemd.unit-dropin.remote-fs.target~bcvk-mounts={encoded_dropin}"
         );
         mount_unit_smbios_creds.push(dropin_cred);
         debug!(
-            "Created local-fs.target dropin for {} mount units",
+            "Created remote-fs.target dropin for {} mount units",
             mount_unit_names.len()
         );
     }