enhancement: reliable ValkeyNode.Status.Role

[jdheyburn]

This PR closes #261

Summary

The existing Status.Role being reported on ValkeyNode was quite naive in that it would only be updated after a reconciliation cycle, which is currently every 60s. This can mean it lags behind when failovers happen for whatever reason.

While nothing depends on Status.Role for now (it is meant purely for the end user), behind this PR will be one to patch a label (smth like valkey.io/current-role) on Pods of what their current role is (to be used in PDBs per-shard as part of #303). That implementation will sit in the same logic as this PR is adding.

Features / Behaviour Changes

When the pod is not ready, the role reported will be an empty string - given that we do not want to serve stale data.

• Status.Role now reflects role changes (failover, restart) within a reconcile or two rather than waiting up to 60s.
• When the pod is not ready the role is reported as an empty string, since we don't want to serve stale data.
• Restarted replicas no longer transiently report primary, the role is read from cluster slot ownership rather than INFO replication (which briefly reports master before a restarted replica re-syncs).
• The operator sets an internal valkey.io/observed-role annotation on ValkeyNode objects to drive prompt updates. It's a trigger only and written on change; end users should keep reading Status.Role.

Implementation

Role resolution stays with the ValkeyNode controller (it remains the only writer of Status.Role), but it now becomes event-driven instead of relying on the 60s cadence:

• How the role is determined
  • In cluster mode the role is read from slot ownership in the node's own CLUSTER NODES (owns a slot → primary, else replica), not from INFO replication
  • A just-restarted replica reports role:master for a few seconds before it re-establishes replication, so INFO's role (and the master/slave flag) is transiently wrong; slot ownership is correct throughout
  • Standalone (non-cluster) nodes have no slots and fall back to INFO replication.
• When it re-resolves
  • A filtered Pod watch triggers a reconcile on readiness / IP / phase changes
  • That covers restarts, but not the failover case when a replica is promoted its own pod doesn't change, so there's no Kubernetes event.
    • To cover that, the ValkeyCluster controller (which already scrapes CLUSTER NODES) writes a valkey.io/observed-role annotation on the ValkeyNode when it observes the live role drift, which pokes the node controller to re-resolve.
    • The annotation is a trigger only so the node never trusts it, it always re-reads live
    • A 30s backstop requeue (10s if a ready node's role couldn't be resolved) is the self-heal net.
• Not-ready / transient handling
  • Role is cleared to "" when the pod is not ready
  • If the pod is ready but the live read fails transiently, the last-known role is kept (not blanked) and we requeue sooner.
• The ValkeyCluster controller now scrapes cluster state whenever nodes are running, not only during rolls, so the hint actually fires in steady state (previously it only ran mid-roll).

Reviewer attention:

• The two-controller boundary: the ValkeyCluster controller writes only the observed-role annotation; Status.Role is written only by the ValkeyNode controller.
• The choice to read role from slot ownership rather than INFO replication (see the restart-transient reasoning above).
• The broadened cluster-state scrape (reconcileValkeyNodes), it's ~10ms per reconcile for a 6-node cluster today, and drops further once the client registry is implemented as part of [chore] Introduce client registry and provider to centralise Valkey client instantiation #266

Limitations

• Role is best-effort with a bounded lag
  • After a failover it catches up within a couple of seconds via the annotation hint; a fully silent failover (no pod event at all, e.g. a network partition) is bounded by the 30s backstop.
• The observed-role annotation is keyed on its own value, so if Status.Role somehow drifts while the annotation already matches the live role, the backstop heals it rather than an instant re-poke.
• A cluster primary that owns no slots yet (during initial formation or resharding) reads as replica until it owns slots.
• The wiring is covered by unit/envtest plus manual testing

Testing

Unit + envtest cover the role parsers, the set/clear/keep-last-known logic, the pod predicate and map function, the observed-role hint, and the scrape gating.

Tested locally on a Kind cluster:

• Deleting a replica pod
  • it rejoins as replica. (Before the slot-ownership change it flashed primary for ~7s, since INFO replication reports role:master until the replica re-syncs.)
• Deleting a primary pod
  • the promoted replica's Status.Role updates in ~2s (was ~15s), and observed-role annotations stay consistent across all nodes.

Checklist

Before submitting the PR make sure the following are checked:

• This Pull Request is related to one issue.
• Commit message explains what changed and why
• Tests are added or updated.
• Documentation files are updated.
• I have run pre-commit locally (pre-commit run --all-files or hooks on commit)

[jdheyburn]

@greptile-apps Review

[greptile-apps]

Greptile Summary

This PR makes ValkeyNode.Status.Role update from live node state more quickly. The main changes are:

• Adds pod-watch triggers for readiness, IP, and phase changes.
• Resolves cluster-mode roles from CLUSTER NODES slot ownership instead of transient replication info.
• Clears stale roles when pods are not ready and keeps the last known role on transient read failures.
• Adds valkey.io/observed-role annotations as cluster-controller hints for prompt failover updates.
• Expands unit, envtest, and e2e coverage for role parsing and role reporting.

Confidence Score: 5/5

Safe to merge with low risk.

The changed paths are focused and covered by unit, envtest, and e2e additions. No correctness or security issues were found in the role resolution, annotation trigger, or pod watch wiring.

No files require special attention.

T-Rex Logs

What T-Rex did

• The targeted role parser subtests ran and all passed with exit code 0.
• An envtest run attempted to start the kube-apiserver but timed out after 5 retries, no specs were run, and the process exited with code 1.
• The E2E compile step completed successfully with no tests run, using the E2E test binary with -tags=e2e and exiting with code 0.

_{Ran code and verified through T-Rex}

Important Files Changed

Filename	Overview
internal/controller/valkeynode_controller.go	Makes role resolution event-driven, clears stale roles for unready pods, and resolves cluster roles from slot ownership.
internal/controller/valkeycluster_controller.go	Broadens cluster-state scraping and patches observed-role annotations without changing Status ownership.
internal/controller/valkeynode_controller_test.go	Adds status role, resolver injection, pod mapping, and pod predicate coverage.
internal/controller/valkeycluster_controller_test.go	Adds coverage for PodIP scrape gating and observed-role hint behavior.
test/e2e/valkeycluster_test.go	Extends cluster e2e validation to require resolved node roles and one primary per shard.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Pod as Valkey Pod
participant NodeCtl as ValkeyNode controller
participant ClusterCtl as ValkeyCluster controller
participant VN as ValkeyNode CR
participant Valkey as Live Valkey cluster

Pod-->>NodeCtl: Pod Ready/IP/Phase event
NodeCtl->>Valkey: INFO / CLUSTER NODES
Valkey-->>NodeCtl: cluster_enabled + slot ownership
NodeCtl->>VN: Patch Status.Role

ClusterCtl->>Valkey: Scrape CLUSTER NODES during reconcile
Valkey-->>ClusterCtl: Live shard topology
ClusterCtl->>VN: Patch valkey.io/observed-role annotation on change
VN-->>NodeCtl: Annotation watch event
NodeCtl->>Valkey: Re-resolve role from live data
NodeCtl->>VN: Patch Status.Role if changed

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Pod as Valkey Pod
participant NodeCtl as ValkeyNode controller
participant ClusterCtl as ValkeyCluster controller
participant VN as ValkeyNode CR
participant Valkey as Live Valkey cluster

Pod-->>NodeCtl: Pod Ready/IP/Phase event
NodeCtl->>Valkey: INFO / CLUSTER NODES
Valkey-->>NodeCtl: cluster_enabled + slot ownership
NodeCtl->>VN: Patch Status.Role

ClusterCtl->>Valkey: Scrape CLUSTER NODES during reconcile
Valkey-->>ClusterCtl: Live shard topology
ClusterCtl->>VN: Patch valkey.io/observed-role annotation on change
VN-->>NodeCtl: Annotation watch event
NodeCtl->>Valkey: Re-resolve role from live data
NodeCtl->>VN: Patch Status.Role if changed

Loading

_{Reviews (1): Last reviewed commit: "fix: lint issue" | Re-trigger Greptile}

[melancholictheory]

this is the right foundation to build the label on, and keying the hint on the annotation value rather than Status.Role is a neat way to keep it churn-free. the bounded gap you documented is the honest tradeoff.

one thing worth pinning down now, since this PR fixes the meaning of "role" that the PDB in #303 will consume: valkey only counts masters that serve at least one slot as voters in a failover election (a master with numslots == 0 does not vote). so the set that matters for the quorum math we landed on, maxUnavailable = floor((shards-1)/2) over current primaries, is "masters with slots", not "masters".

that distinction shows up during a reshard: a primary whose last slots have just migrated away is still flagged master until it demotes, but it has already stopped voting. if liveRoleForAddress resolves through the master flag rather than slot ownership, that node reads as a primary and gets counted in the quorum while contributing nothing to it, which makes the PDB more permissive than the math intends. does GetPrimaryNode key on slots here?

for the follow-up label: worth thinking about the lifecycle when a pod is recreated. the label lives on the pod, not the pod template, so a fresh pod starts without it and a primaries PDB selecting on it won't match until the next reconcile patches it back. brief and self-healing, but it's a window where the node is unprotected right after a roll, so patching it as soon as the pod reports Ready is worth the ordering care.

and to state the invariant from #310 explicitly, since both live in the same area now: this label is the right input for the PDB because eviction re-evaluates it continuously, and the wrong input for topology spread, which is evaluated once at placement and wants the stable node-index=0 identity. same label, opposite conclusions.