Skip to content

feat: allow removing deadline for running workspace #16085

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.

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 5 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 3 additions & 31 deletions coderd/autobuild/lifecycle_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -722,45 +722,17 @@ func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) {
err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTLMillis: nil})
require.NoError(t, err)

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

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

// Then: the workspace should stop
stats := <-statsCh
assert.Len(t, stats.Errors, 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.AwaitWorkspaceBuildJobCompleted(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 autostop
newTTL := 8 * time.Hour
err = client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTLMillis: ptr.Ref(newTTL.Milliseconds())})
require.NoError(t, err)

// Then: the deadline should remain at the zero value
updated = coderdtest.MustWorkspace(t, client, workspace.ID)
assert.Zero(t, updated.LatestBuild.Deadline)

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

// Then: the workspace should not stop
stats = <-statsCh
stats := <-statsCh
assert.Len(t, stats.Errors, 0)
assert.Len(t, stats.Transitions, 0)
}
Expand Down
20 changes: 20 additions & 0 deletions coderd/workspaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -1029,6 +1029,26 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) {
return xerrors.Errorf("update workspace time until shutdown: %w", err)
}

// If autostop has been disabled, we want to remove the deadline from the
// existing workspace build (if there is one).
if !dbTTL.Valid {
build, err := s.GetLatestWorkspaceBuildByWorkspaceID(ctx, workspace.ID)
if err != nil {
return xerrors.Errorf("get latest workspace build: %w", err)
}

if build.Transition == database.WorkspaceTransitionStart {
if err = s.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{
ID: build.ID,
Deadline: time.Time{},
MaxDeadline: build.MaxDeadline,
UpdatedAt: dbtime.Time(api.Clock.Now()),
}); err != nil {
return xerrors.Errorf("update workspace build deadline: %w", err)
}
}
}

return nil
}, nil)
if err != nil {
Expand Down
87 changes: 87 additions & 0 deletions coderd/workspaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2394,6 +2394,93 @@ func TestWorkspaceUpdateTTL(t *testing.T) {
})
}

t.Run("ModifyAutostopWithRunningWorkspace", func(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
fromTTL *int64
toTTL *int64
afterUpdate func(t *testing.T, before, after codersdk.NullTime)
}{
{
name: "RemoveAutostopRemovesDeadline",
fromTTL: ptr.Ref((8 * time.Hour).Milliseconds()),
toTTL: nil,
afterUpdate: func(t *testing.T, before, after codersdk.NullTime) {
require.NotZero(t, before)
require.Zero(t, after)
},
},
{
name: "AddAutostopDoesNotAddDeadline",
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps I'm thinking of this wrong, but it seems a bit weird that removing autostop removes a deadline, but adding it doesn't add one. Then again, deciding whether or not that deadline should be applied from workspace start, last activity or now is perhaps why it's not that way, oh well.. 😄

fromTTL: nil,
toTTL: ptr.Ref((8 * time.Hour).Milliseconds()),
afterUpdate: func(t *testing.T, before, after codersdk.NullTime) {
require.Zero(t, before)
require.Zero(t, after)
},
},
{
name: "IncreaseAutostopDoesNotModifyDeadline",
fromTTL: ptr.Ref((4 * time.Hour).Milliseconds()),
toTTL: ptr.Ref((8 * time.Hour).Milliseconds()),
afterUpdate: func(t *testing.T, before, after codersdk.NullTime) {
require.NotZero(t, before)
require.NotZero(t, after)
require.Equal(t, before, after)
},
},
{
name: "DecreaseAutostopDoesNotModifyDeadline",
fromTTL: ptr.Ref((8 * time.Hour).Milliseconds()),
toTTL: ptr.Ref((4 * time.Hour).Milliseconds()),
afterUpdate: func(t *testing.T, before, after codersdk.NullTime) {
require.NotZero(t, before)
require.NotZero(t, after)
require.Equal(t, before, after)
},
},
Comment on lines +2424 to +2443
Copy link
Member

Choose a reason for hiding this comment

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

praise, non-blocking: I feel like this requirement may change in future. These tests look very simple to modify in that case, so 👍 from me here

}

for _, testCase := range testCases {
testCase := testCase

t.Run(testCase.name, func(t *testing.T) {
t.Parallel()

var (
client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user = coderdtest.CreateFirstUser(t, client)
version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
template = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
workspace = coderdtest.CreateWorkspace(t, client, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.TTLMillis = testCase.fromTTL
})
build = coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
)

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()

err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{
TTLMillis: testCase.toTTL,
})
require.NoError(t, err)

deadlineBefore := build.Deadline

build, err = client.WorkspaceBuild(ctx, build.ID)
require.NoError(t, err)

deadlineAfter := build.Deadline

testCase.afterUpdate(t, deadlineBefore, deadlineAfter)
})
}
})

t.Run("CustomAutostopDisabledByTemplate", func(t *testing.T) {
t.Parallel()
var (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,10 @@ export const WorkspaceSchedulePage: FC = () => {

await submitScheduleMutation.mutateAsync(data);

if (data.autostopChanged) {
if (
data.autostopChanged &&
getAutostop(workspace).autostopEnabled
) {
Comment on lines +121 to +124
Copy link
Member

Choose a reason for hiding this comment

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

My gut tells me that this will need an update to WorkspaceSchedulePage.stories.tsx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll take a look 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like there aren't any existing stories covering hitting the "Save" button on the form.

setIsConfirmingApply(true);
}
}}
Expand Down
Loading