fix: allow setting workspace deadline as early as now plus 30 minutes#2328
fix: allow setting workspace deadline as early as now plus 30 minutes#2328
Conversation
| now := time.Now() | ||
| if new.Before(now) { | ||
| return xerrors.New("new deadline must be in the future") | ||
| } | ||
|
|
||
| delta := new.Sub(old) | ||
| if delta < time.Minute { | ||
| return xerrors.New("minimum extension is one minute") | ||
| } | ||
|
|
||
| if delta > 24*time.Hour { | ||
| return xerrors.New("maximum extension is 24 hours") |
There was a problem hiding this comment.
review: Dropping these validations for the moment till we have more information from #2318
There was a problem hiding this comment.
Actually, there's one very important validation involving templates that I may have overlooked! Adding this in now.
| // And with a deadline greater than the template max_ttl should also fail | ||
| deadlineExceedsMaxTTL := time.Now().Add(time.Duration(template.MaxTTLMillis) * time.Millisecond).Add(time.Minute) | ||
| err = client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{ | ||
| Deadline: deadlineExceedsMaxTTL, | ||
| }) | ||
| require.ErrorContains(t, err, "new deadline is greater than template allows", "setting a deadline greater than that allowed by the template should fail") |
There was a problem hiding this comment.
review: I forgot to add this in earlier. We don't folks folks sneaking around template-level restrictions this way.
| return xerrors.New("minimum extension is one minute") | ||
| // No idea how this could happen. | ||
| if newDeadline.Before(startedAt) { | ||
| return xerrors.Errorf("new deadline must be before workspace start time") |
There was a problem hiding this comment.
I figure if we get here, time is broken.
| if build.Deadline.IsZero() { | ||
| code = http.StatusConflict | ||
| resp.Message = "Workspace shutdown is manual." | ||
| return xerrors.Errorf("workspace shutdown is manual") | ||
| } |
There was a problem hiding this comment.
review: I'm not allowing this yet as if someone does, then there's currently no way to un-set this.
deansheather
left a comment
deansheather
left a comment
There was a problem hiding this comment.
LGTM except for "bump" doesn't make sense anymore
|
|
||
| return nil | ||
| }, | ||
| } |
There was a problem hiding this comment.
Also: this won't be called coder bump any more.
2 participants
This PR makes the following changes:
/api/v2/workspaces/:workspace/extendnow accepts any time at least 30 minutes in the futurecoder bumpcommand also allows the above. Some small copy changes to command.UI changes to come later.