Skip to content

fix: keep image resolution sticky while the selector is unchanged - #58

Merged
johnnyhuy merged 2 commits into
mainfrom
fix/sticky-image-resolution
Oct 6, 2026
Merged

johnnyhuy merged 2 commits into
mainfrom
fix/sticky-image-resolution

Conversation

@johnnyhuy

Copy link
Copy Markdown
Contributor

Problem

Reviewing #51 raised a churn risk: DriftImageSelector compares a NodeClaim's booted image against status.resolvedImages, but the NodeClass controller re-resolves that field to the newest matching snapshot on every pass (every 5 minutes).

So the moment you publish any snapshot that matches your selector, status.resolvedImages moves, and #51's check would replace every running node — with no spec edit, no warning, and nothing in the docs saying so. Resolution was a moving target that drift was about to be pointed at.

Fix — resolution becomes sticky

1. imagefamily gains a preference option. Resolve(ctx, sel, arch, opts ...ResolveOption) now accepts WithPreferredID(id). After the normal match/sort pass, a preferred image that is still in the match set wins over the newest. A preferred ID that is unknown, or that no longer satisfies the selector, is ignored and resolution falls back to the newest — so stickiness self-heals rather than pinning dead IDs.

2. The controller passes what it already recorded. reconcileImages builds an architecture→ID map from status.resolvedImages and hands each arch its previous ID. A fresh NodeClass (no entry) resolves to the newest, exactly as before.

3. Transient failures stop erasing the status. Previously an hcloud API error cleared status.resolvedImages; the next healthy pass then re-picked the newest match — the same churn, triggered by a blip. Now only definitive answers clear it (unsupported family, selector matching nothing). An API failure keeps the recorded images and reports ImageResolutionFailed, which still blocks Create via the Ready condition.

Behaviour after this PR

Event Effect on status.resolvedImages
New snapshot published matching the current selector unchanged — no churn
spec.imageSelector edited so the recorded image stops matching re-resolves (and #51's drift check fires, as intended)
Recorded image deleted / deprecated falls out of the match set, re-resolves
Transient hcloud API error unchanged (previously: cleared)
First reconcile newest match, as before

Also in this PR

docs/configuration.md claimed imageSelector.version is "Ignored when selector is set". It is not — Resolve AND-combines the label selector (server-side) with the family/version substring match (client-side). Corrected while documenting the stickiness rule.

Tests

  • TestResolve_PreferredIDKeepsResolutionSticky — five cases: newest by default, preferred survives a newer snapshot, unknown preferred falls back, a selector edit excludes the preferred image (re-resolves), a preferred image that no longer matches the family falls back.
  • TestReconcile_ResolutionStickyUntilSelectorChanges — the end-to-end rule: reconcile → {amd64:12, arm64:13}; publish a newer snapshot → still {12,13}; edit imageSelector.version → {amd64:14, arm64:13} (the arch whose recorded image still matches stays put).
  • TestReconcile_APIErrorKeepsResolvedImages — 500s after a healthy pass keep the recorded images while flipping ImagesReady to ImageResolutionFailed.

Scope / ordering

pkg/providers/imagefamily, pkg/controllers/nodeclass, docs/configuration.md. No CRD change (nothing new in status), no provider-behaviour change beyond image selection.

Land this before #51 — otherwise #51's image-selector drift check is live against a moving resolution and one snapshot push rolls the fleet. Merge order: #55 → this → #51.

Validation

mise run ci green: lint 0 issues, vet, race tests (unit + e2e), build, generate-verify.

johnnyhuy and others added 2 commits October 6, 2026 22:42
status.resolvedImages was re-resolved to the newest matching snapshot on
every pass, so publishing any snapshot that matched the selector moved the
resolution — and the image-selector drift check in #51 would have replaced
the whole fleet five minutes later with no spec edit.

Resolve now takes an optional preferred image and keeps it while it is
still in the match set; the NodeClass controller passes the ID it already
recorded. A selector edit (or a deleted/deprecated image) drops it out of
the match set, so resolution moves exactly when the operator intended.
Transient hcloud failures now keep the recorded images instead of clearing
them, which had the same churn-through-a-blip failure mode.

Also corrects the imageSelector.version docs: version and selector are
AND-combined, not mutually exclusive.

Co-authored-by: opencode-agent[bot] <219766164+opencode-agent[bot]@users.noreply.github.com>
@johnnyhuy
johnnyhuy marked this pull request as ready for review October 6, 2026 11:51
@johnnyhuy
johnnyhuy merged commit a78cbd2 into main Oct 6, 2026
@johnnyhuy
johnnyhuy deleted the fix/sticky-image-resolution branch October 6, 2026 11:51
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.

1 participant