diff --git a/api/v1beta2/foundationdbcluster_types.go b/api/v1beta2/foundationdbcluster_types.go index 73e308fed..c9cfe5389 100644 --- a/api/v1beta2/foundationdbcluster_types.go +++ b/api/v1beta2/foundationdbcluster_types.go @@ -2233,11 +2233,11 @@ func (cluster *FoundationDBCluster) GetDNSDomain() string { // GetRemovalMode returns the removal mode of the cluster or default to PodUpdateModeZone if unset. func (cluster *FoundationDBCluster) GetRemovalMode() PodUpdateMode { - if cluster.Spec.AutomationOptions.DeletionMode == "" { + if cluster.Spec.AutomationOptions.RemovalMode == "" { return PodUpdateModeZone } - return cluster.Spec.AutomationOptions.DeletionMode + return cluster.Spec.AutomationOptions.RemovalMode } // GetWaitBetweenRemovalsSeconds returns the WaitDurationBetweenRemovals if set or defaults to 60s. diff --git a/api/v1beta2/foundationdbcluster_types_test.go b/api/v1beta2/foundationdbcluster_types_test.go index 20c26999c..b606bbb15 100644 --- a/api/v1beta2/foundationdbcluster_types_test.go +++ b/api/v1beta2/foundationdbcluster_types_test.go @@ -5501,4 +5501,35 @@ var _ = Describe("[api] FoundationDBCluster", func() { fmt.Errorf("process has missing address in exclusion results: 192.168.0.2"), ), ) + + DescribeTable("when getting the removal mode", func(cluster *FoundationDBCluster, expected PodUpdateMode) { + Expect(cluster.GetRemovalMode()).To(Equal(expected)) + }, + Entry("no removal mode defined", + &FoundationDBCluster{ + Spec: FoundationDBClusterSpec{ + AutomationOptions: FoundationDBClusterAutomationOptions{ + DeletionMode: PodUpdateModeAll, + }, + }, + }, PodUpdateModeZone), + Entry("removal mode zone defined", + &FoundationDBCluster{ + Spec: FoundationDBClusterSpec{ + AutomationOptions: FoundationDBClusterAutomationOptions{ + DeletionMode: PodUpdateModeAll, + RemovalMode: PodUpdateModeZone, + }, + }, + }, PodUpdateModeZone), + Entry("removal mode none defined", + &FoundationDBCluster{ + Spec: FoundationDBClusterSpec{ + AutomationOptions: FoundationDBClusterAutomationOptions{ + DeletionMode: PodUpdateModeAll, + RemovalMode: PodUpdateModeNone, + }, + }, + }, PodUpdateModeNone), + ) }) diff --git a/controllers/remove_process_groups_test.go b/controllers/remove_process_groups_test.go index 65da4d490..0f2b23714 100644 --- a/controllers/remove_process_groups_test.go +++ b/controllers/remove_process_groups_test.go @@ -179,106 +179,204 @@ var _ = Describe("remove_process_groups", func() { var initialCnt int var secondRemovedProcessGroup *fdbv1beta2.ProcessGroupStatus - BeforeEach(func() { - // To allow multiple process groups to be removed we have to use the update mode all - cluster.Spec.AutomationOptions.RemovalMode = fdbv1beta2.PodUpdateModeAll - err := k8sClient.Update(context.TODO(), cluster) - Expect(err).NotTo(HaveOccurred()) - - initialCnt = len(cluster.Status.ProcessGroups) - secondRemovedProcessGroup = cluster.Status.ProcessGroups[1] - marked, processGroup := fdbv1beta2.MarkProcessGroupForRemoval(cluster.Status.ProcessGroups, secondRemovedProcessGroup.ProcessGroupID, secondRemovedProcessGroup.ProcessClass, removedProcessGroup.Addresses[0]) - Expect(marked).To(BeTrue()) - Expect(processGroup).To(BeNil()) - // Exclude the process group - adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient) - Expect(err).NotTo(HaveOccurred()) - for _, address := range secondRemovedProcessGroup.Addresses { - adminClient.ExcludedAddresses[address] = fdbv1beta2.None{} - } - }) - - // TODO(johscheuer): Fix this flaky test properly, for now retry failing test occurrences with a maximum of 3 retries. - It("should remove only one process group", FlakeAttempts(3), func() { - Expect(result).To(BeNil()) - Expect(initialCnt - len(cluster.Status.ProcessGroups)).To(BeNumerically("==", 1)) - // Ensure resources are deleted - removed, include, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, removedProcessGroup.ProcessGroupID) - Expect(err).To(BeNil()) - Expect(removed).To(BeTrue()) - Expect(include).To(BeTrue()) - // Ensure resources are not deleted - removed, include, err = confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID) - Expect(err).To(BeNil()) - Expect(removed).To(BeFalse()) - Expect(include).To(BeFalse()) - }) - - When("a process group is marked as terminating and all resources are removed it should be removed", func() { + When("the removal mode is the default zone", func() { BeforeEach(func() { - secondRemovedProcessGroup.ProcessGroupConditions = append(secondRemovedProcessGroup.ProcessGroupConditions, fdbv1beta2.NewProcessGroupCondition(fdbv1beta2.ResourcesTerminating)) - err := removeProcessGroup(context.Background(), clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID) + Expect(cluster.Spec.AutomationOptions.RemovalMode).To(BeEmpty()) + initialCnt = len(cluster.Status.ProcessGroups) + secondRemovedProcessGroup = cluster.Status.ProcessGroups[1] + marked, processGroup := fdbv1beta2.MarkProcessGroupForRemoval(cluster.Status.ProcessGroups, secondRemovedProcessGroup.ProcessGroupID, secondRemovedProcessGroup.ProcessClass, removedProcessGroup.Addresses[0]) + Expect(marked).To(BeTrue()) + Expect(processGroup).To(BeNil()) + // Exclude the process group + adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient) Expect(err).NotTo(HaveOccurred()) + for _, address := range secondRemovedProcessGroup.Addresses { + adminClient.ExcludedAddresses[address] = fdbv1beta2.None{} + } }) - It("should remove the process group and the terminated process group", func() { + It("should remove only one process group", func() { Expect(result).To(BeNil()) - Expect(initialCnt - len(cluster.Status.ProcessGroups)).To(BeNumerically("==", 2)) - // Ensure resources are deleted - removed, include, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID) + Expect(initialCnt - len(cluster.Status.ProcessGroups)).To(BeNumerically("==", 1)) + // Check if resources are deleted + removed, include, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, removedProcessGroup.ProcessGroupID) Expect(err).To(BeNil()) - Expect(removed).To(BeTrue()) - Expect(include).To(BeTrue()) - // Ensure resources are deleted - removed, include, err = confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, removedProcessGroup.ProcessGroupID) + // Check if resources are deleted + removedSecondary, includeSecondary, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID) Expect(err).To(BeNil()) - Expect(removed).To(BeTrue()) - Expect(include).To(BeTrue()) + // Make sure only one of the process groups was deleted. + Expect(removed).NotTo(Equal(removedSecondary)) + Expect(include).NotTo(Equal(includeSecondary)) + }) + + When("a process group is marked as terminating and all resources are removed it should be removed", func() { + BeforeEach(func() { + secondRemovedProcessGroup.ProcessGroupConditions = append(secondRemovedProcessGroup.ProcessGroupConditions, fdbv1beta2.NewProcessGroupCondition(fdbv1beta2.ResourcesTerminating)) + Expect(removeProcessGroup(context.Background(), clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID)).NotTo(HaveOccurred()) + }) + + It("should remove the process group and the terminated process group", func() { + Expect(result).To(BeNil()) + Expect(initialCnt - len(cluster.Status.ProcessGroups)).To(BeNumerically("==", 2)) + // Ensure resources are deleted + removed, include, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID) + Expect(err).To(BeNil()) + Expect(removed).To(BeTrue()) + Expect(include).To(BeTrue()) + // Ensure resources are deleted + removed, include, err = confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, removedProcessGroup.ProcessGroupID) + Expect(err).To(BeNil()) + Expect(removed).To(BeTrue()) + Expect(include).To(BeTrue()) + }) + }) + + When("a process group is marked as terminating and the resources are not removed", func() { + BeforeEach(func() { + secondRemovedProcessGroup.ProcessGroupConditions = append(secondRemovedProcessGroup.ProcessGroupConditions, fdbv1beta2.NewProcessGroupCondition(fdbv1beta2.ResourcesTerminating)) + }) + + It("should remove the process group and the terminated process group", func() { + Expect(result).To(BeNil()) + Expect(initialCnt - len(cluster.Status.ProcessGroups)).To(BeNumerically("==", 2)) + // Ensure resources are deleted + removed, include, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID) + Expect(err).To(BeNil()) + Expect(removed).To(BeTrue()) + Expect(include).To(BeTrue()) + // Ensure resources are deleted + removed, include, err = confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, removedProcessGroup.ProcessGroupID) + Expect(err).To(BeNil()) + Expect(removed).To(BeTrue()) + Expect(include).To(BeTrue()) + }) + }) + + When("a process group is marked as terminating and not fully removed", func() { + BeforeEach(func() { + secondRemovedProcessGroup.ProcessGroupConditions = append(secondRemovedProcessGroup.ProcessGroupConditions, fdbv1beta2.NewProcessGroupCondition(fdbv1beta2.ResourcesTerminating)) + // Set the wait time to the default value + cluster.Spec.AutomationOptions.WaitBetweenRemovalsSeconds = pointer.Int(60) + }) + + It("should remove only one process group", func() { + Expect(result).NotTo(BeNil()) + Expect(result.message).To(HavePrefix("not allowed to remove process groups, waiting:")) + Expect(initialCnt - len(cluster.Status.ProcessGroups)).To(BeNumerically("==", 0)) + // Ensure resources are not deleted + removed, include, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, removedProcessGroup.ProcessGroupID) + Expect(err).To(BeNil()) + Expect(removed).To(BeFalse()) + Expect(include).To(BeFalse()) + // Ensure resources are deleted + removed, include, err = confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID) + Expect(err).To(BeNil()) + Expect(removed).To(BeFalse()) + Expect(include).To(BeFalse()) + }) }) }) - When("a process group is marked as terminating and the resources are not removed", func() { + When("the removal mode is PodUpdateModeAll", func() { BeforeEach(func() { - secondRemovedProcessGroup.ProcessGroupConditions = append(secondRemovedProcessGroup.ProcessGroupConditions, fdbv1beta2.NewProcessGroupCondition(fdbv1beta2.ResourcesTerminating)) + // To allow multiple process groups to be removed we have to use the update mode all + cluster.Spec.AutomationOptions.RemovalMode = fdbv1beta2.PodUpdateModeAll + err := k8sClient.Update(context.TODO(), cluster) + Expect(err).NotTo(HaveOccurred()) + + initialCnt = len(cluster.Status.ProcessGroups) + secondRemovedProcessGroup = cluster.Status.ProcessGroups[1] + marked, processGroup := fdbv1beta2.MarkProcessGroupForRemoval(cluster.Status.ProcessGroups, secondRemovedProcessGroup.ProcessGroupID, secondRemovedProcessGroup.ProcessClass, removedProcessGroup.Addresses[0]) + Expect(marked).To(BeTrue()) + Expect(processGroup).To(BeNil()) + // Exclude the process group + adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient) + Expect(err).NotTo(HaveOccurred()) + for _, address := range secondRemovedProcessGroup.Addresses { + adminClient.ExcludedAddresses[address] = fdbv1beta2.None{} + } }) - It("should remove the process group and the terminated process group", func() { + It("should remove two process groups", func() { Expect(result).To(BeNil()) Expect(initialCnt - len(cluster.Status.ProcessGroups)).To(BeNumerically("==", 2)) // Ensure resources are deleted - removed, include, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID) + removed, include, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, removedProcessGroup.ProcessGroupID) Expect(err).To(BeNil()) Expect(removed).To(BeTrue()) Expect(include).To(BeTrue()) - // Ensure resources are deleted - removed, include, err = confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, removedProcessGroup.ProcessGroupID) + // Ensure resources are deleted as the RemovalMode is PodUpdateModeAll + removed, include, err = confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID) Expect(err).To(BeNil()) Expect(removed).To(BeTrue()) Expect(include).To(BeTrue()) }) - }) - When("a process group is marked as terminating and not fully removed", func() { - BeforeEach(func() { - secondRemovedProcessGroup.ProcessGroupConditions = append(secondRemovedProcessGroup.ProcessGroupConditions, fdbv1beta2.NewProcessGroupCondition(fdbv1beta2.ResourcesTerminating)) - // Set the wait time to the default value - cluster.Spec.AutomationOptions.WaitBetweenRemovalsSeconds = pointer.Int(60) + When("a process group is marked as terminating and all resources are removed it should be removed", func() { + BeforeEach(func() { + secondRemovedProcessGroup.ProcessGroupConditions = append(secondRemovedProcessGroup.ProcessGroupConditions, fdbv1beta2.NewProcessGroupCondition(fdbv1beta2.ResourcesTerminating)) + err := removeProcessGroup(context.Background(), clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should remove the process group and the terminated process group", func() { + Expect(result).To(BeNil()) + Expect(initialCnt - len(cluster.Status.ProcessGroups)).To(BeNumerically("==", 2)) + // Ensure resources are deleted + removed, include, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID) + Expect(err).To(BeNil()) + Expect(removed).To(BeTrue()) + Expect(include).To(BeTrue()) + // Ensure resources are deleted + removed, include, err = confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, removedProcessGroup.ProcessGroupID) + Expect(err).To(BeNil()) + Expect(removed).To(BeTrue()) + Expect(include).To(BeTrue()) + }) }) - It("should remove only one process group", func() { - Expect(result).NotTo(BeNil()) - Expect(result.message).To(HavePrefix("not allowed to remove process groups, waiting:")) - Expect(initialCnt - len(cluster.Status.ProcessGroups)).To(BeNumerically("==", 0)) - // Ensure resources are not deleted - removed, include, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, removedProcessGroup.ProcessGroupID) - Expect(err).To(BeNil()) - Expect(removed).To(BeFalse()) - Expect(include).To(BeFalse()) - // Ensure resources are deleted - removed, include, err = confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID) - Expect(err).To(BeNil()) - Expect(removed).To(BeFalse()) - Expect(include).To(BeFalse()) + When("a process group is marked as terminating and the resources are not removed", func() { + BeforeEach(func() { + secondRemovedProcessGroup.ProcessGroupConditions = append(secondRemovedProcessGroup.ProcessGroupConditions, fdbv1beta2.NewProcessGroupCondition(fdbv1beta2.ResourcesTerminating)) + }) + + It("should remove the process group and the terminated process group", func() { + Expect(result).To(BeNil()) + Expect(initialCnt - len(cluster.Status.ProcessGroups)).To(BeNumerically("==", 2)) + // Ensure resources are deleted + removed, include, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID) + Expect(err).To(BeNil()) + Expect(removed).To(BeTrue()) + Expect(include).To(BeTrue()) + // Ensure resources are deleted + removed, include, err = confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, removedProcessGroup.ProcessGroupID) + Expect(err).To(BeNil()) + Expect(removed).To(BeTrue()) + Expect(include).To(BeTrue()) + }) + }) + + When("a process group is marked as terminating and not fully removed", func() { + BeforeEach(func() { + secondRemovedProcessGroup.ProcessGroupConditions = append(secondRemovedProcessGroup.ProcessGroupConditions, fdbv1beta2.NewProcessGroupCondition(fdbv1beta2.ResourcesTerminating)) + // Set the wait time to the default value + cluster.Spec.AutomationOptions.WaitBetweenRemovalsSeconds = pointer.Int(60) + }) + + It("should remove all process groups", func() { + Expect(result).To(BeNil()) + Expect(initialCnt - len(cluster.Status.ProcessGroups)).To(BeNumerically("==", 2)) + // Ensure resources are not deleted + removed, include, err := confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, removedProcessGroup.ProcessGroupID) + Expect(err).To(BeNil()) + Expect(removed).To(BeTrue()) + Expect(include).To(BeTrue()) + // Ensure resources are deleted + removed, include, err = confirmRemoval(context.Background(), globalControllerLogger, clusterReconciler, cluster, secondRemovedProcessGroup.ProcessGroupID) + Expect(err).To(BeNil()) + Expect(removed).To(BeTrue()) + Expect(include).To(BeTrue()) + }) }) }) })