diff --git a/api/v1beta2/foundationdbcluster_types.go b/api/v1beta2/foundationdbcluster_types.go index 0d314c2ed..b5e2037e1 100644 --- a/api/v1beta2/foundationdbcluster_types.go +++ b/api/v1beta2/foundationdbcluster_types.go @@ -1215,6 +1215,14 @@ type AutomaticReplacementOptions struct { // The default is false. Enabled *bool `json:"enabled,omitempty"` + // FaultDomainBasedReplacements controls whether automatic replacements are targeting all failed process groups + // in a fault domain or only specific Process Groups. If this setting is enabled, the number of different fault + // domains that can have all their failed process groups replaced at the same time will be equal to MaxConcurrentReplacements. + // e.g. MaxConcurrentReplacements = 2 would mean that at most 2 different fault domains can have + // their failed process groups replaced at the same time. + // The default is false. + FaultDomainBasedReplacements *bool `json:"faultDomainBasedReplacements,omitempty"` + // FailureDetectionTimeSeconds controls how long a process must be // failed or missing before it is automatically replaced. // The default is 7200 seconds, or 2 hours. @@ -2155,6 +2163,12 @@ func (cluster *FoundationDBCluster) GetMaxConcurrentAutomaticReplacements() int return pointer.IntDeref(cluster.Spec.AutomationOptions.Replacements.MaxConcurrentReplacements, 1) } +// FaultDomainBasedReplacements returns true if the operator is allowed to replace all failed process groups of a +// fault domain. Default is false +func (cluster *FoundationDBCluster) FaultDomainBasedReplacements() bool { + return pointer.BoolDeref(cluster.Spec.AutomationOptions.Replacements.FaultDomainBasedReplacements, false) +} + // CoordinatorSelectionSetting defines the process class and the priority of it. // A higher priority means that the process class is preferred over another. type CoordinatorSelectionSetting struct { diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index a8d1cc0c9..f484e0b74 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -36,6 +36,11 @@ func (in *AutomaticReplacementOptions) DeepCopyInto(out *AutomaticReplacementOpt *out = new(bool) **out = **in } + if in.FaultDomainBasedReplacements != nil { + in, out := &in.FaultDomainBasedReplacements, &out.FaultDomainBasedReplacements + *out = new(bool) + **out = **in + } if in.FailureDetectionTimeSeconds != nil { in, out := &in.FailureDetectionTimeSeconds, &out.FailureDetectionTimeSeconds *out = new(int) diff --git a/config/crd/bases/apps.foundationdb.org_foundationdbclusters.yaml b/config/crd/bases/apps.foundationdb.org_foundationdbclusters.yaml index e02c00dd6..94085f3ff 100644 --- a/config/crd/bases/apps.foundationdb.org_foundationdbclusters.yaml +++ b/config/crd/bases/apps.foundationdb.org_foundationdbclusters.yaml @@ -10194,6 +10194,8 @@ spec: type: boolean failureDetectionTimeSeconds: type: integer + faultDomainBasedReplacements: + type: boolean maxConcurrentReplacements: default: 1 minimum: 0 diff --git a/controllers/replace_failed_process_groups_test.go b/controllers/replace_failed_process_groups_test.go index 012726c56..b8901ae3b 100644 --- a/controllers/replace_failed_process_groups_test.go +++ b/controllers/replace_failed_process_groups_test.go @@ -48,8 +48,8 @@ var _ = Describe("replace_failed_process_groups", func() { BeforeEach(func() { cluster = internal.CreateDefaultCluster() - err = k8sClient.Create(ctx.TODO(), cluster) - Expect(err).NotTo(HaveOccurred()) + cluster.Spec.AutomationOptions.Replacements.FaultDomainBasedReplacements = pointer.Bool(false) + Expect(k8sClient.Create(ctx.TODO(), cluster)).NotTo(HaveOccurred()) result, err := reconcileCluster(cluster) Expect(err).NotTo(HaveOccurred()) @@ -641,294 +641,665 @@ var _ = Describe("replace_failed_process_groups", func() { }) }) - Context("with a process that has been missing for a long time", func() { - BeforeEach(func() { - processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") - processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ - ProcessGroupConditionType: fdbv1beta2.MissingProcesses, - Timestamp: time.Now().Add(-1 * time.Hour).Unix(), + When("fault domain replacements are disabled", func() { + Context("with a process that has been missing for a long time", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ + ProcessGroupConditionType: fdbv1beta2.MissingProcesses, + Timestamp: time.Now().Add(-1 * time.Hour).Unix(), + }) }) - }) - Context("with no other removals", func() { - It("should requeue", func() { - Expect(result).NotTo(BeNil()) - Expect(result.message).To(Equal("Removals have been updated in the cluster status")) - }) + Context("with no other removals", func() { + It("should requeue", func() { + Expect(result).NotTo(BeNil()) + Expect(result.message).To(Equal("Removals have been updated in the cluster status")) + }) - It("should mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) - }) + It("should mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) + }) + + It("should not be marked to skip exclusion", func() { + for _, pg := range cluster.Status.ProcessGroups { + if pg.ProcessGroupID != "storage-2" { + continue + } - It("should not be marked to skip exclusion", func() { - for _, pg := range cluster.Status.ProcessGroups { - if pg.ProcessGroupID != "storage-2" { - continue + Expect(pg.ExclusionSkipped).To(BeFalse()) } + }) - Expect(pg.ExclusionSkipped).To(BeFalse()) - } - }) + When("EmptyMonitorConf is set to true", func() { + BeforeEach(func() { + cluster.Spec.Buggify.EmptyMonitorConf = true + }) - When("EmptyMonitorConf is set to true", func() { - BeforeEach(func() { - cluster.Spec.Buggify.EmptyMonitorConf = true + It("should return nil", func() { + Expect(result).To(BeNil()) + }) + + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + }) }) - It("should return nil", func() { - Expect(result).To(BeNil()) + When("Crash loop is set for all process groups", func() { + BeforeEach(func() { + cluster.Spec.Buggify.CrashLoop = []fdbv1beta2.ProcessGroupID{"*"} + }) + + It("should return nil", func() { + Expect(result).To(BeNil()) + }) + + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + }) }) - It("should not mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + When("Crash loop is set for the specific process group", func() { + BeforeEach(func() { + cluster.Spec.Buggify.CrashLoop = []fdbv1beta2.ProcessGroupID{"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([]fdbv1beta2.ProcessGroupID{})) + }) + }) + + When("Crash loop is set for the main container", func() { + BeforeEach(func() { + cluster.Spec.Buggify.CrashLoopContainers = []fdbv1beta2.CrashLoopContainerObject{ + { + ContainerName: fdbv1beta2.MainContainerName, + Targets: []fdbv1beta2.ProcessGroupID{"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([]fdbv1beta2.ProcessGroupID{})) + }) + }) + + When("Crash loop is set for the sidecar container", func() { + BeforeEach(func() { + cluster.Spec.Buggify.CrashLoopContainers = []fdbv1beta2.CrashLoopContainerObject{ + { + ContainerName: fdbv1beta2.SidecarContainerName, + Targets: []fdbv1beta2.ProcessGroupID{"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([]fdbv1beta2.ProcessGroupID{})) + }) }) }) - When("Crash loop is set for all process groups", func() { + Context("with multiple failed processes", func() { BeforeEach(func() { - cluster.Spec.Buggify.CrashLoop = []fdbv1beta2.ProcessGroupID{"*"} + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-3") + processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ + ProcessGroupConditionType: fdbv1beta2.MissingProcesses, + Timestamp: time.Now().Add(-1 * time.Hour).Unix(), + }) }) - It("should return nil", func() { - Expect(result).To(BeNil()) + It("should requeue", func() { + Expect(result).NotTo(BeNil()) + Expect(result.message).To(Equal("Removals have been updated in the cluster status")) }) - It("should not mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + It("should mark the first process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) + }) + + It("should not be marked to skip exclusion", func() { + for _, pg := range cluster.Status.ProcessGroups { + if pg.ProcessGroupID != "storage-2" { + continue + } + + Expect(pg.ExclusionSkipped).To(BeFalse()) + } }) }) - When("Crash loop is set for the specific process group", func() { + Context("with another in-flight exclusion", func() { BeforeEach(func() { - cluster.Spec.Buggify.CrashLoop = []fdbv1beta2.ProcessGroupID{"storage-2"} + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-3") + processGroup.MarkForRemoval() }) - It("should return nil", func() { - Expect(result).To(BeNil()) + It("should not return nil", func() { + Expect(result).NotTo(BeNil()) + Expect(result.delayedRequeue).To(BeTrue()) + Expect(result.message).To(Equal("More failed process groups are detected")) }) It("should not mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-3"})) + }) + + When("max concurrent replacements is set to two", func() { + BeforeEach(func() { + cluster.Spec.AutomationOptions.Replacements.MaxConcurrentReplacements = pointer.Int(2) + }) + + It("should requeue", func() { + Expect(result).NotTo(BeNil()) + Expect(result.message).To(Equal("Removals have been updated in the cluster status")) + }) + + It("should mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2", "storage-3"})) + }) + }) + + When("max concurrent replacements is set to zero", func() { + BeforeEach(func() { + cluster.Spec.AutomationOptions.Replacements.MaxConcurrentReplacements = pointer.Int(0) + }) + + It("should return nil", func() { + Expect(result).To(BeNil()) + }) + + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-3"})) + }) }) }) - When("Crash loop is set for the main container", func() { + Context("with another complete exclusion", func() { BeforeEach(func() { - cluster.Spec.Buggify.CrashLoopContainers = []fdbv1beta2.CrashLoopContainerObject{ - { - ContainerName: fdbv1beta2.MainContainerName, - Targets: []fdbv1beta2.ProcessGroupID{"storage-2"}, - }, - } + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-3") + processGroup.MarkForRemoval() + processGroup.SetExclude() }) - It("should return nil", func() { - Expect(result).To(BeNil()) + It("should requeue", func() { + Expect(result).NotTo(BeNil()) + Expect(result.message).To(Equal("Removals have been updated in the cluster status")) }) - It("should not mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + It("should mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2", "storage-3"})) }) }) - When("Crash loop is set for the sidecar container", func() { + Context("with no addresses", func() { BeforeEach(func() { - cluster.Spec.Buggify.CrashLoopContainers = []fdbv1beta2.CrashLoopContainerObject{ - { - ContainerName: fdbv1beta2.SidecarContainerName, - Targets: []fdbv1beta2.ProcessGroupID{"storage-2"}, - }, + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.Addresses = nil + }) + + It("should requeue", func() { + Expect(result).NotTo(BeNil()) + Expect(result.message).To(Equal("Removals have been updated in the cluster status")) + }) + + It("should mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) + }) + + It("should marked to skip exclusion", func() { + for _, pg := range cluster.Status.ProcessGroups { + if pg.ProcessGroupID != "storage-2" { + continue + } + + Expect(pg.ExclusionSkipped).To(BeTrue()) } }) - It("should return nil", func() { - Expect(result).To(BeNil()) + When("the cluster is not available", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.Addresses = nil + + adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient) + Expect(err).NotTo(HaveOccurred()) + adminClient.FrozenStatus = &fdbv1beta2.FoundationDBStatus{ + Client: fdbv1beta2.FoundationDBStatusLocalClientInfo{ + DatabaseStatus: fdbv1beta2.FoundationDBStatusClientDBStatus{ + Available: false, + }, + }, + } + }) + + It("should return nil", func() { + Expect(result).To(BeNil()) + }) + + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + }) + }) + + When("the cluster doesn't have full fault tolerance", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.Addresses = nil + + adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient) + Expect(err).NotTo(HaveOccurred()) + adminClient.TeamTracker = []fdbv1beta2.FoundationDBStatusTeamTracker{ + { + Primary: true, + State: fdbv1beta2.FoundationDBStatusDataState{ + Healthy: false, + MinReplicasRemaining: 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([]fdbv1beta2.ProcessGroupID{})) + }) + }) + + When("the cluster uses localities for exclusions", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.Addresses = nil + + cluster.Spec.Version = fdbv1beta2.Versions.SupportsLocalityBasedExclusions71.String() + cluster.Status.RunningVersion = fdbv1beta2.Versions.SupportsLocalityBasedExclusions71.String() + cluster.Spec.AutomationOptions.UseLocalitiesForExclusion = pointer.Bool(true) + Expect(k8sClient.Update(ctx.TODO(), cluster)).NotTo(HaveOccurred()) + }) + + It("should requeue", func() { + Expect(result).NotTo(BeNil()) + Expect(result.message).To(Equal("Removals have been updated in the cluster status")) + }) + + It("should mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) + }) + + It("should not skip the exclusion", func() { + for _, pg := range cluster.Status.ProcessGroups { + if pg.ProcessGroupID != "storage-2" { + continue + } + + Expect(pg.ExclusionSkipped).To(BeFalse()) + } + }) + + }) + }) + + Context("with maintenance mode enabled", func() { + BeforeEach(func() { + adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient) + Expect(err).NotTo(HaveOccurred()) + Expect(adminClient.SetMaintenanceZone("operator-test-1-storage-2", 0)).NotTo(HaveOccurred()) }) It("should not mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + Expect(getRemovedProcessGroupIDs(cluster)).To(BeEmpty()) }) }) }) - Context("with multiple failed processes", func() { + Context("with a process that has been missing for a brief time", func() { BeforeEach(func() { - processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-3") + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ ProcessGroupConditionType: fdbv1beta2.MissingProcesses, + Timestamp: time.Now().Unix(), + }) + }) + + It("should return nil", func() { + Expect(result).To(BeNil()) + }) + + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + }) + }) + + Context("with a process that has had an incorrect pod spec for a long time", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ + ProcessGroupConditionType: fdbv1beta2.IncorrectPodSpec, Timestamp: time.Now().Add(-1 * time.Hour).Unix(), }) }) - It("should requeue", func() { - Expect(result).NotTo(BeNil()) - Expect(result.message).To(Equal("Removals have been updated in the cluster status")) + It("should return nil", func() { + Expect(result).To(BeNil()) }) - It("should mark the first process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) }) + }) - It("should not be marked to skip exclusion", func() { - for _, pg := range cluster.Status.ProcessGroups { - if pg.ProcessGroupID != "storage-2" { - continue - } + When("a process is not marked for removal but is excluded", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ + ProcessGroupConditionType: fdbv1beta2.ProcessIsMarkedAsExcluded, + Timestamp: time.Now().Add(-1 * time.Hour).Unix(), + }) + }) - Expect(pg.ExclusionSkipped).To(BeFalse()) - } + It("should return not nil", + func() { + Expect(result).NotTo(BeNil()) + }) + + It("should mark the process group to be removed", func() { + removedIDs := getRemovedProcessGroupIDs(cluster) + Expect(removedIDs).To(HaveLen(1)) + Expect(removedIDs).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) }) }) - Context("with another in-flight exclusion", func() { + When("a process is marked for removal and has the ProcessIsMarkedAsExcluded condition", func() { BeforeEach(func() { - processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-3") + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ + ProcessGroupConditionType: fdbv1beta2.ProcessIsMarkedAsExcluded, + Timestamp: time.Now().Add(-1 * time.Hour).Unix(), + }) processGroup.MarkForRemoval() }) - It("should not return nil", func() { - Expect(result).NotTo(BeNil()) - Expect(result.delayedRequeue).To(BeTrue()) - Expect(result.message).To(Equal("More failed process groups are detected")) + It("should return nil", func() { + Expect(result).To(BeNil()) }) - It("should not mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-3"})) + It("should mark the process group to be removed", func() { + // The process group is marked as removal in the BeforeEach step. + removedIDs := getRemovedProcessGroupIDs(cluster) + Expect(removedIDs).To(HaveLen(1)) + Expect(removedIDs).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) }) + }) + }) - When("max concurrent replacements is set to two", func() { - BeforeEach(func() { - cluster.Spec.AutomationOptions.Replacements.MaxConcurrentReplacements = pointer.Int(2) + When("fault domain replacements are enabled", func() { + BeforeEach(func() { + cluster.Spec.AutomationOptions.Replacements.FaultDomainBasedReplacements = pointer.Bool(true) + Expect(k8sClient.Update(ctx.TODO(), cluster)).NotTo(HaveOccurred()) + }) + + Context("with a process that has been missing for a long time", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ + ProcessGroupConditionType: fdbv1beta2.MissingProcesses, + Timestamp: time.Now().Add(-1 * time.Hour).Unix(), }) + }) + Context("with no other removals", func() { It("should requeue", func() { Expect(result).NotTo(BeNil()) Expect(result.message).To(Equal("Removals have been updated in the cluster status")) }) It("should mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2", "storage-3"})) + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) }) - }) - When("max concurrent replacements is set to zero", func() { - BeforeEach(func() { - cluster.Spec.AutomationOptions.Replacements.MaxConcurrentReplacements = pointer.Int(0) + It("should not be marked to skip exclusion", func() { + for _, pg := range cluster.Status.ProcessGroups { + if pg.ProcessGroupID != "storage-2" { + continue + } + + Expect(pg.ExclusionSkipped).To(BeFalse()) + } }) - It("should return nil", func() { - Expect(result).To(BeNil()) + When("EmptyMonitorConf is set to true", func() { + BeforeEach(func() { + cluster.Spec.Buggify.EmptyMonitorConf = true + }) + + It("should return nil", func() { + Expect(result).To(BeNil()) + }) + + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + }) }) - It("should not mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-3"})) + When("Crash loop is set for all process groups", func() { + BeforeEach(func() { + cluster.Spec.Buggify.CrashLoop = []fdbv1beta2.ProcessGroupID{"*"} + }) + + It("should return nil", func() { + Expect(result).To(BeNil()) + }) + + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + }) }) - }) - }) - Context("with another complete exclusion", func() { - BeforeEach(func() { - processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-3") - processGroup.MarkForRemoval() - processGroup.SetExclude() - }) + When("Crash loop is set for the specific process group", func() { + BeforeEach(func() { + cluster.Spec.Buggify.CrashLoop = []fdbv1beta2.ProcessGroupID{"storage-2"} + }) - It("should requeue", func() { - Expect(result).NotTo(BeNil()) - Expect(result.message).To(Equal("Removals have been updated in the cluster status")) - }) + It("should return nil", func() { + Expect(result).To(BeNil()) + }) - It("should mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2", "storage-3"})) - }) - }) + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + }) + }) - Context("with no addresses", func() { - BeforeEach(func() { - processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") - processGroup.Addresses = nil - }) + When("Crash loop is set for the main container", func() { + BeforeEach(func() { + cluster.Spec.Buggify.CrashLoopContainers = []fdbv1beta2.CrashLoopContainerObject{ + { + ContainerName: fdbv1beta2.MainContainerName, + Targets: []fdbv1beta2.ProcessGroupID{"storage-2"}, + }, + } + }) - It("should requeue", func() { - Expect(result).NotTo(BeNil()) - Expect(result.message).To(Equal("Removals have been updated in the cluster status")) - }) + It("should return nil", func() { + Expect(result).To(BeNil()) + }) - It("should mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) - }) + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + }) + }) - It("should marked to skip exclusion", func() { - for _, pg := range cluster.Status.ProcessGroups { - if pg.ProcessGroupID != "storage-2" { - continue - } + When("Crash loop is set for the sidecar container", func() { + BeforeEach(func() { + cluster.Spec.Buggify.CrashLoopContainers = []fdbv1beta2.CrashLoopContainerObject{ + { + ContainerName: fdbv1beta2.SidecarContainerName, + Targets: []fdbv1beta2.ProcessGroupID{"storage-2"}, + }, + } + }) - Expect(pg.ExclusionSkipped).To(BeTrue()) - } + It("should return nil", func() { + Expect(result).To(BeNil()) + }) + + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + }) + }) }) - When("the cluster is not available", func() { + Context("with multiple failed processes", func() { BeforeEach(func() { - processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") - processGroup.Addresses = nil + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-3") + processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ + ProcessGroupConditionType: fdbv1beta2.MissingProcesses, + Timestamp: time.Now().Add(-1 * time.Hour).Unix(), + }) + }) - adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient) - Expect(err).NotTo(HaveOccurred()) - adminClient.FrozenStatus = &fdbv1beta2.FoundationDBStatus{ - Client: fdbv1beta2.FoundationDBStatusLocalClientInfo{ - DatabaseStatus: fdbv1beta2.FoundationDBStatusClientDBStatus{ - Available: false, - }, - }, - } + When("those failed processes are on different fault domains", func() { + It("should requeue", func() { + Expect(result).NotTo(BeNil()) + Expect(result.message).To(Equal("Removals have been updated in the cluster status")) + }) + + It("should mark the first process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) + }) + + It("should not be marked to skip exclusion", func() { + for _, pg := range cluster.Status.ProcessGroups { + if pg.ProcessGroupID != "storage-2" { + continue + } + + Expect(pg.ExclusionSkipped).To(BeFalse()) + } + }) }) - It("should return nil", func() { - Expect(result).To(BeNil()) + When("those failed processes are on the same fault domain", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-3") + processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ + ProcessGroupConditionType: fdbv1beta2.MissingProcesses, + Timestamp: time.Now().Add(-1 * time.Hour).Unix(), + }) + // Put the storage-3 on the same fault domain + processGroup.FaultDomain = fdbv1beta2.FaultDomain(cluster.Name + "-storage-2") + }) + + It("should requeue", func() { + Expect(result).NotTo(BeNil()) + Expect(result.message).To(Equal("Removals have been updated in the cluster status")) + }) + + It("should mark both process groups for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2", "storage-3"})) + }) + }) + }) + + Context("with another in-flight exclusion", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-3") + processGroup.MarkForRemoval() + }) + + It("should not return nil", func() { + Expect(result).NotTo(BeNil()) + Expect(result.delayedRequeue).To(BeTrue()) + Expect(result.message).To(Equal("More failed process groups are detected")) }) It("should not mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-3"})) + }) + + When("both processes are in the same fault domain", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-3") + // Put the storage-3 on the same fault domain + processGroup.FaultDomain = fdbv1beta2.FaultDomain(cluster.Name + "-storage-2") + }) + + It("should not return nil", func() { + Expect(result).NotTo(BeNil()) + Expect(result.delayedRequeue).To(BeFalse()) + Expect(result.message).To(Equal("Removals have been updated in the cluster status")) + }) + + It("should mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2", "storage-3"})) + }) + }) + + When("max concurrent replacements is set to two", func() { + BeforeEach(func() { + cluster.Spec.AutomationOptions.Replacements.MaxConcurrentReplacements = pointer.Int(2) + }) + + It("should requeue", func() { + Expect(result).NotTo(BeNil()) + Expect(result.message).To(Equal("Removals have been updated in the cluster status")) + }) + + It("should mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2", "storage-3"})) + }) + }) + + When("max concurrent replacements is set to zero", func() { + BeforeEach(func() { + cluster.Spec.AutomationOptions.Replacements.MaxConcurrentReplacements = pointer.Int(0) + }) + + It("should return nil", func() { + Expect(result).To(BeNil()) + }) + + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-3"})) + }) }) }) - When("the cluster doesn't have full fault tolerance", func() { + Context("with another complete exclusion", func() { BeforeEach(func() { - processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") - processGroup.Addresses = nil - - adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient) - Expect(err).NotTo(HaveOccurred()) - adminClient.TeamTracker = []fdbv1beta2.FoundationDBStatusTeamTracker{ - { - Primary: true, - State: fdbv1beta2.FoundationDBStatusDataState{ - Healthy: false, - MinReplicasRemaining: 2, - }, - }, - } + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-3") + processGroup.MarkForRemoval() + processGroup.SetExclude() }) - It("should return nil", func() { - Expect(result).To(BeNil()) + It("should requeue", func() { + Expect(result).NotTo(BeNil()) + Expect(result.message).To(Equal("Removals have been updated in the cluster status")) }) - It("should not mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + It("should mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2", "storage-3"})) }) }) - When("the cluster uses localities for exclusions", func() { + Context("with no addresses", func() { BeforeEach(func() { processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") processGroup.Addresses = nil - - cluster.Spec.Version = fdbv1beta2.Versions.SupportsLocalityBasedExclusions71.String() - cluster.Status.RunningVersion = fdbv1beta2.Versions.SupportsLocalityBasedExclusions71.String() - cluster.Spec.AutomationOptions.UseLocalitiesForExclusion = pointer.Bool(true) - Expect(k8sClient.Update(ctx.TODO(), cluster)).NotTo(HaveOccurred()) }) It("should requeue", func() { @@ -940,108 +1311,191 @@ var _ = Describe("replace_failed_process_groups", func() { Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) }) - It("should not skip the exclusion", func() { + It("should marked to skip exclusion", func() { for _, pg := range cluster.Status.ProcessGroups { if pg.ProcessGroupID != "storage-2" { continue } - Expect(pg.ExclusionSkipped).To(BeFalse()) + Expect(pg.ExclusionSkipped).To(BeTrue()) } }) + When("the cluster is not available", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.Addresses = nil + + adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient) + Expect(err).NotTo(HaveOccurred()) + adminClient.FrozenStatus = &fdbv1beta2.FoundationDBStatus{ + Client: fdbv1beta2.FoundationDBStatusLocalClientInfo{ + DatabaseStatus: fdbv1beta2.FoundationDBStatusClientDBStatus{ + Available: false, + }, + }, + } + }) + + It("should return nil", func() { + Expect(result).To(BeNil()) + }) + + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) + }) + }) + + When("the cluster doesn't have full fault tolerance", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.Addresses = nil + + adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient) + Expect(err).NotTo(HaveOccurred()) + adminClient.TeamTracker = []fdbv1beta2.FoundationDBStatusTeamTracker{ + { + Primary: true, + State: fdbv1beta2.FoundationDBStatusDataState{ + Healthy: false, + MinReplicasRemaining: 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([]fdbv1beta2.ProcessGroupID{})) + }) + }) + + When("the cluster uses localities for exclusions", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.Addresses = nil + + cluster.Spec.Version = fdbv1beta2.Versions.SupportsLocalityBasedExclusions71.String() + cluster.Status.RunningVersion = fdbv1beta2.Versions.SupportsLocalityBasedExclusions71.String() + cluster.Spec.AutomationOptions.UseLocalitiesForExclusion = pointer.Bool(true) + Expect(k8sClient.Update(ctx.TODO(), cluster)).NotTo(HaveOccurred()) + }) + + It("should requeue", func() { + Expect(result).NotTo(BeNil()) + Expect(result.message).To(Equal("Removals have been updated in the cluster status")) + }) + + It("should mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) + }) + + It("should not skip the exclusion", func() { + for _, pg := range cluster.Status.ProcessGroups { + if pg.ProcessGroupID != "storage-2" { + continue + } + + Expect(pg.ExclusionSkipped).To(BeFalse()) + } + }) + + }) + }) + + Context("with maintenance mode enabled", func() { + BeforeEach(func() { + adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient) + Expect(err).NotTo(HaveOccurred()) + Expect(adminClient.SetMaintenanceZone("operator-test-1-storage-2", 0)).NotTo(HaveOccurred()) + }) + + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(BeEmpty()) + }) }) }) - Context("with maintenance mode enabled", func() { + Context("with a process that has been missing for a brief time", func() { BeforeEach(func() { - adminClient, err := mock.NewMockAdminClientUncast(cluster, k8sClient) - Expect(err).NotTo(HaveOccurred()) - Expect(adminClient.SetMaintenanceZone("operator-test-1-storage-2", 0)).NotTo(HaveOccurred()) + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ + ProcessGroupConditionType: fdbv1beta2.MissingProcesses, + Timestamp: time.Now().Unix(), + }) }) - It("should not mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(BeEmpty()) + It("should return nil", func() { + Expect(result).To(BeNil()) }) - }) - }) - Context("with a process that has been missing for a brief time", func() { - BeforeEach(func() { - processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") - processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ - ProcessGroupConditionType: fdbv1beta2.MissingProcesses, - Timestamp: time.Now().Unix(), + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) }) }) - It("should return nil", func() { - Expect(result).To(BeNil()) - }) + Context("with a process that has had an incorrect pod spec for a long time", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ + ProcessGroupConditionType: fdbv1beta2.IncorrectPodSpec, + Timestamp: time.Now().Add(-1 * time.Hour).Unix(), + }) + }) - It("should not mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) - }) - }) + It("should return nil", func() { + Expect(result).To(BeNil()) + }) - Context("with a process that has had an incorrect pod spec for a long time", func() { - BeforeEach(func() { - processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") - processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ - ProcessGroupConditionType: fdbv1beta2.IncorrectPodSpec, - Timestamp: time.Now().Add(-1 * time.Hour).Unix(), + It("should not mark the process group for removal", func() { + Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) }) }) - It("should return nil", func() { - Expect(result).To(BeNil()) - }) + When("a process is not marked for removal but is excluded", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ + ProcessGroupConditionType: fdbv1beta2.ProcessIsMarkedAsExcluded, + Timestamp: time.Now().Add(-1 * time.Hour).Unix(), + }) + }) - It("should not mark the process group for removal", func() { - Expect(getRemovedProcessGroupIDs(cluster)).To(Equal([]fdbv1beta2.ProcessGroupID{})) - }) - }) + It("should return not nil", + func() { + Expect(result).NotTo(BeNil()) + }) - When("a process is not marked for removal but is excluded", func() { - BeforeEach(func() { - processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") - processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ - ProcessGroupConditionType: fdbv1beta2.ProcessIsMarkedAsExcluded, - Timestamp: time.Now().Add(-1 * time.Hour).Unix(), + It("should mark the process group to be removed", func() { + removedIDs := getRemovedProcessGroupIDs(cluster) + Expect(removedIDs).To(HaveLen(1)) + Expect(removedIDs).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) }) }) - It("should return not nil", - func() { - Expect(result).NotTo(BeNil()) + When("a process is marked for removal and has the ProcessIsMarkedAsExcluded condition", func() { + BeforeEach(func() { + processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") + processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ + ProcessGroupConditionType: fdbv1beta2.ProcessIsMarkedAsExcluded, + Timestamp: time.Now().Add(-1 * time.Hour).Unix(), + }) + processGroup.MarkForRemoval() }) - It("should mark the process group to be removed", func() { - removedIDs := getRemovedProcessGroupIDs(cluster) - Expect(removedIDs).To(HaveLen(1)) - Expect(removedIDs).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) - }) - }) - - When("a process is marked for removal and has the ProcessIsMarkedAsExcluded condition", func() { - BeforeEach(func() { - processGroup := fdbv1beta2.FindProcessGroupByID(cluster.Status.ProcessGroups, "storage-2") - processGroup.ProcessGroupConditions = append(processGroup.ProcessGroupConditions, &fdbv1beta2.ProcessGroupCondition{ - ProcessGroupConditionType: fdbv1beta2.ProcessIsMarkedAsExcluded, - Timestamp: time.Now().Add(-1 * time.Hour).Unix(), + It("should return nil", func() { + Expect(result).To(BeNil()) }) - processGroup.MarkForRemoval() - }) - It("should return nil", func() { - Expect(result).To(BeNil()) - }) - - It("should mark the process group to be removed", func() { - // The process group is marked as removal in the BeforeEach step. - removedIDs := getRemovedProcessGroupIDs(cluster) - Expect(removedIDs).To(HaveLen(1)) - Expect(removedIDs).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) + It("should mark the process group to be removed", func() { + // The process group is marked as removal in the BeforeEach step. + removedIDs := getRemovedProcessGroupIDs(cluster) + Expect(removedIDs).To(HaveLen(1)) + Expect(removedIDs).To(Equal([]fdbv1beta2.ProcessGroupID{"storage-2"})) + }) }) }) }) diff --git a/docs/cluster_spec.md b/docs/cluster_spec.md index 896f251e7..3d7f452b4 100644 --- a/docs/cluster_spec.md +++ b/docs/cluster_spec.md @@ -47,6 +47,7 @@ AutomaticReplacementOptions controls options for automatically replacing failed | Field | Description | Scheme | Required | | ----- | ----------- | ------ | -------- | | enabled | Enabled controls whether automatic replacements are enabled. The default is false. | *bool | false | +| faultDomainBasedReplacements | FaultDomainBasedReplacements controls whether automatic replacements are targeting all failed process groups in a fault domain or only specific Process Groups. If this setting is enabled, the number of different fault domains that can have all their failed process groups replaced at the same time will be equal to MaxConcurrentReplacements. e.g. MaxConcurrentReplacements = 2 would mean that at most 2 different fault domains can have their failed process groups replaced at the same time. The default is false. | *bool | false | | failureDetectionTimeSeconds | FailureDetectionTimeSeconds controls how long a process must be failed or missing before it is automatically replaced. The default is 7200 seconds, or 2 hours. | *int | false | | taintReplacementTimeSeconds | TaintReplacementTimeSeconds controls how long a pod stays in NodeTaintReplacing condition before it is automatically replaced. The default is 1800 seconds, i.e., 30min | *int | false | | maxConcurrentReplacements | MaxConcurrentReplacements controls how many automatic replacements are allowed to take part. This will take the list of current replacements and then calculate the difference between maxConcurrentReplacements and the size of the list. e.g. if currently 3 replacements are queued (e.g. in the processGroupsToRemove list) and maxConcurrentReplacements is 5 the operator is allowed to replace at most 2 process groups. Setting this to 0 will basically disable the automatic replacements. | *int | false | diff --git a/internal/replacements/replace_failed_process_groups.go b/internal/replacements/replace_failed_process_groups.go index a497f62a8..387ca9815 100644 --- a/internal/replacements/replace_failed_process_groups.go +++ b/internal/replacements/replace_failed_process_groups.go @@ -28,7 +28,10 @@ import ( "github.com/go-logr/logr" ) -func getMaxReplacements(cluster *fdbv1beta2.FoundationDBCluster, maxReplacements int) int { +// getReplacementInformation will return the maximum allowed replacements for process group based replacements and the +// fault domains that have an ongoing replacement. +func getReplacementInformation(cluster *fdbv1beta2.FoundationDBCluster, maxReplacements int) (int, map[fdbv1beta2.FaultDomain]fdbv1beta2.None) { + faultDomains := map[fdbv1beta2.FaultDomain]fdbv1beta2.None{} // The maximum number of replacements will be the defined number in the cluster spec // minus all currently ongoing replacements e.g. process groups marked for removal but // not fully excluded. @@ -37,10 +40,38 @@ func getMaxReplacements(cluster *fdbv1beta2.FoundationDBCluster, maxReplacements if processGroupStatus.IsMarkedForRemoval() && !processGroupStatus.IsExcluded() { // Count all removals that are in-flight. removalCount++ + faultDomains[processGroupStatus.FaultDomain] = fdbv1beta2.None{} } } - return maxReplacements - removalCount + return maxReplacements - removalCount, faultDomains +} + +// removalAllowed will return true if the removal is allowed based on the clusters automatic replacement configuration. +func removalAllowed(cluster *fdbv1beta2.FoundationDBCluster, maxReplacements int, faultDomainsWithReplacements map[fdbv1beta2.FaultDomain]fdbv1beta2.None, faultDomain fdbv1beta2.FaultDomain) bool { + if !cluster.FaultDomainBasedReplacements() { + // If we are here we target the replacements on a process group level + return maxReplacements > 0 + } + + // We have to check how many fault domains currently have a replacement ongoing. If more than MaxConcurrentReplacements + // fault domains have a replacement ongoing, we will reject any further replacement. + maxFaultDomainsWithAReplacement := cluster.GetMaxConcurrentAutomaticReplacements() + if len(faultDomainsWithReplacements) > maxFaultDomainsWithAReplacement { + return false + } + + // If the current fault domains with a replacements equals to MaxConcurrentReplacements we are only allowed + // to approve the replacement of process groups that are in a fault domain that currently has an ongoing + // replacement. + if len(faultDomainsWithReplacements) == maxFaultDomainsWithAReplacement { + _, faultDomainHasReplacements := faultDomainsWithReplacements[faultDomain] + return faultDomainHasReplacements + } + + // At this point we have less than MaxConcurrentReplacements fault domains with a replacement, so it's fine to + // approve the replacement. + return true } // ReplaceFailedProcessGroups flags failed processes groups for removal. The first return value will indicate if any @@ -63,7 +94,7 @@ func ReplaceFailedProcessGroups(log logr.Logger, cluster *fdbv1beta2.FoundationD ignore = targets } - maxReplacements := getMaxReplacements(cluster, cluster.GetMaxConcurrentAutomaticReplacements()) + maxReplacements, faultDomainsWithReplacements := getReplacementInformation(cluster, cluster.GetMaxConcurrentAutomaticReplacements()) hasReplacement := false hasMoreFailedProcesses := false localitiesUsedForExclusion := cluster.UseLocalitiesForExclusion() @@ -117,7 +148,7 @@ func ReplaceFailedProcessGroups(log logr.Logger, cluster *fdbv1beta2.FoundationD } // We are not allowed to replace additional process groups. - if maxReplacements <= 0 { + if !removalAllowed(cluster, maxReplacements, faultDomainsWithReplacements, processGroupStatus.FaultDomain) { // If there are more processes that should be replaced but we hit the replace limit, we want to make sure // the controller queues another reconciliation to eventually replace this failed process group. hasMoreFailedProcesses = true @@ -139,6 +170,7 @@ func ReplaceFailedProcessGroups(log logr.Logger, cluster *fdbv1beta2.FoundationD hasReplacement = true processGroupStatus.ExclusionSkipped = skipExclusion maxReplacements-- + faultDomainsWithReplacements[processGroupStatus.FaultDomain] = fdbv1beta2.None{} } return hasReplacement, hasMoreFailedProcesses diff --git a/internal/replacements/replace_failed_process_groups_test.go b/internal/replacements/replace_failed_process_groups_test.go new file mode 100644 index 000000000..cd66c6950 --- /dev/null +++ b/internal/replacements/replace_failed_process_groups_test.go @@ -0,0 +1,117 @@ +/* + * replace_failed_process_groups_test.go + * + * This source file is part of the FoundationDB open source project + * + * Copyright 2024 Apple Inc. and the FoundationDB project authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package replacements + +import ( + fdbv1beta2 "github.com/FoundationDB/fdb-kubernetes-operator/api/v1beta2" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/utils/pointer" +) + +var _ = Describe("replace_failed_process_groups", func() { + DescribeTable("check if removal is allowed", func(cluster *fdbv1beta2.FoundationDBCluster, maxReplacements int, faultDomainsWithReplacements map[fdbv1beta2.FaultDomain]fdbv1beta2.None, faultDomain fdbv1beta2.FaultDomain, expected bool) { + Expect(removalAllowed(cluster, maxReplacements, faultDomainsWithReplacements, faultDomain)).To(Equal(expected)) + }, + Entry("process group based replacement: with 1 replacement allowed", + &fdbv1beta2.FoundationDBCluster{}, + 1, + nil, + fdbv1beta2.FaultDomain(""), + true, + ), + Entry("process group based replacement: with 0 replacements allowed", + &fdbv1beta2.FoundationDBCluster{}, + 0, + nil, + fdbv1beta2.FaultDomain(""), + false, + ), + Entry("fault domain based replacement: no ongoing replacements", + &fdbv1beta2.FoundationDBCluster{ + Spec: fdbv1beta2.FoundationDBClusterSpec{ + AutomationOptions: fdbv1beta2.FoundationDBClusterAutomationOptions{ + Replacements: fdbv1beta2.AutomaticReplacementOptions{ + FaultDomainBasedReplacements: pointer.Bool(true), + }, + }, + }, + }, + 0, + nil, + fdbv1beta2.FaultDomain("zone1"), + true, + ), + Entry("fault domain based replacement: ongoing replacements same fault domain", + &fdbv1beta2.FoundationDBCluster{ + Spec: fdbv1beta2.FoundationDBClusterSpec{ + AutomationOptions: fdbv1beta2.FoundationDBClusterAutomationOptions{ + Replacements: fdbv1beta2.AutomaticReplacementOptions{ + FaultDomainBasedReplacements: pointer.Bool(true), + }, + }, + }, + }, + 0, + map[fdbv1beta2.FaultDomain]fdbv1beta2.None{ + "zone1": {}, + }, + fdbv1beta2.FaultDomain("zone1"), + true, + ), + Entry("fault domain based replacement: ongoing replacements different fault domain", + &fdbv1beta2.FoundationDBCluster{ + Spec: fdbv1beta2.FoundationDBClusterSpec{ + AutomationOptions: fdbv1beta2.FoundationDBClusterAutomationOptions{ + Replacements: fdbv1beta2.AutomaticReplacementOptions{ + FaultDomainBasedReplacements: pointer.Bool(true), + }, + }, + }, + }, + 0, + map[fdbv1beta2.FaultDomain]fdbv1beta2.None{ + "zone1": {}, + }, + fdbv1beta2.FaultDomain("zone2"), + false, + ), + Entry("fault domain based replacement: too many ongoing replacements same fault domain", + &fdbv1beta2.FoundationDBCluster{ + Spec: fdbv1beta2.FoundationDBClusterSpec{ + AutomationOptions: fdbv1beta2.FoundationDBClusterAutomationOptions{ + Replacements: fdbv1beta2.AutomaticReplacementOptions{ + FaultDomainBasedReplacements: pointer.Bool(true), + }, + }, + }, + }, + 0, + map[fdbv1beta2.FaultDomain]fdbv1beta2.None{ + "zone1": {}, + "zone2": {}, + }, + fdbv1beta2.FaultDomain("zone1"), + false, + ), + ) + +}) diff --git a/internal/replacements/replacements.go b/internal/replacements/replacements.go index 0ce7195a5..78924e8e1 100644 --- a/internal/replacements/replacements.go +++ b/internal/replacements/replacements.go @@ -42,7 +42,7 @@ import ( func ReplaceMisconfiguredProcessGroups(ctx context.Context, podManager podmanager.PodLifecycleManager, client client.Client, log logr.Logger, cluster *fdbv1beta2.FoundationDBCluster, pvcMap map[fdbv1beta2.ProcessGroupID]corev1.PersistentVolumeClaim) (bool, error) { hasReplacements := false - maxReplacements := getMaxReplacements(cluster, cluster.GetMaxConcurrentReplacements()) + maxReplacements, _ := getReplacementInformation(cluster, cluster.GetMaxConcurrentReplacements()) for _, processGroup := range cluster.Status.ProcessGroups { if maxReplacements <= 0 { log.Info("Early abort, reached limit of concurrent replacements")