From 26b54512b34c12179808e7a3f7435e5c662d7e1a Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 15 Nov 2024 17:04:11 -0800 Subject: [PATCH 1/5] Confirm instance reboot --- app/pages/project/instances/actions.tsx | 32 +++++++++++++++++-------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/app/pages/project/instances/actions.tsx b/app/pages/project/instances/actions.tsx index 6b50afd038..6a1e4f313b 100644 --- a/app/pages/project/instances/actions.tsx +++ b/app/pages/project/instances/actions.tsx @@ -41,7 +41,7 @@ export const useMakeInstanceActions = ( const opts = { onSuccess: options.onSuccess } const { mutateAsync: startInstanceAsync } = useApiMutation('instanceStart', opts) const { mutateAsync: stopInstanceAsync } = useApiMutation('instanceStop', opts) - const { mutate: rebootInstance } = useApiMutation('instanceReboot', opts) + const { mutateAsync: rebootInstanceAsync } = useApiMutation('instanceReboot', opts) // delete has its own const { mutateAsync: deleteInstanceAsync } = useApiMutation('instanceDelete', { onSuccess: options.onDelete, @@ -122,15 +122,27 @@ export const useMakeInstanceActions = ( { label: 'Reboot', onActivate() { - rebootInstance(instanceParams, { - onSuccess: () => - addToast(<>Rebooting instance {instance.name}), // prettier-ignore - onError: (error) => - addToast({ - variant: 'error', - title: `Error rebooting instance '${instance.name}'`, - content: error.message, + confirmAction({ + actionType: 'danger', + doAction: () => + rebootInstanceAsync(instanceParams, { + onSuccess: () => + addToast(<>Rebooting instance {instance.name}), // prettier-ignore }), + modalTitle: 'Confirm reboot instance', + modalContent: ( +
+

+ Are you sure you want to reboot {instance.name}? +

+

+ Rebooted instances are reset to their cold-boot state and then + restarted. They retain attached disks and IP addresses; allocated CPU + and memory are freed. +

+
+ ), + errorTitle: `Error rebooting ${instance.name}`, }) }, disabled: !instanceCan.reboot(instance) && ( @@ -162,7 +174,7 @@ export const useMakeInstanceActions = ( }, ] }, - [project, deleteInstanceAsync, navigate, rebootInstance] + [project, deleteInstanceAsync, navigate, rebootInstanceAsync] ) return { makeButtonActions, makeMenuActions } From 3d95f6674799e0c230b37c1c9ebb01b76d48744c Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 15 Nov 2024 17:32:15 -0800 Subject: [PATCH 2/5] Add test re: rebooting instances --- test/e2e/instance.e2e.ts | 47 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/e2e/instance.e2e.ts b/test/e2e/instance.e2e.ts index 8da3ba836f..bd4fba95fb 100644 --- a/test/e2e/instance.e2e.ts +++ b/test/e2e/instance.e2e.ts @@ -99,6 +99,53 @@ test('can stop a starting instance, then start it again', async ({ page }) => { await expectInstanceState(page, 'not-there-yet', 'running') }) +test('can reboot a running instance', async ({ page }) => { + await page.goto('/projects/mock-project/instances') + await expect(page).toHaveTitle('Instances / mock-project / Projects / Oxide Console') + + await expectInstanceState(page, 'db1', 'running') + await clickRowAction(page, 'db1', 'Reboot') + await page.getByRole('button', { name: 'Confirm' }).click() + await expectInstanceState(page, 'db1', 'rebooting') + await expectInstanceState(page, 'db1', 'running') +}) + +test('cannot reboot a failed instance', async ({ page }) => { + await page.goto('/projects/mock-project/instances') + await expect(page).toHaveTitle('Instances / mock-project / Projects / Oxide Console') + await expectInstanceState(page, 'you-fail', 'failed') + await page + .getByRole('row', { name: 'you-fail', exact: false }) + .getByRole('button', { name: 'Row actions' }) + .click() + await expect(page.getByRole('menuitem', { name: 'Reboot' })).toBeDisabled() +}) + +test('cannot reboot a starting instance, or a stopped instance', async ({ page }) => { + await page.goto('/projects/mock-project/instances') + await expect(page).toHaveTitle('Instances / mock-project / Projects / Oxide Console') + await expectInstanceState(page, 'not-there-yet', 'starting') + await page + .getByRole('row', { name: 'not-there-yet', exact: false }) + .getByRole('button', { name: 'Row actions' }) + .click() + await expect(page.getByRole('menuitem', { name: 'Reboot' })).toBeDisabled() + // hit escape to close the menu + await page.keyboard.press('Escape') + + // stop it so we can try rebooting a stopped instance + await clickRowAction(page, 'not-there-yet', 'Stop') + await page.getByRole('button', { name: 'Confirm' }).click() + await expectInstanceState(page, 'not-there-yet', 'stopping') + await expectInstanceState(page, 'not-there-yet', 'stopped') + // reboot is still disabled for a stopped instance + await page + .getByRole('row', { name: 'not-there-yet', exact: false }) + .getByRole('button', { name: 'Row actions' }) + .click() + await expect(page.getByRole('menuitem', { name: 'Reboot' })).toBeDisabled() +}) + test('delete from instance detail', async ({ page }) => { await page.goto('/projects/mock-project/instances/you-fail') From daad6ab38a8d97deb012c3b0f859f5567c9aea44 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 15 Nov 2024 22:23:33 -0800 Subject: [PATCH 3/5] Add helper function to help with testing menu actions --- test/e2e/instance.e2e.ts | 29 ++++++++++++----------------- test/e2e/utils.ts | 10 +++++++--- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/test/e2e/instance.e2e.ts b/test/e2e/instance.e2e.ts index bd4fba95fb..3e89112a28 100644 --- a/test/e2e/instance.e2e.ts +++ b/test/e2e/instance.e2e.ts @@ -5,7 +5,14 @@ * * Copyright Oxide Computer Company */ -import { clickRowAction, expect, expectRowVisible, test, type Page } from './utils' +import { + clickRowAction, + expect, + expectRowVisible, + openRowActions, + test, + type Page, +} from './utils' const expectInstanceState = async (page: Page, instance: string, state: string) => { await expectRowVisible(page.getByRole('table'), { @@ -33,10 +40,7 @@ test('can start a failed instance', async ({ page }) => { await page.goto('/projects/mock-project/instances') // check the start button disabled message on a running instance - await page - .getByRole('row', { name: 'db1', exact: false }) - .getByRole('button', { name: 'Row actions' }) - .click() + await openRowActions(page, 'db1') await page.getByRole('menuitem', { name: 'Start' }).hover() await expect( page.getByText('Only stopped or failed instances can be started') @@ -114,10 +118,7 @@ test('cannot reboot a failed instance', async ({ page }) => { await page.goto('/projects/mock-project/instances') await expect(page).toHaveTitle('Instances / mock-project / Projects / Oxide Console') await expectInstanceState(page, 'you-fail', 'failed') - await page - .getByRole('row', { name: 'you-fail', exact: false }) - .getByRole('button', { name: 'Row actions' }) - .click() + await openRowActions(page, 'you-fail') await expect(page.getByRole('menuitem', { name: 'Reboot' })).toBeDisabled() }) @@ -125,10 +126,7 @@ test('cannot reboot a starting instance, or a stopped instance', async ({ page } await page.goto('/projects/mock-project/instances') await expect(page).toHaveTitle('Instances / mock-project / Projects / Oxide Console') await expectInstanceState(page, 'not-there-yet', 'starting') - await page - .getByRole('row', { name: 'not-there-yet', exact: false }) - .getByRole('button', { name: 'Row actions' }) - .click() + await openRowActions(page, 'not-there-yet') await expect(page.getByRole('menuitem', { name: 'Reboot' })).toBeDisabled() // hit escape to close the menu await page.keyboard.press('Escape') @@ -139,10 +137,7 @@ test('cannot reboot a starting instance, or a stopped instance', async ({ page } await expectInstanceState(page, 'not-there-yet', 'stopping') await expectInstanceState(page, 'not-there-yet', 'stopped') // reboot is still disabled for a stopped instance - await page - .getByRole('row', { name: 'not-there-yet', exact: false }) - .getByRole('button', { name: 'Row actions' }) - .click() + await openRowActions(page, 'not-there-yet') await expect(page.getByRole('menuitem', { name: 'Reboot' })).toBeDisabled() }) diff --git a/test/e2e/utils.ts b/test/e2e/utils.ts index 9cb7864751..2f8437d0cd 100644 --- a/test/e2e/utils.ts +++ b/test/e2e/utils.ts @@ -145,12 +145,16 @@ export async function closeToast(page: Page) { export const clipboardText = async (page: Page) => page.evaluate(() => navigator.clipboard.readText()) -/** Select row by `rowText`, click the row actions button, and click `actionName` */ -export async function clickRowAction(page: Page, rowText: string, actionName: string) { +export const openRowActions = async (page: Page, name: string) => { await page - .getByRole('row', { name: rowText, exact: false }) + .getByRole('row', { name, exact: false }) .getByRole('button', { name: 'Row actions' }) .click() +} + +/** Select row by `rowName`, click the row actions button, and click `actionName` */ +export async function clickRowAction(page: Page, rowName: string, actionName: string) { + await openRowActions(page, rowName) await page.getByRole('menuitem', { name: actionName }).click() } From 5f3c535dc7329daf9d265dc076f205d17f7058ff Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Sat, 16 Nov 2024 06:17:20 -0800 Subject: [PATCH 4/5] Remove duplicate expectations --- test/e2e/instance.e2e.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/e2e/instance.e2e.ts b/test/e2e/instance.e2e.ts index 3e89112a28..e011157bbf 100644 --- a/test/e2e/instance.e2e.ts +++ b/test/e2e/instance.e2e.ts @@ -116,7 +116,6 @@ test('can reboot a running instance', async ({ page }) => { test('cannot reboot a failed instance', async ({ page }) => { await page.goto('/projects/mock-project/instances') - await expect(page).toHaveTitle('Instances / mock-project / Projects / Oxide Console') await expectInstanceState(page, 'you-fail', 'failed') await openRowActions(page, 'you-fail') await expect(page.getByRole('menuitem', { name: 'Reboot' })).toBeDisabled() @@ -124,11 +123,10 @@ test('cannot reboot a failed instance', async ({ page }) => { test('cannot reboot a starting instance, or a stopped instance', async ({ page }) => { await page.goto('/projects/mock-project/instances') - await expect(page).toHaveTitle('Instances / mock-project / Projects / Oxide Console') await expectInstanceState(page, 'not-there-yet', 'starting') await openRowActions(page, 'not-there-yet') await expect(page.getByRole('menuitem', { name: 'Reboot' })).toBeDisabled() - // hit escape to close the menu + // hit escape to close the menu so clickRowAction succeeds await page.keyboard.press('Escape') // stop it so we can try rebooting a stopped instance From 6e3228e0e22f214ac44aef3f4401c8c997cd9b3e Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Mon, 18 Nov 2024 16:03:00 -0800 Subject: [PATCH 5/5] Updated copy in modal --- app/pages/project/instances/actions.tsx | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/app/pages/project/instances/actions.tsx b/app/pages/project/instances/actions.tsx index 6a1e4f313b..b59e09d45a 100644 --- a/app/pages/project/instances/actions.tsx +++ b/app/pages/project/instances/actions.tsx @@ -131,16 +131,9 @@ export const useMakeInstanceActions = ( }), modalTitle: 'Confirm reboot instance', modalContent: ( -
-

- Are you sure you want to reboot {instance.name}? -

-

- Rebooted instances are reset to their cold-boot state and then - restarted. They retain attached disks and IP addresses; allocated CPU - and memory are freed. -

-
+

+ Are you sure you want to reboot {instance.name}? +

), errorTitle: `Error rebooting ${instance.name}`, })