fix: allow setting workspace deadline as early as now plus 30 minutes (coder#2328)

johnstcn · web-flow · commit 02ad60fd758b · 2022-06-14T22:39:15.000+01:00
This PR makes the following changes:

- coderd: /api/v2/workspaces/:workspace/extend now accepts any time at least 30 minutes in the future.
- coder bump command also allows the above. Some small copy changes to command.
- coder bump now actually enforces template-level maxima.
diff --git a/cli/bump.go b/cli/bump.go
@@ -8,19 +8,26 @@ import (
 	"github.com/spf13/cobra"
 	"golang.org/x/xerrors"
 
+	"github.com/coder/coder/coderd/util/tz"
 	"github.com/coder/coder/codersdk"
 )
 
 const (
-	bumpDescriptionLong = `To extend the autostop deadline for a workspace.`
+	bumpDescriptionShort = `Shut your workspace down after a given duration has passed.`
+	bumpDescriptionLong  = `Modify the time at which your workspace will shut down automatically.
+* Provide a duration from now (for example, 1h30m).
+* The minimum duration is 30 minutes.
+* If the workspace template restricts the maximum runtime of a workspace, this will be enforced here.
+* If the workspace does not already have a shutdown scheduled, this does nothing.
+`
 )
 
 func bump() *cobra.Command {
 	bumpCmd := &cobra.Command{
 		Args:        cobra.RangeArgs(1, 2),
 		Annotations: workspaceCommand,
-		Use:         "bump <workspace-name> <duration>",
-		Short:       "Extend the autostop deadline for a workspace.",
+		Use:         "bump <workspace-name> <duration from now>",
+		Short:       bumpDescriptionShort,
 		Long:        bumpDescriptionLong,
 		Example:     "coder bump my-workspace 90m",
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -39,17 +46,20 @@ func bump() *cobra.Command {
 				return xerrors.Errorf("get workspace: %w", err)
 			}
 
-			newDeadline := time.Now().Add(bumpDuration)
+			loc, err := tz.TimezoneIANA()
+			if err != nil {
+				loc = time.UTC // best effort
+			}
 
-			if newDeadline.Before(workspace.LatestBuild.Deadline) {
+			if bumpDuration < 29*time.Minute {
 				_, _ = fmt.Fprintf(
 					cmd.OutOrStdout(),
-					"The proposed deadline is %s before the current deadline.\n",
-					workspace.LatestBuild.Deadline.Sub(newDeadline).Round(time.Minute),
+					"Please specify a duration of at least 30 minutes.\n",
 				)
 				return nil
 			}
 
+			newDeadline := time.Now().In(loc).Add(bumpDuration)
 			if err := client.PutExtendWorkspace(cmd.Context(), workspace.ID, codersdk.PutExtendWorkspaceRequest{
 				Deadline: newDeadline,
 			}); err != nil {
@@ -62,7 +72,6 @@ func bump() *cobra.Command {
 				newDeadline.Format(timeFormat),
 				newDeadline.Format(dateFormat),
 			)
-
 			return nil
 		},
 	}
diff --git a/cli/bump_test.go b/cli/bump_test.go
@@ -124,8 +124,8 @@ func TestBump(t *testing.T) {
 		workspace, err = client.Workspace(ctx, workspace.ID)
 		require.NoError(t, err)
 
-		// 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
+		// NOTE(cian): need to stop and start the workspace as we do not update the deadline
+		//             see: https://github.com/coder/coder/issues/2224
 		coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
 		coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStop, database.WorkspaceTransitionStart)
 
diff --git a/coderd/workspaces.go b/coderd/workspaces.go
@@ -575,21 +575,47 @@ func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) {
 	resp := httpapi.Response{}
 
 	err := api.Database.InTx(func(s database.Store) error {
+		template, err := s.GetTemplateByID(r.Context(), workspace.TemplateID)
+		if err != nil {
+			code = http.StatusInternalServerError
+			resp.Message = "Error fetching workspace template!"
+			return xerrors.Errorf("get workspace template: %w", err)
+		}
+
 		build, err := s.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID)
 		if err != nil {
 			code = http.StatusInternalServerError
-			resp.Message = "Workspace not found."
+			resp.Message = "Error fetching workspace build."
 			return xerrors.Errorf("get latest workspace build: %w", err)
 		}
 
+		job, err := s.GetProvisionerJobByID(r.Context(), build.JobID)
+		if err != nil {
+			code = http.StatusInternalServerError
+			resp.Message = "Error fetching workspace provisioner job."
+			return xerrors.Errorf("get provisioner job: %w", err)
+		}
+
 		if build.Transition != database.WorkspaceTransitionStart {
 			code = http.StatusConflict
 			resp.Message = "Workspace must be started, current status: " + string(build.Transition)
 			return xerrors.Errorf("workspace must be started, current status: %s", build.Transition)
 		}
 
+		if !job.CompletedAt.Valid {
+			code = http.StatusConflict
+			resp.Message = "Workspace is still building!"
+			return xerrors.Errorf("workspace is still building")
+		}
+
+		if build.Deadline.IsZero() {
+			code = http.StatusConflict
+			resp.Message = "Workspace shutdown is manual."
+			return xerrors.Errorf("workspace shutdown is manual")
+		}
+
 		newDeadline := req.Deadline.UTC()
-		if err := validWorkspaceDeadline(build.Deadline, newDeadline); err != nil {
+		if err := validWorkspaceDeadline(job.CompletedAt.Time, newDeadline, time.Duration(template.MaxTtl)); err != nil {
 			code = http.StatusBadRequest
 			resp.Message = "Bad extend workspace request."
 			resp.Validations = append(resp.Validations, httpapi.Error{Field: "deadline", Detail: err.Error()})
@@ -878,23 +904,20 @@ func validWorkspaceTTLMillis(millis *int64, max time.Duration) (sql.NullInt64, e
 	}, nil
 }
 
-func validWorkspaceDeadline(old, new time.Time) error {
-	if old.IsZero() {
-		return xerrors.New("nothing to do: no existing deadline set")
-	}
-
-	now := time.Now()
-	if new.Before(now) {
-		return xerrors.New("new deadline must be in the future")
+func validWorkspaceDeadline(startedAt, newDeadline time.Time, max time.Duration) error {
+	soon := time.Now().Add(29 * time.Minute)
+	if newDeadline.Before(soon) {
+		return xerrors.New("new deadline must be at least 30 minutes in the future")
 	}
 
-	delta := new.Sub(old)
-	if delta < time.Minute {
-		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")
 	}
 
-	if delta > 24*time.Hour {
-		return xerrors.New("maximum extension is 24 hours")
+	delta := newDeadline.Sub(startedAt)
+	if delta > max {
+		return xerrors.New("new deadline is greater than template allows")
 	}
 
 	return nil
diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go
@@ -974,22 +974,23 @@ func TestWorkspaceUpdateTTL(t *testing.T) {
 func TestWorkspaceExtend(t *testing.T) {
 	t.Parallel()
 	var (
+		ttl         = 8 * time.Hour
+		newDeadline = time.Now().Add(ttl + time.Hour).UTC()
 		ctx         = context.Background()
 		client      = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
 		user        = coderdtest.CreateFirstUser(t, client)
 		version     = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
 		_           = coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
-		project     = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
-		workspace   = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID)
-		extend      = 90 * time.Minute
-		_           = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
-		oldDeadline = time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond).UTC()
-		newDeadline = time.Now().Add(time.Duration(*workspace.TTLMillis)*time.Millisecond + extend).UTC()
+		template    = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
+		workspace   = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
+			cwr.TTLMillis = ptr.Ref(ttl.Milliseconds())
+		})
+		_ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
 	)
 
 	workspace, err := client.Workspace(ctx, workspace.ID)
 	require.NoError(t, err, "fetch provisioned workspace")
-	require.InDelta(t, oldDeadline.Unix(), workspace.LatestBuild.Deadline.Unix(), 60)
+	oldDeadline := workspace.LatestBuild.Deadline
 
 	// Updating the deadline should succeed
 	req := codersdk.PutExtendWorkspaceRequest{
@@ -1001,30 +1002,45 @@ func TestWorkspaceExtend(t *testing.T) {
 	// Ensure deadline set correctly
 	updated, err := client.Workspace(ctx, workspace.ID)
 	require.NoError(t, err, "failed to fetch updated workspace")
-	require.InDelta(t, newDeadline.Unix(), updated.LatestBuild.Deadline.Unix(), 60)
+	require.WithinDuration(t, newDeadline, updated.LatestBuild.Deadline, time.Minute)
 
 	// Zero time should fail
 	err = client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{
 		Deadline: time.Time{},
 	})
 	require.ErrorContains(t, err, "deadline: Validation failed for tag \"required\" with value: \"0001-01-01 00:00:00 +0000 UTC\"", "setting an empty deadline on a workspace should fail")
 
-	// Updating with an earlier time should also fail
+	// Updating with a deadline 29 minutes in the future should fail
+	deadlineTooSoon := time.Now().Add(29 * time.Minute)
+	err = client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{
+		Deadline: deadlineTooSoon,
+	})
+	require.ErrorContains(t, err, "new deadline must be at least 30 minutes in the future", "setting a deadline less than 30 minutes in the future should fail")
+
+	// 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")
+
+	// Updating with a deadline 30 minutes in the future should succeed
+	deadlineJustSoonEnough := time.Now().Add(30 * time.Minute)
 	err = client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{
-		Deadline: oldDeadline,
+		Deadline: deadlineJustSoonEnough,
 	})
-	require.ErrorContains(t, err, "deadline: minimum extension is one minute", "setting an earlier deadline should fail")
+	require.NoError(t, err, "setting a deadline at least 30 minutes in the future should succeed")
 
-	// Updating with a time far in the future should also fail
+	// Updating with a deadline an hour before the previous deadline should succeed
 	err = client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{
-		Deadline: oldDeadline.AddDate(1, 0, 0),
+		Deadline: oldDeadline.Add(-time.Hour),
 	})
-	require.ErrorContains(t, err, "deadline: maximum extension is 24 hours", "setting an earlier deadline should fail")
+	require.NoError(t, err, "setting an earlier deadline should not fail")
 
 	// Ensure deadline still set correctly
 	updated, err = client.Workspace(ctx, workspace.ID)
 	require.NoError(t, err, "failed to fetch updated workspace")
-	require.InDelta(t, newDeadline.Unix(), updated.LatestBuild.Deadline.Unix(), 60)
+	require.WithinDuration(t, oldDeadline.Add(-time.Hour), updated.LatestBuild.Deadline, time.Minute)
 }
 
 func TestWorkspaceWatcher(t *testing.T) {
