Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix get removal mode #1751

Merged
merged 2 commits into from
Jul 25, 2023
Merged

Fix get removal mode #1751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9452a71
Fix the GetRemovalMode function to actuvally use removal mode
johscheuer Jul 20, 2023
b9562e0
Fix the GetRemovalMode method to return the correct value
johscheuer Jul 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/v1beta2/foundationdbcluster_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
31 changes: 31 additions & 0 deletions api/v1beta2/foundationdbcluster_types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
})
246 changes: 172 additions & 74 deletions controllers/remove_process_groups_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
})
})
})
Expand Down
Loading