Repository: FoundationDB/fdb-kubernetes-operator
Pull request #1871 (2023-11-06 15:14:20)

Simplify test setup
# Description

Remove some steps in the test setup for update status.

## Type of change

*Please select one of the options below.*

- Other

## Discussion

-

## Testing

Unit test.

## Documentation

-

## Follow-up

-


foundationdb-ci: 
### Result of fdb-kubernetes-operator-pr on Linux CentOS 7

* Commit ID: b3416356c60b0625329852c5de6021c2cd9089e8
* Duration 2:41:49
* Result: :x: **FAILED**
* Error: `Error while executing command: if $fail_test; then exit 1; fi. Reason: exit status 1`
* [Build Log](https://d1e0l78xbkh3xa.cloudfront.net/fdb-kubernetes-operator/fdb-kubernetes-operator-pr/3ebdb80e-a341-4014-b8c2-732d58e1551e.gz) terminal output (available for 30 days)
* [Build Workspace](https://d1e0l78xbkh3xa.cloudfront.net/fdb-kubernetes-operator-pr/3ebdb80e-a341-4014-b8c2-732d58e1551e/workspace.zip) zip file of the working directory (available for 30 days)


Patch:
diff --git a/api/v1beta2/foundationdb_version.go b/api/v1beta2/foundationdb_version.go
index 706a8f14..30021b1c 100644
--- a/api/v1beta2/foundationdb_version.go
+++ b/api/v1beta2/foundationdb_version.go
@@ -221,6 +221,17 @@ func (version Version) SupportsVersionChange(other Version) bool {
 	return version.IsProtocolCompatible(other) || other.IsAtLeast(version)
 }
 
+// SupportsLocalityBasedExclusions returns true if the current version supports locality based exclusions.
+func (version Version) SupportsLocalityBasedExclusions() bool {
+	// If the version is 7.1.* we have to check if it supports locality based exclusions. For all newer versions
+	// we will check against the 7.3 version.
+	if version.IsProtocolCompatible(Version{Major: 7, Minor: 1, Patch: 0}) {
+		return version.IsAtLeast(Versions.SupportsLocalityBasedExclusions71)
+	}
+
+	return version.IsAtLeast(Versions.SupportsLocalityBasedExclusions)
+}
+
 // Versions provides a shorthand for known versions.
 // This is only to be used in testing.
 var Versions = struct {
@@ -236,19 +247,23 @@ var Versions = struct {
 	PreviousPatchVersion,
 	SupportsRecoveryState,
 	SupportsDNSInClusterFile,
+	SupportsLocalityBasedExclusions71,
+	SupportsLocalityBasedExclusions,
 	Default Version
 }{
-	Default:                      Version{Major: 6, Minor: 2, Patch: 21},
-	IncompatibleVersion:          Version{Major: 6, Minor: 1, Patch: 0},
-	PreviousPatchVersion:         Version{Major: 6, Minor: 2, Patch: 20},
-	NextPatchVersion:             Version{Major: 6, Minor: 2, Patch: 22},
-	NextMajorVersion:             Version{Major: 7, Minor: 0, Patch: 0},
-	MinimumVersion:               Version{Major: 6, Minor: 2, Patch: 20},
-	SupportsRocksDBV1:            Version{Major: 7, Minor: 1, Patch: 0, ReleaseCandidate: 4},
-	SupportsIsPresent:            Version{Major: 7, Minor: 1, Patch: 4},
-	SupportsShardedRocksDB:       Version{Major: 7, Minor: 2, Patch: 0},
-	SupportsRedwood1Experimental: Version{Major: 7, Minor: 0, Patch: 0},
-	SupportsRedwood1:             Version{Major: 7, Minor: 3, Patch: 0},
-	SupportsRecoveryState:        Version{Major: 7, Minor: 1, Patch: 22},
-	SupportsDNSInClusterFile:     Version{Major: 7, Minor: 0, Patch: 0},
+	Default:                           Version{Major: 6, Minor: 2, Patch: 21},
+	IncompatibleVersion:               Version{Major: 6, Minor: 1, Patch: 0},
+	PreviousPatchVersion:              Version{Major: 6, Minor: 2, Patch: 20},
+	NextPatchVersion:                  Version{Major: 6, Minor: 2, Patch: 22},
+	NextMajorVersion:                  Version{Major: 7, Minor: 0, Patch: 0},
+	MinimumVersion:                    Version{Major: 6, Minor: 2, Patch: 20},
+	SupportsRocksDBV1:                 Version{Major: 7, Minor: 1, Patch: 0, ReleaseCandidate: 4},
+	SupportsIsPresent:                 Version{Major: 7, Minor: 1, Patch: 4},
+	SupportsShardedRocksDB:            Version{Major: 7, Minor: 2, Patch: 0},
+	SupportsRedwood1Experimental:      Version{Major: 7, Minor: 0, Patch: 0},
+	SupportsRedwood1:                  Version{Major: 7, Minor: 3, Patch: 0},
+	SupportsRecoveryState:             Version{Major: 7, Minor: 1, Patch: 22},
+	SupportsDNSInClusterFile:          Version{Major: 7, Minor: 0, Patch: 0},
+	SupportsLocalityBasedExclusions71: Version{Major: 7, Minor: 1, Patch: 42},
+	SupportsLocalityBasedExclusions:   Version{Major: 7, Minor: 3, Patch: 26},
 }
diff --git a/api/v1beta2/foundationdbcluster_types.go b/api/v1beta2/foundationdbcluster_types.go
index dd9afeee..d0a2244c 100644
--- a/api/v1beta2/foundationdbcluster_types.go
+++ b/api/v1beta2/foundationdbcluster_types.go
@@ -1086,7 +1086,8 @@ type FoundationDBClusterAutomationOptions struct {
 	// The default is false.
 	UseNonBlockingExcludes *bool `json:"useNonBlockingExcludes,omitempty"`
 
-	// UseLocalitiesForExclusion defines whether the exclusions are done using localities instead of IP addresses.
+	// UseLocalitiesForExclusion defines whether the exclusions are done using localities instead of IP addresses. This
+	// feature requires at least FDB 7.1.42 or 7.3.26.
 	// The default is false.
 	UseLocalitiesForExclusion *bool `json:"useLocalitiesForExclusion,omitempty"`
 
@@ -2248,13 +2249,14 @@ func (cluster *FoundationDBCluster) GetUseNonBlockingExcludes() bool {
 
 // UseLocalitiesForExclusion returns the value of UseLocalitiesForExclusion or false if unset.
 func (cluster *FoundationDBCluster) UseLocalitiesForExclusion() bool {
-	fdbVersion, err := ParseFdbVersion(cluster.Spec.Version)
+	fdbVersion, err := ParseFdbVersion(cluster.GetRunningVersion())
 	if err != nil {
 		// Fall back to use exclusions with IP if we can't parse the version.
 		// This should never happen since the version is validated in earlier steps.
 		return false
 	}
-	return fdbVersion.IsAtLeast(Versions.NextMajorVersion) && pointer.BoolDeref(cluster.Spec.AutomationOptions.UseLocalitiesForExclusion, false)
+
+	return fdbVersion.SupportsLocalityBasedExclusions() && pointer.BoolDeref(cluster.Spec.AutomationOptions.UseLocalitiesForExclusion, false)
 }
 
 // GetProcessClassLabel provides the label that this cluster is using for the
diff --git a/docs/cluster_spec.md b/docs/cluster_spec.md
index 2b42b90d..896f251e 100644
--- a/docs/cluster_spec.md
+++ b/docs/cluster_spec.md
@@ -183,7 +183,7 @@ FoundationDBClusterAutomationOptions provides flags for enabling or disabling op
 | replacements | Replacements contains options for automatically replacing failed processes. | [AutomaticReplacementOptions](#automaticreplacementoptions) | false |
 | ignorePendingPodsDuration | IgnorePendingPodsDuration defines how long a Pod has to be in the Pending Phase before ignore it during reconciliation. This prevents Pod that are stuck in Pending to block further reconciliation. | time.Duration | false |
 | useNonBlockingExcludes | UseNonBlockingExcludes defines whether the operator is allowed to use non blocking exclude commands. The default is false. | *bool | false |
-| useLocalitiesForExclusion | UseLocalitiesForExclusion defines whether the exclusions are done using localities instead of IP addresses. The default is false. | *bool | false |
+| useLocalitiesForExclusion | UseLocalitiesForExclusion defines whether the exclusions are done using localities instead of IP addresses. This feature requires at least FDB 7.1.42 or 7.3.26. The default is false. | *bool | false |
 | ignoreTerminatingPodsSeconds | IgnoreTerminatingPodsSeconds defines how long a Pod has to be in the Terminating Phase before we ignore it during reconciliation. This prevents Pod that are stuck in Terminating to block further reconciliation. | *int | false |
 | ignoreMissingProcessesSeconds | IgnoreMissingProcessesSeconds defines how long a process group has to be in the MissingProcess condition until it will be ignored during reconciliation. This prevents that a process will block reconciliation. | *int | false |
 | failedPodDurationSeconds | FailedPodDurationSeconds defines the duration a Pod can stay in the deleted state (deletionTimestamp != 0) before it gets marked as PodFailed. This is important in cases where a fdbserver process is still reporting but the Pod resource is marked for deletion. This can happen when the kubelet or a node fails. Setting this condition will ensure that the operator is replacing affected Pods. | *int | false |


Test patch:
diff --git a/api/v1beta2/foundationdb_version_test.go b/api/v1beta2/foundationdb_version_test.go
index 132a2314..a3f9bff5 100644
--- a/api/v1beta2/foundationdb_version_test.go
+++ b/api/v1beta2/foundationdb_version_test.go
@@ -251,4 +251,54 @@ var _ = Describe("[api] FDBVersion", func() {
 			false,
 		),
 	)
+
+	DescribeTable("validating if the provided version supports locality based exclusions", func(version Version, expected bool) {
+		Expect(version.SupportsLocalityBasedExclusions()).To(Equal(expected))
+	},
+		Entry(
+			"Version is 6.3",
+			Version{
+				Major: 6,
+				Minor: 3,
+				Patch: 0,
+			},
+			false,
+		),
+		Entry(
+			"Version is 7.1.41",
+			Version{
+				Major: 7,
+				Minor: 1,
+				Patch: 41,
+			},
+			false,
+		),
+		Entry(
+			"Version is 7.1.42",
+			Version{
+				Major: 7,
+				Minor: 1,
+				Patch: 42,
+			},
+			true,
+		),
+		Entry(
+			"Version is 7.3.25",
+			Version{
+				Major: 7,
+				Minor: 3,
+				Patch: 25,
+			},
+			false,
+		),
+		Entry(
+			"Version is 7.3.36",
+			Version{
+				Major: 7,
+				Minor: 3,
+				Patch: 26,
+			},
+			true,
+		),
+	)
 })
diff --git a/controllers/exclude_processes_test.go b/controllers/exclude_processes_test.go
index 95b9b184..a9da13f2 100644
--- a/controllers/exclude_processes_test.go
+++ b/controllers/exclude_processes_test.go
@@ -223,7 +223,7 @@ var _ = Describe("exclude_processes", func() {
 					Expect(missingProcesses).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-1", "storage-2"}))
 				})
 
-				FWhen("the missing timestamp is older than 5 minutes", func() {
+				When("the missing timestamp is older than 5 minutes", func() {
 					BeforeEach(func() {
 						for idx, processGroup := range cluster.Status.ProcessGroups {
 							timestamp := processGroup.GetConditionTime(fdbv1beta2.MissingProcesses)
@@ -509,7 +509,7 @@ var _ = Describe("exclude_processes", func() {
 
 		Context("cluster supports locality based exclusions", func() {
 			BeforeEach(func() {
-				cluster.Spec.Version = fdbv1beta2.Versions.NextMajorVersion.String()
+				cluster.Spec.Version = fdbv1beta2.Versions.SupportsLocalityBasedExclusions.String()
 			})
 
 			When("there are no exclusions", func() {
diff --git a/controllers/remove_process_groups_test.go b/controllers/remove_process_groups_test.go
index e501b741..f1460a4d 100644
--- a/controllers/remove_process_groups_test.go
+++ b/controllers/remove_process_groups_test.go
@@ -526,7 +526,7 @@ var _ = Describe("remove_process_groups", func() {
 
 		Context("cluster support inclusions using locality", func() {
 			BeforeEach(func() {
-				cluster.Spec.Version = fdbv1beta2.Versions.NextMajorVersion.String()
+				cluster.Spec.Version = fdbv1beta2.Versions.SupportsLocalityBasedExclusions.String()
 			})
 
 			When("including no process", func() {
diff --git a/controllers/update_status_test.go b/controllers/update_status_test.go
index 41f9a293..280bf3f7 100644
--- a/controllers/update_status_test.go
+++ b/controllers/update_status_test.go
@@ -65,10 +65,8 @@ var _ = Describe("update_status", func() {
 		taintValue := "unhealthy"
 
 		BeforeEach(func() {
-			var pods []*corev1.Pod
 			cluster = internal.CreateDefaultCluster()
-			err = setupClusterForTest(cluster)
-			Expect(err).NotTo(HaveOccurred())
+			Expect(setupClusterForTest(cluster)).NotTo(HaveOccurred())
 			// Define cluster's taint policy
 			cluster.Spec.AutomationOptions.Replacements.TaintReplacementOptions = []fdbv1beta2.TaintReplacementOption{
 				{
@@ -81,11 +79,9 @@ var _ = Describe("update_status", func() {
 				},
 			}
 
-			pods, err = clusterReconciler.PodLifecycleManager.GetPods(context.TODO(), clusterReconciler, cluster, internal.GetSinglePodListOptions(cluster, storageOneProcessGroupID)...)
+			processGroupStatus = fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, storageOneProcessGroupID)
+			pod, err = clusterReconciler.PodLifecycleManager.GetPod(context.TODO(), clusterReconciler, cluster, processGroupStatus.GetPodName(cluster))
 			Expect(err).NotTo(HaveOccurred())
-			Expect(len(pods)).To(Equal(1))
-
-			pod = pods[0]
 			node = &corev1.Node{
 				ObjectMeta: metav1.ObjectMeta{Name: pod.Spec.NodeName},
 			}
@@ -94,7 +90,6 @@ var _ = Describe("update_status", func() {
 			err = clusterReconciler.List(context.TODO(), allPvcs, internal.GetPodListOptions(cluster, "", "")...)
 			Expect(err).NotTo(HaveOccurred())
 
-			processGroupStatus = fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, internal.GetProcessGroupIDFromMeta(cluster, pod.ObjectMeta))
 			pvcMap := internal.CreatePVCMap(cluster, allPvcs)
 			pvcValue, pvcExists := pvcMap[processGroupStatus.ProcessGroupID]
 			if pvcExists {
@@ -296,11 +291,10 @@ var _ = Describe("update_status", func() {
 			cluster = internal.CreateDefaultCluster()
 			Expect(setupClusterForTest(cluster)).NotTo(HaveOccurred())
 
-			pods, err := clusterReconciler.PodLifecycleManager.GetPods(context.TODO(), clusterReconciler, cluster, internal.GetSinglePodListOptions(cluster, storageOneProcessGroupID)...)
-			Expect(err).NotTo(HaveOccurred())
-			Expect(pods).To(HaveLen(1))
+			processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, storageOneProcessGroupID)
 
-			storagePod = pods[0]
+			storagePod, err = clusterReconciler.PodLifecycleManager.GetPod(context.TODO(), clusterReconciler, cluster, processGroup.GetPodName(cluster))
+			Expect(err).NotTo(HaveOccurred())
 			for _, container := range storagePod.Spec.Containers {
 				storagePod.Status.ContainerStatuses = append(storagePod.Status.ContainerStatuses, corev1.ContainerStatus{Ready: true, Name: container.Name})
 			}
diff --git a/e2e/Makefile b/e2e/Makefile
index b44d6821..3030a134 100644
--- a/e2e/Makefile
+++ b/e2e/Makefile
@@ -10,9 +10,9 @@ TIMEOUT?=168h
 NAMESPACE?=
 PREFIX?=
 CONTEXT?=
-FDB_VERSION?=7.1.37
+FDB_VERSION?=7.1.43
 # This will be the version used for upgrade tests.
-NEXT_FDB_VERSION?=7.3.15
+NEXT_FDB_VERSION?=7.3.25
 ## Expectation is that you are running standard build image which generates both regular and debug (Symbols) images.
 FDB_IMAGE?=foundationdb/foundationdb:$(FDB_VERSION)
 SIDECAR_IMAGE?=foundationdb/foundationdb-kubernetes-sidecar:$(FDB_VERSION)-1