Change the ordering of the coordinator change and the exclusion

controllers/exclude_processes.go

@@ -166,6 +166,19 @@ func (e excludeProcesses) reconcile(ctx context.Context, r *FoundationDBClusterR
 		}
 	}
 
+	r.Recorder.Event(cluster, corev1.EventTypeNormal, "ExcludingProcesses", fmt.Sprintf("Excluding %v", fdbProcessesToExclude))
+	// We use the no_wait exclusion here to trigger the exclusion without waiting for the data movement to complete.
+	// There is no need to wait for the data movement to complete in this call as later calls will verify that the
+	// data is moved and the processes are fully excluded. Using the no_wait flag here will reduce the timeout errors
+	// as those are hit most of the time if at least one storage process is included in the exclusion list.
+	err = adminClient.ExcludeProcessesWithNoWait(fdbProcessesToExclude, true)
+	// Reset the SecondsSinceLastRecovered since the operator just excluded some processes, which will cause a recovery.
+	status.Cluster.RecoveryState.SecondsSinceLastRecovered = 0.0
+	// If the exclusion failed, we don't want to change the coordinators and delay the coordinators change to a later time.
+	if err != nil {
+		return &requeue{curError: err, delayedRequeue: true}
+	}
+
 	var coordinatorExcluded bool
 	for _, excludeProcess := range fdbProcessesToExclude {
 		excludeString := excludeProcess.String()
@@ -178,25 +191,11 @@ func (e excludeProcesses) reconcile(ctx context.Context, r *FoundationDBClusterR
 		}
 	}
 
-	var coordinatorErr error
-	// If a coordinator should be excluded, we will change the coordinators before doing the exclusion. This should reduce the
-	// observed recoveries, see: https://github.com/FoundationDB/fdb-kubernetes-operator/v2/issues/2018.
-	if coordinatorExcluded {
-		coordinatorErr = coordinator.ChangeCoordinators(logger, adminClient, cluster, status)
-	}
-
-	r.Recorder.Event(cluster, corev1.EventTypeNormal, "ExcludingProcesses", fmt.Sprintf("Excluding %v", fdbProcessesToExclude))
-	// We use the no_wait exclusion here to trigger the exclusion without waiting for the data movement to complete.
-	// There is no need to wait for the data movement to complete in this call as later calls will verify that the
-	// data is moved and the processes are fully excluded. Using the no_wait flag here will reduce the timeout errors
-	// as those are hit most of the time if at least one storage process is included in the exclusion list.
-	err = adminClient.ExcludeProcessesWithNoWait(fdbProcessesToExclude, true)
-	if err != nil {
-		return &requeue{curError: err, delayedRequeue: true}
-	}
-
 	// Only if a coordinator was excluded we have to check for an error and update the cluster.
 	if coordinatorExcluded {
+		// If a coordinator should be excluded, we will change the coordinators directly after the exclusion.
+		// This should reduce the observed recoveries, see: https://github.com/FoundationDB/fdb-kubernetes-operator/v2/issues/2018.
+		coordinatorErr := coordinator.ChangeCoordinators(logger, adminClient, cluster, status)
 		if coordinatorErr != nil {
 			return &requeue{curError: coordinatorErr, delayedRequeue: true}
 		}
@@ -207,9 +206,6 @@ func (e excludeProcesses) reconcile(ctx context.Context, r *FoundationDBClusterR
 		}
 	}
 
-	// Reset the SecondsSinceLastRecovered since the operator just excluded some processes, which will cause a recovery.
-	status.Cluster.RecoveryState.SecondsSinceLastRecovered = 0.0
-
 	return nil
 }
 

internal/locality/locality.go

@@ -384,7 +384,6 @@ func CheckCoordinatorValidity(logger logr.Logger, cluster *fdbv1beta2.Foundation
 			Flags:         ipAddress.Flags,
 		}
 		_, isCoordinatorWithDNS := coordinatorStatus[dnsAddress.String()]
-
 		if !isCoordinatorWithDNS {
 			dnsAddress = ipAddress
 			dnsAddress.FromHostname = true