From 99966814cea6f655466003d6f70a37148437aa4f Mon Sep 17 00:00:00 2001 From: "Johannes M. Scheuermann" Date: Fri, 29 Sep 2023 17:33:06 +0200 Subject: [PATCH 1/3] 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 2029c1ad0..bdea34395 100644 --- a/e2e/test_operator/operator_test.go +++ b/e2e/test_operator/operator_test.go @@ -1353,6 +1353,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() { From 77ba0111850e9f6620115440b2bc7357105b13bb Mon Sep 17 00:00:00 2001 From: "Johannes M. Scheuermann" Date: Thu, 5 Oct 2023 10:30:26 +0200 Subject: [PATCH 2/3] Update if statement --- e2e/test_operator/operator_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/e2e/test_operator/operator_test.go b/e2e/test_operator/operator_test.go index bdea34395..9583068c0 100644 --- a/e2e/test_operator/operator_test.go +++ b/e2e/test_operator/operator_test.go @@ -1359,9 +1359,7 @@ var _ = Describe("Operator", Label("e2e", "pr"), func() { 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}) { + if !parsedVersion.SupportsDNSInClusterFile() { Skip(fmt.Sprintf("current FoundationDB version %s doesn't support DNS", parsedVersion.String())) } From 678cb425a8d32986ba5117d08ed8a3d1094e64b1 Mon Sep 17 00:00:00 2001 From: Johannes Scheuermann Date: Thu, 5 Oct 2023 10:36:15 +0200 Subject: [PATCH 3/3] Update controllers/change_coordinators.go --- controllers/change_coordinators.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/change_coordinators.go b/controllers/change_coordinators.go index a4dc4362e..e37ec8f7e 100644 --- a/controllers/change_coordinators.go +++ b/controllers/change_coordinators.go @@ -125,7 +125,7 @@ func selectCandidates(cluster *fdbv1beta2.FoundationDBCluster, status *fdbv1beta continue } - // If the cluster should be using DNS in the cluster file we should make use the locality is set. + // If the cluster should be using DNS in the cluster file we should make sure the locality is set. if cluster.UseDNSInClusterFile() { _, ok := process.Locality[fdbv1beta2.FDBLocalityDNSNameKey] if !ok {