From 532cb7e187d355cd9f2a718a3ff9582f747e6e47 Mon Sep 17 00:00:00 2001
From: "Johannes M. Scheuermann" <jscheuermann@apple.com>
Date: Fri, 29 Sep 2023 17:33:06 +0200
Subject: [PATCH] Add e2e test for changing a cluster to use DNS in cluster
 files

---
 controllers/change_coordinators.go | 15 ++++++++++++++-
 e2e/test_operator/operator_test.go | 31 ++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/controllers/change_coordinators.go b/controllers/change_coordinators.go
index 42d7d4f56..a4dc4362e 100644
--- a/controllers/change_coordinators.go
+++ b/controllers/change_coordinators.go
@@ -108,7 +108,6 @@ func (c changeCoordinators) reconcile(ctx context.Context, r *FoundationDBCluste
 	return nil
 }
 
-// TODO move them into separate package?
 // selectCandidates is a helper for Reconcile that picks non-excluded, not-being-removed class-matching process groups.
 func selectCandidates(cluster *fdbv1beta2.FoundationDBCluster, status *fdbv1beta2.FoundationDBStatus) ([]locality.Info, error) {
 	candidates := make([]locality.Info, 0, len(status.Cluster.Processes))
@@ -121,6 +120,19 @@ func selectCandidates(cluster *fdbv1beta2.FoundationDBCluster, status *fdbv1beta
 			continue
 		}
 
+		// Ignore processes with missing locality, see: https://github.com/FoundationDB/fdb-kubernetes-operator/issues/1254
+		if len(process.Locality) == 0 {
+			continue
+		}
+
+		// If the cluster should be using DNS in the cluster file we should make use the locality is set.
+		if cluster.UseDNSInClusterFile() {
+			_, ok := process.Locality[fdbv1beta2.FDBLocalityDNSNameKey]
+			if !ok {
+				continue
+			}
+		}
+
 		if cluster.ProcessGroupIsBeingRemoved(fdbv1beta2.ProcessGroupID(process.Locality[fdbv1beta2.FDBLocalityInstanceIDKey])) {
 			continue
 		}
@@ -187,5 +199,6 @@ func getCoordinatorAddress(cluster *fdbv1beta2.FoundationDBCluster, locality loc
 			Flags:         address.Flags,
 		}
 	}
+
 	return address
 }
diff --git a/e2e/test_operator/operator_test.go b/e2e/test_operator/operator_test.go
index 191613126..0f9b48063 100644
--- a/e2e/test_operator/operator_test.go
+++ b/e2e/test_operator/operator_test.go
@@ -1354,6 +1354,37 @@ var _ = Describe("Operator", Label("e2e", "pr"), func() {
 		})
 	})
 
+	When("migrating a cluster to make use of DNS in the cluster file", func() {
+		BeforeEach(func() {
+			cluster := fdbCluster.GetCluster()
+			parsedVersion, err := fdbv1beta2.ParseFdbVersion(cluster.Status.RunningVersion)
+			Expect(err).NotTo(HaveOccurred())
+
+			// TODO (johscheuer): Change this once https://github.com/FoundationDB/fdb-kubernetes-operator/pull/1820 is merged.
+			// if parsedVersion.SupportsDNSInClusterFile() {
+			if !parsedVersion.IsAtLeast(fdbv1beta2.Version{Major: 7, Minor: 0, Patch: 0}) {
+				Skip(fmt.Sprintf("current FoundationDB version %s doesn't support DNS", parsedVersion.String()))
+			}
+
+			if cluster.UseDNSInClusterFile() {
+				Skip("cluster already uses DNS")
+			}
+
+			Expect(fdbCluster.SetUseDNSInClusterFile(true)).ToNot(HaveOccurred())
+		})
+
+		It("should migrate the cluster", func() {
+			cluster := fdbCluster.GetCluster()
+			Eventually(func() string {
+				return fdbCluster.GetStatus().Cluster.ConnectionString
+			}).Should(ContainSubstring(cluster.GetDNSDomain()))
+		})
+
+		AfterEach(func() {
+			Expect(fdbCluster.SetUseDNSInClusterFile(false)).ToNot(HaveOccurred())
+		})
+	})
+
 	// This test is pending, as all Pods will be restarted at the same time, which will lead to unavailability without
 	// using DNS.
 	PWhen("crash looping the sidecar for all Pods", func() {