If the database is unavailable and caching is enabled, allow operator to proceed (#1932)

* If the database is unavailable and caching is enabled we should allow the operator to proceed and just try to refetch the machine-readable status later again

Author: johscheuer

controllers/cluster_controller.go

@@ -137,7 +137,7 @@ func (r *FoundationDBClusterReconciler) Reconcile(ctx context.Context, request c
 		clusterLog.Info("Fetch machine-readable status for reconcilitation loop", "cacheStatus", cacheStatus)
 		status, err = r.getStatusFromClusterOrDummyStatus(clusterLog, cluster)
 		if err != nil {
-			return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, err
+			clusterLog.Info("could not fetch machine-readable status and therefore didn't cache the it")
 		}
 	}
 

controllers/replace_failed_process_groups.go

@@ -48,13 +48,13 @@ func (c replaceFailedProcessGroups) reconcile(ctx context.Context, r *Foundation
 	if status == nil {
 		adminClient, err := r.DatabaseClientProvider.GetAdminClient(cluster, r)
 		if err != nil {
-			return &requeue{curError: err}
+			return &requeue{curError: err, delayedRequeue: true}
 		}
 		defer adminClient.Close()
 
 		status, err = adminClient.GetStatus()
 		if err != nil {
-			return &requeue{curError: err}
+			return &requeue{curError: err, delayedRequeue: true}
 		}
 	}
 

controllers/update_lock_configuration.go

@@ -40,12 +40,12 @@ func (updateLockConfiguration) reconcile(_ context.Context, r *FoundationDBClust
 
 	lockClient, err := r.getLockClient(cluster)
 	if err != nil {
-		return &requeue{curError: err}
+		return &requeue{curError: err, delayedRequeue: true}
 	}
 
 	err = lockClient.UpdateDenyList(cluster.Spec.LockOptions.DenyList)
 	if err != nil {
-		return &requeue{curError: err}
+		return &requeue{curError: err, delayedRequeue: true}
 	}
 
 	return nil

controllers/update_status.go

@@ -67,7 +67,7 @@ func (updateStatus) reconcile(ctx context.Context, r *FoundationDBClusterReconci
 		var err error
 		databaseStatus, err = r.getStatusFromClusterOrDummyStatus(logger, cluster)
 		if err != nil {
-			return &requeue{curError: fmt.Errorf("update_status error fetching status: %w", err)}
+			return &requeue{curError: fmt.Errorf("update_status error fetching status: %w", err), delayedRequeue: true}
 		}
 	}
 

e2e/test_operator/operator_test.go

@@ -49,9 +49,10 @@ import (
 )
 
 var (
-	factory     *fixtures.Factory
-	fdbCluster  *fixtures.FdbCluster
-	testOptions *fixtures.FactoryOptions
+	factory               *fixtures.Factory
+	fdbCluster            *fixtures.FdbCluster
+	testOptions           *fixtures.FactoryOptions
+	scheduleInjectPodKill *fixtures.ChaosMeshExperiment
 )
 
 func init() {
@@ -81,7 +82,7 @@ var _ = BeforeSuite(func() {
 
 	// In order to test the robustness of the operator we try to kill the operator Pods every minute.
 	if factory.ChaosTestsEnabled() {
-		factory.ScheduleInjectPodKill(
+		scheduleInjectPodKill = factory.ScheduleInjectPodKill(
 			fixtures.GetOperatorSelector(fdbCluster.Namespace()),
 			"*/2 * * * *",
 			chaosmesh.OneMode,
@@ -1893,4 +1894,86 @@ var _ = Describe("Operator", Label("e2e", "pr"), func() {
 			})
 		})
 	})
+
+	When("the cluster makes use of DNS in the cluster file", func() {
+		var initialSetting bool
+
+		BeforeEach(func() {
+			// Until the race condition is resolved in the FDB go bindings make sure the operator is not restarted.
+			factory.DeleteChaosMeshExperimentSafe(scheduleInjectPodKill)
+			cluster := fdbCluster.GetCluster()
+			parsedVersion, err := fdbv1beta2.ParseFdbVersion(cluster.Status.RunningVersion)
+			Expect(err).NotTo(HaveOccurred())
+
+			if !parsedVersion.SupportsDNSInClusterFile() {
+				Skip(fmt.Sprintf("current FoundationDB version %s doesn't support DNS", parsedVersion.String()))
+			}
+
+			initialSetting = cluster.UseDNSInClusterFile()
+			if !cluster.UseDNSInClusterFile() {
+				Expect(fdbCluster.SetUseDNSInClusterFile(true)).ToNot(HaveOccurred())
+			}
+		})
+
+		AfterEach(func() {
+			Expect(fdbCluster.SetUseDNSInClusterFile(initialSetting)).ToNot(HaveOccurred())
+
+			if factory.ChaosTestsEnabled() {
+				scheduleInjectPodKill = factory.ScheduleInjectPodKill(
+					fixtures.GetOperatorSelector(fdbCluster.Namespace()),
+					"*/2 * * * *",
+					chaosmesh.OneMode,
+				)
+			}
+		})
+
+		When("all Pods are deleted", func() {
+			var initialPodsCnt int
+			var initialReplaceTime time.Duration
+
+			BeforeEach(func() {
+				availabilityCheck = false
+				initialReplaceTime = time.Duration(pointer.IntDeref(
+					fdbCluster.GetClusterSpec().AutomationOptions.Replacements.FailureDetectionTimeSeconds,
+					90,
+				)) * time.Second
+				Expect(fdbCluster.SetAutoReplacements(false, 30*time.Hour)).ShouldNot(HaveOccurred())
+				// Make sure the operator is not taking any action to prevent any race condition.
+				Expect(fdbCluster.SetSkipReconciliation(true)).NotTo(HaveOccurred())
+
+				// Delete all Pods
+				pods := fdbCluster.GetPods()
+				initialPodsCnt = len(pods.Items)
+				for _, pod := range pods.Items {
+					podToDelete := &pod
+					factory.Delete(podToDelete)
+				}
+
+				// Make sure the Pods are all deleted.
+				Eventually(func() []corev1.Pod {
+					return fdbCluster.GetPods().Items
+				}).WithTimeout(5 * time.Minute).WithPolling(2 * time.Second).Should(BeEmpty())
+
+				// Enable the operator again
+				Expect(fdbCluster.SetSkipReconciliation(false)).NotTo(HaveOccurred())
+			})
+
+			It("should recreate all Pods and bring the cluster into a healthy state again", func() {
+				Eventually(func() int {
+					return len(fdbCluster.GetPods().Items)
+				}).WithTimeout(5 * time.Minute).WithPolling(2 * time.Second).Should(BeNumerically(">=", initialPodsCnt))
+
+				Eventually(func() bool {
+					return fdbCluster.GetStatus().Client.DatabaseStatus.Available
+				}).WithTimeout(5 * time.Minute).WithPolling(2 * time.Second).Should(BeTrue())
+
+				Expect(fdbCluster.WaitForReconciliation()).NotTo(HaveOccurred())
+			})
+
+			AfterEach(func() {
+				Expect(fdbCluster.SetSkipReconciliation(false)).NotTo(HaveOccurred())
+				Expect(fdbCluster.SetAutoReplacements(true, initialReplaceTime)).ShouldNot(HaveOccurred())
+			})
+		})
+	})
 })