Skip to content

Commit 2ef6e7a

Browse files
Disable add-instance-to-anti-affinity-group when instance not stopped (#2790)
* Disable instance adding to anti-affinity group when instance is not stopped * Use Message to explicitly note issue with non-stopped instances * update test * testing * use new instanceCan * change omicron source link for can add to affinity group --------- Co-authored-by: David Crespo <david.crespo@oxidecomputer.com>
1 parent 04aceb5 commit 2ef6e7a

File tree

4 files changed

+47
-8
lines changed

4 files changed

+47
-8
lines changed

app/api/util.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,9 @@ const instanceActions = {
132132
// https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/src/app/instance.rs#L1520-L1522
133133
serialConsole: ['running', 'rebooting', 'migrating', 'repairing'],
134134

135-
// https://github.com/oxidecomputer/omicron/blob/5e27bde/nexus/src/app/affinity.rs#L357 checks to see that there's no VMM
136-
// TODO: determine whether the intent is only `stopped` or also `failed`
137-
addToAntiAffinityGroup: ['stopped'],
135+
// check to see that there's no VMM
136+
// https://github.com/oxidecomputer/omicron/blob/c496683/nexus/db-queries/src/db/datastore/affinity.rs#L1025-L1034
137+
addToAffinityGroup: ['stopped'],
138138
} satisfies Record<string, InstanceState[]>
139139

140140
// setting .states is a cute way to make it ergonomic to call the test function

app/forms/anti-affinity-group-member-add.tsx

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@
99
import { useId } from 'react'
1010
import { useForm } from 'react-hook-form'
1111

12-
import { queryClient, useApiMutation, type Instance } from '~/api'
12+
import { instanceCan, queryClient, useApiMutation, type Instance } from '~/api'
1313
import { ComboboxField } from '~/components/form/fields/ComboboxField'
1414
import { HL } from '~/components/HL'
1515
import { useAntiAffinityGroupSelector } from '~/hooks/use-params'
1616
import { addToast } from '~/stores/toast'
1717
import { toComboboxItems } from '~/ui/lib/Combobox'
18+
import { Message } from '~/ui/lib/Message'
1819
import { Modal } from '~/ui/lib/Modal'
1920

2021
type Values = { instance: string }
@@ -45,6 +46,12 @@ export default function AddAntiAffinityGroupMemberForm({ instances, onDismiss }:
4546
})
4647
})
4748

49+
const instance = form.watch('instance')
50+
const selectedInstance = instances.find((i) => i.name === instance)
51+
const canAddInstance = selectedInstance
52+
? instanceCan.addToAffinityGroup(selectedInstance)
53+
: false
54+
4855
return (
4956
<Modal isOpen onDismiss={onDismiss} title="Add instance to group">
5057
<Modal.Body>
@@ -63,9 +70,20 @@ export default function AddAntiAffinityGroupMemberForm({ instances, onDismiss }:
6370
control={form.control}
6471
/>
6572
</form>
73+
{!canAddInstance && (
74+
<Message
75+
variant="notice"
76+
content="An instance must be stopped to add it to a group"
77+
/>
78+
)}
6679
</Modal.Section>
6780
</Modal.Body>
68-
<Modal.Footer onDismiss={onDismiss} actionText="Add to group" formId={formId} />
81+
<Modal.Footer
82+
onDismiss={onDismiss}
83+
actionText="Add to group"
84+
formId={formId}
85+
disabled={!canAddInstance}
86+
/>
6987
</Modal>
7088
)
7189
}

app/pages/project/instances/AntiAffinityCard.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ export function AntiAffinityCard() {
152152
})
153153

154154
let disabledReason = undefined
155-
if (!instanceCan.addToAntiAffinityGroup(instanceData)) {
155+
if (!instanceCan.addToAffinityGroup(instanceData)) {
156156
disabledReason =
157157
<>Only <HL>stopped</HL> instances can be added to a group</> // prettier-ignore
158158
} else if (allGroups.items.length === 0) {

test/e2e/anti-affinity.e2e.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ test('can add a new anti-affinity group', async ({ page }) => {
5959
const addInstanceButton = page.getByRole('button', { name: 'Add instance' })
6060
const addInstanceModal = page.getByRole('dialog', { name: 'Add instance to group' })
6161
const instanceCombobox = page.getByRole('combobox', { name: 'Instance' })
62+
const modalAddButton = page.getByRole('button', { name: 'Add to group' })
6263

6364
// open modal and pick instance
6465
await addInstanceButton.click()
@@ -72,10 +73,30 @@ test('can add a new anti-affinity group', async ({ page }) => {
7273
await expect(addInstanceModal).toBeHidden()
7374
await addInstanceButton.click()
7475
await expect(instanceCombobox).toHaveValue('')
76+
await page.getByRole('option', { name: 'db1' }).click()
77+
// the submit button should be disabled
78+
await expect(modalAddButton).toBeDisabled()
79+
80+
// go disable db1
81+
await page.getByRole('button', { name: 'Cancel' }).click()
82+
await page.getByRole('link', { name: 'Instances' }).click()
83+
clickRowAction(page, 'db1', 'Stop')
84+
await page.getByRole('button', { name: 'Confirm' }).click()
85+
await expectRowVisible(page.getByRole('table'), {
86+
name: 'db1',
87+
state: expect.stringContaining('stopped'),
88+
})
7589

76-
// now do it again for real and submit
90+
// go back to the anti-affinity group and add the instance
91+
await page.getByRole('link', { name: 'Affinity' }).click()
92+
await page.getByRole('link', { name: 'new-anti-affinity-group' }).click()
93+
await addInstanceButton.click()
94+
await expect(addInstanceModal).toBeVisible()
95+
await instanceCombobox.fill('db1')
7796
await page.getByRole('option', { name: 'db1' }).click()
78-
await page.getByRole('button', { name: 'Add to group' }).click()
97+
await expect(instanceCombobox).toHaveValue('db1')
98+
// the submit button should be enabled
99+
await modalAddButton.click()
79100

80101
const cell = page.getByRole('cell', { name: 'db1' })
81102
await expect(cell).toBeVisible()

0 commit comments

Comments
 (0)