-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
scheduler: Fix bug where the would treat multiregion jobs as paused for job types that don't use deployments #14659
Conversation
…or job types that don't use deployments
switch j.Type { | ||
case JobTypeService: | ||
return true | ||
default: | ||
return false | ||
} |
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.
Could this not be simplified?
switch j.Type { | |
case JobTypeService: | |
return true | |
default: | |
return false | |
} | |
return j.Type == JobTypeService |
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.
It could, but I intentionally did this since a goal is to support Deployments for more job types in the future. Maybe this is premature? I was hoping the team would discuss so thanks for taking the bait!
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
Co-authored-by: Tim Gross <tgross@hashicorp.com>
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This PR fixes a bug in the
allocReconciler
where multiregion job types that don't support deployments would be flagged as paused.structs.Job
- Added aUsesDeployments
function that can be expanded as more job types support deploymentsscheduler.allocReconciler
- Added check tocomputeDeploymentPaused
to guard against settingdeploymentPaused = true
if the job type doesn't use deployments.