Merge branch 'main' into update-e2e-test-versions

Author: johscheuer

controllers/change_coordinators_test.go

@@ -24,14 +24,13 @@ import (
 	"context"
 	"math"
 
-	"k8s.io/utils/ptr"
-
 	fdbv1beta2 "github.com/FoundationDB/fdb-kubernetes-operator/v2/api/v1beta2"
 	"github.com/FoundationDB/fdb-kubernetes-operator/v2/internal"
 	"github.com/FoundationDB/fdb-kubernetes-operator/v2/pkg/fdbadminclient/mock"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/utils/ptr"
 )
 
 var _ = Describe("Change coordinators", func() {

controllers/metrics.go

@@ -3,7 +3,7 @@
  *
  * This source file is part of the FoundationDB open source project
  *
- * Copyright 2021 Apple Inc. and the FoundationDB project authors
+ * Copyright 2021-2025 Apple Inc. and the FoundationDB project authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,84 +24,11 @@ import (
 	"context"
 
 	fdbv1beta2 "github.com/FoundationDB/fdb-kubernetes-operator/v2/api/v1beta2"
+	internalMetrics "github.com/FoundationDB/fdb-kubernetes-operator/v2/internal/metrics"
 	"github.com/prometheus/client_golang/prometheus"
 	"sigs.k8s.io/controller-runtime/pkg/metrics"
 )
 
-var (
-	descClusterDefaultLabels = []string{"namespace", "name"}
-
-	descClusterCreated = prometheus.NewDesc(
-		"fdb_operator_cluster_created_time",
-		"Creation time in unix timestamp for Fdb Cluster.",
-		descClusterDefaultLabels,
-		nil,
-	)
-
-	descClusterStatus = prometheus.NewDesc(
-		"fdb_operator_cluster_status",
-		"status of the Fdb Cluster.",
-		append(descClusterDefaultLabels, "status_type"),
-		nil,
-	)
-
-	descClusterLastReconciled = prometheus.NewDesc(
-		"fdb_operator_cluster_latest_reconciled_status",
-		"the latest generation that was reconciled.",
-		descClusterDefaultLabels,
-		nil,
-	)
-
-	descProcessGroupsToRemove = prometheus.NewDesc(
-		"fdb_operator_process_groups_to_remove_total",
-		"the count of process groups that should be removed from the cluster.",
-		descClusterDefaultLabels,
-		nil,
-	)
-
-	descProcessGroupsToRemoveWithoutExclusion = prometheus.NewDesc(
-		"fdb_operator_process_group_to_remove_without_exclusion_total",
-		"the count of process groups that should be removed from the cluster without excluding.",
-		descClusterDefaultLabels,
-		nil,
-	)
-
-	descClusterReconciled = prometheus.NewDesc(
-		"fdb_operator_cluster_reconciled_status",
-		"status if the Fdb Cluster is reconciled.",
-		descClusterDefaultLabels,
-		nil,
-	)
-
-	descProcessGroupStatus = prometheus.NewDesc(
-		"fdb_operator_process_group_total",
-		"the count of Fdb process groups in a specific condition.",
-		append(descClusterDefaultLabels, "process_class", "condition"),
-		nil,
-	)
-
-	descProcessGroupMarkedRemoval = prometheus.NewDesc(
-		"fdb_operator_process_group_marked_removal",
-		"the count of Fdb process groups that are marked for removal.",
-		append(descClusterDefaultLabels, "process_class"),
-		nil,
-	)
-
-	descProcessGroupMarkedExcluded = prometheus.NewDesc(
-		"fdb_operator_process_group_marked_excluded",
-		"the count of Fdb process groups that are marked as excluded.",
-		append(descClusterDefaultLabels, "process_class"),
-		nil,
-	)
-
-	desDesiredProcessGroups = prometheus.NewDesc(
-		"fdb_operator_desired_process_group_total",
-		"the count of the desired Fdb process groups",
-		append(descClusterDefaultLabels, "process_class"),
-		nil,
-	)
-)
-
 type fdbClusterCollector struct {
 	reconciler *FoundationDBClusterReconciler
 }
@@ -112,8 +39,8 @@ func newFDBClusterCollector(reconciler *FoundationDBClusterReconciler) *fdbClust
 
 // Describe implements the prometheus.Collector interface
 func (c *fdbClusterCollector) Describe(ch chan<- *prometheus.Desc) {
-	ch <- descClusterCreated
-	ch <- descClusterStatus
+	ch <- internalMetrics.DescClusterCreated
+	ch <- internalMetrics.DescClusterStatus
 }
 
 // Collect implements the prometheus.Collector interface
@@ -124,118 +51,14 @@ func (c *fdbClusterCollector) Collect(ch chan<- prometheus.Metric) {
 		return
 	}
 	for _, cluster := range clusters.Items {
-		collectMetrics(ch, &cluster)
-	}
-}
-
-func collectMetrics(ch chan<- prometheus.Metric, cluster *fdbv1beta2.FoundationDBCluster) {
-	addConstMetric := func(desc *prometheus.Desc, t prometheus.ValueType, v float64, lv ...string) {
-		lv = append([]string{cluster.Namespace, cluster.Name}, lv...)
-		ch <- prometheus.MustNewConstMetric(desc, t, v, lv...)
-	}
-	addGauge := func(desc *prometheus.Desc, v float64, lv ...string) {
-		addConstMetric(desc, prometheus.GaugeValue, v, lv...)
-	}
-	// These are the correct metrics with the prefix "fdb_operator"
-	addGauge(descClusterCreated, float64(cluster.CreationTimestamp.Unix()))
-	addGauge(descClusterStatus, boolFloat64(cluster.Status.Health.Healthy), "health")
-	addGauge(descClusterStatus, boolFloat64(cluster.Status.Health.Available), "available")
-	addGauge(descClusterStatus, boolFloat64(cluster.Status.Health.FullReplication), "replication")
-	addGauge(
-		descClusterStatus,
-		float64(cluster.Status.Health.DataMovementPriority),
-		"datamovementpriority",
-	)
-	addGauge(descClusterLastReconciled, float64(cluster.Status.Generations.Reconciled))
-	addGauge(
-		descClusterReconciled,
-		boolFloat64(cluster.ObjectMeta.Generation == cluster.Status.Generations.Reconciled),
-	)
-	addGauge(descProcessGroupsToRemove, float64(len(cluster.Spec.ProcessGroupsToRemove)))
-	addGauge(
-		descProcessGroupsToRemoveWithoutExclusion,
-		float64(len(cluster.Spec.ProcessGroupsToRemoveWithoutExclusion)),
-	)
-
-	// Calculate the process group metrics
-	conditionMap, removals, exclusions := getProcessGroupMetrics(cluster)
-
-	for pclass, conditionMap := range conditionMap {
-		for condition, count := range conditionMap {
-			addGauge(descProcessGroupStatus, float64(count), string(pclass), string(condition))
-		}
-
-		addGauge(descProcessGroupMarkedRemoval, float64(removals[pclass]), string(pclass))
-		addGauge(descProcessGroupMarkedExcluded, float64(exclusions[pclass]), string(pclass))
-	}
-
-	counts, err := cluster.GetProcessCountsWithDefaults()
-	if err != nil {
-		return
-	}
-
-	for processCount, count := range counts.Map() {
-		addGauge(desDesiredProcessGroups, float64(count), string(processCount))
+		internalMetrics.CollectMetrics(ch, &cluster)
 	}
 }
 
-func getProcessGroupMetrics(
-	cluster *fdbv1beta2.FoundationDBCluster,
-) (map[fdbv1beta2.ProcessClass]map[fdbv1beta2.ProcessGroupConditionType]int, map[fdbv1beta2.ProcessClass]int, map[fdbv1beta2.ProcessClass]int) {
-	metricMap := map[fdbv1beta2.ProcessClass]map[fdbv1beta2.ProcessGroupConditionType]int{}
-	removals := map[fdbv1beta2.ProcessClass]int{}
-	exclusions := map[fdbv1beta2.ProcessClass]int{}
-
-	for _, processGroup := range cluster.Status.ProcessGroups {
-		if _, exits := metricMap[processGroup.ProcessClass]; !exits {
-			metricMap[processGroup.ProcessClass] = map[fdbv1beta2.ProcessGroupConditionType]int{}
-			removals[processGroup.ProcessClass] = 0
-			exclusions[processGroup.ProcessClass] = 0
-		}
-
-		if processGroup.IsMarkedForRemoval() {
-			removals[processGroup.ProcessClass]++
-		}
-
-		if processGroup.IsExcluded() {
-			exclusions[processGroup.ProcessClass]++
-		}
-
-		if len(processGroup.ProcessGroupConditions) == 0 {
-			metricMap[processGroup.ProcessClass][fdbv1beta2.ReadyCondition]++
-		}
-
-		for _, condition := range processGroup.ProcessGroupConditions {
-			metricMap[processGroup.ProcessClass][condition.ProcessGroupConditionType]++
-		}
-	}
-
-	// Ensure that all conditions are present
-	for pClass := range metricMap {
-		if _, exits := metricMap[pClass]; !exits {
-			metricMap[pClass] = map[fdbv1beta2.ProcessGroupConditionType]int{}
-		}
-
-		for _, condition := range fdbv1beta2.AllProcessGroupConditionTypes() {
-			if _, exists := metricMap[pClass][condition]; !exists {
-				metricMap[pClass][condition] = 0
-			}
-		}
-	}
-
-	return metricMap, removals, exclusions
-}
-
 // InitCustomMetrics initializes the metrics collectors for the operator.
 func InitCustomMetrics(reconciler *FoundationDBClusterReconciler) {
 	metrics.Registry.MustRegister(
 		newFDBClusterCollector(reconciler),
+		internalMetrics.CoordinatorChangesCounter,
 	)
 }
-
-func boolFloat64(b bool) float64 {
-	if b {
-		return 1
-	}
-	return 0
-}

fdbclient/admin_client.go

@@ -35,6 +35,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/FoundationDB/fdb-kubernetes-operator/v2/internal/metrics"
+
 	fdbv1beta2 "github.com/FoundationDB/fdb-kubernetes-operator/v2/api/v1beta2"
 	"github.com/FoundationDB/fdb-kubernetes-operator/v2/internal"
 	"github.com/FoundationDB/fdb-kubernetes-operator/v2/pkg/fdbadminclient"
@@ -779,6 +781,9 @@ func (client *cliAdminClient) ChangeCoordinators(
 		}
 	}
 
+	// Increment the coordinator changes counter for this cluster
+	metrics.CoordinatorChangesCounter.WithLabelValues(client.Cluster.Namespace, client.Cluster.Name).
+		Inc()
 	return getConnectionStringFromDB(client.fdbLibClient, client.getTimeout())
 }
 

fdbclient/admin_client_test.go

@@ -29,9 +29,12 @@ import (
 	"time"
 
 	fdbv1beta2 "github.com/FoundationDB/fdb-kubernetes-operator/v2/api/v1beta2"
+	"github.com/FoundationDB/fdb-kubernetes-operator/v2/internal/metrics"
 	"github.com/go-logr/logr"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/prometheus/client_golang/prometheus"
+	dto "github.com/prometheus/client_model/go"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
@@ -1263,4 +1266,70 @@ protocol fdb00b071010000`,
 		Entry("when it is passed in", ptr.To(uint64(1234567890123)), "1234567890123"),
 		Entry("when it is not passed in", nil, ""),
 	)
+
+	When("changing the coordinators", func() {
+		var cliClient *cliAdminClient
+		var mockRunner *mockCommandRunner
+
+		BeforeEach(func() {
+			mockRunner = &mockCommandRunner{
+				mockedError:  nil,
+				mockedOutput: []string{""},
+			}
+
+			cliClient = &cliAdminClient{
+				Cluster: &fdbv1beta2.FoundationDBCluster{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test",
+						Namespace: "test",
+					},
+					Spec: fdbv1beta2.FoundationDBClusterSpec{
+						Version: "7.3.1",
+					},
+					Status: fdbv1beta2.FoundationDBClusterStatus{
+						RunningVersion:   "7.3.1",
+						ConnectionString: "abc:dfg@test:4500",
+					},
+				},
+				log:       logr.Discard(),
+				cmdRunner: mockRunner,
+				fdbLibClient: &mockFdbLibClient{
+					mockedOutput: []byte("abc:xyz@127.0.01:4500"),
+				},
+			}
+
+			_, err := cliClient.ChangeCoordinators([]fdbv1beta2.ProcessAddress{
+				{
+					IPAddress: net.ParseIP("127.0.0.1"),
+					Port:      4500,
+				},
+			})
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("should change the coordinators and increment the counter", func() {
+			Expect(mockRunner.receivedArgs).To(HaveLen(1))
+			Expect(
+				mockRunner.receivedArgs[0][:2],
+			).To(Equal([]string{"--exec", "coordinators 127.0.0.1:4500"}))
+
+			// The counter should have been incremented since a coordinator change occurred
+			// in the JustBeforeEach block above
+			counterValue := getCounterValue(
+				metrics.CoordinatorChangesCounter,
+				"test",
+				"test",
+			)
+			Expect(
+				counterValue,
+			).To(BeNumerically(">", 0), "coordinator changes counter should be incremented after a coordinator change")
+		})
+	})
 })
+
+// getCounterValue extracts the current value of a prometheus counter for specific labels
+func getCounterValue(counter *prometheus.CounterVec, namespace, name string) float64 {
+	metric := &dto.Metric{}
+	Expect(counter.WithLabelValues(namespace, name).Write(metric)).To(Succeed())
+	return metric.Counter.GetValue()
+}

go.mod

@@ -36,6 +36,7 @@ require (
 
 require (
 	github.com/pkg/errors v0.9.1
+	github.com/prometheus/client_model v0.6.2
 	github.com/robfig/cron/v3 v3.0.1
 )
 
@@ -86,7 +87,6 @@ require (
 	github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f // indirect
 	github.com/pelletier/go-toml/v2 v2.2.3 // indirect
 	github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
-	github.com/prometheus/client_model v0.6.2 // indirect
 	github.com/prometheus/procfs v0.15.1 // indirect
 	github.com/russross/blackfriday/v2 v2.1.0 // indirect
 	github.com/sagikazarmark/locafero v0.7.0 // indirect