From 879a2242a153cbb95255f27d8ae909d730263410 Mon Sep 17 00:00:00 2001 From: Paco Date: Thu, 22 Oct 2026 23:20:51 -0700 Subject: [PATCH] tailor: warn on swallowed cleanup and cache errors Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../tailor/crates/tailor-exec/src/executor.rs | 20 ++++++++++--- .../tailor-exec/src/output_artifacts.rs | 19 ++++++++++--- tools/tailor/crates/tailor/src/run.rs | 28 ++++++++++++------- 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/tools/tailor/crates/tailor-exec/src/executor.rs b/tools/tailor/crates/tailor-exec/src/executor.rs index 6875d0c76c..e87cd7a923 100644 --- a/tools/tailor/crates/tailor-exec/src/executor.rs +++ b/tools/tailor/crates/tailor-exec/src/executor.rs @@ -200,7 +200,7 @@ impl Executor for IcExecutor { cancel.clone(), ) .await; - let _ = fs::remove_file(&working_copy_path); + remove_working_copy(&working_copy_path); // Reclaim IC's root-owned staging tree. On success: chown so the caller can read the // outputs, then remove if the policy is scratch. On an IC failure: reclaim best-effort, @@ -360,7 +360,7 @@ impl IcExecutor { // A signed cell must stage `output.artifacts` — that is how IC emits the boot artifacts and // the `inject-files.yaml` manifest. No staging ⇒ the config declared no `output.artifacts`. let Some(plan) = staging else { - let _ = fs::remove_file(working_copy_path); + remove_working_copy(working_copy_path); return Err(ExecError::Other(format!( "image `{}` requests `signing:` but its IC config declares no `output.artifacts` \ (add the `output-artifacts` preview feature and an `output.artifacts` block)", @@ -383,7 +383,7 @@ impl IcExecutor { cancel.clone(), ) .await; - let _ = fs::remove_file(working_copy_path); + remove_working_copy(working_copy_path); let customize = customize?; if customize.exit_code != 0 { self.reclaim_scratch(&plan.dir, &intermediate, &context.runtime, cancel.clone()) @@ -505,6 +505,12 @@ fn ic_run_failed(result: &Result) -> bool { !matches!(result, Ok(outcome) if outcome.exit_code == 0) } +fn remove_working_copy(path: &Path) { + if let Err(err) = fs::remove_file(path) { + warn!(path = %path.display(), error = %err, "failed to remove IC working copy"); + } +} + #[derive(Debug, Default)] struct PreparedToolsDir { rw_copy: Option, @@ -579,7 +585,13 @@ async fn ensure_tools_dir_cache( ) .await { - let _ = fs::remove_dir_all(&plan.cache_dir); + if let Err(cleanup_err) = fs::remove_dir_all(&plan.cache_dir) { + warn!( + path = %plan.cache_dir.display(), + error = %cleanup_err, + "failed to remove incomplete tools-dir cache" + ); + } return Err(err); } Ok(()) diff --git a/tools/tailor/crates/tailor-exec/src/output_artifacts.rs b/tools/tailor/crates/tailor-exec/src/output_artifacts.rs index e6dca7f5f0..e18f7e8eee 100644 --- a/tools/tailor/crates/tailor-exec/src/output_artifacts.rs +++ b/tools/tailor/crates/tailor-exec/src/output_artifacts.rs @@ -10,6 +10,8 @@ use std::{ }; use serde_yaml_ng::Value; +use tracing::warn; + use tailor_config::OutputArtifactsPolicy; use crate::path_translate; @@ -114,12 +116,21 @@ pub(crate) fn apply( /// ever matches tailor's own `..*` — never a user-named directory. pub(crate) fn stale_staging_dirs(image_dir: &Path, slug: &str) -> Vec { let prefix = format!("{STAGE_PREFIX}.{slug}."); - let Ok(entries) = std::fs::read_dir(image_dir) else { - return Vec::new(); + let entries = match std::fs::read_dir(image_dir) { + Ok(entries) => entries, + Err(err) => { + warn!(path = %image_dir.display(), error = %err, "failed to scan for stale staging directories"); + return Vec::new(); + } }; entries - .flatten() - .map(|entry| entry.path()) + .filter_map(|entry| match entry { + Ok(entry) => Some(entry.path()), + Err(err) => { + warn!(path = %image_dir.display(), error = %err, "failed to read staging directory entry"); + None + } + }) .filter(|path| { path.file_name() .and_then(|name| name.to_str()) diff --git a/tools/tailor/crates/tailor/src/run.rs b/tools/tailor/crates/tailor/src/run.rs index fff51e9559..b9d5d3ce0f 100644 --- a/tools/tailor/crates/tailor/src/run.rs +++ b/tools/tailor/crates/tailor/src/run.rs @@ -13,6 +13,9 @@ use std::{ use indexmap::IndexMap; use serde::Serialize; +use tokio_util::sync::CancellationToken; +use tracing::warn; + use tailor_config::{ Arch, BaseImageCatalogue, BaseSource, ImageDefinition, Operation, OutputArtifactsPolicy, OutputFormat, OutputSpec, PreviewFeature, PullPolicy, RenderedCell, Runtime, ToolConfig, @@ -32,7 +35,6 @@ use tailor_exec::{ BollardRuntime, IcExecutor, NoopRuntime, ResolveInputs, WorktreeLock, ca_cert_name, resolve, }; use tailor_resolve::{OciFetcher, OciResolver}; -use tokio_util::sync::CancellationToken; use crate::{ cli::{ @@ -1344,7 +1346,9 @@ async fn convert(args: &ConvertArgs, engine: &EngineOverride) -> Result<(), AppE }), _ => Ok(()), }; - let _ = fs::remove_dir_all(&staging); + if let Err(err) = fs::remove_dir_all(&staging) { + warn!(path = %staging.display(), error = %err, "failed to remove convert staging directory"); + } let result = result.map_err(AppError::from)?; if result.exit_code != 0 { return Err(AppError::Message(format!( @@ -1575,7 +1579,9 @@ async fn build( // unreachable (`meta/docs/2026-06-29-container-runtimes.md` §4-§5). let runtime = establish_runtime(engine, &tool).await?; let base_hash_cache_dir = output_dir.join(TAILOR_STATE_DIR).join(BASE_HASH_CACHE_DIR); - let _ = fs::create_dir_all(&base_hash_cache_dir); + if let Err(err) = fs::create_dir_all(&base_hash_cache_dir) { + warn!(path = %base_hash_cache_dir.display(), error = %err, "base hash cache unavailable; files will be rehashed"); + } let resolver = OciResolver::with_cache_dir(base_hash_cache_dir.clone()); let lock = Lockfile::read(&workspace.root.join(LOCK_FILE))?; // Resolve toolchains and tools-dir sources over the full dependency closure @@ -1675,15 +1681,17 @@ async fn clean( let mut paths = Vec::new(); for target in &targets { - // A signed image also drops an enrollable CA cert beside each cell's image (§6); remove it - // too. Lenient on config: `validate` surfaces signing errors; cleanup should not fail on them. - let signed = tailor_config::resolve_signing( + // Keep cleanup lenient on invalid signing config, but surface the skipped CA cert. + let signed = match tailor_config::resolve_signing( target.definition.signing.as_ref(), tool.signing.as_ref(), - ) - .ok() - .flatten() - .is_some(); + ) { + Ok(signing) => signing.is_some(), + Err(err) => { + warn!(image = target.name(), error = %err, "cannot resolve signing during clean; CA certificate may remain"); + false + } + }; for cell in cells_selected(target, selector)? { paths.push(output_dir.join(tailor_core::artifact_name( cell.slug.as_ref(),