Repository: FoundationDB/fdb-kubernetes-operator
Pull request #1219 (2022-05-15 16:14:53)

Don't replace process groups if they are set into crash loop
# Description

Fixes: https://github.com/FoundationDB/fdb-kubernetes-operator/issues/1201

## Type of change

*Please select one of the options below.*

- Bug fix (non-breaking change which fixes an issue)

## Discussion

-

## Testing

Unit tests + local testing.

## Documentation

-

## Follow-up

-



Patch:
diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml
index 45378969..e4dad084 100644
--- a/.github/workflows/pull_request.yml
+++ b/.github/workflows/pull_request.yml
@@ -6,7 +6,8 @@ on:
   pull_request:
 
 env:
-  FDB_VER: "7.1.4"
+  FDB_VER: "7.1.5"
+
 
 jobs:
   lint-go:
diff --git a/Dockerfile b/Dockerfile
index 381245cc..242fdc3c 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -2,7 +2,7 @@
 FROM docker.io/library/golang:1.17.8 as builder
 
 # Install FDB this version is only required to compile the fdb operator
-ARG FDB_VERSION=7.1.4
+ARG FDB_VERSION=7.1.5
 ARG FDB_WEBSITE=https://github.com/apple/foundationdb/releases/download
 ARG TAG="latest"
 
diff --git a/api/v1beta2/foundationdbcluster_types.go b/api/v1beta2/foundationdbcluster_types.go
index 542f9234..64bfbe96 100644
--- a/api/v1beta2/foundationdbcluster_types.go
+++ b/api/v1beta2/foundationdbcluster_types.go
@@ -1868,6 +1868,17 @@ func (cluster *FoundationDBCluster) NeedsReplacement(processGroup *ProcessGroupS
 		return false
 	}
 
+	// See: https://github.com/FoundationDB/fdb-kubernetes-operator/issues/1201 when a process group is set into crash loop
+	// we don't want to replace it (otherwise the newly created process group won't have that setting).
+	crashLoop, crashLoopAll := cluster.GetCrashLoopProcessGroups()
+	if crashLoopAll {
+		return false
+	}
+
+	if _, ok := crashLoop[processGroup.ProcessGroupID]; ok {
+		return false
+	}
+
 	if cluster.Spec.AutomationOptions.PodUpdateStrategy == PodUpdateStrategyReplacement {
 		return true
 	}
@@ -2008,3 +2019,17 @@ func (cluster *FoundationDBCluster) GetRunningVersion() string {
 
 	return versionString
 }
+
+// GetCrashLoopProcessGroups returns the process group IDs that are marked for crash looping. The second return value indicates
+// if all process group IDs in a cluster should be crash looping.
+func (cluster *FoundationDBCluster) GetCrashLoopProcessGroups() (map[string]None, bool) {
+	crashLoopPods := make(map[string]None, len(cluster.Spec.Buggify.CrashLoop))
+	for _, processGroupID := range cluster.Spec.Buggify.CrashLoop {
+		if processGroupID == "*" {
+			return nil, true
+		}
+		crashLoopPods[processGroupID] = None{}
+	}
+
+	return crashLoopPods, false
+}
diff --git a/config/deployment/manager.yaml b/config/deployment/manager.yaml
index b9b13cc6..1d07cc63 100644
--- a/config/deployment/manager.yaml
+++ b/config/deployment/manager.yaml
@@ -64,7 +64,7 @@ spec:
             - name: fdb-binaries
               mountPath: /var/output-files
         - name: foundationdb-kubernetes-init-7-1
-          image: foundationdb/foundationdb-kubernetes-sidecar:7.1.2-1
+          image: foundationdb/foundationdb-kubernetes-sidecar:7.1.5-1
           args:
             - "--copy-library"
             - "7.1"
@@ -75,7 +75,7 @@ spec:
             - "--copy-binary"
             - "fdbrestore"
             - "--output-dir"
-            - "/var/output-files/7.1.2"
+            - "/var/output-files/7.1.5"
             - "--init-mode"
           volumeMounts:
             - name: fdb-binaries
diff --git a/config/samples/deployment.yaml b/config/samples/deployment.yaml
index bc3a075f..3741236f 100644
--- a/config/samples/deployment.yaml
+++ b/config/samples/deployment.yaml
@@ -218,9 +218,9 @@ spec:
         - --copy-binary
         - fdbrestore
         - --output-dir
-        - /var/output-files/7.1.2
+        - /var/output-files/7.1.5
         - --init-mode
-        image: foundationdb/foundationdb-kubernetes-sidecar:7.1.2-1
+        image: foundationdb/foundationdb-kubernetes-sidecar:7.1.5-1
         name: foundationdb-kubernetes-init-7-1
         volumeMounts:
         - mountPath: /var/output-files
diff --git a/controllers/add_pods.go b/controllers/add_pods.go
index 0c275b41..96d5d6e7 100644
--- a/controllers/add_pods.go
+++ b/controllers/add_pods.go
@@ -70,7 +70,7 @@ func (a addPods) reconcile(ctx context.Context, r *FoundationDBClusterReconciler
 		// If this process group is marked for removal, we normally don't want to spin it back up
 		// again. However, in a downscaling scenario, it could be that this is a storage node that
 		// is still draining its data onto another one. Therefore, we only want to leave it off
-		// (by continue'ing) if the cluster says that this process group is fully drained and safe
+		// (by continuing) if the cluster says that this process group is fully drained and safe
 		// to delete, which is the case if a previous run of the `removeProcessGroups` subreconciler
 		// has marked it as excluded in the cluster status (it does so only after executing the
 		// `exclude` FDB command and being told that the nodes in question are fully excluded).
diff --git a/controllers/check_client_compatibility.go b/controllers/check_client_compatibility.go
index 12ca9abf..653256b6 100644
--- a/controllers/check_client_compatibility.go
+++ b/controllers/check_client_compatibility.go
@@ -42,12 +42,6 @@ func (c checkClientCompatibility) reconcile(_ context.Context, r *FoundationDBCl
 		return nil
 	}
 
-	adminClient, err := r.getDatabaseClientProvider().GetAdminClient(cluster, r)
-	if err != nil {
-		return &requeue{curError: err}
-	}
-	defer adminClient.Close()
-
 	runningVersion, err := fdbv1beta2.ParseFdbVersion(cluster.Status.RunningVersion)
 	if err != nil {
 		return &requeue{curError: err}
@@ -70,6 +64,11 @@ func (c checkClientCompatibility) reconcile(_ context.Context, r *FoundationDBCl
 		return nil
 	}
 
+	adminClient, err := r.getDatabaseClientProvider().GetAdminClient(cluster, r)
+	if err != nil {
+		return &requeue{curError: err}
+	}
+	defer adminClient.Close()
 	status, err := adminClient.GetStatus()
 	if err != nil {
 		return &requeue{curError: err}
@@ -85,9 +84,8 @@ func (c checkClientCompatibility) reconcile(_ context.Context, r *FoundationDBCl
 		if versionInfo.ProtocolVersion == "Unknown" {
 			continue
 		}
-		match := versionInfo.ProtocolVersion == protocolVersion
 
-		if !match {
+		if versionInfo.ProtocolVersion != protocolVersion {
 			for _, client := range versionInfo.MaxProtocolClients {
 				unsupportedClients = append(unsupportedClients, client.Description())
 			}
diff --git a/controllers/delete_pods_for_buggification.go b/controllers/delete_pods_for_buggification.go
index 20847d8d..d246c081 100644
--- a/controllers/delete_pods_for_buggification.go
+++ b/controllers/delete_pods_for_buggification.go
@@ -45,15 +45,7 @@ func (d deletePodsForBuggification) reconcile(ctx context.Context, r *Foundation
 	podMap := internal.CreatePodMap(cluster, pods)
 	updates := make([]*corev1.Pod, 0)
 
-	crashLoopPods := make(map[string]bool, len(cluster.Spec.Buggify.CrashLoop))
-	crashLoopAll := false
-	for _, processGroupID := range cluster.Spec.Buggify.CrashLoop {
-		if processGroupID == "*" {
-			crashLoopAll = true
-		} else {
-			crashLoopPods[processGroupID] = true
-		}
-	}
+	crashLoopPods, crashLoopAll := cluster.GetCrashLoopProcessGroups()
 
 	for _, processGroup := range cluster.Status.ProcessGroups {
 		if processGroup.IsMarkedForRemoval() {
@@ -76,7 +68,10 @@ func (d deletePodsForBuggification) reconcile(ctx context.Context, r *Foundation
 			}
 		}
 
-		shouldCrashLoop := crashLoopAll || crashLoopPods[processGroup.ProcessGroupID]
+		shouldCrashLoop := crashLoopAll
+		if !shouldCrashLoop {
+			_, shouldCrashLoop = crashLoopPods[processGroup.ProcessGroupID]
+		}
 
 		if shouldCrashLoop != inCrashLoop {
 			logger.Info("Deleting pod for buggification",
diff --git a/controllers/remove_process_groups.go b/controllers/remove_process_groups.go
index c54e76c3..7993cc3d 100644
--- a/controllers/remove_process_groups.go
+++ b/controllers/remove_process_groups.go
@@ -75,7 +75,7 @@ func (u removeProcessGroups) reconcile(ctx context.Context, r *FoundationDBClust
 	// query the cluster gets into a degraded state.
 	// We could be smarter here and only block removals that target stateful processes by e.g. filtering those out of the
 	// processGroupsToRemove slice.
-	hasDesiredFaultTolerance, err := internal.HasDesiredFaultTolerance(adminClient, cluster)
+	hasDesiredFaultTolerance, err := internal.HasDesiredFaultTolerance(logger, adminClient, cluster)
 	if err != nil {
 		return &requeue{curError: err}
 	}
diff --git a/controllers/replace_failed_process_groups.go b/controllers/replace_failed_process_groups.go
index 2aa8436a..ae3a5e85 100644
--- a/controllers/replace_failed_process_groups.go
+++ b/controllers/replace_failed_process_groups.go
@@ -34,7 +34,7 @@ type replaceFailedProcessGroups struct{}
 // reconcile runs the reconciler's work.
 func (c replaceFailedProcessGroups) reconcile(ctx context.Context, r *FoundationDBClusterReconciler, cluster *fdbv1beta2.FoundationDBCluster) *requeue {
 	logger := log.WithValues("namespace", cluster.Namespace, "cluster", cluster.Name, "reconciler", "replaceFailedProcessGroups")
-	// If the EmptyMonitorConf setting is set we expect that all fdb processes of this cluster part are missing in order
+	// If the EmptyMonitorConf setting is set we expect that all fdb processes in this part of the cluster are missing. In order
 	// to prevent the operator from replacing any process groups we skip this reconciliation here.
 	if cluster.Spec.Buggify.EmptyMonitorConf {
 		logger.V(1).Info("Skipping because EmptyMonitorConf is set to true")
diff --git a/controllers/update_pods.go b/controllers/update_pods.go
index df7f6cb8..930c757d 100644
--- a/controllers/update_pods.go
+++ b/controllers/update_pods.go
@@ -209,7 +209,7 @@ func deletePodsForUpdates(ctx context.Context, r *FoundationDBClusterReconciler,
 		return &requeue{curError: err}
 	}
 
-	ready, err := r.PodLifecycleManager.CanDeletePods(ctx, adminClient, cluster)
+	ready, err := r.PodLifecycleManager.CanDeletePods(logr.NewContext(ctx, logger), adminClient, cluster)
 	if err != nil {
 		return &requeue{curError: err}
 	}
diff --git a/distrolesss.dockerfile b/distrolesss.dockerfile
index bbfa2148..8acf02e9 100644
--- a/distrolesss.dockerfile
+++ b/distrolesss.dockerfile
@@ -2,7 +2,7 @@
 FROM docker.io/library/golang:1.17.8 as builder
 
 # Install FDB this version is only required to compile the fdb operator
-ARG FDB_VERSION=7.1.4
+ARG FDB_VERSION=7.1.5
 ARG FDB_WEBSITE=https://github.com/apple/foundationdb/releases/download
 ARG TAG="latest"
 
diff --git a/docs/manual/replacements_and_deletions.md b/docs/manual/replacements_and_deletions.md
index 892e82bb..68c8232e 100644
--- a/docs/manual/replacements_and_deletions.md
+++ b/docs/manual/replacements_and_deletions.md
@@ -40,6 +40,9 @@ The following conditions are currently eligible for replacement:
 * `MissingProcesses`: This indicates that a process is not reporting to the database.
 * `PodFailing`: This indicates that one of the containers is not ready.
 
+Process groups that are set into the crash loop state with the `Buggify` setting won't be replaced by the operator.
+If the `cluster.Spec.Buggify.EmptyMonitorConf` setting is active the operator won't replace any process groups.
+
 ## Enforce Full Replication
 
 The operator only removes ProcessGroups when the cluster has the desired fault tolerance and is available. This is enforced by default in 1.0.0 without disabling.
diff --git a/docs/manual/technical_design.md b/docs/manual/technical_design.md
index f63f53d5..5830b352 100644
--- a/docs/manual/technical_design.md
+++ b/docs/manual/technical_design.md
@@ -48,7 +48,7 @@ In our operator, we add an additional abstraction to help structure the reconcil
 ## Locking Operations
 
 This document will note which operations require a lock in order to complete.
-This means that the operation needs to ensure that it is the only instance of the operator acting on the cluster, to prevent conflicts in multi-DC clusters.
+This means that the operator needs to ensure that it is the only instance of the operator acting on the cluster, to prevent conflicts in multi-DC clusters.
 For more using and configuring on the locking system, see the section on [Coordinating Global Operations](fault_domains.md#coordinating-global-operations).
 
 The locking system works by setting a key in the database to indicate which instance of the operator can perform global operations. This key is `\xff\x02/org.foundationdb.kubernetes-operator/global`. This key will be set to a value of `tuple.Tuple{lockID,start,end}`. `lockID` is the `processGroupIDPrefix` from the cluster spec. `start` is a 64-bit integer representing a Unix timestamp with precision to the second, giving the time when this instance of the operator took the lock. `end` is a similar timestamp representing the time when the lock will automatically expire. The default lock duration is 10 minutes. If the operator tries to acquire a lock and sees that it already has the lock, it will extend it for another 10 minutes past the current time. If it sees that another instance of the operator has a lock, and the current time is past the end of the lock, it will clear the old lock and take a new lock for itself. If it sees that another instance of the operator has a lock, and the current time is before the end of the lock, it will requeue reconciliation until it can acquire the lock.
@@ -93,7 +93,7 @@ The cluster reconciler runs the following subreconcilers:
 1. UpdateConfigMap
 1. CheckClientCompatibility
 1. ReplaceMisconfiguredProcessGroups
-1. ReplaceFailedPods
+1. ReplaceFailedProcessGroups
 1. DeletePodsForBuggification
 1. AddProcessGroups
 1. AddServices
@@ -115,9 +115,9 @@ The cluster reconciler runs the following subreconcilers:
 
 ### Tracking Reconciliation Stages
 
-We track the progress of reconciliation through a `GenerationStatus` object, in the `status.generationStatus` field in the cluster object. The generation status has fields within it that indicate how far reconciliation has gotten, with an integer for each field indicating the generation that was seen for that reconciliation. The most important field to track here is the `reconciled` field, which is set when we consider reconciliation _mostly_ complete. If you want to track a rollout, you can check for whether the generation number in `status.generationStatus.reconciled` is equal to the generation number in `metadata.generation`.
+We track the progress of reconciliation through a `Generations` object, in the `status.generations` field in the cluster object. The generation status has fields within it that indicate how far reconciliation has gotten, with an integer for each field indicating the generation that was seen for that reconciliation. The most important field to track here is the `reconciled` field, which is set when we consider reconciliation _mostly_ complete. If you want to track a rollout, you can check for whether the generation number in `status.generations.reconciled` is equal to the generation number in `metadata.generation`.
 
-There are some cases where we set the `reconciled` field to the current generation even though we are requeuing reconciliation and continuing to due more work. These cases are listed below:
+There are some cases where we set the `reconciled` field to the current generation even though we are requeuing reconciliation and continuing to do more work. These cases are listed below:
 
 1. Pods are in terminating. If we have fully excluded processes and have started the termination of the pods, we set both `reconciled` and `hasPendingRemoval` to the current generation. Termination cannot complete until the kubelet confirms the processes has been shut down, which can take an arbitrary long period of time if the kubelet is in a broken state. The processes will remain excluded until the termination completes, at which point the operator will include the processes again and the `hasPendingRemoval` field will be cleared. In general it should be fine for the cluster to stay in this state indefinitely, and you can continue to make other changes to the cluster. However, you may encounter issues with the stuck pods taking up resource quota until they are fully terminated.
 
@@ -141,13 +141,13 @@ You can skip this check by setting the `ignoreUpgradabilityChecks` flag in the c
 
 ### ReplaceMisconfiguredProcessGroups
 
-The `ReplaceMisconfiguredProcessGroups` subreconciler checks for process groups that need to be replaced in order to safely bring them up on a new configuration. The core action this subreconciler takes is setting the `remove` field on the `ProcessGroup` in the cluster status. Later subreconcilers will do the work for handling the replacement, whether processes are marked for replacement through this subreconciler or another mechanism.
+The `ReplaceMisconfiguredProcessGroups` subreconciler checks for process groups that need to be replaced in order to safely bring them up on a new configuration. The core action this subreconciler takes is setting the `removalTimestamp` field on the `ProcessGroup` in the cluster status. Later subreconcilers will do the work for handling the replacement, whether processes are marked for replacement through this subreconciler or another mechanism.
 
 See the [Replacements and Deletions](replacements_and_deletions.md) document for more details on when we do these replacements.
 
-### ReplaceFailedPods
+### ReplaceFailedProcessGroups
 
-The `ReplaceFailedPods` subreconciler checks for process groups that need to be replaced because they are in an unhealthy state. This only takes action when automatic replacements are enabled. The core action this subreconciler takes is setting the `remove` field on the `ProcessGroup` in the cluster status. Later subreconcilers will do the work for handling the replacement, whether processes are marked for replacement through this subreconciler or another mechanism.
+The `ReplaceFailedProcessGroups` subreconciler checks for process groups that need to be replaced because they are in an unhealthy state. This only takes action when automatic replacements are enabled. The core action this subreconciler takes is setting the `removalTimestamp` field on the `ProcessGroup` in the cluster status. Later subreconcilers will do the work for handling the replacement, whether processes are marked for replacement through this subreconciler or another mechanism.
 
 See the [Replacements and Deletions](replacements_and_deletions.md) document for more details on when we do these replacements.
 
@@ -198,7 +198,7 @@ If these things are not true, then the operator will requeue reconciliation. Thi
 
 ### UpdateLabels
 
-The `UpdateLabels` subreconciler updates the labels and annotations for the resources created by the operator based on the process settings, as well as setting core labels and annotations that the operator uses for its own purposes. Any labels or annotations that do not have values specified in the spec will be left unmodified. This means that if you set define a label in the cluster spec, and then remove that label from the spec, you will have to manually remove it from any existing resources in order for the label to completely go away.
+The `UpdateLabels` subreconciler updates the labels and annotations for the resources created by the operator based on the process settings, as well as setting core labels and annotations that the operator uses for its own purposes. Any labels or annotations that do not have values specified in the spec will be left unmodified. This means that if you define a label in the cluster spec, and then remove that label from the spec, you will have to manually remove it from any existing resources in order for the label to completely go away.
 
 ### UpdateDatabaseConfiguration
 
@@ -212,7 +212,7 @@ This action requires a lock.
 
 ### ChooseRemovals
 
-The `ChooseRemovals` subreconciler flags processes for removal when the current process count is more than the desired process count. The processes that are removed will be chosen so that the remaining process are spread across as many fault domains as possible. The core action this subreconciler takes is setting the `remove` field on the `ProcessGroup` in the cluster status. Later subreconcilers will do the work for handling the removal.
+The `ChooseRemovals` subreconciler flags processes for removal when the current process count is more than the desired process count. The processes that are removed will be chosen so that the remaining process are spread across as many fault domains as possible. The core action this subreconciler takes is setting the `removalTimestamp` field on the `ProcessGroup` in the cluster status. Later subreconcilers will do the work for handling the removal.
 
 ### ExcludeProcesses
 
@@ -226,7 +226,7 @@ This action requires a lock.
 
 The `ChangeCoordinators` subreconciler ensures that the cluster has a healthy set of coordinators that fulfill the fault tolerance requirements for the cluster. If any coordinators have failed, or if the database configuration requires more coordinators or better-distributed coordinators, the operator will choose new coordinators and run a `coordinators` command to tell the database to use the new set. It will then read the new connection string and update it in the cluster status.
 
-This will recruit coordinators based on the process list in the database status to ensure that the coordinators it recruits are properly connecting to the database. This will prefer to recruit coordinators only from `storage` processes. If it cannot fulfill the fault tolerance requirements using storage requirements, it will expand the candidate list to include `log` processes, and then to include `transaction` processes if necessary. It will ensure that the coordinators are distributed across failure domains as evenly as possible. It will also require that ever coordinator has a different `zoneid` locality. For multi-DC clusters, it will require that we do not have a majority of coordinators using the same value for the `dcid` locality.
+This will recruit coordinators based on the process list in the database status to ensure that the coordinators it recruits are properly connecting to the database. This will prefer to recruit coordinators only from `storage` processes. If it cannot fulfill the fault tolerance requirements using storage requirements, it will expand the candidate list to include `log` processes, and then to include `transaction` processes if necessary. It will ensure that the coordinators are distributed across failure domains as evenly as possible. It will also require that every coordinator has a different `zoneid` locality. For multi-DC clusters, it will require that we do not have a majority of coordinators using the same value for the `dcid` locality.
 
 For single-DC clusters, the number of coordinators will be `2R-1`, where `R` is the replication factor. For multi-DC clusters, we will always use 9 coordinators.
 
@@ -306,21 +306,21 @@ The `UpdateBackupAgents` subreconciler is responsible for creating and updating
 
 ### StartBackup
 
-The `StartBackup` subreconciler is responsible for starting a backup. If a backup is supposed to be running, but the database status reports no ongoing backup, this will run an `start` command in `fdbbackup`.
+The `StartBackup` subreconciler is responsible for starting a backup. If a backup is supposed to be running, but the database status reports no ongoing backup, this will run the `start` command in `fdbbackup`.
 
 ### StopBackup
 
-The `StopBackup` subreconciler is responsible for stopping a backup. If a backup is not supposed to be running, but the database status reports an ongoing backup, this will run a `stop` command in `fdbbackup`.
+The `StopBackup` subreconciler is responsible for stopping a backup. If a backup is not supposed to be running, but the database status reports an ongoing backup, this will run the `stop` command in `fdbbackup`.
 
 ### ToggleBackupPaused
 
-The `ToggleBackupPaused` subreconciler is responsible for pausing and unpausing a backup. Pausing a backup means that the backup will be be configured in the cluster, but the backup agents will not do any work. If the desired state of the backup is `Paused`, and the backup is not paused, this will run a `pause` command in `fdbbackup`.  If the desired state of the backup is `Running`, and the backup is paused, this will run a `resume` command in `fdbbackup`.
+The `ToggleBackupPaused` subreconciler is responsible for pausing and unpausing a backup. Pausing a backup means that the backup will be configured in the cluster, but the backup agents will not do any work. If the desired state of the backup is `Paused`, and the backup is not paused, this will run the `pause` command in `fdbbackup`.  If the desired state of the backup is `Running`, and the backup is paused, this will run the `resume` command in `fdbbackup`.
 
 ### ModifyBackup
 
-The `ModifyBackup` command ensures that any properties that can be configured on a live backup are configured to the values in the backup spec. This will run a `modify` command in `fdbbackup` to set the properties from the spec.
+The `ModifyBackup` command ensures that any properties that can be configured on a live backup are configured to the values in the backup spec. This will run the `modify` command in `fdbbackup` to set the properties from the spec.
 
-Currently this only supports the `snapshotPeriodSeconds` property.
+Currently, this only supports the `snapshotPeriodSeconds` property.
 
 ### UpdateBackupStatus (again)
 
@@ -334,15 +334,15 @@ The restore reconciler runs the following subreconcilers:
 
 ### StartRestore
 
-The `StartRestore` subreconciler starts a restore. If there is no active restore, this will run a `start` command in `fdbrestore`.
+The `StartRestore` subreconciler starts a restore. If there is no active restore, this will run the `start` command in `fdbrestore`.
 
 ## Interaction Between the Operator and the Pods
 
-The operator communicates with processes running inside of the FoundationDB pods at multiple stages in the reconciliation flow. The exact flow will depend on whether you are using split images (which is the current default) or unified images.
+The operator communicates with processes running inside the FoundationDB pods at multiple stages in the reconciliation flow. The exact flow will depend on whether you are using split images (which is the current default) or unified images.
 
 ### Split Image
 
-When using split images, the `foundationdb` container runs an `fdbmonitor` process, which is responsible for starting `fdbserver` processes. `fdbmonitor` receives its configuration in the form of a monitor conf file, which contains the start command and arguments for the fdbserver process. This configuration can vary based on dynamic information like the node where the pod is running, so the operator provides a templated configuration file, which contains placeholders that are filled in based on the environment variables. That templating process is handled by the sidecar process in the `foundationdb-kubernetes-sidecar` container. The sidecar also provides an HTTP API for getting information about the state of the configuration in the pod. The sidecar mounts the config map containing dynamic conf as its input directory, and shares an `emptyDir` volume with the `foundationdb` container where it can put its output.
+When using split images, the `foundationdb` container runs a `fdbmonitor` process, which is responsible for starting the `fdbserver` processes. `fdbmonitor` receives its configuration in the form of a monitor conf file, which contains the start command and arguments for the fdbserver process. This configuration can vary based on dynamic information like the node where the pod is running, so the operator provides a templated configuration file, which contains placeholders that are filled in based on the environment variables. That templating process is handled by the sidecar process in the `foundationdb-kubernetes-sidecar` container. The sidecar also provides an HTTP API for getting information about the state of the configuration in the pod. The sidecar mounts the config map containing dynamic conf as its input directory, and shares an `emptyDir` volume with the `foundationdb` container where it can put its output.
 
 The flow for updating the fdbserver processes has the following steps:
 
@@ -351,9 +351,9 @@ The flow for updating the fdbserver processes has the following steps:
 3. The operator sees that the config is out of date, and sets an annotation on the pod to indicate that it needs an update.
 4. Kubernetes fetches the contents of the config map from the API server and updates the template in the sidecar container.
 5. The operator tells the sidecar to regenerate the monitor conf based on the new template. The sidecar places the generated monitor conf in its output directory.
-6. The operator checks the latest output monitor conf to see if it is now correct.
+6. The operator checks the latest output monitor conf to see if it is correct.
 7. Once the monitor conf is correct, the operator uses the CLI to shut down the fdbserver processes.
-8. fdbmonitor sees that that processes have exited and starts new processes with the latest configuration.
+8. fdbmonitor sees that the processes have exited and starts new processes with the latest configuration.
 
 The operator follows a similar process when the `fdb.cluster` file needs to be updated. However, because this file is not templated, the sidecar simply copies the file from the input directory to the output directory. Cluster file updates do not require restarting processes.
 
diff --git a/fdbclient/lock_client.go b/fdbclient/lock_client.go
index cba68bff..f65a6378 100644
--- a/fdbclient/lock_client.go
+++ b/fdbclient/lock_client.go
@@ -127,7 +127,7 @@ func (client *realLockClient) takeLockInTransaction(transaction fdb.Transaction)
 		return true, nil
 	}
 
-	if ownerID == client.cluster.GetLockID() {
+	if ownerID == cluster.GetLockID() {
 		client.log.Info("Extending previous lock", "namespace", cluster.Namespace, "cluster", cluster.Name, "owner", ownerID, "startTime", time.Unix(startTime, 0), "endTime", time.Unix(endTime, 0))
 		client.updateLock(transaction, startTime)
 		return true, nil
diff --git a/helm/fdb-operator/values.yaml b/helm/fdb-operator/values.yaml
index 122d9927..6964b724 100644
--- a/helm/fdb-operator/values.yaml
+++ b/helm/fdb-operator/values.yaml
@@ -18,7 +18,7 @@ initContainers:
   7.1:
     image:
       repository: foundationdb/foundationdb-kubernetes-sidecar
-      tag: 7.1.2-1
+      tag: 7.1.5-1
       pullPolicy: IfNotPresent
 
 globalMode:
diff --git a/internal/fault_tolerance.go b/internal/fault_tolerance.go
index 7f2bf839..6d3d629e 100644
--- a/internal/fault_tolerance.go
+++ b/internal/fault_tolerance.go
@@ -23,6 +23,7 @@ package internal
 import (
 	fdbv1beta2 "github.com/FoundationDB/fdb-kubernetes-operator/api/v1beta2"
 	"github.com/FoundationDB/fdb-kubernetes-operator/pkg/fdbadminclient"
+	"github.com/go-logr/logr"
 )
 
 func hasDesiredFaultTolerance(expectedFaultTolerance int, maxZoneFailuresWithoutLosingData int, maxZoneFailuresWithoutLosingAvailability int) bool {
@@ -32,7 +33,7 @@ func hasDesiredFaultTolerance(expectedFaultTolerance int, maxZoneFailuresWithout
 }
 
 // HasDesiredFaultTolerance checks if the cluster has the desired fault tolerance.
-func HasDesiredFaultTolerance(adminClient fdbadminclient.AdminClient, cluster *fdbv1beta2.FoundationDBCluster) (bool, error) {
+func HasDesiredFaultTolerance(log logr.Logger, adminClient fdbadminclient.AdminClient, cluster *fdbv1beta2.FoundationDBCluster) (bool, error) {
 	status, err := adminClient.GetStatus()
 	if err != nil {
 		return false, err
@@ -47,12 +48,10 @@ func HasDesiredFaultTolerance(adminClient fdbadminclient.AdminClient, cluster *f
 	}
 
 	expectedFaultTolerance := cluster.DesiredFaultTolerance()
-	//log.V(0).Info("Check desired fault tolerance",
-	//	"namespace", cluster.Namespace,
-	//	"cluster", cluster.Name,
-	//	"expectedFaultTolerance", expectedFaultTolerance,
-	//	"maxZoneFailuresWithoutLosingData", status.Cluster.FaultTolerance.MaxZoneFailuresWithoutLosingData,
-	//	"maxZoneFailuresWithoutLosingAvailability", status.Cluster.FaultTolerance.MaxZoneFailuresWithoutLosingAvailability)
+	log.Info("Check desired fault tolerance",
+		"expectedFaultTolerance", expectedFaultTolerance,
+		"maxZoneFailuresWithoutLosingData", status.Cluster.FaultTolerance.MaxZoneFailuresWithoutLosingData,
+		"maxZoneFailuresWithoutLosingAvailability", status.Cluster.FaultTolerance.MaxZoneFailuresWithoutLosingAvailability)
 
 	return hasDesiredFaultTolerance(
 		expectedFaultTolerance,
diff --git a/internal/replacements/replace_failed_process_groups.go b/internal/replacements/replace_failed_process_groups.go
index b6b2e086..ff287ebc 100644
--- a/internal/replacements/replace_failed_process_groups.go
+++ b/internal/replacements/replace_failed_process_groups.go
@@ -49,11 +49,17 @@ func getMaxReplacements(cluster *fdbv1beta2.FoundationDBCluster, maxReplacements
 // ReplaceFailedProcessGroups flags failed processes groups for removal and returns an indicator
 // of whether any processes were thus flagged.
 func ReplaceFailedProcessGroups(log logr.Logger, cluster *fdbv1beta2.FoundationDBCluster, adminClient fdbadminclient.AdminClient) bool {
-	// Automatic replacements are disabled so we don't have to check anything further
+	// Automatic replacements are disabled, so we don't have to check anything further
 	if !cluster.GetEnableAutomaticReplacements() {
 		return false
 	}
 
+	crashLoopProcessGroups, crashLoopAll := cluster.GetCrashLoopProcessGroups()
+	// If all process groups are in crash loop don't replace any process group.
+	if crashLoopAll {
+		return false
+	}
+
 	maxReplacements := getMaxReplacements(cluster, cluster.GetMaxConcurrentAutomaticReplacements())
 	hasReplacement := false
 	for _, processGroupStatus := range cluster.Status.ProcessGroups {
@@ -61,6 +67,12 @@ func ReplaceFailedProcessGroups(log logr.Logger, cluster *fdbv1beta2.FoundationD
 			return hasReplacement
 		}
 
+		// Don't replace processes that are in the crash loop setting. Otherwise, we might replace process groups that
+		// are in that state for debugging or stability.
+		if _, ok := crashLoopProcessGroups[processGroupStatus.ProcessGroupID]; ok {
+			continue
+		}
+
 		needsReplacement, missingTime := processGroupStatus.NeedsReplacement(cluster.GetFailureDetectionTimeSeconds())
 		if !needsReplacement {
 			continue
@@ -69,7 +81,7 @@ func ReplaceFailedProcessGroups(log logr.Logger, cluster *fdbv1beta2.FoundationD
 		if len(processGroupStatus.Addresses) == 0 {
 			// Only replace process groups without an address if the cluster has the desired fault tolerance
 			// and is available.
-			hasDesiredFaultTolerance, err := internal.HasDesiredFaultTolerance(adminClient, cluster)
+			hasDesiredFaultTolerance, err := internal.HasDesiredFaultTolerance(log, adminClient, cluster)
 			if err != nil {
 				log.Error(err, "Could not fetch if cluster has desired fault tolerance")
 				continue
diff --git a/pkg/podmanager/pod_lifecycle_manager.go b/pkg/podmanager/pod_lifecycle_manager.go
index ff4baede..1caeeda8 100644
--- a/pkg/podmanager/pod_lifecycle_manager.go
+++ b/pkg/podmanager/pod_lifecycle_manager.go
@@ -26,6 +26,7 @@ import (
 	fdbv1beta2 "github.com/FoundationDB/fdb-kubernetes-operator/api/v1beta2"
 	"github.com/FoundationDB/fdb-kubernetes-operator/internal"
 	"github.com/FoundationDB/fdb-kubernetes-operator/pkg/fdbadminclient"
+	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -109,8 +110,8 @@ func (manager StandardPodLifecycleManager) DeletePod(ctx context.Context, r clie
 }
 
 // CanDeletePods checks whether it is safe to delete Pods.
-func (manager StandardPodLifecycleManager) CanDeletePods(_ context.Context, adminClient fdbadminclient.AdminClient, cluster *fdbv1beta2.FoundationDBCluster) (bool, error) {
-	return internal.HasDesiredFaultTolerance(adminClient, cluster)
+func (manager StandardPodLifecycleManager) CanDeletePods(ctx context.Context, adminClient fdbadminclient.AdminClient, cluster *fdbv1beta2.FoundationDBCluster) (bool, error) {
+	return internal.HasDesiredFaultTolerance(logr.FromContextOrDiscard(ctx), adminClient, cluster)
 }
 
 // UpdatePods updates a list of Pods to match the latest specs.


Test patch:
diff --git a/api/v1beta2/foundationdbcluster_types_test.go b/api/v1beta2/foundationdbcluster_types_test.go
index cef7fa36..c701f898 100644
--- a/api/v1beta2/foundationdbcluster_types_test.go
+++ b/api/v1beta2/foundationdbcluster_types_test.go
@@ -4385,6 +4385,44 @@ var _ = Describe("[api] FoundationDBCluster", func() {
 				},
 				false,
 			),
+			Entry("Default update strategy log process that is in crash loop",
+				&FoundationDBCluster{
+					Spec: FoundationDBClusterSpec{
+						AutomationOptions: FoundationDBClusterAutomationOptions{
+							PodUpdateStrategy: "",
+						},
+						Buggify: BuggifyConfig{
+							CrashLoop: []string{
+								"log-1",
+							},
+						},
+					},
+				},
+				&ProcessGroupStatus{
+					ProcessGroupID: "log-1",
+					ProcessClass:   ProcessClassTransaction,
+				},
+				false,
+			),
+			Entry("Default update strategy log process with crash loop all",
+				&FoundationDBCluster{
+					Spec: FoundationDBClusterSpec{
+						AutomationOptions: FoundationDBClusterAutomationOptions{
+							PodUpdateStrategy: "",
+						},
+						Buggify: BuggifyConfig{
+							CrashLoop: []string{
+								"*",
+							},
+						},
+					},
+				},
+				&ProcessGroupStatus{
+					ProcessGroupID: "log-1",
+					ProcessClass:   ProcessClassTransaction,
+				},
+				false,
+			),
 		)
 	})
 
diff --git a/controllers/replace_failed_process_groups_test.go b/controllers/replace_failed_process_groups_test.go
index df83f559..a9ad8397 100644
--- a/controllers/replace_failed_process_groups_test.go
+++ b/controllers/replace_failed_process_groups_test.go
@@ -111,6 +111,34 @@ var _ = Describe("replace_failed_process_groups", func() {
 					Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]string{}))
 				})
 			})
+
+			When("Crash loop is set for all process groups", func() {
+				BeforeEach(func() {
+					cluster.Spec.Buggify.CrashLoop = []string{"*"}
+				})
+
+				It("should return nil", func() {
+					Expect(result).To(BeNil())
+				})
+
+				It("should not mark the process group for removal", func() {
+					Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]string{}))
+				})
+			})
+
+			When("Crash loop is set for the specific process group", func() {
+				BeforeEach(func() {
+					cluster.Spec.Buggify.CrashLoop = []string{"storage-2"}
+				})
+
+				It("should return nil", func() {
+					Expect(result).To(BeNil())
+				})
+
+				It("should not mark the process group for removal", func() {
+					Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]string{}))
+				})
+			})
 		})
 
 		Context("with multiple failed processes", func() {
@@ -158,8 +186,7 @@ var _ = Describe("replace_failed_process_groups", func() {
 
 			When("max concurrent replacements is set to two", func() {
 				BeforeEach(func() {
-					replacements := 2
-					cluster.Spec.AutomationOptions.Replacements.MaxConcurrentReplacements = &replacements
+					cluster.Spec.AutomationOptions.Replacements.MaxConcurrentReplacements = pointer.Int(2)
 				})
 
 				It("should requeue", func() {
@@ -174,8 +201,7 @@ var _ = Describe("replace_failed_process_groups", func() {
 
 			When("max concurrent replacements is set to zero", func() {
 				BeforeEach(func() {
-					replacements := 0
-					cluster.Spec.AutomationOptions.Replacements.MaxConcurrentReplacements = &replacements
+					cluster.Spec.AutomationOptions.Replacements.MaxConcurrentReplacements = pointer.Int(0)
 				})
 
 				It("should return nil", func() {