Remove the additional get status call for getting the coordinator set (…

…#1759)
FoundationDB · Jul 27, 2023 · c733b7c · c733b7c
1 parent dd9bfd0
commit c733b7c
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 25 deletions.
diff --git a/controllers/cluster_controller.go b/controllers/cluster_controller.go
@@ -411,16 +411,6 @@ func (r *FoundationDBClusterReconciler) newFdbPodClient(cluster *fdbv1beta2.Foun
 	return internal.NewFdbPodClient(cluster, pod, globalControllerLogger.WithValues("namespace", cluster.Namespace, "cluster", cluster.Name, "pod", pod.Name), r.GetTimeout, r.PostTimeout)
 }
 
-func (r *FoundationDBClusterReconciler) getCoordinatorSet(cluster *fdbv1beta2.FoundationDBCluster) (map[string]fdbv1beta2.None, error) {
-	adminClient, err := r.DatabaseClientProvider.GetAdminClient(cluster, r)
-	if err != nil {
-		return map[string]fdbv1beta2.None{}, err
-	}
-	defer adminClient.Close()
-
-	return adminClient.GetCoordinatorSet()
-}
-
 // updateOrApply updates the status either with server-side apply or if disabled with the normal update call.
 func (r *FoundationDBClusterReconciler) updateOrApply(ctx context.Context, cluster *fdbv1beta2.FoundationDBCluster) error {
 	if r.ServerSideApply {

diff --git a/controllers/remove_process_groups.go b/controllers/remove_process_groups.go
@@ -64,7 +64,8 @@ func (u removeProcessGroups) reconcile(ctx context.Context, r *FoundationDBClust
 		return &requeue{curError: err}
 	}
 
-	allExcluded, newExclusions, processGroupsToRemove := r.getProcessGroupsToRemove(logger, cluster, remainingMap)
+	coordinators := fdbstatus.GetCoordinatorsFromStatus(status)
+	allExcluded, newExclusions, processGroupsToRemove := r.getProcessGroupsToRemove(logger, cluster, remainingMap, coordinators)
 	// If no process groups are marked to remove we have to check if all process groups are excluded.
 	if len(processGroupsToRemove) == 0 {
 		if !allExcluded {
@@ -297,8 +298,7 @@ func getProcessesToInclude(cluster *fdbv1beta2.FoundationDBCluster, removedProce
 	return fdbProcessesToInclude
 }
 
-func (r *FoundationDBClusterReconciler) getProcessGroupsToRemove(logger logr.Logger, cluster *fdbv1beta2.FoundationDBCluster, remainingMap map[string]bool) (bool, bool, []*fdbv1beta2.ProcessGroupStatus) {
-	var cordSet map[string]fdbv1beta2.None
+func (r *FoundationDBClusterReconciler) getProcessGroupsToRemove(logger logr.Logger, cluster *fdbv1beta2.FoundationDBCluster, remainingMap map[string]bool, cordSet map[string]fdbv1beta2.None) (bool, bool, []*fdbv1beta2.ProcessGroupStatus) {
 	allExcluded := true
 	newExclusions := false
 	processGroupsToRemove := make([]*fdbv1beta2.ProcessGroupStatus, 0, len(cluster.Status.ProcessGroups))
@@ -308,17 +308,6 @@ func (r *FoundationDBClusterReconciler) getProcessGroupsToRemove(logger logr.Log
 			continue
 		}
 
-		// Only query FDB if we have a pending removal otherwise don't query FDB
-		if len(cordSet) == 0 {
-			var err error
-			cordSet, err = r.getCoordinatorSet(cluster)
-
-			if err != nil {
-				logger.Error(err, "Fetching coordinator set for removal")
-				return false, false, nil
-			}
-		}
-
 		if _, ok := cordSet[string(processGroup.ProcessGroupID)]; ok {
 			logger.Info("Block removal of Coordinator", "processGroupID", processGroup.ProcessGroupID)
 			allExcluded = false

diff --git a/controllers/remove_process_groups_test.go b/controllers/remove_process_groups_test.go
@@ -79,7 +79,11 @@ var _ = Describe("remove_process_groups", func() {
 					coordinatorIP: false,
 				}
 
-				allExcluded, newExclusions, processes := clusterReconciler.getProcessGroupsToRemove(globalControllerLogger, cluster, remaining)
+				coordSet := map[string]fdbv1beta2.None{
+					coordinatorIP: {},
+				}
+
+				allExcluded, newExclusions, processes := clusterReconciler.getProcessGroupsToRemove(globalControllerLogger, cluster, remaining, coordSet)
 				Expect(allExcluded).To(BeFalse())
 				Expect(processes).To(BeEmpty())
 				Expect(newExclusions).To(BeFalse())