Skip to content

Commit 9152b6e

Browse files
authored
fix(vm): boot a digest-pinned image from the cache without a registry call (#4582)
A registry-sourced sandbox boot authenticated with the registry, and for a tag resolved the manifest, before it looked at the root disk cache. It did so on every boot, even when the image reference carries a digest and the root disk for that digest is already cached. Each boot then waits on the registry, and a registry outage stops every boot of an image the host already has. A digest names the image's content, and since #4342 a cached root disk is published only after its manifest was verified against that digest. On a cache hit for a digest-pinned reference the registry cannot add anything, so return the hit before authenticating. Authentication and the pull stay on the miss path, and a tag still resolves through the registry. The hit emits the same Pulling, CacheHit and Pulled events as before. Behaviour change: a host whose registry credentials were revoked still boots digest-pinned images it has already cached, as a kubelet does with IfNotPresent and a digest-pinned image. Test: bootstrap_registry_cache_hit_for_a_digest_pin_makes_no_registry_request covers a miss (authenticates and fetches), a cached digest pin (no request at all) and a tag for the same content (still resolved). It fails without this change with ["GET /v2/"]. Closes #4581 Signed-off-by: Balaji Ganesan <bganesan@nvidia.com>
1 parent cb42f5d commit 9152b6e

2 files changed

Lines changed: 126 additions & 24 deletions

File tree

‎crates/openshell-driver-vm/src/driver.rs‎

Lines changed: 66 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3311,6 +3311,43 @@ impl VmDriver {
33113311
)
33123312
}
33133313

3314+
fn publish_pulling_event(&self, sandbox_id: &str, image_ref: &str) {
3315+
self.publish_platform_event(
3316+
sandbox_id.to_string(),
3317+
platform_event(
3318+
"vm",
3319+
"Normal",
3320+
"Pulling",
3321+
format!("Pulling image \"{image_ref}\""),
3322+
),
3323+
);
3324+
}
3325+
3326+
/// A registry-sourced root disk found in the cache: `Pulling`, the
3327+
/// `CacheHit` progress and `Pulled`, so the image step still advances.
3328+
async fn publish_registry_cache_hit(
3329+
&self,
3330+
sandbox_id: &str,
3331+
image_ref: &str,
3332+
image_identity: &str,
3333+
image_path: &Path,
3334+
) {
3335+
self.publish_pulling_event(sandbox_id, image_ref);
3336+
self.publish_vm_progress(
3337+
sandbox_id,
3338+
"CacheHit",
3339+
format!("Using cached VM root disk for image \"{image_ref}\""),
3340+
HashMap::from([
3341+
("image_ref".to_string(), image_ref.to_string()),
3342+
("image_source".to_string(), "registry".to_string()),
3343+
("cache_hit".to_string(), "true".to_string()),
3344+
("image_identity".to_string(), image_identity.to_string()),
3345+
]),
3346+
);
3347+
self.publish_pulled_event(sandbox_id, image_ref, image_path)
3348+
.await;
3349+
}
3350+
33143351
async fn ensure_cached_registry_rootfs_image(
33153352
&self,
33163353
sandbox_id: &str,
@@ -3321,6 +3358,31 @@ impl VmDriver {
33213358
let span_status = openshell_otel::ErrorStatusGuard::current();
33223359
info!(image_ref = %image_ref, "vm driver: ensuring cached root disk image (registry)");
33233360
let reference = parse_registry_reference(image_ref)?;
3361+
// A digest pin already names the image's content, and a cached root
3362+
// disk is only published after its manifest was verified against
3363+
// that digest. On a hit the registry cannot add anything, so neither
3364+
// authenticate nor resolve: the sandbox boots from the cache without a
3365+
// network round trip, and keeps booting while the registry is down.
3366+
// Tags still resolve through the registry below.
3367+
if let Some(digest) = reference.digest() {
3368+
let image_identity = bootstrap_image_cache_identity(digest);
3369+
let image_path = image_cache_rootfs_image(&self.config.state_dir, &image_identity);
3370+
if tokio::fs::metadata(&image_path).await.is_ok() {
3371+
info!(
3372+
image_identity = %image_identity,
3373+
image_path = %image_path.display(),
3374+
"vm driver: root disk image cache hit (digest-pinned, no registry call)"
3375+
);
3376+
self.publish_registry_cache_hit(
3377+
sandbox_id,
3378+
image_ref,
3379+
&image_identity,
3380+
&image_path,
3381+
)
3382+
.await;
3383+
return span_status.finish(Ok(image_identity));
3384+
}
3385+
}
33243386
info!(image_ref = %image_ref, "vm driver: authenticating with registry");
33253387
self.publish_vm_progress(
33263388
sandbox_id,
@@ -3365,40 +3427,20 @@ impl VmDriver {
33653427
let image_identity = bootstrap_image_cache_identity(&source_image_identity);
33663428
let image_path = image_cache_rootfs_image(&self.config.state_dir, &image_identity);
33673429

3368-
// Emit a driver progress hint for cache hits too and immediately
3369-
// follow with `Pulled` so the image step still advances cleanly.
3370-
self.publish_platform_event(
3371-
sandbox_id.to_string(),
3372-
platform_event(
3373-
"vm",
3374-
"Normal",
3375-
"Pulling",
3376-
format!("Pulling image \"{image_ref}\""),
3377-
),
3378-
);
3379-
33803430
if tokio::fs::metadata(&image_path).await.is_ok() {
33813431
info!(
33823432
image_identity = %image_identity,
33833433
image_path = %image_path.display(),
33843434
"vm driver: root disk image cache hit (no build needed)"
33853435
);
3386-
self.publish_vm_progress(
3387-
sandbox_id,
3388-
"CacheHit",
3389-
format!("Using cached VM root disk for image \"{image_ref}\""),
3390-
HashMap::from([
3391-
("image_ref".to_string(), image_ref.to_string()),
3392-
("image_source".to_string(), "registry".to_string()),
3393-
("cache_hit".to_string(), "true".to_string()),
3394-
("image_identity".to_string(), image_identity.clone()),
3395-
]),
3396-
);
3397-
self.publish_pulled_event(sandbox_id, image_ref, &image_path)
3436+
self.publish_registry_cache_hit(sandbox_id, image_ref, &image_identity, &image_path)
33983437
.await;
33993438
return span_status.finish(Ok(image_identity));
34003439
}
34013440

3441+
// A driver progress hint before the build, so the image step advances.
3442+
self.publish_pulling_event(sandbox_id, image_ref);
3443+
34023444
info!(
34033445
image_identity = %image_identity,
34043446
"vm driver: root disk image cache miss, acquiring build lock"

‎crates/openshell-driver-vm/src/registry_cache_tests.rs‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,3 +478,63 @@ async fn registry_cache_preserves_caller_digest_when_head_uses_another_algorithm
478478
.all(|request| request == "GET /v2/")
479479
);
480480
}
481+
482+
#[tokio::test]
483+
async fn bootstrap_registry_cache_hit_for_a_digest_pin_makes_no_registry_request() {
484+
let body = manifest("pinned");
485+
let digest = sha256(&body);
486+
let registry = Registry::start(
487+
digest.clone(),
488+
HashMap::from([("latest".to_string(), body.clone()), (digest.clone(), body)]),
489+
None,
490+
false,
491+
)
492+
.await;
493+
let root = tempfile::tempdir().unwrap();
494+
let driver = driver(root.path());
495+
let client = client();
496+
497+
// Not cached yet: a digest pin still authenticates and fetches (and here
498+
// fails at the blobs the fixture does not serve).
499+
ensure_image(&driver, &registry.reference(Some(&digest)), &client, true)
500+
.await
501+
.unwrap_err();
502+
assert!(
503+
registry
504+
.requests()
505+
.iter()
506+
.any(|request| request.contains(&format!("/manifests/{digest}"))),
507+
"{:?}",
508+
registry.requests()
509+
);
510+
511+
// Cached: the same pin boots from the cache with no request at all.
512+
let identity = bootstrap_image_cache_identity(&digest);
513+
let path = image_cache_rootfs_image(root.path(), &identity);
514+
fs::create_dir_all(path.parent().unwrap()).unwrap();
515+
fs::write(&path, b"cached disk").unwrap();
516+
let before = registry.requests().len();
517+
let resolved = ensure_image(&driver, &registry.reference(Some(&digest)), &client, true)
518+
.await
519+
.unwrap();
520+
assert_eq!(resolved, identity);
521+
assert_eq!(
522+
registry.requests().len(),
523+
before,
524+
"a digest-pinned cache hit reached the registry: {:?}",
525+
&registry.requests()[before..]
526+
);
527+
528+
// A tag naming the same content still resolves through the registry.
529+
let resolved = ensure_image(&driver, &registry.reference(None), &client, true)
530+
.await
531+
.unwrap();
532+
assert_eq!(resolved, identity);
533+
assert!(
534+
registry.requests()[before..]
535+
.iter()
536+
.any(|request| request == "HEAD /v2/test/image/manifests/latest"),
537+
"{:?}",
538+
registry.requests()
539+
);
540+
}

0 commit comments

Comments
 (0)