feat: allow bumping workspace deadline #1828
Conversation
…d of from workspace TTL
4b868d2 to
7a5c873
Compare
34ed95f to
4b868d2
Compare
| if duration > time.Hour { | ||
| duration = duration.Truncate(time.Hour) | ||
| } | ||
| if duration > time.Minute { | ||
| duration = duration.Truncate(time.Minute) | ||
| } | ||
| days := 0 | ||
| for duration.Hours() > 24 { | ||
| days++ | ||
| duration -= 24 * time.Hour | ||
| } | ||
| durationDisplay := duration.String() | ||
| if days > 0 { | ||
| durationDisplay = fmt.Sprintf("%dd%s", days, durationDisplay) | ||
| } | ||
| if strings.HasSuffix(durationDisplay, "m0s") { | ||
| durationDisplay = durationDisplay[:len(durationDisplay)-2] | ||
| } | ||
| if strings.HasSuffix(durationDisplay, "h0m") { | ||
| durationDisplay = durationDisplay[:len(durationDisplay)-2] | ||
| } | ||
|
|
There was a problem hiding this comment.
review: moved to its own func
| `Your Coder workspace %s is scheduled to stop at %s.`, | ||
| ws.Name, | ||
| deadline.Format(time.Kitchen), | ||
| ) | ||
| `Your Coder workspace %s is scheduled to stop in %.0f mins`, ws.Name, ttl.Minutes()) |
There was a problem hiding this comment.
review: using a relative time here makes more sense, plus it also means we don't need to worry about timezones!
| // For stopping, do not truncate. This is inconsistent with autostart, but | ||
| // it ensures we will not stop too early. | ||
| nextTransition = priorHistory.Deadline |
There was a problem hiding this comment.
review: I decided to stop truncating this as the consistency didn't seem worth losing the fidelity in other places. In the worst case, we'll stop a little bit later (but not too early).
coderd/workspaces.go
Outdated
| // Disallow updates within than one minute | ||
| if withinDuration(newDeadline, build.Deadline, time.Minute) { |
There was a problem hiding this comment.
review: it's an edge case, but it doesn't make sense to allow this.
| if dt < -d || dt > d { | ||
| return false | ||
| } |
There was a problem hiding this comment.
review: I just lifted this from require.WithinDuration.
| Long: bumpDescriptionLong, | ||
| Example: "coder bump my-workspace 90m", | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| bumpDuration := defaultBumpDuration |
There was a problem hiding this comment.
If we have a default, should it just be a flag? --duration=90m or something?
Cobra supports duration strings
There was a problem hiding this comment.
I don't like the way that looks, without the flag is less typing!
| return d, nil | ||
| } | ||
|
|
||
| func isDigit(s string) bool { |
There was a problem hiding this comment.
This would also return true for the empty string, "", although that shouldn't matter in the current implementation (parse error).
| @@ -597,7 +597,7 @@ func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) { | |||
| return xerrors.Errorf("workspace must be started, current status: %s", build.Transition) | |||
| } | |||
|
|
|||
| newDeadline := req.Deadline.Truncate(time.Minute).UTC() | |||
| newDeadline := req.Deadline.UTC() | |||
There was a problem hiding this comment.
Very nice getting rid of the truncates! 💪🏻
Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
|
nit: Update title to be |
* Adds a `bump` command to extend workspace build deadline
* Reduces WARN-level logging spam from autobuild executor
* Modifies `cli/ssh` notifications to read from workspace build deadline and to notify relative time instead (sidestepping the problem of figuring out a user's timezone across multiple OSes)
* Shows workspace extension time in `coder list` output e.g.
```
WORKSPACE TEMPLATE STATUS LAST BUILT OUTDATED AUTOSTART TTL
developer/test1 docker Running 4m false 0 9 * * MON-FRI 15m (+5m)
```
bumpcommand to extend workspace build deadlinecli/sshnotifications to read from workspace build deadline and to notify relative time instead (sidestepping the problem of figuring out a user's timezone across multiple OSes)coder listoutput e.g.