Skip to content
5 changes: 3 additions & 2 deletions cli/bump.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@ func bump() *cobra.Command {

_, _ = fmt.Fprintf(
cmd.OutOrStdout(),
"Workspace %q will now stop at %s\n", workspace.Name,
newDeadline.Format(time.RFC822),
"Workspace %q will now stop at %s on %s\n", workspace.Name,
newDeadline.Format(timeFormat),
newDeadline.Format(dateFormat),
)

return nil
Expand Down
6 changes: 6 additions & 0 deletions cli/constants.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package cli

const (
timeFormat = "3:04:05 PM MST"
dateFormat = "Jan 2, 2006"
)
5 changes: 4 additions & 1 deletion cli/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,17 @@ func hasExtension(ws codersdk.Workspace) (bool, time.Duration) {
if ws.LatestBuild.Transition != codersdk.WorkspaceTransitionStart {
return false, 0
}
if ws.LatestBuild.Job.CompletedAt == nil {
return false, 0
}
if ws.LatestBuild.Deadline.IsZero() {
return false, 0
}
if ws.TTLMillis == nil {
return false, 0
}
ttl := time.Duration(*ws.TTLMillis) * time.Millisecond
delta := ws.LatestBuild.Deadline.Add(-ttl).Sub(ws.LatestBuild.CreatedAt)
delta := ws.LatestBuild.Deadline.Add(-ttl).Sub(*ws.LatestBuild.Job.CompletedAt)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review: this was causing us to measure deadline extensions incorrectly, resulting in things like 8h (+3m) if your workspace took 3 minutes to buidl.

if delta < time.Minute {
return false, 0
}
Expand Down
62 changes: 21 additions & 41 deletions cli/ttl.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package cli

import (
"errors"
"fmt"
"time"

"github.com/spf13/cobra"
"golang.org/x/xerrors"

"github.com/coder/coder/cli/cliui"
"github.com/coder/coder/coderd/autobuild/schedule"
"github.com/coder/coder/coderd/util/ptr"
"github.com/coder/coder/codersdk"
)

Expand Down Expand Up @@ -91,37 +91,32 @@ func ttlset() *cobra.Command {
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "warning: ttl rounded down to %s\n", truncated)
}

if changed, newDeadline := changedNewDeadline(workspace, truncated); changed {
// For the purposes of the user, "less than a minute" is essentially the same as "immediately".
timeRemaining := time.Until(newDeadline).Truncate(time.Minute)
humanRemaining := "in " + timeRemaining.String()
if timeRemaining <= 0 {
humanRemaining = "immediately"
}
_, err = cliui.Prompt(cmd, cliui.PromptOptions{
Text: fmt.Sprintf(
"Workspace %q will be stopped %s. Are you sure?",
workspace.Name,
humanRemaining,
),
Default: "yes",
IsConfirm: true,
})
if err != nil {
if errors.Is(err, cliui.Canceled) {
return nil
}
return err
}
}

millis := truncated.Milliseconds()
if err = client.UpdateWorkspaceTTL(cmd.Context(), workspace.ID, codersdk.UpdateWorkspaceTTLRequest{
TTLMillis: &millis,
}); err != nil {
return xerrors.Errorf("update workspace ttl: %w", err)
}

if ptr.NilOrEmpty(workspace.AutostartSchedule) {
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%q will shut down %s after start.\n", workspace.Name, truncated)
return nil
}

sched, err := schedule.Weekly(*workspace.AutostartSchedule)
if err != nil {
return xerrors.Errorf("parse workspace schedule: %w", err)
}

nextShutdown := sched.Next(time.Now()).Add(truncated).In(sched.Location())
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%q will shut down at %s on %s (%s after start).\n",
workspace.Name,
nextShutdown.Format(timeFormat),
nextShutdown.Format(dateFormat),
truncated,
)

_, _ = fmt.Fprintf(cmd.OutOrStdout(), "NOTE: this will only take effect the next time the workspace is started.\n")
return nil
},
}
Expand Down Expand Up @@ -157,18 +152,3 @@ func ttlunset() *cobra.Command {
},
}
}

func changedNewDeadline(ws codersdk.Workspace, newTTL time.Duration) (changed bool, newDeadline time.Time) {
if ws.LatestBuild.Transition != codersdk.WorkspaceTransitionStart {
// not running
return false, newDeadline
}

if ws.LatestBuild.Job.CompletedAt == nil {
// still building
return false, newDeadline
}

newDeadline = ws.LatestBuild.Job.CompletedAt.Add(newTTL)
return true, newDeadline
}
4 changes: 0 additions & 4 deletions cli/ttl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package cli_test
import (
"bytes"
"context"
"fmt"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -109,9 +108,6 @@ func TestTTL(t *testing.T) {
assert.NoError(t, err, "unexpected error")
}()

pty.ExpectMatch(fmt.Sprintf("warning: ttl rounded down to %s", ttl.Truncate(time.Minute)))
pty.ExpectMatch(fmt.Sprintf("Workspace %q will be stopped in 8h29m0s. Are you sure?", workspace.Name))
pty.WriteLine("yes")
// Ensure ttl updated
updated, err := client.Workspace(ctx, workspace.ID)
require.NoError(t, err, "fetch updated workspace")
Expand Down
8 changes: 3 additions & 5 deletions coderd/autobuild/executor/lifecycle_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,9 @@ func (e *Executor) runOnce(t time.Time) Stats {
// is what we compare against when performing autostop operations, rounded down
// to the minute.
//
// NOTE: Currently, if a workspace build is created with a given TTL and then
// the user either changes or unsets the TTL, the deadline for the workspace
// build will not have changed. So, autostop will still happen at the
// original TTL value from when the workspace build was created.
// Whether this is expected behavior from a user's perspective is not yet known.
// NOTE: If a workspace build is created with a given TTL and then the user either
// changes or unsets the TTL, the deadline for the workspace build will not
// have changed. This behavior is as expected per #2229.
Comment on lines +88 to +90
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review: this is now the expected behaviour until we're told otherwise!

eligibleWorkspaces, err := db.GetWorkspacesAutostart(e.ctx)
if err != nil {
return xerrors.Errorf("get eligible workspaces for autostart or autostop: %w", err)
Expand Down
29 changes: 17 additions & 12 deletions coderd/autobuild/executor/lifecycle_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,7 @@ func TestExecutorAutostopNotEnabled(t *testing.T) {
require.NoError(t, err)
require.Nil(t, workspace.TTLMillis)

// TODO(cian): need to stop and start the workspace as we do not update the deadline yet
// see: https://github.com/coder/coder/issues/1783
// TODO(cian): need to stop and start the workspace as we do not update the deadline. See: #2229
coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStop, database.WorkspaceTransitionStart)

Expand Down Expand Up @@ -440,41 +439,47 @@ func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) {
err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTLMillis: nil})
require.NoError(t, err)

// Then: the deadline should be the zero value
// Then: the deadline should still be the original value
updated := coderdtest.MustWorkspace(t, client, workspace.ID)
assert.Zero(t, updated.LatestBuild.Deadline)
assert.WithinDuration(t, workspace.LatestBuild.Deadline, updated.LatestBuild.Deadline, time.Minute)

// When: the autobuild executor ticks after the original deadline
go func() {
tickCh <- workspace.LatestBuild.Deadline.Add(time.Minute)
}()

// Then: the workspace should not stop
// Then: the workspace should stop
stats := <-statsCh
assert.NoError(t, stats.Error)
assert.Len(t, stats.Transitions, 0)
assert.Len(t, stats.Transitions, 1)
assert.Equal(t, stats.Transitions[workspace.ID], database.WorkspaceTransitionStop)

// Wait for stop to complete
updated = coderdtest.MustWorkspace(t, client, workspace.ID)
_ = coderdtest.AwaitWorkspaceBuildJob(t, client, updated.LatestBuild.ID)

// Start the workspace again
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStop, database.WorkspaceTransitionStart)

// Given: the user changes their mind again and wants to enable auto-stop
newTTL := 8 * time.Hour
expectedDeadline := workspace.LatestBuild.UpdatedAt.Add(newTTL)
err = client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTLMillis: ptr.Ref(newTTL.Milliseconds())})
require.NoError(t, err)

// Then: the deadline should be updated based on the TTL
// Then: the deadline should remain at the zero value
updated = coderdtest.MustWorkspace(t, client, workspace.ID)
assert.WithinDuration(t, expectedDeadline, updated.LatestBuild.Deadline, time.Minute)
assert.Zero(t, updated.LatestBuild.Deadline)

// When: the relentless onward march of time continues
go func() {
tickCh <- workspace.LatestBuild.Deadline.Add(newTTL + time.Minute)
close(tickCh)
}()

// Then: the workspace should stop
// Then: the workspace should not stop
stats = <-statsCh
assert.NoError(t, stats.Error)
assert.Len(t, stats.Transitions, 1)
assert.Equal(t, stats.Transitions[workspace.ID], database.WorkspaceTransitionStop)
assert.Len(t, stats.Transitions, 0)
}

func TestExecutorAutostartMultipleOK(t *testing.T) {
Expand Down
91 changes: 33 additions & 58 deletions coderd/workspaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import (
"github.com/coder/coder/codersdk"
)

const workspaceDefaultTTL = 12 * time.Hour

func (api *API) workspace(rw http.ResponseWriter, r *http.Request) {
workspace := httpmw.WorkspaceParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, workspace) {
Expand Down Expand Up @@ -289,8 +291,8 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
}

if !dbTTL.Valid {
// Default to template maximum when creating a new workspace
dbTTL = sql.NullInt64{Valid: true, Int64: template.MaxTtl}
// Default to min(12 hours, template maximum). Just defaulting to template maximum can be surprising.
dbTTL = sql.NullInt64{Valid: true, Int64: min(template.MaxTtl, int64(workspaceDefaultTTL))}
}

workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(r.Context(), database.GetWorkspaceByOwnerIDAndNameParams{
Expand Down Expand Up @@ -511,75 +513,41 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) {
return
}

template, err := api.Database.GetTemplateByID(r.Context(), workspace.TemplateID)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Error fetching workspace template!",
})
return
}
var validErrs []httpapi.Error

dbTTL, err := validWorkspaceTTLMillis(req.TTLMillis, time.Duration(template.MaxTtl))
if err != nil {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: "Invalid workspace TTL.",
Detail: err.Error(),
Validations: []httpapi.Error{
{
Field: "ttl_ms",
Detail: err.Error(),
},
},
})
return
}
err := api.Database.InTx(func(s database.Store) error {
template, err := s.GetTemplateByID(r.Context(), workspace.TemplateID)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Error fetching workspace template!",
})
return xerrors.Errorf("fetch workspace template: %w", err)
}

err = api.Database.InTx(func(s database.Store) error {
dbTTL, err := validWorkspaceTTLMillis(req.TTLMillis, time.Duration(template.MaxTtl))
if err != nil {
validErrs = append(validErrs, httpapi.Error{Field: "ttl_ms", Detail: err.Error()})
return err
}
if err := s.UpdateWorkspaceTTL(r.Context(), database.UpdateWorkspaceTTLParams{
ID: workspace.ID,
Ttl: dbTTL,
}); err != nil {
return xerrors.Errorf("update workspace TTL: %w", err)
}

// Also extend the workspace deadline if the workspace is running
latestBuild, err := s.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID)
if err != nil {
return xerrors.Errorf("get latest workspace build: %w", err)
}

if latestBuild.Transition != database.WorkspaceTransitionStart {
return nil // nothing to do
}

if latestBuild.UpdatedAt.IsZero() {
// Build in progress; provisionerd should update with the new TTL.
return nil
}

var newDeadline time.Time
if dbTTL.Valid {
newDeadline = latestBuild.UpdatedAt.Add(time.Duration(dbTTL.Int64))
}

if err := s.UpdateWorkspaceBuildByID(
r.Context(),
database.UpdateWorkspaceBuildByIDParams{
ID: latestBuild.ID,
UpdatedAt: latestBuild.UpdatedAt,
ProvisionerState: latestBuild.ProvisionerState,
Deadline: newDeadline,
},
); err != nil {
return xerrors.Errorf("update workspace deadline: %w", err)
}
return nil
})

if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Error updating workspace time until shutdown!",
Detail: err.Error(),
code := http.StatusInternalServerError
if len(validErrs) > 0 {
code = http.StatusBadRequest
}
httpapi.Write(rw, code, httpapi.Response{
Message: "Error updating workspace time until shutdown!",
Validations: validErrs,
Detail: err.Error(),
})
return
}
Expand Down Expand Up @@ -1024,3 +992,10 @@ func splitQueryParameterByDelimiter(query string, delimiter rune, maintainQuotes

return parts
}

func min(x, y int64) int64 {
if x < y {
return x
}
return y
}
Loading