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. #376 has since been merged. The first commit here reverts it, removing the field from the API again, and the second commit adds the annotation. 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.

CODEOWNERS

The third commit makes Timofei Larkin (@lllamnyp) the code owner of the API types (/api/) and the generated CRDs (/charts/etcd-operator/crd-bases/).

Summary by CodeRabbit

• New Features
  • Configure client Service traffic distribution with an EtcdCluster annotation, including zone-local routing options. Removing the annotation clears the operator-managed setting; independently set Service values are preserved.
  • Invalid or unsupported values generate warning events without stopping reconciliation.
• Changes
  • The spec.clientService.trafficDistribution configuration has been removed; use the annotation instead.
• Documentation
  • Updated guidance on supported values, Kubernetes version requirements, routing behavior, and prerequisites for zone-local routing.

[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: c2f68f38-9ce2-40ce-9aa5-a691233578d4

📥 Commits

Reviewing files that changed from the base of the PR and between 88c3174 and 6d847e7.

📒 Files selected for processing: 1

• .github/CODEOWNERS

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

📝 Walkthrough

The operator now reads client Service traffic distribution from an EtcdCluster annotation instead of a typed spec field. It applies or releases the Service field through server-side apply, reports specified failures with Warning Events, and returns other apply errors.

Changes: Client Service traffic distribution

Layer / File(s)	Summary
Annotation contract and API removal controllers/helpers.go, api/v1alpha2/etcdcluster_types.go, api/v1alpha2/zz_generated.deepcopy.go, charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yaml, api/v1alpha2/client_service_envtest_test.go, .github/CODEOWNERS	Adds the annotation key and removes the spec.clientService API type, generated deep-copy code, CRD schema, and related envtest checks. Adds ownership rules for the API and CRD directories.
Traffic-distribution field reconciliation controllers/client_service.go, controllers/etcdcluster_controller.go, main.go	Parses the annotation and applies or releases the client Service field with server-side apply. Configures event recording and Event permissions, and handles invalid values, API rejection, and fields that the API server does not persist.
Reconciliation validation and documentation controllers/client_service_test.go, controllers/client_service_envtest_test.go, README.md, docs/concepts.md	Tests annotation parsing, field ownership, warnings, and API-server outcomes. Documents configuration, accepted values, behavior, and routing prerequisites.

Priority: ➖ Normal

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 traffic-distribution annotation
  EtcdClusterReconciler->>KubernetesAPIServer: apply or release client Service field
  KubernetesAPIServer-->>EtcdClusterReconciler: return persisted value or apply error
  EtcdClusterReconciler->>EventRecorder: emit Warning Event when required

Loading

Merge Risk: ⚪ Minimal · up to 6d847

The ownership change has no established merge-blocking issue in the supplied review scope.

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. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
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.
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 annotation.

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. (1 skipped: 1 unsupported.)

Resolution

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

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 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.}