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

.github/CODEOWNERS

@@ -7,3 +7,7 @@
 # because the area is theirs, others because the role carries the repository.
 
 * @lllamnyp @androndo @sircthulhu
+
+# API surface: the Go types and the CRDs generated from them.
+/api/ @lllamnyp
+/charts/etcd-operator/crd-bases/ @lllamnyp

README.md

@@ -25,7 +25,7 @@ The full design rationale is in [docs/concepts.md](docs/concepts.md).
 - **TLS (BYO Secrets or cert-manager)**: `spec.tls.client` / `spec.tls.peer` enable TLS on each surface independently. Material comes from either user-provided Secrets (`serverSecretRef` / `operatorClientSecretRef` / `secretRef`) or operator-emitted `cert-manager.io/v1` Certificates (`certManager.{serverIssuerRef,operatorClientIssuerRef,issuerRef}`) — mutually exclusive per subtree, enforced by CEL. mTLS is the implicit mode when an operator-client source is supplied; server-TLS-only when it isn't. The whole `tls` subtree is CEL-locked immutable post-create. cert-manager-emitted certs auto-renew via cert-manager; Pod-side rotation is a manual one-at-a-time `kubectl delete pod` either way. See [docs/concepts.md](docs/concepts.md#tls).
 - **Resource sizing**: `spec.resources` (a `corev1.ResourceRequirements`) sets the etcd container's CPU/memory requests and limits. Unset uses a conservative 100m/128Mi-request default. Updates take effect on newly-created members; pair with a `VerticalPodAutoscaler` targeting the cluster for live recommendation/rollout.
 - **Scheduling & extra metadata**: `spec.affinity` and `spec.topologySpreadConstraints` pass through to every member Pod (anti-affinity is not defaulted — set it for production); `spec.additionalMetadata` merges user labels/annotations onto every object the operator creates (member Pods, data PVCs, Services, PDB, `EtcdMember` CRs), with operator-owned keys winning on collision. All three apply on object creation and are latched like the rest of the spec. See [docs/concepts.md](docs/concepts.md#pod-scheduling-and-additional-metadata).
-- **Zone-local clients**: `spec.clientService.trafficDistribution` (`PreferClose` / `PreferSameZone` / `PreferSameNode`) sets `trafficDistribution` on the `<cluster>-client` Service, so in a cluster stretched across zones clients prefer a ready member in their own zone. Unset leaves the field to whoever manages the Service. See [docs/concepts.md](docs/concepts.md#client-service).
+- **Zone-local clients**: the `etcd-operator.cozystack.io/client-service-traffic-distribution` annotation on an `EtcdCluster` (`PreferClose` / `PreferSameZone` / `PreferSameNode`) sets `trafficDistribution` on the `<cluster>-client` Service, so in a cluster stretched across zones clients prefer a ready member in their own zone. Removing the annotation clears the field; an invalid value is reported as a Warning Event and ignored. See [docs/concepts.md](docs/concepts.md#client-service-traffic-distribution).
 - **Monitoring / autoscaling hooks**: every member Pod always exposes a plaintext `metrics` container port at `2381` (etcd's `/health` + Prometheus `/metrics`) for `VMPodScrape` / `PodMonitor`. The `EtcdCluster` CRD exposes the `/scale` subresource with a populated `status.selector`, making it a valid target for `kubectl scale` and `VerticalPodAutoscaler.targetRef`.
 - **Locking pattern**: `status.observed` snapshots the in-flight target so mid-flight spec edits don't corrupt consensus; `progressDeadline` bounds how long the operator will spend trying to reach a target.
 - **Cluster deletion**: cascading owner refs clean up everything; finalizers detect "the whole cluster is going away" and skip etcd-side removal to avoid deadlock.

api/v1alpha2/etcdcluster_types.go

@@ -569,41 +569,6 @@ type EtcdClusterSpec struct {
 	// status.observed.
 	// +optional
 	ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty"`
-
-	// ClientService configures the "<cluster>-client" Service the operator
-	// creates for applications. Absent means the operator manages only the
-	// fields it always owns (selector and the client port) and leaves the
-	// rest of the Service alone. Does not affect the headless Service.
-	//
-	// Unlike the member-level fields above, this is not latched through
-	// status.observed: the Service carries no consensus state, so a change
-	// is applied on the next reconcile.
-	// +optional
-	ClientService *ClientServiceSpec `json:"clientService,omitempty"`
-}
-
-// ClientServiceSpec configures the "<cluster>-client" Service. Grouped in its
-// own struct so further Service-level knobs land next to TrafficDistribution
-// rather than as loose fields on EtcdClusterSpec.
-type ClientServiceSpec struct {
-	// TrafficDistribution sets the client Service's spec.trafficDistribution,
-	// a hint to the Service proxy (kube-proxy, Cilium, ...) on how to route
-	// traffic across the ready etcd members. The common use is a cluster
-	// stretched across zones: "PreferClose" (or "PreferSameZone") keeps
-	// clients talking to a member in their own zone while one is ready.
-	// The values mirror corev1.ServiceSpec.TrafficDistribution.
-	//
-	// When set, the operator writes it on create and reconciles drift back
-	// to it. When absent, the operator does not touch the field, so a value
-	// set on the Service by hand is preserved.
-	//
-	// Requires an apiserver that serves the field: Kubernetes 1.31+ for
-	// "PreferClose" (beta, on by default; GA in 1.33), and 1.34+ for
-	// "PreferSameZone" and "PreferSameNode" (PreferSameTrafficDistribution
-	// feature gate, beta and on by default from 1.34).
-	// +kubebuilder:validation:Enum=PreferClose;PreferSameZone;PreferSameNode
-	// +optional
-	TrafficDistribution *string `json:"trafficDistribution,omitempty"`
 }
 
 // AdditionalMetadata is a set of labels and annotations the operator merges

charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yaml

@@ -1166,40 +1166,6 @@ spec:
                       rule: '!has(self.source.pvc) || (has(self.source.pvc.subPath)
                         && size(self.source.pvc.subPath) > 0)'
                 type: object
-              clientService:
-                description: |-
-                  ClientService configures the "<cluster>-client" Service the operator
-                  creates for applications. Absent means the operator manages only the
-                  fields it always owns (selector and the client port) and leaves the
-                  rest of the Service alone. Does not affect the headless Service.
-
-                  Unlike the member-level fields above, this is not latched through
-                  status.observed: the Service carries no consensus state, so a change
-                  is applied on the next reconcile.
-                properties:
-                  trafficDistribution:
-                    description: |-
-                      TrafficDistribution sets the client Service's spec.trafficDistribution,
-                      a hint to the Service proxy (kube-proxy, Cilium, ...) on how to route
-                      traffic across the ready etcd members. The common use is a cluster
-                      stretched across zones: "PreferClose" (or "PreferSameZone") keeps
-                      clients talking to a member in their own zone while one is ready.
-                      The values mirror corev1.ServiceSpec.TrafficDistribution.
-
-                      When set, the operator writes it on create and reconciles drift back
-                      to it. When absent, the operator does not touch the field, so a value
-                      set on the Service by hand is preserved.
-
-                      Requires an apiserver that serves the field: Kubernetes 1.31+ for
-                      "PreferClose" (beta, on by default; GA in 1.33), and 1.34+ for
-                      "PreferSameZone" and "PreferSameNode" (PreferSameTrafficDistribution
-                      feature gate, beta and on by default from 1.34).
-                    enum:
-                    - PreferClose
-                    - PreferSameZone
-                    - PreferSameNode
-                    type: string
-                type: object
               imagePullSecrets:
                 description: |-
                   ImagePullSecrets is a list of Secret references in the cluster's

controllers/client_service.go

@@ -0,0 +1,165 @@
+package controllers
+
+import (
+	"context"
+	"fmt"
+
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+	corev1ac "k8s.io/client-go/applyconfigurations/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+
+	lll "github.com/cozystack/etcd-operator/api/v1alpha2"
+)
+
+// trafficDistributionFieldManager is the server-side-apply field manager
+// that owns spec.trafficDistribution on the client Service. Kept apart
+// from the operator's merge patches so the apiserver tracks exactly which
+// value the annotation put there.
+const trafficDistributionFieldManager = "etcd-operator-traffic-distribution"
+
+// Event reasons for AnnClientServiceTrafficDistribution problems. Each one
+// leaves the cluster's reconcile running and the Service field unset.
+const (
+	ReasonTrafficDistributionInvalid      = "InvalidTrafficDistribution"
+	ReasonTrafficDistributionRejected     = "TrafficDistributionRejected"
+	ReasonTrafficDistributionNotPersisted = "TrafficDistributionNotPersisted"
+)
+
+// knownTrafficDistributions are the values corev1.ServiceSpec.TrafficDistribution
+// defines. Whether the apiserver accepts one depends on its version, which
+// reconcileClientTrafficDistribution handles at write time.
+var knownTrafficDistributions = []string{
+	corev1.ServiceTrafficDistributionPreferClose,
+	corev1.ServiceTrafficDistributionPreferSameZone,
+	corev1.ServiceTrafficDistributionPreferSameNode,
+}
+
+func clientServiceName(cluster *lll.EtcdCluster) string {
+	return cluster.Name + "-client"
+}
+
+// desiredTrafficDistribution reads AnnClientServiceTrafficDistribution.
+// It returns "" when the annotation is absent, and "" with valid=false
+// when its value is not one of knownTrafficDistributions.
+func desiredTrafficDistribution(cluster *lll.EtcdCluster) (value string, valid bool) {
+	v, ok := cluster.Annotations[AnnClientServiceTrafficDistribution]
+	if !ok {
+		return "", true
+	}
+	for _, known := range knownTrafficDistributions {
+		if v == known {
+			return v, true
+		}
+	}
+	return "", false
+}
+
+// reconcileClientTrafficDistribution drives spec.trafficDistribution on the
+// client Service from AnnClientServiceTrafficDistribution, via server-side
+// apply under trafficDistributionFieldManager:
+//
+//   - annotation set: the value is applied (forcing ownership) whenever the
+//     Service carries a different one;
+//   - annotation absent or invalid: the field manager's ownership is
+//     released, which clears the field unless another manager also owns
+//     it. A value set by someone else is never touched.
+//
+// A value the apiserver rejects (e.g. PreferSameZone before Kubernetes
+// 1.34) or silently drops (no trafficDistribution support at all) is
+// reported as a Warning Event and does not fail the reconcile.
+func (r *EtcdClusterReconciler) reconcileClientTrafficDistribution(ctx context.Context, cluster *lll.EtcdCluster) error {
+	svc := &corev1.Service{}
+	if err := r.Get(ctx, types.NamespacedName{Namespace: cluster.Namespace, Name: clientServiceName(cluster)}, svc); err != nil {
+		// Just created and not in the cache yet: its watch event brings
+		// the cluster back here.
+		return client.IgnoreNotFound(err)
+	}
+
+	want, valid := desiredTrafficDistribution(cluster)
+	if !valid {
+		r.event(cluster, corev1.EventTypeWarning, ReasonTrafficDistributionInvalid, fmt.Sprintf(
+			"annotation %s=%q is not one of %v; leaving trafficDistribution unset on Service %s",
+			AnnClientServiceTrafficDistribution, cluster.Annotations[AnnClientServiceTrafficDistribution],
+			knownTrafficDistributions, svc.Name))
+	}
+
+	if want == "" {
+		owned, err := ownsTrafficDistribution(svc)
+		if err != nil || !owned {
+			return err
+		}
+		_, err = r.applyTrafficDistribution(ctx, svc, "")
+		return err
+	}
+
+	if svc.Spec.TrafficDistribution != nil && *svc.Spec.TrafficDistribution == want {
+		return nil
+	}
+	got, err := r.applyTrafficDistribution(ctx, svc, want)
+	switch {
+	case errors.IsInvalid(err) || errors.IsBadRequest(err):
+		r.event(cluster, corev1.EventTypeWarning, ReasonTrafficDistributionRejected, fmt.Sprintf(
+			"apiserver rejected trafficDistribution=%q on Service %s; leaving it unset: %v", want, svc.Name, err))
+		owned, err := ownsTrafficDistribution(svc)
+		if err != nil || !owned {
+			return err
+		}
+		_, err = r.applyTrafficDistribution(ctx, svc, "")
+		return err
+	case err != nil:
+		return err
+	case got != want:
+		r.event(cluster, corev1.EventTypeWarning, ReasonTrafficDistributionNotPersisted, fmt.Sprintf(
+			"apiserver dropped trafficDistribution=%q on Service %s; it does not serve the field (Kubernetes 1.31+ required)", want, svc.Name))
+	default:
+		log.FromContext(ctx).Info("set client Service trafficDistribution", "service", svc.Name, "trafficDistribution", want)
+	}
+	return nil
+}
+
+// applyTrafficDistribution server-side-applies spec.trafficDistribution=value
+// (or, for "", an empty configuration that releases the field) and returns
+// the value the apiserver persisted.
+func (r *EtcdClusterReconciler) applyTrafficDistribution(ctx context.Context, svc *corev1.Service, value string) (string, error) {
+	ac := corev1ac.Service(svc.Name, svc.Namespace)
+	if value != "" {
+		ac.WithSpec(corev1ac.ServiceSpec().WithTrafficDistribution(value))
+	}
+	raw, err := runtime.DefaultUnstructuredConverter.ToUnstructured(ac)
+	if err != nil {
+		return "", err
+	}
+	obj := &unstructured.Unstructured{Object: raw}
+	if err := r.Patch(ctx, obj, client.Apply, client.FieldOwner(trafficDistributionFieldManager), client.ForceOwnership); err != nil {
+		return "", err
+	}
+	got, _, err := unstructured.NestedString(obj.Object, "spec", "trafficDistribution")
+	return got, err
+}
+
+// ownsTrafficDistribution reports whether trafficDistributionFieldManager
+// holds spec.trafficDistribution on svc. The managedFields scan keeps the
+// common case (the annotation was never used) from parsing anything.
+func ownsTrafficDistribution(svc *corev1.Service) (bool, error) {
+	applied := false
+	for _, mf := range svc.ManagedFields {
+		if mf.Manager == trafficDistributionFieldManager && mf.Operation == metav1.ManagedFieldsOperationApply {
+			applied = true
+			break
+		}
+	}
+	if !applied {
+		return false, nil
+	}
+	ac, err := corev1ac.ExtractService(svc, trafficDistributionFieldManager)
+	if err != nil {
+		return false, err
+	}
+	return ac.Spec != nil && ac.Spec.TrafficDistribution != nil, nil
+}