diff --git a/pkg/controllers/deprovisioning/controller.go b/pkg/controllers/deprovisioning/controller.go index f5be95099a..cb76bc42ee 100644 --- a/pkg/controllers/deprovisioning/controller.go +++ b/pkg/controllers/deprovisioning/controller.go @@ -106,6 +106,7 @@ func (c *Controller) Builder(_ context.Context, m manager.Manager) controller.Bu func (c *Controller) Reconcile(ctx context.Context, _ reconcile.Request) (reconcile.Result, error) { // Attempt different deprovisioning methods. We'll only let one method perform an action + isConsolidated := c.cluster.Consolidated() for _, d := range c.deprovisioners { candidates, err := candidateNodes(ctx, c.cluster, c.kubeClient, c.clock, c.cloudProvider, d.ShouldDeprovision) if err != nil { @@ -135,8 +136,11 @@ func (c *Controller) Reconcile(ctx context.Context, _ reconcile.Request) (reconc return reconcile.Result{Requeue: true}, nil } + if !isConsolidated { + // Mark cluster as consolidated, only if the deprovisioners ran and were not able to perform any work. + c.cluster.SetConsolidated(true) + } // All deprovisioners did nothing, so return nothing to do - c.cluster.SetConsolidated(true) // Mark cluster as consolidated return reconcile.Result{RequeueAfter: pollingPeriod}, nil } diff --git a/pkg/controllers/deprovisioning/suite_test.go b/pkg/controllers/deprovisioning/suite_test.go index 5b2cad9106..b9af38bd8c 100644 --- a/pkg/controllers/deprovisioning/suite_test.go +++ b/pkg/controllers/deprovisioning/suite_test.go @@ -134,6 +134,26 @@ var _ = AfterEach(func() { } }) +var _ = Describe("Consolidation State", func() { + It("should not reset consolidation state if consolidation hasn't run", func() { + // this assumes that the consolidate reset period is 5 minutes, which it is currently + _, err := deprovisioningController.Reconcile(ctx, reconcile.Request{}) + Expect(err).ToNot(HaveOccurred()) + Expect(cluster.Consolidated()).To(BeTrue()) + fakeClock.Step(1 * time.Minute) + Expect(cluster.Consolidated()).To(BeTrue()) + + // reconciling now shouldn't set the last consolidated time to current time, as consolidation isn't actually + // running since it last ran 1 minute ago + _, err = deprovisioningController.Reconcile(ctx, reconcile.Request{}) + Expect(err).ToNot(HaveOccurred()) + // but advancing the clock 4:30, so we are at 5:30 past the last run time should cause consolidated to return + // false + fakeClock.Step(4*time.Minute + 30*time.Second) + Expect(cluster.Consolidated()).To(BeFalse()) + }) +}) + var _ = Describe("Pod Eviction Cost", func() { const standardPodCost = 1.0 It("should have a standard disruptionCost for a pod with no priority or disruptionCost specified", func() {