Skip to content

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

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.

Sign up for GitHub

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 3 commits into from
Jun 14, 2022
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c6df467
fix: allow setting workspace deadline as early as now plus 30 minutes
johnstcn Jun 14, 2022
308b348
copy changes
johnstcn Jun 14, 2022
1708c21
address PR comments, actually enforce template level limits
johnstcn Jun 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix: allow setting workspace deadline as early as now plus 30 minutes
  • Loading branch information
@johnstcn
johnstcn committed Jun 14, 2022
commit c6df467c7cce8900fc3c405ca74322828bd9bb08
20 changes: 12 additions & 8 deletions cli/bump.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,24 @@ import (
"strings"
"time"

"github.com/coder/coder/coderd/util/tz"

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

"github.com/coder/coder/codersdk"
)

const (
bumpDescriptionLong = `To extend the autostop deadline for a workspace.`
bumpDescriptionLong = `Make your workspace stop at a certain point in the future.`
)

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: "Make your workspace stop at a certain point in the future.",
Long: bumpDescriptionLong,
Example: "coder bump my-workspace 90m",
RunE: func(cmd *cobra.Command, args []string) error {
Expand All @@ -39,17 +41,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 guess
}

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 {
Expand All @@ -62,7 +67,6 @@ func bump() *cobra.Command {
newDeadline.Format(timeFormat),
newDeadline.Format(dateFormat),
)

return nil
},
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Also: this won't be called coder bump any more.

Expand Down
4 changes: 2 additions & 2 deletions cli/bump_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
15 changes: 3 additions & 12 deletions coderd/workspaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -883,18 +883,9 @@ func validWorkspaceDeadline(old, new time.Time) error {
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")
}

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")
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: Dropping these validations for the moment till we have more information from #2318

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, there's one very important validation involving templates that I may have overlooked! Adding this in now.

soon := time.Now().Add(29 * time.Minute)
if new.Before(soon) {
return xerrors.New("new deadline must be at least 30 minutes in the future")
}

return nil
Expand Down
37 changes: 23 additions & 14 deletions coderd/workspaces_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.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{
Expand All @@ -1001,30 +1002,38 @@ 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")

// 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) {
Expand Down