-
Notifications
You must be signed in to change notification settings - Fork 923
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
Changes from all commits
46f6f9c
5ed74c0
7a9e872
1698b13
d787abb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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", | ||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My gut tells me that this will need an update to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll take a look 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
} | ||
}} | ||
|
There was a problem hiding this comment.
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.. 😄