-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: job canonicalization should set job priority to 50, not 0. #17314
Conversation
I see the problem in nomad/command/agent/job_endpoint_test.go Line 380 in 2aa3c74
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pkazmierczak this looks right at first in terms of the canonicalization but in the Job.Register
RPC we set the priority to the default priority (50) if the priority is 0. So this would break that and make it so that jobs with unset priority are set to 1 instead of 50.
ref https://github.com/hashicorp/nomad/blob/v1.5.5/nomad/job_endpoint.go#L866-L869
api/jobs.go
Outdated
@@ -1003,7 +1003,7 @@ func (j *Job) Canonicalize() { | |||
j.Namespace = pointerOf(DefaultNamespace) | |||
} | |||
if j.Priority == nil { | |||
j.Priority = pointerOf(0) | |||
j.Priority = pointerOf(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we claim the default is 50
- should that be the value set here?
https://developer.hashicorp.com/nomad/docs/job-specification/job#priority
edit: dangit, forgot to hit submit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Nomad API will reject jobs with priority set to 0.
Nomad API will reject jobs with priority set to 0.
Nomad API will reject jobs with priority set to 0.