diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index efa4d0fc6..91f6db052 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -6441,6 +6441,10 @@ func (tg *TaskGroup) Canonicalize(job *Job) { tg.EphemeralDisk = DefaultEphemeralDisk() } + if job.Type == JobTypeSystem && tg.Count == 0 { + tg.Count = 1 + } + if tg.Scaling != nil { tg.Scaling.Canonicalize() } diff --git a/scheduler/system_util_test.go b/scheduler/system_util_test.go index 333d8787c..9bc529859 100644 --- a/scheduler/system_util_test.go +++ b/scheduler/system_util_test.go @@ -5,8 +5,8 @@ import ( "testing" "time" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" + "github.com/shoenig/test" + "github.com/shoenig/test/must" "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/helper/pointer" @@ -15,6 +15,23 @@ import ( "github.com/hashicorp/nomad/nomad/structs" ) +// diffResultCount is a test helper struct that makes it easier to specify an +// expected diff +type diffResultCount struct { + place, update, migrate, stop, ignore, lost, disconnecting, reconnecting int +} + +// assertDiffCount is a test helper that compares against a diffResult +func assertDiffCount(t *testing.T, expected diffResultCount, diff *diffResult) { + t.Helper() + test.Len(t, expected.update, diff.update, test.Sprintf("expected update")) + test.Len(t, expected.ignore, diff.ignore, test.Sprintf("expected ignore")) + test.Len(t, expected.stop, diff.stop, test.Sprintf("expected stop")) + test.Len(t, expected.migrate, diff.migrate, test.Sprintf("expected migrate")) + test.Len(t, expected.lost, diff.lost, test.Sprintf("expected lost")) + test.Len(t, expected.place, diff.place, test.Sprintf("expected place")) +} + func TestDiffSystemAllocsForNode_Sysbatch_terminal(t *testing.T) { ci.Parallel(t) @@ -22,6 +39,7 @@ func TestDiffSystemAllocsForNode_Sysbatch_terminal(t *testing.T) { // that has become terminal, unless the job has been updated. job := mock.SystemBatchJob() + job.TaskGroups[0].Count = 2 required := materializeSystemTaskGroups(job) eligible := map[string]*structs.Node{ @@ -46,12 +64,11 @@ func TestDiffSystemAllocsForNode_Sysbatch_terminal(t *testing.T) { } diff := diffSystemAllocsForNode(job, "node1", eligible, nil, tainted, required, live, terminal, true) - require.Empty(t, diff.place) - require.Empty(t, diff.update) - require.Empty(t, diff.stop) - require.Empty(t, diff.migrate) - require.Empty(t, diff.lost) - require.True(t, len(diff.ignore) == 1 && diff.ignore[0].Alloc == terminal["node1"]["my-sysbatch.pinger[0]"]) + + assertDiffCount(t, diffResultCount{ignore: 1, place: 1}, diff) + if len(diff.ignore) > 0 { + must.Eq(t, terminal["node1"]["my-sysbatch.pinger[0]"], diff.ignore[0].Alloc) + } }) t.Run("outdated job", func(t *testing.T) { @@ -68,205 +85,309 @@ func TestDiffSystemAllocsForNode_Sysbatch_terminal(t *testing.T) { }, } - expAlloc := terminal["node1"]["my-sysbatch.pinger[0]"] - expAlloc.NodeID = "node1" - diff := diffSystemAllocsForNode(job, "node1", eligible, nil, tainted, required, live, terminal, true) - require.Empty(t, diff.place) - require.Len(t, diff.update, 1) - require.Empty(t, diff.stop) - require.Empty(t, diff.migrate) - require.Empty(t, diff.lost) - require.Empty(t, diff.ignore) + assertDiffCount(t, diffResultCount{update: 1, place: 1}, diff) }) + } -func TestDiffSystemAllocsForNode(t *testing.T) { - ci.Parallel(t) - - job := mock.Job() - required := materializeSystemTaskGroups(job) - - // The "old" job has a previous modify index - oldJob := new(structs.Job) - *oldJob = *job - oldJob.JobModifyIndex -= 1 - - eligibleNode := mock.Node() - eligibleNode.ID = "zip" - - drainNode := mock.DrainNode() - - deadNode := mock.Node() - deadNode.Status = structs.NodeStatusDown - - tainted := map[string]*structs.Node{ - "dead": deadNode, - "drainNode": drainNode, - } - - eligible := map[string]*structs.Node{ - eligibleNode.ID: eligibleNode, - } - - allocs := []*structs.Allocation{ - // Update the 1st - { - ID: uuid.Generate(), - NodeID: "zip", - Name: "my-job.web[0]", - Job: oldJob, - }, - - // Ignore the 2rd - { - ID: uuid.Generate(), - NodeID: "zip", - Name: "my-job.web[1]", - Job: job, - }, - - // Evict 11th - { - ID: uuid.Generate(), - NodeID: "zip", - Name: "my-job.web[10]", - Job: oldJob, - }, - - // Migrate the 3rd - { - ID: uuid.Generate(), - NodeID: "drainNode", - Name: "my-job.web[2]", - Job: oldJob, - DesiredTransition: structs.DesiredTransition{ - Migrate: pointer.Of(true), - }, - }, - // Mark the 4th lost - { - ID: uuid.Generate(), - NodeID: "dead", - Name: "my-job.web[3]", - Job: oldJob, - }, - } - - // Have three terminal allocs - terminal := structs.TerminalByNodeByName{ - "zip": map[string]*structs.Allocation{ - "my-job.web[4]": { - ID: uuid.Generate(), - NodeID: "zip", - Name: "my-job.web[4]", - Job: job, - }, - "my-job.web[5]": { - ID: uuid.Generate(), - NodeID: "zip", - Name: "my-job.web[5]", - Job: job, - }, - "my-job.web[6]": { - ID: uuid.Generate(), - NodeID: "zip", - Name: "my-job.web[6]", - Job: job, - }, - }, - } - - diff := diffSystemAllocsForNode(job, "zip", eligible, nil, tainted, required, allocs, terminal, true) - - // We should update the first alloc - require.Len(t, diff.update, 1) - require.Equal(t, allocs[0], diff.update[0].Alloc) - - // We should ignore the second alloc - require.Len(t, diff.ignore, 1) - require.Equal(t, allocs[1], diff.ignore[0].Alloc) - - // We should stop the 3rd alloc - require.Len(t, diff.stop, 1) - require.Equal(t, allocs[2], diff.stop[0].Alloc) - - // We should migrate the 4rd alloc - require.Len(t, diff.migrate, 1) - require.Equal(t, allocs[3], diff.migrate[0].Alloc) - - // We should mark the 5th alloc as lost - require.Len(t, diff.lost, 1) - require.Equal(t, allocs[4], diff.lost[0].Alloc) - - // We should place 6 - require.Len(t, diff.place, 6) - - // Ensure that the allocations which are replacements of terminal allocs are - // annotated. - for _, m := range terminal { - for _, alloc := range m { - for _, tuple := range diff.place { - if alloc.Name == tuple.Name { - require.Equal(t, alloc, tuple.Alloc) - } - } - } - } -} - -// Test the desired diff for an updated system job running on a -// ineligible node -func TestDiffSystemAllocsForNode_ExistingAllocIneligibleNode(t *testing.T) { +// TestDiffSystemAllocsForNode_Placements verifies we only place on nodes that +// need placements +func TestDiffSystemAllocsForNode_Placements(t *testing.T) { ci.Parallel(t) job := mock.SystemJob() required := materializeSystemTaskGroups(job) - // The "old" job has a previous modify index + goodNode := mock.Node() + unusedNode := mock.Node() + drainNode := mock.DrainNode() + deadNode := mock.Node() + deadNode.Status = structs.NodeStatusDown + + tainted := map[string]*structs.Node{ + deadNode.ID: deadNode, + drainNode.ID: drainNode, + } + + eligible := map[string]*structs.Node{ + goodNode.ID: goodNode, + } + + terminal := structs.TerminalByNodeByName{} + allocsForNode := []*structs.Allocation{} + + testCases := []struct { + name string + nodeID string + expected diffResultCount + }{ + { + name: "expect placement on good node", + nodeID: goodNode.ID, + expected: diffResultCount{place: 1}, + }, + { // "unused" here means outside of the eligible set + name: "expect no placement on unused node", + nodeID: unusedNode.ID, + expected: diffResultCount{}, + }, + { + name: "expect no placement on dead node", + nodeID: deadNode.ID, + expected: diffResultCount{}, + }, + { + name: "expect no placement on draining node", + nodeID: drainNode.ID, + expected: diffResultCount{}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + diff := diffSystemAllocsForNode( + job, tc.nodeID, eligible, nil, + tainted, required, allocsForNode, terminal, true) + + assertDiffCount(t, tc.expected, diff) + }) + } +} + +// TestDiffSystemAllocsForNodes_Stops verifies we stop allocs we no longer need +func TestDiffSystemAllocsForNode_Stops(t *testing.T) { + ci.Parallel(t) + + job := mock.SystemJob() + required := materializeSystemTaskGroups(job) + + // The "old" job has a previous modify index but is otherwise unchanged, so + // existing non-terminal allocs for this version should be updated in-place + + // TODO(tgross): *unless* there's another alloc for the same job already on + // the node. See https://github.com/hashicorp/nomad/pull/16097 oldJob := new(structs.Job) *oldJob = *job oldJob.JobModifyIndex -= 1 - eligibleNode := mock.Node() - ineligibleNode := mock.Node() - ineligibleNode.SchedulingEligibility = structs.NodeSchedulingIneligible - - tainted := map[string]*structs.Node{} + node := mock.Node() eligible := map[string]*structs.Node{ - eligibleNode.ID: eligibleNode, + node.ID: node, } allocs := []*structs.Allocation{ - // Update the TG alloc running on eligible node { + // extraneous alloc for old version of job should be updated + // TODO(tgross): this should actually be stopped. + // See https://github.com/hashicorp/nomad/pull/16097 ID: uuid.Generate(), - NodeID: eligibleNode.ID, + NodeID: node.ID, Name: "my-job.web[0]", Job: oldJob, }, - - // Ignore the TG alloc running on ineligible node - { + { // most recent alloc for current version of job should be ignored ID: uuid.Generate(), - NodeID: ineligibleNode.ID, + NodeID: node.ID, + Name: "my-job.web[0]", + Job: job, + }, + { // task group not required, should be stopped + ID: uuid.Generate(), + NodeID: node.ID, + Name: "my-job.something-else[0]", + Job: job, + }, + } + + tainted := map[string]*structs.Node{} + terminal := structs.TerminalByNodeByName{} + + diff := diffSystemAllocsForNode( + job, node.ID, eligible, nil, tainted, required, allocs, terminal, true) + + assertDiffCount(t, diffResultCount{ignore: 1, stop: 1, update: 1}, diff) + if len(diff.update) > 0 { + test.Eq(t, allocs[0], diff.update[0].Alloc) + } + if len(diff.ignore) > 0 { + test.Eq(t, allocs[1], diff.ignore[0].Alloc) + } + if len(diff.stop) > 0 { + test.Eq(t, allocs[2], diff.stop[0].Alloc) + } +} + +// Test the desired diff for an updated system job running on a ineligible node +func TestDiffSystemAllocsForNode_IneligibleNode(t *testing.T) { + ci.Parallel(t) + + job := mock.SystemJob() + required := materializeSystemTaskGroups(job) + + ineligibleNode := mock.Node() + ineligibleNode.SchedulingEligibility = structs.NodeSchedulingIneligible + ineligible := map[string]struct{}{ + ineligibleNode.ID: {}, + } + + eligible := map[string]*structs.Node{} + tainted := map[string]*structs.Node{} + + terminal := structs.TerminalByNodeByName{ + ineligibleNode.ID: map[string]*structs.Allocation{ + "my-job.web[0]": { // terminal allocs should not appear in diff + ID: uuid.Generate(), + NodeID: ineligibleNode.ID, + Name: "my-job.web[0]", + Job: job, + ClientStatus: structs.AllocClientStatusComplete, + }, + }, + } + + testCases := []struct { + name string + nodeID string + expect diffResultCount + }{ + { + name: "non-terminal alloc on ineligible node should be ignored", + nodeID: ineligibleNode.ID, + expect: diffResultCount{ignore: 1}, + }, + { + name: "non-terminal alloc on node not in eligible set should be stopped", + nodeID: uuid.Generate(), + expect: diffResultCount{stop: 1}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + alloc := &structs.Allocation{ + ID: uuid.Generate(), + NodeID: tc.nodeID, + Name: "my-job.web[0]", + Job: job, + } + + diff := diffSystemAllocsForNode( + job, tc.nodeID, eligible, ineligible, tainted, + required, []*structs.Allocation{alloc}, terminal, true, + ) + assertDiffCount(t, tc.expect, diff) + }) + } +} + +func TestDiffSystemAllocsForNode_DrainingNode(t *testing.T) { + ci.Parallel(t) + + job := mock.SystemJob() + required := materializeSystemTaskGroups(job) + + // The "old" job has a previous modify index but is otherwise unchanged, so + // existing non-terminal allocs for this version should be updated in-place + oldJob := job.Copy() + oldJob.JobModifyIndex -= 1 + + drainNode := mock.DrainNode() + tainted := map[string]*structs.Node{ + drainNode.ID: drainNode, + } + + // Terminal allocs don't get touched + terminal := structs.TerminalByNodeByName{ + drainNode.ID: map[string]*structs.Allocation{ + "my-job.web[0]": { + ID: uuid.Generate(), + NodeID: drainNode.ID, + Name: "my-job.web[0]", + Job: job, + ClientStatus: structs.AllocClientStatusComplete, + }, + }, + } + + allocs := []*structs.Allocation{ + { // allocs for draining node should be migrated + ID: uuid.Generate(), + NodeID: drainNode.ID, + Name: "my-job.web[0]", + Job: oldJob, + DesiredTransition: structs.DesiredTransition{ + Migrate: pointer.Of(true), + }, + }, + { // allocs not marked for drain should be ignored + ID: uuid.Generate(), + NodeID: drainNode.ID, Name: "my-job.web[0]", Job: job, }, } - // No terminal allocs - terminal := make(structs.TerminalByNodeByName) + diff := diffSystemAllocsForNode( + job, drainNode.ID, map[string]*structs.Node{}, nil, + tainted, required, allocs, terminal, true) - diff := diffSystemAllocsForNode(job, eligibleNode.ID, eligible, nil, tainted, required, allocs, terminal, true) + assertDiffCount(t, diffResultCount{migrate: 1, ignore: 1}, diff) + if len(diff.migrate) > 0 { + test.Eq(t, allocs[0], diff.migrate[0].Alloc) + } +} - require.Len(t, diff.place, 0) - require.Len(t, diff.update, 1) - require.Len(t, diff.migrate, 0) - require.Len(t, diff.stop, 0) - require.Len(t, diff.ignore, 1) - require.Len(t, diff.lost, 0) +func TestDiffSystemAllocsForNode_LostNode(t *testing.T) { + ci.Parallel(t) + + job := mock.SystemJob() + required := materializeSystemTaskGroups(job) + + // The "old" job has a previous modify index but is otherwise unchanged, so + // existing non-terminal allocs for this version should be updated in-place + oldJob := new(structs.Job) + *oldJob = *job + oldJob.JobModifyIndex -= 1 + + deadNode := mock.Node() + deadNode.Status = structs.NodeStatusDown + + tainted := map[string]*structs.Node{ + deadNode.ID: deadNode, + } + + allocs := []*structs.Allocation{ + { // current allocs on a lost node are lost, even if terminal + ID: uuid.Generate(), + NodeID: deadNode.ID, + Name: "my-job.web[0]", + Job: job, + }, + { // old allocs on a lost node are also lost + ID: uuid.Generate(), + NodeID: deadNode.ID, + Name: "my-job.web[0]", + Job: oldJob, + }, + } + + // Terminal allocs don't get touched + terminal := structs.TerminalByNodeByName{ + deadNode.ID: map[string]*structs.Allocation{ + "my-job.web[0]": allocs[0], + }, + } + + diff := diffSystemAllocsForNode( + job, deadNode.ID, map[string]*structs.Node{}, nil, + tainted, required, allocs, terminal, true) + + assertDiffCount(t, diffResultCount{lost: 2}, diff) + if len(diff.migrate) > 0 { + test.Eq(t, allocs[0], diff.migrate[0].Alloc) + } } func TestDiffSystemAllocsForNode_DisconnectedNode(t *testing.T) { @@ -295,10 +416,6 @@ func TestDiffSystemAllocsForNode_DisconnectedNode(t *testing.T) { required := materializeSystemTaskGroups(job) terminal := make(structs.TerminalByNodeByName) - type diffResultCount struct { - place, update, migrate, stop, ignore, lost, disconnecting, reconnecting int - } - testCases := []struct { name string node *structs.Node @@ -311,25 +428,20 @@ func TestDiffSystemAllocsForNode_DisconnectedNode(t *testing.T) { allocFn: func(alloc *structs.Allocation) { alloc.ClientStatus = structs.AllocClientStatusRunning }, - expect: diffResultCount{ - disconnecting: 1, - }, + expect: diffResultCount{disconnecting: 1}, }, { name: "disconnected alloc reconnects", node: readyNode, allocFn: func(alloc *structs.Allocation) { alloc.ClientStatus = structs.AllocClientStatusRunning - alloc.AllocStates = []*structs.AllocState{{ Field: structs.AllocStateFieldClientStatus, Value: structs.AllocClientStatusUnknown, Time: time.Now().Add(-time.Minute), }} }, - expect: diffResultCount{ - reconnecting: 1, - }, + expect: diffResultCount{reconnecting: 1}, }, { name: "alloc not reconnecting after it reconnects", @@ -350,41 +462,33 @@ func TestDiffSystemAllocsForNode_DisconnectedNode(t *testing.T) { }, } }, - expect: diffResultCount{ - ignore: 1, - }, + expect: diffResultCount{ignore: 1}, }, { name: "disconnected alloc is lost after it expires", node: disconnectedNode, allocFn: func(alloc *structs.Allocation) { alloc.ClientStatus = structs.AllocClientStatusUnknown - alloc.AllocStates = []*structs.AllocState{{ Field: structs.AllocStateFieldClientStatus, Value: structs.AllocClientStatusUnknown, Time: time.Now().Add(-10 * time.Hour), }} }, - expect: diffResultCount{ - lost: 1, - }, + expect: diffResultCount{lost: 1}, }, { name: "disconnected allocs are ignored", node: disconnectedNode, allocFn: func(alloc *structs.Allocation) { alloc.ClientStatus = structs.AllocClientStatusUnknown - alloc.AllocStates = []*structs.AllocState{{ Field: structs.AllocStateFieldClientStatus, Value: structs.AllocClientStatusUnknown, Time: time.Now(), }} }, - expect: diffResultCount{ - ignore: 1, - }, + expect: diffResultCount{ignore: 1}, }, } @@ -403,27 +507,26 @@ func TestDiffSystemAllocsForNode_DisconnectedNode(t *testing.T) { job, tc.node.ID, eligibleNodes, nil, taintedNodes, required, []*structs.Allocation{alloc}, terminal, true, ) - - assert.Len(t, got.place, tc.expect.place, "place") - assert.Len(t, got.update, tc.expect.update, "update") - assert.Len(t, got.migrate, tc.expect.migrate, "migrate") - assert.Len(t, got.stop, tc.expect.stop, "stop") - assert.Len(t, got.ignore, tc.expect.ignore, "ignore") - assert.Len(t, got.lost, tc.expect.lost, "lost") - assert.Len(t, got.disconnecting, tc.expect.disconnecting, "disconnecting") - assert.Len(t, got.reconnecting, tc.expect.reconnecting, "reconnecting") + assertDiffCount(t, tc.expect, got) }) } } +// TestDiffSystemAllocs is a higher-level test of interactions of diffs across +// multiple nodes. func TestDiffSystemAllocs(t *testing.T) { ci.Parallel(t) job := mock.SystemJob() + tg := job.TaskGroups[0].Copy() + tg.Name = "other" + job.TaskGroups = append(job.TaskGroups, tg) drainNode := mock.DrainNode() + drainNode.ID = "drain" deadNode := mock.Node() + deadNode.ID = "dead" deadNode.Status = structs.NodeStatusDown tainted := map[string]*structs.Node{ @@ -431,9 +534,9 @@ func TestDiffSystemAllocs(t *testing.T) { drainNode.ID: drainNode, } - // Create three alive nodes. + // Create four alive nodes. nodes := []*structs.Node{{ID: "foo"}, {ID: "bar"}, {ID: "baz"}, - {ID: "pipe"}, {ID: drainNode.ID}, {ID: deadNode.ID}} + {ID: "has-term"}, {ID: drainNode.ID}, {ID: deadNode.ID}} // The "old" job has a previous modify index oldJob := new(structs.Job) @@ -476,12 +579,12 @@ func TestDiffSystemAllocs(t *testing.T) { }, } - // Have three (?) terminal allocs + // Have one terminal allocs terminal := structs.TerminalByNodeByName{ - "pipe": map[string]*structs.Allocation{ + "has-term": map[string]*structs.Allocation{ "my-job.web[0]": { ID: uuid.Generate(), - NodeID: "pipe", + NodeID: "has-term", Name: "my-job.web[0]", Job: job, }, @@ -490,35 +593,29 @@ func TestDiffSystemAllocs(t *testing.T) { diff := diffSystemAllocs(job, nodes, nil, tainted, allocs, terminal, true) - // We should update the first alloc - require.Len(t, diff.update, 1) - require.Equal(t, allocs[0], diff.update[0].Alloc) + assertDiffCount(t, diffResultCount{ + update: 1, ignore: 1, migrate: 1, lost: 1, place: 6}, diff) - // We should ignore the second alloc - require.Len(t, diff.ignore, 1) - require.Equal(t, allocs[1], diff.ignore[0].Alloc) - - // We should stop the third alloc - require.Empty(t, diff.stop) - - // There should be no migrates. - require.Len(t, diff.migrate, 1) - require.Equal(t, allocs[2], diff.migrate[0].Alloc) - - // We should mark the 5th alloc as lost - require.Len(t, diff.lost, 1) - require.Equal(t, allocs[3], diff.lost[0].Alloc) - - // We should place 2 - require.Len(t, diff.place, 2) + if len(diff.update) > 0 { + must.Eq(t, allocs[0], diff.update[0].Alloc) // first alloc should be updated + } + if len(diff.ignore) > 0 { + must.Eq(t, allocs[1], diff.ignore[0].Alloc) // We should ignore the second alloc + } + if len(diff.migrate) > 0 { + must.Eq(t, allocs[2], diff.migrate[0].Alloc) + } + if len(diff.lost) > 0 { + must.Eq(t, allocs[3], diff.lost[0].Alloc) // We should mark the 5th alloc as lost + } // Ensure that the allocations which are replacements of terminal allocs are // annotated. for _, m := range terminal { for _, alloc := range m { for _, tuple := range diff.place { - if alloc.NodeID == tuple.Alloc.NodeID { - require.Equal(t, alloc, tuple.Alloc) + if alloc.NodeID == tuple.Alloc.NodeID && alloc.TaskGroup == "web" { + must.Eq(t, alloc, tuple.Alloc) } } } @@ -538,9 +635,12 @@ func TestEvictAndPlace_LimitLessThanAllocs(t *testing.T) { diff := &diffResult{} limit := 2 - require.True(t, evictAndPlace(ctx, diff, allocs, "", &limit), "evictAndReplace() should have returned true") - require.Zero(t, limit, "evictAndReplace() should decremented limit; got %v; want 0", limit) - require.Equal(t, 2, len(diff.place), "evictAndReplace() didn't insert into diffResult properly: %v", diff.place) + must.True(t, evictAndPlace(ctx, diff, allocs, "", &limit), + must.Sprintf("evictAndReplace() should have returned true")) + must.Zero(t, limit, + must.Sprint("evictAndReplace() should decrement limit")) + must.Len(t, 2, diff.place, + must.Sprintf("evictAndReplace() didn't insert into diffResult properly: %v", diff.place)) } func TestEvictAndPlace_LimitEqualToAllocs(t *testing.T) { @@ -556,9 +656,11 @@ func TestEvictAndPlace_LimitEqualToAllocs(t *testing.T) { diff := &diffResult{} limit := 4 - require.False(t, evictAndPlace(ctx, diff, allocs, "", &limit), "evictAndReplace() should have returned false") - require.Zero(t, limit, "evictAndReplace() should decremented limit; got %v; want 0", limit) - require.Equal(t, 4, len(diff.place), "evictAndReplace() didn't insert into diffResult properly: %v", diff.place) + must.False(t, evictAndPlace(ctx, diff, allocs, "", &limit), + must.Sprint("evictAndReplace() should have returned false")) + must.Zero(t, limit, must.Sprint("evictAndReplace() should decrement limit")) + must.Len(t, 4, diff.place, + must.Sprintf("evictAndReplace() didn't insert into diffResult properly: %v", diff.place)) } func TestEvictAndPlace_LimitGreaterThanAllocs(t *testing.T) { @@ -574,7 +676,7 @@ func TestEvictAndPlace_LimitGreaterThanAllocs(t *testing.T) { diff := &diffResult{} limit := 6 - require.False(t, evictAndPlace(ctx, diff, allocs, "", &limit)) - require.Equal(t, 2, limit, "evictAndReplace() should decremented limit") - require.Equal(t, 4, len(diff.place), "evictAndReplace() didn't insert into diffResult properly: %v", diff.place) + must.False(t, evictAndPlace(ctx, diff, allocs, "", &limit)) + must.Eq(t, 2, limit, must.Sprint("evictAndReplace() should decrement limit")) + must.Len(t, 4, diff.place, must.Sprintf("evictAndReplace() didn't insert into diffResult properly: %v", diff.place)) }