feat(agent): add the image-cache-agent DaemonSet and ImageCache CRD - #4
Merged
Conversation
ezekiel-alexrod
force-pushed
the
feature/MK8S-159-image-cache-agent
branch
from
July 27, 2026 10:31
9eba110 to
6518dcf
Compare
ezekiel-alexrod
force-pushed
the
feature/MK8S-159-image-cache-agent
branch
from
July 28, 2026 09:33
6518dcf to
05c81cf
Compare
This was referenced Jul 28, 2026
Design for the ImageCache CRD and the node agent ahead of the implementation: node-state reconciliation (single key, level-triggered, no finalizers), per-resource cache subdirectories with a sentinel file for ownership/completeness/GC, node labels as the sync-status contract, go-containerregistry for pull/extract, distroless rootless DaemonSet with explicit handling of host mount and permission constraints. Relates-to: MK8S-159
Scaffolding, with no hand-written Go code. Generated with kubebuilder
v4.15.0:
kubebuilder init --domain scality.com \
--repo github.com/scality/image-cache/agent \
--project-name image-cache-agent \
--skip-go-version-check
kubebuilder create api --group image-cache --version v1alpha1 \
--kind ImageCache --resource --controller --namespaced=false
followed by `make manifests generate fmt vet`. The only non-default
flag is --skip-go-version-check: the local Go toolchain reports a
non-standard version string that kubebuilder's parser rejects.
The generated README is the one exception: its placeholders are replaced
by what the module does and by a pointer to DESIGN.md, so the tree never
carries "TODO(user)" prose.
Relates-to: MK8S-159
Cluster-scoped, no status subresource (per-node status lives in node labels). spec: nodeSelector (exact match, empty selects all), required source, cachePath defaulting to /var/lib/image-cache. Names are capped at 63 characters as they become node label names. Hand-written: the API types and their validation markers. Regenerated from them with `make manifests generate`: the CRD manifest and the deepcopy code. Relates-to: MK8S-159
ezekiel-alexrod
force-pushed
the
feature/MK8S-159-image-cache-agent
branch
from
July 30, 2026 16:55
05c81cf to
b9caa17
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Per-resource cache directories under a cache path: atomic extraction (temporary directory, sentinel written last, rename into place) with file names flattened to their base names, state reporting for the reconciler, and garbage collection restricted to agent-owned directories so flat tarballs and foreign content are never touched. The sentinel records the resolved image digest and the expected file list, so manual deletions are detected. Relates-to: MK8S-159
Pull and flatten cache images with go-containerregistry: the reference is resolved for linux/amd64 (multi-arch indexes are resolved client side), the digest is reported for traceability, and the flattened filesystem comes out as a tar stream. The Puller interface keeps the implementation swappable and lets the reconciler tests use a fake. Relates-to: MK8S-159
Exact key/value node-selector matching (pod semantics, empty selects all) and idempotent computation of the image-cache.scality.com/ node labels: entries outside the prefix are never touched, stale prefixed entries are removed. Relates-to: MK8S-159
fsnotify events on the cache directories are forwarded as reconcile triggers on a single-slot channel: the workqueue collapses duplicates and the periodic resync covers missed events, so dropped bursts are harmless. Watched paths are adjusted after each reconcile pass and not-yet-existing directories are retried on the next pass. Relates-to: MK8S-159
Node-state reconciliation: every trigger (resource events, filesystem changes, a guaranteed startup pass, periodic resync) converges the whole node in one pass — pull and extract what is missing, garbage-collect what nothing desires, and maintain the image-cache.scality.com/<name> node labels. Labels are patched before the pulls so orchestration gating sees pending state early. Failures are per-resource: one unpullable image keeps its label pending and gets an event while the others converge; no finalizers are needed since deletion is just absence from the desired state. Hand-written: the reconciler and its tests. Regenerated from the new RBAC markers with `make manifests`: config/rbac/role.yaml. Relates-to: MK8S-159
Each pod reconciles its own node: NODE_NAME comes from the downward API and is required at startup, the filesystem watcher is wired in, and leader election is removed (per-node agents do not compete). The pod mounts the default cache path from the host at an identical path, tolerates control-plane taints, and runs rootless behind a root init container that hands the cache directory to the agent UID (fsGroup does not apply to hostPath volumes). Relates-to: MK8S-159
The scaffolded .dockerignore excluded everything without effectively re-including the Go sources (a bare !**/*.go cannot re-include files under an excluded tree), so the very first docker build failed with the sources missing from the context. Re-include the source trees explicitly and keep test files out. Default TARGETARCH to amd64 and trim the buildx platform list accordingly: the agent only ships for amd64. Relates-to: MK8S-159
Lint (via the module's own golangci-lint build), test (envtest), and image-build jobs on any agent/ change, mirroring the rpm-ci layout. The kubebuilder-scaffolded workflows lived under agent/.github/ where GitHub never discovers them; they are replaced by this root workflow. Relates-to: MK8S-159
The agent lays tarballs out in one subdirectory per ImageCache resource (see agent/DESIGN.md), so the preload script now also globs one level below the cache directory. Flat tarballs keep working and non-tar files such as the agent sentinel are ignored as before. Relates-to: MK8S-159
Deploy on kind, wait for the DaemonSet to be ready, and exercise the label contract end to end without a registry: an ImageCache whose source is unreachable must mark the node pending, and deleting it must remove the label. Relates-to: MK8S-159
ezekiel-alexrod
force-pushed
the
feature/MK8S-159-image-cache-agent
branch
from
July 30, 2026 17:13
b9caa17 to
80659cd
Compare
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Each package now defines the sentinels its failures fall into, and stamps foreign errors with one where they enter the agent: the API client, the filesystem and the registry. Doing it at the boundary is what keeps the classification, since Wrap on an error that is not a *errors.Error retitles it "unknown error". The library has no equivalent of errors.Join, so a pass that collects several failures aggregates them with apimachinery's NewAggregate, whose Is visits the aggregated errors and leaves the sentinels usable. The dependency pulls ginkgo v2.29.0 and gomega v1.41.0.
The reconcile key was the constant "node". Nothing reads it back, but it is what the manager logs as the object being reconciled, so every line of every agent carried the same placeholder. It is the node's name now, which is what a log aggregator groups on. The key stays a single value per process, so the workqueue keeps collapsing every trigger into one pass.
The constructor started a forwarding goroutine behind its caller's back. FSWatcher implements manager.Runnable instead, so the manager starts it, cancels it through its context and waits for it on shutdown. Three things follow. Runnables are gated on leader election by default, which is meaningless for a DaemonSet where every agent converges the node it runs on, so the watcher opts out and the flag goes away entirely: it was already dead, the lease RBAC having been dropped from config/rbac. Watch errors were dropped; they are logged now, being the only sign that repairs went from prompt to delayed. Start used to return nil when the fsnotify channels closed, which the manager reads as a runnable that finished its work. An agent whose watcher died would have looked healthy while silently falling back on the periodic resync. It returns an error now unless the context was cancelled.
Tarballs live in <cachePath>/<resource>/ and fsnotify is not recursive, so deleting one raised no event at all: only the periodic resync repaired it, although repairing that tampering promptly is what the watcher is for. Only a whole resource directory disappearing was ever observed. SetPaths watches each cache path together with the resource directories under it. A directory created later is not missed, since its creation is itself an event on the watched root and the pass that follows adds the watch. The repair test used to delete the whole resource directory, and documented the limitation in a comment. It deletes a single tarball now, which is what an operator reclaiming disk space does.
The resync was pinned at ten minutes in the constructor. It is a flag now, and defaults to an hour: it is not what retries a failed pass, which is requeued with backoff, nor what repairs tampering, which raises a filesystem event. What is left is drift that raised no event at all, such as one spanning an agent restart. Zero is accepted and turns the periodic pass off, which the help says, since RequeueAfter: 0 means "do not requeue" rather than "as often as possible".
Every logged error carried a stack trace, the expected ones included, which buried the message they were attached to.
A source that no registry could resolve was accepted and only failed on the node, one pull later, as a pending label and an event. It is checked at admission now, against the usual registry[:port]/repository[:tag][@digest] shape. The 512 character cap is not cosmetic: the API server derives the CEL rule cost from the declared maximum length, and rejects the CRD without it. The reference is not resolved at admission, only its shape is checked.
A watch that could not be added was skipped in silence, which covers two very different cases: a cache path no resource has created yet, which the next pass retries, and an exhausted inotify watch limit, which downgrades every repair to the periodic resync for as long as it lasts. The first stays silent, the second is logged.
The cache path is a map key, and the CRD accepts any absolute path, a trailing slash included. A resource declaring /var/lib/image-cache/ was keyed apart from the default /var/lib/image-cache that every pass scans, so the garbage collector visited that one directory twice: once with the keep set of the resource, once with none. The second visit deleted what the pass had just extracted, and the next pass pulled it again, forever. The path is canonical everywhere it is used now, including the event and the error a missing mount produces. The test creates two resources whose paths differ only by a trailing slash, and fails without the fix.
FSWatcher satisfies manager.Runnable and manager.LeaderElectionRunnable by convention only: nothing in the package names those interfaces, so removing NeedLeaderElection would compile and silently move the watcher back into the group the manager gates on leadership, leaving a single agent watching. Assert both at compile time. patchLabels reads as a small helper, and carries two rules the code does not show. It patches those labels alone, never a full update, because this agent is not the only writer of a Node object. And its base has to be copied before the labels are assigned, since the patch is the difference computed at Patch time: taking the base afterwards, or aliasing the node, would send an empty patch. Both are written down, and the base is named rather than built inline in the call.
ezekiel-alexrod
force-pushed
the
feature/MK8S-159-image-cache-agent
branch
from
August 14, 2026 16:00
e1fe94b to
6ab5753
Compare
anthony-treuillier-scality
approved these changes
Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add the image-cache-agent: a DaemonSet that keeps the on-disk image cache of
each node in sync with the desired state declared by
ImageCachecustomresources. It is the write side of the cache the
containerd-image-preloadRPM already reads from, so nodes can be given new content while they run
instead of only at provisioning time.
agent/DESIGN.mdis the authoritative spec and the best entry point for thereview. The main decisions:
ImageCacheCRD is cluster-scoped and has no status subresource:a single cluster-scoped object cannot represent divergent per-node states.
Per-node status is published as node labels
(
image-cache.scality.com/<name>: synced|pending), which orchestration cangate on, and failures are reported as events on the resource.
time. Every trigger (resource events, fsnotify, periodic resync, startup)
enqueues the same key, and each pass rebuilds the full desired state. That
makes deletion an ordinary case and removes the need for finalizers, which
would otherwise have to be one-per-node on a shared cluster-scoped object.
cachePath, marked complete by asentinel written last. The sentinel carries ownership (garbage collection
never touches foreign files, such as the flat tarballs written at
bootstrap), completeness, and the resolved digest.
images, so an artifact-oriented client would bring no benefit here. The
puller sits behind a small interface.
Two deployment constraints follow from the cache living on the host and are
documented in DESIGN.md and in the sample manifests: the namespace must
enforce the
privilegedPod Security Standard (hostPath is alreadydisallowed at
baseline), and the host directory must be writable by UID65532 (
fsGroupdoes not apply to hostPath, hence the chown init containerintegrators can drop).
Also in this branch:
containerd-image-preloadnow imports tarballs from one level ofsubdirectories, which is where the agent writes them.
agent-ciworkflow runs the linter, the envtest suite and a containerimage build on changes under
agent/.The commits are meant to be read one by one; kubebuilder scaffolding is kept
in its own commits whose messages record the exact command that generated
them, and the later commits separate hand-written code from regenerated
files.
Verified locally:
make lintclean, unit and envtest suites green(controller 92.9%, puller 90.0%, cache 80.3% coverage), the RPM bats suite
green on Rocky Linux 8 and 9, and the kind-based e2e smoke test green.
Relates-to: MK8S-159