Fix the e2e test case for the new FDB version that supports to remove old tester processes automatically (#1955)

Author: johscheuer

api/v1beta2/foundationdb_version.go

@@ -232,6 +232,16 @@ func (version Version) SupportsLocalityBasedExclusions() bool {
 	return version.IsAtLeast(Versions.SupportsLocalityBasedExclusions)
 }
 
+// AutomaticallyRemovesDeadTesterProcesses returns true if the FDB version automatically removes old tester processes
+// from the list of processes.
+func (version Version) AutomaticallyRemovesDeadTesterProcesses() bool {
+	if version.IsProtocolCompatible(Version{Major: 7, Minor: 3, Patch: 0}) {
+		return version.IsAtLeast(Version{Major: 7, Minor: 1, Patch: 35})
+	}
+
+	return version.IsAtLeast(Version{Major: 7, Minor: 1, Patch: 55})
+}
+
 // Versions provides a shorthand for known versions.
 // This is only to be used in testing.
 var Versions = struct {

controllers/bounce_processes.go

@@ -78,7 +78,7 @@ func (bounceProcesses) reconcile(_ context.Context, r *FoundationDBClusterReconc
 		// Check if the status contains unreachable tester processes. In this case the cluster controller must be restarted.
 		// Otherwise the status will contain a message with "status_incomplete" and "unreachable_processes". Those messages
 		// could block further actions like the check if a process is exclude and doesn't serve any roles.
-		clusterControllerAddress := checkIfClusterControllerNeedsRestart(status)
+		clusterControllerAddress := checkIfClusterControllerNeedsRestart(logger, cluster, status)
 		if clusterControllerAddress != nil {
 			logger.Info("found unreachable tester processes in status which requires a cluster controller restart")
 			// Adding the same address twice is not a problem for the kill command, so we can just append the returned address.
@@ -351,8 +351,20 @@ func getAddressesForUpgrade(logger logr.Logger, r *FoundationDBClusterReconciler
 // this might be required. One case is when at least on tester process is running in the cluster and that tester process
 // fails. Currently this leads to the cluster controller reporting unreachable processes and the status incomplete message.
 // Having those messages in the cluster's machine-readable status could block some operations of the operator and the
-// solution to that is to restart the cluster controller process.
-func checkIfClusterControllerNeedsRestart(status *fdbv1beta2.FoundationDBStatus) *fdbv1beta2.ProcessAddress {
+// solution to that is to restart the cluster controller process. If the FDB version supports to remove the old tester
+// worker automatically this step will return no processes to be restarted.
+func checkIfClusterControllerNeedsRestart(logger logr.Logger, cluster *fdbv1beta2.FoundationDBCluster, status *fdbv1beta2.FoundationDBStatus) *fdbv1beta2.ProcessAddress {
+	runningVersion, err := fdbv1beta2.ParseFdbVersion(cluster.GetRunningVersion())
+	if err != nil {
+		logger.Error(err, "could not parse running version in checkIfClusterControllerNeedsRestart")
+		return nil
+	}
+
+	// If the cluster controller automatically removes the dead tester processes, the operator can skip any further work.
+	if runningVersion.AutomaticallyRemovesDeadTesterProcesses() {
+		return nil
+	}
+
 	// If the status contains no cluster messages we can skip further check.
 	if len(status.Cluster.Messages) == 0 {
 		return nil

controllers/bounce_processes_test.go

@@ -723,6 +723,30 @@ var _ = Describe("bounceProcesses", func() {
 					Expect(adminClient.KilledAddresses).To(BeEmpty())
 				})
 			})
+
+			When("the FDB version automatically removes old tester processes", func() {
+				var previousVersion string
+
+				BeforeEach(func() {
+					previousVersion = cluster.Status.RunningVersion
+					cluster.Spec.Version = "7.1.57"
+					cluster.Status.RunningVersion = "7.1.57"
+				})
+
+				AfterEach(func() {
+					cluster.Status.RunningVersion = previousVersion
+					cluster.Spec.Version = previousVersion
+				})
+
+				It("should not requeue", func() {
+					Expect(err).NotTo(HaveOccurred())
+					Expect(requeue).To(BeNil())
+				})
+
+				It("should not kill the cluster controller", func() {
+					Expect(adminClient.KilledAddresses).To(BeEmpty())
+				})
+			})
 		})
 
 		When("the unreachable processes include no tester processes", func() {
@@ -774,7 +798,7 @@ var _ = Describe("bounceProcesses", func() {
 			})
 		})
 
-		When("the cluster message doesn not contain unreachable processes", func() {
+		When("the cluster message does not contain unreachable processes", func() {
 			BeforeEach(func() {
 				adminClient, err = mock.NewMockAdminClientUncast(cluster, k8sClient)
 				Expect(err).NotTo(HaveOccurred())

e2e/Makefile

@@ -10,9 +10,9 @@ TIMEOUT?=168h
 CLUSTER_NAME?=
 NAMESPACE?=
 CONTEXT?=
-FDB_VERSION?=7.1.53
+FDB_VERSION?=7.1.57
 # This will be the version used for upgrade tests.
-NEXT_FDB_VERSION?=7.3.29
+NEXT_FDB_VERSION?=7.3.33
 ## 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

e2e/test_operator/operator_test.go

@@ -1750,7 +1750,17 @@ var _ = Describe("Operator", Label("e2e", "pr"), func() {
 	})
 
 	When("running with tester processes", func() {
+		fdbAutomaticallyRemoveOldTester := false
+
 		BeforeEach(func() {
+			fdbVersion := fdbCluster.GetCluster().GetRunningVersion()
+			version, err := fdbv1beta2.ParseFdbVersion(fdbVersion)
+			Expect(err).NotTo(HaveOccurred())
+
+			if version.AutomaticallyRemovesDeadTesterProcesses() {
+				fdbAutomaticallyRemoveOldTester = true
+			}
+
 			// We will be restarting the CC, so we can ignore this check.
 			availabilityCheck = false
 			spec := fdbCluster.GetCluster().Spec.DeepCopy()
@@ -1789,25 +1799,27 @@ var _ = Describe("Operator", Label("e2e", "pr"), func() {
 					factory.Delete(&pod)
 				}
 
-				// Wait until the cluster shows the unreachable process.
-				Eventually(func() []string {
-					status := fdbCluster.GetStatus()
+				if !fdbAutomaticallyRemoveOldTester {
+					// Wait until the cluster shows the unreachable process.
+					Eventually(func() []string {
+						status := fdbCluster.GetStatus()
 
-					messages := make([]string, 0, len(status.Cluster.Messages))
-					for _, message := range status.Cluster.Messages {
-						messages = append(messages, message.Name)
-					}
+						messages := make([]string, 0, len(status.Cluster.Messages))
+						for _, message := range status.Cluster.Messages {
+							messages = append(messages, message.Name)
+						}
 
-					log.Println("current messages:", messages)
+						log.Println("current messages:", messages)
 
-					return messages
-				}).WithPolling(1 * time.Second).WithTimeout(2 * time.Minute).MustPassRepeatedly(5).Should(ContainElements("status_incomplete", "unreachable_processes"))
+						return messages
+					}).WithPolling(1 * time.Second).WithTimeout(2 * time.Minute).MustPassRepeatedly(5).Should(ContainElements("status_incomplete", "unreachable_processes"))
+				}
 
 				// Let the operator fix the issue.
 				Expect(fdbCluster.SetSkipReconciliation(false)).NotTo(HaveOccurred())
 			})
 
-			It("should restart the cluster controller", func() {
+			It("should show the status without any messages", func() {
 				// The operator should be restarting the cluster controller and this should clean the unreachable_processes
 				Eventually(func() []string {
 					status := fdbCluster.GetStatus()
@@ -1854,19 +1866,21 @@ var _ = Describe("Operator", Label("e2e", "pr"), func() {
 					chaosmesh.From,
 				)
 
-				// Wait until the cluster shows the unreachable process.
-				Eventually(func() []string {
-					status := fdbCluster.GetStatus()
+				if !fdbAutomaticallyRemoveOldTester {
+					// Wait until the cluster shows the unreachable process.
+					Eventually(func() []string {
+						status := fdbCluster.GetStatus()
 
-					messages := make([]string, 0, len(status.Cluster.Messages))
-					for _, message := range status.Cluster.Messages {
-						messages = append(messages, message.Name)
-					}
+						messages := make([]string, 0, len(status.Cluster.Messages))
+						for _, message := range status.Cluster.Messages {
+							messages = append(messages, message.Name)
+						}
 
-					log.Println("current messages:", messages)
+						log.Println("current messages:", messages)
 
-					return messages
-				}).WithPolling(1 * time.Second).WithTimeout(2 * time.Minute).MustPassRepeatedly(5).Should(ContainElements("status_incomplete", "unreachable_processes"))
+						return messages
+					}).WithPolling(1 * time.Second).WithTimeout(2 * time.Minute).MustPassRepeatedly(5).Should(ContainElements("status_incomplete", "unreachable_processes"))
+				}
 
 				// Let the operator fix the issue.
 				Expect(fdbCluster.SetSkipReconciliation(false)).NotTo(HaveOccurred())
@@ -1876,8 +1890,7 @@ var _ = Describe("Operator", Label("e2e", "pr"), func() {
 				factory.DeleteChaosMeshExperimentSafe(exp)
 			})
 
-			// TODO what will be the status here?
-			It("should restart the cluster controller", func() {
+			It("should show the status without any messages", func() {
 				// The operator should be restarting the cluster controller and this should clean the unreachable_processes
 				Eventually(func() []string {
 					status := fdbCluster.GetStatus()