From 182cc848091eb30f7c1079223fdb3ffde85eed31 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 27 Sep 2024 16:55:36 -0700 Subject: [PATCH 1/3] Allow user to stop failed instances --- app/api/util.ts | 2 +- test/e2e/instance.e2e.ts | 81 ++++++++++++++-------------------------- 2 files changed, 29 insertions(+), 54 deletions(-) diff --git a/app/api/util.ts b/app/api/util.ts index 5d997c02c4..a6ed207235 100644 --- a/app/api/util.ts +++ b/app/api/util.ts @@ -101,7 +101,7 @@ const instanceActions: Record = { // https://github.com/oxidecomputer/propolis/blob/b278193/bin/propolis-server/src/lib/vm/request_queue.rs // https://github.com/oxidecomputer/console/pull/2387#discussion_r1722368236 reboot: ['running'], // technically rebooting allowed but too weird to say it - stop: ['running', 'starting', 'rebooting'], + stop: ['running', 'starting', 'rebooting', 'failed'], // NoVmm maps to to Stopped: // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-model/src/instance_state.rs#L55 diff --git a/test/e2e/instance.e2e.ts b/test/e2e/instance.e2e.ts index ff2eda9e15..3d1fd4ae16 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 } from './utils' +import { clickRowAction, expect, expectRowVisible, test, type Page } from './utils' + +const expectInstanceState = async (page: Page, instance: string, state: string) => { + await expectRowVisible(page.getByRole('table'), { + name: instance, + state: expect.stringContaining(state), + }) +} test('can delete a failed instance', async ({ page }) => { await page.goto('/projects/mock-project/instances') @@ -14,12 +21,7 @@ test('can delete a failed instance', async ({ page }) => { const cell = page.getByRole('cell', { name: 'you-fail' }) await expect(cell).toBeVisible() // just to match hidden check at the end - - const table = page.getByRole('table') - await expectRowVisible(table, { - name: 'you-fail', - state: expect.stringContaining('failed'), - }) + expectInstanceState(page, 'you-fail', 'failed') await clickRowAction(page, 'you-fail', 'Delete') await page.getByRole('button', { name: 'Confirm' }).click() @@ -42,28 +44,24 @@ test('can start a failed instance', async ({ page }) => { await page.keyboard.press('Escape') // get out of the menu // now start the failed one - const table = page.getByRole('table') - await expectRowVisible(table, { - name: 'you-fail', - state: expect.stringContaining('failed'), - }) - + await expectInstanceState(page, 'you-fail', 'failed') await clickRowAction(page, 'you-fail', 'Start') + await expectInstanceState(page, 'you-fail', 'starting') +}) - await expectRowVisible(table, { - name: 'you-fail', - state: expect.stringContaining('starting'), - }) +test('can stop a failed instance', async ({ page }) => { + await page.goto('/projects/mock-project/instances') + await expect(page).toHaveTitle('Instances / mock-project / Oxide Console') + await expectInstanceState(page, 'you-fail', 'failed') + await clickRowAction(page, 'you-fail', 'Start') + await expectInstanceState(page, 'you-fail', 'starting') + await expectInstanceState(page, 'you-fail', 'running') }) test('can stop and delete a running instance', async ({ page }) => { await page.goto('/projects/mock-project/instances') - const table = page.getByRole('table') - await expectRowVisible(table, { - name: 'db1', - state: expect.stringContaining('running'), - }) + await expectInstanceState(page, 'db1', 'running') const row = page.getByRole('row', { name: 'db1', exact: false }) // can't delete, can stop @@ -73,14 +71,8 @@ test('can stop and delete a running instance', async ({ page }) => { await page.getByRole('button', { name: 'Confirm' }).click() // polling makes it go stopping and then stopped - await expectRowVisible(table, { - name: 'db1', - state: expect.stringContaining('stopping'), - }) - await expectRowVisible(table, { - name: 'db1', - state: expect.stringContaining('stopped'), - }) + await expectInstanceState(page, 'db1', 'stopping') + await expectInstanceState(page, 'db1', 'stopped') // now delete await clickRowAction(page, 'db1', 'Delete') @@ -91,34 +83,17 @@ test('can stop and delete a running instance', async ({ page }) => { test('can stop a starting instance, then start it again', async ({ page }) => { await page.goto('/projects/mock-project/instances') + await expect(page).toHaveTitle('Instances / mock-project / Oxide Console') - const table = page.getByRole('table') - await expectRowVisible(table, { - name: 'not-there-yet', - state: expect.stringContaining('starting'), - }) - + await expectInstanceState(page, 'not-there-yet', 'starting') await clickRowAction(page, 'not-there-yet', 'Stop') await page.getByRole('button', { name: 'Confirm' }).click() - - await expectRowVisible(table, { - name: 'not-there-yet', - state: expect.stringContaining('stopping'), - }) - await expectRowVisible(table, { - name: 'not-there-yet', - state: expect.stringContaining('stopped'), - }) + await expectInstanceState(page, 'not-there-yet', 'stopping') + await expectInstanceState(page, 'not-there-yet', 'stopped') await clickRowAction(page, 'not-there-yet', 'Start') - await expectRowVisible(table, { - name: 'not-there-yet', - state: expect.stringContaining('starting'), - }) - await expectRowVisible(table, { - name: 'not-there-yet', - state: expect.stringContaining('running'), - }) + await expectInstanceState(page, 'not-there-yet', 'starting') + await expectInstanceState(page, 'not-there-yet', 'running') }) test('delete from instance detail', async ({ page }) => { From 99c129f4c53f96dd8a3f90965fff37bb4b17fc26 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Fri, 27 Sep 2024 16:59:18 -0700 Subject: [PATCH 2/3] test the right thing --- test/e2e/instance.e2e.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/e2e/instance.e2e.ts b/test/e2e/instance.e2e.ts index 3d1fd4ae16..6f967f46c9 100644 --- a/test/e2e/instance.e2e.ts +++ b/test/e2e/instance.e2e.ts @@ -53,9 +53,10 @@ test('can stop a failed instance', async ({ page }) => { await page.goto('/projects/mock-project/instances') await expect(page).toHaveTitle('Instances / mock-project / Oxide Console') await expectInstanceState(page, 'you-fail', 'failed') - await clickRowAction(page, 'you-fail', 'Start') - await expectInstanceState(page, 'you-fail', 'starting') - await expectInstanceState(page, 'you-fail', 'running') + await clickRowAction(page, 'you-fail', 'Stop') + await page.getByRole('button', { name: 'Confirm' }).click() + await expectInstanceState(page, 'you-fail', 'stopping') + await expectInstanceState(page, 'you-fail', 'stopped') }) test('can stop and delete a running instance', async ({ page }) => { From 4dc419e66fab4f12dfd6b7298f713f9694709067 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Mon, 30 Sep 2024 09:02:10 -0700 Subject: [PATCH 3/3] add a comment to relevant omicron code --- app/api/util.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/app/api/util.ts b/app/api/util.ts index a6ed207235..6b50e0cbaf 100644 --- a/app/api/util.ts +++ b/app/api/util.ts @@ -101,6 +101,7 @@ const instanceActions: Record = { // https://github.com/oxidecomputer/propolis/blob/b278193/bin/propolis-server/src/lib/vm/request_queue.rs // https://github.com/oxidecomputer/console/pull/2387#discussion_r1722368236 reboot: ['running'], // technically rebooting allowed but too weird to say it + // stopping a failed disk: https://github.com/oxidecomputer/omicron/blob/f0b804818b898bebdb317ac2b000618944c02457/nexus/src/app/instance.rs#L818-L830 stop: ['running', 'starting', 'rebooting', 'failed'], // NoVmm maps to to Stopped: