[P0] Replace trigger-based Deployment drift checks with declarative reconciliation

[jacaudi]

Context

The Deployment is not reconciled declaratively. It is only rebuilt when one of four specific checks fires in internal/controller/wireguard_controller.go:

• agent image drift (:824)
• userspace flag toggle (:844)
• tunnel sidecar toggle (:861)
• nodeSelector / tolerations change (:871)

Anything else can be edited on the Wireguard resource and simply never take effect on an already-created Deployment.

Why it matters

Every new spec field on the roadmap (init containers, extra containers, volumes, gluetun) would otherwise need its own bespoke drift check. Miss one and the field silently does nothing after first create — the worst kind of bug to diagnose.

Scope

Compute the desired Deployment once, semantically diff it against the actual, and update when they differ. Take care not to hot-loop on server-defaulted fields (creationTimestamp, defaulted probe fields, terminationMessagePath, etc.).

Acceptance criteria

• Desired Deployment computed once and diffed against actual
• Changing any pod-affecting spec field triggers an update
• No update churn when nothing has changed — verified by (a) resourceVersion stable across several reconciles and (b) at most one apply per owned object per reconcile, counted per-object through an instrumented client. See the design-phase comment below for why (a) alone is insufficient.

Depends on #1. See docs/ROADMAP.md § Phase 0.2.

[jacaudi]

Design phase: two corrections to this issue, both from measurement

Design written to docs/designs/2026-08-29-declarative-deployment-reconciliation-design.md. Before proposing an approach I ran the candidate strategies against a real API server (envtest at the pinned Kubernetes 1.30.0, inside this module at its own pins — controller-runtime v0.18.5, k8s.io v0.30.3 — using the repo's actual builders at be6d4e4). Two things came back that change this issue.

1. The third acceptance criterion could not distinguish good from bad — now amended

The criterion read "verified by watching resourceVersion over several reconciles". It passes for a deliberately wasteful implementation.

A naive controllerutil.CreateOrUpdate whose mutate function does obj.Spec = desired.Spec — dropping every server-defaulted field — issues a genuine PUT on every single reconcile. Measured result:

Q5 reconcile #1 -> op=created   resourceVersion=227
Q5 reconcile #2 -> op=updated   resourceVersion=227 (stable vs prev=true)
Q5 reconcile #3 -> op=updated   resourceVersion=227 (stable vs prev=true)
Q5 reconcile #4 -> op=updated   resourceVersion=227 (stable vs prev=true)

resourceVersion and generation both stay pinned, because the apiserver re-defaults the submitted object, finds the serialized result byte-identical to what is stored, and discards the write. That short circuit lives in the generic registry's etcd3 GuaranteedUpdate (if !origState.stale && bytes.Equal(data, origState.data)), not in Deployment-specific code — so it is version-independent, not a 1.30 artifact.

The criterion has been amended to assert on applies issued per object as well.

One honest caveat, so nobody reads more into this than it supports: a write counter does not distinguish Server-Side Apply from CreateOrUpdate — both issue a round-trip every reconcile. What it catches is the defect this issue actually removes: the stale-read cascade, where all four drift checks compare against one deploymentFound fetched before the first write, so a reconcile with two drifting conditions issues two full writes.

Also worth recording, since it is the opposite of the common assumption: naive full-replace CreateOrUpdate does not hot-loop on server-defaulted fields. The Scope section's warning about creationTimestamp / terminationMessagePath is a real hazard for client-side diffing, but not for a full replace.

2. equality.Semantic.DeepDerivative is disqualified — it cannot detect removals

Worth recording because it is the obvious answer to "ignore server-defaulted fields" and it is wrong. In apimachinery/third_party/forked/golang/reflect/deep_equal.go, deepValueDerive returns true unconditionally when the desired value is a nil or empty slice, map or string, and otherwise compares only over len(desired).

Measured against this repo's own wstunnel toggle:

Q4  tunnel ENABLED  -> containers=2
Q4  tunnel DISABLED -> containers=1 (removal detected=true, under SSA)
Q4b DeepDerivative(desired=1-container, actual=2-container) = true   <-- "no change needed", i.e. MISSED

Adopting it would regress against criterion (b) in the removal direction.

Note: the line numbers in the issue body are stale

:824 / :844 / :861 / :871 predate PRs #39 and #49. Correct at be6d4e4: agent image :850, userspace toggle :874, wstunnel toggle :895, nodeSelector/tolerations :909, plus the create path at :830. Reconcile spans :432–:1035. Left in place rather than rewritten, since they will drift again — the design anchors on function and comment strings instead.

Scope decisions

• Service is included. It has the same disease in smaller form: the port-drift check compares len(Ports) and Ports[0].Port only, so changes to Ports[1], target ports, protocols, or spec.serviceAnnotations never take effect.
• updateWireguardPeers is explicitly deferred — peer-config rendering, not owned-object reconciliation, and [P3] Emit an internal peer config flavor pointing at the ClusterIP #17/[P3] Per-path MTU for direct vs tunnel configs #22 already touch that template.
• The Secret and ConfigMap stay out. The Secret already reconciles correctly via bytes.Equal on state.json; the ConfigMap has no data and therefore no drift surface (see The <name>-config ConfigMap and its envFrom are a vestigial pair #51 for the separate finding there).
• Criterion (b) will be met for every spec field except spec.metric, which is declared in the CRD and rendered nowhere. Filed separately as Remove spec.metric — declared in the CRD, rendered nowhere #50 — it is not fixable here, because there is nothing to render it into.