Three data hall fault domain storage check bug fix (#1869)

* Change minimum fault domains from 4 to 3 for three_data_hall redundancy
FoundationDB · Nov 6, 2023 · fbb4c9f · fbb4c9f
1 parent f4780b5
commit fbb4c9f
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 4 deletions.
diff --git a/api/v1beta2/foundationdb_database_configuration.go b/api/v1beta2/foundationdb_database_configuration.go
@@ -718,10 +718,8 @@ func MinimumFaultDomains(redundancyMode RedundancyMode) int {
 		return 1
 	case RedundancyModeDouble, RedundancyModeUnset:
 		return 2
-	case RedundancyModeTriple:
+	case RedundancyModeTriple, RedundancyModeThreeDataHall:
 		return 3
-	case RedundancyModeThreeDataHall:
-		return 4
 	default:
 		return 1
 	}

diff --git a/controllers/remove_process_groups_test.go b/controllers/remove_process_groups_test.go
@@ -117,7 +117,7 @@ var _ = Describe("remove_process_groups", func() {
 					})
 				})
 
-				When("the cluster has degraded stroage fault tolerance", func() {
+				When("the cluster has degraded storage fault tolerance", func() {
 					BeforeEach(func() {
 						adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient)
 						Expect(err).NotTo(HaveOccurred())
@@ -192,6 +192,62 @@ var _ = Describe("remove_process_groups", func() {
 				})
 			})
 
+			When("the cluster has three_data_hall redundancy", func() {
+				BeforeEach(func() {
+					adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient)
+					Expect(err).NotTo(HaveOccurred())
+					adminClient.DatabaseConfiguration.RedundancyMode = fdbv1beta2.RedundancyModeThreeDataHall
+
+				})
+				When("storage have 3 replicas", func() {
+					BeforeEach(func() {
+						adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient)
+						Expect(err).NotTo(HaveOccurred())
+						adminClient.TeamTracker = []fdbv1beta2.FoundationDBStatusTeamTracker{
+							{
+								Primary: true,
+								State: fdbv1beta2.FoundationDBStatusDataState{
+									Healthy:              true,
+									MinReplicasRemaining: 3,
+								},
+							},
+						}
+					})
+					It("should successfully remove that process group", func() {
+						Expect(result).To(BeNil())
+						// Ensure resources are deleted
+						removed, include, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, removedProcessGroup)
+						Expect(err).To(BeNil())
+						Expect(removed).To(BeTrue())
+						Expect(include).To(BeTrue())
+					})
+				})
+				When("storage have 2 replicas", func() {
+					BeforeEach(func() {
+						adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient)
+						Expect(err).NotTo(HaveOccurred())
+						adminClient.TeamTracker = []fdbv1beta2.FoundationDBStatusTeamTracker{
+							{
+								Primary: true,
+								State: fdbv1beta2.FoundationDBStatusDataState{
+									Healthy:              true,
+									MinReplicasRemaining: 2,
+								},
+							},
+						}
+					})
+					It("should not remove the process group and should not exclude processes", func() {
+						Expect(result).NotTo(BeNil())
+						Expect(result.message).To(Equal("Removals cannot proceed because cluster has degraded fault tolerance"))
+						// Ensure resources are not deleted
+						removed, include, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, removedProcessGroup)
+						Expect(err).To(BeNil())
+						Expect(removed).To(BeFalse())
+						Expect(include).To(BeFalse())
+					})
+				})
+			})
+
 			When("removing multiple process groups", func() {
 				var initialCnt int
 				var secondRemovedProcessGroup *fdbv1beta2.ProcessGroupStatus