diff --git a/CLAUDE.md b/CLAUDE.md index 837cc43..545f969 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -436,6 +436,39 @@ security update. Migration is the non-destructive way out; Reset is the destruct bump: churn on the old base, and it would consume the "you should migrate" signal without migrating. `get_container_staleness` surfaces it; `migrate_project_to_base` acts on it. - **A missing lineage label means "unknown, probe instead", never "stale".** +- **The snapshot image is not a checkpoint — never read its absence as "nothing to inspect".** + `commit_container_snapshot` runs only before a container is destroyed (a config-change recreate) + or inside a migration. **Never on stop.** So a project in daily use for a year can legitimately + have no `triple-c-snapshot-{id}:latest` at all, and one that has is stale by everything installed + since. `pick_probe_source` therefore reads a *stopped* container directly — commit its writable + layer to `triple-c-probe-{cid}:latest`, probe that, drop it — and ranks it **above** the snapshot, + for the same reason a running container already outranked it. Assuming a snapshot existed is what + made a stopped, never-recreated project report "no container or snapshot image yet" with its + container sitting right there, and left Update disabled on the projects furthest behind. +- **`bollard` never gives you the image id back from a commit.** Its `Commit` response model + deserialises `"ID"`; the daemon sends `"Id"`, so `commit_container` returns `id: None` every time + (verified: bollard 0.18.1, Engine 29.6). Neither long-standing commit site notices because both + discard the response — but it means any commit you need a *reference* to has to be **tagged**. +- **A tagged leftover is the one orphan no sweep can reach, so the probe image has its own reaper.** + `sweep_orphaned_snapshots` collects `dangling` + `triple-c.managed=true`; `reap_stale_migration_pins` + and `scrub_secrets_from_snapshots` both filter `triple-c-snapshot-*`. A `triple-c-probe-*` image is + tagged and so matches none of them, which would make a crashed probe a permanent multi-gigabyte + leak with no UI to find it. `reap_probe_images` runs at startup beside `reap_probe_containers` and + is **load-bearing, not tidying** — it is also what makes the probe image's unscrubbed writable + layer acceptable. Two rules it earned the hard way: + - **Age-gate it** (`PROBE_REAP_MIN_AGE_SECS`, same as the container reaper). `reference=` is + daemon-wide, so a second copy of the app has live probe images matching the glob. + - **Remove by tag, never by image id.** A `force` removal by id untags an image *everywhere*; a + fixture that tagged `alpine:latest` into this namespace deleted the user's alpine that way. +- **Probe image names are unique per call, and must stay that way.** A stable per-container name was + tried: container ids do not survive a recreate, so most leftovers were stranded permanently, and + two concurrent probes fought over one tag — whichever finished first force-removed the image the + other was still reading, reporting a bogus `probe_error` on a healthy project. `get_container_staleness` + takes no `project_lock` claim (the migration banner needs it to answer *during* a migration), so + uniqueness is what makes overlapping probes safe. +- **An image's `Created` is the image's own, not its tag's.** Tagging an existing image gives you + that image's age; BuildKit stamps `docker build` output with a fixed epoch. Only `docker commit` + stamps *now* — which is what real probe images do, and what any fixture for them must do. - **`:latest` keeps pointing at the old lineage until the final commit.** That is what makes every crash before that point self-heal — `start_project_container` just recreates from the old snapshot. After the container swap, the new container's `triple-c.migration-state=in-progress` diff --git a/app/src-tauri/src/commands/migration_commands.rs b/app/src-tauri/src/commands/migration_commands.rs index 6be50da..e97fc31 100644 --- a/app/src-tauri/src/commands/migration_commands.rs +++ b/app/src-tauri/src/commands/migration_commands.rs @@ -92,8 +92,72 @@ fn pick_recorded_lineage( .or_else(|| from_snapshot.filter(|v| !v.is_empty())) } -/// Read-only. Runs two filesystem probes (~3 s each) and is therefore meant to -/// be called on demand, not polled. +/// Reported as `probe_error` when there is genuinely nothing to read: no +/// container, stopped or otherwise, and no snapshot image. +/// +/// It used to be reported for a *stopped* container too, which was simply +/// untrue — the container was sitting right there — and it disabled Update on +/// exactly the long-lived projects that had never been recreated and so had no +/// snapshot to fall back on. +const NOTHING_TO_PROBE: &str = "This project has no container or snapshot image yet, so there is nothing to compare against the base image."; + +/// Where [`get_container_staleness`] reads the project's *current* filesystem +/// from, in descending order of how current the answer is. +#[derive(Debug, PartialEq, Eq)] +enum ProbeSource { + /// `docker exec` into the live container. The only source that includes + /// everything installed since the last commit *in this session*. + RunningContainer, + /// Commit the stopped container's writable layer to a throwaway image and + /// probe that. Exactly as current as the container, which is what makes it + /// preferable to the snapshot — see below. + StoppedContainer, + /// A throwaway container from `triple-c-snapshot-:latest`. + Snapshot, + /// Nothing to read: no container, no snapshot. + Nothing, +} + +/// Pick the probe source. `container_running` is `None` when the project has no +/// container at all, `Some(false)` when it has a stopped one. +/// +/// **A stopped container outranks the snapshot.** The snapshot image is not a +/// checkpoint — `commit_container_snapshot` runs only before a removal (a +/// config-change recreate) or inside a migration, so a project that has never +/// hit either has *no snapshot at all*, however long it has been in use, and +/// one that has is stale by everything installed since. The container's +/// writable layer is the truth in both cases. This is the same argument +/// [`mig::manifest_from_container`] already makes for the running case; it does +/// not stop applying when the container is stopped. +/// +/// Getting this wrong is what made a stopped, never-recreated project report +/// "no container or snapshot image yet" — with its container sitting right +/// there — and left Update disabled on the projects that most needed it. +fn pick_probe_source(container_running: Option, snapshot_exists: bool) -> ProbeSource { + match (container_running, snapshot_exists) { + (Some(true), _) => ProbeSource::RunningContainer, + (Some(false), _) => ProbeSource::StoppedContainer, + (None, true) => ProbeSource::Snapshot, + (None, false) => ProbeSource::Nothing, + } +} + +/// Runs two filesystem probes (~3 s each) and is therefore meant to be called +/// on demand, not polled. +/// +/// **Not read-only, despite only reporting.** The stopped-container path commits +/// a throwaway image and force-removes it, which makes this a writer of a +/// `triple-c-probe-*` image and puts it in the class of thing +/// [`crate::project_lock`] exists for — and it takes no claim. That is +/// deliberate: this is what the migration banner calls to decide whether to +/// offer an update, including while a migration is in flight, so refusing it +/// under a claim would blank the banner exactly when it has the most to say. +/// The exposure is bounded to a surfaced error — a concurrent Recreate, Reset or +/// migration can remove the container out from under the commit, and the result +/// is a `probe_error` the user can retry, never a damaged container or a +/// mislabelled image. Two overlapping probes cannot collide either, because +/// probe image names are unique per call; see +/// [`crate::docker::container::get_probe_image_name`]. #[tauri::command] pub async fn get_container_staleness( project_id: String, @@ -145,16 +209,25 @@ pub async fn get_container_staleness( }; // ── Probes ─────────────────────────────────────────────────────────── - let running = match &container_id { - Some(id) => docker::is_container_running(id).await.unwrap_or(false), - None => false, + let container_running = match &container_id { + Some(id) => Some(docker::is_container_running(id).await.unwrap_or(false)), + None => None, }; - let from_manifest = if running { - mig::manifest_from_container(container_id.as_ref().unwrap()).await - } else if docker::image_exists(&snapshot_image).await.unwrap_or(false) { - mig::manifest_from_image(&snapshot_image).await - } else { - Err("This project has no container or snapshot image yet, so there is nothing to compare against the base image.".to_string()) + let snapshot_exists = docker::image_exists(&snapshot_image).await.unwrap_or(false); + let from_manifest = match ( + pick_probe_source(container_running, snapshot_exists), + &container_id, + ) { + (ProbeSource::RunningContainer, Some(id)) => mig::manifest_from_container(id).await, + (ProbeSource::StoppedContainer, Some(id)) => { + mig::manifest_from_stopped_container(id).await + } + (ProbeSource::Snapshot, _) => mig::manifest_from_image(&snapshot_image).await, + // `container_running` is `Some` exactly when `container_id` is, so the + // two arms above are the only ones those variants can reach. This arm + // is `ProbeSource::Nothing` — and now *only* that: it used to also + // swallow every stopped container, which is the bug. + (_, _) => Err(NOTHING_TO_PROBE.to_string()), }; let (from_manifest, base_manifest) = match from_manifest { @@ -1964,6 +2037,42 @@ mod tests { assert_eq!(pick_recorded_lineage(some(""), None), None); } + #[test] + fn a_stopped_container_is_probed_rather_than_reported_missing() { + // The regression: a container that exists but is stopped, with no + // snapshot ever taken, read as "nothing to compare against". + assert_eq!( + pick_probe_source(Some(false), false), + ProbeSource::StoppedContainer + ); + } + + #[test] + fn the_container_outranks_the_snapshot_whether_or_not_it_is_running() { + // The snapshot lags the container by everything installed since the + // last commit, in both states. + assert_eq!( + pick_probe_source(Some(true), true), + ProbeSource::RunningContainer + ); + assert_eq!( + pick_probe_source(Some(false), true), + ProbeSource::StoppedContainer + ); + } + + #[test] + fn the_snapshot_is_the_fallback_only_once_the_container_is_gone() { + assert_eq!(pick_probe_source(None, true), ProbeSource::Snapshot); + } + + #[test] + fn nothing_to_probe_is_reserved_for_no_container_and_no_snapshot() { + // The one case the "no container or snapshot image yet" message may + // still describe. + assert_eq!(pick_probe_source(None, false), ProbeSource::Nothing); + } + #[test] fn byte_sizes_read_the_way_a_disk_warning_should() { assert_eq!(human_bytes(512), "512 B"); diff --git a/app/src-tauri/src/docker/container.rs b/app/src-tauri/src/docker/container.rs index 038e837..6bda607 100644 --- a/app/src-tauri/src/docker/container.rs +++ b/app/src-tauri/src/docker/container.rs @@ -3052,6 +3052,118 @@ fn blanked_secret_env() -> Vec { .collect() } +/// Image-name prefix for the throwaway commit a staleness probe of a stopped +/// container makes. The reaper's only handle on a leftover — see +/// [`crate::docker::migration::reap_probe_images`] — so nothing else may use it. +pub const PROBE_IMAGE_PREFIX: &str = "triple-c-probe-"; + +/// The throwaway image a staleness probe of a **stopped** container commits to. +/// +/// **Unique per call**, and both halves of the name earn their place: the +/// container id prefix makes a leftover traceable in `docker images`, and the +/// counter makes two overlapping probes independent. +/// +/// An earlier version of this was deliberately *stable* per container, on the +/// theory that the next probe would move the tag off an abandoned image and +/// leave it dangling for [`sweep_orphaned_snapshots`]. That was wrong twice +/// over. A container id does not survive a recreate, so for most leftovers +/// there is no "next probe of the same container" and the image was stranded +/// permanently; and a stable name made two concurrent probes fight over one +/// tag, where whichever finished first force-removed the image the other was +/// still reading and turned a healthy project into a bogus `probe_error`. +/// Uniqueness fixes both, and [`crate::docker::migration::reap_probe_images`] +/// is what collects the leftovers instead. +pub fn get_probe_image_name(container_id: &str) -> String { + use std::sync::atomic::{AtomicU64, Ordering}; + static SEQ: AtomicU64 = AtomicU64::new(0); + + let short: String = container_id.chars().take(12).collect(); + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0); + format!( + "{}{}-{}-{}:latest", + PROBE_IMAGE_PREFIX, + short, + nanos, + SEQ.fetch_add(1, Ordering::Relaxed) + ) +} + +/// Commit a **stopped** container's filesystem to a throwaway image, returning +/// its name. The caller owns the image and must remove it. +/// +/// This exists so a stopped project can be read at all. `docker exec` needs a +/// running container and the snapshot image is not a checkpoint — see +/// [`crate::commands::migration_commands`]'s probe-source pick — so without +/// this there is no way to see inside a project that is merely stopped. +/// +/// ## Why it is tagged at all +/// +/// An untagged commit would be tidier: untagged plus the `triple-c.managed=true` +/// that `docker commit` copies off the container is exactly the pair +/// [`sweep_orphaned_snapshots`] already collects, so a leftover would self-heal +/// with no new machinery. **It is not available.** `bollard`'s `Commit` response +/// model deserialises `"ID"` while the daemon sends `"Id"`, so +/// `commit_container` hands back `id: None` every time and there is no +/// reference left to probe. Neither existing commit site notices, because both +/// discard the response. Verified against Engine 29.6, bollard 0.18.1. +/// +/// So the image needs a name, a tagged image is not dangling, and the sweep +/// therefore cannot be the safety net. [`crate::docker::migration::reap_probe_images`] +/// is, and [`get_probe_image_name`] carries the rest of that argument. +/// +/// ## What is in the image, and what is not +/// +/// `pause: false` because nothing is running — pausing a stopped container is +/// an error, the same reason [`recommit_without_secrets`]'s scratch commit +/// passes `false`. +/// +/// Secrets are blanked from the env for the same reason +/// [`commit_container_snapshot`] blanks them: the commit bakes the container's +/// full ENV into the image, and "it only lives a few seconds" is not a property +/// this function can promise after a crash. +/// +/// **The writable layer is committed unscrubbed, and that is unavoidable here.** +/// [`commit_container_snapshot`] runs [`scrub_writable_layer`] first precisely +/// because a commit stacks a layer and never rewrites one — but that scrub is a +/// `docker exec`, which is exactly what a stopped container cannot serve, and +/// scrubbing is not wanted anyway: the probe's whole job is to report the +/// filesystem as it actually is. What makes it acceptable is that this copies +/// bytes that are *already on this disk* in the container's own writable layer, +/// into an image that is never pushed, never created from, and reaped — so it +/// duplicates data inside one trust domain rather than widening it. That +/// argument depends on the reaping actually happening; treat +/// [`crate::docker::migration::reap_probe_images`] as load-bearing, not tidying. +pub async fn commit_container_for_probe(container_id: &str) -> Result { + let docker = get_docker()?; + let image_name = get_probe_image_name(container_id); + let (repo, tag) = image_name + .rsplit_once(':') + .map(|(r, t)| (r.to_string(), t.to_string())) + .expect("get_probe_image_name always emits a tag"); + + docker + .commit_container( + CommitContainerOptions { + container: container_id.to_string(), + repo, + tag, + pause: false, + ..Default::default() + }, + Config:: { + env: Some(blanked_secret_env()), + ..Default::default() + }, + ) + .await + .map_err(|e| format!("Failed to commit stopped container {}: {}", container_id, e))?; + + Ok(image_name) +} + /// Whether `env` (an image's `Config.Env`) holds a non-empty value for any /// name in [`SECRET_ENV_KEYS`]. fn env_holds_a_secret(env: &[String]) -> bool { @@ -3518,9 +3630,10 @@ pub async fn remove_snapshot_image(project: &Project) -> Result<(), String> { remove_image_by_name(&get_snapshot_image_name(project)).await } -/// Remove a Docker image by name/tag, treating "does not exist" as success. -/// Shared by [`remove_snapshot_image`] and the pending-cleanup retry, which -/// only has the image name (the project record is already gone by then). +/// Remove a Docker image by name, tag or **id**, treating "does not exist" as +/// success. Shared by [`remove_snapshot_image`], the pending-cleanup retry +/// (which only has the image name — the project record is already gone by +/// then), and the staleness probe's throwaway commit, which has only an id. pub async fn remove_image_by_name(image_name: &str) -> Result<(), String> { let docker = get_docker()?; @@ -3536,7 +3649,7 @@ pub async fn remove_image_by_name(image_name: &str) -> Result<(), String> { .await { Ok(_) => { - log::info!("Removed snapshot image {}", image_name); + log::info!("Removed image {}", image_name); Ok(()) } Err(bollard::errors::Error::DockerResponseServerError { @@ -4464,6 +4577,29 @@ mod tests { assert!(env_holds_a_secret(&env)); } + /// The probe image's name must be **unique per call**. A stable name was + /// tried and is wrong twice over: a container id does not survive a + /// recreate, so a crashed probe's leftover would never be reclaimed by "the + /// next probe of the same container"; and two concurrent probes sharing one + /// tag means whichever finishes first force-removes the image the other is + /// still reading. See `commit_container_for_probe` and `reap_probe_images`. + #[test] + fn probe_image_names_are_unique_per_call_and_reapable_by_prefix() { + let id = "75993e6d5e1ab473b029a408c5ff0339"; + let a = get_probe_image_name(id); + let b = get_probe_image_name(id); + assert_ne!(a, b, "two probes of one container must not share a tag"); + + // The prefix is the reaper's only handle on a leftover, so every name + // has to carry it — and it must not be the snapshot namespace, which is + // what a project is rebuilt from. + assert!(a.starts_with(PROBE_IMAGE_PREFIX), "{}", a); + assert!(!a.starts_with("triple-c-snapshot-"), "{}", a); + // Traceable back to its container, which is the point of the prefix. + assert!(a.contains("75993e6d5e1a"), "{}", a); + assert!(a.ends_with(":latest"), "{}", a); + } + #[test] fn the_scrub_report_only_claims_success_when_nothing_is_left() { let clean = SnapshotScrubReport { diff --git a/app/src-tauri/src/docker/migration.rs b/app/src-tauri/src/docker/migration.rs index 3425970..a5332c0 100644 --- a/app/src-tauri/src/docker/migration.rs +++ b/app/src-tauri/src/docker/migration.rs @@ -886,6 +886,100 @@ pub async fn reap_probe_containers() { } } +/// Remove throwaway images left behind by a staleness probe of a stopped +/// container — [`super::container::commit_container_for_probe`]'s commits. +/// +/// **Load-bearing, not tidying.** A probe image is *tagged*, because bollard +/// gives no image id back from a commit and there has to be something to probe. +/// Tagged means not dangling, so [`super::container::sweep_orphaned_snapshots`] +/// — which collects every other kind of orphan this app can leave — will never +/// see one. Without this, a probe that dies between its commit and its own +/// cleanup (SIGKILL, a crash, a 409 from a concurrent remove) strands a +/// multi-gigabyte image that **no code path can ever reclaim**, and there is no +/// UI to find it either. That is the one leak in this app with no floor on it, +/// so this runs at startup beside [`reap_probe_containers`]. +/// +/// Age-gated for exactly the reason that one is: `reference=` is a daemon-wide +/// filter, so a second copy of the app probing a project on the same daemon has +/// images matching this glob, and removing one mid-capture fails that probe with +/// "No such image" — the bogus `probe_error` the staleness work exists to get +/// rid of. In-process state cannot see the other instance, so age is the only +/// brake, and [`PROBE_REAP_MIN_AGE_SECS`] is already the right one: a probe is a +/// `find` over a root filesystem, not a multi-minute job. +/// +/// Never fails the caller. Housekeeping, like every other sweep here. +pub async fn reap_probe_images() { + use bollard::image::{ListImagesOptions, RemoveImageOptions}; + + let docker = match get_docker() { + Ok(d) => d, + Err(e) => { + log::warn!("Could not reap leftover probe images: {}", e); + return; + } + }; + + let filters = HashMap::from([( + "reference".to_string(), + vec![format!("{}*", super::container::PROBE_IMAGE_PREFIX)], + )]); + let images = match docker + .list_images(Some(ListImagesOptions { + all: false, + filters, + ..Default::default() + })) + .await + { + Ok(images) => images, + Err(e) => { + log::warn!("Could not list leftover probe images: {}", e); + return; + } + }; + + let now = chrono::Utc::now().timestamp(); + for image in images { + // Unlike a container summary, an image summary always carries a + // `Created`, so there is no unknown-age case to defend against here. + if now - image.created < PROBE_REAP_MIN_AGE_SECS { + log::info!( + "Leaving probe image {:?} alone — it is younger than {} minutes, so it may belong \ + to another Triple-C instance's live probe", + image.repo_tags, + PROBE_REAP_MIN_AGE_SECS / 60 + ); + continue; + } + // By **tag**, never by image id. A `force` removal by id untags an + // image everywhere, so an id that happens to carry another name loses + // that name too — which is how a test fixture that tagged + // `alpine:latest` into this namespace deleted the user's alpine. A real + // leftover has exactly the one probe tag, so removing the tag removes + // the image; anything else keeps whatever other names it has. + for tag in image + .repo_tags + .iter() + .filter(|t| t.starts_with(super::container::PROBE_IMAGE_PREFIX)) + { + log::info!("Removing leftover probe image {}", tag); + if let Err(e) = docker + .remove_image( + tag, + Some(RemoveImageOptions { + force: true, + noprune: false, + }), + None, + ) + .await + { + log::warn!("Could not remove leftover probe image {}: {}", tag, e); + } + } + } +} + /// How old a `triple-c.probe=migration` container must be before /// [`reap_probe_containers`] will force-remove it, in seconds. /// @@ -993,6 +1087,36 @@ pub async fn manifest_from_container(container_id: &str) -> Result Result { + let image = super::container::commit_container_for_probe(container_id).await?; + + let manifest = manifest_from_image(&image) + .await + .map_err(|e| format!("Probe of the stopped container did not complete: {}", e)); + + if let Err(e) = super::container::remove_image_by_name(&image).await { + log::warn!( + "Could not remove the staleness probe's throwaway image {}: {} — the next probe of \ + this container reuses the name, which leaves this one dangling for the orphan sweep", + image, + e + ); + } + + manifest +} + /// The image ID (`sha256:…`) of a local image, or `None` if it is not present. /// /// Deliberately the **ID**, not a repo digest: locally built images and custom @@ -2146,4 +2270,169 @@ mod tests { assert!(!pin_is_reapable("pre-migration-handmade", false, ancient, &now)); assert!(!pin_is_reapable("latest", false, ancient, &now)); } + + // ── Live Docker ───────────────────────────────────────────────────────── + + /// The reaper finds a leftover probe image by prefix and — crucially — + /// refuses to remove a young one, because that image may be another + /// Triple-C instance's live probe. Only a real daemon can say whether the + /// `reference=` glob matches the names `get_probe_image_name` produces. + /// + /// The fixture is **committed**, not tagged and not built. An image's + /// `Created` is its own, not its tag's, so tagging something already on disk + /// into this namespace yields a fixture the reaper is right to call ancient + /// — and BuildKit stamps a fixed epoch on `docker build` output, so a built + /// one looks ancient too. A commit stamps *now*, verified against Engine + /// 29.6, which is also how real probe images get their age. + /// + /// Both of those mistakes were made here first, and one of them deleted an + /// unrelated `alpine:latest` — which is why `reap_probe_images` removes by + /// tag rather than by image id. + /// + /// ```text + /// cargo test -- --ignored --nocapture reaper_spares + /// ``` + #[cfg(unix)] + #[tokio::test] + #[ignore = "needs a Docker daemon; builds and removes a throwaway image"] + async fn the_reaper_spares_a_probe_image_young_enough_to_be_someone_elses() { + use std::process::Command; + + fn docker_out(args: &[&str]) -> std::process::Output { + Command::new("docker").args(args).output().expect("docker CLI") + } + + let base = std::env::var("TRIPLE_C_TEST_IMAGE") + .unwrap_or_else(|_| "alpine:latest".to_string()); + let name = crate::docker::container::get_probe_image_name("reapertest01234"); + + // A never-started container is enough to commit from, and leaves the + // daemon's run state alone entirely. + let created = docker_out(&["create", &base, "true"]); + assert!( + created.status.success(), + "could not create the fixture container from {}: {}", + base, + String::from_utf8_lossy(&created.stderr) + ); + let cid = String::from_utf8_lossy(&created.stdout).trim().to_string(); + + let committed = docker_out(&["commit", "--pause=false", &cid, &name]); + let _ = docker_out(&["rm", "-f", &cid]); + assert!( + committed.status.success(), + "could not commit the fixture image: {}", + String::from_utf8_lossy(&committed.stderr) + ); + + reap_probe_images().await; + + let still_there = Command::new("docker") + .args(["image", "inspect", &name]) + .output() + .expect("docker image inspect") + .status + .success(); + + let _ = Command::new("docker").args(["rmi", &name]).output(); + + assert!( + still_there, + "a probe image committed seconds ago was reaped — that is another \ + instance's live probe being broken, see PROBE_REAP_MIN_AGE_SECS" + ); + } + + /// A *stopped* container is readable, and what comes back is its writable + /// layer rather than the image it was created from. This is the whole point + /// of the function: the base image cannot answer it, and the project may + /// well have no snapshot image at all. + /// + /// Also asserts the throwaway commit leaves nothing behind, which no unit + /// test can. It has to assert on the `triple-c-probe-*` tags specifically: + /// the probe image is *tagged*, so a leak never shows up as a dangling + /// image and a dangling-set assertion here would pass either way. + /// + /// Ignored because it needs Docker and commits a container; run it with + /// + /// ```text + /// cargo test -- --ignored --nocapture stopped_container + /// ``` + #[cfg(unix)] + #[tokio::test] + #[ignore = "needs a Docker daemon; creates, commits and removes a throwaway container"] + async fn a_stopped_container_is_read_from_its_writable_layer() { + fn docker_cli(args: &[&str]) -> String { + let out = std::process::Command::new("docker") + .args(args) + .output() + .expect("docker CLI"); + assert!( + out.status.success(), + "docker {:?} failed: {}", + args, + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).trim().to_string() + } + fn probe_images() -> Vec { + let mut ids: Vec = docker_cli(&[ + "images", "-q", + "--filter", + &format!("reference={}*", crate::docker::container::PROBE_IMAGE_PREFIX), + ]) + .lines() + .map(|l| l.trim().to_string()) + .filter(|l| !l.is_empty()) + .collect(); + ids.sort(); + ids + } + + let image = std::env::var("TRIPLE_C_TEST_IMAGE") + .unwrap_or_else(|_| "ghcr.io/shadowdao/triple-c-sandbox:latest".to_string()); + // A marker only the writable layer can carry, under a MANIFEST_ROOTS root. + let marker = format!("/opt/probe-marker-{}", std::process::id()); + + // Another instance's live probe images are allowed to exist; what must + // hold is that this probe adds none of its own. + let before = probe_images(); + + let id = docker_cli(&[ + "run", "-d", "--label", "triple-c.managed=true", + "--entrypoint", "/bin/sh", + &image, "-c", "sleep 300", + ]); + let cleanup = |id: &str| { + let _ = std::process::Command::new("docker") + .args(["rm", "-f", id]) + .output(); + }; + + docker_cli(&["exec", &id, "mkdir", "-p", &marker]); + docker_cli(&["stop", "-t", "1", &id]); + + let result = manifest_from_stopped_container(&id).await; + + cleanup(&id); + + let manifest = result.expect("a stopped container must be probeable"); + assert!( + manifest.paths.iter().any(|e| e.path == marker && e.is_dir()), + "the probe read the image, not the container's writable layer: {} missing", + marker + ); + // Non-empty package sets prove the probe script really ran, rather than + // parsing an empty transcript into an empty-but-Ok manifest. + assert!( + !manifest.apt_manual.is_empty(), + "apt-mark showmanual came back empty, so the probe did not run" + ); + + assert_eq!( + probe_images(), + before, + "the throwaway probe image was not cleaned up" + ); + } } diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index 950cf35..dc62069 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -263,12 +263,20 @@ pub fn run() { // logged warning rather than a failed start. // // Ordering matters. Probes are removed first because a probe holds - // an image open and the sweep will not force; pins are untagged + // an image open and the sweep will not force — both the probe + // containers and the probe images, the latter being the one orphan + // the sweep can never reach on its own; pins are untagged // second so the images they were holding are dangling by the time // the sweep lists them; the sweep runs last and collects both. let projects_store_for_cleanup = projects_store_setup.clone(); tauri::async_runtime::spawn(async move { crate::docker::reap_probe_containers().await; + // Probe *images* too, and for a sharper reason: a probe + // container merely pins an image the sweep then refuses to + // touch, whereas a leftover probe image is tagged and so + // nothing else in this app can ever collect it. See + // `reap_probe_images`. + crate::docker::reap_probe_images().await; let reaped = crate::docker::reap_stale_migration_pins().await; if reaped > 0 { log::info!("Startup housekeeping dropped {} stale rollback pin(s)", reaped);