diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 64ab8d85..da4b2a7d 100644 --- a/.github/CODEOWNERS +++ b/.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 diff --git a/README.md b/README.md index 9fa61424..2bf78daf 100644 --- a/README.md +++ b/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 `-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 `-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. diff --git a/api/v1alpha2/client_service_envtest_test.go b/api/v1alpha2/client_service_envtest_test.go deleted file mode 100644 index 4e322294..00000000 --- a/api/v1alpha2/client_service_envtest_test.go +++ /dev/null @@ -1,83 +0,0 @@ -/* -Copyright 2023 Timofey Larkin. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 -*/ - -package v1alpha2_test - -import ( - "context" - "strings" - "testing" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" - - lll "github.com/cozystack/etcd-operator/api/v1alpha2" -) - -// spec.clientService.trafficDistribution admits exactly the values the core -// Service API defines; anything else is rejected at admission rather than -// surfacing later as a failed Service patch inside the reconcile loop. -func TestClientService_TrafficDistributionEnum(t *testing.T) { - skipIfNoEnvtest(t) - ctx := context.Background() - - for _, v := range []string{"PreferClose", "PreferSameZone", "PreferSameNode"} { - c := validCluster("td-" + strings.ToLower(v)) - c.Spec.ClientService = &lll.ClientServiceSpec{TrafficDistribution: &v} - if err := k8s.Create(ctx, c); err != nil { - t.Fatalf("apiserver rejected trafficDistribution=%q: %v", v, err) - } - t.Cleanup(func() { _ = k8s.Delete(ctx, c) }) - } - - bogus := "Anywhere" - c := validCluster("td-bogus") - c.Spec.ClientService = &lll.ClientServiceSpec{TrafficDistribution: &bogus} - err := k8s.Create(ctx, c) - if err == nil { - _ = k8s.Delete(ctx, c) - t.Fatalf("apiserver accepted trafficDistribution=%q; expected rejection", bogus) - } - if !strings.Contains(err.Error(), "spec.clientService.trafficDistribution") { - t.Fatalf("error did not name the field: %v", err) - } -} - -// The envtest apiserver (pinned to the k8s.io/api minor in go.mod) must -// persist trafficDistribution on a Service. An apiserver with the field -// disabled drops it silently, which would leave the operator re-patching -// the client Service on every reconcile; this guards the version floor the -// field's docs promise. -func TestClientService_ApiserverPersistsTrafficDistribution(t *testing.T) { - skipIfNoEnvtest(t) - ctx := context.Background() - - td := corev1.ServiceTrafficDistributionPreferClose - svc := &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{Name: "td-probe", Namespace: "default"}, - Spec: corev1.ServiceSpec{ - Ports: []corev1.ServicePort{{Name: "client", Port: 2379}}, - TrafficDistribution: &td, - }, - } - if err := k8s.Create(ctx, svc); err != nil { - t.Fatalf("Create Service: %v", err) - } - t.Cleanup(func() { _ = k8s.Delete(ctx, svc) }) - - got := &corev1.Service{} - if err := k8s.Get(ctx, ctrlclient.ObjectKeyFromObject(svc), got); err != nil { - t.Fatalf("Get Service: %v", err) - } - if got.Spec.TrafficDistribution == nil || *got.Spec.TrafficDistribution != td { - t.Fatalf("apiserver did not persist trafficDistribution: %v", got.Spec.TrafficDistribution) - } -} diff --git a/api/v1alpha2/etcdcluster_types.go b/api/v1alpha2/etcdcluster_types.go index 2e88106f..8bdd5253 100644 --- a/api/v1alpha2/etcdcluster_types.go +++ b/api/v1alpha2/etcdcluster_types.go @@ -569,41 +569,6 @@ type EtcdClusterSpec struct { // status.observed. // +optional ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty"` - - // ClientService configures the "-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 "-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 diff --git a/api/v1alpha2/zz_generated.deepcopy.go b/api/v1alpha2/zz_generated.deepcopy.go index b4a7f23f..7f59359e 100644 --- a/api/v1alpha2/zz_generated.deepcopy.go +++ b/api/v1alpha2/zz_generated.deepcopy.go @@ -116,26 +116,6 @@ func (in *ClientCertManagerTLS) DeepCopy() *ClientCertManagerTLS { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ClientServiceSpec) DeepCopyInto(out *ClientServiceSpec) { - *out = *in - if in.TrafficDistribution != nil { - in, out := &in.TrafficDistribution, &out.TrafficDistribution - *out = new(string) - **out = **in - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClientServiceSpec. -func (in *ClientServiceSpec) DeepCopy() *ClientServiceSpec { - if in == nil { - return nil - } - out := new(ClientServiceSpec) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClientTLS) DeepCopyInto(out *ClientTLS) { *out = *in @@ -322,11 +302,6 @@ func (in *EtcdClusterSpec) DeepCopyInto(out *EtcdClusterSpec) { *out = make([]v1.LocalObjectReference, len(*in)) copy(*out, *in) } - if in.ClientService != nil { - in, out := &in.ClientService, &out.ClientService - *out = new(ClientServiceSpec) - (*in).DeepCopyInto(*out) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EtcdClusterSpec. diff --git a/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yaml b/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yaml index 8663fa1a..ed4d78ad 100644 --- a/charts/etcd-operator/crd-bases/etcd-operator.cozystack.io_etcdclusters.yaml +++ b/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 "-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 diff --git a/controllers/client_service.go b/controllers/client_service.go new file mode 100644 index 00000000..32c40e19 --- /dev/null +++ b/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 +} diff --git a/controllers/client_service_envtest_test.go b/controllers/client_service_envtest_test.go new file mode 100644 index 00000000..f79cdcb7 --- /dev/null +++ b/controllers/client_service_envtest_test.go @@ -0,0 +1,145 @@ +/* +Copyright 2023 Timofey Larkin. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 +*/ + +package controllers + +import ( + "context" + "os" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + + lll "github.com/cozystack/etcd-operator/api/v1alpha2" +) + +// The ownership semantics of AnnClientServiceTrafficDistribution live in +// the apiserver's server-side apply, which the fake client doesn't +// implement, so these run against envtest. `make test` sets +// KUBEBUILDER_ASSETS; without it the test skips. +func TestClientServiceTrafficDistribution_Envtest(t *testing.T) { + if os.Getenv("KUBEBUILDER_ASSETS") == "" { + t.Skip("KUBEBUILDER_ASSETS not set; envtest harness not initialized") + } + env := &envtest.Environment{Scheme: testScheme(t)} + cfg, err := env.Start() + if err != nil { + t.Fatalf("envtest start: %v", err) + } + t.Cleanup(func() { _ = env.Stop() }) + k8s, err := client.New(cfg, client.Options{Scheme: testScheme(t)}) + if err != nil { + t.Fatalf("client: %v", err) + } + ctx := context.Background() + + // Each subtest gets its own namespace and an in-memory cluster: the + // Services only need its name, UID and annotations. + setup := func(t *testing.T, ns string) (*lll.EtcdCluster, *EtcdClusterReconciler, *record.FakeRecorder) { + t.Helper() + if err := k8s.Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}}); err != nil { + t.Fatalf("create namespace: %v", err) + } + cluster := &lll.EtcdCluster{ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: ns, UID: types.UID(ns + "-uid")}} + rec := record.NewFakeRecorder(20) + return cluster, &EtcdClusterReconciler{Client: k8s, Scheme: testScheme(t), Recorder: rec}, rec + } + reconcile := func(t *testing.T, r *EtcdClusterReconciler, cluster *lll.EtcdCluster, ann *string) { + t.Helper() + if ann == nil { + delete(cluster.Annotations, AnnClientServiceTrafficDistribution) + } else { + cluster.Annotations = map[string]string{AnnClientServiceTrafficDistribution: *ann} + } + if err := r.ensureServices(ctx, cluster); err != nil { + t.Fatalf("ensureServices: %v", err) + } + } + // handSet writes trafficDistribution the way `kubectl edit` would: an + // update-style patch under a different field manager. + handSet := func(t *testing.T, ns string, v *string) { + t.Helper() + svc := &corev1.Service{} + if err := k8s.Get(ctx, types.NamespacedName{Namespace: ns, Name: "test-client"}, svc); err != nil { + t.Fatal(err) + } + orig := svc.DeepCopy() + svc.Spec.TrafficDistribution = v + if err := k8s.Patch(ctx, svc, client.MergeFrom(orig), client.FieldOwner("kubectl-edit")); err != nil { + t.Fatalf("hand-set trafficDistribution: %v", err) + } + } + preferClose := strPtr(corev1.ServiceTrafficDistributionPreferClose) + + t.Run("set, then remove clears", func(t *testing.T) { + cluster, r, _ := setup(t, "td-set-remove") + reconcile(t, r, cluster, preferClose) + assertTrafficDistribution(t, k8s, cluster.Namespace, "test-client", preferClose) + assertTrafficDistribution(t, k8s, cluster.Namespace, "test", nil) + + reconcile(t, r, cluster, nil) + assertTrafficDistribution(t, k8s, cluster.Namespace, "test-client", nil) + }) + + t.Run("drift is reconciled back", func(t *testing.T) { + cluster, r, _ := setup(t, "td-drift") + reconcile(t, r, cluster, preferClose) + handSet(t, cluster.Namespace, nil) + reconcile(t, r, cluster, preferClose) + assertTrafficDistribution(t, k8s, cluster.Namespace, "test-client", preferClose) + + // The hand edit took ownership away; the re-apply took it back, + // so removing the annotation still clears the field. + reconcile(t, r, cluster, nil) + assertTrafficDistribution(t, k8s, cluster.Namespace, "test-client", nil) + }) + + t.Run("hand-set value without annotation is left alone", func(t *testing.T) { + cluster, r, _ := setup(t, "td-hand-set") + reconcile(t, r, cluster, nil) + handSet(t, cluster.Namespace, preferClose) + reconcile(t, r, cluster, nil) + assertTrafficDistribution(t, k8s, cluster.Namespace, "test-client", preferClose) + }) + + t.Run("invalid value clears and warns", func(t *testing.T) { + cluster, r, rec := setup(t, "td-invalid") + reconcile(t, r, cluster, preferClose) + reconcile(t, r, cluster, strPtr("Anywhere")) + assertTrafficDistribution(t, k8s, cluster.Namespace, "test-client", nil) + requireEvent(t, drainEvents(rec), ReasonTrafficDistributionInvalid) + }) + + // PreferSameZone is served from Kubernetes 1.34 (beta, on by default). + // The envtest apiserver follows the k8s.io/api minor in go.mod, so this + // checks whichever outcome that version produces: updated in place, or + // rejected with an Event and the field cleared. + t.Run("update to PreferSameZone", func(t *testing.T) { + cluster, r, rec := setup(t, "td-update") + reconcile(t, r, cluster, preferClose) + reconcile(t, r, cluster, strPtr(corev1.ServiceTrafficDistributionPreferSameZone)) + + svc := &corev1.Service{} + if err := k8s.Get(ctx, types.NamespacedName{Namespace: cluster.Namespace, Name: "test-client"}, svc); err != nil { + t.Fatal(err) + } + if svc.Spec.TrafficDistribution == nil { + requireEvent(t, drainEvents(rec), ReasonTrafficDistributionRejected) + t.Logf("apiserver rejected PreferSameZone; field cleared as documented") + return + } + assertTrafficDistribution(t, k8s, cluster.Namespace, "test-client", strPtr(corev1.ServiceTrafficDistributionPreferSameZone)) + }) +} diff --git a/controllers/client_service_test.go b/controllers/client_service_test.go index 1e0f0b07..fafed8b0 100644 --- a/controllers/client_service_test.go +++ b/controllers/client_service_test.go @@ -12,61 +12,45 @@ package controllers import ( "context" + "strings" "testing" corev1 "k8s.io/api/core/v1" + apierrors "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/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" lll "github.com/cozystack/etcd-operator/api/v1alpha2" ) -// clusterWithTrafficDistribution returns a cluster whose client Service asks -// for td; a nil td leaves spec.clientService unset. -func clusterWithTrafficDistribution(td *string) *lll.EtcdCluster { +// clusterWithTrafficDistributionAnnotation returns a cluster carrying the +// annotation with value v; a nil v leaves it off. +func clusterWithTrafficDistributionAnnotation(v *string) *lll.EtcdCluster { cluster := &lll.EtcdCluster{ - ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns", UID: "test-uid"}, } - if td != nil { - cluster.Spec.ClientService = &lll.ClientServiceSpec{TrafficDistribution: td} + if v != nil { + cluster.Annotations = map[string]string{AnnClientServiceTrafficDistribution: *v} } return cluster } -// existingService returns a Service as it would sit on the apiserver before -// the operator's reconcile, carrying td on spec.trafficDistribution. -func existingService(name string, td *string) *corev1.Service { - svc := &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "ns"}, - Spec: corev1.ServiceSpec{ - Selector: map[string]string{LabelCluster: "test"}, - Ports: []corev1.ServicePort{{Name: "client", Port: 2379}}, - TrafficDistribution: td, - }, - } - if name == "test" { - svc.Spec.ClusterIP = corev1.ClusterIPNone - svc.Spec.PublishNotReadyAddresses = true - svc.Spec.Ports = append(svc.Spec.Ports, corev1.ServicePort{Name: "peer", Port: 2380}) - } - return svc -} - func strPtr(s string) *string { return &s } -func getTrafficDistribution(t *testing.T, c client.Client, name string) *string { +func assertTrafficDistribution(t *testing.T, c client.Client, ns, name string, want *string) { t.Helper() svc := &corev1.Service{} - if err := c.Get(context.Background(), types.NamespacedName{Namespace: "ns", Name: name}, svc); err != nil { + if err := c.Get(context.Background(), types.NamespacedName{Namespace: ns, Name: name}, svc); err != nil { t.Fatalf("Get Service %s: %v", name, err) } - return svc.Spec.TrafficDistribution -} - -func assertTrafficDistribution(t *testing.T, c client.Client, name string, want *string) { - t.Helper() - got := getTrafficDistribution(t, c, name) + got := svc.Spec.TrafficDistribution switch { case want == nil && got != nil: t.Fatalf("Service %s trafficDistribution = %q, want unset", name, *got) @@ -77,79 +61,185 @@ func assertTrafficDistribution(t *testing.T, c client.Client, name string, want } } -// On create the client Service carries the requested value; the headless -// Service never does. -func TestEnsureServices_TrafficDistributionOnCreate(t *testing.T) { - cluster := clusterWithTrafficDistribution(strPtr(corev1.ServiceTrafficDistributionPreferClose)) - c, _ := newTestClient(t, cluster) - r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t)} +// drainEvents returns every event the FakeRecorder has buffered. +func drainEvents(rec *record.FakeRecorder) []string { + var out []string + for { + select { + case e := <-rec.Events: + out = append(out, e) + default: + return out + } + } +} - if err := r.ensureServices(context.Background(), cluster); err != nil { - t.Fatalf("ensureServices: %v", err) +func requireEvent(t *testing.T, events []string, reason string) { + t.Helper() + for _, e := range events { + if strings.HasPrefix(e, corev1.EventTypeWarning+" "+reason+" ") { + return + } } - assertTrafficDistribution(t, c, "test-client", strPtr(corev1.ServiceTrafficDistributionPreferClose)) - assertTrafficDistribution(t, c, "test", nil) + t.Fatalf("no Warning %s event among %q", reason, events) } -// A client Service that drifted (value removed or changed by hand) is -// brought back to the spec. -func TestEnsureServices_TrafficDistributionDriftReconciled(t *testing.T) { - for name, onService := range map[string]*string{ - "unset": nil, - "different": strPtr(corev1.ServiceTrafficDistributionPreferSameNode), - } { - t.Run(name, func(t *testing.T) { - cluster := clusterWithTrafficDistribution(strPtr(corev1.ServiceTrafficDistributionPreferClose)) - c, _ := newTestClient(t, cluster, existingService("test", nil), existingService("test-client", onService)) - r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t)} +// applyInterceptingClient is a fake client whose server-side-apply patches +// (unsupported by the fake) go to onApply instead; applies counts them. +func applyInterceptingClient(t *testing.T, applies *int, onApply func(obj client.Object) error, objs ...client.Object) client.Client { + t.Helper() + return fake.NewClientBuilder(). + WithScheme(testScheme(t)). + WithObjects(objs...). + WithInterceptorFuncs(interceptor.Funcs{ + Patch: func(ctx context.Context, c client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + if patch.Type() == types.ApplyPatchType { + *applies++ + return onApply(obj) + } + return c.Patch(ctx, obj, patch, opts...) + }, + }). + Build() +} - if err := r.ensureServices(context.Background(), cluster); err != nil { - t.Fatalf("ensureServices: %v", err) +func TestDesiredTrafficDistribution(t *testing.T) { + for _, tc := range []struct { + name string + ann *string + wantValue string + wantValid bool + }{ + {"absent", nil, "", true}, + {"PreferClose", strPtr("PreferClose"), "PreferClose", true}, + {"PreferSameZone", strPtr("PreferSameZone"), "PreferSameZone", true}, + {"PreferSameNode", strPtr("PreferSameNode"), "PreferSameNode", true}, + {"empty", strPtr(""), "", false}, + {"wrong case", strPtr("preferclose"), "", false}, + {"unknown", strPtr("Anywhere"), "", false}, + } { + t.Run(tc.name, func(t *testing.T) { + v, ok := desiredTrafficDistribution(clusterWithTrafficDistributionAnnotation(tc.ann)) + if v != tc.wantValue || ok != tc.wantValid { + t.Fatalf("got (%q, %v), want (%q, %v)", v, ok, tc.wantValue, tc.wantValid) } - assertTrafficDistribution(t, c, "test-client", strPtr(corev1.ServiceTrafficDistributionPreferClose)) - assertTrafficDistribution(t, c, "test", nil) }) } } -// With the field unset — no clientService block, or a block without -// trafficDistribution — the operator does not own it: a value patched onto -// the Service by hand survives reconcile, and no value is added. -func TestEnsureServices_TrafficDistributionUnsetLeavesServiceAlone(t *testing.T) { - for name, cluster := range map[string]*lll.EtcdCluster{ - "no clientService": clusterWithTrafficDistribution(nil), - "empty clientService": func() *lll.EtcdCluster { - cl := clusterWithTrafficDistribution(nil) - cl.Spec.ClientService = &lll.ClientServiceSpec{} - return cl - }(), - } { - t.Run(name, func(t *testing.T) { - manual := strPtr(corev1.ServiceTrafficDistributionPreferClose) - c, _ := newTestClient(t, cluster, existingService("test", nil), existingService("test-client", manual)) - r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t)} +// Without the annotation and without prior ownership, the operator never +// issues an apply: clusters that don't use the annotation see no extra +// writes, and a value someone set by hand stays. +func TestEnsureServices_NoAnnotationNoApply(t *testing.T) { + cluster := clusterWithTrafficDistributionAnnotation(nil) + applies := 0 + c := applyInterceptingClient(t, &applies, func(client.Object) error { + t.Fatal("unexpected apply") + return nil + }) + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t)} - if err := r.ensureServices(context.Background(), cluster); err != nil { - t.Fatalf("ensureServices: %v", err) - } - assertTrafficDistribution(t, c, "test-client", manual) - assertTrafficDistribution(t, c, "test", nil) + if err := r.ensureServices(context.Background(), cluster); err != nil { + t.Fatalf("ensureServices (create): %v", err) + } + // Hand-set value on the existing client Service. + svc := &corev1.Service{} + if err := c.Get(context.Background(), types.NamespacedName{Namespace: "ns", Name: "test-client"}, svc); err != nil { + t.Fatal(err) + } + svc.Spec.TrafficDistribution = strPtr(corev1.ServiceTrafficDistributionPreferClose) + if err := c.Update(context.Background(), svc); err != nil { + t.Fatal(err) + } + if err := r.ensureServices(context.Background(), cluster); err != nil { + t.Fatalf("ensureServices (existing): %v", err) + } + if applies != 0 { + t.Fatalf("applies = %d, want 0", applies) + } + assertTrafficDistribution(t, c, "ns", "test-client", strPtr(corev1.ServiceTrafficDistributionPreferClose)) +} + +// An annotation value Kubernetes doesn't define is reported and otherwise +// ignored: the reconcile carries on and nothing is applied. +func TestEnsureServices_InvalidAnnotation(t *testing.T) { + cluster := clusterWithTrafficDistributionAnnotation(strPtr("Anywhere")) + applies := 0 + c := applyInterceptingClient(t, &applies, func(client.Object) error { return nil }) + rec := record.NewFakeRecorder(10) + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t), Recorder: rec} + + for range 2 { // create, then reconcile the existing Services + if err := r.ensureServices(context.Background(), cluster); err != nil { + t.Fatalf("ensureServices: %v", err) + } + } + if applies != 0 { + t.Fatalf("applies = %d, want 0", applies) + } + requireEvent(t, drainEvents(rec), ReasonTrafficDistributionInvalid) + assertTrafficDistribution(t, c, "ns", "test-client", nil) + assertTrafficDistribution(t, c, "ns", "test", nil) +} + +// A value the apiserver refuses (PreferSameZone before Kubernetes 1.34) is +// reported as an Event; ensureServices still succeeds so bootstrap, scaling +// and status updates for the cluster are not blocked. +func TestEnsureServices_ApiserverRejectsValue(t *testing.T) { + cluster := clusterWithTrafficDistributionAnnotation(strPtr(corev1.ServiceTrafficDistributionPreferSameZone)) + applies := 0 + c := applyInterceptingClient(t, &applies, func(obj client.Object) error { + return apierrors.NewInvalid(schema.GroupKind{Kind: "Service"}, obj.GetName(), field.ErrorList{ + field.NotSupported(field.NewPath("spec", "trafficDistribution"), "PreferSameZone", []string{"PreferClose"}), }) + }) + rec := record.NewFakeRecorder(10) + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t), Recorder: rec} + + for range 2 { + if err := r.ensureServices(context.Background(), cluster); err != nil { + t.Fatalf("ensureServices: %v", err) + } + } + if applies == 0 { + t.Fatal("expected an apply attempt") } + requireEvent(t, drainEvents(rec), ReasonTrafficDistributionRejected) + assertTrafficDistribution(t, c, "ns", "test-client", nil) } -// The headless Service is not the operator's to set traffic distribution on: -// a value someone put there is neither cleared nor overwritten with the -// client Service's value. -func TestEnsureServices_HeadlessTrafficDistributionUntouched(t *testing.T) { - cluster := clusterWithTrafficDistribution(strPtr(corev1.ServiceTrafficDistributionPreferClose)) - manual := strPtr(corev1.ServiceTrafficDistributionPreferSameNode) - c, _ := newTestClient(t, cluster, existingService("test", manual), existingService("test-client", nil)) +// An apiserver that doesn't serve the field accepts the write and drops the +// value; the operator reports that instead of claiming success. +func TestEnsureServices_ApiserverDropsValue(t *testing.T) { + cluster := clusterWithTrafficDistributionAnnotation(strPtr(corev1.ServiceTrafficDistributionPreferClose)) + applies := 0 + // The "response" is the applied object minus the field. + c := applyInterceptingClient(t, &applies, func(obj client.Object) error { + unstructured.RemoveNestedField(obj.(*unstructured.Unstructured).Object, "spec", "trafficDistribution") + return nil + }) + rec := record.NewFakeRecorder(10) + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t), Recorder: rec} + + for range 2 { + if err := r.ensureServices(context.Background(), cluster); err != nil { + t.Fatalf("ensureServices: %v", err) + } + } + requireEvent(t, drainEvents(rec), ReasonTrafficDistributionNotPersisted) +} + +// Errors that aren't about the value (apiserver unavailable, throttling) +// fail the reconcile so it retries, like every other Service write. +func TestEnsureServices_TransientApplyErrorRetries(t *testing.T) { + cluster := clusterWithTrafficDistributionAnnotation(strPtr(corev1.ServiceTrafficDistributionPreferClose)) + applies := 0 + c := applyInterceptingClient(t, &applies, func(client.Object) error { + return apierrors.NewServiceUnavailable("try later") + }) r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t)} - if err := r.ensureServices(context.Background(), cluster); err != nil { - t.Fatalf("ensureServices: %v", err) + if err := r.ensureServices(context.Background(), cluster); !apierrors.IsServiceUnavailable(err) { + t.Fatalf("ensureServices err = %v, want ServiceUnavailable", err) } - assertTrafficDistribution(t, c, "test", manual) - assertTrafficDistribution(t, c, "test-client", strPtr(corev1.ServiceTrafficDistributionPreferClose)) } diff --git a/controllers/etcdcluster_controller.go b/controllers/etcdcluster_controller.go index 49d8230b..49464ae3 100644 --- a/controllers/etcdcluster_controller.go +++ b/controllers/etcdcluster_controller.go @@ -34,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -56,6 +57,9 @@ type EtcdClusterReconciler struct { // production wiring uses DefaultEtcdClientFactory. EtcdClientFactory EtcdClientFactory + // Recorder emits events on the EtcdCluster. Tests may leave it nil. + Recorder record.EventRecorder + // CertManagerAvailable reports whether the cert-manager.io/v1 API // is registered on this cluster. Detected once at operator startup // via the discovery API. When false, clusters using @@ -84,6 +88,7 @@ type EtcdClusterReconciler struct { //+kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch;create;update;patch //+kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch +//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch // `delete` is intentionally omitted from the certificates verb list: // each Certificate is owned by the EtcdCluster via SetControllerReference, // and spec.tls is CEL-immutable post-create, so the only Certificate @@ -1917,25 +1922,22 @@ func (r *EtcdClusterReconciler) ensureServices(ctx context.Context, cluster *lll } // Client service — stable endpoint for applications. - clientSpec := corev1.ServiceSpec{ + if err := r.ensureService(ctx, cluster, clientServiceName(cluster), corev1.ServiceSpec{ Selector: map[string]string{LabelCluster: cluster.Name}, Ports: []corev1.ServicePort{ {Name: "client", Port: 2379}, }, + }); err != nil { + return err } - if cs := cluster.Spec.ClientService; cs != nil { - clientSpec.TrafficDistribution = cs.TrafficDistribution - } - return r.ensureService(ctx, cluster, cluster.Name+"-client", clientSpec) + return r.reconcileClientTrafficDistribution(ctx, cluster) } // ensureService creates the Service if absent and reconciles drift on the // fields the operator actually owns: Selector, PublishNotReadyAddresses, -// the named Ports it requires, and TrafficDistribution when the desired -// spec sets it. Everything else (labels, annotations, user-added ports, -// type, a hand-set TrafficDistribution the spec doesn't ask for) is -// preserved — admission webhooks routinely inject these and we don't want -// to fight them. +// and the named Ports it requires. Everything else (labels, annotations, +// user-added ports, type) is preserved — admission webhooks routinely +// inject these and we don't want to fight them. func (r *EtcdClusterReconciler) ensureService( ctx context.Context, cluster *lll.EtcdCluster, @@ -1976,14 +1978,6 @@ func (r *EtcdClusterReconciler) ensureService( svc.Spec.PublishNotReadyAddresses = desired.PublishNotReadyAddresses changed = true } - // TrafficDistribution is owned only when asked for: nil leaves whatever - // is on the Service, including a value set by hand. - if desired.TrafficDistribution != nil && - (svc.Spec.TrafficDistribution == nil || *svc.Spec.TrafficDistribution != *desired.TrafficDistribution) { - td := *desired.TrafficDistribution - svc.Spec.TrafficDistribution = &td - changed = true - } // Required ports must be present with correct values. Extra ports the // user has added stay. for _, want := range desired.Ports { @@ -2027,6 +2021,12 @@ func upsertPortByName(ports *[]corev1.ServicePort, want corev1.ServicePort) bool return true } +func (r *EtcdClusterReconciler) event(obj client.Object, eventType, reason, msg string) { + if r.Recorder != nil { + r.Recorder.Event(obj, eventType, reason, msg) + } +} + // ── Manager wiring ─────────────────────────────────────────────────────── func (r *EtcdClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { diff --git a/controllers/helpers.go b/controllers/helpers.go index 1202f78f..2792ad17 100644 --- a/controllers/helpers.go +++ b/controllers/helpers.go @@ -82,6 +82,12 @@ const ( // footgun. Superseded by an explicit spec.tls.peer.secretRef/certManager // (real mTLS wins; precedence lives in clusterPeerAutoTLS). AnnPeerAutoTLS = ReservedAnnotationPrefix + "peer-auto-tls" + + // AnnClientServiceTrafficDistribution, set on an EtcdCluster, is copied + // to spec.trafficDistribution of the "-client" Service. A + // stopgap until the setting gets a typed spec field; see + // reconcileClientTrafficDistribution. + AnnClientServiceTrafficDistribution = ReservedAnnotationPrefix + "client-service-traffic-distribution" ) // etcdDataDirRoot is the mount path of every member's data volume; --data-dir diff --git a/docs/concepts.md b/docs/concepts.md index b0e0103a..54eefabc 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -281,26 +281,57 @@ Merge semantics: - **Apply-on-create.** Objects are stamped when created; editing the field re-stamps nothing retroactively. Newly-created objects (scale-up members and their PVCs, replacements) pick up the latest latched value. - **Latched.** A mid-flight edit only takes effect once the current `status.observed` target is reached, like every other latched field. -## Client Service +## Client Service traffic distribution -The operator creates two Services per cluster: the headless `` (per-Pod DNS for peers, publishes not-ready addresses) and `-client` (a ClusterIP over ready members, the stable endpoint for applications). On both it reconciles drift only on the fields it owns — the selector, `publishNotReadyAddresses`, and its named ports — and leaves everything else (labels, annotations, extra ports, type) to whoever set it. - -`spec.clientService` configures `-client` further. Today it carries one field: +The operator creates two Services per cluster: the headless `` (per-Pod DNS for peers, publishes not-ready addresses) and `-client` (a ClusterIP over ready members, the stable endpoint for applications). In a cluster stretched across zones, clients of `-client` can be kept on a member in their own zone by setting the Service's [`spec.trafficDistribution`](https://kubernetes.io/docs/concepts/services-networking/service/#traffic-distribution). The operator manages that field from an annotation on the `EtcdCluster`: ```yaml +apiVersion: etcd-operator.cozystack.io/v1alpha2 +kind: EtcdCluster +metadata: + name: my-etcd + annotations: + etcd-operator.cozystack.io/client-service-traffic-distribution: PreferClose spec: - clientService: - trafficDistribution: PreferClose # PreferClose | PreferSameZone | PreferSameNode + replicas: 3 + topologySpreadConstraints: # one member per zone + - maxSkew: 1 + topologyKey: topology.kubernetes.io/zone + whenUnsatisfiable: DoNotSchedule + labelSelector: + matchLabels: + etcd-operator.cozystack.io/cluster: my-etcd ``` -`trafficDistribution` is copied to the Service's `spec.trafficDistribution`. The main use is a cluster stretched across zones (for example two sites plus a witness): with `PreferClose`, the EndpointSlice controller writes zone hints on the client Service's endpoints and a proxy that honours them (kube-proxy, Cilium) sends each client to a ready member in its own zone, falling back to the others when its zone has none. Clients stop paying cross-zone latency on every request, and a site keeps serving reads locally. +or, on an existing cluster, `kubectl annotate etcdcluster my-etcd etcd-operator.cozystack.io/client-service-traffic-distribution=PreferClose`. + +The annotation is a stopgap until the setting gets a typed field in the `EtcdCluster` spec. When that field lands it will take precedence, and the annotation will be deprecated. + +Accepted values, and the Kubernetes version whose apiserver serves each: + +| Value | Routes to | Kubernetes | +|---|---|---| +| `PreferClose` | a ready member in the client's zone | 1.31+ (beta, on by default; GA in 1.33). From 1.34 an alias of `PreferSameZone`. | +| `PreferSameZone` | a ready member in the client's zone | 1.34+ (beta, on by default) | +| `PreferSameNode` | a ready member on the client's node | 1.34+ (beta, on by default) | + +In every case the proxy falls back to all ready members when none matches. Semantics: -- **Owned only when set.** With the field set, the operator writes it on create and reconciles any drift back to it. With it unset (no `clientService`, or no `trafficDistribution` in it), the operator does not touch the field — a value patched onto the Service by hand survives. -- **Client Service only.** The headless Service is never given a traffic distribution. -- **Not latched.** The Service carries no consensus state, so unlike the member-level fields a change is applied on the next reconcile rather than waiting for the current `status.observed` target. -- **Apiserver support.** `PreferClose` needs Kubernetes 1.31+ (beta, on by default; GA in 1.33). `PreferSameZone` and `PreferSameNode` need 1.34+, where the `PreferSameTrafficDistribution` feature gate is beta and on by default. An apiserver that does not know the field drops it silently (the setting has no effect); one that knows the field but not the value rejects the Service write, which surfaces as a reconcile error for that cluster. +- **Set or changed:** applied to `-client` on the next reconcile. It is not latched through `status.observed`: the Service carries no consensus state. +- **Removed:** the field is cleared from the Service. +- **Owned via server-side apply.** The operator writes only this one field, with field manager `etcd-operator-traffic-distribution`, and reconciles drift back while the annotation is set. Removing the annotation releases the field: it is cleared unless another field manager also set it. A value set on the Service by someone else while there is no annotation is left alone. +- **Invalid values never block the cluster.** A value outside the table (typos, wrong case, empty) leaves the field unset and emits a `Warning` Event `InvalidTrafficDistribution` on the `EtcdCluster`. A value the apiserver rejects (`PreferSameZone` / `PreferSameNode` before 1.34) emits `TrafficDistributionRejected` and also leaves the field unset. An apiserver that doesn't serve the field at all (before 1.31) drops it; the operator notices and emits `TrafficDistributionNotPersisted`. In all three cases the rest of the reconcile carries on. Check with `kubectl describe etcdcluster my-etcd`. +- **Client Service only.** The headless Service is never given a traffic distribution: it has no ClusterIP, so no proxy routes through it. + +Prerequisites for zone-local routing (`PreferClose` / `PreferSameZone`): + +- Every node that runs etcd members **or their clients** carries the [`topology.kubernetes.io/zone`](https://kubernetes.io/docs/reference/labels-annotations-taints/#topologykubernetesiozone) label. Cloud providers set it; on bare metal, label nodes yourself (`kubectl label node topology.kubernetes.io/zone=`). The EndpointSlice controller derives each endpoint's zone hint from its node's label, and kube-proxy ignores hints altogether (routing cluster-wide) when an endpoint has none or the client's node has no zone. +- Members are spread across the zones, e.g. with the `topologySpreadConstraints` above. A zone without a ready member sends its clients cluster-wide. +- The Service proxy honours EndpointSlice hints: kube-proxy does; for a kube-proxy replacement such as Cilium, check that its service-topology support is enabled. + +`PreferSameNode` needs no labels; it only helps when clients run on the same nodes as members. ## etcd tuning options diff --git a/main.go b/main.go index 6f82c523..0051bdfe 100644 --- a/main.go +++ b/main.go @@ -229,6 +229,7 @@ func main() { Scheme: mgr.GetScheme(), CertManagerAvailable: certManagerAvailable, ClusterDomain: clusterDomain, + Recorder: mgr.GetEventRecorderFor("etcd-operator"), }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "EtcdCluster") os.Exit(1)