From 46f6f9cda7a72d79bebefdf074782f99aeb9f895 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 8 Jan 2025 11:15:56 +0000 Subject: [PATCH 1/5] chore: remove deadline from running workspaces on ttl change --- coderd/workspaces.go | 19 +++++++++++++++++++ .../WorkspaceSchedulePage.tsx | 5 ++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 19fb1ec1ce810..2cff588d55f2b 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -1029,6 +1029,25 @@ 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, + }); err != nil { + return xerrors.Errorf("update workspace build deadline: %w", err) + } + } + } + return nil }, nil) if err != nil { diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx index b5e9bfdba8da6..4ee96204dbdd5 100644 --- a/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx @@ -118,7 +118,10 @@ export const WorkspaceSchedulePage: FC = () => { await submitScheduleMutation.mutateAsync(data); - if (data.autostopChanged) { + if ( + data.autostopChanged && + getAutostop(workspace).autostopEnabled + ) { setIsConfirmingApply(true); } }} From 5ed74c02beb85dd2cb64f40949b36d8d7500fd7f Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Thu, 9 Jan 2025 15:30:10 +0000 Subject: [PATCH 2/5] test: add tests --- coderd/workspaces_test.go | 83 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index d6e365011b929..1633ad2515313 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -2394,6 +2394,89 @@ func TestWorkspaceUpdateTTL(t *testing.T) { }) } + t.Run("ModifyAutostopWithRunningWorkspace", func(t *testing.T) { + 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", + 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) + }, + }, + } + + for _, testCase := range testCases { + testCase := testCase + + t.Run(testCase.name, func(t *testing.T) { + 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 ( From 7a9e872ddc9efdcd3fa40489b591da9b44a5604f Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 10 Jan 2025 14:51:40 +0000 Subject: [PATCH 3/5] test: remove test for old behaviour --- coderd/autobuild/lifecycle_executor_test.go | 63 --------------------- 1 file changed, 63 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 3eb779376cc5c..e47388974f688 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -702,69 +702,6 @@ func TestExecuteAutostopSuspendedUser(t *testing.T) { assert.Equal(t, codersdk.WorkspaceStatusStopped, workspaceBuild.Status) } -func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) { - t.Parallel() - - var ( - ctx = context.Background() - tickCh = make(chan time.Time) - statsCh = make(chan autobuild.Stats) - client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerDaemon: true, - AutobuildStats: statsCh, - }) - // Given: we have a user with a workspace - workspace = mustProvisionWorkspace(t, client) - ) - - // Given: the user changes their mind and decides their workspace should not autostop - err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTLMillis: nil}) - require.NoError(t, err) - - // Then: the deadline should still be the original value - updated := coderdtest.MustWorkspace(t, client, workspace.ID) - assert.WithinDuration(t, workspace.LatestBuild.Deadline.Time, updated.LatestBuild.Deadline.Time, time.Minute) - - // 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 - assert.Len(t, stats.Errors, 0) - assert.Len(t, stats.Transitions, 0) -} - func TestExecutorAutostartMultipleOK(t *testing.T) { if os.Getenv("DB") == "" { t.Skip(`This test only really works when using a "real" database, similar to a HA setup`) From 1698b13fd131d9317e28e413f7f9a48007559d9d Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 13 Jan 2025 11:22:08 +0000 Subject: [PATCH 4/5] chore: fix lint issues --- coderd/workspaces.go | 1 + coderd/workspaces_test.go | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 2cff588d55f2b..0e7a4b5972dfd 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -1042,6 +1042,7 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) { 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) } diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 1633ad2515313..115549e28cc2b 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -2395,6 +2395,8 @@ func TestWorkspaceUpdateTTL(t *testing.T) { } t.Run("ModifyAutostopWithRunningWorkspace", func(t *testing.T) { + t.Parallel() + testCases := []struct { name string fromTTL *int64 @@ -2445,6 +2447,8 @@ func TestWorkspaceUpdateTTL(t *testing.T) { 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) From d787abb5e81de91eec03dbd4f2b77c82e0399427 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Mon, 13 Jan 2025 12:21:58 +0000 Subject: [PATCH 5/5] chore: re-add old test but refactored --- coderd/autobuild/lifecycle_executor_test.go | 35 +++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index e47388974f688..b271fc43d1267 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -702,6 +702,41 @@ func TestExecuteAutostopSuspendedUser(t *testing.T) { assert.Equal(t, codersdk.WorkspaceStatusStopped, workspaceBuild.Status) } +func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) { + t.Parallel() + + var ( + ctx = context.Background() + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerDaemon: true, + AutobuildStats: statsCh, + }) + // Given: we have a user with a workspace + workspace = mustProvisionWorkspace(t, client) + ) + + // Given: the user changes their mind and decides their workspace should not autostop + err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTLMillis: nil}) + require.NoError(t, err) + + // Then: the deadline should be set to zero + updated := coderdtest.MustWorkspace(t, client, workspace.ID) + 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 not stop + stats := <-statsCh + assert.Len(t, stats.Errors, 0) + assert.Len(t, stats.Transitions, 0) +} + func TestExecutorAutostartMultipleOK(t *testing.T) { if os.Getenv("DB") == "" { t.Skip(`This test only really works when using a "real" database, similar to a HA setup`)