Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit b46b41a

Browse files
authoredOct 24, 2023
scheduler: appropriately unblock evals with quotas (hashicorp#18838)
When an eval is blocked due to e.g. cpu exhausted on nodes, but there happens to also be a quota on the job's namespace, the eval would not get auto- unblocked when the node cpu got freed up. This change ensures, when considering quota during BlockedEvals.unblock(), that the block was due to quota in the first place, so unblocking does not get skipped due to the mere existence of a quota on the namespace.
1 parent 5cf4c6c commit b46b41a

File tree

3 files changed

+36
-2
lines changed

3 files changed

+36
-2
lines changed
 

‎.changelog/18838.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:bug
2+
scheduler (Enterprise): auto-unblock evals with associated quotas when node resources are freed up
3+
```

‎nomad/blocked_evals.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -567,10 +567,13 @@ func (b *BlockedEvals) unblock(computedClass, quota string, index uint64) {
567567
// never saw a node with the given computed class and thus needs to be
568568
// unblocked for correctness.
569569
for id, wrapped := range b.captured {
570-
if quota != "" && wrapped.eval.QuotaLimitReached != quota {
570+
if quota != "" &&
571+
wrapped.eval.QuotaLimitReached != "" &&
572+
wrapped.eval.QuotaLimitReached != quota {
571573
// We are unblocking based on quota and this eval doesn't match
572574
continue
573-
} else if elig, ok := wrapped.eval.ClassEligibility[computedClass]; ok && !elig {
575+
}
576+
if elig, ok := wrapped.eval.ClassEligibility[computedClass]; ok && !elig {
574577
// Can skip because the eval has explicitly marked the node class
575578
// as ineligible.
576579
continue

‎nomad/blocked_evals_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/hashicorp/nomad/nomad/mock"
1414
"github.com/hashicorp/nomad/nomad/structs"
1515
"github.com/hashicorp/nomad/testutil"
16+
"github.com/shoenig/test/must"
1617
"github.com/stretchr/testify/require"
1718
)
1819

@@ -322,6 +323,33 @@ func TestBlockedEvals_UnblockEligible_Quota(t *testing.T) {
322323
requireBlockedEvalsEnqueued(t, blocked, broker, 1)
323324
}
324325

326+
// The quota here is incidental. The eval is blocked due to something else,
327+
// e.g. cpu exhausted, but there happens to also be a quota on the namespace.
328+
func TestBlockedEvals_UnblockEligible_IncidentalQuota(t *testing.T) {
329+
ci.Parallel(t)
330+
331+
blocked, broker := testBlockedEvals(t)
332+
333+
e := mock.BlockedEval()
334+
e.Status = structs.EvalStatusBlocked
335+
e.QuotaLimitReached = "" // explicitly not blocked due to quota limit
336+
blocked.Block(e)
337+
338+
// Verify block caused the eval to be tracked.
339+
blockedStats := blocked.Stats()
340+
must.Eq(t, 1, blockedStats.TotalBlocked)
341+
must.MapLen(t, 1, blockedStats.BlockedResources.ByJob)
342+
// but not due to quota.
343+
must.Eq(t, 0, blockedStats.TotalQuotaLimit)
344+
345+
// When unblocking, the quota name from the alloc is passed in,
346+
// regardless of the cause of the initial blockage.
347+
// Since the initial block in this test was due to something else,
348+
// it should be unblocked without regard to quota.
349+
blocked.UnblockQuota("foo", 1000)
350+
requireBlockedEvalsEnqueued(t, blocked, broker, 1)
351+
}
352+
325353
func TestBlockedEvals_UnblockIneligible_Quota(t *testing.T) {
326354
ci.Parallel(t)
327355
require := require.New(t)

0 commit comments

Comments
 (0)
Failed to load comments.