From 65614d74ac38b8e2eb8c5bd06a6a443d96265568 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 8 Apr 2025 14:59:03 -0700 Subject: [PATCH 1/6] Disable instance adding to anti-affinity group when instance is not stopped --- app/forms/anti-affinity-group-member-add.tsx | 16 ++++++++++- app/ui/lib/Modal.tsx | 28 ++++++++++++-------- test/e2e/anti-affinity.e2e.ts | 22 +++++++++++++-- 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/app/forms/anti-affinity-group-member-add.tsx b/app/forms/anti-affinity-group-member-add.tsx index 64a6c8b5cf..4e8e1fc350 100644 --- a/app/forms/anti-affinity-group-member-add.tsx +++ b/app/forms/anti-affinity-group-member-add.tsx @@ -27,6 +27,7 @@ export default function AddAntiAffinityGroupMemberForm({ instances, onDismiss }: const { project, antiAffinityGroup } = useAntiAffinityGroupSelector() const form = useForm({ defaultValues }) + const { instance } = form.watch() const formId = useId() const { mutateAsync: addMember } = useApiMutation('antiAffinityGroupMemberInstanceAdd', { @@ -45,6 +46,9 @@ export default function AddAntiAffinityGroupMemberForm({ instances, onDismiss }: }) }) + const selectedInstanceIsStopped = + instances.find((i) => i.name === instance)?.runState === 'stopped' || false + return ( @@ -65,7 +69,17 @@ export default function AddAntiAffinityGroupMemberForm({ instances, onDismiss }: - + ) } diff --git a/app/ui/lib/Modal.tsx b/app/ui/lib/Modal.tsx index ba48886dde..1aeae1788a 100644 --- a/app/ui/lib/Modal.tsx +++ b/app/ui/lib/Modal.tsx @@ -14,9 +14,11 @@ import { Close12Icon } from '@oxide/design-system/icons/react' import { classed } from '~/util/classed' +import { Wrap } from '../util/wrap' import { Button } from './Button' import { DialogOverlay } from './DialogOverlay' import { ModalContext } from './modal-context' +import { Tooltip } from './Tooltip' type Width = 'narrow' | 'medium' | 'free' @@ -113,6 +115,7 @@ type FooterProps = { actionLoading?: boolean cancelText?: string disabled?: boolean + disabledReason?: string } & MergeExclusive<{ formId: string }, { onAction: () => void }> Modal.Footer = ({ @@ -124,6 +127,7 @@ Modal.Footer = ({ actionLoading, cancelText, disabled = false, + disabledReason, formId, }: FooterProps) => ( ) diff --git a/test/e2e/anti-affinity.e2e.ts b/test/e2e/anti-affinity.e2e.ts index 36c7bb3f5b..a958ab1a22 100644 --- a/test/e2e/anti-affinity.e2e.ts +++ b/test/e2e/anti-affinity.e2e.ts @@ -59,6 +59,7 @@ test('can add a new anti-affinity group', async ({ page }) => { const addInstanceButton = page.getByRole('button', { name: 'Add instance' }) const addInstanceModal = page.getByRole('dialog', { name: 'Add instance to group' }) const instanceCombobox = page.getByRole('combobox', { name: 'Instance' }) + const modalAddButton = page.getByRole('button', { name: 'Add to group' }) // open modal and pick instance await addInstanceButton.click() @@ -72,10 +73,27 @@ test('can add a new anti-affinity group', async ({ page }) => { await expect(addInstanceModal).toBeHidden() await addInstanceButton.click() await expect(instanceCombobox).toHaveValue('') + await page.getByRole('option', { name: 'db1' }).click() + // the submit button should be disabled + await expect(modalAddButton).toBeDisabled() - // now do it again for real and submit + // go disable db1 + await page.getByRole('button', { name: 'Cancel' }).click() + await page.getByRole('link', { name: 'Instances' }).click() + clickRowAction(page, 'db1', 'Stop') + await page.getByRole('button', { name: 'Confirm' }).click() + await expect(page.getByRole('cell', { name: 'db1' })).toHaveText('Stopped') + + // go back to the anti-affinity group and add the instance + await page.getByRole('link', { name: 'Affinity' }).click() + await page.getByRole('link', { name: 'new-anti-affinity-group' }).click() + await addInstanceButton.click() + await expect(addInstanceModal).toBeVisible() + await instanceCombobox.fill('db1') await page.getByRole('option', { name: 'db1' }).click() - await page.getByRole('button', { name: 'Add to group' }).click() + await expect(instanceCombobox).toHaveValue('db1') + // the submit button should be enabled + await modalAddButton.click() const cell = page.getByRole('cell', { name: 'db1' }) await expect(cell).toBeVisible() From 222ff6ada3cca284572a90e1cc3b4aa81c1d3659 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 8 Apr 2025 15:37:40 -0700 Subject: [PATCH 2/6] Use Message to explicitly note issue with non-stopped instances --- app/forms/anti-affinity-group-member-add.tsx | 12 +++++---- app/ui/lib/Modal.tsx | 28 ++++++++------------ 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/app/forms/anti-affinity-group-member-add.tsx b/app/forms/anti-affinity-group-member-add.tsx index 4e8e1fc350..e84883864b 100644 --- a/app/forms/anti-affinity-group-member-add.tsx +++ b/app/forms/anti-affinity-group-member-add.tsx @@ -15,6 +15,7 @@ import { HL } from '~/components/HL' import { useAntiAffinityGroupSelector } from '~/hooks/use-params' import { addToast } from '~/stores/toast' import { toComboboxItems } from '~/ui/lib/Combobox' +import { Message } from '~/ui/lib/Message' import { Modal } from '~/ui/lib/Modal' type Values = { instance: string } @@ -67,6 +68,12 @@ export default function AddAntiAffinityGroupMemberForm({ instances, onDismiss }: control={form.control} /> + {!selectedInstanceIsStopped && ( + + )} ) diff --git a/app/ui/lib/Modal.tsx b/app/ui/lib/Modal.tsx index 1aeae1788a..ba48886dde 100644 --- a/app/ui/lib/Modal.tsx +++ b/app/ui/lib/Modal.tsx @@ -14,11 +14,9 @@ import { Close12Icon } from '@oxide/design-system/icons/react' import { classed } from '~/util/classed' -import { Wrap } from '../util/wrap' import { Button } from './Button' import { DialogOverlay } from './DialogOverlay' import { ModalContext } from './modal-context' -import { Tooltip } from './Tooltip' type Width = 'narrow' | 'medium' | 'free' @@ -115,7 +113,6 @@ type FooterProps = { actionLoading?: boolean cancelText?: string disabled?: boolean - disabledReason?: string } & MergeExclusive<{ formId: string }, { onAction: () => void }> Modal.Footer = ({ @@ -127,7 +124,6 @@ Modal.Footer = ({ actionLoading, cancelText, disabled = false, - disabledReason, formId, }: FooterProps) => (
@@ -136,19 +132,17 @@ Modal.Footer = ({ - }> - - +
) From e80c13964718f29a6a11f16a2e4ec3fda99ea3f1 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 8 Apr 2025 15:47:44 -0700 Subject: [PATCH 3/6] update test --- test/e2e/anti-affinity.e2e.ts | 3 ++- test/e2e/instance.e2e.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/test/e2e/anti-affinity.e2e.ts b/test/e2e/anti-affinity.e2e.ts index a958ab1a22..c8f5ce164b 100644 --- a/test/e2e/anti-affinity.e2e.ts +++ b/test/e2e/anti-affinity.e2e.ts @@ -8,6 +8,7 @@ import { expect, test } from '@playwright/test' +import { expectInstanceState } from './instance.e2e' import { clickRowAction, closeToast, expectRowVisible } from './utils' test('can nav to Affinity from /', async ({ page }) => { @@ -82,7 +83,7 @@ test('can add a new anti-affinity group', async ({ page }) => { await page.getByRole('link', { name: 'Instances' }).click() clickRowAction(page, 'db1', 'Stop') await page.getByRole('button', { name: 'Confirm' }).click() - await expect(page.getByRole('cell', { name: 'db1' })).toHaveText('Stopped') + await expectInstanceState(page, 'db1', 'stopped') // go back to the anti-affinity group and add the instance await page.getByRole('link', { name: 'Affinity' }).click() diff --git a/test/e2e/instance.e2e.ts b/test/e2e/instance.e2e.ts index c1e61bdbd3..9e0d4c69fa 100644 --- a/test/e2e/instance.e2e.ts +++ b/test/e2e/instance.e2e.ts @@ -15,7 +15,7 @@ import { type Page, } from './utils' -const expectInstanceState = async (page: Page, instance: string, state: string) => { +export const expectInstanceState = async (page: Page, instance: string, state: string) => { await expectRowVisible(page.getByRole('table'), { name: instance, state: expect.stringContaining(state), From 715c6296d59f555a9aae9e8201d106c76253cda7 Mon Sep 17 00:00:00 2001 From: Charlie Park Date: Tue, 8 Apr 2025 16:02:24 -0700 Subject: [PATCH 4/6] testing --- test/e2e/anti-affinity.e2e.ts | 6 ++++-- test/e2e/instance.e2e.ts | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/test/e2e/anti-affinity.e2e.ts b/test/e2e/anti-affinity.e2e.ts index c8f5ce164b..3318929871 100644 --- a/test/e2e/anti-affinity.e2e.ts +++ b/test/e2e/anti-affinity.e2e.ts @@ -8,7 +8,6 @@ import { expect, test } from '@playwright/test' -import { expectInstanceState } from './instance.e2e' import { clickRowAction, closeToast, expectRowVisible } from './utils' test('can nav to Affinity from /', async ({ page }) => { @@ -83,7 +82,10 @@ test('can add a new anti-affinity group', async ({ page }) => { await page.getByRole('link', { name: 'Instances' }).click() clickRowAction(page, 'db1', 'Stop') await page.getByRole('button', { name: 'Confirm' }).click() - await expectInstanceState(page, 'db1', 'stopped') + await expectRowVisible(page.getByRole('table'), { + name: 'db1', + state: expect.stringContaining('stopped'), + }) // go back to the anti-affinity group and add the instance await page.getByRole('link', { name: 'Affinity' }).click() diff --git a/test/e2e/instance.e2e.ts b/test/e2e/instance.e2e.ts index 9e0d4c69fa..c1e61bdbd3 100644 --- a/test/e2e/instance.e2e.ts +++ b/test/e2e/instance.e2e.ts @@ -15,7 +15,7 @@ import { type Page, } from './utils' -export const expectInstanceState = async (page: Page, instance: string, state: string) => { +const expectInstanceState = async (page: Page, instance: string, state: string) => { await expectRowVisible(page.getByRole('table'), { name: instance, state: expect.stringContaining(state), From 96a67b96ab223df7a17bfe166d805dfa65f51015 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 9 Apr 2025 00:44:30 -0500 Subject: [PATCH 5/6] use new instanceCan --- app/forms/anti-affinity-group-member-add.tsx | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/app/forms/anti-affinity-group-member-add.tsx b/app/forms/anti-affinity-group-member-add.tsx index 648ba725a5..c9da58dbc3 100644 --- a/app/forms/anti-affinity-group-member-add.tsx +++ b/app/forms/anti-affinity-group-member-add.tsx @@ -9,7 +9,7 @@ import { useId } from 'react' import { useForm } from 'react-hook-form' -import { queryClient, useApiMutation, type Instance } from '~/api' +import { instanceCan, queryClient, useApiMutation, type Instance } from '~/api' import { ComboboxField } from '~/components/form/fields/ComboboxField' import { HL } from '~/components/HL' import { useAntiAffinityGroupSelector } from '~/hooks/use-params' @@ -28,7 +28,6 @@ export default function AddAntiAffinityGroupMemberForm({ instances, onDismiss }: const { project, antiAffinityGroup } = useAntiAffinityGroupSelector() const form = useForm({ defaultValues }) - const { instance } = form.watch() const formId = useId() const { mutateAsync: addMember } = useApiMutation('antiAffinityGroupMemberInstanceAdd', { @@ -47,8 +46,11 @@ export default function AddAntiAffinityGroupMemberForm({ instances, onDismiss }: }) }) - const selectedInstanceIsStopped = - instances.find((i) => i.name === instance)?.runState === 'stopped' || false + const instance = form.watch('instance') + const selectedInstance = instances.find((i) => i.name === instance) + const canAddInstance = selectedInstance + ? instanceCan.addToAntiAffinityGroup(selectedInstance) + : false return ( @@ -68,7 +70,7 @@ export default function AddAntiAffinityGroupMemberForm({ instances, onDismiss }: control={form.control} /> - {!selectedInstanceIsStopped && ( + {!canAddInstance && ( ) From dca09a79f54c9b7ae69abf77a6bb1792954f31e3 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 9 Apr 2025 09:44:00 -0500 Subject: [PATCH 6/6] change omicron source link for can add to affinity group --- app/api/util.ts | 6 +++--- app/forms/anti-affinity-group-member-add.tsx | 2 +- app/pages/project/instances/AntiAffinityCard.tsx | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/api/util.ts b/app/api/util.ts index fb763d4945..ea227eb65a 100644 --- a/app/api/util.ts +++ b/app/api/util.ts @@ -132,9 +132,9 @@ const instanceActions = { // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/src/app/instance.rs#L1520-L1522 serialConsole: ['running', 'rebooting', 'migrating', 'repairing'], - // https://github.com/oxidecomputer/omicron/blob/5e27bde/nexus/src/app/affinity.rs#L357 checks to see that there's no VMM - // TODO: determine whether the intent is only `stopped` or also `failed` - addToAntiAffinityGroup: ['stopped'], + // check to see that there's no VMM + // https://github.com/oxidecomputer/omicron/blob/c496683/nexus/db-queries/src/db/datastore/affinity.rs#L1025-L1034 + addToAffinityGroup: ['stopped'], } satisfies Record // setting .states is a cute way to make it ergonomic to call the test function diff --git a/app/forms/anti-affinity-group-member-add.tsx b/app/forms/anti-affinity-group-member-add.tsx index c9da58dbc3..00498b5ff1 100644 --- a/app/forms/anti-affinity-group-member-add.tsx +++ b/app/forms/anti-affinity-group-member-add.tsx @@ -49,7 +49,7 @@ export default function AddAntiAffinityGroupMemberForm({ instances, onDismiss }: const instance = form.watch('instance') const selectedInstance = instances.find((i) => i.name === instance) const canAddInstance = selectedInstance - ? instanceCan.addToAntiAffinityGroup(selectedInstance) + ? instanceCan.addToAffinityGroup(selectedInstance) : false return ( diff --git a/app/pages/project/instances/AntiAffinityCard.tsx b/app/pages/project/instances/AntiAffinityCard.tsx index fdaa4606c4..d9accb606d 100644 --- a/app/pages/project/instances/AntiAffinityCard.tsx +++ b/app/pages/project/instances/AntiAffinityCard.tsx @@ -152,7 +152,7 @@ export function AntiAffinityCard() { }) let disabledReason = undefined - if (!instanceCan.addToAntiAffinityGroup(instanceData)) { + if (!instanceCan.addToAffinityGroup(instanceData)) { disabledReason = <>Only stopped instances can be added to a group // prettier-ignore } else if (allGroups.items.length === 0) {