feat(agent): adopt what the command seeded, and leave it until then (MK8S-434) by ezekiel-alexrod · Pull Request #26 · scality/image-cache

ezekiel-alexrod · 2026-09-24T10:35:34Z

Component

agent, config, docs

Problem

Stacked on #25. The agent's GC removes every directory with a sentinel that no ImageCache on the node claims, on every pass, the first one included. At install the DaemonSet can land before the resources, so it deletes what imagecachectl just seeded and pulls the same gigabyte again.
Also, once a resource claims the directory, nothing checks that it holds the image the resource asks for.

Fix

The sentinel now records who wrote the directory (owner), where it was read from (source) and the diff IDs of the image layers (layers), next to the manifest digest.

The GC only removes a directory whose sentinel names the agent. A sentinel with no owner is foreign too: the agent was never released, so none was written before. One that doesn't parse stays collectable, since it's damaged.
A resource claiming a seeded directory keeps its node pending until the agent checks the content. The agent resolves spec.source, which reads the manifest and the config, never a layer (TestRemoteResolveReadsNoLayer). Same layers: it rewrites the owner and labels the node synced with no pull. Other layers: it replaces the directory. Source unreachable: it leaves the directory alone and retries on the next pass.
The content is identified by the layer diff IDs, in order. They don't depend on how the image is stored. The source string differs between the ISO archive and the registry reference, the manifest digest changes on save and push, and the config digest changes when a Docker image is converted to OCI (keys written in another order). On kind, an archive saved from the Docker daemon was never adopted with the config digest. TestBothPullersAgreeOnTheLayers reproduces it.
The owner is rewritten through a temporary file renamed over the sentinel, so a crash never leaves a half-written sentinel.
imagecachectl runs as root and leaves root-owned 0700 directories, so the agent (UID 65532) couldn't read them. The chown-cache init container now also hands over the directories with a sentinel and the hidden temporaries, one level down, not recursively. Nothing else under the cache path changes.
A directory the store can't replace is refused before the pull, not only at the swap. On kind, the agent pulled the whole image on every pass, about 950 layer pulls in 90 seconds. An unreadable sentinel is reported with its cause.
The pull and extract sequence lives once, in internal/fill, called by both the command and the agent. Its own package, so cache keeps no registry code.
sentinel embeds Record, so the four recorded fields are written once. The file keeps the same flat keys.

Test

make -C agent test and lint are green, and every commit builds and passes on its own.
envtest: a seeded directory survives until a resource claims it, while the agent's own orphan next to it is collected. A matching one is adopted with 0 pull. A different image is replaced. An unreachable source leaves the directory untouched and the node pending, then adopts once it answers.
make -C agent test-e2e on kind seeds as root, before the agent deploys, a directory with a sentinel, a temporary with a file in it, and a directory without a sentinel. Only the first changes hands and the agent collects the temporary --> OK. Without the chown of the temporaries, the spec times out.
On kind with the real registry stack (operator, static-oci-registry, node agent): both ISO archives adopted with 0 layer, another image replaced with 1 layer, a foreign root directory refused with 0 registry request over 15 retries --> OK.
Mutations, each caught by a test: the GC owner check, a foreign owner ignored, never or always adopting, syncing after a failed resolve, comparing config digests or only the number of layers, pulling before the replaceable check, an unreadable sentinel read as missing.

Out of scope

Calling the command from MetalK8s (MK8S-394) and creating the resources at bootstrap (MK8S-396).
A directory seeded under a name no resource ever carries stays until someone removes it. The agent can't tell a name that will come from one that never will.

The docs describing this behaviour are updated in the same pull request: README.md, agent/README.md, DESIGN.md, agent/DESIGN.md, CONTRIBUTING.md, whichever owns it.
A change to the cache directory layout (subdirectory scheme, archive names, sentinel, permissions) lands in both halves and in agent/DESIGN.md. The preload service only globs *.tar and never reads the sentinel, so only agent/DESIGN.md changes.

Relates-to: MK8S-434

The agent and the command ran the same steps in the same order: check the cache path, pull the image, close the stream on the way out, extract it. They differed only in how they reported. The next changes touch what an extraction records, and two copies would have had to move together. Both now call internal/fill. Two small differences fall out of it. The agent refuses a cache path that is a regular file, as the command did, instead of failing later on the extraction. And it logs a failure to close the stream only once the extraction went through, as the command did: before that, the error it returns already says what happened. Relates-to: MK8S-434

…lling The digest a puller returned was the manifest's, and the manifest depends on how the image is stored: the same image read from a registry and from a docker archive has two. The digest of the image's configuration does not change with the transport or the registry endpoint, so it is the one two sources can be compared by. Both pullers now report both. Resolve returns them without reading a layer. For a registry that is the manifest alone, a few kilobytes, which is what the agent needs to tell whether a directory seeded from somewhere else holds the image a resource names. Relates-to: MK8S-434

The sentinel listed the files and a digest nothing read back. It now also records the owner, the source the content was read from, and the digest of the image's configuration. The agent writes image-cache-agent as owner and the command imagecachectl, so that the agent can tell a directory it wrote from one seeded before it arrived. The configuration digest is what the agent will compare against the image a resource names. The source is kept for whoever looks at the directory, not for that comparison: the same image is named by an archive path at install and by a registry reference in the resource. A sentinel written before this has none of the three fields and reads as before; an empty owner is the agent's. Relates-to: MK8S-434

Garbage collection removed every sentinel-bearing directory no resource claimed on the node, and it runs on every pass, the first one included. At install the agent can land before the resources that name what the command seeded, and it collected that cache and pulled it back from the registry: about a gigabyte, with the node holding none of the images in between. A directory whose sentinel names another owner is now left alone, whatever claims it or not. An empty owner still means the agent, so the directories every existing cluster holds are collected as before, and a sentinel that does not parse is still the agent's. Relates-to: MK8S-434

A resource that claims a directory the command seeded now checks what the directory holds before counting it as synced. The agent resolves the resource's source, which reads the manifest and the configuration and never a layer, and compares the configuration digest with the one the command recorded: - the same image: the agent takes the directory over, rewriting only the owner in the sentinel, and pulls nothing; - another image: the directory is replaced like any other that does not hold what its resource asks for; - the source cannot be resolved: the directory is left as it is, the resource stays pending, and the next pass tries again. The configuration digest is what identifies the content because it is the one identity a docker archive and a registry agree on. Comparing the source string would fail on the very first node, seeded from an archive path and claimed through a registry reference, and again whenever the registry endpoint changes. The manifest digest differs between an archive and a pushed image too. The sentinel is rewritten through a temporary file renamed over it, so a crash leaves the old owner or the new one, never a half written sentinel that would read as an incomplete directory to pull again. Relates-to: MK8S-434

The README warned that nothing checked a seeded directory against the resource that claims it, and the design said garbage collection removed a directory the command wrote under any other name. Neither holds now: the sentinel names its writer, garbage collection leaves the command's directories alone, and the agent compares configuration digests before taking one over. agent/DESIGN.md gains a section on adoption, with the reason the configuration digest identifies the content rather than the source string or the manifest digest. Relates-to: MK8S-434

The two log messages start with a capital letter, as agent/AGENTS.md asks, and the Adopted event names the resource and the writer apart: "adopted the directory imagecachectl seeded" read as if imagecachectl were the directory. Relates-to: MK8S-434

Three comments and two sentences of agent/DESIGN.md still said a sentinel marks a directory as the agent's, that replaceable() applies the same rule as garbage collection, and that a complete directory is never pulled again. The sentinel now names its writer: collection leaves another writer's directory alone, the swap still replaces any directory the store wrote under the resource's own name, and a seeded directory is checked once before the agent takes it over. Relates-to: MK8S-434

…nt's The design's introduction to the sentinel still called it the mark of an agent-owned directory, and the README called it the agent's, while the list right under the first one and the rest of the README explain that the command writes it too and that it names who did. Relates-to: MK8S-434

anthony-treuillier-scality · 2026-09-28T07:27:58Z

-			if _, err := os.Stat(filepath.Join(cachePath, e.Name(), sentinelName)); err != nil {
-				continue
-			}
+		if !stale && !s.agentOwned(filepath.Join(cachePath, e.Name())) {


Something strikes me: Does it mean that files put by anyone are left as is ?

Yes, on purpose. The cache path is shared, so the GC only removes what the agent wrote: a directory whose sentinel names the agent, or one of its own temporaries. A flat file or a directory from someone else is never touched.
Note that the preload service still imports any *.tar it finds there, that part doesn't change.

To be discussed with @TeddyAndrieux I believe

…-434-sentinel-owner Brings in the review fixes of #25. The cancellation test they add passes a record to Extract, which takes one on this branch. Relates-to: MK8S-434

…digest The agent adopted a seeded directory when the configuration digest the command recorded matched the one the resource's source resolves to. That digest hashes the configuration's bytes. A conversion from the Docker manifest format to OCI writes the configuration out again in another key order: same fields, same values, another digest. On a kind cluster serving the carrier through the registry stack, an ISO archive saved from the Docker daemon never matched the image the registry served, and every seeded directory was pulled again. The sentinel now records the layers' diff IDs, the digests of the uncompressed layers, and adoption compares them in order. They are what the directory holds and they do not depend on how the image is stored. Resolving still reads the manifest and the configuration, never a layer. TestBothPullersAgreeOnTheLayers serves an image from a registry and the same image, its configuration in another key order, from an archive. Compared by configuration digest the two read b7b1... and a6d6... Relates-to: MK8S-434

imagecachectl runs as root and leaves each resource directory owned by root, in mode 0700. The init container only chowned the cache root, so the agent, running as UID 65532, could not read the sentinel of a seeded directory, let alone rewrite it to adopt the directory or remove it to replace the content. The init container now also chowns the directories that hold the store's sentinel, and the hidden temporaries an interrupted extraction leaves, one level down and not recursively: the agent only has to write in the directory itself. It skips symbolic links, and nothing else under the shared path changes. The e2e suite seeds, as root on the kind node before the agent deploys, a directory with a sentinel, a temporary holding a file, and a directory without a sentinel. It checks that only the first changed hands and that the agent collects the temporary, which it cannot do without write access to it: with the temporaries left out of the chown, the spec times out. Relates-to: MK8S-434

…mage The store checked whether it could replace a resource's directory at the swap only, after the whole image had been pulled and extracted. A directory the agent could not take over was therefore pulled again on every pass: on a kind cluster, with a seeded directory the agent could not read, about 950 layer pulls in a minute and a half. Fill now asks the store first, through Store.Replaceable, and pulls nothing when the answer is no. Extract still checks at the swap. A sentinel that cannot be read is also reported as such, with its cause, instead of as a directory the store did not write: the refusal then points at who owns the directory, not at what it holds. Relates-to: MK8S-434

The store never uses it: to garbage collection, any owner other than the agent is foreign. Only the command writes it, so it lives in internal/cli, and the store tests use a writer of their own. Relates-to: MK8S-434

The sentinel held the same four fields as Record, plus the files, and three places copied them one by one. It now embeds Record, which carries the JSON names. The file keeps the same flat keys. Relates-to: MK8S-434

Store.Record and the Record type read the same at a call site. The method reads the sentinel from disk, which its name now says. Relates-to: MK8S-434

An empty owner meant the agent, so that sentinels written before owners were recorded stayed collectable. None exists: the agent was never released, and the command only now writes sentinels. The rule and the test that kept it go, and only a sentinel naming the agent is the agent's. One that does not parse is still collected, since it is damaged. Relates-to: MK8S-434

…-434-sentinel-owner Brings in cobra for the command line, and the design introduction fix of #25. cli.go keeps this branch's Owner next to the new help text. Relates-to: MK8S-434

anthony-treuillier-scality · 2026-09-30T12:32:26Z

+          command:
+            - sh
+            - -c
+            - |
+              cache=/var/lib/image-cache
+              chown 65532:65532 "$cache" || exit 1
+              for d in "$cache"/*/; do
+                [ -L "${d%/}" ] && continue
+                [ -f "$d.image-cache-agent.json" ] || continue
+                chown 65532:65532 "$d" || exit 1
+              done
+              for d in "$cache"/.*.tmp-*/; do
+                [ -L "${d%/}" ] && continue
+                [ -d "$d" ] || continue
+                chown 65532:65532 "$d" || exit 1
+              done


Why not using:
"chown -R 65532:65532 /var/lib/image-cache"

To discuss with @TeddyAndrieux: is /var/lib/image-cache a directory only for imagecachectl and the agent? If not, to whom else?
If yes, chown -R 65532:65532 /var/lib/image-cache is easier

ezekiel-alexrod added 8 commits September 24, 2026 10:55

ezekiel-alexrod requested a review from a team as a code owner September 24, 2026 10:35

ezekiel-alexrod requested review from TeddyAndrieux and anthony-treuillier-scality and removed request for a team September 24, 2026 10:37

ezekiel-alexrod self-assigned this Sep 24, 2026

ezekiel-alexrod added agent The image-cache-agent DaemonSet and its CRD P1 High priority labels Sep 24, 2026

claude Bot reviewed Sep 24, 2026

View reviewed changes

Comment thread agent/DESIGN.md

anthony-treuillier-scality reviewed Sep 29, 2026

View reviewed changes

ezekiel-alexrod added 8 commits September 29, 2026 12:01

Merge branch 'feature/MK8S-430-image-cache-command' into feature/MK8S…

f4f5d64

…-434-sentinel-owner Brings in the review fixes of #25. The cancellation test they add passes a record to Extract, which takes one on this branch. Relates-to: MK8S-434

refactor(agent): keep the command's owner name in the command

300b42e

The store never uses it: to garbage collection, any owner other than the agent is foreign. Only the command writes it, so it lives in internal/cli, and the store tests use a writer of their own. Relates-to: MK8S-434

refactor(agent): embed the record in the sentinel

8dbc108

The sentinel held the same four fields as Record, plus the files, and three places copied them one by one. It now embeds Record, which carries the JSON names. The file keeps the same flat keys. Relates-to: MK8S-434

refactor(agent): name the method that reads a record ReadRecord

d7a0d57

Store.Record and the Record type read the same at a call site. The method reads the sentinel from disk, which its name now says. Relates-to: MK8S-434

ezekiel-alexrod requested a review from anthony-treuillier-scality September 29, 2026 14:02

Merge branch 'feature/MK8S-430-image-cache-command' into feature/MK8S…

3884e91

…-434-sentinel-owner Brings in cobra for the command line, and the design introduction fix of #25. cli.go keeps this branch's Owner next to the new help text. Relates-to: MK8S-434

anthony-treuillier-scality reviewed Sep 30, 2026

View reviewed changes

Base automatically changed from feature/MK8S-430-image-cache-command to main October 1, 2026 13:52

Navigation Menu

feat(agent): adopt what the command seeded, and leave it until then (MK8S-434) - #26

feat(agent): adopt what the command seeded, and leave it until then (MK8S-434)#26
ezekiel-alexrod wants to merge 18 commits into
mainfrom
feature/MK8S-434-sentinel-owner

ezekiel-alexrod commented Sep 24, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anthony-treuillier-scality Sep 28, 2026

Uh oh!

ezekiel-alexrod Sep 29, 2026

Uh oh!

ezekiel-alexrod Oct 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anthony-treuillier-scality Sep 30, 2026

Uh oh!

ezekiel-alexrod Oct 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ezekiel-alexrod commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Component

Problem

Fix

Test

Out of scope

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anthony-treuillier-scality Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

ezekiel-alexrod Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

ezekiel-alexrod Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anthony-treuillier-scality Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

ezekiel-alexrod Oct 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ezekiel-alexrod commented Sep 24, 2026 •

edited

Loading