From cb0c067aa8cf58f3963d6c756356890828447b4f Mon Sep 17 00:00:00 2001 From: Xavier Lange Date: Thu, 18 Jun 2026 19:15:01 -0400 Subject: [PATCH 1/8] feat: CR-configurable per-cluster domain metrics on /metrics Add an internal/metrics package that defines and registers custom Prometheus collectors for every reconciled EtcdCluster against the controller-runtime global registry, so they are served on the operator's existing /metrics endpoint. All series are labelled with the cluster namespace and name. Metrics: member_count_desired, member_count, member_count_ready, member_count_healthy, learner_count, has_quorum, has_leader, tls_enabled, leader_changes_total, reconcile_duration_seconds, reconcile_errors_total. Values are derived from the freshly-computed EtcdClusterStatus at the end of updateStatus (no extra API calls); reconcile duration/errors are observed in Reconcile; leader changes are tracked per cluster. Series are deleted when a cluster is removed to bound cardinality. Add spec.metrics to EtcdCluster: spec.metrics.enabled (default true) toggles/scopes the domain metrics per cluster, and spec.metrics.podMonitor makes the operator reconcile a prometheus-operator PodMonitor (owned by the cluster) for the etcd member pods. The PodMonitor is addressed via unstructured so the operator takes no hard dependency on the prometheus-operator module and degrades gracefully when the CRD is absent. Unit tests register to an isolated registry, drive state, scrape with prometheus testutil, and assert values/labels. A compile-verified e2e test creates a cluster with spec.metrics.podMonitor and scrapes /metrics. Refs #381 Co-Authored-By: Claude Opus 4.8 Signed-off-by: Xavier Lange --- api/v1alpha1/etcdcluster_types.go | 59 ++++ api/v1alpha1/zz_generated.deepcopy.go | 47 ++- cmd/main.go | 5 + .../bases/operator.etcd.io_etcdclusters.yaml | 42 ++- config/rbac/role.yaml | 12 + config/samples/kustomization.yaml | 1 + .../samples/sample_metrics_etcdcluster.yaml | 27 ++ go.mod | 3 +- internal/controller/etcdcluster_controller.go | 23 +- internal/controller/podmonitor.go | 128 +++++++ internal/metrics/metrics.go | 284 +++++++++++++++ internal/metrics/metrics_test.go | 329 ++++++++++++++++++ test/e2e/metrics_test.go | 185 ++++++++++ 13 files changed, 1141 insertions(+), 4 deletions(-) create mode 100644 config/samples/sample_metrics_etcdcluster.yaml create mode 100644 internal/controller/podmonitor.go create mode 100644 internal/metrics/metrics.go create mode 100644 internal/metrics/metrics_test.go create mode 100644 test/e2e/metrics_test.go diff --git a/api/v1alpha1/etcdcluster_types.go b/api/v1alpha1/etcdcluster_types.go index f007ae04..b2fdb0d4 100644 --- a/api/v1alpha1/etcdcluster_types.go +++ b/api/v1alpha1/etcdcluster_types.go @@ -49,6 +49,65 @@ type EtcdClusterSpec struct { EtcdOptions []string `json:"etcdOptions,omitempty"` // PodTemplate is the pod template to use for the etcd cluster. PodTemplate *PodTemplate `json:"podTemplate,omitempty"` + // Metrics configures Prometheus observability for this cluster. When unset, + // the operator still exports its own per-cluster domain metrics on its + // /metrics endpoint, but does not create a PodMonitor for the etcd member + // pods. See MetricsSpec for the available knobs. + // +optional + Metrics *MetricsSpec `json:"metrics,omitempty"` +} + +// MetricsSpec configures Prometheus observability for an EtcdCluster. +type MetricsSpec struct { + // Enabled toggles emission of the operator's per-cluster domain metrics + // (member counts, quorum, leader changes, reconcile timings, etc.) for this + // cluster. When nil it defaults to true: metrics are exported unless + // explicitly disabled. Disabling drops the cluster's series from the + // operator's /metrics endpoint. + // +optional + Enabled *bool `json:"enabled,omitempty"` + + // PodMonitor, when set and enabled, instructs the operator to create and + // maintain a prometheus-operator PodMonitor selecting this cluster's etcd + // member pods so that Prometheus scrapes etcd's own /metrics endpoint. + // This requires the prometheus-operator PodMonitor CRD to be installed in + // the cluster; if it is absent the operator logs and skips PodMonitor + // reconciliation without failing the rest of the reconcile. + // +optional + PodMonitor *PodMonitorSpec `json:"podMonitor,omitempty"` +} + +// PodMonitorSpec controls creation of a PodMonitor for the etcd member pods. +type PodMonitorSpec struct { + // Enabled toggles creation of the PodMonitor. Defaults to false. + // +optional + Enabled bool `json:"enabled,omitempty"` + + // Interval at which Prometheus scrapes the etcd member pods, e.g. "30s". + // When empty the prometheus-operator default is used. + // +optional + Interval string `json:"interval,omitempty"` + + // Port is the name of the pod port exposing etcd's /metrics endpoint. + // Defaults to "client" when empty. + // +optional + Port string `json:"port,omitempty"` +} + +// MetricsEnabled reports whether per-cluster domain metrics should be exported +// for this cluster. Metrics are on by default and only disabled when +// spec.metrics.enabled is explicitly set to false. +func (s *EtcdClusterSpec) MetricsEnabled() bool { + if s.Metrics == nil || s.Metrics.Enabled == nil { + return true + } + return *s.Metrics.Enabled +} + +// PodMonitorEnabled reports whether a PodMonitor should be reconciled for this +// cluster's etcd member pods. +func (s *EtcdClusterSpec) PodMonitorEnabled() bool { + return s.MetricsEnabled() && s.Metrics != nil && s.Metrics.PodMonitor != nil && s.Metrics.PodMonitor.Enabled } type PodTemplate struct { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 9db6ca12..09cdefd5 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -23,7 +23,7 @@ package v1alpha1 import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime" netx "net" ) @@ -161,6 +161,11 @@ func (in *EtcdClusterSpec) DeepCopyInto(out *EtcdClusterSpec) { *out = new(PodTemplate) (*in).DeepCopyInto(*out) } + if in.Metrics != nil { + in, out := &in.Metrics, &out.Metrics + *out = new(MetricsSpec) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EtcdClusterSpec. @@ -215,6 +220,31 @@ func (in *MemberStatus) DeepCopy() *MemberStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MetricsSpec) DeepCopyInto(out *MetricsSpec) { + *out = *in + if in.Enabled != nil { + in, out := &in.Enabled, &out.Enabled + *out = new(bool) + **out = **in + } + if in.PodMonitor != nil { + in, out := &in.PodMonitor, &out.PodMonitor + *out = new(PodMonitorSpec) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MetricsSpec. +func (in *MetricsSpec) DeepCopy() *MetricsSpec { + if in == nil { + return nil + } + out := new(MetricsSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PodMetadata) DeepCopyInto(out *PodMetadata) { *out = *in @@ -244,6 +274,21 @@ func (in *PodMetadata) DeepCopy() *PodMetadata { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PodMonitorSpec) DeepCopyInto(out *PodMonitorSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PodMonitorSpec. +func (in *PodMonitorSpec) DeepCopy() *PodMonitorSpec { + if in == nil { + return nil + } + out := new(PodMonitorSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PodSpec) DeepCopyInto(out *PodSpec) { *out = *in diff --git a/cmd/main.go b/cmd/main.go index a04983ec..df2ad200 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -35,6 +35,7 @@ import ( operatorv1alpha1 "go.etcd.io/etcd-operator/api/v1alpha1" "go.etcd.io/etcd-operator/internal/controller" + "go.etcd.io/etcd-operator/internal/metrics" // nolint:gci // +kubebuilder:scaffold:imports ) @@ -85,6 +86,10 @@ func main() { ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + // Register the operator's custom per-cluster domain metrics with the + // controller-runtime global registry so they are served on /metrics. + metrics.MustRegister() + // if the enable-http2 flag is false (the default), http/2 should be disabled // due to its vulnerabilities. More specifically, disabling http/2 will // prevent from being vulnerable to the HTTP/2 Stream Cancellation and diff --git a/config/crd/bases/operator.etcd.io_etcdclusters.yaml b/config/crd/bases/operator.etcd.io_etcdclusters.yaml index 0ae1e80e..1d7ba526 100644 --- a/config/crd/bases/operator.etcd.io_etcdclusters.yaml +++ b/config/crd/bases/operator.etcd.io_etcdclusters.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.20.1 + controller-gen.kubebuilder.io/version: v0.21.0 name: etcdclusters.operator.etcd.io spec: group: operator.etcd.io @@ -52,6 +52,46 @@ spec: If unset, it defaults to the value provided via the controller's --image-registry flag, which itself defaults to "gcr.io/etcd-development/etcd". type: string + metrics: + description: |- + Metrics configures Prometheus observability for this cluster. When unset, + the operator still exports its own per-cluster domain metrics on its + /metrics endpoint, but does not create a PodMonitor for the etcd member + pods. See MetricsSpec for the available knobs. + properties: + enabled: + description: |- + Enabled toggles emission of the operator's per-cluster domain metrics + (member counts, quorum, leader changes, reconcile timings, etc.) for this + cluster. When nil it defaults to true: metrics are exported unless + explicitly disabled. Disabling drops the cluster's series from the + operator's /metrics endpoint. + type: boolean + podMonitor: + description: |- + PodMonitor, when set and enabled, instructs the operator to create and + maintain a prometheus-operator PodMonitor selecting this cluster's etcd + member pods so that Prometheus scrapes etcd's own /metrics endpoint. + This requires the prometheus-operator PodMonitor CRD to be installed in + the cluster; if it is absent the operator logs and skips PodMonitor + reconciliation without failing the rest of the reconcile. + properties: + enabled: + description: Enabled toggles creation of the PodMonitor. Defaults + to false. + type: boolean + interval: + description: |- + Interval at which Prometheus scrapes the etcd member pods, e.g. "30s". + When empty the prometheus-operator default is used. + type: string + port: + description: |- + Port is the name of the pod port exposing etcd's /metrics endpoint. + Defaults to "client" when empty. + type: string + type: object + type: object podTemplate: description: PodTemplate is the pod template to use for the etcd cluster. properties: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 6c9ac65e..233cbceb 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -61,6 +61,18 @@ rules: - get - list - watch +- apiGroups: + - monitoring.coreos.com + resources: + - podmonitors + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - operator.etcd.io resources: diff --git a/config/samples/kustomization.yaml b/config/samples/kustomization.yaml index f3c97ab2..92b9dc88 100644 --- a/config/samples/kustomization.yaml +++ b/config/samples/kustomization.yaml @@ -1,4 +1,5 @@ ## Append samples of your project ## resources: - operator_v1alpha1_etcdcluster.yaml +- sample_metrics_etcdcluster.yaml # +kubebuilder:scaffold:manifestskustomizesamples diff --git a/config/samples/sample_metrics_etcdcluster.yaml b/config/samples/sample_metrics_etcdcluster.yaml new file mode 100644 index 00000000..3517c07b --- /dev/null +++ b/config/samples/sample_metrics_etcdcluster.yaml @@ -0,0 +1,27 @@ +apiVersion: operator.etcd.io/v1alpha1 +kind: EtcdCluster +metadata: + labels: + app.kubernetes.io/name: etcd-operator + app.kubernetes.io/managed-by: kustomize + name: etcdcluster-metrics-sample +spec: + version: v3.5.21 + size: 3 + # The operator always exports per-cluster domain metrics on its own /metrics + # endpoint (member counts, quorum, leader changes, reconcile timings, TLS-ready, + # learner count). The spec.metrics block lets you scope that behaviour per + # cluster and, optionally, emit a PodMonitor so Prometheus also scrapes etcd's + # own /metrics endpoint on the member pods. + metrics: + # Export the operator's per-cluster domain metrics for this cluster. + # Defaults to true when omitted; set to false to drop this cluster's series. + enabled: true + podMonitor: + # Create a prometheus-operator PodMonitor selecting this cluster's etcd + # member pods. Requires the PodMonitor CRD (prometheus-operator) to be + # installed; if absent the operator logs and skips it without failing. + enabled: true + interval: "30s" + # Name of the pod port exposing etcd's /metrics endpoint (default: client). + port: "client" diff --git a/go.mod b/go.mod index 6ae8ff14..60fec7ab 100644 --- a/go.mod +++ b/go.mod @@ -26,6 +26,7 @@ require ( github.com/golang-jwt/jwt/v5 v5.3.0 // indirect github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus v1.1.0 // indirect github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.3 // indirect + github.com/kylelemons/godebug v1.1.0 // indirect github.com/moby/spdystream v0.5.1 // indirect github.com/vladimirvivien/gexe v0.5.0 // indirect go.etcd.io/raft/v3 v3.6.0 // indirect @@ -77,7 +78,7 @@ require ( github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect - github.com/prometheus/client_golang v1.23.2 // indirect + github.com/prometheus/client_golang v1.23.2 github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/common v0.67.5 // indirect github.com/prometheus/procfs v0.19.2 // indirect diff --git a/internal/controller/etcdcluster_controller.go b/internal/controller/etcdcluster_controller.go index 867886a3..e2f2d437 100644 --- a/internal/controller/etcdcluster_controller.go +++ b/internal/controller/etcdcluster_controller.go @@ -36,6 +36,7 @@ import ( ecv1alpha1 "go.etcd.io/etcd-operator/api/v1alpha1" "go.etcd.io/etcd-operator/internal/etcdutils" + "go.etcd.io/etcd-operator/internal/metrics" etcdversions "go.etcd.io/etcd/api/v3/version" clientv3 "go.etcd.io/etcd/client/v3" ) @@ -73,6 +74,7 @@ type reconcileState struct { // +kubebuilder:rbac:groups="cert-manager.io",resources=certificates,verbs=get;list;watch;create;patch;update;delete // +kubebuilder:rbac:groups="cert-manager.io",resources=clusterissuers,verbs=get;list;watch // +kubebuilder:rbac:groups="cert-manager.io",resources=issuers,verbs=get;list;watch +// +kubebuilder:rbac:groups="monitoring.coreos.com",resources=podmonitors,verbs=get;list;watch;create;update;patch;delete // Reconcile orchestrates a single reconciliation cycle for an EtcdCluster. It // sequentially fetches resources, ensures primitive objects exist, checks the @@ -86,8 +88,13 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) var res ctrl.Result var err error - // Defer status update to ensure it's called regardless of return path + start := time.Now() + + // Defer status update to ensure it's called regardless of return path. + // Reconcile timing/error metrics are always recorded; per-cluster gauges are + // refreshed inside updateStatus once observed state is available. defer func() { + metrics.ObserveReconcile(req.Namespace, req.Name, time.Since(start).Seconds(), err) if state != nil { if statusErr := r.updateStatus(ctx, state); statusErr != nil { // Log but don't override the main reconciliation error @@ -123,6 +130,8 @@ func (r *EtcdClusterReconciler) fetchAndValidateState(ctx context.Context, req c if err := r.Get(ctx, req.NamespacedName, ec); err != nil { if errors.IsNotFound(err) { logger.Info("EtcdCluster resource not found. Ignoring since object may have been deleted") + // Drop any metric series for the deleted cluster to bound cardinality. + metrics.DeleteClusterMetrics(req.Namespace, req.Name) return nil, ctrl.Result{}, nil } return nil, ctrl.Result{}, err @@ -461,6 +470,18 @@ func (r *EtcdClusterReconciler) updateStatus(ctx context.Context, s *reconcileSt // Update conditions r.updateConditions(s) + // Refresh per-cluster Prometheus domain metrics from the freshly computed + // status. Metrics are on by default; an operator can opt a cluster out via + // spec.metrics.enabled=false, in which case any stale series are dropped. + if s.cluster.Spec.MetricsEnabled() { + metrics.RecordClusterMetrics(s.cluster) + } else { + metrics.DeleteClusterMetrics(s.cluster.Namespace, s.cluster.Name) + } + + // Reconcile an optional PodMonitor for the etcd member pods. + r.reconcilePodMonitor(ctx, s.cluster) + // Persist status update if err := r.Status().Update(ctx, s.cluster); err != nil { logger.Error(err, "Failed to update EtcdCluster status") diff --git a/internal/controller/podmonitor.go b/internal/controller/podmonitor.go new file mode 100644 index 00000000..5d11bb27 --- /dev/null +++ b/internal/controller/podmonitor.go @@ -0,0 +1,128 @@ +/* +Copyright 2025. + +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 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/log" + + ecv1alpha1 "go.etcd.io/etcd-operator/api/v1alpha1" +) + +// podMonitorGVK is the prometheus-operator PodMonitor type. We address it via +// unstructured so the operator does not take a hard dependency on the +// prometheus-operator Go module or require its CRDs to be present unless a user +// opts in to the PodMonitor feature. +var podMonitorGVK = schema.GroupVersionKind{ + Group: "monitoring.coreos.com", + Version: "v1", + Kind: "PodMonitor", +} + +const defaultPodMonitorPort = "client" + +// podMonitorCRDAbsent reports whether the error indicates the PodMonitor kind is +// not registered in the cluster (CRD absent), which we treat as a soft skip +// rather than a reconcile failure. +func podMonitorCRDAbsent(err error) bool { + return err != nil && meta.IsNoMatchError(err) +} + +// newPodMonitorObject returns an empty unstructured object addressing this +// cluster's PodMonitor (same namespace/name as the EtcdCluster). +func newPodMonitorObject(ec *ecv1alpha1.EtcdCluster) *unstructured.Unstructured { + u := &unstructured.Unstructured{} + u.SetGroupVersionKind(podMonitorGVK) + u.SetNamespace(ec.Namespace) + u.SetName(ec.Name) + return u +} + +// reconcilePodMonitor creates, updates, or deletes a PodMonitor selecting the +// cluster's etcd member pods, driven by spec.metrics.podMonitor. It never fails +// the reconcile: if the PodMonitor CRD is not installed it logs and returns. The +// PodMonitor is owned by the EtcdCluster so it is garbage-collected on deletion. +func (r *EtcdClusterReconciler) reconcilePodMonitor(ctx context.Context, ec *ecv1alpha1.EtcdCluster) { + logger := log.FromContext(ctx) + desired := newPodMonitorObject(ec) + + if !ec.Spec.PodMonitorEnabled() { + // Best-effort removal of a previously created PodMonitor. Ignore a + // missing object (already gone) and a missing CRD (never installed). + if err := r.Delete(ctx, desired); err != nil && + !apierrors.IsNotFound(err) && !podMonitorCRDAbsent(err) { + logger.Error(err, "Failed to delete PodMonitor") + } + return + } + + port := defaultPodMonitorPort + interval := "" + if pm := ec.Spec.Metrics.PodMonitor; pm != nil { + if pm.Port != "" { + port = pm.Port + } + interval = pm.Interval + } + + endpoint := map[string]any{ + "port": port, + "path": "/metrics", + } + if interval != "" { + endpoint["interval"] = interval + } + + op, err := controllerutil.CreateOrUpdate(ctx, r.Client, desired, func() error { + desired.SetLabels(map[string]string{ + "app.kubernetes.io/name": "etcd-operator", + "app.kubernetes.io/managed-by": "etcd-operator", + "app.kubernetes.io/instance": ec.Name, + }) + spec := map[string]any{ + "selector": map[string]any{ + "matchLabels": map[string]any{ + "app": ec.Name, + "controller": ec.Name, + }, + }, + "podMetricsEndpoints": []any{endpoint}, + } + if err := unstructured.SetNestedMap(desired.Object, spec, "spec"); err != nil { + return err + } + return controllerutil.SetControllerReference(ec, desired, r.Scheme) + }) + if err != nil { + if podMonitorCRDAbsent(err) { + logger.Info("PodMonitor CRD not installed; skipping PodMonitor reconciliation. " + + "Install prometheus-operator CRDs to enable spec.metrics.podMonitor.") + return + } + logger.Error(err, "Failed to reconcile PodMonitor") + return + } + if op != controllerutil.OperationResultNone { + logger.Info("Reconciled PodMonitor", "operation", op) + } +} diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go new file mode 100644 index 00000000..7af0cb78 --- /dev/null +++ b/internal/metrics/metrics.go @@ -0,0 +1,284 @@ +/* +Copyright 2025. + +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 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package metrics defines and registers the custom domain metrics exposed by +// the etcd-operator for every reconciled EtcdCluster. The collectors are +// registered against the controller-runtime global Prometheus registry +// (sigs.k8s.io/controller-runtime/pkg/metrics) so that they are served on the +// operator's existing /metrics endpoint with no additional HTTP plumbing. +// +// All per-cluster gauges are labelled with the cluster's namespace and name so +// that a single operator instance managing many clusters produces one time +// series per (namespace,name). When a cluster is deleted the operator should +// call DeleteClusterMetrics to drop its series and avoid unbounded cardinality +// growth from stale clusters. +package metrics + +import ( + "sync" + + "github.com/prometheus/client_golang/prometheus" + ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics" + + ecv1alpha1 "go.etcd.io/etcd-operator/api/v1alpha1" +) + +const ( + subsystem = "etcd_operator_cluster" + + // labelNamespace and labelName identify the EtcdCluster a series belongs to. + labelNamespace = "namespace" + labelName = "name" +) + +var ( + // MemberCountDesired is the size requested in EtcdCluster.Spec.Size. + MemberCountDesired = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Subsystem: subsystem, + Name: "member_count_desired", + Help: "Desired number of etcd members for the cluster (spec.size).", + }, []string{labelNamespace, labelName}) + + // MemberCount is the number of members reported by the etcd member list API. + MemberCount = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Subsystem: subsystem, + Name: "member_count", + Help: "Number of members currently registered in the etcd cluster (member list API).", + }, []string{labelNamespace, labelName}) + + // MemberCountReady is the number of ready etcd pods (StatefulSet readyReplicas). + MemberCountReady = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Subsystem: subsystem, + Name: "member_count_ready", + Help: "Number of ready etcd members (StatefulSet readyReplicas).", + }, []string{labelNamespace, labelName}) + + // MemberCountHealthy is the number of members reporting healthy. + MemberCountHealthy = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Subsystem: subsystem, + Name: "member_count_healthy", + Help: "Number of etcd members reported healthy in the cluster status.", + }, []string{labelNamespace, labelName}) + + // LearnerCount is the number of members that are currently learners. + LearnerCount = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Subsystem: subsystem, + Name: "learner_count", + Help: "Number of etcd members that are currently learners (non-voting).", + }, []string{labelNamespace, labelName}) + + // HasQuorum is 1 when a quorum of members is healthy, 0 otherwise. + HasQuorum = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Subsystem: subsystem, + Name: "has_quorum", + Help: "Whether the etcd cluster currently has quorum (1) or not (0).", + }, []string{labelNamespace, labelName}) + + // HasLeader is 1 when a leader is known, 0 otherwise. + HasLeader = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Subsystem: subsystem, + Name: "has_leader", + Help: "Whether the etcd cluster currently has a known leader (1) or not (0).", + }, []string{labelNamespace, labelName}) + + // TLSEnabled is 1 when the cluster is configured for TLS, 0 otherwise. + TLSEnabled = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Subsystem: subsystem, + Name: "tls_enabled", + Help: "Whether TLS is configured for the etcd cluster (1) or not (0).", + }, []string{labelNamespace, labelName}) + + // LeaderChangesTotal counts observed changes of the cluster leader ID. + LeaderChangesTotal = prometheus.NewCounterVec(prometheus.CounterOpts{ + Subsystem: subsystem, + Name: "leader_changes_total", + Help: "Total number of etcd leader changes observed by the operator for the cluster.", + }, []string{labelNamespace, labelName}) + + // ReconcileDurationSeconds observes the wall-clock duration of a reconcile. + ReconcileDurationSeconds = prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Subsystem: subsystem, + Name: "reconcile_duration_seconds", + Help: "Duration of EtcdCluster reconcile loops in seconds.", + Buckets: prometheus.DefBuckets, + }, []string{labelNamespace, labelName}) + + // ReconcileErrorsTotal counts reconcile loops that returned an error. + ReconcileErrorsTotal = prometheus.NewCounterVec(prometheus.CounterOpts{ + Subsystem: subsystem, + Name: "reconcile_errors_total", + Help: "Total number of EtcdCluster reconcile loops that returned an error.", + }, []string{labelNamespace, labelName}) +) + +// allCollectors returns every collector defined by this package. It is the +// single source of truth used by both registration and per-cluster deletion. +func allCollectors() []prometheus.Collector { + return []prometheus.Collector{ + MemberCountDesired, + MemberCount, + MemberCountReady, + MemberCountHealthy, + LearnerCount, + HasQuorum, + HasLeader, + TLSEnabled, + LeaderChangesTotal, + ReconcileDurationSeconds, + ReconcileErrorsTotal, + } +} + +var registerOnce sync.Once + +// MustRegister registers all custom collectors against the controller-runtime +// global registry. It is safe to call more than once; only the first call has +// an effect, which keeps it usable from both cmd/main.go and from tests that +// share the global registry. +func MustRegister() { + registerOnce.Do(func() { + ctrlmetrics.Registry.MustRegister(allCollectors()...) + }) +} + +// RegisterTo registers all custom collectors against an arbitrary registry. +// This is primarily intended for unit tests that want an isolated registry so +// that scraped values are deterministic and independent of process state. +func RegisterTo(r prometheus.Registerer) error { + for _, c := range allCollectors() { + if err := r.Register(c); err != nil { + return err + } + } + return nil +} + +// leaderTracker remembers the last observed leader ID per cluster so that +// transitions can be counted as leader changes. It is keyed by namespace/name. +type leaderTracker struct { + mu sync.Mutex + leaders map[string]string +} + +var tracker = &leaderTracker{leaders: map[string]string{}} + +func clusterKey(namespace, name string) string { + return namespace + "/" + name +} + +// observeLeader records the current leader for a cluster and returns true if it +// changed from a previously-known, non-empty leader. The first observation (or +// any observation following an unknown/empty leader) does not count as a change +// so that operator restarts and transient leaderless windows do not inflate the +// leader_changes_total counter. +func (t *leaderTracker) observeLeader(namespace, name, leaderID string) bool { + if leaderID == "" { + return false + } + t.mu.Lock() + defer t.mu.Unlock() + key := clusterKey(namespace, name) + prev, ok := t.leaders[key] + t.leaders[key] = leaderID + return ok && prev != "" && prev != leaderID +} + +// forget drops any tracked leader for the cluster. +func (t *leaderTracker) forget(namespace, name string) { + t.mu.Lock() + defer t.mu.Unlock() + delete(t.leaders, clusterKey(namespace, name)) +} + +// RecordClusterMetrics derives all per-cluster gauge values from the cluster's +// spec and (already-populated) status, and counts a leader change if the +// observed leader differs from the previous reconcile. It is intended to be +// called once per reconcile, after the controller has computed status, with the +// in-memory EtcdCluster object (no extra API calls required). +func RecordClusterMetrics(cluster *ecv1alpha1.EtcdCluster) { + ns, name := cluster.Namespace, cluster.Name + + MemberCountDesired.WithLabelValues(ns, name).Set(float64(cluster.Spec.Size)) + MemberCount.WithLabelValues(ns, name).Set(float64(cluster.Status.MemberCount)) + MemberCountReady.WithLabelValues(ns, name).Set(float64(cluster.Status.ReadyReplicas)) + + healthy := 0 + learners := 0 + for _, m := range cluster.Status.Members { + if m.IsHealthy { + healthy++ + } + if m.IsLearner { + learners++ + } + } + MemberCountHealthy.WithLabelValues(ns, name).Set(float64(healthy)) + LearnerCount.WithLabelValues(ns, name).Set(float64(learners)) + + // Quorum: a strict majority of the registered members must be healthy. + HasQuorum.WithLabelValues(ns, name).Set(boolToFloat(hasQuorum(int(cluster.Status.MemberCount), healthy))) + + HasLeader.WithLabelValues(ns, name).Set(boolToFloat(cluster.Status.LeaderID != "")) + TLSEnabled.WithLabelValues(ns, name).Set(boolToFloat(cluster.Spec.TLS != nil)) + + if tracker.observeLeader(ns, name, cluster.Status.LeaderID) { + LeaderChangesTotal.WithLabelValues(ns, name).Inc() + } +} + +// ObserveReconcile records the duration of a reconcile loop and increments the +// error counter when the reconcile returned an error. +func ObserveReconcile(namespace, name string, seconds float64, reconcileErr error) { + ReconcileDurationSeconds.WithLabelValues(namespace, name).Observe(seconds) + if reconcileErr != nil { + ReconcileErrorsTotal.WithLabelValues(namespace, name).Inc() + } +} + +// DeleteClusterMetrics removes every per-cluster series for the given cluster. +// It should be called when an EtcdCluster is deleted to bound metric +// cardinality. Deleting a label set that was never set is a no-op. +func DeleteClusterMetrics(namespace, name string) { + lvs := []string{namespace, name} + MemberCountDesired.DeleteLabelValues(lvs...) + MemberCount.DeleteLabelValues(lvs...) + MemberCountReady.DeleteLabelValues(lvs...) + MemberCountHealthy.DeleteLabelValues(lvs...) + LearnerCount.DeleteLabelValues(lvs...) + HasQuorum.DeleteLabelValues(lvs...) + HasLeader.DeleteLabelValues(lvs...) + TLSEnabled.DeleteLabelValues(lvs...) + LeaderChangesTotal.DeleteLabelValues(lvs...) + ReconcileErrorsTotal.DeleteLabelValues(lvs...) + ReconcileDurationSeconds.DeleteLabelValues(lvs...) + tracker.forget(namespace, name) +} + +// hasQuorum reports whether healthy members constitute a strict majority of the +// registered members. With zero members there is no quorum. +func hasQuorum(members, healthy int) bool { + if members <= 0 { + return false + } + return healthy >= (members/2)+1 +} + +func boolToFloat(b bool) float64 { + if b { + return 1 + } + return 0 +} diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go new file mode 100644 index 00000000..4c8b6f21 --- /dev/null +++ b/internal/metrics/metrics_test.go @@ -0,0 +1,329 @@ +/* +Copyright 2025. + +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 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package metrics + +import ( + "errors" + "strings" + "testing" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/testutil" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + ecv1alpha1 "go.etcd.io/etcd-operator/api/v1alpha1" +) + +// resetState clears all collectors and the leader tracker so each test starts +// from a clean slate even though the collectors are package-level globals. +func resetState() { + MemberCountDesired.Reset() + MemberCount.Reset() + MemberCountReady.Reset() + MemberCountHealthy.Reset() + LearnerCount.Reset() + HasQuorum.Reset() + HasLeader.Reset() + TLSEnabled.Reset() + LeaderChangesTotal.Reset() + ReconcileErrorsTotal.Reset() + ReconcileDurationSeconds.Reset() + tracker.mu.Lock() + tracker.leaders = map[string]string{} + tracker.mu.Unlock() +} + +func newCluster(ns, name string, size int) *ecv1alpha1.EtcdCluster { + return &ecv1alpha1.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: name}, + Spec: ecv1alpha1.EtcdClusterSpec{Size: size}, + } +} + +// newRegistry returns an isolated registry with all collectors registered, so +// scrape values are deterministic and independent of process/global state. +func newRegistry(t *testing.T) *prometheus.Registry { + t.Helper() + resetState() + reg := prometheus.NewRegistry() + if err := RegisterTo(reg); err != nil { + t.Fatalf("RegisterTo failed: %v", err) + } + return reg +} + +func TestRegisterToIsComplete(t *testing.T) { + reg := newRegistry(t) + // Every collector defined by the package must register without conflict. + mfs, err := reg.Gather() + if err != nil { + t.Fatalf("Gather failed: %v", err) + } + if len(mfs) == 0 { + // Counters/histograms with no observations may not appear until used, + // but gauges with no series also won't. This just ensures Gather works. + t.Log("no metric families yet (expected before any values are set)") + } +} + +func TestRecordClusterMetrics_HealthyQuorate(t *testing.T) { + reg := newRegistry(t) + + c := newCluster("ns1", "etcd-a", 3) + c.Spec.TLS = &ecv1alpha1.TLSCertificate{Provider: "auto"} + c.Status.MemberCount = 3 + c.Status.ReadyReplicas = 3 + c.Status.LeaderID = "abc123" + c.Status.Members = []ecv1alpha1.MemberStatus{ + {ID: "1", IsHealthy: true, IsLeader: true}, + {ID: "2", IsHealthy: true}, + {ID: "3", IsHealthy: true}, + } + + RecordClusterMetrics(c) + + assertGauge(t, reg, "etcd_operator_cluster_member_count_desired", 3, "ns1", "etcd-a") + assertGauge(t, reg, "etcd_operator_cluster_member_count", 3, "ns1", "etcd-a") + assertGauge(t, reg, "etcd_operator_cluster_member_count_ready", 3, "ns1", "etcd-a") + assertGauge(t, reg, "etcd_operator_cluster_member_count_healthy", 3, "ns1", "etcd-a") + assertGauge(t, reg, "etcd_operator_cluster_learner_count", 0, "ns1", "etcd-a") + assertGauge(t, reg, "etcd_operator_cluster_has_quorum", 1, "ns1", "etcd-a") + assertGauge(t, reg, "etcd_operator_cluster_has_leader", 1, "ns1", "etcd-a") + assertGauge(t, reg, "etcd_operator_cluster_tls_enabled", 1, "ns1", "etcd-a") +} + +func TestRecordClusterMetrics_DegradedNoQuorumNoLeader(t *testing.T) { + reg := newRegistry(t) + + c := newCluster("ns1", "etcd-b", 3) + c.Status.MemberCount = 3 + c.Status.ReadyReplicas = 1 + c.Status.LeaderID = "" // no leader known + c.Status.Members = []ecv1alpha1.MemberStatus{ + {ID: "1", IsHealthy: true}, + {ID: "2", IsHealthy: false}, + {ID: "3", IsHealthy: false}, + } + + RecordClusterMetrics(c) + + // 1/3 healthy -> below quorum of 2. + assertGauge(t, reg, "etcd_operator_cluster_member_count_healthy", 1, "ns1", "etcd-b") + assertGauge(t, reg, "etcd_operator_cluster_has_quorum", 0, "ns1", "etcd-b") + assertGauge(t, reg, "etcd_operator_cluster_has_leader", 0, "ns1", "etcd-b") + assertGauge(t, reg, "etcd_operator_cluster_tls_enabled", 0, "ns1", "etcd-b") +} + +func TestRecordClusterMetrics_LearnerCounted(t *testing.T) { + reg := newRegistry(t) + + c := newCluster("ns1", "etcd-c", 4) + c.Status.MemberCount = 4 + c.Status.Members = []ecv1alpha1.MemberStatus{ + {ID: "1", IsHealthy: true, IsLeader: true}, + {ID: "2", IsHealthy: true}, + {ID: "3", IsHealthy: true}, + {ID: "4", IsHealthy: true, IsLearner: true}, + } + c.Status.LeaderID = "1" + + RecordClusterMetrics(c) + + assertGauge(t, reg, "etcd_operator_cluster_learner_count", 1, "ns1", "etcd-c") + // 4 healthy of 4 members -> quorum (>=3) satisfied. + assertGauge(t, reg, "etcd_operator_cluster_has_quorum", 1, "ns1", "etcd-c") +} + +func TestLeaderChangesCounter(t *testing.T) { + reg := newRegistry(t) + c := newCluster("ns1", "etcd-d", 3) + c.Status.MemberCount = 3 + c.Status.Members = []ecv1alpha1.MemberStatus{{ID: "1", IsHealthy: true}} + + // First observation: no change counted. + c.Status.LeaderID = "leader1" + RecordClusterMetrics(c) + assertCounter(t, reg, "etcd_operator_cluster_leader_changes_total", 0, "ns1", "etcd-d") + + // Same leader again: still no change. + RecordClusterMetrics(c) + assertCounter(t, reg, "etcd_operator_cluster_leader_changes_total", 0, "ns1", "etcd-d") + + // Leader changes: counted once. + c.Status.LeaderID = "leader2" + RecordClusterMetrics(c) + assertCounter(t, reg, "etcd_operator_cluster_leader_changes_total", 1, "ns1", "etcd-d") + + // Leaderless window does not count, and recovery to a new leader counts again. + c.Status.LeaderID = "" + RecordClusterMetrics(c) + assertCounter(t, reg, "etcd_operator_cluster_leader_changes_total", 1, "ns1", "etcd-d") + c.Status.LeaderID = "leader3" + RecordClusterMetrics(c) + assertCounter(t, reg, "etcd_operator_cluster_leader_changes_total", 2, "ns1", "etcd-d") +} + +func TestObserveReconcile(t *testing.T) { + reg := newRegistry(t) + + ObserveReconcile("ns1", "etcd-e", 0.5, nil) + ObserveReconcile("ns1", "etcd-e", 0.7, errors.New("boom")) + + // Two observations recorded in the histogram. + if got := testutil.CollectAndCount(ReconcileDurationSeconds); got == 0 { + t.Fatalf("expected reconcile duration observations, got none") + } + assertCounter(t, reg, "etcd_operator_cluster_reconcile_errors_total", 1, "ns1", "etcd-e") +} + +func TestDeleteClusterMetrics(t *testing.T) { + reg := newRegistry(t) + + c := newCluster("ns1", "etcd-f", 1) + c.Status.MemberCount = 1 + c.Status.LeaderID = "x" + c.Status.Members = []ecv1alpha1.MemberStatus{{ID: "1", IsHealthy: true}} + RecordClusterMetrics(c) + ObserveReconcile("ns1", "etcd-f", 0.1, errors.New("e")) + + if c := testutil.CollectAndCount(MemberCount); c == 0 { + t.Fatalf("expected member_count series present before delete") + } + + DeleteClusterMetrics("ns1", "etcd-f") + + for _, coll := range []prometheus.Collector{ + MemberCount, MemberCountDesired, HasQuorum, HasLeader, TLSEnabled, + LeaderChangesTotal, ReconcileErrorsTotal, + } { + if got := testutil.CollectAndCount(coll); got != 0 { + t.Errorf("expected 0 series after delete, got %d", got) + } + } + _ = reg + + // Leader tracker must also be reset, so a re-created cluster does not + // spuriously count a leader change on its first observation. + c.Status.LeaderID = "y" + RecordClusterMetrics(c) + assertCounter(t, reg, "etcd_operator_cluster_leader_changes_total", 0, "ns1", "etcd-f") +} + +func TestMetricsExposedWithExpectedLabels(t *testing.T) { + reg := newRegistry(t) + c := newCluster("prod", "etcd-1", 3) + c.Status.MemberCount = 3 + c.Status.Members = []ecv1alpha1.MemberStatus{{ID: "1", IsHealthy: true}} + c.Status.LeaderID = "1" + RecordClusterMetrics(c) + + expected := ` +# HELP etcd_operator_cluster_member_count_desired Desired number of etcd members for the cluster (spec.size). +# TYPE etcd_operator_cluster_member_count_desired gauge +etcd_operator_cluster_member_count_desired{name="etcd-1",namespace="prod"} 3 +` + if err := testutil.GatherAndCompare(reg, strings.NewReader(expected), + "etcd_operator_cluster_member_count_desired"); err != nil { + t.Errorf("unexpected metric output: %v", err) + } +} + +func TestSpecHelpers(t *testing.T) { + // Default (nil Metrics) -> metrics on, podmonitor off. + s := &ecv1alpha1.EtcdClusterSpec{} + if !s.MetricsEnabled() { + t.Error("expected metrics enabled by default") + } + if s.PodMonitorEnabled() { + t.Error("expected podmonitor disabled by default") + } + + // Explicit disable. + off := false + s.Metrics = &ecv1alpha1.MetricsSpec{Enabled: &off} + if s.MetricsEnabled() { + t.Error("expected metrics disabled when enabled=false") + } + if s.PodMonitorEnabled() { + t.Error("podmonitor must follow metrics disable") + } + + // Explicit enable + podmonitor on. + on := true + s.Metrics = &ecv1alpha1.MetricsSpec{Enabled: &on, PodMonitor: &ecv1alpha1.PodMonitorSpec{Enabled: true}} + if !s.PodMonitorEnabled() { + t.Error("expected podmonitor enabled") + } + + // PodMonitor enabled but metrics disabled -> podmonitor must be off. + s.Metrics.Enabled = &off + if s.PodMonitorEnabled() { + t.Error("podmonitor must be off when metrics disabled") + } +} + +func assertGauge(t *testing.T, reg *prometheus.Registry, name string, want float64, ns, clusterName string) { + t.Helper() + got := readSample(t, reg, name, ns, clusterName) + if got != want { + t.Errorf("%s{namespace=%q,name=%q}=%v, want %v", name, ns, clusterName, got, want) + } +} + +func assertCounter(t *testing.T, reg *prometheus.Registry, name string, want float64, ns, clusterName string) { + t.Helper() + got := readSample(t, reg, name, ns, clusterName) + if got != want { + t.Errorf("%s{namespace=%q,name=%q}=%v, want %v", name, ns, clusterName, got, want) + } +} + +// readSample gathers the registry and returns the value of the named metric for +// the given (namespace,name) label pair, or 0 if no such series exists. +func readSample(t *testing.T, reg *prometheus.Registry, name, ns, clusterName string) float64 { + t.Helper() + mfs, err := reg.Gather() + if err != nil { + t.Fatalf("Gather: %v", err) + } + for _, mf := range mfs { + if mf.GetName() != name { + continue + } + for _, m := range mf.GetMetric() { + var gotNS, gotName string + for _, l := range m.GetLabel() { + switch l.GetName() { + case labelNamespace: + gotNS = l.GetValue() + case labelName: + gotName = l.GetValue() + } + } + if gotNS != ns || gotName != clusterName { + continue + } + if m.Gauge != nil { + return m.Gauge.GetValue() + } + if m.Counter != nil { + return m.Counter.GetValue() + } + } + } + return 0 +} diff --git a/test/e2e/metrics_test.go b/test/e2e/metrics_test.go new file mode 100644 index 00000000..24e56d46 --- /dev/null +++ b/test/e2e/metrics_test.go @@ -0,0 +1,185 @@ +/* +Copyright 2025. + +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 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2e + +import ( + "context" + "fmt" + "strings" + "testing" + "time" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "sigs.k8s.io/e2e-framework/klient/k8s" + "sigs.k8s.io/e2e-framework/klient/wait" + "sigs.k8s.io/e2e-framework/klient/wait/conditions" + "sigs.k8s.io/e2e-framework/pkg/envconf" + "sigs.k8s.io/e2e-framework/pkg/features" + + ecv1alpha1 "go.etcd.io/etcd-operator/api/v1alpha1" +) + +// metricNamesToAssert lists the custom domain metrics this feature expects to +// find on the operator's /metrics endpoint once at least one EtcdCluster has +// been reconciled. +var metricNamesToAssert = []string{ + "etcd_operator_cluster_member_count_desired", + "etcd_operator_cluster_member_count", + "etcd_operator_cluster_member_count_ready", + "etcd_operator_cluster_member_count_healthy", + "etcd_operator_cluster_learner_count", + "etcd_operator_cluster_has_quorum", + "etcd_operator_cluster_has_leader", + "etcd_operator_cluster_tls_enabled", + "etcd_operator_cluster_reconcile_duration_seconds", +} + +// TestDomainMetricsExposed creates an EtcdCluster with metrics + a PodMonitor +// configured via spec.metrics, waits for it to become ready, and asserts that +// the operator's Prometheus /metrics endpoint exposes the custom per-cluster +// domain metrics labelled with this cluster's namespace and name. +// +// NOTE: This test is authored primarily for compile-time verification in CI +// alongside the unit/envtest coverage of the metrics package; it only executes +// against a live KinD cluster as part of the e2e suite (make test-e2e). +func TestDomainMetricsExposed(t *testing.T) { + clusterName := "etcd-metrics" + feature := features.New("domain-metrics") + + feature.Setup(func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + metricsCreateCluster(ctx, t, c, clusterName, 3) + metricsWaitForStatefulSetReady(ctx, t, c, clusterName, 3) + return ctx + }) + + feature.Assess("operator /metrics exposes per-cluster domain metrics", + func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + body := metricsScrapeOperator(ctx, t, c) + for _, name := range metricNamesToAssert { + if !strings.Contains(body, name) { + t.Errorf("expected /metrics to contain %q, but it did not", name) + } + } + // At least one series must be labelled with this cluster's name. + if !strings.Contains(body, fmt.Sprintf("name=%q", clusterName)) { + t.Errorf("expected /metrics to contain a series labelled name=%q", clusterName) + } + return ctx + }) + + feature.Teardown(func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + metricsDeleteCluster(ctx, t, c, clusterName) + return ctx + }) + + _ = testEnv.Test(t, feature.Feature()) +} + +// metricsCreateCluster creates an EtcdCluster with metrics + PodMonitor enabled. +// It is a self-contained helper (not shared with other e2e files) to keep this +// PR independent of in-flight e2e changes. +func metricsCreateCluster(ctx context.Context, t *testing.T, c *envconf.Config, name string, size int) { + t.Helper() + enabled := true + ec := &ecv1alpha1.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + Spec: ecv1alpha1.EtcdClusterSpec{ + Size: size, + Version: "v3.5.21", + Metrics: &ecv1alpha1.MetricsSpec{ + Enabled: &enabled, + PodMonitor: &ecv1alpha1.PodMonitorSpec{ + Enabled: true, + Interval: "30s", + Port: "client", + }, + }, + }, + } + if err := c.Client().Resources().Create(ctx, ec); err != nil { + t.Fatalf("failed to create EtcdCluster %q: %v", name, err) + } +} + +// metricsDeleteCluster removes the EtcdCluster created for this feature. +func metricsDeleteCluster(ctx context.Context, t *testing.T, c *envconf.Config, name string) { + t.Helper() + ec := &ecv1alpha1.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + } + if err := c.Client().Resources().Delete(ctx, ec); err != nil { + t.Logf("failed to delete EtcdCluster %q: %v", name, err) + } +} + +// metricsWaitForStatefulSetReady blocks until the cluster's StatefulSet reports +// the expected number of ready replicas. +func metricsWaitForStatefulSetReady(ctx context.Context, t *testing.T, c *envconf.Config, name string, expectedReplicas int) { + t.Helper() + sts := &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + } + if err := wait.For( + conditions.New(c.Client().Resources()).ResourceMatch(sts, func(object k8s.Object) bool { + s, ok := object.(*appsv1.StatefulSet) + return ok && int(s.Status.ReadyReplicas) == expectedReplicas + }), + wait.WithTimeout(5*time.Minute), + wait.WithInterval(10*time.Second), + wait.WithContext(ctx), + ); err != nil { + t.Fatalf("statefulset %q did not reach %d ready replicas: %v", name, expectedReplicas, err) + } +} + +// metricsScrapeOperator scrapes the operator pod's metrics endpoint through the +// API server pod-proxy and returns the raw Prometheus exposition text. The e2e +// manager is expected to serve plain-HTTP metrics on :8080 +// (--metrics-bind-address=:8080 --metrics-secure=false) for scrape simplicity. +func metricsScrapeOperator(ctx context.Context, t *testing.T, c *envconf.Config) string { + t.Helper() + pod, err := getEtcdOperatorPod(t, c.Client()) + if err != nil { + t.Fatalf("failed to find operator pod: %v", err) + } + + clientset, err := kubernetes.NewForConfig(c.Client().RESTConfig()) + if err != nil { + t.Fatalf("failed to build clientset: %v", err) + } + + res := clientset.CoreV1().RESTClient().Get(). + Namespace(pod.Namespace). + Resource("pods"). + SubResource("proxy"). + Name(fmt.Sprintf("%s:8080", pod.Name)). + Suffix("metrics"). + Do(ctx) + if err := res.Error(); err != nil { + t.Fatalf("failed to scrape /metrics via pod proxy: %v", err) + } + raw, err := res.Raw() + if err != nil { + t.Fatalf("failed to read /metrics response: %v", err) + } + return string(raw) +} + +var _ = corev1.Pod{} From 1f7c0d95a878b332dd7628826c2fb368394f2000 Mon Sep 17 00:00:00 2001 From: Xavier Lange Date: Thu, 18 Jun 2026 20:03:22 -0400 Subject: [PATCH 2/8] fix(metrics): stop has_leader and reconcile series from lying after deletion/leaderloss Two correctness fixes in the reconcile path that fed the per-cluster metrics: - Gate ObserveReconcile/updateStatus on state != nil in the Reconcile defer. On the NotFound path fetchAndValidateState already drops every series via DeleteClusterMetrics; observing afterwards re-created reconcile_duration_seconds (and the other series via updateStatus) for the just-deleted cluster on every tombstone reconcile, defeating the cardinality bound the package exists to keep. - Clear Status.LeaderID to "" when no leader is currently known in updateStatus. Previously LeaderID was only ever set, never cleared, so has_leader (derived from LeaderID != "") stuck at 1 forever once any leader was seen, hiding the exact leaderless outage it should report. observeLeader already treats "" as 'no change', so leader_changes_total is unaffected. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Xavier Lange --- internal/controller/etcdcluster_controller.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/controller/etcdcluster_controller.go b/internal/controller/etcdcluster_controller.go index e2f2d437..a7951029 100644 --- a/internal/controller/etcdcluster_controller.go +++ b/internal/controller/etcdcluster_controller.go @@ -91,11 +91,13 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) start := time.Now() // Defer status update to ensure it's called regardless of return path. - // Reconcile timing/error metrics are always recorded; per-cluster gauges are - // refreshed inside updateStatus once observed state is available. + // Reconcile timing/error metrics and per-cluster gauges are only recorded when + // the cluster still exists: on the NotFound path fetchAndValidateState already + // dropped the series via DeleteClusterMetrics, so observing here would + // re-create them for a deleted cluster and defeat that cardinality bound. defer func() { - metrics.ObserveReconcile(req.Namespace, req.Name, time.Since(start).Seconds(), err) if state != nil { + metrics.ObserveReconcile(req.Namespace, req.Name, time.Since(start).Seconds(), err) if statusErr := r.updateStatus(ctx, state); statusErr != nil { // Log but don't override the main reconciliation error log.FromContext(ctx).Error(statusErr, "Failed to update status") @@ -453,10 +455,14 @@ func (r *EtcdClusterReconciler) updateStatus(ctx context.Context, s *reconcileSt s.cluster.Status.Members = append(s.cluster.Status.Members, memberStatus) } - // Update leader ID + // Update leader ID. Clear it when no leader is currently known so that + // has_leader can drop to 0 during a leaderless window instead of + // reporting a permanently stale leader from an earlier reconcile. _, leaderStatus := etcdutils.FindLeaderStatus(s.memberHealth, logger) if leaderStatus != nil { s.cluster.Status.LeaderID = fmt.Sprintf("%x", leaderStatus.Leader) + } else { + s.cluster.Status.LeaderID = "" } // Update current version from leader or first healthy member From bb710d94be9d42cb38f83ea1ea28f226c7b8e992 Mon Sep 17 00:00:00 2001 From: Xavier Lange Date: Thu, 18 Jun 2026 20:03:29 -0400 Subject: [PATCH 3/8] fix(metrics): compute has_quorum over voting members only RecordClusterMetrics computed quorum as hasQuorum(MemberCount, healthy), but both totals included learners. etcd raft quorum is over voting members only, so a healthy learner could mask a degraded voting set (e.g. 2 voting / 1 healthy plus 1 healthy learner reported quorum=1 during a real outage). Track healthyVoting (healthy and not a learner) and derive voting = MemberCount - learners, then call hasQuorum(voting, healthyVoting). Replace the learner test with one that exercises the learner-exclusion case and add a fully-healthy voting-set-with-learner case. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Xavier Lange --- internal/metrics/metrics.go | 11 +++++++++-- internal/metrics/metrics_test.go | 34 +++++++++++++++++++++++++++----- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 7af0cb78..47eae0db 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -216,10 +216,14 @@ func RecordClusterMetrics(cluster *ecv1alpha1.EtcdCluster) { MemberCountReady.WithLabelValues(ns, name).Set(float64(cluster.Status.ReadyReplicas)) healthy := 0 + healthyVoting := 0 learners := 0 for _, m := range cluster.Status.Members { if m.IsHealthy { healthy++ + if !m.IsLearner { + healthyVoting++ + } } if m.IsLearner { learners++ @@ -228,8 +232,11 @@ func RecordClusterMetrics(cluster *ecv1alpha1.EtcdCluster) { MemberCountHealthy.WithLabelValues(ns, name).Set(float64(healthy)) LearnerCount.WithLabelValues(ns, name).Set(float64(learners)) - // Quorum: a strict majority of the registered members must be healthy. - HasQuorum.WithLabelValues(ns, name).Set(boolToFloat(hasQuorum(int(cluster.Status.MemberCount), healthy))) + // Quorum is decided over voting members only: learners do not participate in + // the etcd raft quorum, so a healthy learner must not be able to make a + // degraded voting set look quorate. + voting := int(cluster.Status.MemberCount) - learners + HasQuorum.WithLabelValues(ns, name).Set(boolToFloat(hasQuorum(voting, healthyVoting))) HasLeader.WithLabelValues(ns, name).Set(boolToFloat(cluster.Status.LeaderID != "")) TLSEnabled.WithLabelValues(ns, name).Set(boolToFloat(cluster.Spec.TLS != nil)) diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index 4c8b6f21..87d6a3f8 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -128,10 +128,34 @@ func TestRecordClusterMetrics_DegradedNoQuorumNoLeader(t *testing.T) { assertGauge(t, reg, "etcd_operator_cluster_tls_enabled", 0, "ns1", "etcd-b") } -func TestRecordClusterMetrics_LearnerCounted(t *testing.T) { +func TestRecordClusterMetrics_LearnerExcludedFromQuorum(t *testing.T) { reg := newRegistry(t) - c := newCluster("ns1", "etcd-c", 4) + // 2 voting members (1 healthy, 1 down) plus 1 healthy learner. The learner + // does not vote, so the voting set (1/2 healthy) has no quorum even though a + // naive headcount (2/3 healthy) would look quorate. + c := newCluster("ns1", "etcd-c", 3) + c.Status.MemberCount = 3 + c.Status.Members = []ecv1alpha1.MemberStatus{ + {ID: "1", IsHealthy: true, IsLeader: true}, + {ID: "2", IsHealthy: false}, + {ID: "3", IsHealthy: true, IsLearner: true}, + } + c.Status.LeaderID = "1" + + RecordClusterMetrics(c) + + assertGauge(t, reg, "etcd_operator_cluster_learner_count", 1, "ns1", "etcd-c") + // Voting members: 2, healthy voting: 1 -> below quorum of 2. + assertGauge(t, reg, "etcd_operator_cluster_has_quorum", 0, "ns1", "etcd-c") +} + +func TestRecordClusterMetrics_LearnerHealthyVotingQuorate(t *testing.T) { + reg := newRegistry(t) + + // 3 healthy voting members plus 1 healthy learner: the voting set is fully + // healthy, so quorum holds and the learner is merely counted. + c := newCluster("ns1", "etcd-c2", 4) c.Status.MemberCount = 4 c.Status.Members = []ecv1alpha1.MemberStatus{ {ID: "1", IsHealthy: true, IsLeader: true}, @@ -143,9 +167,9 @@ func TestRecordClusterMetrics_LearnerCounted(t *testing.T) { RecordClusterMetrics(c) - assertGauge(t, reg, "etcd_operator_cluster_learner_count", 1, "ns1", "etcd-c") - // 4 healthy of 4 members -> quorum (>=3) satisfied. - assertGauge(t, reg, "etcd_operator_cluster_has_quorum", 1, "ns1", "etcd-c") + assertGauge(t, reg, "etcd_operator_cluster_learner_count", 1, "ns1", "etcd-c2") + // 3 voting members, all healthy -> quorum (>=2) satisfied. + assertGauge(t, reg, "etcd_operator_cluster_has_quorum", 1, "ns1", "etcd-c2") } func TestLeaderChangesCounter(t *testing.T) { From 8b5b37db0689a9a7eb6dd6dc2e804b6b64cbd4e3 Mon Sep 17 00:00:00 2001 From: Xavier Lange Date: Thu, 18 Jun 2026 20:03:36 -0400 Subject: [PATCH 4/8] test(e2e): serve metrics on :8080 insecure in e2e overlay and assert a value The e2e scrape hits the operator pod's :8080 over plain HTTP via pod-proxy, but the default overlay serves secure HTTPS metrics on :8443 behind authn/authz, so the scrape reached a port nothing served (or 401'd) and the test only ever 'compile-verified'. Add a config/e2e patch-metrics.yaml overriding the manager args to --metrics-bind-address=:8080 --metrics-secure=false (composed with the existing patch-env.yaml), so the scrape actually reaches the manager. Also upgrade an assertion from a metric-name substring (which passes from HELP/TYPE lines even with zero series) to a value check on member_count_desired{name=...} 3, proving the per-cluster gauge was recorded. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Xavier Lange --- config/e2e/kustomization.yaml | 4 ++++ config/e2e/patch-metrics.yaml | 20 ++++++++++++++++++++ test/e2e/metrics_test.go | 24 ++++++++++++++++++++++++ 3 files changed, 48 insertions(+) create mode 100644 config/e2e/patch-metrics.yaml diff --git a/config/e2e/kustomization.yaml b/config/e2e/kustomization.yaml index e5f4e7a3..d2769ac3 100644 --- a/config/e2e/kustomization.yaml +++ b/config/e2e/kustomization.yaml @@ -6,3 +6,7 @@ patches: target: kind: Deployment name: controller-manager + - path: patch-metrics.yaml + target: + kind: Deployment + name: controller-manager diff --git a/config/e2e/patch-metrics.yaml b/config/e2e/patch-metrics.yaml new file mode 100644 index 00000000..9d744aba --- /dev/null +++ b/config/e2e/patch-metrics.yaml @@ -0,0 +1,20 @@ +# Serve metrics over plain HTTP on :8080 for the e2e suite, which scrapes the +# operator pod via the API server pod-proxy (see test/e2e/metrics_test.go). The +# default overlay serves secure HTTPS metrics on :8443 behind authn/authz, which +# the pod-proxy scrape cannot reach. This replaces the manager's args so the +# metrics endpoint is reachable; it intentionally does not touch the env patched +# in patch-env.yaml. +apiVersion: apps/v1 +kind: Deployment +metadata: + name: etcd-operator +spec: + template: + spec: + containers: + - name: manager + args: + - --metrics-bind-address=:8080 + - --metrics-secure=false + - --leader-elect + - --health-probe-bind-address=:8081 diff --git a/test/e2e/metrics_test.go b/test/e2e/metrics_test.go index 24e56d46..501a2c2e 100644 --- a/test/e2e/metrics_test.go +++ b/test/e2e/metrics_test.go @@ -81,6 +81,17 @@ func TestDomainMetricsExposed(t *testing.T) { if !strings.Contains(body, fmt.Sprintf("name=%q", clusterName)) { t.Errorf("expected /metrics to contain a series labelled name=%q", clusterName) } + // Assert an actual value, not just the metric name: a name-substring + // check passes purely from collector registration (# HELP/# TYPE + // lines) even with zero series. The desired member count is the + // cluster's spec.size (3), so this proves the per-cluster gauge was + // recorded for the reconciled cluster. + wantDesired := fmt.Sprintf( + "etcd_operator_cluster_member_count_desired{name=%q,namespace=%q} 3", + clusterName, namespace) + if !metricsContainsSample(body, wantDesired) { + t.Errorf("expected /metrics to contain sample %q; body did not match", wantDesired) + } return ctx }) @@ -149,6 +160,19 @@ func metricsWaitForStatefulSetReady(ctx context.Context, t *testing.T, c *envcon } } +// metricsContainsSample reports whether the Prometheus exposition text contains +// a line equal to want after trimming surrounding whitespace. Sample lines are +// compared line-by-line so a stray prefix/suffix elsewhere in the body cannot +// produce a false positive. +func metricsContainsSample(body, want string) bool { + for _, line := range strings.Split(body, "\n") { + if strings.TrimSpace(line) == want { + return true + } + } + return false +} + // metricsScrapeOperator scrapes the operator pod's metrics endpoint through the // API server pod-proxy and returns the raw Prometheus exposition text. The e2e // manager is expected to serve plain-HTTP metrics on :8080 From 031d8eb66b3e63e6497913974cddbd48f16b2b29 Mon Sep 17 00:00:00 2001 From: Xavier Lange Date: Thu, 18 Jun 2026 20:03:41 -0400 Subject: [PATCH 5/8] test(controller): add unit tests for reconcilePodMonitor The PodMonitor reconcile (the riskiest new control flow) had no coverage. Add table of fake-client tests: created-with-expected-selector/port/interval/owner when enabled, default port when unset, CRD-absent (NoMatchError) treated as a soft skip with no panic, delete-on-disable, and NotFound-on-delete swallowed. Directly validates the headline 'never fails reconcile when the CRD is absent'. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Xavier Lange --- internal/controller/podmonitor_test.go | 197 +++++++++++++++++++++++++ 1 file changed, 197 insertions(+) create mode 100644 internal/controller/podmonitor_test.go diff --git a/internal/controller/podmonitor_test.go b/internal/controller/podmonitor_test.go new file mode 100644 index 00000000..7ba4dafc --- /dev/null +++ b/internal/controller/podmonitor_test.go @@ -0,0 +1,197 @@ +/* +Copyright 2025. + +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 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + 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/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + ecv1alpha1 "go.etcd.io/etcd-operator/api/v1alpha1" +) + +// podMonitorTestScheme returns a scheme that knows the EtcdCluster types and the +// unstructured PodMonitor GVK so the fake client can store and retrieve the +// PodMonitor object reconcilePodMonitor creates. +func podMonitorTestScheme() *runtime.Scheme { + s := runtime.NewScheme() + _ = ecv1alpha1.AddToScheme(s) + s.AddKnownTypeWithName(podMonitorGVK, &unstructured.Unstructured{}) + listGVK := podMonitorGVK + listGVK.Kind += "List" + s.AddKnownTypeWithName(listGVK, &unstructured.UnstructuredList{}) + return s +} + +// getPodMonitor fetches the cluster's PodMonitor via the unstructured GVK. +func getPodMonitor(ctx context.Context, c client.Client, ec *ecv1alpha1.EtcdCluster) (*unstructured.Unstructured, error) { + u := newPodMonitorObject(ec) + err := c.Get(ctx, types.NamespacedName{Namespace: ec.Namespace, Name: ec.Name}, u) + return u, err +} + +func newMetricsCluster(podMonitorEnabled bool, port, interval string) *ecv1alpha1.EtcdCluster { + ec := &ecv1alpha1.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "etcd-x", Namespace: "ns1"}, + Spec: ecv1alpha1.EtcdClusterSpec{Size: 3}, + } + enabled := true + ec.Spec.Metrics = &ecv1alpha1.MetricsSpec{ + Enabled: &enabled, + PodMonitor: &ecv1alpha1.PodMonitorSpec{ + Enabled: podMonitorEnabled, + Port: port, + Interval: interval, + }, + } + return ec +} + +// TestReconcilePodMonitor_CreatesWhenEnabled asserts the PodMonitor is created +// with the expected pod selector, port, interval, and controller owner ref. +func TestReconcilePodMonitor_CreatesWhenEnabled(t *testing.T) { + scheme := podMonitorTestScheme() + ec := newMetricsCluster(true, "client", "45s") + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(ec).Build() + r := &EtcdClusterReconciler{Client: fakeClient, Scheme: scheme} + + r.reconcilePodMonitor(context.Background(), ec) + + pm, err := getPodMonitor(context.Background(), fakeClient, ec) + require.NoError(t, err, "expected PodMonitor to be created") + + sel, found, err := unstructured.NestedStringMap(pm.Object, "spec", "selector", "matchLabels") + require.NoError(t, err) + require.True(t, found, "selector.matchLabels missing") + assert.Equal(t, ec.Name, sel["app"]) + assert.Equal(t, ec.Name, sel["controller"]) + + endpoints, found, err := unstructured.NestedSlice(pm.Object, "spec", "podMetricsEndpoints") + require.NoError(t, err) + require.True(t, found, "podMetricsEndpoints missing") + require.Len(t, endpoints, 1) + endpoint := endpoints[0].(map[string]any) + assert.Equal(t, "client", endpoint["port"]) + assert.Equal(t, "/metrics", endpoint["path"]) + assert.Equal(t, "45s", endpoint["interval"]) + + owners := pm.GetOwnerReferences() + require.Len(t, owners, 1) + assert.Equal(t, ec.Name, owners[0].Name) + require.NotNil(t, owners[0].Controller) + assert.True(t, *owners[0].Controller) +} + +// TestReconcilePodMonitor_DefaultsPortWhenUnset asserts an unset port falls back +// to the default scrape port. +func TestReconcilePodMonitor_DefaultsPortWhenUnset(t *testing.T) { + scheme := podMonitorTestScheme() + ec := newMetricsCluster(true, "", "") + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(ec).Build() + r := &EtcdClusterReconciler{Client: fakeClient, Scheme: scheme} + + r.reconcilePodMonitor(context.Background(), ec) + + pm, err := getPodMonitor(context.Background(), fakeClient, ec) + require.NoError(t, err) + endpoints, _, err := unstructured.NestedSlice(pm.Object, "spec", "podMetricsEndpoints") + require.NoError(t, err) + require.Len(t, endpoints, 1) + endpoint := endpoints[0].(map[string]any) + assert.Equal(t, defaultPodMonitorPort, endpoint["port"]) + // interval omitted when unset. + _, hasInterval := endpoint["interval"] + assert.False(t, hasInterval, "interval should be omitted when unset") +} + +// TestReconcilePodMonitor_CRDAbsentIsSoftSkip asserts that a missing PodMonitor +// CRD (NoMatchError on the underlying Get) is treated as a soft skip: the +// reconcile does not panic and creates nothing. +func TestReconcilePodMonitor_CRDAbsentIsSoftSkip(t *testing.T) { + scheme := podMonitorTestScheme() + ec := newMetricsCluster(true, "client", "30s") + + noMatch := &meta.NoKindMatchError{GroupKind: schema.GroupKind{Group: podMonitorGVK.Group, Kind: podMonitorGVK.Kind}} + getCalled := false + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(ec). + WithInterceptorFuncs(interceptor.Funcs{ + Get: func(ctx context.Context, c client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if obj.GetObjectKind().GroupVersionKind() == podMonitorGVK { + getCalled = true + return noMatch + } + return c.Get(ctx, key, obj, opts...) + }, + }).Build() + r := &EtcdClusterReconciler{Client: fakeClient, Scheme: scheme} + + // Must not panic and must not surface the error (reconcilePodMonitor returns void). + assert.NotPanics(t, func() { + r.reconcilePodMonitor(context.Background(), ec) + }) + assert.True(t, getCalled, "expected a PodMonitor Get attempt") +} + +// TestReconcilePodMonitor_DeletesWhenDisabled asserts a previously created +// PodMonitor is removed once the feature is disabled. +func TestReconcilePodMonitor_DeletesWhenDisabled(t *testing.T) { + scheme := podMonitorTestScheme() + ec := newMetricsCluster(true, "client", "30s") + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(ec).Build() + r := &EtcdClusterReconciler{Client: fakeClient, Scheme: scheme} + + // Create it first. + r.reconcilePodMonitor(context.Background(), ec) + if _, err := getPodMonitor(context.Background(), fakeClient, ec); err != nil { + t.Fatalf("precondition: PodMonitor not created: %v", err) + } + + // Now disable and reconcile again. + ec.Spec.Metrics.PodMonitor.Enabled = false + r.reconcilePodMonitor(context.Background(), ec) + + _, err := getPodMonitor(context.Background(), fakeClient, ec) + assert.True(t, apierrors.IsNotFound(err), "expected PodMonitor to be deleted, got err=%v", err) +} + +// TestReconcilePodMonitor_DeleteNotFoundSwallowed asserts that disabling the +// feature when no PodMonitor exists is a no-op (the NotFound on delete is +// swallowed and nothing panics). +func TestReconcilePodMonitor_DeleteNotFoundSwallowed(t *testing.T) { + scheme := podMonitorTestScheme() + ec := newMetricsCluster(false, "client", "30s") + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(ec).Build() + r := &EtcdClusterReconciler{Client: fakeClient, Scheme: scheme} + + assert.NotPanics(t, func() { + r.reconcilePodMonitor(context.Background(), ec) + }) + _, err := getPodMonitor(context.Background(), fakeClient, ec) + assert.True(t, apierrors.IsNotFound(err), "expected no PodMonitor to exist") +} From f46e552863f1d67d1b37cd7462ba5fc7e2fc6cc1 Mon Sep 17 00:00:00 2001 From: Xavier Lange Date: Thu, 18 Jun 2026 20:18:20 -0400 Subject: [PATCH 6/8] feat(metrics): ship dashboard, alerts, tls_ready/available gauges + docs Completes the observability surface for the per-cluster domain metrics: - Add etcd_operator_cluster_available (mirrors the Available status condition) and etcd_operator_cluster_tls_ready (only present for TLS-configured clusters; 1 when the TLS-secured cluster is Available), wired into RecordClusterMetrics and DeleteClusterMetrics with unit tests using prometheus testutil. The has_quorum gauge and leader_changes_total counter already existed and remain the recovery feature's signals. - Add config/prometheus/alerts.yaml (PrometheusRule) with quorum-lost, no-leader, members-not-ready, tls-not-ready, reconcile-errors, leader-flapping, and not-available alerts over the domain metrics, and wire it into the config/prometheus kustomization alongside the existing ServiceMonitor. - Add config/grafana/etcd-operator-dashboard.json and a ConfigMap- generator kustomization (grafana_dashboard sidecar label) covering cluster health, membership, and reconcile stability with datasource/ namespace/cluster template variables. - Add docs/metrics.md documenting every metric, its labels, the spec.metrics CR knobs, the ServiceMonitor vs PodMonitor scrape paths, and the shipped dashboard/alert assets. Verified: go build/vet across the module, metrics unit tests, and the controller envtest suite pass; kustomize build renders config/prometheus and config/grafana, and the prometheus component composes under the default overlay's namespace/prefix transforms. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Xavier Lange --- config/grafana/etcd-operator-dashboard.json | 314 ++++++++++++++++++++ config/grafana/kustomization.yaml | 23 ++ config/prometheus/alerts.yaml | 129 ++++++++ config/prometheus/kustomization.yaml | 1 + docs/metrics.md | 142 +++++++++ internal/metrics/metrics.go | 46 ++- internal/metrics/metrics_test.go | 89 +++++- 7 files changed, 742 insertions(+), 2 deletions(-) create mode 100644 config/grafana/etcd-operator-dashboard.json create mode 100644 config/grafana/kustomization.yaml create mode 100644 config/prometheus/alerts.yaml create mode 100644 docs/metrics.md diff --git a/config/grafana/etcd-operator-dashboard.json b/config/grafana/etcd-operator-dashboard.json new file mode 100644 index 00000000..c7c90ca2 --- /dev/null +++ b/config/grafana/etcd-operator-dashboard.json @@ -0,0 +1,314 @@ +{ + "annotations": { + "list": [ + { + "builtIn": 1, + "datasource": { + "type": "grafana", + "uid": "-- Grafana --" + }, + "enable": true, + "hide": true, + "iconColor": "rgba(0, 211, 255, 1)", + "name": "Annotations & Alerts", + "type": "dashboard" + } + ] + }, + "description": "Per-cluster domain metrics exported by the etcd-operator (subsystem etcd_operator_cluster_*).", + "editable": true, + "fiscalYearStartMonth": 0, + "graphTooltip": 1, + "links": [], + "liveNow": false, + "panels": [ + { + "collapsed": false, + "gridPos": { "h": 1, "w": 24, "x": 0, "y": 0 }, + "id": 100, + "title": "Cluster health", + "type": "row" + }, + { + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "description": "1 when a strict majority of voting members is healthy.", + "fieldConfig": { + "defaults": { + "mappings": [ + { "options": { "0": { "color": "red", "text": "No quorum" } }, "type": "value" }, + { "options": { "1": { "color": "green", "text": "Quorum" } }, "type": "value" } + ], + "thresholds": { "mode": "absolute", "steps": [ { "color": "red", "value": null }, { "color": "green", "value": 1 } ] } + } + }, + "gridPos": { "h": 6, "w": 6, "x": 0, "y": 1 }, + "id": 1, + "options": { + "colorMode": "background", + "graphMode": "none", + "justifyMode": "auto", + "orientation": "auto", + "reduceOptions": { "calcs": ["lastNotNull"], "fields": "", "values": false }, + "textMode": "auto" + }, + "pluginVersion": "10.0.0", + "targets": [ + { + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "expr": "etcd_operator_cluster_has_quorum{namespace=~\"$namespace\", name=~\"$cluster\"}", + "legendFormat": "{{namespace}}/{{name}}", + "refId": "A" + } + ], + "title": "Has quorum", + "type": "stat" + }, + { + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "description": "1 when the operator knows the current leader.", + "fieldConfig": { + "defaults": { + "mappings": [ + { "options": { "0": { "color": "red", "text": "No leader" } }, "type": "value" }, + { "options": { "1": { "color": "green", "text": "Leader" } }, "type": "value" } + ], + "thresholds": { "mode": "absolute", "steps": [ { "color": "red", "value": null }, { "color": "green", "value": 1 } ] } + } + }, + "gridPos": { "h": 6, "w": 6, "x": 6, "y": 1 }, + "id": 2, + "options": { + "colorMode": "background", + "graphMode": "none", + "justifyMode": "auto", + "orientation": "auto", + "reduceOptions": { "calcs": ["lastNotNull"], "fields": "", "values": false }, + "textMode": "auto" + }, + "pluginVersion": "10.0.0", + "targets": [ + { + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "expr": "etcd_operator_cluster_has_leader{namespace=~\"$namespace\", name=~\"$cluster\"}", + "legendFormat": "{{namespace}}/{{name}}", + "refId": "A" + } + ], + "title": "Has leader", + "type": "stat" + }, + { + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "description": "Available condition reported by the operator.", + "fieldConfig": { + "defaults": { + "mappings": [ + { "options": { "0": { "color": "red", "text": "Not available" } }, "type": "value" }, + { "options": { "1": { "color": "green", "text": "Available" } }, "type": "value" } + ], + "thresholds": { "mode": "absolute", "steps": [ { "color": "red", "value": null }, { "color": "green", "value": 1 } ] } + } + }, + "gridPos": { "h": 6, "w": 6, "x": 12, "y": 1 }, + "id": 3, + "options": { + "colorMode": "background", + "graphMode": "none", + "justifyMode": "auto", + "orientation": "auto", + "reduceOptions": { "calcs": ["lastNotNull"], "fields": "", "values": false }, + "textMode": "auto" + }, + "pluginVersion": "10.0.0", + "targets": [ + { + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "expr": "etcd_operator_cluster_available{namespace=~\"$namespace\", name=~\"$cluster\"}", + "legendFormat": "{{namespace}}/{{name}}", + "refId": "A" + } + ], + "title": "Available", + "type": "stat" + }, + { + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "description": "For TLS-configured clusters, whether the TLS-secured cluster is Available. No series for plaintext clusters.", + "fieldConfig": { + "defaults": { + "mappings": [ + { "options": { "0": { "color": "red", "text": "TLS not ready" } }, "type": "value" }, + { "options": { "1": { "color": "green", "text": "TLS ready" } }, "type": "value" } + ], + "thresholds": { "mode": "absolute", "steps": [ { "color": "red", "value": null }, { "color": "green", "value": 1 } ] } + } + }, + "gridPos": { "h": 6, "w": 6, "x": 18, "y": 1 }, + "id": 4, + "options": { + "colorMode": "background", + "graphMode": "none", + "justifyMode": "auto", + "orientation": "auto", + "reduceOptions": { "calcs": ["lastNotNull"], "fields": "", "values": false }, + "textMode": "auto" + }, + "pluginVersion": "10.0.0", + "targets": [ + { + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "expr": "etcd_operator_cluster_tls_ready{namespace=~\"$namespace\", name=~\"$cluster\"}", + "legendFormat": "{{namespace}}/{{name}}", + "refId": "A" + } + ], + "title": "TLS ready", + "type": "stat" + }, + { + "collapsed": false, + "gridPos": { "h": 1, "w": 24, "x": 0, "y": 7 }, + "id": 101, + "title": "Membership", + "type": "row" + }, + { + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "description": "Desired vs registered vs ready vs healthy member counts.", + "fieldConfig": { + "defaults": { + "custom": { "drawStyle": "line", "fillOpacity": 10, "lineWidth": 2, "stepInterval": "" }, + "unit": "short" + } + }, + "gridPos": { "h": 8, "w": 12, "x": 0, "y": 8 }, + "id": 5, + "options": { "legend": { "calcs": ["lastNotNull"], "displayMode": "table", "placement": "bottom", "showLegend": true }, "tooltip": { "mode": "multi", "sort": "none" } }, + "targets": [ + { "datasource": { "type": "prometheus", "uid": "${datasource}" }, "expr": "etcd_operator_cluster_member_count_desired{namespace=~\"$namespace\", name=~\"$cluster\"}", "legendFormat": "desired {{namespace}}/{{name}}", "refId": "A" }, + { "datasource": { "type": "prometheus", "uid": "${datasource}" }, "expr": "etcd_operator_cluster_member_count{namespace=~\"$namespace\", name=~\"$cluster\"}", "legendFormat": "registered {{namespace}}/{{name}}", "refId": "B" }, + { "datasource": { "type": "prometheus", "uid": "${datasource}" }, "expr": "etcd_operator_cluster_member_count_ready{namespace=~\"$namespace\", name=~\"$cluster\"}", "legendFormat": "ready {{namespace}}/{{name}}", "refId": "C" }, + { "datasource": { "type": "prometheus", "uid": "${datasource}" }, "expr": "etcd_operator_cluster_member_count_healthy{namespace=~\"$namespace\", name=~\"$cluster\"}", "legendFormat": "healthy {{namespace}}/{{name}}", "refId": "D" } + ], + "title": "Member counts", + "type": "timeseries" + }, + { + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "description": "Number of members currently acting as learners (non-voting).", + "fieldConfig": { "defaults": { "custom": { "drawStyle": "line", "fillOpacity": 10, "lineWidth": 2 }, "unit": "short" } }, + "gridPos": { "h": 8, "w": 12, "x": 12, "y": 8 }, + "id": 6, + "options": { "legend": { "calcs": ["lastNotNull"], "displayMode": "table", "placement": "bottom", "showLegend": true }, "tooltip": { "mode": "multi", "sort": "none" } }, + "targets": [ + { "datasource": { "type": "prometheus", "uid": "${datasource}" }, "expr": "etcd_operator_cluster_learner_count{namespace=~\"$namespace\", name=~\"$cluster\"}", "legendFormat": "learners {{namespace}}/{{name}}", "refId": "A" } + ], + "title": "Learner count", + "type": "timeseries" + }, + { + "collapsed": false, + "gridPos": { "h": 1, "w": 24, "x": 0, "y": 16 }, + "id": 102, + "title": "Stability & reconcile", + "type": "row" + }, + { + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "description": "Rate of observed leader changes. Sustained non-zero indicates election churn.", + "fieldConfig": { "defaults": { "custom": { "drawStyle": "line", "fillOpacity": 10, "lineWidth": 2 }, "unit": "short" } }, + "gridPos": { "h": 8, "w": 8, "x": 0, "y": 17 }, + "id": 7, + "options": { "legend": { "calcs": ["sum"], "displayMode": "table", "placement": "bottom", "showLegend": true }, "tooltip": { "mode": "multi", "sort": "none" } }, + "targets": [ + { "datasource": { "type": "prometheus", "uid": "${datasource}" }, "expr": "rate(etcd_operator_cluster_leader_changes_total{namespace=~\"$namespace\", name=~\"$cluster\"}[5m])", "legendFormat": "{{namespace}}/{{name}}", "refId": "A" } + ], + "title": "Leader changes (rate)", + "type": "timeseries" + }, + { + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "description": "Rate of reconcile loops that returned an error.", + "fieldConfig": { "defaults": { "custom": { "drawStyle": "line", "fillOpacity": 10, "lineWidth": 2 }, "unit": "short" } }, + "gridPos": { "h": 8, "w": 8, "x": 8, "y": 17 }, + "id": 8, + "options": { "legend": { "calcs": ["sum"], "displayMode": "table", "placement": "bottom", "showLegend": true }, "tooltip": { "mode": "multi", "sort": "none" } }, + "targets": [ + { "datasource": { "type": "prometheus", "uid": "${datasource}" }, "expr": "rate(etcd_operator_cluster_reconcile_errors_total{namespace=~\"$namespace\", name=~\"$cluster\"}[5m])", "legendFormat": "{{namespace}}/{{name}}", "refId": "A" } + ], + "title": "Reconcile errors (rate)", + "type": "timeseries" + }, + { + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "description": "p50/p90/p99 reconcile duration from the histogram.", + "fieldConfig": { "defaults": { "custom": { "drawStyle": "line", "fillOpacity": 10, "lineWidth": 2 }, "unit": "s" } }, + "gridPos": { "h": 8, "w": 8, "x": 16, "y": 17 }, + "id": 9, + "options": { "legend": { "calcs": ["lastNotNull"], "displayMode": "table", "placement": "bottom", "showLegend": true }, "tooltip": { "mode": "multi", "sort": "none" } }, + "targets": [ + { "datasource": { "type": "prometheus", "uid": "${datasource}" }, "expr": "histogram_quantile(0.50, sum(rate(etcd_operator_cluster_reconcile_duration_seconds_bucket{namespace=~\"$namespace\", name=~\"$cluster\"}[5m])) by (le))", "legendFormat": "p50", "refId": "A" }, + { "datasource": { "type": "prometheus", "uid": "${datasource}" }, "expr": "histogram_quantile(0.90, sum(rate(etcd_operator_cluster_reconcile_duration_seconds_bucket{namespace=~\"$namespace\", name=~\"$cluster\"}[5m])) by (le))", "legendFormat": "p90", "refId": "B" }, + { "datasource": { "type": "prometheus", "uid": "${datasource}" }, "expr": "histogram_quantile(0.99, sum(rate(etcd_operator_cluster_reconcile_duration_seconds_bucket{namespace=~\"$namespace\", name=~\"$cluster\"}[5m])) by (le))", "legendFormat": "p99", "refId": "C" } + ], + "title": "Reconcile duration quantiles", + "type": "timeseries" + } + ], + "refresh": "30s", + "schemaVersion": 38, + "tags": ["etcd", "etcd-operator"], + "templating": { + "list": [ + { + "current": {}, + "hide": 0, + "includeAll": false, + "label": "Datasource", + "name": "datasource", + "options": [], + "query": "prometheus", + "refresh": 1, + "regex": "", + "type": "datasource" + }, + { + "current": {}, + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "definition": "label_values(etcd_operator_cluster_has_quorum, namespace)", + "hide": 0, + "includeAll": true, + "label": "Namespace", + "multi": true, + "name": "namespace", + "query": { "query": "label_values(etcd_operator_cluster_has_quorum, namespace)", "refId": "StandardVariableQuery" }, + "refresh": 2, + "regex": "", + "sort": 1, + "type": "query" + }, + { + "current": {}, + "datasource": { "type": "prometheus", "uid": "${datasource}" }, + "definition": "label_values(etcd_operator_cluster_has_quorum{namespace=~\"$namespace\"}, name)", + "hide": 0, + "includeAll": true, + "label": "Cluster", + "multi": true, + "name": "cluster", + "query": { "query": "label_values(etcd_operator_cluster_has_quorum{namespace=~\"$namespace\"}, name)", "refId": "StandardVariableQuery" }, + "refresh": 2, + "regex": "", + "sort": 1, + "type": "query" + } + ] + }, + "time": { "from": "now-6h", "to": "now" }, + "timepicker": {}, + "timezone": "", + "title": "etcd-operator / Cluster domain metrics", + "uid": "etcd-operator-domain", + "version": 1, + "weekStart": "" +} diff --git a/config/grafana/kustomization.yaml b/config/grafana/kustomization.yaml new file mode 100644 index 00000000..e2676176 --- /dev/null +++ b/config/grafana/kustomization.yaml @@ -0,0 +1,23 @@ +# Ships the etcd-operator domain-metrics Grafana dashboard as a ConfigMap that +# the Grafana dashboard sidecar (label grafana_dashboard: "1") auto-imports. +# +# This component is opt-in: it is not referenced by config/default. Apply it on +# its own (kustomize build config/grafana | kubectl apply -f -) or add it to an +# overlay in the namespace where your Grafana sidecar watches for dashboards. +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + +generatorOptions: + # Keep a stable ConfigMap name (no content hash suffix) so the sidecar and any + # GitOps tooling can reference it deterministically. + disableNameSuffixHash: true + +configMapGenerator: + - name: etcd-operator-grafana-dashboard + files: + - etcd-operator-dashboard.json + options: + labels: + grafana_dashboard: "1" + app.kubernetes.io/name: etcd-operator + app.kubernetes.io/managed-by: kustomize diff --git a/config/prometheus/alerts.yaml b/config/prometheus/alerts.yaml new file mode 100644 index 00000000..47cb8fa1 --- /dev/null +++ b/config/prometheus/alerts.yaml @@ -0,0 +1,129 @@ +# PrometheusRule defining alerts over the etcd-operator's per-cluster domain +# metrics (subsystem etcd_operator_cluster_*, exported on the operator's own +# /metrics endpoint and labelled by namespace,name). These rules are opt-in: +# they are only included when you uncomment the prometheus component in +# config/default/kustomization.yaml and run against a cluster with the +# prometheus-operator PrometheusRule CRD installed. +# +# See docs/metrics.md for the full metric reference. The series are produced by +# the operator process, so a single rule covers every EtcdCluster it manages. +apiVersion: monitoring.coreos.com/v1 +kind: PrometheusRule +metadata: + labels: + control-plane: controller-manager + app.kubernetes.io/name: etcd-operator + app.kubernetes.io/managed-by: kustomize + # Allow operators to select these rules with a Prometheus ruleSelector. + role: alert-rules + name: etcd-operator-domain-rules + namespace: system +spec: + groups: + - name: etcd-operator.cluster.rules + rules: + # Quorum lost: the operator no longer sees a strict majority of voting + # members healthy. This is the recovery feature's primary signal. + - alert: EtcdClusterQuorumLost + expr: etcd_operator_cluster_has_quorum == 0 + for: 1m + labels: + severity: critical + annotations: + summary: "EtcdCluster {{ $labels.namespace }}/{{ $labels.name }} has lost quorum" + description: >- + The etcd-operator reports has_quorum=0 for EtcdCluster + {{ $labels.namespace }}/{{ $labels.name }} for more than 1 minute. + The cluster cannot make progress until a majority of voting members + is healthy again. + + # No leader: a known leader is absent. Brief leaderless windows during an + # election are normal, so this only fires after a sustained gap. + - alert: EtcdClusterNoLeader + expr: etcd_operator_cluster_has_leader == 0 + for: 2m + labels: + severity: critical + annotations: + summary: "EtcdCluster {{ $labels.namespace }}/{{ $labels.name }} has no leader" + description: >- + The etcd-operator reports has_leader=0 for EtcdCluster + {{ $labels.namespace }}/{{ $labels.name }} for more than 2 minutes. + A persistent leaderless state usually accompanies quorum loss or a + network partition. + + # Members not ready: fewer pods are Ready than the cluster desires. + - alert: EtcdClusterMembersNotReady + expr: >- + etcd_operator_cluster_member_count_ready + < etcd_operator_cluster_member_count_desired + for: 10m + labels: + severity: warning + annotations: + summary: "EtcdCluster {{ $labels.namespace }}/{{ $labels.name }} has members not ready" + description: >- + EtcdCluster {{ $labels.namespace }}/{{ $labels.name }} has fewer + ready members than desired (shortfall {{ $value }}) for more than + 10 minutes. A single replica may roll briefly; a sustained shortfall + indicates a stuck or crash-looping member. + + # TLS not ready: TLS is configured but the cluster is not Available. The + # tls_ready series only exists for TLS-configured clusters, so this never + # fires on plaintext clusters. + - alert: EtcdClusterTLSNotReady + expr: etcd_operator_cluster_tls_ready == 0 + for: 5m + labels: + severity: warning + annotations: + summary: "EtcdCluster {{ $labels.namespace }}/{{ $labels.name }} TLS-secured cluster not ready" + description: >- + EtcdCluster {{ $labels.namespace }}/{{ $labels.name }} is configured + for TLS but has not been Available for more than 5 minutes. Check the + cert-manager Certificate / TLS secrets and member readiness. + + # Reconcile errors: the operator's reconcile loop for this cluster is + # returning errors. A few transient errors are tolerable; a sustained + # error rate means reconciliation is wedged. + - alert: EtcdOperatorReconcileErrors + expr: >- + increase(etcd_operator_cluster_reconcile_errors_total[10m]) > 0 + for: 15m + labels: + severity: warning + annotations: + summary: "etcd-operator reconcile errors for {{ $labels.namespace }}/{{ $labels.name }}" + description: >- + The etcd-operator reconcile loop for EtcdCluster + {{ $labels.namespace }}/{{ $labels.name }} has returned errors + continuously for 15 minutes. Inspect the operator logs. + + # Leader flapping: repeated leader changes indicate instability (flaky + # network, overloaded members) even while quorum is technically held. + - alert: EtcdClusterLeaderFlapping + expr: >- + increase(etcd_operator_cluster_leader_changes_total[15m]) > 3 + for: 0m + labels: + severity: warning + annotations: + summary: "EtcdCluster {{ $labels.namespace }}/{{ $labels.name }} leader is flapping" + description: >- + EtcdCluster {{ $labels.namespace }}/{{ $labels.name }} observed + {{ $value }} leader changes in the last 15 minutes. Frequent + elections degrade write availability. + + # Cluster not Available: the operator's Available condition has been false + # for a sustained period regardless of TLS. + - alert: EtcdClusterNotAvailable + expr: etcd_operator_cluster_available == 0 + for: 10m + labels: + severity: warning + annotations: + summary: "EtcdCluster {{ $labels.namespace }}/{{ $labels.name }} is not Available" + description: >- + The Available condition for EtcdCluster + {{ $labels.namespace }}/{{ $labels.name }} has been false for more + than 10 minutes. diff --git a/config/prometheus/kustomization.yaml b/config/prometheus/kustomization.yaml index ed137168..7285e7b4 100644 --- a/config/prometheus/kustomization.yaml +++ b/config/prometheus/kustomization.yaml @@ -1,2 +1,3 @@ resources: - monitor.yaml +- alerts.yaml diff --git a/docs/metrics.md b/docs/metrics.md new file mode 100644 index 00000000..06c57dcd --- /dev/null +++ b/docs/metrics.md @@ -0,0 +1,142 @@ +# Metrics + +The etcd-operator exports a set of **per-cluster domain metrics** describing the +state of every `EtcdCluster` it reconciles. These are emitted by the operator +process itself (not by etcd) and are served on the operator's existing +`/metrics` endpoint, alongside the standard controller-runtime and Go runtime +metrics. They are registered against the controller-runtime global Prometheus +registry, so no extra HTTP plumbing is required. + +This document lists every custom metric, its labels, the CR knobs that control +emission and scraping, and the shipped dashboard/alert assets. + +## Metric reference + +All custom metrics share the subsystem prefix `etcd_operator_cluster_` and are +labelled with the cluster's `namespace` and `name`, so a single operator +managing many clusters produces one time series per `(namespace, name)`. + +| Metric | Type | Labels | Meaning | +| --- | --- | --- | --- | +| `etcd_operator_cluster_member_count_desired` | gauge | `namespace`, `name` | Desired members, i.e. `spec.size`. | +| `etcd_operator_cluster_member_count` | gauge | `namespace`, `name` | Members registered in the etcd member-list API (`status.memberCount`). | +| `etcd_operator_cluster_member_count_ready` | gauge | `namespace`, `name` | Ready etcd pods (StatefulSet `readyReplicas`). | +| `etcd_operator_cluster_member_count_healthy` | gauge | `namespace`, `name` | Members reported healthy in cluster status. | +| `etcd_operator_cluster_learner_count` | gauge | `namespace`, `name` | Members currently acting as learners (non-voting). | +| `etcd_operator_cluster_has_quorum` | gauge | `namespace`, `name` | `1` when a strict majority of **voting** members is healthy, else `0`. | +| `etcd_operator_cluster_has_leader` | gauge | `namespace`, `name` | `1` when the operator knows the current leader, else `0`. | +| `etcd_operator_cluster_available` | gauge | `namespace`, `name` | `1` when the `Available` status condition is `True`, else `0`. | +| `etcd_operator_cluster_tls_enabled` | gauge | `namespace`, `name` | `1` when the cluster is configured for TLS (`spec.tls` set), else `0`. | +| `etcd_operator_cluster_tls_ready` | gauge | `namespace`, `name` | **Only present for TLS-configured clusters.** `1` when the TLS-secured cluster is `Available`, `0` when configured but not yet Available. | +| `etcd_operator_cluster_leader_changes_total` | counter | `namespace`, `name` | Leader changes observed by the operator. The first observation and recovery from a leaderless window are **not** counted. | +| `etcd_operator_cluster_reconcile_duration_seconds` | histogram | `namespace`, `name` | Wall-clock duration of reconcile loops (default Prometheus buckets). | +| `etcd_operator_cluster_reconcile_errors_total` | counter | `namespace`, `name` | Reconcile loops that returned an error. | + +### Notes on semantics + +- **Quorum is computed over voting members only.** Learners do not participate + in the raft quorum, so a healthy learner can never make a degraded voting set + look quorate. `has_quorum` uses `voting = memberCount - learnerCount` and + requires `healthyVoting >= floor(voting/2) + 1`. +- **`has_leader` reflects the current reconcile.** When no leader is known the + operator clears `status.leaderID`, so this gauge drops to `0` during a + leaderless window rather than reporting a stale leader. +- **`tls_ready` exists only when TLS is configured.** Plaintext clusters emit no + `tls_ready` series, so alerts on "TLS configured but not ready" never fire for + clusters that do not use TLS. Removing `spec.tls` drops the series. +- **`leader_changes_total` ignores the first/leaderless transitions.** Operator + restarts and transient leaderless windows do not inflate the counter; only a + transition between two known, distinct leaders increments it. +- **Cardinality is bounded.** When an `EtcdCluster` is deleted (or has metrics + disabled), the operator drops all of that cluster's series so stale clusters + do not accumulate time series. + +## Controlling emission: the `spec.metrics` knob + +Per-cluster metric emission is configured through the optional `spec.metrics` +block on `EtcdCluster`: + +```yaml +apiVersion: operator.etcd.io/v1alpha1 +kind: EtcdCluster +metadata: + name: my-etcd +spec: + size: 3 + version: v3.5.21 + metrics: + # Export the operator's per-cluster domain metrics for this cluster. + # Defaults to true when the whole metrics block (or this field) is omitted. + enabled: true + podMonitor: + # Create a prometheus-operator PodMonitor selecting this cluster's etcd + # member pods so Prometheus also scrapes etcd's own /metrics endpoint. + enabled: true + interval: "30s" # optional; prometheus-operator default when empty + port: "client" # optional; pod port name, defaults to "client" +``` + +| Field | Default | Effect | +| --- | --- | --- | +| `spec.metrics` | unset | When omitted, the operator still exports per-cluster domain metrics; no PodMonitor is created. | +| `spec.metrics.enabled` | `true` | When `false`, the operator drops this cluster's domain-metric series from `/metrics`. | +| `spec.metrics.podMonitor.enabled` | `false` | When `true` (and metrics enabled), the operator creates/maintains a `PodMonitor` for the etcd member pods. | +| `spec.metrics.podMonitor.interval` | prometheus default | Scrape interval for the etcd member pods. | +| `spec.metrics.podMonitor.port` | `client` | Name of the pod port exposing etcd's `/metrics`. | + +`PodMonitorEnabled()` is `true` only when metrics are enabled **and** +`podMonitor.enabled` is `true`, so disabling metrics also disables the +PodMonitor. The PodMonitor is owned by the `EtcdCluster` (garbage-collected on +delete). If the `PodMonitor` CRD (prometheus-operator) is not installed, the +operator logs and skips PodMonitor reconciliation without failing the rest of +the reconcile. + +A ready-to-apply example lives at +[`config/samples/sample_metrics_etcdcluster.yaml`](../config/samples/sample_metrics_etcdcluster.yaml). + +## Scraping the operator's own metrics + +The operator's `/metrics` endpoint (where the `etcd_operator_cluster_*` series +live) is scraped via the `ServiceMonitor` in +[`config/prometheus/monitor.yaml`](../config/prometheus/monitor.yaml), which +selects the controller-manager metrics Service. Enable it by uncommenting the +`../prometheus` line in `config/default/kustomization.yaml` (this also pulls in +the alert rules below). By default the endpoint serves HTTPS on `:8443` behind +authn/authz; see the ServiceMonitor comments for TLS verification options. + +The two scrape paths are complementary: + +- **ServiceMonitor** (always, via `config/prometheus`) → scrapes the **operator** + for the domain metrics in this document. +- **PodMonitor** (opt-in, via `spec.metrics.podMonitor`) → scrapes the **etcd + member pods** for etcd's own built-in metrics. + +## Dashboard and alerts + +- **Grafana dashboard:** + [`config/grafana/etcd-operator-dashboard.json`](../config/grafana/etcd-operator-dashboard.json) + (uid `etcd-operator-domain`). It has rows for cluster health (quorum, leader, + available, TLS-ready), membership counts/learners, and stability/reconcile + (leader-change rate, reconcile error rate, reconcile-duration quantiles), with + `datasource`, `namespace`, and `cluster` template variables. + `kustomize build config/grafana` ships it as a ConfigMap labelled + `grafana_dashboard: "1"` for the Grafana dashboard sidecar. + +- **PrometheusRule alerts:** + [`config/prometheus/alerts.yaml`](../config/prometheus/alerts.yaml) + (`PrometheusRule/etcd-operator-domain-rules`): + + | Alert | Expr (summary) | For | Severity | + | --- | --- | --- | --- | + | `EtcdClusterQuorumLost` | `has_quorum == 0` | 1m | critical | + | `EtcdClusterNoLeader` | `has_leader == 0` | 2m | critical | + | `EtcdClusterMembersNotReady` | `member_count_ready < member_count_desired` | 10m | warning | + | `EtcdClusterTLSNotReady` | `tls_ready == 0` | 5m | warning | + | `EtcdOperatorReconcileErrors` | `increase(reconcile_errors_total[10m]) > 0` | 15m | warning | + | `EtcdClusterLeaderFlapping` | `increase(leader_changes_total[15m]) > 3` | — | warning | + | `EtcdClusterNotAvailable` | `available == 0` | 10m | warning | + + Both the ServiceMonitor and the alert rules are included by the + `config/prometheus` kustomization; uncommenting `../prometheus` in the default + overlay ships them together. They require the prometheus-operator + `ServiceMonitor`/`PrometheusRule` CRDs. diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 47eae0db..c9a64e85 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -31,11 +31,17 @@ import ( "sync" "github.com/prometheus/client_golang/prometheus" + "k8s.io/apimachinery/pkg/api/meta" ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics" ecv1alpha1 "go.etcd.io/etcd-operator/api/v1alpha1" ) +// conditionTypeAvailable is the EtcdCluster status condition that reports +// whether the cluster is ready to serve. It must match the Type set by the +// controller's updateConditions. +const conditionTypeAvailable = "Available" + const ( subsystem = "etcd_operator_cluster" @@ -101,6 +107,26 @@ var ( Help: "Whether TLS is configured for the etcd cluster (1) or not (0).", }, []string{labelNamespace, labelName}) + // TLSReady is exported only for clusters that configure TLS. It is 1 when the + // cluster is both TLS-configured and Available (its TLS-secured data plane is + // serving), and 0 when TLS is configured but the cluster is not yet Available. + // Clusters without TLS produce no tls_ready series, so alerts can target + // "tls configured but not ready" without firing on plaintext clusters. + TLSReady = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Subsystem: subsystem, + Name: "tls_ready", + Help: "For TLS-configured clusters, whether the TLS-secured cluster is Available (1) or not (0). Absent for non-TLS clusters.", + }, []string{labelNamespace, labelName}) + + // ClusterAvailable mirrors the EtcdCluster "Available" status condition: 1 + // when the condition is True, 0 otherwise. It is the operator's view of + // whether the cluster is ready to serve. + ClusterAvailable = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Subsystem: subsystem, + Name: "available", + Help: "Whether the EtcdCluster Available condition is True (1) or not (0).", + }, []string{labelNamespace, labelName}) + // LeaderChangesTotal counts observed changes of the cluster leader ID. LeaderChangesTotal = prometheus.NewCounterVec(prometheus.CounterOpts{ Subsystem: subsystem, @@ -136,6 +162,8 @@ func allCollectors() []prometheus.Collector { HasQuorum, HasLeader, TLSEnabled, + TLSReady, + ClusterAvailable, LeaderChangesTotal, ReconcileDurationSeconds, ReconcileErrorsTotal, @@ -239,7 +267,21 @@ func RecordClusterMetrics(cluster *ecv1alpha1.EtcdCluster) { HasQuorum.WithLabelValues(ns, name).Set(boolToFloat(hasQuorum(voting, healthyVoting))) HasLeader.WithLabelValues(ns, name).Set(boolToFloat(cluster.Status.LeaderID != "")) - TLSEnabled.WithLabelValues(ns, name).Set(boolToFloat(cluster.Spec.TLS != nil)) + + tlsEnabled := cluster.Spec.TLS != nil + TLSEnabled.WithLabelValues(ns, name).Set(boolToFloat(tlsEnabled)) + + available := meta.IsStatusConditionTrue(cluster.Status.Conditions, conditionTypeAvailable) + ClusterAvailable.WithLabelValues(ns, name).Set(boolToFloat(available)) + + // tls_ready only exists for TLS-configured clusters: it reflects whether the + // TLS-secured cluster is Available. For plaintext clusters we drop any prior + // series so alerts on "tls configured but not ready" never see them. + if tlsEnabled { + TLSReady.WithLabelValues(ns, name).Set(boolToFloat(available)) + } else { + TLSReady.DeleteLabelValues(ns, name) + } if tracker.observeLeader(ns, name, cluster.Status.LeaderID) { LeaderChangesTotal.WithLabelValues(ns, name).Inc() @@ -268,6 +310,8 @@ func DeleteClusterMetrics(namespace, name string) { HasQuorum.DeleteLabelValues(lvs...) HasLeader.DeleteLabelValues(lvs...) TLSEnabled.DeleteLabelValues(lvs...) + TLSReady.DeleteLabelValues(lvs...) + ClusterAvailable.DeleteLabelValues(lvs...) LeaderChangesTotal.DeleteLabelValues(lvs...) ReconcileErrorsTotal.DeleteLabelValues(lvs...) ReconcileDurationSeconds.DeleteLabelValues(lvs...) diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index 87d6a3f8..f635a197 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -39,6 +39,8 @@ func resetState() { HasQuorum.Reset() HasLeader.Reset() TLSEnabled.Reset() + TLSReady.Reset() + ClusterAvailable.Reset() LeaderChangesTotal.Reset() ReconcileErrorsTotal.Reset() ReconcileDurationSeconds.Reset() @@ -172,6 +174,89 @@ func TestRecordClusterMetrics_LearnerHealthyVotingQuorate(t *testing.T) { assertGauge(t, reg, "etcd_operator_cluster_has_quorum", 1, "ns1", "etcd-c2") } +// withAvailable sets the cluster's Available condition to the given status. +func withAvailable(c *ecv1alpha1.EtcdCluster, available bool) { + status := metav1.ConditionFalse + if available { + status = metav1.ConditionTrue + } + c.Status.Conditions = []metav1.Condition{{ + Type: "Available", + Status: status, + Reason: "Test", + LastTransitionTime: metav1.Now(), + }} +} + +func TestRecordClusterMetrics_AvailableGauge(t *testing.T) { + reg := newRegistry(t) + + c := newCluster("ns1", "etcd-av", 3) + c.Status.MemberCount = 3 + c.Status.Members = []ecv1alpha1.MemberStatus{{ID: "1", IsHealthy: true}} + + // No condition yet -> available is 0. + RecordClusterMetrics(c) + assertGauge(t, reg, "etcd_operator_cluster_available", 0, "ns1", "etcd-av") + + // Available=True -> 1. + withAvailable(c, true) + RecordClusterMetrics(c) + assertGauge(t, reg, "etcd_operator_cluster_available", 1, "ns1", "etcd-av") + + // Available=False -> back to 0. + withAvailable(c, false) + RecordClusterMetrics(c) + assertGauge(t, reg, "etcd_operator_cluster_available", 0, "ns1", "etcd-av") +} + +func TestRecordClusterMetrics_TLSReadyOnlyForTLSClusters(t *testing.T) { + reg := newRegistry(t) + + // Plaintext cluster: no tls_ready series should exist even when Available. + plain := newCluster("ns1", "etcd-plain", 3) + plain.Status.MemberCount = 3 + withAvailable(plain, true) + RecordClusterMetrics(plain) + if got := readSample(t, reg, "etcd_operator_cluster_tls_ready", "ns1", "etcd-plain"); got != 0 { + t.Errorf("expected no tls_ready series for plaintext cluster, got %v", got) + } + assertGauge(t, reg, "etcd_operator_cluster_tls_enabled", 0, "ns1", "etcd-plain") + + // TLS cluster, Available -> tls_ready=1. + tls := newCluster("ns1", "etcd-tls", 3) + tls.Spec.TLS = &ecv1alpha1.TLSCertificate{Provider: "auto"} + tls.Status.MemberCount = 3 + withAvailable(tls, true) + RecordClusterMetrics(tls) + assertGauge(t, reg, "etcd_operator_cluster_tls_enabled", 1, "ns1", "etcd-tls") + assertGauge(t, reg, "etcd_operator_cluster_tls_ready", 1, "ns1", "etcd-tls") + + // TLS cluster goes not-Available -> tls_ready=0 (the alertable state). + withAvailable(tls, false) + RecordClusterMetrics(tls) + assertGauge(t, reg, "etcd_operator_cluster_tls_ready", 0, "ns1", "etcd-tls") +} + +func TestRecordClusterMetrics_TLSReadyClearedWhenTLSRemoved(t *testing.T) { + reg := newRegistry(t) + + c := newCluster("ns1", "etcd-flip", 3) + c.Spec.TLS = &ecv1alpha1.TLSCertificate{Provider: "auto"} + c.Status.MemberCount = 3 + withAvailable(c, true) + RecordClusterMetrics(c) + assertGauge(t, reg, "etcd_operator_cluster_tls_ready", 1, "ns1", "etcd-flip") + + // TLS removed from spec: the tls_ready series must disappear so a "not ready" + // alert never fires on a cluster that no longer uses TLS. + c.Spec.TLS = nil + RecordClusterMetrics(c) + if got := readSample(t, reg, "etcd_operator_cluster_tls_ready", "ns1", "etcd-flip"); got != 0 { + t.Errorf("expected tls_ready series dropped after TLS removed, got %v", got) + } +} + func TestLeaderChangesCounter(t *testing.T) { reg := newRegistry(t) c := newCluster("ns1", "etcd-d", 3) @@ -218,9 +303,11 @@ func TestDeleteClusterMetrics(t *testing.T) { reg := newRegistry(t) c := newCluster("ns1", "etcd-f", 1) + c.Spec.TLS = &ecv1alpha1.TLSCertificate{Provider: "auto"} c.Status.MemberCount = 1 c.Status.LeaderID = "x" c.Status.Members = []ecv1alpha1.MemberStatus{{ID: "1", IsHealthy: true}} + withAvailable(c, true) RecordClusterMetrics(c) ObserveReconcile("ns1", "etcd-f", 0.1, errors.New("e")) @@ -232,7 +319,7 @@ func TestDeleteClusterMetrics(t *testing.T) { for _, coll := range []prometheus.Collector{ MemberCount, MemberCountDesired, HasQuorum, HasLeader, TLSEnabled, - LeaderChangesTotal, ReconcileErrorsTotal, + TLSReady, ClusterAvailable, LeaderChangesTotal, ReconcileErrorsTotal, } { if got := testutil.CollectAndCount(coll); got != 0 { t.Errorf("expected 0 series after delete, got %d", got) From 4ecf19e3695640bfbb69fe6d428b91d41ad96409 Mon Sep 17 00:00:00 2001 From: Xavier Lange Date: Thu, 18 Jun 2026 22:26:09 -0400 Subject: [PATCH 7/8] test(e2e): assert domain-metric values, PodMonitor, and poll the scrape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strengthen TestDomainMetricsExposed from a name-substring check (which the test's own comment admitted "passes purely from collector registration") into a true end-to-end proof of the feature's derived values. Improvements: - Value-assert the derived gauges for a healthy cluster instead of just metric names: member_count{,_ready,_healthy,_desired} == size, has_quorum == 1, has_leader == 1, learner_count == 0, tls_enabled == 0. has_quorum/has_leader are the dangerous booleans that require a real etcd round-trip + populated Status.Members/LeaderID, so they distinguish "operator fully reconciled etcd" from "operator never talked to etcd" (the previous member_count_desired check is sourced from spec.Size and needs no etcd round-trip at all). - Assert reconcile_duration_seconds_count >= 1 (the _count child series), proving the reconcile-timing hook actually fired; a histogram's # HELP line exists with zero observations so the name check proved nothing. - Assert tls_ready is ABSENT for the plaintext cluster — the documented invariant that keeps "tls configured but not ready" alerts from firing on plaintext clusters. - Assert the operator actually created a PodMonitor for the cluster with endpoint port "client" and interval "30s". reconcilePodMonitor swallows all errors (CRD-absent is a silent skip), so without observing the object the whole podMonitor half of the feature was dead setup. The e2e suite installs the prometheus-operator CRDs, so the object must exist. - Poll the scrape with wait.For instead of a one-shot read after STS readiness. The derived gauges are written by the async reconcile loop, which races STS ReadyReplicas; a one-shot scrape in that window would flake the new value assertions. No fixed sleeps anywhere. - Optimize: drop the cluster from size 3 to size 1. Quorum (1/1), a leader, healthy members, the PodMonitor path, and every derived gauge are all exercised at size 1; the quorum-over-voting-members arithmetic is covered by the metrics package unit tests. This cuts the dominant cost (sequential StatefulSet bootstrap + member-join): live e2e wall-clock 202.8s -> 108.1s. - Drop the dead `var _ = corev1.Pod{}` keepalive and its unused import, and rewrite the misleading "authored primarily for compile-time verification" doc comment to describe what the test now proves. Live-validated on KinD (kindest/node:v1.32.0): both runs green. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Xavier Lange --- test/e2e/metrics_test.go | 233 ++++++++++++++++++++++++++++++++++----- 1 file changed, 203 insertions(+), 30 deletions(-) diff --git a/test/e2e/metrics_test.go b/test/e2e/metrics_test.go index 501a2c2e..5c1d8187 100644 --- a/test/e2e/metrics_test.go +++ b/test/e2e/metrics_test.go @@ -19,13 +19,15 @@ package e2e import ( "context" "fmt" + "strconv" "strings" "testing" "time" appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes" "sigs.k8s.io/e2e-framework/klient/k8s" "sigs.k8s.io/e2e-framework/klient/wait" @@ -38,7 +40,10 @@ import ( // metricNamesToAssert lists the custom domain metrics this feature expects to // find on the operator's /metrics endpoint once at least one EtcdCluster has -// been reconciled. +// been reconciled. A bare name-substring check only proves the collector was +// registered (the # HELP/# TYPE lines are emitted even with zero series), so it +// is a necessary-but-not-sufficient guard. The value assertions in the Assess +// block below are what actually prove the per-cluster series were recorded. var metricNamesToAssert = []string{ "etcd_operator_cluster_member_count_desired", "etcd_operator_cluster_member_count", @@ -52,45 +57,146 @@ var metricNamesToAssert = []string{ } // TestDomainMetricsExposed creates an EtcdCluster with metrics + a PodMonitor -// configured via spec.metrics, waits for it to become ready, and asserts that -// the operator's Prometheus /metrics endpoint exposes the custom per-cluster -// domain metrics labelled with this cluster's namespace and name. +// configured via spec.metrics, waits for it to become ready, and proves +// end-to-end that the operator's Prometheus /metrics endpoint exposes the +// custom per-cluster domain metrics with the CORRECT derived values for a +// healthy cluster -- not merely that the collectors are registered. // -// NOTE: This test is authored primarily for compile-time verification in CI -// alongside the unit/envtest coverage of the metrics package; it only executes -// against a live KinD cluster as part of the e2e suite (make test-e2e). +// Specifically it asserts, by polling the live /metrics scrape until the +// operator's asynchronous reconcile has populated status: +// - member_count_desired / member_count / member_count_ready / +// member_count_healthy all equal the cluster size (the operator actually +// talked to etcd and counted real members), +// - has_quorum == 1 and has_leader == 1 (the dangerous derived booleans that a +// real outage dashboard reads; these require a real etcd round-trip and a +// populated Status.Members/LeaderID, so they distinguish "operator fully +// reconciled etcd" from "operator never talked to etcd"), +// - learner_count == 0 and tls_enabled == 0 for a steady plaintext cluster, +// - reconcile_duration_seconds_count >= 1 (the reconcile-timing hook fired), +// - tls_ready is ABSENT for this plaintext cluster (the documented invariant +// so alerts never fire on plaintext clusters), +// - a PodMonitor object was actually created for the cluster with endpoint +// port "client" and interval "30s". +// +// A single-member cluster is sufficient: quorum (1/1), a leader, healthy +// members, the PodMonitor path, and every derived gauge are all exercised, and +// the quorum-over-voting-members arithmetic is covered by the metrics package +// unit tests. Using size 1 avoids the dominant cost of sequential 3-node +// StatefulSet bootstrap + member-join. func TestDomainMetricsExposed(t *testing.T) { clusterName := "etcd-metrics" + const clusterSize = 1 feature := features.New("domain-metrics") feature.Setup(func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { - metricsCreateCluster(ctx, t, c, clusterName, 3) - metricsWaitForStatefulSetReady(ctx, t, c, clusterName, 3) + metricsCreateCluster(ctx, t, c, clusterName, clusterSize) + metricsWaitForStatefulSetReady(ctx, t, c, clusterName, clusterSize) return ctx }) - feature.Assess("operator /metrics exposes per-cluster domain metrics", + feature.Assess("operator /metrics exposes correct per-cluster domain values", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { - body := metricsScrapeOperator(ctx, t, c) - for _, name := range metricNamesToAssert { - if !strings.Contains(body, name) { - t.Errorf("expected /metrics to contain %q, but it did not", name) + // The derived gauges are written by the operator's reconcile loop, + // which is asynchronous to StatefulSet readiness: there is a race + // window where pods are Ready but the operator has not yet run the + // reconcile that populates Status.Members / quorum / leader. Poll the + // scrape until every expected sample is present rather than reading + // once and racing. No fixed sleeps anywhere. + lvs := fmt.Sprintf("{name=%q,namespace=%q}", clusterName, namespace) + want := map[string]string{ + "etcd_operator_cluster_member_count_desired" + lvs: strconv.Itoa(clusterSize), + "etcd_operator_cluster_member_count" + lvs: strconv.Itoa(clusterSize), + "etcd_operator_cluster_member_count_ready" + lvs: strconv.Itoa(clusterSize), + "etcd_operator_cluster_member_count_healthy" + lvs: strconv.Itoa(clusterSize), + "etcd_operator_cluster_learner_count" + lvs: "0", + // The two strongest, most dangerous assertions: a regression that + // leaves Status.Members/LeaderID unpopulated (e.g. status-update + // ordering) would ship has_quorum 0 / has_leader 0 green under a + // name-only check. These require the operator to have actually + // reconciled etcd. + "etcd_operator_cluster_has_quorum" + lvs: "1", + "etcd_operator_cluster_has_leader" + lvs: "1", + "etcd_operator_cluster_tls_enabled" + lvs: "0", + } + + var lastBody string + err := wait.For(func(ctx context.Context) (bool, error) { + lastBody = metricsScrapeOperator(ctx, t, c) + for metric, val := range want { + if !metricsContainsSample(lastBody, metric+" "+val) { + return false, nil + } + } + // The reconcile-timing histogram must have observed at least one + // reconcile for this cluster. A histogram's # HELP line exists with + // zero observations, so assert the _count child series is >= 1. + if !metricsReconcileObserved(lastBody, clusterName, namespace) { + return false, nil + } + return true, nil + }, wait.WithTimeout(2*time.Minute), wait.WithInterval(5*time.Second), wait.WithContext(ctx)) + if err != nil { + // Surface the metric names + the offending body for diagnosis. + for _, name := range metricNamesToAssert { + if !strings.Contains(lastBody, name) { + t.Errorf("metric family %q missing from /metrics (collector not registered?)", name) + } } + for metric, val := range want { + if !metricsContainsSample(lastBody, metric+" "+val) { + t.Errorf("expected sample %q not found in /metrics", metric+" "+val) + } + } + if !metricsReconcileObserved(lastBody, clusterName, namespace) { + t.Errorf("reconcile_duration_seconds_count for %s/%s was not >= 1", namespace, clusterName) + } + t.Fatalf("timed out waiting for correct domain-metric values: %v", err) } - // At least one series must be labelled with this cluster's name. - if !strings.Contains(body, fmt.Sprintf("name=%q", clusterName)) { - t.Errorf("expected /metrics to contain a series labelled name=%q", clusterName) + + // tls_ready must be ABSENT for a plaintext cluster: the metrics package + // DeleteLabelValues()-es it for non-TLS clusters so "tls configured but + // not ready" alerts never fire on plaintext. A present series here is a + // real regression. + tlsReadyPrefix := fmt.Sprintf( + "etcd_operator_cluster_tls_ready{name=%q,namespace=%q}", clusterName, namespace) + for _, line := range strings.Split(lastBody, "\n") { + if strings.HasPrefix(strings.TrimSpace(line), tlsReadyPrefix) { + t.Errorf("expected NO tls_ready series for plaintext cluster, found: %q", strings.TrimSpace(line)) + } } - // Assert an actual value, not just the metric name: a name-substring - // check passes purely from collector registration (# HELP/# TYPE - // lines) even with zero series. The desired member count is the - // cluster's spec.size (3), so this proves the per-cluster gauge was - // recorded for the reconciled cluster. - wantDesired := fmt.Sprintf( - "etcd_operator_cluster_member_count_desired{name=%q,namespace=%q} 3", - clusterName, namespace) - if !metricsContainsSample(body, wantDesired) { - t.Errorf("expected /metrics to contain sample %q; body did not match", wantDesired) + return ctx + }) + + feature.Assess("operator reconciles a PodMonitor with the configured endpoint", + func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + // The PodMonitor branch swallows all errors (CRD-absent is a silent + // skip), so without observing the object the whole podMonitor half of + // the feature is dead setup. The e2e suite installs the + // prometheus-operator CRDs, so the object MUST exist. Poll because + // PodMonitor reconcile is part of the same async reconcile loop. + pm := newPodMonitorUnstructured(clusterName, namespace) + if err := wait.For( + conditions.New(c.Client().Resources()).ResourceMatch(pm, func(object k8s.Object) bool { + u, ok := object.(*unstructured.Unstructured) + if !ok { + return false + } + port, interval, found := podMonitorEndpoint0(u) + return found && port == "client" && interval == "30s" + }), + wait.WithTimeout(90*time.Second), + wait.WithInterval(5*time.Second), + wait.WithContext(ctx), + ); err != nil { + // Re-Get for a precise diagnosis of what was (or wasn't) there. + got := newPodMonitorUnstructured(clusterName, namespace) + if gerr := c.Client().Resources().Get(ctx, clusterName, namespace, got); gerr != nil { + t.Fatalf("PodMonitor %s/%s was never created (operator silently skipped it?): %v", + namespace, clusterName, gerr) + } + port, interval, found := podMonitorEndpoint0(got) + t.Fatalf("PodMonitor %s/%s endpoint mismatch (found=%v port=%q interval=%q), want port=client interval=30s: %v", + namespace, clusterName, found, port, interval, err) } return ctx }) @@ -173,6 +279,75 @@ func metricsContainsSample(body, want string) bool { return false } +// metricsReconcileObserved reports whether the reconcile-duration histogram has +// observed at least one reconcile for the given cluster, by parsing the +// _count child series. The _count line looks like: +// +// etcd_operator_cluster_reconcile_duration_seconds_count{name="x",namespace="y"} 3 +// +// We match on the label set (order-independent within the {...}) and require the +// trailing value to parse as a number >= 1. +func metricsReconcileObserved(body, name, ns string) bool { + const prefix = "etcd_operator_cluster_reconcile_duration_seconds_count{" + for _, line := range strings.Split(body, "\n") { + line = strings.TrimSpace(line) + if !strings.HasPrefix(line, prefix) { + continue + } + brace := strings.IndexByte(line, '}') + if brace < 0 { + continue + } + labels := line[len(prefix):brace] + if !strings.Contains(labels, fmt.Sprintf("name=%q", name)) || + !strings.Contains(labels, fmt.Sprintf("namespace=%q", ns)) { + continue + } + valStr := strings.TrimSpace(line[brace+1:]) + v, err := strconv.ParseFloat(valStr, 64) + if err == nil && v >= 1 { + return true + } + } + return false +} + +// podMonitorGVK addresses the prometheus-operator PodMonitor type via +// unstructured so the test takes no hard dependency on the prometheus-operator +// Go module (mirroring the controller). +var podMonitorGVK = schema.GroupVersionKind{ + Group: "monitoring.coreos.com", + Version: "v1", + Kind: "PodMonitor", +} + +// newPodMonitorUnstructured returns an empty unstructured object addressing the +// PodMonitor the operator creates for the cluster (same namespace/name). +func newPodMonitorUnstructured(name, ns string) *unstructured.Unstructured { + u := &unstructured.Unstructured{} + u.SetGroupVersionKind(podMonitorGVK) + u.SetNamespace(ns) + u.SetName(name) + return u +} + +// podMonitorEndpoint0 extracts the port and interval of the first +// podMetricsEndpoints entry from a PodMonitor unstructured object. found is +// false if the spec/endpoint is missing or malformed. +func podMonitorEndpoint0(u *unstructured.Unstructured) (port, interval string, found bool) { + eps, ok, err := unstructured.NestedSlice(u.Object, "spec", "podMetricsEndpoints") + if err != nil || !ok || len(eps) == 0 { + return "", "", false + } + ep, ok := eps[0].(map[string]any) + if !ok { + return "", "", false + } + port, _ = ep["port"].(string) + interval, _ = ep["interval"].(string) + return port, interval, true +} + // metricsScrapeOperator scrapes the operator pod's metrics endpoint through the // API server pod-proxy and returns the raw Prometheus exposition text. The e2e // manager is expected to serve plain-HTTP metrics on :8080 @@ -205,5 +380,3 @@ func metricsScrapeOperator(ctx context.Context, t *testing.T, c *envconf.Config) } return string(raw) } - -var _ = corev1.Pod{} From e878bef25835391e835e719730d1609614b89db1 Mon Sep 17 00:00:00 2001 From: Xavier Lange Date: Fri, 19 Jun 2026 19:02:06 -0400 Subject: [PATCH 8/8] feat(metrics): allow custom labels on the generated PodMonitor A namespaced/release-scoped Prometheus selects PodMonitors by label (e.g. podMonitorSelector matchLabels release: kvs-prometheus). The operator-generated PodMonitor only carried fixed app.kubernetes.io/* labels, so such a Prometheus never discovered it and downstream had to hand-author a PodMonitor. Add spec.metrics.podMonitor.labels (map[string]string). The labels are merged on top of the operator-managed labels and win on conflict; nil/ empty preserves today's behavior. Regenerated CRD + deepcopy and added unit coverage asserting release: kvs-prometheus lands on the object and that user keys override managed defaults. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Xavier Lange --- api/v1alpha1/etcdcluster_types.go | 11 +++++ api/v1alpha1/zz_generated.deepcopy.go | 9 +++- .../bases/operator.etcd.io_etcdclusters.yaml | 13 +++++ internal/controller/podmonitor.go | 13 ++++- internal/controller/podmonitor_test.go | 47 +++++++++++++++++++ 5 files changed, 90 insertions(+), 3 deletions(-) diff --git a/api/v1alpha1/etcdcluster_types.go b/api/v1alpha1/etcdcluster_types.go index b2fdb0d4..cdf4c9c4 100644 --- a/api/v1alpha1/etcdcluster_types.go +++ b/api/v1alpha1/etcdcluster_types.go @@ -92,6 +92,17 @@ type PodMonitorSpec struct { // Defaults to "client" when empty. // +optional Port string `json:"port,omitempty"` + + // Labels are additional metadata labels to set on the generated + // PodMonitor. They are merged on top of the operator-managed labels + // (app.kubernetes.io/name, /managed-by, /instance): user-provided keys + // take precedence on conflict. This is typically used to satisfy a + // namespaced Prometheus's podMonitorSelector, e.g. setting + // "release: kvs-prometheus" so a release-scoped Prometheus discovers and + // scrapes this PodMonitor. When empty, only the operator-managed labels + // are applied (today's behavior). + // +optional + Labels map[string]string `json:"labels,omitempty"` } // MetricsEnabled reports whether per-cluster domain metrics should be exported diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 09cdefd5..b131727d 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -231,7 +231,7 @@ func (in *MetricsSpec) DeepCopyInto(out *MetricsSpec) { if in.PodMonitor != nil { in, out := &in.PodMonitor, &out.PodMonitor *out = new(PodMonitorSpec) - **out = **in + (*in).DeepCopyInto(*out) } } @@ -277,6 +277,13 @@ func (in *PodMetadata) DeepCopy() *PodMetadata { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PodMonitorSpec) DeepCopyInto(out *PodMonitorSpec) { *out = *in + if in.Labels != nil { + in, out := &in.Labels, &out.Labels + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PodMonitorSpec. diff --git a/config/crd/bases/operator.etcd.io_etcdclusters.yaml b/config/crd/bases/operator.etcd.io_etcdclusters.yaml index 1d7ba526..338bb93a 100644 --- a/config/crd/bases/operator.etcd.io_etcdclusters.yaml +++ b/config/crd/bases/operator.etcd.io_etcdclusters.yaml @@ -85,6 +85,19 @@ spec: Interval at which Prometheus scrapes the etcd member pods, e.g. "30s". When empty the prometheus-operator default is used. type: string + labels: + additionalProperties: + type: string + description: |- + Labels are additional metadata labels to set on the generated + PodMonitor. They are merged on top of the operator-managed labels + (app.kubernetes.io/name, /managed-by, /instance): user-provided keys + take precedence on conflict. This is typically used to satisfy a + namespaced Prometheus's podMonitorSelector, e.g. setting + "release: kvs-prometheus" so a release-scoped Prometheus discovers and + scrapes this PodMonitor. When empty, only the operator-managed labels + are applied (today's behavior). + type: object port: description: |- Port is the name of the pod port exposing etcd's /metrics endpoint. diff --git a/internal/controller/podmonitor.go b/internal/controller/podmonitor.go index 5d11bb27..e3285a48 100644 --- a/internal/controller/podmonitor.go +++ b/internal/controller/podmonitor.go @@ -78,11 +78,13 @@ func (r *EtcdClusterReconciler) reconcilePodMonitor(ctx context.Context, ec *ecv port := defaultPodMonitorPort interval := "" + var userLabels map[string]string if pm := ec.Spec.Metrics.PodMonitor; pm != nil { if pm.Port != "" { port = pm.Port } interval = pm.Interval + userLabels = pm.Labels } endpoint := map[string]any{ @@ -94,11 +96,18 @@ func (r *EtcdClusterReconciler) reconcilePodMonitor(ctx context.Context, ec *ecv } op, err := controllerutil.CreateOrUpdate(ctx, r.Client, desired, func() error { - desired.SetLabels(map[string]string{ + // Operator-managed labels first, then overlay any user-provided + // labels so spec.metrics.podMonitor.labels (e.g. release selectors) + // win on conflict while leaving the defaults intact when absent. + labels := map[string]string{ "app.kubernetes.io/name": "etcd-operator", "app.kubernetes.io/managed-by": "etcd-operator", "app.kubernetes.io/instance": ec.Name, - }) + } + for k, v := range userLabels { + labels[k] = v + } + desired.SetLabels(labels) spec := map[string]any{ "selector": map[string]any{ "matchLabels": map[string]any{ diff --git a/internal/controller/podmonitor_test.go b/internal/controller/podmonitor_test.go index 7ba4dafc..6d17ee8f 100644 --- a/internal/controller/podmonitor_test.go +++ b/internal/controller/podmonitor_test.go @@ -130,6 +130,53 @@ func TestReconcilePodMonitor_DefaultsPortWhenUnset(t *testing.T) { assert.False(t, hasInterval, "interval should be omitted when unset") } +// TestReconcilePodMonitor_AppliesCustomLabels asserts that user-provided +// spec.metrics.podMonitor.labels are merged onto the generated PodMonitor's +// metadata.labels (so a release-scoped Prometheus can select it) while the +// operator-managed labels remain intact. +func TestReconcilePodMonitor_AppliesCustomLabels(t *testing.T) { + scheme := podMonitorTestScheme() + ec := newMetricsCluster(true, "client", "30s") + ec.Spec.Metrics.PodMonitor.Labels = map[string]string{ + "release": "kvs-prometheus", + "team": "kv", + } + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(ec).Build() + r := &EtcdClusterReconciler{Client: fakeClient, Scheme: scheme} + + r.reconcilePodMonitor(context.Background(), ec) + + pm, err := getPodMonitor(context.Background(), fakeClient, ec) + require.NoError(t, err, "expected PodMonitor to be created") + + labels := pm.GetLabels() + // User-provided labels are present (release selector + arbitrary label). + assert.Equal(t, "kvs-prometheus", labels["release"]) + assert.Equal(t, "kv", labels["team"]) + // Operator-managed labels are preserved. + assert.Equal(t, "etcd-operator", labels["app.kubernetes.io/name"]) + assert.Equal(t, "etcd-operator", labels["app.kubernetes.io/managed-by"]) + assert.Equal(t, ec.Name, labels["app.kubernetes.io/instance"]) +} + +// TestReconcilePodMonitor_CustomLabelsOverrideManaged asserts that a +// user-provided label key wins over an operator-managed default on conflict. +func TestReconcilePodMonitor_CustomLabelsOverrideManaged(t *testing.T) { + scheme := podMonitorTestScheme() + ec := newMetricsCluster(true, "client", "30s") + ec.Spec.Metrics.PodMonitor.Labels = map[string]string{ + "app.kubernetes.io/name": "override", + } + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(ec).Build() + r := &EtcdClusterReconciler{Client: fakeClient, Scheme: scheme} + + r.reconcilePodMonitor(context.Background(), ec) + + pm, err := getPodMonitor(context.Background(), fakeClient, ec) + require.NoError(t, err) + assert.Equal(t, "override", pm.GetLabels()["app.kubernetes.io/name"]) +} + // TestReconcilePodMonitor_CRDAbsentIsSoftSkip asserts that a missing PodMonitor // CRD (NoMatchError on the underlying Get) is treated as a soft skip: the // reconcile does not panic and creates nothing.