Only error on constraints if no allocs are running #25850
Conversation
When running `nomad job run <JOB>` multiple times with constraints defined, there should be no error as a result of filtering out nodes that do not/have not ever satsified the constraints. When running a systems job with constraint, any run after an initial startup returns an exit(2) and a warning about unplaced allocations due to constraints. An error that is not encountered on the initial run, though the constraint stays the same. This is because the node that satisfies the condition is already running the allocation, and the placement is ignored. Another placement is attempted, but the only node(s) left are the ones that do not satisfy the constraint. Nomad views this case (no allocations that were attempted to placed could be placed successfully) as an error, and reports it as such. In reality, no allocations should be placed or updated in this case, but it should not be treated as an error. This change uses the `ignored` placements from diffSystemAlloc to attempt to determine if the case encountered is an error (no ignored placements means that nothing is already running, and is an error), or is not one (an ignored placement means that the task is already running somewhere on a node). It does this at the point where `failedTGAlloc` is populated, so placement functionality isn't changed, just the field that populates error. There is functionality that should be preserved which (correctly) notifies a user if a job is attempted that cannot be run on any node due to the constraints filtering out all available nodes. This should still behave as expected.
allisonlarson
force-pushed
the
b-system-job-constraints
branch
from
c97c260 to
6af511c
Compare
allisonlarson
added
backport/ent/1.8.x+ent
| // Ensure `groupA` fails to be placed due to its constraint, but `groupB` doesn't | ||
| require.Len(t, h.Evals[2].FailedTGAllocs, 1) | ||
| require.Contains(t, h.Evals[2].FailedTGAllocs, "groupA") | ||
| require.NotContains(t, h.Evals[2].FailedTGAllocs, "groupB") |
There was a problem hiding this comment.
If we're only suppressing the error in the case where a specific task group has an alloc, shouldn't these assertions and the ones in scheduler/scheduler_sysbatch_test.go still work? Or am I misunderstand why we're removing these? (totally a possibility! 😁 )
There was a problem hiding this comment.
I think these assertions are a bit of a misdirect to what is actually being tested. The test is testing that a node can be added an existing node pool where there are allocations running, and the node is correctly evaluated in the context of the defined constraints and the new node only get allocs that match the constraint. Since the allocs are already running in this case, the new behavior says that it shouldn't mark any of them as failed.
Theres an assertion later in the test that the allocations are only running on the nodes that they are expected to be running on, which seems like what the desired behavior should do.
|
|
||
| // Test that the system scheduler can handle a job with a constraint on | ||
| // subsequent runs, and report the outcome appropriately | ||
| func TestSystemSched_JobConstraint_RunMultipleTimes(t *testing.T) { |
Co-authored-by: Piotr Kazmierczak <[email protected]>
Description
When running a system job with a constraint, any run after an initial startup returns an exit(2) and a warning about unplaced allocations due to constraints. An error that is not encountered on the initial run, though the constraint stays the same. This is because the node that satisfies the condition is already running the allocation, and the placement is ignored. Another placement is attempted, but the only node(s) left are the ones that do not satisfy the constraint. Nomad views this case (no allocations that were attempted to placed could be placed successfully) as an error, and reports it as such. In reality, no allocations should be placed or updated in this case, but it should not be treated as an error.
This change uses the
ignored& in-placeupdatedplacements from diffSystemAlloc to attempt to determine if the case encountered is an error (no ignored/in-place updates placements means that nothing is already running, and is an error), or is not one (an ignored placement means that the task is already running somewhere on a node). It does this at the point wherefailedTGAllocis populated, so placement functionality isn't changed, just the field that populates the error.There is functionality that should be preserved which (correctly) notifies a user if a job is attempted that cannot be run on any node due to the constraints filtering out all available nodes. This should still behave as expected, and an explicit test has been added for it.
Testing & Reproduction steps
Define a system jobspec with a constraint on a node in the node pool, and run it. Once an allocation is running on an available node, run (or plan) the job again. In the below example, there are 3 nodes and the constraint on the job is defined as
Previous behavior (on second run):
Reports a failure to place an allocation due to the constraint filtering out the node
New behavior (on second run):
Links
Fixes #12748 #12016 #19413 #12366
Contributor Checklist
changelog entry using the
make clcommand.ensure regressions will be caught.
and job configuration, please update the Nomad website documentation to reflect this. Refer to
the website README for docs guidelines. Please also consider whether the
change requires notes within the upgrade guide.
Reviewer Checklist
backporting document.
in the majority of situations. The main exceptions are long-lived feature branches or merges where
history should be preserved.
within the public repository.