feat(main): add observed versions of EtcdMembers#340
Conversation
Observe the etcd version each member is actually running (via the Maintenance Status RPC against the member's own endpoint) into EtcdMember.status.version, surfaced as the "Running" print column. Add an informational VersionDrifted condition comparing the observed running version against the intended spec.version; it is left unset when intent is not yet known. Observation is best-effort and never gates readiness. Extract the operator-side dial config out of discoverMemberID into a shared etcdDialConfig helper. Implements #338. Signed-off-by: Andrey Kolkov <androndo@gmail.com>
📝 WalkthroughWalkthroughAdds observed etcd running-version tracking to ChangesObserved member version and drift detection
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Reconciler as EtcdMemberReconciler
participant DialConfig as etcdDialConfig
participant EtcdClient as EtcdClusterClient
participant Status as EtcdMember.Status
Reconciler->>Reconciler: updateStatus (ready=true)
Reconciler->>DialConfig: etcdDialConfig(cluster)
DialConfig-->>Reconciler: tlsCfg, user, pass
Reconciler->>EtcdClient: Status(ctx, endpoint)
EtcdClient-->>Reconciler: StatusResponse or error
alt Status succeeds
Reconciler->>Status: set Version, compare to spec.version
Reconciler->>Status: set MemberVersionDrifted condition
else Status fails
Reconciler->>Status: keep previous Version (best-effort)
end
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces the capability to observe and report the actual running etcd version for each member. It adds a Version field to EtcdMemberStatus and a VersionDrifted condition to detect intent-vs-reality version drift. The member controller now queries the member's etcd endpoint via the Maintenance Status API when the member is ready. These changes are accompanied by updated CRD definitions, comprehensive unit tests, and updated documentation. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
controllers/etcdmember_controller.go (1)
1273-1300: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptional: version observation opens a fresh etcd connection every reconcile.
observeVersiondials a new etcd client (dial config +EtcdClientFactory+Status) on every Ready reconcile per member, purely to read a version that changes only across upgrades. At the 30s cadence this is tolerable, but you could reduce connection churn by observing on a longer interval, or only re-observing when a version change is plausible (e.g., after a Pod restart/image change). Best-effort behavior and error handling here look correct.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controllers/etcdmember_controller.go` around lines 1273 - 1300, `observeVersion` is opening a new etcd client on every Ready reconcile, which creates unnecessary connection churn. Update `EtcdMemberReconciler.observeVersion` to only run the `etcdDialConfig`/`EtcdClientFactory`/`Status` flow when a version change is actually plausible, or move this check to a longer-lived/less frequent path so it does not happen every reconcile. Keep the existing best-effort error handling and return the previous `member.Status.Version` on failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@controllers/etcdmember_controller.go`:
- Around line 1273-1300: `observeVersion` is opening a new etcd client on every
Ready reconcile, which creates unnecessary connection churn. Update
`EtcdMemberReconciler.observeVersion` to only run the
`etcdDialConfig`/`EtcdClientFactory`/`Status` flow when a version change is
actually plausible, or move this check to a longer-lived/less frequent path so
it does not happen every reconcile. Keep the existing best-effort error handling
and return the previous `member.Status.Version` on failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7cc27f5-e658-4dd5-84a7-647669aeff81
📒 Files selected for processing (7)
api/v1alpha2/etcdmember_types.gocharts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdmembers.yamlcontrollers/etcd_client.gocontrollers/etcdmember_controller.gocontrollers/etcdmember_controller_test.gocontrollers/testing_helpers_test.godocs/concepts.md
Timofei Larkin (lllamnyp)
left a comment
There was a problem hiding this comment.
LGTM — approving.
Adds an observed running-version field (EtcdMember.status.version), a VersionDrifted condition, a Running print column, and refactors the operator-side dial config out of discoverMemberID into a shared etcdDialConfig. The change is careful and well-tested: go build ./..., go vet ./..., and go test ./controllers/... all pass, make generate manifests produces no drift (CRD, deepcopy, print columns in sync), and the docs/concepts.md claims check out against the code. All five new tests exercise both ready paths (discovery branch with MemberID=="" and the steady-state default branch): version match → drift False/VersionMatched, mismatch → drift True/VersionMismatch, empty intent → condition unset, drift resolution, and Status-RPC error being non-fatal. Endpoint selection is correct — observeVersion dials only self (per-endpoint Status), unlike discoverMemberID which routes through voter peers. Deletion is guarded (Reconcile returns early on DeletionTimestamp before updateStatus). Version formats align: spec.version is bare and etcd StatusResponse.Version is bare too, so the direct string comparison is correct.
Two non-blocking recommendations:
-
Recurring steady-state cost (
controllers/etcdmember_controller.go:1104-1108). Before this change, a Ready member's 30 s reconcile hitting thedefaultbranch did no etcd I/O. Now every Ready reconcile opens a fresh etcd gRPC client (TLS handshake + auth) and issues a Status RPC, and for TLS clustersetcdDialConfigadditionally fetches the parent EtcdCluster plus the CA/client/credential secrets each time. That is N dials + several secret reads per 30 s cluster-wide, forever, purely for an informational field. It is bounded (5 s timeout) and best-effort, so not a defect, but consider observing less often than every reconcile if fleet load matters. -
Empty-version clobber guard (
controllers/etcdmember_controller.go:1105).if v := r.observeVersion(...); v != member.Status.Versionwill overwrite a goodstatus.versionwith""if a successful Status ever returns an emptyVersion. Real etcd always populates it, so this is not currently reachable, but tightening toif v != "" && v != member.Status.Versionis cheap hardening (cover with a test: fake with emptystatusVersionand nil error, assert the prior value survives).
Observe the etcd version each member is actually running (via the Maintenance Status RPC against the member's own endpoint) into EtcdMember.status.version, surfaced as the "Running" print column. Add an informational VersionDrifted condition comparing the observed running version against the intended spec.version; it is left unset when intent is not yet known. Observation is best-effort and never gates readiness. Extract the operator-side dial config out of discoverMemberID into a shared etcdDialConfig helper.
Implements #338.
Summary by CodeRabbit
New Features
Bug Fixes