scheduler: basic cluster reconciler safety properties for batch jobs

[tgross]

Property test assertions for the core safety properties of the cluster reconciler, for batch jobs.

Ref: https://hashicorp.atlassian.net/browse/NMD-814
Ref: #26167

[tgross]

Test case that discovered this:

• group count 1
• alloc with desired=evict and client=lost (alloc was lost and missing heartbeat)
• alloc with desired=run and client=failed
• we get stop=2, place=2 with both placements being for the failed alloc

This patch is a bit of a sledgehammer on the test but seems like an obvious thing to assert as well. TBD

[tgross]

The following test shows that this is a problem only in the reconciler and not the scheduler overall:

func TestServiceSched_PlacementOvercount(t *testing.T) {
	ci.Parallel(t)

	h := tests.NewHarness(t)

	lostNode := mock.Node()
	lostNode.Status = structs.NodeStatusDown
	must.NoError(t, h.State.UpsertNode(structs.MsgTypeTestSetup, h.NextIndex(), lostNode))

	node := mock.Node()
	must.NoError(t, h.State.UpsertNode(structs.MsgTypeTestSetup, h.NextIndex(), node))

	job := mock.Job()
	job.TaskGroups[0].Count = 1
	must.NoError(t, h.State.UpsertJob(structs.MsgTypeTestSetup, h.NextIndex(), nil, job))

	lostAlloc := mock.AllocForNode(lostNode)
	lostAlloc.Job = job
	lostAlloc.JobID = job.ID
	lostAlloc.Name = "my-job.web[0]"
	lostAlloc.DesiredStatus = structs.AllocDesiredStatusEvict
	lostAlloc.ClientStatus = structs.AllocClientStatusLost

	failedAlloc := mock.AllocForNode(node)
	failedAlloc.Job = job
	failedAlloc.JobID = job.ID
	failedAlloc.Name = "my-job.web[0]"
	failedAlloc.DesiredStatus = structs.AllocDesiredStatusRun
	failedAlloc.ClientStatus = structs.AllocClientStatusFailed

	allocs := []*structs.Allocation{lostAlloc, failedAlloc}
	must.NoError(t, h.State.UpsertAllocs(structs.MsgTypeTestSetup, h.NextIndex(), allocs))

	eval := &structs.Evaluation{
		Namespace:    structs.DefaultNamespace,
		ID:           uuid.Generate(),
		Priority:     job.Priority,
		TriggeredBy:  structs.EvalTriggerAllocStop,
		JobID:        job.ID,
		Status:       structs.EvalStatusPending,
		AnnotatePlan: true,
	}

	must.NoError(t, h.State.UpsertEvals(
		structs.MsgTypeTestSetup, h.NextIndex(), []*structs.Evaluation{eval}))

	err := h.Process(NewServiceScheduler, eval)
	must.NoError(t, err)
	must.Len(t, 1, h.Plans)
	must.Eq(t, 1, h.Plans[0].Annotations.DesiredTGUpdates["web"].Place)
	must.Eq(t, 1, h.Plans[0].Annotations.DesiredTGUpdates["web"].Stop)
}

However, the check we're doing here isn't correct either, because result.Place is the results from all task groups. We need to check for the specific task group and only remove result.Place for other task groups. I'll pull this out to its own PR.

[tgross]

However, the check we're doing here isn't correct either, because result.Place is the results from all task groups.

Whoops, this isn't true either! This is computeGroup. It'll get merged later with the other group.

[tgross]

#26239

[pkazmierczak]

makes me wonder: shouldn't these also hold for service jobs?

[tgross]

Yeah I assume there's going to be a whole lot of overlap between the properties we want to test in our two PRs. Once we've got a set we're mostly-happy with we can refactor the test to pull out the common set.

[pkazmierczak]

LGTM!