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.

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

scheduler: appropriately unblock evals with quotas #18838

Merged
merged 1 commit into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .changelog/18838.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
scheduler (Enterprise): auto-unblock evals with associated quotas when node resources are freed up
```
7 changes: 5 additions & 2 deletions nomad/blocked_evals.go
Original file line number Diff line number Diff line change
Expand Up @@ -567,10 +567,13 @@ func (b *BlockedEvals) unblock(computedClass, quota string, index uint64) {
// never saw a node with the given computed class and thus needs to be
// unblocked for correctness.
for id, wrapped := range b.captured {
if quota != "" && wrapped.eval.QuotaLimitReached != quota {
if quota != "" &&
wrapped.eval.QuotaLimitReached != "" &&
wrapped.eval.QuotaLimitReached != quota {
// We are unblocking based on quota and this eval doesn't match
continue
} else if elig, ok := wrapped.eval.ClassEligibility[computedClass]; ok && !elig {
}
if elig, ok := wrapped.eval.ClassEligibility[computedClass]; ok && !elig {
// Can skip because the eval has explicitly marked the node class
// as ineligible.
continue
Expand Down
28 changes: 28 additions & 0 deletions nomad/blocked_evals_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/hashicorp/nomad/testutil"
"github.com/shoenig/test/must"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -322,6 +323,33 @@ func TestBlockedEvals_UnblockEligible_Quota(t *testing.T) {
requireBlockedEvalsEnqueued(t, blocked, broker, 1)
}

// The quota here is incidental. The eval is blocked due to something else,
// e.g. cpu exhausted, but there happens to also be a quota on the namespace.
func TestBlockedEvals_UnblockEligible_IncidentalQuota(t *testing.T) {
ci.Parallel(t)

blocked, broker := testBlockedEvals(t)

e := mock.BlockedEval()
e.Status = structs.EvalStatusBlocked
e.QuotaLimitReached = "" // explicitly not blocked due to quota limit
blocked.Block(e)

// Verify block caused the eval to be tracked.
blockedStats := blocked.Stats()
must.Eq(t, 1, blockedStats.TotalBlocked)
must.MapLen(t, 1, blockedStats.BlockedResources.ByJob)
// but not due to quota.
must.Eq(t, 0, blockedStats.TotalQuotaLimit)

// When unblocking, the quota name from the alloc is passed in,
// regardless of the cause of the initial blockage.
// Since the initial block in this test was due to something else,
// it should be unblocked without regard to quota.
blocked.UnblockQuota("foo", 1000)
requireBlockedEvalsEnqueued(t, blocked, broker, 1)
}

func TestBlockedEvals_UnblockIneligible_Quota(t *testing.T) {
ci.Parallel(t)
require := require.New(t)
Expand Down