feat(controllers): set client Service trafficDistribution from an annotation

[Timofei Larkin (lllamnyp)]

What

The etcd-operator.cozystack.io/client-service-traffic-distribution annotation on an EtcdCluster sets spec.trafficDistribution on the <cluster>-client Service. In a cluster stretched across zones, clients then prefer a ready member in their own zone, or on their own node with PreferSameNode. There is no CRD, API type or schema change.

metadata:
  annotations:
    etcd-operator.cozystack.io/client-service-traffic-distribution: PreferClose

Prior work

This builds on #376 by Andrei Kvapil (@kvaps), which adds the same capability as a typed spec.clientService.trafficDistribution field. Thank you for working out the use case, the version matrix and the test shape. Reused from #376:

• the scope: the client Service only, with the headless Service never touched;
• the "not latched through status.observed" decision;
• the Kubernetes version notes;
• the controller-test helpers and cases (create, drift, headless untouched);
• most of the docs prose.

What's different here:

• an annotation instead of an API field;
• removing the annotation clears the field. feat(api): set trafficDistribution on the client Service #376 left the field alone when unset, so there was no way to turn the setting off;
• both of Kirill Ilin (@sircthulhu)'s review points on feat(api): set trafficDistribution on the client Service #376 are handled. A value the apiserver rejects, or silently drops, no longer fails ensureServices and so no longer blocks bootstrap, scaling and status updates for the whole cluster.

Stopgap, and how it migrates

The annotation is a deliberate stopgap: the capability is needed now, and an API change is deferred for now. A typed field can replace it later. Migration would work like this: when both are set, the field wins. The annotation is deprecated, keeps working for a release or more, then is removed. Because both write through the same server-side-apply field manager, switching from one to the other doesn't flap the Service.

Behaviour

• Values: PreferClose (Kubernetes 1.31+), PreferSameZone and PreferSameNode (1.34+). The operator builds against k8s.io/api 0.33, whose constants define all three.

• Ownership: the field is written with server-side apply under field manager etcd-operator-traffic-distribution, and drift is reconciled back while the annotation is set. The operator's other Service writes are merge patches that never include this field. Removing the annotation releases the field, so it is cleared unless another manager also set it. A value set by hand on a cluster without the annotation is left alone. Clusters that don't use the annotation get no extra writes: ownership is checked from managedFields before any apply.

• Invalid values: the operator emits a Warning Event on the EtcdCluster, leaves the field unset and continues the reconcile:

  • a value outside the three above → InvalidTrafficDistribution;
  • a value the apiserver rejects with 422, e.g. PreferSameZone on 1.33 (confirmed against the envtest 1.33 apiserver) → TrafficDistributionRejected;
  • a value an apiserver without the field silently drops (before 1.31) → TrafficDistributionNotPersisted. In that case each reconcile repeats a no-op apply, which costs one request and no storage write.

  Transient errors still fail the reconcile so it is retried, as with every other Service write.

• Updates apply on the next reconcile. The controller watches EtcdCluster without a generation filter, so editing the annotation triggers a reconcile.

• Docs: docs/concepts.md#client-service-traffic-distribution covers the zone-label prerequisites on nodes, spreading members across zones, and proxy support.

Testing

• controllers/client_service_test.go (fake client) covers:
  • parsing of the annotation value;
  • no apply when the annotation is unused;
  • an invalid value;
  • a rejected value;
  • a dropped value;
  • a transient error being retried.
• controllers/client_service_envtest_test.go runs against a real 1.33 apiserver, because the fake client in controller-runtime 0.21 can't apply. It covers:
  • set then remove;
  • drift reconciled back, including after a hand edit takes ownership;
  • a hand-set value with no annotation is left alone;
  • an invalid value clears the field;
  • updating to PreferSameZone takes whichever outcome the apiserver version gives.
• go test ./controllers/ . with envtest 1.33 passes. go vet and gofmt are clean, and make manifests generate shows no drift: the events RBAC rule already existed.

Summary by CodeRabbit

• New Features
  • Added configurable traffic distribution for the client Service using an EtcdCluster annotation, including zone-local routing options.
  • Invalid or unsupported settings generate warning events, while reconciliation continues.
• Documentation
  • Documented supported values, Kubernetes version requirements, routing behavior, and how to clear the setting.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 0138ce9c-7dab-422c-a7bc-33c6573cc112

📥 Commits

Reviewing files that changed from the base of the PR and between e6c1314 and 8735074.

📒 Files selected for processing: 8

• README.md
• controllers/client_service.go
• controllers/client_service_envtest_test.go
• controllers/client_service_test.go
• controllers/etcdcluster_controller.go
• controllers/helpers.go
• docs/concepts.md
• main.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The operator now reads a traffic-distribution annotation on an EtcdCluster and reconciles the value on its client Service. It clears values it owns when the annotation is removed or invalid, reports warning Events for invalid or unpersisted values, and returns other apply errors.

Changes: Client Service traffic distribution

Layer / File(s)	Summary
Annotation and reconcile wiring controllers/helpers.go, controllers/client_service.go, controllers/etcdcluster_controller.go, main.go	Adds the annotation key and accepted-value parsing. Connects client Service reconciliation to the controller and configures event recording and Event permissions.
Traffic-distribution field reconciliation controllers/client_service.go	Applies valid values to the client Service with server-side apply. Releases the field when appropriate, checks field ownership, and handles API responses and warnings.
Validation and documented behavior controllers/client_service_test.go, controllers/client_service_envtest_test.go, README.md, docs/concepts.md	Adds unit and envtest coverage for annotation values, field ownership, warnings, and API-server outcomes. Documents configuration and behavior.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant EtcdCluster
  participant EtcdClusterReconciler
  participant KubernetesAPIServer
  participant EventRecorder
  EtcdCluster->>EtcdClusterReconciler: provide annotation during reconcile
  EtcdClusterReconciler->>KubernetesAPIServer: apply client Service trafficDistribution
  KubernetesAPIServer-->>EtcdClusterReconciler: return persisted value or apply error
  EtcdClusterReconciler->>EventRecorder: emit warning for invalid, rejected, or unpersisted value

Loading

Merge Risk: ⚪ Minimal · up to 87350

The change adds an opt-in annotation to configure traffic distribution on the client Service. Clusters without the annotation should behave as before, and no concrete merge-blocking risk was identified.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 6 files. (2 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: configuring client Service trafficDistribution from an EtcdCluster annotation.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 6 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}