Run the addPods reconciler before connecting to FDB in case we use DNS entries in the cluster file (#2283)

* Run the addPods reconciler before connecting to FDB in case we use DNS entries in the cluster file

Author: johscheuer

controllers/add_pods.go

@@ -114,7 +114,7 @@ func (a addPods) reconcile(ctx context.Context, r *FoundationDBClusterReconciler
 				return &requeue{curError: err, delayedRequeue: true}
 			}
 
-			return &requeue{curError: err}
+			return &requeue{curError: err, delayedRequeue: true}
 		}
 	}
 

controllers/cluster_controller.go

@@ -57,6 +57,39 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+// addPodsReconciler is the reconciler for addPods.
+var addPodsReconciler = addPods{}
+
+// subReconcilers has the ordered list of all reconcilers that should be used by the cluster controller.
+var subReconcilers = []clusterSubReconciler{
+	updateStatus{},
+	updateLockConfiguration{},
+	updateConfigMap{},
+	checkClientCompatibility{},
+	deletePodsForBuggification{},
+	replaceMisconfiguredProcessGroups{},
+	replaceFailedProcessGroups{},
+	addProcessGroups{},
+	addServices{},
+	addPVCs{},
+	addPodsReconciler,
+	generateInitialClusterFile{},
+	removeIncompatibleProcesses{},
+	updateSidecarVersions{},
+	updatePodConfig{},
+	updateMetadata{},
+	updateDatabaseConfiguration{},
+	chooseRemovals{},
+	excludeProcesses{},
+	changeCoordinators{},
+	bounceProcesses{},
+	maintenanceModeChecker{},
+	updatePods{},
+	removeProcessGroups{},
+	removeServices{},
+	updateStatus{},
+}
+
 // FoundationDBClusterReconciler reconciles a FoundationDBCluster object
 type FoundationDBClusterReconciler struct {
 	client.Client
@@ -148,20 +181,19 @@ func (r *FoundationDBClusterReconciler) Reconcile(ctx context.Context, request c
 		return ctrl.Result{}, err
 	}
 
+	err = cluster.Validate()
+	if err != nil {
+		r.Recorder.Event(cluster, corev1.EventTypeWarning, "ClusterSpec not valid", err.Error())
+		return ctrl.Result{}, fmt.Errorf("ClusterSpec is not valid: %w", err)
+	}
+
 	adminClient, err := r.getAdminClient(clusterLog, cluster)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
 	defer func() {
 		_ = adminClient.Close()
 	}()
-
-	err = cluster.Validate()
-	if err != nil {
-		r.Recorder.Event(cluster, corev1.EventTypeWarning, "ClusterSpec not valid", err.Error())
-		return ctrl.Result{}, fmt.Errorf("ClusterSpec is not valid: %w", err)
-	}
-
 	supportedVersion, err := adminClient.VersionSupported(cluster.Spec.Version)
 	if err != nil {
 		return ctrl.Result{}, err
@@ -170,6 +202,17 @@ func (r *FoundationDBClusterReconciler) Reconcile(ctx context.Context, request c
 		return ctrl.Result{}, fmt.Errorf("version %s is not supported", cluster.Spec.Version)
 	}
 
+	// When using DNS entries in the cluster file, we want to make sure to create pods if required before doing any
+	// interaction with the FDB cluster. If none of the coordinator pods is running, this will cause a crash in the FDB
+	// go bindings (or rather the C client). To prevent those case we run the addPods reconciler before interacting with
+	// the FoundationDB cluster.
+	if cluster.UseDNSInClusterFile() {
+		req := runClusterSubReconciler(ctx, clusterLog, addPodsReconciler, r, cluster, nil)
+		if req != nil && !req.delayedRequeue {
+			return processRequeue(req, addPodsReconciler, cluster, r.Recorder, clusterLog)
+		}
+	}
+
 	var status *fdbv1beta2.FoundationDBStatus
 	if cacheStatus {
 		clusterLog.Info("Fetch machine-readable status for reconcilitation loop", "cacheStatus", cacheStatus)
@@ -179,35 +222,6 @@ func (r *FoundationDBClusterReconciler) Reconcile(ctx context.Context, request c
 		}
 	}
 
-	subReconcilers := []clusterSubReconciler{
-		updateStatus{},
-		updateLockConfiguration{},
-		updateConfigMap{},
-		checkClientCompatibility{},
-		deletePodsForBuggification{},
-		replaceMisconfiguredProcessGroups{},
-		replaceFailedProcessGroups{},
-		addProcessGroups{},
-		addServices{},
-		addPVCs{},
-		addPods{},
-		generateInitialClusterFile{},
-		removeIncompatibleProcesses{},
-		updateSidecarVersions{},
-		updatePodConfig{},
-		updateMetadata{},
-		updateDatabaseConfiguration{},
-		chooseRemovals{},
-		excludeProcesses{},
-		changeCoordinators{},
-		bounceProcesses{},
-		maintenanceModeChecker{},
-		updatePods{},
-		removeProcessGroups{},
-		removeServices{},
-		updateStatus{},
-	}
-
 	originalGeneration := cluster.ObjectMeta.Generation
 	normalizedSpec := cluster.Spec.DeepCopy()
 	var delayedRequeueDuration time.Duration

controllers/update_status.go

@@ -166,7 +166,8 @@ func (c updateStatus) reconcile(ctx context.Context, r *FoundationDBClusterRecon
 
 	if clusterStatus.ConnectionString == "" {
 		connectionString, ok := existingConfigMap.Data[fdbv1beta2.ClusterFileKey]
-		if ok {
+		// Only update the connection string if the connection string value is not empty.
+		if ok && connectionString != "" {
 			clusterStatus.ConnectionString = connectionString
 		} else {
 			clusterStatus.ConnectionString = cluster.Spec.SeedConnectionString

e2e/test_operator/operator_test.go

@@ -2076,13 +2076,8 @@ var _ = Describe("Operator", Label("e2e", "pr"), func() {
 				// Make sure the operator is not taking any action to prevent any race condition.
 				fdbCluster.SetSkipReconciliation(true)
 
-				// Delete all Pods
-				pods := fdbCluster.GetPods()
-				initialPodsCnt = len(pods.Items)
-				for _, pod := range pods.Items {
-					podToDelete := &pod
-					factory.Delete(podToDelete)
-				}
+				// Delete all Pods, including the operator pods.
+				Expect(factory.GetControllerRuntimeClient().DeleteAllOf(context.Background(), &corev1.Pod{}, ctrlClient.InNamespace(fdbCluster.Namespace()))).To(Succeed())
 
 				// Make sure the Pods are all deleted.
 				Eventually(func() []corev1.Pod {