Skip to content

Mount an OCI image read-only into an actor - #923

Open
Ron Lev (ronlv10) wants to merge 10 commits into
agent-substrate:mainfrom
ronlv10:actortemplate-image-volume
Open

Mount an OCI image read-only into an actor#923
Ron Lev (ronlv10) wants to merge 10 commits into
agent-substrate:mainfrom
ronlv10:actortemplate-image-volume

Conversation

@ronlv10

@ronlv10 Ron Lev (ronlv10) commented Aug 13, 2026

Copy link
Copy Markdown

Fixes #783
Adds an image source to ActorTemplate's VolumeSource: a container can mount the contents of an OCI image it does not run. This is how tooling gets into images built by third parties without rebuilding them.

spec:
  containers:
  - name: sandbox
    image: docker.io/example/benchmark@sha256:...
    command: ["/ate/agent"]
    volumeMounts:
    - name: agent
      mountPath: /ate
  volumes:
  - name: agent
    image:
      reference: registry.example.com/agent@sha256:...

How it works

atelet pulls the image through the existing layer cache and records the volume's layers in the bundle's overlay spec, next to the rootfs layers. ateom composes the volume inside the bundle — the cached layers with no writable layer on top, so the mount is read-only — and the container binds it at the declared path. The volume is composed per container: containers of one actor may mount the same volume, and each gets its own mount point inside its own bundle, all backed by the same shared layers. On resume the volume is re-composed the same way.

References must be digest-pinned, the same rule as container images: a snapshot is only valid against the exact bytes it was taken with.

On micro-VMs the volume rides the same read-only virtio-fs share as the container rootfs: ateom stages each composed volume beside the rootfs on the host, and the guest binds it into the container at the declared path.

@google-cla

google-cla Bot commented Aug 13, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@ronlv10
Ron Lev (ronlv10) force-pushed the actortemplate-image-volume branch 3 times, most recently from 4e11b94 to 5cb2ccc Compare August 13, 2026 19:34
@ronlv10
Ron Lev (ronlv10) marked this pull request as ready for review August 13, 2026 19:37
@ronlv10
Ron Lev (ronlv10) force-pushed the actortemplate-image-volume branch 3 times, most recently from 8eca1d8 to 416ac52 Compare August 13, 2026 21:49
A volume can name an OCI image; the reference must be digest-pinned and
mounted by at least one container. ateapi projects it into the workload
spec on its own volume type.
@ronlv10
Ron Lev (ronlv10) force-pushed the actortemplate-image-volume branch from 416ac52 to 6137852 Compare August 13, 2026 21:54

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

huge +1 in concept, it's a fair bit of code understandably, I need to read back through it in depth. attaching one plausible looking agent finding now.

// ImageDigest is the manifest digest the volume's ref resolved to, in the
// same form and for the same reason as OverlaySpec.ImageDigest: the GC's
// root-set scan protects an image by digest.
ImageDigest string `json:"imageDigest,omitempty"`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 blocking 🔴 – Nothing roots these, so the GC can evict an image volume's layers while an actor is running on them. addSpecRoots in internal/imagecache/gc.go reads spec.ImageDigest and spec.Layers and never looks at spec.ImageVolumes, so a volume's digest never reaches RootSet.ImageDigests and its layer hexes never reach LayerHexes or LayerSets.

Both eviction guards then miss. The volume's image record fails the digest check and is retired, which drops its layers' refcount to zero, and the bundle-spec fallback that would otherwise save them is keyed on LayerHexes. The layers go while they are bind-mounted at <bundle>/volumes/<name>. A resume is the visible failure: setupImageVolumes calls FinalizeLayer on a directory that no longer exists.

The watermark makes it likelier rather than rarer — the GC fires under cache pressure, which is the state this feature encourages by adding images per actor.

addSpecRoots walking spec.ImageVolumes covers it. Worth giving each volume its own LayerSets signature rather than folding its layers into the rootfs one, since the signature is meant to match a record's exact layer list.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch- added !

The bundle overlay spec gains ImageVolumes; ateom composes each at
ateompath.ImageVolumeMountPath — a bind for one layer, an upperless
overlay for several, FinalizeLayer first so whiteouts hold.
resolveImageVolumes pulls each mounted image through the layer cache and
records its layers in the overlay spec; the OCI spec binds the composed
volume read-only at the declared path.
The micro-VM runtime builds its own guest OCI spec, so image volumes are
carried explicitly: ateompb.Container gains image_volume_mounts, staging
binds each composed volume into the read-only share beside the rootfs,
and the guest re-adds the binds at the declared paths.
A three-layer fixture pushed at test time: every layer visible, an upper
layer shadows a lower one, a whiteout hides a file, writes are refused,
and the volume survives suspend/resume.
Comment thread internal/proto/ateompb/ateom.proto Outdated
message DurableDirVolumeMount {
// volume_name is the name the ActorTemplate gave the volume. It selects the
// per-volume directory atelet prepared for the actor on the host.
// volume_name is the name the ActorTemplate gave the volume.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 nit 🟢 – This lost a sentence. It used to read "It selects the per-volume directory atelet prepared for the actor on the host", which is the only place that said what volume_name actually resolves to for a durable volume. The new ImageVolumeMount below doesn't need the same sentence — its source is composed in the bundle, not prepared by atelet — so this looks like collateral from writing the two messages side by side rather than an intended trim.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done - returned it back as in main

Comment thread internal/ateompath/ateompath.go Outdated
)
}

// The path is per-container: containers of one actor may mount the same

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 nit 🟢 – Two new exported helpers, and neither has a conventional doc comment: this one starts mid-thought ("The path is per-container") instead of with the identifier, and ImageVolumeMountPathInBundle below has none at all. The content is worth keeping — it explains why the path is per-container — it just needs "ImageVolumeMountPath returns ..." in front, and a line on the in-bundle variant saying when to reach for it (the caller already has a bundle path, e.g. setupImageVolumes).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done.

Comment thread cmd/ateom-microvm/run.go Outdated
if err := kata.ReconstructSharedDirFromImage(ctx, c.bundleRootfs, id, c.name); err != nil {
return nil, fmt.Errorf("while staging overlay lower for %q: %w", c.name, err)
}
// Image volumes ride the same read-only virtiofsd share.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We're moving towards using a single share for host <> guest and overlay otherwise.

cc Lucky Abolorunke (@Oneimu)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

(One share for everything, instead of per volume type with the carrier container back, for performance reasons)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

As far as I understand, this is aligned with the current implementation, right?
Did you mean this as an FYI, should I drop the comment as obsolete, or something else?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

removed the comment

@ronlv10 Ron Lev (ronlv10) left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Comment thread internal/proto/ateompb/ateom.proto Outdated
message DurableDirVolumeMount {
// volume_name is the name the ActorTemplate gave the volume. It selects the
// per-volume directory atelet prepared for the actor on the host.
// volume_name is the name the ActorTemplate gave the volume.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done - returned it back as in main

Comment thread internal/ateompath/ateompath.go Outdated
)
}

// The path is per-container: containers of one actor may mount the same

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done.

Comment thread cmd/ateom-microvm/run.go Outdated
if err := kata.ReconstructSharedDirFromImage(ctx, c.bundleRootfs, id, c.name); err != nil {
return nil, fmt.Errorf("while staging overlay lower for %q: %w", c.name, err)
}
// Image volumes ride the same read-only virtiofsd share.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

As far as I understand, this is aligned with the current implementation, right?
Did you mean this as an FYI, should I drop the comment as obsolete, or something else?

Comment thread internal/e2e/probe.go
// E2E_SANDBOX_CLASS selects the probe manifest variant; suites copy the
// probe's runtime, so this is what runs a suite on gVisor or micro-VM.
tmplName := "probe.yaml.tmpl"
if os.Getenv("E2E_SANDBOX_CLASS") == "microvm" {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nothing sets this, so we're not testing microvm?

we probably need to refactor e2e a bit so we don't depend on CI plumbing a bunch of options, but as a stopgap it seems OK to add this to the actions config.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ahh I didn't realize that - added

// StageImageVolume bind-mounts one composed image volume read-only at
// <cid>/volumes/<name> under SharedDir(id), so virtiofsd exposes it to the
// guest.
func StageImageVolume(ctx context.Context, src, id, cid, volumeName string) error {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion on ordering: StageImageVolume and #1034's kata.BindIntoShare are the same stale-umount/mkdir/rbind skeleton (this one adds the ro remount). Rather than landing two parallel bind-into-share helpers, could we let #1034 merge first and rebase this over it? Then this function becomes BindIntoShare + the ro remount (or a small ro option on it), and the tree has one copy of the mount logic.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If your pr merge first I can to do the unification on my side instead.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm fine with both ways let's see if it is merged today :)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I chatted with lucky, let's land your PR first and we'll have his PR more generally consolidate virtiofsd

run: hack/run-e2e-kind.sh ./internal/e2e/suites/demo -v -args --no-color
- name: Run E2E tests (micro-VM image volumes)
# Same image volume e2e suite on the micro-VM runtime.
env:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

TODO: factor this out so it's easy to test locally (not exclusive to this PR, it's the established pattern for now)

Comment thread internal/e2e/probe.go
}
RunCmdWithEnv(t, []string{"KO_CONFIG_PATH=" + root}, filepath.Join(root, "hack/run-tool.sh"), applyArgs...)

t.Cleanup(func() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Human comment: I think we should probably stop hardcoding ate-e2e-probe as the preferable fix.


🤖 should-fix 🟡 – This deletes a fixture two suites now share, and nothing serializes them.

hack/run-e2e.sh ends in go test -v ./internal/e2e/suites/... with no -p 1, so Go runs the suite packages concurrently as separate binaries — the mutexes in namespace.go and testmain.go only coordinate within one process. Before this PR only identity used the probe; now imagevolume calls DeployProbe too, and both get a cleanup that runs kubectl delete -f over the same manifest, namespace ate-e2e-probe included.

The likely ordering is the bad one. imagevolume only reads the probe's WorkerPool and ActorTemplate to clone them into its own namespace, so it finishes early; identity runs its actors inside ate-e2e-probe for the whole test (CreateActor, SuspendActor, ResumeActor, /whoami all against probeNamespace). So imagevolume's cleanup deletes the namespace out from under identity's running actors. The reverse order fails too: a re-apply into a Terminating namespace is rejected.

The kind cluster is throwaway, so simply not deleting the shared fixture would do it. Giving DeployProbe a namespace argument so each suite gets its own copy is the other direction, and would also let identity stop hardcoding "ate-e2e-probe" alongside imagevolume's use of e2e.ProbeNamespace.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add an image volume source to ActorTemplate

3 participants