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

fix(nodeclaim): dedup logs marking consolidatable #2018

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(nodeclaim): dedup logs marking consolidatable #2018

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
518ff10
fix(nodeclaim): dedup logs marking consolidatable
flavono123 Feb 21, 2025
155c1ec
fix(nodeclaim): set and sleep only when the nodeclaim becomes consoli…
flavono123 Feb 22, 2025
f02a182
fix(nodeclaim): sleep for each reconciliation of consolidation contro…
flavono123 Feb 26, 2025
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
7 changes: 6 additions & 1 deletion pkg/controllers/nodeclaim/disruption/consolidation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package disruption

import (
"context"
"time"

"github.com/samber/lo"
"k8s.io/utils/clock"
Expand Down Expand Up @@ -69,8 +70,12 @@ func (c *Consolidation) Reconcile(ctx context.Context, nodePool *v1.NodePool, no
return reconcile.Result{RequeueAfter: consolidatableTime.Sub(c.clock.Now())}, nil
}

// 6. Otherwise, add the consolidatable status condition
// 3. Otherwise, add the consolidatable status condition if not already set
nodeClaim.StatusConditions().SetTrue(v1.ConditionTypeConsolidatable)
// We sleep here after a set operation since we want to ensure that we are able to read our own writes
// so that we avoid duplicating log lines due to quick re-queues from our node watcher
time.Sleep(100 * time.Millisecond)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should only sleep if the condition has changed. Otherwise, we risk sleeping on every iteration


if !hasConsolidatableCondition {
log.FromContext(ctx).V(1).Info("marking consolidatable")
}
Expand Down
Loading