Skip to content

fix(image): force-pull when digest pinned but missing locally#39

Merged
CMGS merged 2 commits into
masterfrom
fix-issue-37-force-pull-digest
May 11, 2026
Merged

fix(image): force-pull when digest pinned but missing locally#39
CMGS merged 2 commits into
masterfrom
fix-issue-37-force-pull-digest

Conversation

@CMGS
Copy link
Copy Markdown
Contributor

@CMGS CMGS commented May 11, 2026

Summary

cmd/core/helpers.go:EnsureImage already proves at the first Inspect(lookupRef=ImageDigest) that the requested digest isn't in the local cache. The subsequent Pull then passed force=false, which lets images/cloudimg/pull.go:62's URL-level short-circuit win:

if !force {
    if _, entry, ok := idx.Lookup(url); ok {
        if utils.ValidFile(conf.BlobPath(entry.ContentSum.Hex())) {
            skip = true   // skip HTTP re-download
        }
    }
}

That short-circuit's policy is "any cached blob for this URL is fine" — perfect for non-digest-pinned reuse, wrong when the caller wants a specific digest. The URL→blob mapping in the local index can point at a stale digest (cached from a previous version of the upstream tag); the post-pull Inspect(ImageDigest) then warns and returns, and the failure surfaces deep in vm.restore as a Backing file I/O error against a qcow2 backing file that doesn't exist locally.

This PR flips force=true exactly when vmCfg.ImageDigest != "". Same semantics users get manually from cocoon image pull --force.

needForce := vmCfg.ImageDigest != ""
if pullErr := b.Pull(ctx, pullRef, needForce, progress.Nop); pullErr != nil { ... }

Why is this fine

  • Cheap path stays cheap: no digest pinned → force=false, URL cache short-circuit fires when blob present
  • Slow path triggers only when the caller has shown the digest isn't local. Re-download recomputes sha256, overwrites the URL→blob index entry, post-pull Inspect(ImageDigest) validates
  • No behavior change for the OCI backend: digestPullRef already converts image@sha256:... for OCI refs, which are digest-immutable; the OCI Pull happily handles the redundant force flag
  • No new error paths: existing warning-on-failure semantics preserved for imported images

Test plan

  • New unit tests in cmd/core/ensure_image_test.go — fake backend records Pull(force), asserts:
    • ImageDigest pinned + digest missing locally → force=true
    • No ImageDigest set → force=false
    • ImageDigest pinned + digest already local → Pull not called at all
  • go test ./cmd/core/... -v passes
  • go build ./... clean
  • Integration regression on testbed: reproduce issue 37 scenario by manually planting a stale URL→digest entry, then verify clone now succeeds instead of dying in VerifyBaseFiles

Related

Closes #37.

Companion work:

  • cocoonstack/epoch#4/dl/{name}/{ref} route adds tag- and digest-aware URL forms, so issue 38's cocoonstack.snapshot.baseimage annotations can use immutable digest URLs that pin perfectly via this fix

EnsureImage already proves at the first Inspect call that the requested
ImageDigest isn't in the local cache. The subsequent Pull, however,
passed force=false and so hit the URL-level short-circuit in
images/cloudimg/pull.go that skips re-downloading when "any blob is
already cached under this URL." For mutable tags (e.g. epoch's /dl/
endpoint pointing at the latest push of a repo), the cached blob can
be a stale digest — exactly the scenario that left clones failing
downstream in vm.restore with a "Backing file I/O error" against a
qcow2 backing file whose digest doesn't exist on disk.

Pass force=true to Pull when ImageDigest is set. Cheap path (no digest
pinned) stays cheap. Slow path triggers only when the caller wants a
specific digest and we've shown it isn't local — same semantics users
already get from `cocoon image pull --force`.

Fixes #37.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a clone/restore failure mode for cloud image URLs when the VM config pins a specific ImageDigest but the local URL→blob cache points at stale content. It makes EnsureImage force a re-pull when a digest is pinned, aligning EnsureImage behavior with cocoon image pull --force.

Changes:

  • Update cmd/core/helpers.go:EnsureImage to call Pull(..., force=true, ...) when vmCfg.ImageDigest != "".
  • Add unit tests to assert the force decision and that Pull is skipped when the digest is already local.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
cmd/core/helpers.go Forces pulls when ImageDigest is pinned to bypass cloudimg URL-cache short-circuiting.
cmd/core/ensure_image_test.go Adds regression tests verifying EnsureImage passes the expected force flag and skips pulling when already local.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cmd/core/helpers.go Outdated
- Collapse 6-line rationale to one line in EnsureImage force-pull guard.
- Drop unused pullErr/postPullInspect from the test fake; tighten test docstrings.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EnsureImage with force=false silently skips re-pull when local URL cache is stale, clone dies in VerifyBaseFiles

2 participants