-
Notifications
You must be signed in to change notification settings - Fork 885
fix: do not set max deadline for workspaces on template update #12446
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
Conversation
When templates are updated and schedule data is changed, we update all running workspaces to have up-to-date scheduling information that sticks to the new policy. When updating the max_deadline for existing running workspaces, if the max_deadline was before now()+2h we would set the max_deadline to now()+2h. Builds that don't/shouldn't have a max_deadline have it set to 0, which is always before now()+2h, and thus would always have the max_deadline updated.
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.
Looks good but I'm going to wait for tests from @Emyrk before approving 👍
added a user without quiet hours and a template without a autostop requirment to trigger the bug.
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've added unit tests that fail on main
reproducing the bug.
In practice (I have not manually confirmed), I think these are the steps to reproduce.
- Template autostart requirements must be disabled. This if condition needs to be falsy
- You need to have a workspace that has a deadline beyond 2hrs in the future using said template.
- Update the template schedule (use max ttl should be off)
- Workspace deadline should now be 2hrs after the template
updated_at
Test:
coder/enterprise/coderd/schedule/template_test.go
Lines 135 to 154 in 295b755
{ | |
// There was a bug if a user has no quiet hours set, and autostop | |
// req is not turned on, then the max deadline is set to `time.Time{}`. | |
// This zero value was "in the past", so the workspace deadline would | |
// be set to "now" + 2 hours. | |
// This is a mistake because the max deadline being zero means | |
// there is no max deadline. | |
name: "MaxDeadlineShouldBeUnset", | |
now: buildTime, | |
deadline: buildTime.Add(time.Hour * 8), | |
maxDeadline: time.Time{}, // No max set | |
// Should be unchanged | |
newDeadline: ptr.Ref(buildTime.Add(time.Hour * 8)), | |
newMaxDeadline: time.Time{}, | |
noQuietHours: true, | |
autostopReq: &agplschedule.TemplateAutostopRequirement{ | |
DaysOfWeek: 0, | |
Weeks: 0, | |
}, | |
}, |
When templates are updated and schedule data is changed, we update all running workspaces to have up-to-date scheduling information that sticks to the new policy.
When updating the
max_deadline
for existing running workspaces, if themax_deadline
was beforenow()+2h
we would set themax_deadline
tonow()+2h
.Builds that don't/shouldn't have a
max_deadline
have it set to0
, which is always beforenow()+2h
, and thus would always have themax_deadline
updated.Closes https://github.com/coder/customers/issues/530