Add pull and inspect OCI capabilities - #302
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
767c0cc to
29667f6
Compare
jserv
left a comment
jserv
left a comment
There was a problem hiding this comment.
Reviewed the store, lock, durability, platform, and build wiring paths end to end against the branch. go vet, gofmt -l, and go test -race ./cmd/elfuse-oci/ are all clean on go1.26.6.
The design decisions in the commit message mostly hold, with one cluster that does not: the crash-recovery story. Findings 1 and 2 are the same root cause seen from two ends, and I reproduced both against the real store rather than reasoning about them. The rest are independent.
Comments are inline.
| return "", err | ||
| } | ||
| err = s.withLock(func() error { | ||
| present, err := s.hasImageLocked(h) |
There was a problem hiding this comment.
Presence is decided from index.json alone, so a store damaged by a crash can never heal.
Remove one layer blob and re-pull: it fails with store: sync image blobs: open .../blobs/sha256/<hex>: no such file or directory, and it fails identically on every later pull, because appendImageLocked is skipped and the blob is never refetched. The ref is stuck permanently.
Zero-filling that same blob at its original size is worse: the re-pull succeeds and pins the corrupt content, since ggcr's size check in layout/write.go:216 only rewrites a blob whose size differs.
Make presence mean "descriptor in index.json and every blob it names present at the right size", so a damaged image falls back to appendImageLocked.
| if err != nil { | ||
| return err | ||
| } | ||
| if err := writeFileDurable(s.indexPath(), b, 0o644); err != nil { |
There was a problem hiding this comment.
index.json commits durably here, but the layer blobs it names are fsynced later, in addImage's syncImageBlobs call at line 308. WriteImage above leaves them rename-atomic and nothing more, so the index that names them commits first.
A crash in this window is exactly what produces the unrecoverable store described above. The ordering the design comment claims holds for the pin, but it is inverted for the index.
Move the blob fsync ahead of the index write: call syncImageBlobs at the end of appendImageLocked, before writeFileDurable(s.indexPath(), ...).
| if err != nil { | ||
| return "", err | ||
| } | ||
| err = s.withLock(func() error { |
There was a problem hiding this comment.
crane.Pull returns a lazy image, so the layer download happens inside this lock, at s.path.WriteImage on line 357. The exclusive store flock is held for the whole network transfer, not just the index and pin update the lock comment describes.
One slow or hung registry therefore blocks every other elfuse-oci process, including a first-run openStore bootstrap. There is no context or deadline on the pull either, so "slow" has no upper bound.
Blobs are content-addressed, so write them unlocked and take the lock only around hasImageLocked, the index write, and the pin. Pass crane.WithContext with a timeout while you are here.
|
|
||
| # Go unit tests for the OCI image CLI (offline). | ||
| .PHONY: oci-test | ||
| oci-test: |
There was a problem hiding this comment.
Nothing runs this. oci-test is absent from check (mk/tests.mk:204) and from every workflow, and no job builds build/elfuse-oci, so the new Go package and its test suite are never compiled or exercised in CI. A regression here lands green.
Add oci-test to check behind HAVE_GO, or add a Linux job running go vet ./cmd/elfuse-oci/ && make oci-test. The package is pure Go, so a Linux runner covers it.
jserv
changed the title
29667f6 to
af5e37d
Compare
elfuse-oci is a standalone Go binary that owns the OCI image pipeline; elfuse itself stays a pure Linux syscall-to-Darwin runtime with no OCI commands. This first slice is the acquisition half: an OCI image-layout store plus the pull and inspect commands, built on go-containerregistry ($ELFUSE_OCI_STORE or ~/.local/share/elfuse/oci by default). The store is a spec-shape image layout other tools can read, with a refs.json pin table keyed by reference and platform, so one ref holds its arm64 and amd64 variants side by side rather than the second pull replacing the first; index descriptors carry their platform for the same reason, and inspect takes --platform to pick one. A corrupt store fails the command instead of being read as empty: a nil-object or flat-shape refs.json is rejected, and digestFor returns a distinct errNotPulled so callers can tell "not pulled" from "store broken". An exclusive flock serializes the refs.json and index.json read-modify-write cycles so concurrent pulls cannot lose pins or descriptors. Blob writes stay outside it, since blobs are content-addressed and crane's lazy layer download would otherwise hold the lock across the whole network transfer, and they run whether or not the index already names the image, so a blob a crash or an operator lost is refetched instead of failing every later pull. Store metadata writes are durable, not merely atomic: writeFileDurable writes a temp sibling, fsyncs, renames, then fsyncs the parent directory. Config, manifest, and index.json go through it rather than the layout package, whose in-place write could leave a partial blob a later size check accepts or a truncated index the store fails closed on; layer blobs keep the layout package's size-checked temp-and-rename write. The order is blobs, fsync, index, pin, so index.json never names blobs the page cache could lose. pull rejects a non-linux --platform OS at parsing, since a store entry for another OS could never be consumed; the arch stays open for Rosetta. crane.WithPlatform applies only to a manifest list, so pull also checks the fetched config against the request and rejects a single-manifest ref served for the wrong platform. Credentials come from the ambient default keychain, wrapped to bound its resolution: go-containerregistry drops the context around the credential-helper exec, so a wedged helper would hang the pull silently. timedKeychain caps the wait and names DOCKER_CONFIG as the way to force an anonymous pull. --timeout bounds the whole transfer, unbounded by default since a fixed bound would fail large images on slow links. make all builds elfuse-oci when a Go toolchain is on PATH and skips it with a notice otherwise; make oci-test runs the package tests, joins make check behind the same toolchain gate, and runs with go vet in a Linux CI job. go.mod declares the minimum language version rather than a patch-exact toolchain, so a host Go at or above it needs no download. docs/usage.md documents the store layout and the commands, and docs/oci-design.md the model, the C/Go boundary, and the scope limits.
af5e37d to
0f591a4
Compare
2 participants
We will introduce the go toolchain in this PR, as we rely on the existing libraries to perform the job.
Summary by cubic
Adds
elfuse-oci, a standalone Go CLI to pull and inspect OCI images into a spec-compliant local store, keepingelfuseOCI-agnostic. New behavior: pins images by reference and platform in a durable, lock-serialized store; default platform is linux/arm64, non-linux is rejected.refs.json(ref→manifest digest keyed byos/arch[/variant]). Metadata writes are durable (temp+fsync+rename+fsync dir). An exclusive.lockflock serializesindex.jsonand pin updates; corruptrefs.json/index.jsonfail closed. Index descriptors record platform. Blob writes and fsync happen before index/pin and heal missing/truncated blobs.github.com/google/go-containerregistry; supports--platformand--timeout(bounds the whole pull). Validates the pulled config matches the requested platform (single-manifest refs cannot silently mismatch). Progress prints to stderr. Credentials come from the default keychain, time-bounded with an actionable error that suggestsDOCKER_CONFIGfor anonymous pulls.--jsonthe raw config blob (not re-marshaled). Write errors propagate. Includeshelpandversion.make allbuildselfuse-ocionly whenGOis on PATH;make oci-testruns package tests. Adds a Linux CI job to vet and test the CLI. No impact toelfuseon Go-less hosts.$ELFUSE_OCI_STOREor~/.local/share/elfuse/oci.Rollout
elfuse-oci(make oci-test). Existingelfuseworkflows remain unchanged.build/elfuse-oci pull [--store DIR] [--platform] [--timeout DUR] <ref>andbuild/elfuse-oci inspect [--store DIR] [--platform] [--json] <ref>. Set$ELFUSE_OCI_STOREto override the store location.Written for commit 0f591a4. Summary will update on new commits.