Skip to content

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

Merged
merged 7 commits into from
Mar 7, 2024

Conversation

deansheather
Copy link
Member

@deansheather deansheather commented Mar 6, 2024

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.

Closes https://github.com/coder/customers/issues/530

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.
Copy link
Collaborator

@sreya sreya left a 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 👍

Copy link
Member

@Emyrk Emyrk left a 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.

  1. Template autostart requirements must be disabled. This if condition needs to be falsy
  2. You need to have a workspace that has a deadline beyond 2hrs in the future using said template.
  3. Update the template schedule (use max ttl should be off)
  4. Workspace deadline should now be 2hrs after the template updated_at

Test:

{
// 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,
},
},

@Emyrk Emyrk merged commit 586586e into main Mar 7, 2024
@Emyrk Emyrk deleted the dean/template-bug-fix branch March 7, 2024 15:42
@github-actions github-actions bot locked and limited conversation to collaborators Mar 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants