Skip to content

Commit 73dedcc

Browse files
johnstcnjohnstcn
authored
fix: delete related task when deleting workspace (#20567)
* Instead of prompting the user to start a deleted workspace (which is silly), prompt them to create a new task instead. * Adds a warning dialog when deleting a workspace * Updates provisionerdserver to delete the related task if a workspace is related to a task
1 parent 94f6e83 commit 73dedcc

7 files changed

Lines changed: 118 additions & 12 deletions

File tree

‎coderd/aitasks_test.go‎

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -355,10 +355,10 @@ func TestTasks(t *testing.T) {
355355
}
356356
})
357357

358-
t.Run("NoWorkspace", func(t *testing.T) {
358+
t.Run("DeletedWorkspace", func(t *testing.T) {
359359
t.Parallel()
360360

361-
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
361+
client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
362362
user := coderdtest.CreateFirstUser(t, client)
363363
template := createAITemplate(t, client, user)
364364
ctx := testutil.Context(t, testutil.WaitLong)
@@ -372,14 +372,54 @@ func TestTasks(t *testing.T) {
372372
ws, err := client.Workspace(ctx, task.WorkspaceID.UUID)
373373
require.NoError(t, err)
374374
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID)
375-
// Delete the task workspace
376-
coderdtest.MustTransitionWorkspace(t, client, ws.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionDelete)
377-
// We should still be able to fetch the task after deleting its workspace
375+
376+
// Mark the workspace as deleted directly in the database, bypassing provisionerd.
377+
require.NoError(t, db.UpdateWorkspaceDeletedByID(dbauthz.AsProvisionerd(ctx), database.UpdateWorkspaceDeletedByIDParams{
378+
ID: ws.ID,
379+
Deleted: true,
380+
}))
381+
// We should still be able to fetch the task if its workspace was deleted.
382+
// Provisionerdserver will attempt delete the related task when deleting a workspace.
383+
// This test ensures that we can still handle the case where, for some reason, the
384+
// task has not been marked as deleted, but the workspace has.
378385
task, err = exp.TaskByID(ctx, task.ID)
379-
require.NoError(t, err, "fetching a task should still work after deleting its related workspace")
386+
require.NoError(t, err, "fetching a task should still work if its related workspace is deleted")
380387
err = exp.DeleteTask(ctx, task.OwnerID.String(), task.ID)
381388
require.NoError(t, err, "should be possible to delete a task with no workspace")
382389
})
390+
391+
t.Run("DeletingTaskWorkspaceDeletesTask", func(t *testing.T) {
392+
t.Parallel()
393+
394+
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
395+
user := coderdtest.CreateFirstUser(t, client)
396+
template := createAITemplate(t, client, user)
397+
398+
ctx := testutil.Context(t, testutil.WaitLong)
399+
400+
exp := codersdk.NewExperimentalClient(client)
401+
task, err := exp.CreateTask(ctx, "me", codersdk.CreateTaskRequest{
402+
TemplateVersionID: template.ActiveVersionID,
403+
Input: "delete me",
404+
})
405+
require.NoError(t, err)
406+
require.True(t, task.WorkspaceID.Valid, "task should have a workspace ID")
407+
ws, err := client.Workspace(ctx, task.WorkspaceID.UUID)
408+
require.NoError(t, err)
409+
if assert.True(t, ws.TaskID.Valid, "task id should be set on workspace") {
410+
assert.Equal(t, task.ID, ws.TaskID.UUID, "workspace task id should match")
411+
}
412+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID)
413+
414+
// When; the task workspace is deleted
415+
coderdtest.MustTransitionWorkspace(t, client, ws.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionDelete)
416+
// Then: the task associated with the workspace is also deleted
417+
_, err = exp.TaskByID(ctx, task.ID)
418+
require.Error(t, err, "expected an error fetching the task")
419+
var sdkErr *codersdk.Error
420+
require.ErrorAs(t, err, &sdkErr, "expected a codersdk.Error")
421+
require.Equal(t, http.StatusNotFound, sdkErr.StatusCode())
422+
})
383423
})
384424

385425
t.Run("Send", func(t *testing.T) {

‎coderd/database/dbauthz/dbauthz.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,8 @@ var (
219219
rbac.ResourceUser.Type: {policy.ActionRead, policy.ActionReadPersonal, policy.ActionUpdatePersonal},
220220
rbac.ResourceWorkspaceDormant.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStop},
221221
rbac.ResourceWorkspace.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop, policy.ActionCreateAgent},
222-
// Provisionerd needs to read and update tasks associated with workspaces.
223-
rbac.ResourceTask.Type: {policy.ActionRead, policy.ActionUpdate},
222+
// Provisionerd needs to read, update, and delete tasks associated with workspaces.
223+
rbac.ResourceTask.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
224224
rbac.ResourceApiKey.Type: {policy.WildcardSymbol},
225225
// When org scoped provisioner credentials are implemented,
226226
// this can be reduced to read a specific org.

‎coderd/provisionerdserver/provisionerdserver.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2278,6 +2278,14 @@ func (s *server) completeWorkspaceBuildJob(ctx context.Context, job database.Pro
22782278
if err != nil {
22792279
return xerrors.Errorf("update workspace deleted: %w", err)
22802280
}
2281+
if workspace.TaskID.Valid {
2282+
if _, err := db.DeleteTask(ctx, database.DeleteTaskParams{
2283+
ID: workspace.TaskID.UUID,
2284+
DeletedAt: dbtime.Now(),
2285+
}); err != nil && !errors.Is(err, sql.ErrNoRows) {
2286+
return xerrors.Errorf("delete task related to workspace: %w", err)
2287+
}
2288+
}
22812289

22822290
return nil
22832291
}, nil)

‎site/src/modules/workspaces/WorkspaceMoreActions/WorkspaceDeleteDialog.stories.tsx‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { MockFailedWorkspace, MockWorkspace } from "testHelpers/entities";
1+
import {
2+
MockFailedWorkspace,
3+
MockTaskWorkspace,
4+
MockWorkspace,
5+
} from "testHelpers/entities";
26
import type { Meta, StoryObj } from "@storybook/react-vite";
37
import { daysAgo } from "utils/time";
48
import { WorkspaceDeleteDialog } from "./WorkspaceDeleteDialog";
@@ -45,3 +49,9 @@ export const UnhealthyAdminView: Story = {
4549
canDeleteFailedWorkspace: true,
4650
},
4751
};
52+
53+
export const WithTask: Story = {
54+
args: {
55+
workspace: MockTaskWorkspace,
56+
},
57+
};

‎site/src/modules/workspaces/WorkspaceMoreActions/WorkspaceDeleteDialog.tsx‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ export const WorkspaceDeleteDialog: FC<WorkspaceDeleteDialogProps> = ({
5656
(workspace.latest_build.status === "failed" ||
5757
workspace.latest_build.status === "canceled");
5858

59+
const hasTask = !!workspace.task_id;
60+
5961
return (
6062
<ConfirmDialog
6163
type="delete"
@@ -109,8 +111,24 @@ export const WorkspaceDeleteDialog: FC<WorkspaceDeleteDialogProps> = ({
109111
"data-testid": "delete-dialog-name-confirmation",
110112
}}
111113
/>
114+
{hasTask && (
115+
<div css={styles.warnContainer}>
116+
<div css={{ flexDirection: "column" }}>
117+
<p className="info">This workspace is related to a task</p>
118+
<span css={{ fontSize: 12, marginTop: 4, display: "block" }}>
119+
Deleting this workspace will also delete{" "}
120+
<Link
121+
href={`/tasks/${workspace.owner_name}/${workspace.task_id}`}
122+
>
123+
this task
124+
</Link>
125+
.
126+
</span>
127+
</div>
128+
</div>
129+
)}
112130
{canOrphan && (
113-
<div css={styles.orphanContainer}>
131+
<div css={styles.warnContainer}>
114132
<div css={{ flexDirection: "column" }}>
115133
<Checkbox
116134
id="orphan_resources"
@@ -178,7 +196,7 @@ const styles = {
178196
color: theme.palette.text.primary,
179197
},
180198
}),
181-
orphanContainer: (theme) => ({
199+
warnContainer: (theme) => ({
182200
marginTop: 24,
183201
display: "flex",
184202
backgroundColor: theme.roles.danger.background,

‎site/src/pages/TaskPage/TaskPage.stories.tsx‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
MockDeletedWorkspace,
23
MockFailedWorkspace,
34
MockStartingWorkspace,
45
MockStoppedWorkspace,
@@ -169,6 +170,15 @@ export const TerminatedBuildWithStatus: Story = {
169170
},
170171
};
171172

173+
export const DeletedWorkspace: Story = {
174+
beforeEach: () => {
175+
spyOn(API.experimental, "getTask").mockResolvedValue(MockTask);
176+
spyOn(API, "getWorkspaceByOwnerAndName").mockResolvedValue(
177+
MockDeletedWorkspace,
178+
);
179+
},
180+
};
181+
172182
export const WaitingStartupScripts: Story = {
173183
beforeEach: () => {
174184
spyOn(API.experimental, "getTask").mockResolvedValue(MockTask);

‎site/src/pages/TaskPage/TaskPage.tsx‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,27 @@ const WorkspaceNotRunning: FC<WorkspaceNotRunningProps> = ({ workspace }) => {
221221
? mutateStartWorkspace.error
222222
: undefined;
223223

224-
return (
224+
const deleted = workspace.latest_build?.transition === ("delete" as const);
225+
226+
return deleted ? (
227+
<Margins>
228+
<div className="w-full min-h-80 flex items-center justify-center">
229+
<div className="flex flex-col items-center">
230+
<h3 className="m-0 font-medium text-content-primary text-base">
231+
Task workspace was deleted.
232+
</h3>
233+
<span className="text-content-secondary text-sm">
234+
This task cannot be resumed. Delete this task and create a new one.
235+
</span>
236+
<Button size="sm" variant="outline" asChild className="mt-4">
237+
<RouterLink to="/tasks" data-testid="task-create-new">
238+
Create a new task
239+
</RouterLink>
240+
</Button>
241+
</div>
242+
</div>
243+
</Margins>
244+
) : (
225245
<Margins>
226246
<div className="w-full min-h-80 flex items-center justify-center">
227247
<div className="flex flex-col items-center">

0 commit comments

Comments
 (0)