From d773dd13a8643fb379bb4d1cab4eb505b994ab84 Mon Sep 17 00:00:00 2001 From: Joe Thel Date: Mon, 14 Sep 2026 17:27:51 -0700 Subject: [PATCH 1/4] Support canceling imports This constitutes two closely-related workflows: 1. Finalizing a disk from `import_ready` to `detached`. You can get yourself stuck in `import_ready` by starting and immediately abandoning an image upload (which creates a temp disk). 2. Canceling a bulk write and _then_ finalizing a disk, moving from `importing_from_bulk_writes` to `detached`. Same activity, but you get to that stuck state by abandoning an image upload after the actual upload starts. While we won't pretend the states are same when viewing them, "doing something about it" is basically the same thing in both cases, even if the API actions are slightly different. --- app/pages/project/disks/DisksPage.tsx | 70 +++++++++++++++++++++------ mock-api/disk.ts | 28 +++++++++++ test/e2e/disks.e2e.ts | 46 +++++++++++++++++- 3 files changed, 127 insertions(+), 17 deletions(-) diff --git a/app/pages/project/disks/DisksPage.tsx b/app/pages/project/disks/DisksPage.tsx index 956c29182..4ed67f04c 100644 --- a/app/pages/project/disks/DisksPage.tsx +++ b/app/pages/project/disks/DisksPage.tsx @@ -9,6 +9,7 @@ import { useQuery } from '@tanstack/react-query' import { createColumnHelper } from '@tanstack/react-table' import { useCallback, useMemo } from 'react' import { Outlet, type LoaderFunctionArgs } from 'react-router' +import { match } from 'ts-pattern' import { api, @@ -28,6 +29,7 @@ import { DiskStateBadge, DiskTypeBadge, ReadOnlyBadge } from '~/components/State import { makeCrumb } from '~/hooks/use-crumbs' import { getProjectSelector, useProjectSelector } from '~/hooks/use-params' import { useQuickActions } from '~/hooks/use-quick-actions' +import { confirmAction } from '~/stores/confirm-action' import { confirmDelete } from '~/stores/confirm-delete' import { addToast } from '~/stores/toast' import { DiskSourceName } from '~/table/cells/DiskSourceCell' @@ -113,6 +115,9 @@ export default function DisksPage() { }, }) + const { mutateAsync: finalize } = useApiMutation(api.diskFinalizeImport) + const { mutateAsync: stopBulkWriteImport } = useApiMutation(api.diskBulkWriteImportStop) + const makeActions = useCallback( (disk: Disk): MenuAction[] => [ { @@ -131,23 +136,56 @@ export default function DisksPage() { }, disabled: snapshotDisabledReason(disk), }, - { - label: 'Delete', - onActivate: confirmDelete({ - doDelete: () => deleteDisk({ path: { disk: disk.name }, query: { project } }), - label: disk.name, - resourceKind: 'disk', - }), - disabled: - !diskCan.delete(disk) && - (disk.state.state === 'attached' ? ( - 'Disk must be detached before it can be deleted' - ) : ( - <>Only disks in state {fancifyStates(diskCan.delete.states)} can be deleted - )), - }, + match(disk.state.state) + .with('import_ready', 'importing_from_bulk_writes', () => ({ + label: 'Cancel import', + onActivate() { + confirmAction({ + doAction: async () => { + if (disk.state.state === 'importing_from_bulk_writes') { + await stopBulkWriteImport({ + path: { disk: disk.name }, + query: { project }, + }) + } + + await finalize({ + path: { disk: disk.name }, + query: { project }, + body: {}, + }) + + queryClient.invalidateEndpoint('diskList') + addToast( + <> + Import canceled for {disk.name} + + ) + }, + modalTitle: 'Cancel import', + modalContent: `Are you sure you want to cancel import for ${disk.name}?`, + errorTitle: 'Failed to cancel import', + actionType: 'danger', + }) + }, + })) + .otherwise(() => ({ + label: 'Delete', + onActivate: confirmDelete({ + doDelete: () => deleteDisk({ path: { disk: disk.name }, query: { project } }), + label: disk.name, + resourceKind: 'disk', + }), + disabled: + !diskCan.delete(disk) && + (disk.state.state === 'attached' ? ( + 'Disk must be detached before it can be deleted' + ) : ( + <>Only disks in state {fancifyStates(diskCan.delete.states)} can be deleted + )), + })), ], - [createSnapshot, deleteDisk, project] + [createSnapshot, deleteDisk, stopBulkWriteImport, finalize, project] ) const columns = useColsWithActions( diff --git a/mock-api/disk.ts b/mock-api/disk.ts index 577cbddce..2a8621c6a 100644 --- a/mock-api/disk.ts +++ b/mock-api/disk.ts @@ -239,6 +239,34 @@ export const disks: Json[] = [ disk_type: 'distributed', read_only: false, }, + { + id: '7b898827-35a1-4459-a4e3-34db90640b74', + name: 'tmp-for-image-29884739', + description: 'stuck in import_ready after bailling on an image upload early', + project_id: project.id, + time_created: new Date().toISOString(), + time_modified: new Date().toISOString(), + state: { state: 'import_ready' }, + device_path: '/import', + size: 8 * GiB, + block_size: 2048, + disk_type: 'distributed', + read_only: false, + }, + { + id: 'f874c0b9-72ad-4eac-8e55-e6090e10366a', + name: 'tmp-for-image-59986861', + description: 'stuck in bulk-write after bailling on an image upload early', + project_id: project.id, + time_created: new Date().toISOString(), + time_modified: new Date().toISOString(), + state: { state: 'importing_from_bulk_writes' }, + device_path: '/import', + size: 8 * GiB, + block_size: 2048, + disk_type: 'distributed', + read_only: false, + }, { id: '3f23c80f-c523-4d86-8292-2ca3f807bb12', name: 'disk-snapshot-error', diff --git a/test/e2e/disks.e2e.ts b/test/e2e/disks.e2e.ts index 0b9724df1..0ce5d3eaf 100644 --- a/test/e2e/disks.e2e.ts +++ b/test/e2e/disks.e2e.ts @@ -7,6 +7,7 @@ */ import { clickRowAction, + clickRowActions, expect, expectNoToast, expectRowVisible, @@ -93,7 +94,7 @@ test('List disks and snapshot', async ({ page }) => { await page.goto('/projects/mock-project/disks') const table = page.getByRole('table') - await expect(table.getByRole('row')).toHaveCount(16) // 15 + header + await expect(table.getByRole('row')).toHaveCount(18) // 17 + header // check one attached and one not attached await expectRowVisible(table, { @@ -166,6 +167,49 @@ test('Read-only disk snapshot disabled', async ({ page }) => { ) }) +test('Cancel import from import_ready', async ({ page }) => { + const diskImportReadyName = 'tmp-for-image-29884739' + await page.goto('/projects/mock-project/disks') + const table = page.getByRole('table') + await expectRowVisible(table, { name: diskImportReadyName, state: 'import ready' }) + + await clickRowActions(page, diskImportReadyName) + await expect(page.getByRole('menuitem', { name: 'Delete' })).toBeHidden() + await page.getByRole('menuitem', { name: 'Cancel import' }).click() + + const modal = page.getByRole('dialog', { name: 'Cancel import' }) + await expect(modal).toBeVisible() + await modal.getByRole('button', { name: 'Confirm' }).click() + + await expectToast(page, `Import canceled for ${diskImportReadyName}`) + await expectRowVisible(table, { name: diskImportReadyName, state: 'detached' }) + await clickRowActions(page, diskImportReadyName) + await expect(page.getByRole('menuitem', { name: 'Delete' })).toBeVisible() +}) + +test('Cancel import from importing_from_bulk_writes', async ({ page }) => { + const diskImportingName = 'tmp-for-image-59986861' + await page.goto('/projects/mock-project/disks') + const table = page.getByRole('table') + await expectRowVisible(table, { + name: diskImportingName, + state: 'importing from bulk writes', + }) + + await clickRowActions(page, diskImportingName) + await expect(page.getByRole('menuitem', { name: 'Delete' })).toBeHidden() + await page.getByRole('menuitem', { name: 'Cancel import' }).click() + + const modal = page.getByRole('dialog', { name: 'Cancel import' }) + await expect(modal).toBeVisible() + await modal.getByRole('button', { name: 'Confirm' }).click() + + await expectToast(page, `Import canceled for ${diskImportingName}`) + await expectRowVisible(table, { name: diskImportingName, state: 'detached' }) + await clickRowActions(page, diskImportingName) + await expect(page.getByRole('menuitem', { name: 'Delete' })).toBeVisible() +}) + test.describe('Disk create', () => { test.beforeEach(async ({ page }) => { await page.goto('/projects/mock-project/disks-new') From e0dd917b201347a5edb97fd61fe72a3fa81305df Mon Sep 17 00:00:00 2001 From: Joe Thel Date: Mon, 14 Sep 2026 17:51:24 -0700 Subject: [PATCH 2/4] Eagerly refresh disk state while canceling Kind of a middle ground between "show two toasts for this action" and "pretend we aren't making two api calls". --- app/pages/project/disks/DisksPage.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/app/pages/project/disks/DisksPage.tsx b/app/pages/project/disks/DisksPage.tsx index 4ed67f04c..bb6d4d0c6 100644 --- a/app/pages/project/disks/DisksPage.tsx +++ b/app/pages/project/disks/DisksPage.tsx @@ -115,8 +115,16 @@ export default function DisksPage() { }, }) - const { mutateAsync: finalize } = useApiMutation(api.diskFinalizeImport) - const { mutateAsync: stopBulkWriteImport } = useApiMutation(api.diskBulkWriteImportStop) + const { mutateAsync: finalize } = useApiMutation(api.diskFinalizeImport, { + onSuccess() { + queryClient.invalidateEndpoint('diskList') + }, + }) + const { mutateAsync: stopBulkWriteImport } = useApiMutation(api.diskBulkWriteImportStop, { + onSuccess() { + queryClient.invalidateEndpoint('diskList') + }, + }) const makeActions = useCallback( (disk: Disk): MenuAction[] => [ @@ -155,7 +163,6 @@ export default function DisksPage() { body: {}, }) - queryClient.invalidateEndpoint('diskList') addToast( <> Import canceled for {disk.name} From 2bb465f5e58f074937a52344c8183fdae9817a9b Mon Sep 17 00:00:00 2001 From: Joe Thel Date: Mon, 14 Sep 2026 18:14:12 -0700 Subject: [PATCH 3/4] Use deterministic temp disk name for upload tests --- app/forms/image-upload.tsx | 1 + mock-api/disk.ts | 4 ++-- test/e2e/image-upload.e2e.ts | 6 ++++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/forms/image-upload.tsx b/app/forms/image-upload.tsx index bade80684..3945c5604 100644 --- a/app/forms/image-upload.tsx +++ b/app/forms/image-upload.tsx @@ -153,6 +153,7 @@ function getTmpDiskName(imageName: string) { 'import-start-500', 'import-stop-500', 'disk-finalize-500', + 'cancel-upload', ]) if (specialNames.has(imageName)) return imageName } diff --git a/mock-api/disk.ts b/mock-api/disk.ts index 2a8621c6a..757f6bbbf 100644 --- a/mock-api/disk.ts +++ b/mock-api/disk.ts @@ -242,7 +242,7 @@ export const disks: Json[] = [ { id: '7b898827-35a1-4459-a4e3-34db90640b74', name: 'tmp-for-image-29884739', - description: 'stuck in import_ready after bailling on an image upload early', + description: 'stuck in import_ready after bailing on an image upload early', project_id: project.id, time_created: new Date().toISOString(), time_modified: new Date().toISOString(), @@ -256,7 +256,7 @@ export const disks: Json[] = [ { id: 'f874c0b9-72ad-4eac-8e55-e6090e10366a', name: 'tmp-for-image-59986861', - description: 'stuck in bulk-write after bailling on an image upload early', + description: 'stuck in bulk-write after bailing on an image upload early', project_id: project.id, time_created: new Date().toISOString(), time_modified: new Date().toISOString(), diff --git a/test/e2e/image-upload.e2e.ts b/test/e2e/image-upload.e2e.ts index 34df27799..314e6acb1 100644 --- a/test/e2e/image-upload.e2e.ts +++ b/test/e2e/image-upload.e2e.ts @@ -192,7 +192,7 @@ test.describe('Image upload', () => { for (const state of cancelStates) { test(`cancel in state '${state}'`, async ({ page }) => { - await fillForm(page, 'new-image') + await fillForm(page, 'cancel-upload') await page.getByRole('button', { name: 'Upload image' }).click() @@ -219,7 +219,9 @@ test.describe('Image upload', () => { await page.getByRole('button', { name: 'Cancel' }).click() await page.getByRole('link', { name: 'Disks' }).click() await expect(page.getByRole('cell', { name: 'disk-1', exact: true })).toBeVisible() - await expect(page.getByRole('cell', { name: 'tmp' })).toBeHidden() + await expect( + page.getByRole('cell', { name: 'cancel-upload', exact: true }) + ).toBeHidden() }) } From d0ce6d9dde4d44175d09342fcac9bf574e837f54 Mon Sep 17 00:00:00 2001 From: Joe Thel Date: Mon, 14 Sep 2026 19:06:51 -0700 Subject: [PATCH 4/4] Add test for partial success in cancellation --- mock-api/disk.ts | 15 +++++++++++++++ mock-api/msw/handlers.ts | 4 +++- test/e2e/disks.e2e.ts | 21 ++++++++++++++++++++- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/mock-api/disk.ts b/mock-api/disk.ts index 757f6bbbf..23a3887fb 100644 --- a/mock-api/disk.ts +++ b/mock-api/disk.ts @@ -267,6 +267,21 @@ export const disks: Json[] = [ disk_type: 'distributed', read_only: false, }, + { + id: '0f60c28e-ead0-48f0-aab9-e74b917dc8e4', + name: 'disk-finalize-fail', + description: + "stuck in bulk-write after bailing on an image upload early, but can't be finalized", + project_id: project.id, + time_created: new Date().toISOString(), + time_modified: new Date().toISOString(), + state: { state: 'importing_from_bulk_writes' }, + device_path: '/import', + size: 8 * GiB, + block_size: 2048, + disk_type: 'distributed', + read_only: false, + }, { id: '3f23c80f-c523-4d86-8292-2ca3f807bb12', name: 'disk-snapshot-error', diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index d0ce152a1..7ead44793 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -284,7 +284,9 @@ export const handlers = makeHandlers({ diskFinalizeImport: ({ path, query, body }) => { const disk = lookup.disk({ ...path, ...query }) - if (disk.name === 'disk-finalize-500') throw internalError('disk finalize failed') + if (disk.name === 'disk-finalize-500' || disk.name === 'disk-finalize-fail') { + throw internalError('disk finalize failed') + } if (disk.state.state !== 'import_ready') { throw `Cannot finalize disk in state ${disk.state.state}. Must be import_ready.` diff --git a/test/e2e/disks.e2e.ts b/test/e2e/disks.e2e.ts index 0ce5d3eaf..8115c822a 100644 --- a/test/e2e/disks.e2e.ts +++ b/test/e2e/disks.e2e.ts @@ -94,7 +94,7 @@ test('List disks and snapshot', async ({ page }) => { await page.goto('/projects/mock-project/disks') const table = page.getByRole('table') - await expect(table.getByRole('row')).toHaveCount(18) // 17 + header + await expect(table.getByRole('row')).toHaveCount(19) // 18 + header // check one attached and one not attached await expectRowVisible(table, { @@ -210,6 +210,25 @@ test('Cancel import from importing_from_bulk_writes', async ({ page }) => { await expect(page.getByRole('menuitem', { name: 'Delete' })).toBeVisible() }) +test('Cancel import from importing_from_bulk_writes shows error and refreshes state when finalize fails', async ({ + page, +}) => { + const diskName = 'disk-finalize-fail' + await page.goto('/projects/mock-project/disks') + const table = page.getByRole('table') + await expectRowVisible(table, { name: diskName, state: 'importing from bulk writes' }) + + await clickRowActions(page, diskName) + await page.getByRole('menuitem', { name: 'Cancel import' }).click() + await page + .getByRole('dialog', { name: 'Cancel import' }) + .getByRole('button', { name: 'Confirm' }) + .click() + + await expectToast(page, 'Failed to cancel import') + await expectRowVisible(table, { name: diskName, state: 'import ready' }) +}) + test.describe('Disk create', () => { test.beforeEach(async ({ page }) => { await page.goto('/projects/mock-project/disks-new')