rpc: set the deregistration eval priority to the job priority.

Previously when creating an eval for job deregistration, the eval
priority was set to the default value irregardless of the job
priority. In situations where an operator would want to deregister
a high priority job so they could re-register; the evaluation may
get blocked for some time on a busy cluster because of the
deregsiter priority.

If a job had a lower than default priority and was deregistered,
the deregister eval would get a priority higher than that of the
job. If we attempted to register another job with a higher
priority than this, but still below the default, the deregister
would be actioned before the register.

Both situations described above seem incorrect and unexpected from
a user prespective.

This fix modifies to behaviour to set the deregister eval priority
to that of the job, if available. Otherwise the default value is
still used.
This commit is contained in:
James Rasell
2021-11-02 09:11:44 +01:00
parent 94dec4baf8
commit e25c8a6aa7
2 changed files with 65 additions and 1 deletions

View File

@@ -829,12 +829,26 @@ func (j *Job) Deregister(args *structs.JobDeregisterRequest, reply *structs.JobD
// priority even if the job was.
now := time.Now().UnixNano()
// Set our default priority initially, but update this to that configured
// within the job if possible. It is reasonable from a user perspective
// that jobs with a higher priority have their deregister evaluated before
// those of a lower priority.
//
// Alternatively, the previous behaviour was to set the eval priority to
// the default value. Jobs with a lower than default register priority
// would therefore have their deregister eval priorities higher than
// expected.
priority := structs.JobDefaultPriority
if job != nil {
priority = job.Priority
}
// If the job is periodic or parameterized, we don't create an eval.
if job == nil || !(job.IsPeriodic() || job.IsParameterized()) {
eval = &structs.Evaluation{
ID: uuid.Generate(),
Namespace: args.RequestNamespace(),
Priority: structs.JobDefaultPriority,
Priority: priority,
Type: structs.JobTypeService,
TriggeredBy: structs.EvalTriggerJobDeregister,
JobID: args.JobID,

View File

@@ -3825,6 +3825,56 @@ func TestJobEndpoint_BatchDeregister_ACL(t *testing.T) {
require.NotEqual(validResp2.Index, 0)
}
func TestJobEndpoint_Deregister_Priority(t *testing.T) {
t.Parallel()
requireAssertion := require.New(t)
s1, cleanupS1 := TestServer(t, func(c *Config) {
c.NumSchedulers = 0 // Prevent automatic dequeue
})
defer cleanupS1()
codec := rpcClient(t, s1)
testutil.WaitForLeader(t, s1.RPC)
fsmState := s1.fsm.State()
// Create a job which a custom priority and register this.
job := mock.Job()
job.Priority = 90
err := fsmState.UpsertJob(structs.MsgTypeTestSetup, 100, job)
requireAssertion.Nil(err)
// Deregister.
dereg := &structs.JobDeregisterRequest{
JobID: job.ID,
Purge: true,
WriteRequest: structs.WriteRequest{
Region: "global",
Namespace: job.Namespace,
},
}
var resp structs.JobDeregisterResponse
requireAssertion.Nil(msgpackrpc.CallWithCodec(codec, "Job.Deregister", dereg, &resp))
requireAssertion.NotZero(resp.Index)
// Check for the job in the FSM which should not be there as it was purged.
out, err := fsmState.JobByID(nil, job.Namespace, job.ID)
requireAssertion.Nil(err)
requireAssertion.Nil(out)
// Lookup the evaluation
eval, err := fsmState.EvalByID(nil, resp.EvalID)
requireAssertion.Nil(err)
requireAssertion.NotNil(eval)
requireAssertion.EqualValues(resp.EvalCreateIndex, eval.CreateIndex)
requireAssertion.Equal(job.Priority, eval.Priority)
requireAssertion.Equal(job.Type, eval.Type)
requireAssertion.Equal(structs.EvalTriggerJobDeregister, eval.TriggeredBy)
requireAssertion.Equal(job.ID, eval.JobID)
requireAssertion.Equal(structs.EvalStatusPending, eval.Status)
requireAssertion.NotZero(eval.CreateTime)
requireAssertion.NotZero(eval.ModifyTime)
}
func TestJobEndpoint_GetJob(t *testing.T) {
t.Parallel()