Repository: FoundationDB/fdb-kubernetes-operator
Pull request #1834 (2023-10-12 11:50:27)

Add failure condition to logging in automatic replacements
# Description

This change adds additional information to the replacement logic for failed process groups. Historically we only replaced processes based on one condition, but now in the current implementation we have multiple reasons that could lead to an automatic replacement. Adding this information to the logging output, makes it easier to understand why the operator started the replacement.

## Type of change

*Please select one of the options below.*

- Other (improve logging)

## Discussion

-

## Testing

Updated unit tests + CI will run e2e tests.

## Documentation

-

## Follow-up

-



Patch:
diff --git a/api/v1beta2/foundationdbcluster_types.go b/api/v1beta2/foundationdbcluster_types.go
index b400ecfc..6a5b7d5f 100644
--- a/api/v1beta2/foundationdbcluster_types.go
+++ b/api/v1beta2/foundationdbcluster_types.go
@@ -480,15 +480,19 @@ func (processGroupStatus *ProcessGroupStatus) GetPodName(cluster *FoundationDBCl
 	return sb.String()
 }
 
-// NeedsReplacement checks if the ProcessGroupStatus has conditions so that it should be removed
-func (processGroupStatus *ProcessGroupStatus) NeedsReplacement(failureTime int, taintReplacementTime int) (bool, int64) {
+// NeedsReplacement checks if the ProcessGroupStatus has conditions that require a replacement of the failed Process Group.
+// The method will return the failure condition and the timestamp. If no failure is detected an empty condition and a 0
+// will be returned.
+func (processGroupStatus *ProcessGroupStatus) NeedsReplacement(failureTime int, taintReplacementTime int) (ProcessGroupConditionType, int64) {
 	var earliestFailureTime int64 = math.MaxInt64
 	var earliestTaintReplacementTime int64 = math.MaxInt64
 
+	// If the process group is already marked for removal we can ignore it.
 	if processGroupStatus.IsMarkedForRemoval() {
-		return false, 0
+		return "", 0
 	}
 
+	var failureCondition ProcessGroupConditionType
 	for _, conditionType := range conditionsThatNeedReplacement {
 		conditionTimePtr := processGroupStatus.GetConditionTime(conditionType)
 		if conditionTimePtr == nil {
@@ -500,23 +504,29 @@ func (processGroupStatus *ProcessGroupStatus) NeedsReplacement(failureTime int,
 			if earliestTaintReplacementTime > conditionTime {
 				earliestTaintReplacementTime = conditionTime
 			}
-		} else {
-			if earliestFailureTime > conditionTime {
-				earliestFailureTime = conditionTime
-			}
+
+			failureCondition = conditionType
+			continue
+		}
+
+		if earliestFailureTime > conditionTime {
+			earliestFailureTime = conditionTime
+			failureCondition = conditionType
 		}
 	}
 
 	failureWindowStart := time.Now().Add(-1 * time.Duration(failureTime) * time.Second).Unix()
 	if earliestFailureTime < failureWindowStart {
-		return true, earliestFailureTime
+		return failureCondition, earliestFailureTime
 	}
+
 	taintWindowStart := time.Now().Add(-1 * time.Duration(taintReplacementTime) * time.Second).Unix()
 	if earliestTaintReplacementTime < taintWindowStart {
-		return true, earliestTaintReplacementTime
+		return failureCondition, earliestTaintReplacementTime
 	}
 
-	return false, 0
+	// No failure detected.
+	return "", 0
 }
 
 // AddAddresses adds the new address to the ProcessGroupStatus and removes duplicates and old addresses
@@ -809,6 +819,11 @@ func (processGroupStatus *ProcessGroupStatus) GetConditionTime(conditionType Pro
 
 // IsUnderMaintenance checks if the process is in maintenance zone.
 func (processGroupStatus *ProcessGroupStatus) IsUnderMaintenance(maintenanceZone FaultDomain) bool {
+	// Only storage processes are affected by the maintenance zone.
+	if processGroupStatus.ProcessClass != ProcessClassStorage {
+		return false
+	}
+
 	// If the maintenanceZone is not set or the Process Group has not fault domain set, return false.
 	if maintenanceZone == "" || processGroupStatus.FaultDomain == "" {
 		return false
diff --git a/go.mod b/go.mod
index ad9ee8cc..17350722 100644
--- a/go.mod
+++ b/go.mod
@@ -91,11 +91,11 @@ require (
 	go.uber.org/atomic v1.9.0 // indirect
 	go.uber.org/multierr v1.8.0 // indirect
 	go.uber.org/zap v1.24.0 // indirect
-	golang.org/x/net v0.10.0 // indirect
+	golang.org/x/net v0.17.0 // indirect
 	golang.org/x/oauth2 v0.4.0 // indirect
-	golang.org/x/sys v0.8.0 // indirect
-	golang.org/x/term v0.8.0 // indirect
-	golang.org/x/text v0.9.0 // indirect
+	golang.org/x/sys v0.13.0 // indirect
+	golang.org/x/term v0.13.0 // indirect
+	golang.org/x/text v0.13.0 // indirect
 	golang.org/x/time v0.3.0 // indirect
 	golang.org/x/tools v0.9.1 // indirect
 	gomodules.xyz/jsonpatch/v2 v2.2.0 // indirect
diff --git a/go.sum b/go.sum
index 90ab3488..b0f95dc9 100644
--- a/go.sum
+++ b/go.sum
@@ -470,8 +470,8 @@ golang.org/x/net v0.0.0-20201110031124-69a78807bb2b/go.mod h1:sp8m0HH+o8qH0wwXwY
 golang.org/x/net v0.0.0-20201209123823-ac852fbbde11/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
 golang.org/x/net v0.0.0-20201224014010-6772e930b67b/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
 golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
-golang.org/x/net v0.10.0 h1:X2//UzNDwYmtCLn7To6G58Wr6f5ahEAQgKNzv9Y951M=
-golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg=
+golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM=
+golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
 golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
@@ -537,19 +537,19 @@ golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU=
-golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE=
+golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
-golang.org/x/term v0.8.0 h1:n5xxQn2i3PC0yLAbjTpNT85q/Kgzcr2gIoX9OrJUols=
-golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo=
+golang.org/x/term v0.13.0 h1:bb+I9cTfFazGW51MZqBVmZy7+JEJMouUHTUSKVQLBek=
+golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U=
 golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
-golang.org/x/text v0.9.0 h1:2sjJmO8cDvYveuX97RDLsxlyUxLl+GHoLxBiRdHllBE=
-golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
+golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k=
+golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
 golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
diff --git a/internal/locality/locality.go b/internal/locality/locality.go
index 16d66ebb..d3d73f41 100644
--- a/internal/locality/locality.go
+++ b/internal/locality/locality.go
@@ -261,16 +261,21 @@ func CheckCoordinatorValidity(logger logr.Logger, cluster *fdbv1beta2.Foundation
 	allAddressesValid := true
 	allEligible := true
 	allUsingCorrectAddress := true
+	hardLimits := GetHardLimits(cluster)
+	coordinatorLocalities := make(map[string]map[string]int)
+	// Track what fields should be validated.
+	fieldsToValidate := make([]string, 0, len(hardLimits))
+	for field := range hardLimits {
+		fieldsToValidate = append(fieldsToValidate, field)
+	}
 
-	coordinatorZones := make(map[string]int, len(coordinatorStatus))
-	coordinatorDCs := make(map[string]int, len(coordinatorStatus))
-	processGroups := make(map[fdbv1beta2.ProcessGroupID]*fdbv1beta2.ProcessGroupStatus)
-	for _, processGroup := range cluster.Status.ProcessGroups {
-		processGroups[processGroup.ProcessGroupID] = processGroup
+	// Track the coordinator localities to verify if hard limits satisfy the fault tolerance requirements.
+	for _, field := range fieldsToValidate {
+		coordinatorLocalities[field] = make(map[string]int)
 	}
 
 	for _, process := range status.Cluster.Processes {
-		processGroupID, ok := process.Locality[fdbv1beta2.FDBLocalityInstanceIDKey]
+		processGroupID := process.Locality[fdbv1beta2.FDBLocalityInstanceIDKey]
 
 		pLogger := logger.WithValues("process", processGroupID)
 		if process.Address.IsEmpty() {
@@ -278,7 +283,7 @@ func CheckCoordinatorValidity(logger logr.Logger, cluster *fdbv1beta2.Foundation
 			continue
 		}
 
-		if !ok {
+		if len(process.Locality) == 0 {
 			pLogger.Info("Skip process with empty localities")
 			continue
 		}
@@ -288,23 +293,11 @@ func CheckCoordinatorValidity(logger logr.Logger, cluster *fdbv1beta2.Foundation
 			continue
 		}
 
-		processGroupStatus := processGroups[fdbv1beta2.ProcessGroupID(processGroupID)]
-		pendingRemoval := processGroupStatus != nil && processGroupStatus.IsMarkedForRemoval()
-		if processGroupStatus != nil && cluster.SkipProcessGroup(processGroupStatus) {
-			pLogger.Info("Skipping process group with pending Pod",
-				"namespace", cluster.Namespace,
-				"cluster", cluster.Name,
-				"processGroupID", processGroupID,
-				"class", process.ProcessClass)
-			continue
-		}
-
 		addresses, err := fdbv1beta2.ParseProcessAddressesFromCmdline(process.CommandLine)
 		if err != nil {
 			// We will end here in the error case when the address
 			// is not parsable e.g. no IP address is assigned.
 			allAddressesValid = false
-			// add: command_line
 			pLogger.Info("Could not parse address from command_line", "command_line", process.CommandLine)
 			continue
 		}
@@ -341,13 +334,19 @@ func CheckCoordinatorValidity(logger logr.Logger, cluster *fdbv1beta2.Foundation
 			coordinatorAddress = dnsAddress.String()
 		}
 
-		if coordinatorAddress != "" && !process.Excluded && !pendingRemoval && !process.UnderMaintenance {
+		if coordinatorAddress != "" && !process.Excluded && !process.UnderMaintenance {
 			coordinatorStatus[coordinatorAddress] = true
 		}
 
 		if coordinatorAddress != "" {
-			coordinatorZones[process.Locality[fdbv1beta2.FDBLocalityZoneIDKey]]++
-			coordinatorDCs[process.Locality[fdbv1beta2.FDBLocalityDCIDKey]]++
+			for _, field := range fieldsToValidate {
+				locality, ok := process.Locality[field]
+				// If the field is not set ignore it.
+				if !ok {
+					continue
+				}
+				coordinatorLocalities[field][locality]++
+			}
 
 			if !cluster.IsEligibleAsCandidate(process.ProcessClass) {
 				pLogger.Info("Process class of process is not eligible as coordinator", "class", process.ProcessClass, "address", coordinatorAddress)
@@ -367,21 +366,13 @@ func CheckCoordinatorValidity(logger logr.Logger, cluster *fdbv1beta2.Foundation
 		}
 	}
 
-	desiredCount := cluster.DesiredCoordinatorCount()
-	hasEnoughZones := len(coordinatorZones) == desiredCount
-	if !hasEnoughZones {
-		logger.Info("Cluster does not have coordinators in the correct number of zones", "desiredCount", desiredCount, "coordinatorZones", coordinatorZones)
-	}
-
-	var maxCoordinatorsPerDC int
-	hasEnoughDCs := true
-	if cluster.Spec.DatabaseConfiguration.UsableRegions > 1 {
-		maxCoordinatorsPerDC = int(math.Floor(float64(desiredCount) / 2.0))
-
-		for dc, count := range coordinatorDCs {
-			if count > maxCoordinatorsPerDC {
-				logger.Info("Cluster has too many coordinators in a single DC", "DC", dc, "count", count, "max", maxCoordinatorsPerDC)
-				hasEnoughDCs = false
+	// Check if the coordinators are distributed across the localities based on the hard limit requirements.
+	hasCorrectLocalityDistribution := true
+	for field, maxValue := range hardLimits {
+		for locality, currentValue := range coordinatorLocalities[field] {
+			if currentValue > maxValue {
+				logger.Info("Cluster does not have coordinators in the correct number of localities", "desiredCount", maxValue, "currentCount", currentValue, "locality", locality)
+				hasCorrectLocalityDistribution = false
 			}
 		}
 	}
@@ -394,5 +385,13 @@ func CheckCoordinatorValidity(logger logr.Logger, cluster *fdbv1beta2.Foundation
 		}
 	}
 
-	return hasEnoughDCs && hasEnoughZones && allHealthy && allUsingCorrectAddress && allEligible, allAddressesValid, nil
+	// Verify that enough coordinators are running.
+	desiredCoordinatorCount := cluster.DesiredCoordinatorCount()
+	runningCoordinators := len(coordinatorLocalities[fdbv1beta2.FDBLocalityZoneIDKey])
+	hasEnoughCoordinators := runningCoordinators == desiredCoordinatorCount
+	if !hasEnoughCoordinators {
+		logger.Info("Cluster has not enough running coordinators", "runningCoordinators", runningCoordinators, "desiredCount", desiredCoordinatorCount)
+	}
+
+	return hasEnoughCoordinators && hasCorrectLocalityDistribution && allHealthy && allUsingCorrectAddress && allEligible, allAddressesValid, nil
 }
diff --git a/internal/replacements/replace_failed_process_groups.go b/internal/replacements/replace_failed_process_groups.go
index a321ad2a..4d173ba5 100644
--- a/internal/replacements/replace_failed_process_groups.go
+++ b/internal/replacements/replace_failed_process_groups.go
@@ -55,7 +55,6 @@ func ReplaceFailedProcessGroups(log logr.Logger, cluster *fdbv1beta2.FoundationD
 	hasReplacement := false
 	crashLoopContainerProcessGroups := cluster.GetCrashLoopContainerProcessGroups()
 
-ProcessGroupLoop:
 	for _, processGroupStatus := range cluster.Status.ProcessGroups {
 		// If a process group is already marked for removal we can skip it here.
 		if processGroupStatus.IsMarkedForRemoval() {
@@ -70,18 +69,25 @@ ProcessGroupLoop:
 			continue
 		}
 
-		canReplace := maxReplacements > 0
-
+		var shouldBeIgnored bool
 		for _, targets := range crashLoopContainerProcessGroups {
 			if _, ok := targets[processGroupStatus.ProcessGroupID]; ok {
-				continue ProcessGroupLoop
-			} else if _, ok := targets["*"]; ok {
-				continue ProcessGroupLoop
+				shouldBeIgnored = true
+				break
+			}
+
+			if _, ok := targets["*"]; ok {
+				shouldBeIgnored = true
+				break
 			}
 		}
 
-		needsReplacement, missingTime := processGroupStatus.NeedsReplacement(cluster.GetFailureDetectionTimeSeconds(), cluster.GetTaintReplacementTimeSeconds())
-		if !needsReplacement {
+		if shouldBeIgnored {
+			continue
+		}
+
+		failureCondition, failureTime := processGroupStatus.NeedsReplacement(cluster.GetFailureDetectionTimeSeconds(), cluster.GetTaintReplacementTimeSeconds())
+		if failureTime == 0 {
 			continue
 		}
 
@@ -91,7 +97,7 @@ ProcessGroupLoop:
 				log.Info(
 					"Skip process group with missing address",
 					"processGroupID", processGroupStatus.ProcessGroupID,
-					"failureTime", time.Unix(missingTime, 0).UTC().String())
+					"failureTime", time.Unix(failureTime, 0).UTC().String())
 				continue
 			}
 
@@ -102,20 +108,24 @@ ProcessGroupLoop:
 			log.Info(
 				"Replace process group with missing address",
 				"processGroupID", processGroupStatus.ProcessGroupID,
-				"failureTime", time.Unix(missingTime, 0).UTC().String())
+				"failureTime", time.Unix(failureTime, 0).UTC().String())
 		}
 
-		// We are not allowed to replace additional process groups
-		if !canReplace {
+		// We are not allowed to replace additional process groups.
+		if maxReplacements <= 0 {
 			log.Info("Detected replace process group but cannot replace it because we hit the replacement limit",
 				"processGroupID", processGroupStatus.ProcessGroupID,
-				"reason", fmt.Sprintf("automatic replacement detected failure time: %s", time.Unix(missingTime, 0).UTC().String()))
+				"failureCondition", failureCondition,
+				"faultDomain", processGroupStatus.FaultDomain,
+				"reason", fmt.Sprintf("automatic replacement detected failure time: %s", time.Unix(failureTime, 0).UTC().String()))
 			continue
 		}
 
 		log.Info("Replace process group",
 			"processGroupID", processGroupStatus.ProcessGroupID,
-			"reason", fmt.Sprintf("automatic replacement detected failure time: %s", time.Unix(missingTime, 0).UTC().String()))
+			"failureCondition", failureCondition,
+			"faultDomain", processGroupStatus.FaultDomain,
+			"reason", fmt.Sprintf("automatic replacement detected failure time: %s", time.Unix(failureTime, 0).UTC().String()))
 
 		processGroupStatus.MarkForRemoval()
 		hasReplacement = true
diff --git a/kubectl-fdb/cmd/analyze.go b/kubectl-fdb/cmd/analyze.go
index 88a82597..ff15ae35 100644
--- a/kubectl-fdb/cmd/analyze.go
+++ b/kubectl-fdb/cmd/analyze.go
@@ -293,9 +293,11 @@ func analyzeCluster(cmd *cobra.Command, kubeClient client.Client, cluster *fdbv1
 			printStatement(cmd, statement, errorMessage)
 		}
 
-		needsReplacement, _ := processGroup.NeedsReplacement(0, 0)
-		if needsReplacement && autoFix {
-			failedProcessGroups = append(failedProcessGroups, string(processGroup.ProcessGroupID))
+		if autoFix {
+			_, failureTime := processGroup.NeedsReplacement(0, 0)
+			if failureTime > 0 {
+				failedProcessGroups = append(failedProcessGroups, string(processGroup.ProcessGroupID))
+			}
 		}
 	}
 
diff --git a/kubectl-fdb/cmd/remove_process_group.go b/kubectl-fdb/cmd/remove_process_group.go
index bb04e6d2..fcade941 100644
--- a/kubectl-fdb/cmd/remove_process_group.go
+++ b/kubectl-fdb/cmd/remove_process_group.go
@@ -88,7 +88,7 @@ kubectl fdb -n default remove process-group -c cluster --remove-all-failed
 `,
 	}
 
-	cmd.Flags().StringP("fdb-cluster", "c", "", "remove process groupss from the provided cluster.")
+	cmd.Flags().StringP("fdb-cluster", "c", "", "remove process groups from the provided cluster.")
 	cmd.Flags().BoolP("exclusion", "e", true, "define if the process groups should be removed with exclusion.")
 	cmd.Flags().Bool("remove-all-failed", false, "define if all failed processes should be replaced.")
 	cmd.Flags().Bool("use-process-group-id", false, "define if the process-group should be used instead of the Pod name.")
@@ -147,8 +147,8 @@ func replaceProcessGroups(kubeClient client.Client, clusterName string, ids []st
 				continue
 			}
 
-			needsReplacement, _ := processGroupStatus.NeedsReplacement(0, 0)
-			if needsReplacement {
+			_, failureTime := processGroupStatus.NeedsReplacement(0, 0)
+			if failureTime > 0 {
 				processGroupIDs = append(processGroupIDs, processGroupStatus.ProcessGroupID)
 			}
 		}


Test patch:
diff --git a/api/v1beta2/foundationdbcluster_types_test.go b/api/v1beta2/foundationdbcluster_types_test.go
index 758c7bcc..aa747e2a 100644
--- a/api/v1beta2/foundationdbcluster_types_test.go
+++ b/api/v1beta2/foundationdbcluster_types_test.go
@@ -4065,8 +4065,8 @@ var _ = Describe("[api] FoundationDBCluster", func() {
 
 	When("checking if the process group needs a replacement", func() {
 		var processGroup *ProcessGroupStatus
-		var needsReplacement bool
-		var timestamp int64
+		var failureCondition ProcessGroupConditionType
+		var failureTime int64
 		var oldTimestamp int64
 
 		BeforeEach(func() {
@@ -4075,12 +4075,13 @@ var _ = Describe("[api] FoundationDBCluster", func() {
 		})
 
 		JustBeforeEach(func() {
-			needsReplacement, timestamp = processGroup.NeedsReplacement(60, 60)
+			failureCondition, failureTime = processGroup.NeedsReplacement(60, 60)
 		})
 
 		Context("with no conditions", func() {
 			It("should not need replacement", func() {
-				Expect(needsReplacement).To(BeFalse())
+				Expect(failureTime).To(BeZero())
+				Expect(failureCondition).To(Equal(ProcessGroupConditionType("")))
 			})
 		})
 
@@ -4090,7 +4091,8 @@ var _ = Describe("[api] FoundationDBCluster", func() {
 			})
 
 			It("should not need replacement", func() {
-				Expect(needsReplacement).To(BeFalse())
+				Expect(failureTime).To(BeZero())
+				Expect(failureCondition).To(Equal(ProcessGroupConditionType("")))
 			})
 		})
 
@@ -4103,8 +4105,8 @@ var _ = Describe("[api] FoundationDBCluster", func() {
 			})
 
 			It("should need replacement", func() {
-				Expect(needsReplacement).To(BeTrue())
-				Expect(timestamp).To(Equal(targetTimestamp))
+				Expect(failureTime).To(BeNumerically("==", targetTimestamp))
+				Expect(failureCondition).To(Equal(MissingProcesses))
 			})
 		})
 
@@ -4119,8 +4121,8 @@ var _ = Describe("[api] FoundationDBCluster", func() {
 			})
 
 			It("should use the older timestamp", func() {
-				Expect(needsReplacement).To(BeTrue())
-				Expect(timestamp).To(Equal(targetTimestamp - 60))
+				Expect(failureTime).To(BeNumerically("==", targetTimestamp-60))
+				Expect(failureCondition).To(Equal(PodFailing))
 			})
 		})
 
@@ -4131,8 +4133,8 @@ var _ = Describe("[api] FoundationDBCluster", func() {
 			})
 
 			It("should need replacement", func() {
-				Expect(needsReplacement).To(BeTrue())
-				Expect(timestamp).To(Equal(oldTimestamp))
+				Expect(failureTime).To(BeNumerically("==", oldTimestamp))
+				Expect(failureCondition).To(Equal(PodFailing))
 			})
 		})
 
@@ -4143,8 +4145,8 @@ var _ = Describe("[api] FoundationDBCluster", func() {
 			})
 
 			It("should need replacement", func() {
-				Expect(needsReplacement).To(BeTrue())
-				Expect(timestamp).To(Equal(oldTimestamp))
+				Expect(failureTime).To(BeNumerically("==", oldTimestamp))
+				Expect(failureCondition).To(Equal(MissingPod))
 			})
 		})
 
@@ -4155,8 +4157,8 @@ var _ = Describe("[api] FoundationDBCluster", func() {
 			})
 
 			It("should need replacement", func() {
-				Expect(needsReplacement).To(BeTrue())
-				Expect(timestamp).To(Equal(oldTimestamp))
+				Expect(failureTime).To(BeNumerically("==", oldTimestamp))
+				Expect(failureCondition).To(Equal(MissingPVC))
 			})
 		})
 
@@ -4167,8 +4169,8 @@ var _ = Describe("[api] FoundationDBCluster", func() {
 			})
 
 			It("should need replacement", func() {
-				Expect(needsReplacement).To(BeTrue())
-				Expect(timestamp).To(Equal(oldTimestamp))
+				Expect(failureTime).To(BeNumerically("==", oldTimestamp))
+				Expect(failureCondition).To(Equal(MissingService))
 			})
 		})
 
@@ -4179,8 +4181,20 @@ var _ = Describe("[api] FoundationDBCluster", func() {
 			})
 
 			It("should need replacement", func() {
-				Expect(needsReplacement).To(BeTrue())
-				Expect(timestamp).To(Equal(oldTimestamp))
+				Expect(failureTime).To(BeNumerically("==", oldTimestamp))
+				Expect(failureCondition).To(Equal(PodPending))
+			})
+		})
+
+		When("process group is excluded without being marked for removal", func() {
+			BeforeEach(func() {
+				processGroup.UpdateCondition(ProcessIsMarkedAsExcluded, true)
+				processGroup.ProcessGroupConditions[0].Timestamp = oldTimestamp
+			})
+
+			It("should need replacement", func() {
+				Expect(failureTime).To(BeNumerically("==", oldTimestamp))
+				Expect(failureCondition).To(Equal(ProcessIsMarkedAsExcluded))
 			})
 		})
 
@@ -4191,7 +4205,8 @@ var _ = Describe("[api] FoundationDBCluster", func() {
 			})
 
 			It("should not need replacement", func() {
-				Expect(needsReplacement).To(BeFalse())
+				Expect(failureTime).To(BeZero())
+				Expect(failureCondition).To(Equal(ProcessGroupConditionType("")))
 			})
 		})
 	})
diff --git a/internal/locality/locality_test.go b/internal/locality/locality_test.go
index 31cf1379..1cb9e167 100644
--- a/internal/locality/locality_test.go
+++ b/internal/locality/locality_test.go
@@ -22,12 +22,10 @@ package locality
 
 import (
 	"fmt"
+	"github.com/FoundationDB/fdb-kubernetes-operator/pkg/podclient/mock"
 	"math"
 	"net"
 	"strings"
-	"time"
-
-	"github.com/FoundationDB/fdb-kubernetes-operator/pkg/podclient/mock"
 
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
@@ -651,38 +649,6 @@ var _ = Describe("Change coordinators", func() {
 			})
 		})
 
-		When("a process has a process group status with a pending Pod", func() {
-			BeforeEach(func() {
-				status.Cluster.Processes["4"] = fdbv1beta2.FoundationDBStatusProcessInfo{
-					Address: fdbv1beta2.ProcessAddress{
-						IPAddress: net.ParseIP("1.1.1.4"),
-						Port:      4501,
-					},
-					Locality: map[string]string{
-						fdbv1beta2.FDBLocalityInstanceIDKey: "test-4",
-					},
-				}
-
-				cluster.Status.ProcessGroups = append(cluster.Status.ProcessGroups,
-					&fdbv1beta2.ProcessGroupStatus{
-						ProcessGroupID: "test-4",
-						ProcessGroupConditions: []*fdbv1beta2.ProcessGroupCondition{
-							{
-								ProcessGroupConditionType: fdbv1beta2.PodPending,
-								Timestamp:                 time.Now().Add(-45 * time.Minute).Unix(),
-							},
-						},
-					})
-			})
-
-			It("should ignore the process", func() {
-				coordinatorsValid, addressesValid, err := CheckCoordinatorValidity(logr.Discard(), cluster, status, coordinatorStatus)
-				Expect(coordinatorsValid).To(BeTrue())
-				Expect(addressesValid).To(BeTrue())
-				Expect(err).To(BeNil())
-			})
-		})
-
 		When("a process misses the public-address flag", func() {
 			BeforeEach(func() {
 				process := status.Cluster.Processes["3"]