Compare commits

..
Author SHA1 Message Date
shadowdaoandClaude Opus 5 307ea07409 Read a stopped container instead of claiming there is nothing to read
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 5s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m55s
Build App (Preview) / build-linux (pull_request) Successful in 4m56s
Build App (Preview) / build-windows (pull_request) Successful in 5m53s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
A project that was merely stopped reported "This project has no container
or snapshot image yet, so there is nothing to compare against the base
image" — with its container sitting right there — and Update stayed
disabled. Start it and the checks passed, which is the tell: the staleness
probe had only two sources, a *running* container via `docker exec` or the
project's snapshot image.

The snapshot is not a checkpoint. `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 have no
snapshot at all — and five of the six projects on the box that reported
this had none. Absence of a snapshot was being read as absence of anything
to inspect.

So probe the stopped container directly: commit its writable layer to a
throwaway image, probe that, drop it. A stopped container now also outranks
the snapshot, for the same reason a running one already did — the snapshot
lags it by everything installed since the last commit. `pick_probe_source`
is the whole decision and is unit-tested; the message it used to emit now
describes only the case it is true of, no container and no snapshot.

Two things found on the way, both documented in CLAUDE.md:

`bollard` never hands back the image id from a commit — its `Commit` model
deserialises "ID" while the daemon sends "Id" — so the probe image has to be
tagged, and a tagged image is dangling-proof and therefore invisible to
`sweep_orphaned_snapshots`, `reap_stale_migration_pins` and
`scrub_secrets_from_snapshots` alike. Without a reaper of its own a crashed
probe would leak a multi-gigabyte image that nothing could ever reclaim, so
`reap_probe_images` runs at startup beside `reap_probe_containers`, age-gated
for the same reason that one is: `reference=` is daemon-wide and a second
instance's live probe matches the glob.

It removes by tag, never by image id: a force removal by id untags an image
everywhere, which is how a first draft of the reaper test deleted an
unrelated `alpine:latest`. Names are unique per call rather than stable per
container, because container ids do not survive a recreate and two
overlapping probes would otherwise fight over one tag.

Verified against the container that reported the bug: 13,365 paths and an
apt delta of cmake, ffmpeg, libobs-dev, qt6-base-dev and nine more — the
migration payload the Update flow could not see. 546 unit tests plus three
live-Docker tests pass; no new clippy warnings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019RSaoDLovVV2wmH4H8VVxz
2026-09-10 19:24:05 -07:00
5 changed files with 591 additions and 16 deletions
+33
View File
@@ -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 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. migrating. `get_container_staleness` surfaces it; `migrate_project_to_base` acts on it.
- **A missing lineage label means "unknown, probe instead", never "stale".** - **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 - **`: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 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` snapshot. After the container swap, the new container's `triple-c.migration-state=in-progress`
+120 -11
View File
@@ -92,8 +92,72 @@ fn pick_recorded_lineage(
.or_else(|| from_snapshot.filter(|v| !v.is_empty())) .or_else(|| from_snapshot.filter(|v| !v.is_empty()))
} }
/// Read-only. Runs two filesystem probes (~3 s each) and is therefore meant to /// Reported as `probe_error` when there is genuinely nothing to read: no
/// be called on demand, not polled. /// 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-<id>: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<bool>, 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] #[tauri::command]
pub async fn get_container_staleness( pub async fn get_container_staleness(
project_id: String, project_id: String,
@@ -145,16 +209,25 @@ pub async fn get_container_staleness(
}; };
// ── Probes ─────────────────────────────────────────────────────────── // ── Probes ───────────────────────────────────────────────────────────
let running = match &container_id { let container_running = match &container_id {
Some(id) => docker::is_container_running(id).await.unwrap_or(false), Some(id) => Some(docker::is_container_running(id).await.unwrap_or(false)),
None => false, None => None,
}; };
let from_manifest = if running { let snapshot_exists = docker::image_exists(&snapshot_image).await.unwrap_or(false);
mig::manifest_from_container(container_id.as_ref().unwrap()).await let from_manifest = match (
} else if docker::image_exists(&snapshot_image).await.unwrap_or(false) { pick_probe_source(container_running, snapshot_exists),
mig::manifest_from_image(&snapshot_image).await &container_id,
} else { ) {
Err("This project has no container or snapshot image yet, so there is nothing to compare against the base image.".to_string()) (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 { let (from_manifest, base_manifest) = match from_manifest {
@@ -1964,6 +2037,42 @@ mod tests {
assert_eq!(pick_recorded_lineage(some(""), None), None); 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] #[test]
fn byte_sizes_read_the_way_a_disk_warning_should() { fn byte_sizes_read_the_way_a_disk_warning_should() {
assert_eq!(human_bytes(512), "512 B"); assert_eq!(human_bytes(512), "512 B");
+140 -4
View File
@@ -3052,6 +3052,118 @@ fn blanked_secret_env() -> Vec<String> {
.collect() .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<String, String> {
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::<String> {
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 /// Whether `env` (an image's `Config.Env`) holds a non-empty value for any
/// name in [`SECRET_ENV_KEYS`]. /// name in [`SECRET_ENV_KEYS`].
fn env_holds_a_secret(env: &[String]) -> bool { 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_image_by_name(&get_snapshot_image_name(project)).await
} }
/// Remove a Docker image by name/tag, treating "does not exist" as success. /// Remove a Docker image by name, tag or **id**, treating "does not exist" as
/// Shared by [`remove_snapshot_image`] and the pending-cleanup retry, which /// success. Shared by [`remove_snapshot_image`], the pending-cleanup retry
/// only has the image name (the project record is already gone by then). /// (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> { pub async fn remove_image_by_name(image_name: &str) -> Result<(), String> {
let docker = get_docker()?; let docker = get_docker()?;
@@ -3536,7 +3649,7 @@ pub async fn remove_image_by_name(image_name: &str) -> Result<(), String> {
.await .await
{ {
Ok(_) => { Ok(_) => {
log::info!("Removed snapshot image {}", image_name); log::info!("Removed image {}", image_name);
Ok(()) Ok(())
} }
Err(bollard::errors::Error::DockerResponseServerError { Err(bollard::errors::Error::DockerResponseServerError {
@@ -4464,6 +4577,29 @@ mod tests {
assert!(env_holds_a_secret(&env)); 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] #[test]
fn the_scrub_report_only_claims_success_when_nothing_is_left() { fn the_scrub_report_only_claims_success_when_nothing_is_left() {
let clean = SnapshotScrubReport { let clean = SnapshotScrubReport {
+289
View File
@@ -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 /// How old a `triple-c.probe=migration` container must be before
/// [`reap_probe_containers`] will force-remove it, in seconds. /// [`reap_probe_containers`] will force-remove it, in seconds.
/// ///
@@ -993,6 +1087,36 @@ pub async fn manifest_from_container(container_id: &str) -> Result<Manifest, Str
Ok(parse_manifest(&out)) Ok(parse_manifest(&out))
} }
/// Capture a [`Manifest`] from a **stopped** container.
///
/// Commits the container's writable layer to a throwaway image, probes that,
/// and removes it. This is as current as [`manifest_from_container`] — it reads
/// the same filesystem — and it is why a stopped project no longer has to fall
/// back to its snapshot image, which may not exist at all and lags the
/// container by everything installed since the last commit when it does.
///
/// The image is removed on every path, including a failed probe. See
/// [`super::container::commit_container_for_probe`] for what a crash in the
/// window between the two costs, and why it is bounded.
pub async fn manifest_from_stopped_container(container_id: &str) -> Result<Manifest, String> {
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. /// 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 /// 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("pre-migration-handmade", false, ancient, &now));
assert!(!pin_is_reapable("latest", 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<String> {
let mut ids: Vec<String> = 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"
);
}
} }
+9 -1
View File
@@ -263,12 +263,20 @@ pub fn run() {
// logged warning rather than a failed start. // logged warning rather than a failed start.
// //
// Ordering matters. Probes are removed first because a probe holds // 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 // second so the images they were holding are dangling by the time
// the sweep lists them; the sweep runs last and collects both. // the sweep lists them; the sweep runs last and collects both.
let projects_store_for_cleanup = projects_store_setup.clone(); let projects_store_for_cleanup = projects_store_setup.clone();
tauri::async_runtime::spawn(async move { tauri::async_runtime::spawn(async move {
crate::docker::reap_probe_containers().await; 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; let reaped = crate::docker::reap_stale_migration_pins().await;
if reaped > 0 { if reaped > 0 {
log::info!("Startup housekeeping dropped {} stale rollback pin(s)", reaped); log::info!("Startup housekeeping dropped {} stale rollback pin(s)", reaped);