Loading spinner in attach ephemeral IP modal (#3200)

Closes #3197. Turns out we were using a bespoke modal + form thing
instead of our dedicated `ModalForm` component that handles the loading
state in the button. Turns out there are a bunch of modal forms that
aren't using `ModalForm` like they probably should. I'll fix the other
ones in another PR.

Author: david-crespo

AGENTS.md

@@ -67,6 +67,7 @@
 - Store API response objects in the mock tables when possible so state persists across calls.
 - Enforce role checks with `requireFleetViewer`/`requireFleetCollab`/`requireFleetAdmin`, and return realistic errors (e.g. downgrade guard in `systemUpdateStatus`).
 - All UUIDs in `mock-api/` must be valid RFC 4122 (a safety test enforces this). Use `uuidgen` to generate them—do not hand-write UUIDs.
+- To test error paths in e2e tests, do **not** use `page.route` to intercept API calls. Instead, add a sentinel (a well-known name, id, or fixture value) to the mock handler that makes it return the desired error, and drive the test through the real UI. Branch on an input the user controls (e.g., `if (body.name === '<sentinel>') throw 500`) or, if no such input is available, add a sentinel fixture that is otherwise inert. Keeping the mock backend authoritative means the failure path is reproducible in the dev server too, not only from tests.
 - MSW starts fresh with a new db on every page load, so in E2E tests, use client-side navigation (click links/breadcrumbs) after mutations instead of `page.goto` to preserve db state within a test.
 
 # Routing

app/components/AttachEphemeralIpModal.tsx

@@ -21,12 +21,12 @@ import {
   type IpVersion,
 } from '~/api'
 import { ListboxField } from '~/components/form/fields/ListboxField'
+import { ModalForm } from '~/components/form/ModalForm'
 import { HL } from '~/components/HL'
 import { toPoolItem } from '~/components/PoolListboxItem'
 import { useInstanceSelector } from '~/hooks/use-params'
 import { addToast } from '~/stores/toast'
 import { Message } from '~/ui/lib/Message'
-import { Modal } from '~/ui/lib/Modal'
 import { ALL_ISH } from '~/util/consts'
 
 type AttachEphemeralIpModalProps = {
@@ -70,49 +70,40 @@ export const AttachEphemeralIpModal = ({
   const form = useForm({ defaultValues: { pool: defaultPool } })
   const pool = form.watch('pool')
 
-  const disabledReason =
+  const submitDisabled =
     compatibleUnicastPools.length === 0
       ? 'No compatible unicast pools available for this instance'
       : !pool
-        ? 'Select a pool to continue'
+        ? 'Select a pool'
         : undefined
 
   return (
-    <Modal isOpen title="Attach ephemeral IP" onDismiss={onDismiss}>
-      <Modal.Body>
-        <Modal.Section>
-          {infoMessage && <Message variant="info" content={infoMessage} />}
-          <form>
-            <ListboxField
-              name="pool"
-              label="Pool"
-              control={form.control}
-              items={sortPools(compatibleUnicastPools).map(toPoolItem)}
-              disabled={compatibleUnicastPools.length === 0}
-              placeholder="Select a pool"
-              noItemsPlaceholder="No pools available"
-            />
-          </form>
-          {instanceEphemeralIpAttach.error && (
-            <p className="text-error mt-4">{instanceEphemeralIpAttach.error.message}</p>
-          )}
-        </Modal.Section>
-      </Modal.Body>
-      <Modal.Footer
-        actionText="Attach"
-        disabled={!!disabledReason}
-        disabledReason={disabledReason}
-        onAction={() => {
-          instanceEphemeralIpAttach.mutate({
-            path: { instance },
-            query: { project },
-            body: {
-              poolSelector: { type: 'explicit', pool },
-            },
-          })
-        }}
-        onDismiss={onDismiss}
-      ></Modal.Footer>
-    </Modal>
+    <ModalForm
+      form={form}
+      title="Attach ephemeral IP"
+      onDismiss={onDismiss}
+      submitLabel="Attach"
+      submitDisabled={submitDisabled}
+      submitError={instanceEphemeralIpAttach.error}
+      loading={instanceEphemeralIpAttach.isPending}
+      onSubmit={({ pool }) => {
+        instanceEphemeralIpAttach.mutate({
+          path: { instance },
+          query: { project },
+          body: { poolSelector: { type: 'explicit', pool } },
+        })
+      }}
+    >
+      {infoMessage && <Message variant="info" content={infoMessage} />}
+      <ListboxField
+        name="pool"
+        label="Pool"
+        control={form.control}
+        items={sortPools(compatibleUnicastPools).map(toPoolItem)}
+        disabled={compatibleUnicastPools.length === 0}
+        placeholder="Select a pool"
+        noItemsPlaceholder="No pools available"
+      />
+    </ModalForm>
   )
 }

app/components/AttachFloatingIpModal.tsx

@@ -103,7 +103,7 @@ export const AttachFloatingIpModal = ({
           body: { kind: 'instance', parent: instance.id },
         })
       }
-      submitDisabled={!floatingIp}
+      submitDisabled={!floatingIp ? 'Select a floating IP' : undefined}
     >
       <Message
         variant="info"

app/components/form/ModalForm.tsx

@@ -18,8 +18,8 @@ type ModalFormProps<TFieldValues extends FieldValues> = {
   form: UseFormReturn<TFieldValues>
   children: ReactNode
 
-  /** Must be provided with a reason describing why it's disabled */
-  submitDisabled?: boolean
+  /** Reason the submit button is disabled (shown as a tooltip). Undefined means enabled. */
+  submitDisabled?: string
   onSubmit: (values: TFieldValues) => void
   submitLabel: string
 
@@ -35,7 +35,7 @@ export function ModalForm<TFieldValues extends FieldValues>({
   form,
   children,
   onDismiss,
-  submitDisabled = false,
+  submitDisabled,
   submitError,
   title,
   onSubmit,
@@ -77,7 +77,8 @@ export function ModalForm<TFieldValues extends FieldValues>({
         onDismiss={onDismiss}
         formId={id}
         actionText={submitLabel}
-        disabled={submitDisabled}
+        disabled={!!submitDisabled}
+        disabledReason={submitDisabled}
         actionLoading={loading || isSubmitting}
       />
     </Modal>

mock-api/ip-pool.ts

@@ -72,13 +72,26 @@ export const ipPool6Multicast: Json<IpPool> = {
   pool_type: 'multicast',
 }
 
+// Sentinel pool: selecting this pool in an ephemeral-IP attach request causes
+// the mock handler to return a 500 so tests can exercise the failure path.
+export const ipPoolEphemeralAttachFail: Json<IpPool> = {
+  id: 'a82e20a3-1fb3-4d72-910a-2883298304a2',
+  name: 'attach-fail',
+  description: 'Sentinel: ephemeral IP attach returns 500',
+  time_created: new Date().toISOString(),
+  time_modified: new Date().toISOString(),
+  ip_version: 'v6',
+  pool_type: 'unicast',
+}
+
 export const ipPools: Json<IpPool>[] = [
   ipPool1,
   ipPool2,
   ipPool3,
   ipPool4,
   ipPool5Multicast,
   ipPool6Multicast,
+  ipPoolEphemeralAttachFail,
 ]
 
 export const ipPoolSilos: Json<IpPoolSiloLink>[] = [
@@ -105,6 +118,12 @@ export const ipPoolSilos: Json<IpPoolSiloLink>[] = [
     is_default: true,
   },
 
+  {
+    ip_pool_id: ipPoolEphemeralAttachFail.id,
+    silo_id: defaultSilo.id,
+    is_default: false,
+  },
+
   // myriad silo: v4-only default
   {
     ip_pool_id: ipPool1.id,

mock-api/msw/handlers.ts

@@ -962,6 +962,10 @@ export const handlers = makeHandlers({
     const instanceProject = lookup.project(projectParams)
     // Ephemeral IPs must use unicast pools
     const pool = resolvePoolSelector(body.pool_selector, 'unicast', instanceProject.silo_id)
+
+    // Sentinel: see ipPoolEphemeralAttachFail in ../ip-pool.ts
+    if (pool.name === 'attach-fail') throw 'Cannot attach ephemeral IP'
+
     const ip = getIpFromPool(pool)
 
     // Validate that external IP version matches primary NIC's IP stack

test/e2e/instance-networking.e2e.ts

@@ -212,6 +212,26 @@ test('Instance networking tab — Detach / Attach Ephemeral IPs', async ({ page
   })
 })
 
+test('Attach ephemeral IP — error renders in modal, not toast', async ({ page }) => {
+  // Selecting the sentinel `attach-fail` pool causes the mock handler to 500.
+  // See ipPoolEphemeralAttachFail.
+  await page.goto('/projects/mock-project/instances/db1/networking')
+  await page.getByRole('button', { name: 'Attach ephemeral IP' }).click()
+
+  const modal = page.getByRole('dialog', { name: 'Attach ephemeral IP' })
+  await expect(modal).toBeVisible()
+
+  await page.getByLabel('Pool').click()
+  await page.getByRole('option', { name: 'attach-fail' }).click()
+  await page.getByRole('button', { name: 'Attach', exact: true }).click()
+
+  const errorText = 'Cannot attach ephemeral IP'
+  await expect(modal.getByText(errorText)).toBeVisible()
+  await expect(page.getByTestId('Toasts')).not.toContainText(errorText)
+  // Modal stays open so the user can retry or dismiss
+  await expect(modal).toBeVisible()
+})
+
 test('Instance networking tab — floating IPs', async ({ page }) => {
   await page.goto('/projects/mock-project/instances/db1/networking')
   const externalIpTable = page.getByRole('table', { name: 'External IPs' })

test/e2e/ip-pools.e2e.ts

@@ -19,7 +19,7 @@ test('IP pool list', async ({ page }) => {
 
   const table = page.getByRole('table')
 
-  await expect(table.getByRole('row')).toHaveCount(7) // header + 6 rows (includes multicast pools)
+  await expect(table.getByRole('row')).toHaveCount(8) // header + 7 rows (includes multicast pools and `attach-fail` sentinel)
 
   await expectRowVisible(table, {
     name: 'ip-pool-1',

test/e2e/silos.e2e.ts

@@ -269,7 +269,7 @@ test('Silo IP pools', async ({ page }) => {
     name: 'ip-pool-6-multicast-v6default',
     Version: 'v6',
   })
-  await expect(table.getByRole('row')).toHaveCount(5) // header + 4
+  await expect(table.getByRole('row')).toHaveCount(6) // header + 4 + sentinel `attach-fail`
 
   // clicking on pool goes to pool detail
   await page.getByRole('link', { name: 'ip-pool-1' }).click()
@@ -315,7 +315,7 @@ test('Silo IP pools link pool', async ({ page }) => {
     name: 'ip-pool-6-multicast-v6default',
     Version: 'v6',
   })
-  await expect(table.getByRole('row')).toHaveCount(5) // header + 4
+  await expect(table.getByRole('row')).toHaveCount(6) // header + 4 + sentinel `attach-fail`
 
   const modal = page.getByRole('dialog', { name: 'Link pool' })
   await expect(modal).toBeHidden()