From 0b470954994110df10292cc7a0c829ad20004e16 Mon Sep 17 00:00:00 2001 From: HarshwardhanPatil07 Date: Tue, 8 Sep 2026 15:01:17 +0530 Subject: [PATCH 1/3] deploy: Retry transient registry blob pulls Direct image pulls can fail when a registry transiently rejects the GetBlob request. Retry that narrowly classified failure at the whole-pull boundary with a bounded attempt count, while allowing non-registry and unrelated failures to return immediately. Related: https://github.com/bootc-dev/bootc/issues/2177 Assisted-by: AI Signed-off-by: HarshwardhanPatil07 --- crates/lib/Cargo.toml | 1 + crates/lib/src/deploy.rs | 197 +++++++++++++++++- .../ostree-ext/src/container/unencapsulate.rs | 47 ++++- 3 files changed, 237 insertions(+), 8 deletions(-) diff --git a/crates/lib/Cargo.toml b/crates/lib/Cargo.toml index 87ed1184b4..4931616885 100644 --- a/crates/lib/Cargo.toml +++ b/crates/lib/Cargo.toml @@ -80,6 +80,7 @@ uapi-version = "0.4.0" [dev-dependencies] similar-asserts = { workspace = true } static_assertions = { workspace = true } +tokio = { workspace = true, features = ["test-util"] } [features] default = ["install-to-disk"] diff --git a/crates/lib/src/deploy.rs b/crates/lib/src/deploy.rs index b361a3b79f..72cd5dbee7 100644 --- a/crates/lib/src/deploy.rs +++ b/crates/lib/src/deploy.rs @@ -48,6 +48,7 @@ use std::collections::HashSet; use std::io::{BufRead, Write}; use std::os::fd::AsFd; use std::process::Command; +use std::time::Duration; use anyhow::{Context, Result, anyhow}; use bootc_utils::skopeo_bin; @@ -76,6 +77,14 @@ use crate::utils::async_task_with_spinner; // TODO use https://github.com/ostreedev/ostree-rs-ext/pull/493/commits/afc1837ff383681b947de30c0cefc70080a4f87a const BASE_IMAGE_PREFIX: &str = "ostree/container/baseimage/bootc"; +// Match podman's default registry retry policy. A failed attempt has to rebuild +// the importer, so retries are made at the whole-pull boundary instead of for +// individual layers. +// TODO: Read this policy from the proxy once +// https://github.com/podman-container-tools/container-libs/pull/951 is available. +const PULL_MAX_RETRIES: u32 = 3; +const PULL_RETRY_INITIAL_DELAY: Duration = Duration::from_secs(2); + /// Create an ImageProxyConfig with bootc's user agent prefix set. /// /// This allows registries to distinguish "image pulls for bootc client runs" @@ -769,8 +778,53 @@ pub(crate) async fn pull_from_prepared( Ok(Box::new((*import).into())) } -/// Wrapper for pulling a container image, wiring up status output. -pub(crate) async fn pull( +fn is_retryable_pull_error(error: &anyhow::Error) -> bool { + // TODO: Also classify transient prepare/OpenImage failures once + // containers-image-proxy exposes a typed error for them. Its current + // RequestInitiationFailure cannot safely distinguish DNS/TCP failures from + // permanent errors such as a missing image or a signature-policy rejection. + error.chain().any(|source| { + matches!( + source.downcast_ref::(), + Some(ostree_ext::containers_image_proxy::Error::BlobError( + ostree_ext::containers_image_proxy::GetBlobError::Retryable(_) + )) + ) + }) +} + +fn pull_retry_delay(retry: u32) -> Duration { + PULL_RETRY_INITIAL_DELAY.saturating_mul(2_u32.saturating_pow(retry)) +} + +pub(crate) async fn retry_pull_operation(mut operation: F) -> Result +where + F: FnMut() -> Fut, + Fut: std::future::Future>, +{ + let mut retries = 0; + loop { + match operation().await { + Ok(value) => return Ok(value), + Err(error) if retries < PULL_MAX_RETRIES && is_retryable_pull_error(&error) => { + let retry_delay = pull_retry_delay(retries); + let attempt = retries + 1; + tracing::warn!( + attempt, + max_retries = PULL_MAX_RETRIES, + retry_delay_seconds = retry_delay.as_secs(), + error = %error, + "Container image pull failed; retrying" + ); + retries += 1; + tokio::time::sleep(retry_delay).await; + } + Err(error) => return Err(error), + } + } +} + +async fn pull_once( repo: &ostree::Repo, imgref: &ImageReference, target_imgref: Option<&OstreeImageReference>, @@ -810,6 +864,31 @@ pub(crate) async fn pull( } } +/// Wrapper for pulling a container image, wiring up status output. +pub(crate) async fn pull( + repo: &ostree::Repo, + imgref: &ImageReference, + target_imgref: Option<&OstreeImageReference>, + quiet: bool, + prog: ProgressWriter, + booted_deployment: Option<&ostree::Deployment>, +) -> Result> { + let operation = || { + pull_once( + repo, + imgref, + target_imgref, + quiet, + prog.clone(), + booted_deployment, + ) + }; + if imgref.transport != "registry" { + return operation().await; + } + retry_pull_operation(operation).await +} + pub(crate) async fn wipe_ostree(sysroot: Sysroot) -> Result<()> { tokio::task::spawn_blocking(move || { sysroot @@ -1403,6 +1482,120 @@ pub(crate) fn fixup_etc_fstab(root: &Dir) -> Result<()> { mod tests { use super::*; + fn get_blob_failure(error: ostree_ext::containers_image_proxy::GetBlobError) -> anyhow::Error { + let error = ostree_ext::containers_image_proxy::Error::BlobError(error); + anyhow::Error::from(error).context("Unencapsulating base") + } + + fn retryable_blob_failure(message: &str) -> anyhow::Error { + get_blob_failure(ostree_ext::containers_image_proxy::GetBlobError::Retryable( + message.into(), + )) + } + + fn permanent_blob_failure(message: &str) -> anyhow::Error { + get_blob_failure(ostree_ext::containers_image_proxy::GetBlobError::Other( + message.into(), + )) + } + + fn open_image_failure(message: &str) -> anyhow::Error { + let error = ostree_ext::containers_image_proxy::Error::RequestInitiationFailure { + method: "OpenImage".into(), + error: message.into(), + }; + anyhow::Error::from(error).context("Creating importer") + } + + #[test] + fn test_retryable_pull_error_classification() { + let cases = [ + (retryable_blob_failure("502 Bad Gateway"), true), + (permanent_blob_failure("blob unknown"), false), + (open_image_failure("image unknown"), false), + ]; + + for (error, expected) in cases { + assert_eq!(is_retryable_pull_error(&error), expected, "{error:#}"); + } + } + + #[tokio::test(start_paused = true)] + async fn test_retry_pull_operation_succeeds() -> Result<()> { + let attempts = std::cell::Cell::new(0); + let start = tokio::time::Instant::now(); + let value = retry_pull_operation(|| { + let attempt = attempts.get() + 1; + attempts.set(attempt); + async move { + if attempt == 1 { + Err(retryable_blob_failure("502 Bad Gateway")) + } else { + Ok(42) + } + } + }) + .await?; + + assert_eq!(value, 42); + assert_eq!(attempts.get(), 2); + assert_eq!(start.elapsed(), pull_retry_delay(0)); + Ok(()) + } + + #[tokio::test(start_paused = true)] + async fn test_retry_pull_operation_stops_after_max_attempts() { + let attempts = std::cell::Cell::new(0); + let start = tokio::time::Instant::now(); + let error = retry_pull_operation(|| { + attempts.set(attempts.get() + 1); + async { Err::<(), _>(retryable_blob_failure("registry unavailable")) } + }) + .await + .unwrap_err(); + + let expected_delay = (0..PULL_MAX_RETRIES) + .map(pull_retry_delay) + .fold(Duration::ZERO, Duration::saturating_add); + assert_eq!(attempts.get(), PULL_MAX_RETRIES + 1); + assert_eq!(start.elapsed(), expected_delay); + assert_eq!( + error.root_cause().to_string(), + "retryable error: registry unavailable" + ); + } + + #[tokio::test(start_paused = true)] + async fn test_retry_pull_operation_does_not_retry_permanent_blob_error() { + let attempts = std::cell::Cell::new(0); + let error = retry_pull_operation(|| { + attempts.set(attempts.get() + 1); + async { Err::<(), _>(permanent_blob_failure("blob unknown")) } + }) + .await + .unwrap_err(); + + assert_eq!(attempts.get(), 1); + assert_eq!(error.root_cause().to_string(), "other error: blob unknown"); + } + + #[tokio::test(start_paused = true)] + async fn test_retry_pull_operation_does_not_retry_opaque_prepare_error() { + let attempts = std::cell::Cell::new(0); + let error = retry_pull_operation(|| { + attempts.set(attempts.get() + 1); + async { Err::<(), _>(open_image_failure("image unknown")) } + }) + .await + .unwrap_err(); + + assert_eq!(attempts.get(), 1); + assert_eq!( + error.root_cause().to_string(), + "failed to invoke method OpenImage: image unknown" + ); + } + #[test] fn test_new_proxy_config_user_agent() { let config = new_proxy_config(); diff --git a/crates/ostree-ext/src/container/unencapsulate.rs b/crates/ostree-ext/src/container/unencapsulate.rs index bbcbaac4d0..9c67f97291 100644 --- a/crates/ostree-ext/src/container/unencapsulate.rs +++ b/crates/ostree-ext/src/container/unencapsulate.rs @@ -155,7 +155,8 @@ pub struct Import { /// Or to restate all of the above - what this function does is check /// to see if the worker function had an error *and* if the proxy /// had an error, but if the proxy's error ends in `broken pipe` -/// then it means the real only error is from the worker. +/// then it means the real only error is from the worker. Otherwise the +/// proxy error remains in the error chain so callers can inspect typed errors. pub(crate) async fn join_fetch( worker: impl Future>, driver: impl Future>, @@ -169,7 +170,7 @@ pub(crate) async fn join_fetch( tracing::trace!("Ignoring broken pipe failure from driver"); Err(worker) } else { - Err(worker.context(format!("proxy failure: {text} and client error"))) + Err(driver.context(format!("client error: {worker:#}"))) } } (Ok(_), Err(driver)) => Err(driver), @@ -177,6 +178,37 @@ pub(crate) async fn join_fetch( } } +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_join_fetch_preserves_typed_driver_error() { + let runtime = tokio::runtime::Builder::new_current_thread() + .build() + .unwrap(); + let error = runtime + .block_on(join_fetch( + async { Err::<(), _>(anyhow!("client read failed")) }, + async { + let error = containers_image_proxy::Error::BlobError( + containers_image_proxy::GetBlobError::Retryable("connection reset".into()), + ); + Err::<(), _>(anyhow::Error::from(error)) + }, + )) + .unwrap_err(); + + assert!(error.chain().any(|source| matches!( + source.downcast_ref::(), + Some(containers_image_proxy::Error::BlobError( + containers_image_proxy::GetBlobError::Retryable(_) + )) + ))); + assert!(format!("{error:#}").contains("client read failed")); + } +} + /// Fetch a container image and import its embedded OSTree commit. #[context("Importing {}", imgref)] #[instrument(level = "debug", skip(repo))] @@ -185,7 +217,7 @@ pub async fn unencapsulate(repo: &ostree::Repo, imgref: &OstreeImageReference) - importer.unencapsulate().await } -/// A wrapper for [`ImageProxy::get_blob`] which fetches a layer and decompresses it. +/// A wrapper for [`ImageProxy::get_blob_stream`] which fetches a layer and decompresses it. pub(crate) async fn fetch_layer<'a>( proxy: &'a ImageProxy, img: &OpenedImage, @@ -202,7 +234,7 @@ pub(crate) async fn fetch_layer<'a>( use futures_util::future::Either; tracing::debug!("fetching {}", layer.digest()); let layer_index = manifest.layers().iter().position(|x| x == layer).unwrap(); - let (blob, driver, size); + let (blob_digest, size); let mut media_type: oci_image::MediaType; match transport_src { // Both containers-storage and docker-daemon store layers uncompressed in their @@ -231,15 +263,18 @@ pub(crate) async fn fetch_layer<'a>( } } - (blob, driver) = proxy.get_blob(img, &layer_blob.digest, size).await?; + blob_digest = &layer_blob.digest; } _ => { size = layer.size(); media_type = layer.media_type().clone(); - (blob, driver) = proxy.get_blob(img, layer.digest(), size).await?; + blob_digest = layer.digest(); } }; + let stream = proxy.get_blob_stream(img, blob_digest, size).await?; + let (blob, driver) = stream.into_parts(); + let blob = tokio::io::BufReader::new(blob); let driver = async { driver.await.map_err(Into::into) }; if let Some(progress) = progress { From 94cb1a56f05c3406042df8a6eee16456dc6d0146 Mon Sep 17 00:00:00 2001 From: HarshwardhanPatil07 Date: Tue, 8 Sep 2026 15:01:35 +0530 Subject: [PATCH 2/3] install: Use retrying pull for OSTree installs The non-unified installation path prepared and pulled images directly, bypassing the shared retry boundary and leaving bootc install to-filesystem exposed to transient registry failures. Route that path through the retrying pull while preserving unified-storage behavior. Related: https://github.com/bootc-dev/bootc/issues/2177 Assisted-by: AI Signed-off-by: HarshwardhanPatil07 --- crates/lib/src/install.rs | 47 +++++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 3c5182fd75..9232b7b729 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -195,7 +195,9 @@ use crate::bootc_composefs::{ use crate::bootc_kargs::{INITRD_ARG_PREFIX, ROOTFLAGS_KEY}; use crate::boundimage::{BoundImage, ResolvedBoundImage}; use crate::containerenv::ContainerExecutionInfo; -use crate::deploy::{MergeState, PreparedPullResult, prepare_for_pull, pull_from_prepared}; +use crate::deploy::{ + MergeState, PreparedPullResult, prepare_for_pull, pull_from_prepared, retry_pull_operation, +}; use crate::install::config::Filesystem as FilesystemEnum; use crate::lsm; use crate::progress_jsonl::ProgressWriter; @@ -1022,6 +1024,29 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result Ok((storage, has_ostree)) } +async fn pull_ostree_install_once( + repo: &ostree::Repo, + imgref: &ImageReference, + target_imgref: &ostree_container::OstreeImageReference, +) -> Result> { + let prepared = prepare_for_pull(repo, imgref, Some(target_imgref), None).await?; + pull_ostree_install_from_prepared(repo, imgref, prepared).await +} + +async fn pull_ostree_install_from_prepared( + repo: &ostree::Repo, + imgref: &ImageReference, + prepared: PreparedPullResult, +) -> Result> { + match prepared { + PreparedPullResult::AlreadyPresent(existing) => Ok(existing), + PreparedPullResult::Ready(image_meta) => { + crate::deploy::check_disk_space_ostree(repo, &image_meta, imgref)?; + pull_from_prepared(imgref, false, ProgressWriter::default(), *image_meta).await + } + } +} + #[context("Creating ostree deployment")] async fn install_container( state: &State, @@ -1073,25 +1098,23 @@ async fn install_container( // Auto-detection (None) is only appropriate for upgrade/switch on a running system. let use_unified = state.target_opts.unified_storage_exp; - let prepared = if use_unified { + let pulled_image = if use_unified { tracing::info!("Using unified storage path for installation"); - crate::deploy::prepare_for_pull_unified( + let prepared = crate::deploy::prepare_for_pull_unified( repo, &spec_imgref, Some(&state.target_imgref), storage, None, ) - .await? + .await?; + pull_ostree_install_from_prepared(repo, &spec_imgref, prepared).await? } else { - prepare_for_pull(repo, &spec_imgref, Some(&state.target_imgref), None).await? - }; - - let pulled_image = match prepared { - PreparedPullResult::AlreadyPresent(existing) => existing, - PreparedPullResult::Ready(image_meta) => { - crate::deploy::check_disk_space_ostree(repo, &image_meta, &spec_imgref)?; - pull_from_prepared(&spec_imgref, false, ProgressWriter::default(), *image_meta).await? + let operation = || pull_ostree_install_once(repo, &spec_imgref, &state.target_imgref); + if spec_imgref.transport == "registry" { + retry_pull_operation(operation).await? + } else { + operation().await? } }; From 09be175d48a719acb4f7b3e881c4e61aec404539 Mon Sep 17 00:00:00 2001 From: HarshwardhanPatil07 Date: Wed, 16 Sep 2026 16:52:47 +0530 Subject: [PATCH 3/3] deploy: Route pull retries through progress Share the pull progress renderer across attempts and print retry notices through indicatif so tracing output cannot interfere with active progress bars. Assisted-by: AI Signed-off-by: HarshwardhanPatil07 --- crates/lib/src/deploy.rs | 149 +++++++++++++++++++++++--------------- crates/lib/src/install.rs | 17 +++-- 2 files changed, 101 insertions(+), 65 deletions(-) diff --git a/crates/lib/src/deploy.rs b/crates/lib/src/deploy.rs index 72cd5dbee7..410327732c 100644 --- a/crates/lib/src/deploy.rs +++ b/crates/lib/src/deploy.rs @@ -85,6 +85,39 @@ const BASE_IMAGE_PREFIX: &str = "ostree/container/baseimage/bootc"; const PULL_MAX_RETRIES: u32 = 3; const PULL_RETRY_INITIAL_DELAY: Duration = Duration::from_secs(2); +/// Progress output shared by all attempts to pull an image. +#[derive(Clone)] +pub(crate) struct PullProgress { + bars: indicatif::MultiProgress, + json: ProgressWriter, + quiet: bool, +} + +impl PullProgress { + pub(crate) fn new(quiet: bool, json: ProgressWriter) -> Self { + let bars = indicatif::MultiProgress::new(); + if quiet { + bars.set_draw_target(indicatif::ProgressDrawTarget::hidden()); + } + Self { bars, json, quiet } + } + + fn println(&self, message: impl AsRef) { + if self.quiet { + return; + } + if let Err(error) = self.bars.println(message) { + tracing::debug!(%error, "Writing pull progress message"); + } + } + + fn clear(&self) { + if let Err(error) = self.bars.clear() { + tracing::debug!(%error, "Clearing pull progress"); + } + } +} + /// Create an ImageProxyConfig with bootc's user agent prefix set. /// /// This allows registries to distinguish "image pulls for bootc client runs" @@ -234,18 +267,14 @@ struct LayerProgressConfig { layers_total: usize, bytes_to_download: u64, bytes_total: u64, - prog: ProgressWriter, - quiet: bool, + progress: PullProgress, } /// Write container fetch progress to standard output. -async fn handle_layer_progress_print(mut config: LayerProgressConfig) -> ProgressWriter { +async fn handle_layer_progress_print(mut config: LayerProgressConfig) { let start = std::time::Instant::now(); let mut total_read = 0u64; - let bar = indicatif::MultiProgress::new(); - if config.quiet { - bar.set_draw_target(indicatif::ProgressDrawTarget::hidden()); - } + let bar = &config.progress.bars; let layers_bar = bar.add(indicatif::ProgressBar::new( config.n_layers_to_fetch.try_into().unwrap(), )); @@ -307,7 +336,7 @@ async fn handle_layer_progress_print(mut config: LayerProgressConfig) -> Progres subtask.bytes_total = actual_size; subtask.bytes = actual_size; subtasks.push(subtask.clone()); - config.prog.send(Event::ProgressBytes { + config.progress.json.send(Event::ProgressBytes { task: "pulling".into(), description: format!("Pulling Image: {}", config.digest).into(), id: (*config.digest).into(), @@ -342,7 +371,7 @@ async fn handle_layer_progress_print(mut config: LayerProgressConfig) -> Progres byte_bar.set_position(bytes.fetched); subtask.bytes_total = bytes.total; subtask.bytes = byte_bar.position(); - config.prog.send_lossy(Event::ProgressBytes { + config.progress.json.send_lossy(Event::ProgressBytes { task: "pulling".into(), description: format!("Pulling Image: {}", config.digest).into(), id: (*config.digest).into(), @@ -360,27 +389,26 @@ async fn handle_layer_progress_print(mut config: LayerProgressConfig) -> Progres } byte_bar.finish_and_clear(); layers_bar.finish_and_clear(); - if let Err(e) = bar.clear() { - tracing::warn!("clearing bar: {e}"); - } + bar.remove(&byte_bar); + bar.remove(&layers_bar); + config.progress.clear(); let end = std::time::Instant::now(); let elapsed = end.duration_since(start); let persec = total_read as f64 / elapsed.as_secs_f64(); let persec = indicatif::HumanBytes(persec as u64); - if let Err(e) = bar.println(&format!( + config.progress.println(format!( "Fetched layers: {} in {} ({}/s)", indicatif::HumanBytes(total_read), indicatif::HumanDuration(elapsed), persec, - )) { - tracing::warn!("writing to stdout: {e}"); - } + )); // Since the progress notifier closed, we know import has started // use as a heuristic to begin import progress // Cannot be lossy or it is dropped config - .prog + .progress + .json .send(Event::ProgressSteps { task: "importing".into(), description: "Importing Image".into(), @@ -397,9 +425,6 @@ async fn handle_layer_progress_print(mut config: LayerProgressConfig) -> Progres .into(), }) .await; - - // Return the writer - config.prog } /// Gather all bound images in all deployments, then prune the image store, @@ -673,6 +698,7 @@ pub(crate) async fn pull_unified( store: &Storage, booted_deployment: Option<&ostree::Deployment>, ) -> Result> { + let progress = PullProgress::new(quiet, prog); match prepare_for_pull_unified(repo, imgref, target_imgref, store, booted_deployment).await? { PreparedPullResult::AlreadyPresent(existing) => { // Log that the image was already present (Debug level since it's not actionable) @@ -699,7 +725,7 @@ pub(crate) async fn pull_unified( image: imgref.image.clone(), signature: imgref.signature.clone(), }; - pull_from_prepared(&cs_imgref, quiet, prog, *prepared_image_meta).await + pull_from_prepared(&cs_imgref, progress, *prepared_image_meta).await } } } @@ -707,8 +733,7 @@ pub(crate) async fn pull_unified( #[context("Pulling")] pub(crate) async fn pull_from_prepared( imgref: &ImageReference, - quiet: bool, - prog: ProgressWriter, + progress: PullProgress, mut prepared_image: PreparedImportMeta, ) -> Result> { let layer_progress = prepared_image.imp.request_progress(); @@ -716,6 +741,7 @@ pub(crate) async fn pull_from_prepared( let digest = prepared_image.digest.clone(); let digest_imp = prepared_image.digest.clone(); + let printer_progress = progress.clone(); let printer = tokio::task::spawn(async move { handle_layer_progress_print(LayerProgressConfig { layers: layer_progress, @@ -725,30 +751,31 @@ pub(crate) async fn pull_from_prepared( layers_total: prepared_image.layers_total, bytes_to_download: prepared_image.bytes_to_fetch, bytes_total: prepared_image.bytes_total, - prog, - quiet, + progress: printer_progress, }) .await }); let import = prepared_image.imp.import(prepared_image.prep).await; - let prog = printer.await?; + printer.await?; // Both the progress and the import are done, so import is done as well - prog.send(Event::ProgressSteps { - task: "importing".into(), - description: "Importing Image".into(), - id: digest_imp.clone().as_ref().into(), - steps_cached: 0, - steps: 1, - steps_total: 1, - subtasks: [SubTaskStep { - subtask: "importing".into(), + progress + .json + .send(Event::ProgressSteps { + task: "importing".into(), description: "Importing Image".into(), - id: "importing".into(), - completed: true, - }] - .into(), - }) - .await; + id: digest_imp.clone().as_ref().into(), + steps_cached: 0, + steps: 1, + steps_total: 1, + subtasks: [SubTaskStep { + subtask: "importing".into(), + description: "Importing Image".into(), + id: "importing".into(), + completed: true, + }] + .into(), + }) + .await; let import = import?; let imgref_canonicalized = imgref.clone().canonicalize()?; tracing::debug!("Canonicalized image reference: {imgref_canonicalized:#}"); @@ -783,6 +810,7 @@ fn is_retryable_pull_error(error: &anyhow::Error) -> bool { // containers-image-proxy exposes a typed error for them. Its current // RequestInitiationFailure cannot safely distinguish DNS/TCP failures from // permanent errors such as a missing image or a signature-policy rejection. + // https://github.com/bootc-dev/bootc/issues/2466 error.chain().any(|source| { matches!( source.downcast_ref::(), @@ -797,7 +825,10 @@ fn pull_retry_delay(retry: u32) -> Duration { PULL_RETRY_INITIAL_DELAY.saturating_mul(2_u32.saturating_pow(retry)) } -pub(crate) async fn retry_pull_operation(mut operation: F) -> Result +pub(crate) async fn retry_pull_operation( + progress: &PullProgress, + mut operation: F, +) -> Result where F: FnMut() -> Fut, Fut: std::future::Future>, @@ -809,13 +840,10 @@ where Err(error) if retries < PULL_MAX_RETRIES && is_retryable_pull_error(&error) => { let retry_delay = pull_retry_delay(retries); let attempt = retries + 1; - tracing::warn!( - attempt, - max_retries = PULL_MAX_RETRIES, - retry_delay_seconds = retry_delay.as_secs(), - error = %error, - "Container image pull failed; retrying" - ); + progress.println(format!( + "Container image pull failed; retrying in {} seconds ({attempt}/{PULL_MAX_RETRIES}): {error}", + retry_delay.as_secs() + )); retries += 1; tokio::time::sleep(retry_delay).await; } @@ -828,8 +856,7 @@ async fn pull_once( repo: &ostree::Repo, imgref: &ImageReference, target_imgref: Option<&OstreeImageReference>, - quiet: bool, - prog: ProgressWriter, + progress: PullProgress, booted_deployment: Option<&ostree::Deployment>, ) -> Result> { match prepare_for_pull(repo, imgref, target_imgref, booted_deployment).await? { @@ -859,7 +886,7 @@ async fn pull_once( "Pulling new image: {}", imgref ); - Ok(pull_from_prepared(imgref, quiet, prog, *prepared_image_meta).await?) + Ok(pull_from_prepared(imgref, progress, *prepared_image_meta).await?) } } } @@ -873,20 +900,20 @@ pub(crate) async fn pull( prog: ProgressWriter, booted_deployment: Option<&ostree::Deployment>, ) -> Result> { + let progress = PullProgress::new(quiet, prog); let operation = || { pull_once( repo, imgref, target_imgref, - quiet, - prog.clone(), + progress.clone(), booted_deployment, ) }; if imgref.transport != "registry" { return operation().await; } - retry_pull_operation(operation).await + retry_pull_operation(&progress, operation).await } pub(crate) async fn wipe_ostree(sysroot: Sysroot) -> Result<()> { @@ -1524,7 +1551,8 @@ mod tests { async fn test_retry_pull_operation_succeeds() -> Result<()> { let attempts = std::cell::Cell::new(0); let start = tokio::time::Instant::now(); - let value = retry_pull_operation(|| { + let progress = PullProgress::new(true, ProgressWriter::default()); + let value = retry_pull_operation(&progress, || { let attempt = attempts.get() + 1; attempts.set(attempt); async move { @@ -1547,7 +1575,8 @@ mod tests { async fn test_retry_pull_operation_stops_after_max_attempts() { let attempts = std::cell::Cell::new(0); let start = tokio::time::Instant::now(); - let error = retry_pull_operation(|| { + let progress = PullProgress::new(true, ProgressWriter::default()); + let error = retry_pull_operation(&progress, || { attempts.set(attempts.get() + 1); async { Err::<(), _>(retryable_blob_failure("registry unavailable")) } }) @@ -1568,7 +1597,8 @@ mod tests { #[tokio::test(start_paused = true)] async fn test_retry_pull_operation_does_not_retry_permanent_blob_error() { let attempts = std::cell::Cell::new(0); - let error = retry_pull_operation(|| { + let progress = PullProgress::new(true, ProgressWriter::default()); + let error = retry_pull_operation(&progress, || { attempts.set(attempts.get() + 1); async { Err::<(), _>(permanent_blob_failure("blob unknown")) } }) @@ -1582,7 +1612,8 @@ mod tests { #[tokio::test(start_paused = true)] async fn test_retry_pull_operation_does_not_retry_opaque_prepare_error() { let attempts = std::cell::Cell::new(0); - let error = retry_pull_operation(|| { + let progress = PullProgress::new(true, ProgressWriter::default()); + let error = retry_pull_operation(&progress, || { attempts.set(attempts.get() + 1); async { Err::<(), _>(open_image_failure("image unknown")) } }) diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 9232b7b729..7495679e0e 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -196,7 +196,8 @@ use crate::bootc_kargs::{INITRD_ARG_PREFIX, ROOTFLAGS_KEY}; use crate::boundimage::{BoundImage, ResolvedBoundImage}; use crate::containerenv::ContainerExecutionInfo; use crate::deploy::{ - MergeState, PreparedPullResult, prepare_for_pull, pull_from_prepared, retry_pull_operation, + MergeState, PreparedPullResult, PullProgress, prepare_for_pull, pull_from_prepared, + retry_pull_operation, }; use crate::install::config::Filesystem as FilesystemEnum; use crate::lsm; @@ -1028,21 +1029,23 @@ async fn pull_ostree_install_once( repo: &ostree::Repo, imgref: &ImageReference, target_imgref: &ostree_container::OstreeImageReference, + progress: PullProgress, ) -> Result> { let prepared = prepare_for_pull(repo, imgref, Some(target_imgref), None).await?; - pull_ostree_install_from_prepared(repo, imgref, prepared).await + pull_ostree_install_from_prepared(repo, imgref, prepared, progress).await } async fn pull_ostree_install_from_prepared( repo: &ostree::Repo, imgref: &ImageReference, prepared: PreparedPullResult, + progress: PullProgress, ) -> Result> { match prepared { PreparedPullResult::AlreadyPresent(existing) => Ok(existing), PreparedPullResult::Ready(image_meta) => { crate::deploy::check_disk_space_ostree(repo, &image_meta, imgref)?; - pull_from_prepared(imgref, false, ProgressWriter::default(), *image_meta).await + pull_from_prepared(imgref, progress, *image_meta).await } } } @@ -1097,6 +1100,7 @@ async fn install_container( // During install, we only use unified storage if explicitly requested. // Auto-detection (None) is only appropriate for upgrade/switch on a running system. let use_unified = state.target_opts.unified_storage_exp; + let progress = PullProgress::new(false, ProgressWriter::default()); let pulled_image = if use_unified { tracing::info!("Using unified storage path for installation"); @@ -1108,11 +1112,12 @@ async fn install_container( None, ) .await?; - pull_ostree_install_from_prepared(repo, &spec_imgref, prepared).await? + pull_ostree_install_from_prepared(repo, &spec_imgref, prepared, progress.clone()).await? } else { - let operation = || pull_ostree_install_once(repo, &spec_imgref, &state.target_imgref); + let operation = + || pull_ostree_install_once(repo, &spec_imgref, &state.target_imgref, progress.clone()); if spec_imgref.transport == "registry" { - retry_pull_operation(operation).await? + retry_pull_operation(&progress, operation).await? } else { operation().await? }