Skip to content

Commit 563612e

Browse files
authored
fix: delete related task when deleting workspace (coder#20567) (coder#20585)
* 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 related to a task * Updates provisionerdserver to delete the related task if a workspace is related to a task (cherry picked from commit 73dedcc)
1 parent fa43ea8 commit 563612e

File tree

7 files changed

+118
-12
lines changed

7 files changed

+118
-12
lines changed

coderd/aitasks_test.go

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -478,10 +478,10 @@ func TestTasks(t *testing.T) {
478478
}
479479
})
480480

481-
t.Run("NoWorkspace", func(t *testing.T) {
481+
t.Run("DeletedWorkspace", func(t *testing.T) {
482482
t.Parallel()
483483

484-
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
484+
client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
485485
user := coderdtest.CreateFirstUser(t, client)
486486
template := createAITemplate(t, client, user)
487487
ctx := testutil.Context(t, testutil.WaitLong)
@@ -495,14 +495,54 @@ func TestTasks(t *testing.T) {
495495
ws, err := client.Workspace(ctx, task.WorkspaceID.UUID)
496496
require.NoError(t, err)
497497
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID)
498-
// Delete the task workspace
499-
coderdtest.MustTransitionWorkspace(t, client, ws.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionDelete)
500-
// We should still be able to fetch the task after deleting its workspace
498+
499+
// Mark the workspace as deleted directly in the database, bypassing provisionerd.
500+
require.NoError(t, db.UpdateWorkspaceDeletedByID(dbauthz.AsProvisionerd(ctx), database.UpdateWorkspaceDeletedByIDParams{
501+
ID: ws.ID,
502+
Deleted: true,
503+
}))
504+
// We should still be able to fetch the task if its workspace was deleted.
505+
// Provisionerdserver will attempt delete the related task when deleting a workspace.
506+
// This test ensures that we can still handle the case where, for some reason, the
507+
// task has not been marked as deleted, but the workspace has.
501508
task, err = exp.TaskByID(ctx, task.ID)
502-
require.NoError(t, err, "fetching a task should still work after deleting its related workspace")
509+
require.NoError(t, err, "fetching a task should still work if its related workspace is deleted")
503510
err = exp.DeleteTask(ctx, task.OwnerID.String(), task.ID)
504511
require.NoError(t, err, "should be possible to delete a task with no workspace")
505512
})
513+
514+
t.Run("DeletingTaskWorkspaceDeletesTask", func(t *testing.T) {
515+
t.Parallel()
516+
517+
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
518+
user := coderdtest.CreateFirstUser(t, client)
519+
template := createAITemplate(t, client, user)
520+
521+
ctx := testutil.Context(t, testutil.WaitLong)
522+
523+
exp := codersdk.NewExperimentalClient(client)
524+
task, err := exp.CreateTask(ctx, "me", codersdk.CreateTaskRequest{
525+
TemplateVersionID: template.ActiveVersionID,
526+
Input: "delete me",
527+
})
528+
require.NoError(t, err)
529+
require.True(t, task.WorkspaceID.Valid, "task should have a workspace ID")
530+
ws, err := client.Workspace(ctx, task.WorkspaceID.UUID)
531+
require.NoError(t, err)
532+
if assert.True(t, ws.TaskID.Valid, "task id should be set on workspace") {
533+
assert.Equal(t, task.ID, ws.TaskID.UUID, "workspace task id should match")
534+
}
535+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID)
536+
537+
// When; the task workspace is deleted
538+
coderdtest.MustTransitionWorkspace(t, client, ws.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionDelete)
539+
// Then: the task associated with the workspace is also deleted
540+
_, err = exp.TaskByID(ctx, task.ID)
541+
require.Error(t, err, "expected an error fetching the task")
542+
var sdkErr *codersdk.Error
543+
require.ErrorAs(t, err, &sdkErr, "expected a codersdk.Error")
544+
require.Equal(t, http.StatusNotFound, sdkErr.StatusCode())
545+
})
506546
})
507547

508548
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)