From ea0b0b57f16222f940ab00b82715f27475c8b540 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Thu, 2 May 2024 20:16:36 -0700
Subject: [PATCH 01/18] stub of external IP selection

---
 app/forms/instance-create.tsx | 62 +++++++++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index 6d53acc033..cab42c9f96 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -6,7 +6,7 @@
  * Copyright Oxide Computer Company
  */
 import * as Accordion from '@radix-ui/react-accordion'
-import { useEffect, useMemo, useState } from 'react'
+import { useEffect, useMemo, useState, type SetStateAction } from 'react'
 import { useWatch, type Control } from 'react-hook-form'
 import { useNavigate, type LoaderFunctionArgs } from 'react-router-dom'
 import type { SetRequired } from 'type-fest'
@@ -18,6 +18,7 @@ import {
   INSTANCE_MAX_CPU,
   INSTANCE_MAX_RAM_GiB,
   useApiMutation,
+  useApiQuery,
   useApiQueryClient,
   usePrefetchedApiQuery,
   type InstanceCreate,
@@ -50,8 +51,10 @@ import { Form } from '~/components/form/Form'
 import { FullPageForm } from '~/components/form/FullPageForm'
 import { getProjectSelector, useForm, useProjectSelector } from '~/hooks'
 import { addToast } from '~/stores/toast'
+import { Checkbox } from '~/ui/lib/Checkbox'
 import { FormDivider } from '~/ui/lib/Divider'
 import { EmptyMessage } from '~/ui/lib/EmptyMessage'
+import { Listbox } from '~/ui/lib/Listbox'
 import { Message } from '~/ui/lib/Message'
 import { RadioCard } from '~/ui/lib/Radio'
 import { Tabs } from '~/ui/lib/Tabs'
@@ -130,6 +133,7 @@ const baseDefaultValues: InstanceCreateInput = {
   start: true,
 
   userData: null,
+  externalIps: [{ type: 'ephemeral' }],
 }
 
 const DISK_FETCH_LIMIT = 1000
@@ -283,7 +287,7 @@ export function CreateInstanceForm() {
             memory: instance.memory * GiB,
             ncpus: instance.ncpus,
             disks: [bootDisk, ...values.disks],
-            externalIps: [{ type: 'ephemeral' }],
+            externalIps: values.externalIps,
             start: values.start,
             networkInterfaces: values.networkInterfaces,
             sshPublicKeys: values.sshPublicKeys,
@@ -534,6 +538,27 @@ const AdvancedAccordion = ({
   // tell, inside AccordionItem, when an accordion is opened so we can scroll its
   // contents into view
   const [openItems, setOpenItems] = useState<string[]>([])
+  // TODO: move to prefetched query
+  const allPools = useApiQuery('ipPoolList', { query: { limit: 1000 } })
+  const [assignEphemeralIp, setAssignEphemeralIp] = useState(true)
+  const [selectedPool, setSelectedPool] = useState<SetStateAction<string | null>>(
+    allPools?.data?.items[0]?.name || null
+  )
+
+  useEffect(() => {
+    control._formValues.externalIps = assignEphemeralIp
+      ? [{ type: 'ephemeral' }]
+      : selectedPool
+        ? [{ pool: selectedPool }]
+        : []
+  }, [assignEphemeralIp, selectedPool, control._formValues])
+
+  // we need to default to the default pool, but we aren't pulling that info in yet
+  useEffect(() => {
+    if (!selectedPool) {
+      setSelectedPool(allPools?.data?.items[0]?.name || null)
+    }
+  }, [allPools, selectedPool])
 
   return (
     <Accordion.Root
@@ -555,6 +580,39 @@ const AdvancedAccordion = ({
           control={control}
           disabled={isSubmitting}
         />
+
+        <div className="max-w-lg space-y-2">
+          <h2>External IP</h2>
+          <div className="flex items-center gap-2.5">
+            {/* This is currently set as a checkbox, but I'm thinking it should be a radio button, with the options being "ephemeral" or "floating" */}
+            <Checkbox
+              id="assignEphemeralIp"
+              checked={assignEphemeralIp}
+              onChange={() => setAssignEphemeralIp(!assignEphemeralIp)}
+            />
+            <label htmlFor="assignEphemeralIp" className="text-sans-md text-secondary">
+              Assign an ephemeral IP address
+            </label>
+          </div>
+        </div>
+        <Listbox
+          name="pools"
+          label="Pools"
+          // this cannot be right
+          selected={`${selectedPool}`}
+          items={
+            allPools?.data?.items.map((pool) =>
+              // according to the designs, we need to indicate the default pool, but we don't have that info pulled in yet
+              ({
+                label: pool.name,
+                value: pool.name,
+              })
+            ) || []
+          }
+          disabled={assignEphemeralIp || isSubmitting}
+          required
+          onChange={(value) => setSelectedPool(value)}
+        />
       </AccordionItem>
       <AccordionItem
         value="configuration"

From f659f812da68419e90d36a0321191ebd10566952 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Fri, 3 May 2024 11:04:14 -0700
Subject: [PATCH 02/18] refactoring

---
 app/forms/instance-create.tsx | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index cab42c9f96..a7b0ff5280 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -539,26 +539,28 @@ const AdvancedAccordion = ({
   // contents into view
   const [openItems, setOpenItems] = useState<string[]>([])
   // TODO: move to prefetched query
-  const allPools = useApiQuery('ipPoolList', { query: { limit: 1000 } })
+  const { data: allPools } = useApiQuery('projectIpPoolList', { query: { limit: 1000 } })
+  const defaultPool = useMemo(
+    () => (allPools ? allPools.items.find((p) => p.isDefault)?.name : undefined),
+    [allPools]
+  )
+
   const [assignEphemeralIp, setAssignEphemeralIp] = useState(true)
   const [selectedPool, setSelectedPool] = useState<SetStateAction<string | null>>(
-    allPools?.data?.items[0]?.name || null
+    defaultPool || null
   )
 
   useEffect(() => {
     control._formValues.externalIps = assignEphemeralIp
-      ? [{ type: 'ephemeral' }]
-      : selectedPool
-        ? [{ pool: selectedPool }]
-        : []
+      ? [{ type: 'ephemeral' }, { pool: selectedPool }]
+      : []
   }, [assignEphemeralIp, selectedPool, control._formValues])
 
-  // we need to default to the default pool, but we aren't pulling that info in yet
   useEffect(() => {
-    if (!selectedPool) {
-      setSelectedPool(allPools?.data?.items[0]?.name || null)
+    if (!selectedPool && defaultPool) {
+      setSelectedPool(defaultPool)
     }
-  }, [allPools, selectedPool])
+  }, [defaultPool, selectedPool])
 
   return (
     <Accordion.Root
@@ -597,19 +599,19 @@ const AdvancedAccordion = ({
         </div>
         <Listbox
           name="pools"
-          label="Pools"
+          label="Assign ephemeral IP from pool"
           // this cannot be right
           selected={`${selectedPool}`}
           items={
-            allPools?.data?.items.map((pool) =>
+            allPools?.items.map((pool) =>
               // according to the designs, we need to indicate the default pool, but we don't have that info pulled in yet
               ({
-                label: pool.name,
+                label: `${pool.name}${pool.isDefault ? ' (default)' : ''}`,
                 value: pool.name,
               })
             ) || []
           }
-          disabled={assignEphemeralIp || isSubmitting}
+          disabled={!assignEphemeralIp || isSubmitting}
           required
           onChange={(value) => setSelectedPool(value)}
         />

From a8fd20dec099251450db3e644aa1a499cc9197c6 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Fri, 3 May 2024 11:47:42 -0700
Subject: [PATCH 03/18] better handling of default

---
 app/forms/instance-create.tsx | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index a7b0ff5280..f9c76531af 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -538,7 +538,6 @@ const AdvancedAccordion = ({
   // tell, inside AccordionItem, when an accordion is opened so we can scroll its
   // contents into view
   const [openItems, setOpenItems] = useState<string[]>([])
-  // TODO: move to prefetched query
   const { data: allPools } = useApiQuery('projectIpPoolList', { query: { limit: 1000 } })
   const defaultPool = useMemo(
     () => (allPools ? allPools.items.find((p) => p.isDefault)?.name : undefined),
@@ -557,10 +556,10 @@ const AdvancedAccordion = ({
   }, [assignEphemeralIp, selectedPool, control._formValues])
 
   useEffect(() => {
-    if (!selectedPool && defaultPool) {
+    if (defaultPool) {
       setSelectedPool(defaultPool)
     }
-  }, [defaultPool, selectedPool])
+  }, [defaultPool])
 
   return (
     <Accordion.Root

From 883fe3070b89c4cd173b7fcc1d09c3884b707315 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Fri, 3 May 2024 12:03:26 -0700
Subject: [PATCH 04/18] fix ephemeralIp bug

---
 app/forms/instance-create.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index f9c76531af..96c41caf2c 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -551,7 +551,7 @@ const AdvancedAccordion = ({
 
   useEffect(() => {
     control._formValues.externalIps = assignEphemeralIp
-      ? [{ type: 'ephemeral' }, { pool: selectedPool }]
+      ? [{ type: 'ephemeral', pool: selectedPool }]
       : []
   }, [assignEphemeralIp, selectedPool, control._formValues])
 

From 98e3a88d631763454591e68adccb17e2d209de80 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Fri, 3 May 2024 12:22:56 -0700
Subject: [PATCH 05/18] Move api query to loader

---
 app/forms/instance-create.tsx | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index 96c41caf2c..f7a3ea6835 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -18,7 +18,6 @@ import {
   INSTANCE_MAX_CPU,
   INSTANCE_MAX_RAM_GiB,
   useApiMutation,
-  useApiQuery,
   useApiQueryClient,
   usePrefetchedApiQuery,
   type InstanceCreate,
@@ -148,6 +147,7 @@ CreateInstanceForm.loader = async ({ params }: LoaderFunctionArgs) => {
       query: { project, limit: DISK_FETCH_LIMIT },
     }),
     apiQueryClient.prefetchQuery('currentUserSshKeyList', {}),
+    apiQueryClient.prefetchQuery('projectIpPoolList', { query: { limit: 1000 } }),
   ])
   return null
 }
@@ -191,6 +191,14 @@ export function CreateInstanceForm() {
   const { data: sshKeys } = usePrefetchedApiQuery('currentUserSshKeyList', {})
   const allKeys = useMemo(() => sshKeys.items.map((key) => key.id), [sshKeys])
 
+  const { data: allPools } = usePrefetchedApiQuery('projectIpPoolList', {
+    query: { limit: 1000 },
+  })
+  const defaultPool = useMemo(
+    () => (allPools ? allPools.items.find((p) => p.isDefault)?.name : undefined),
+    [allPools]
+  )
+
   const defaultSource =
     siloImages.length > 0 ? 'siloImage' : projectImages.length > 0 ? 'projectImage' : 'disk'
 
@@ -202,6 +210,7 @@ export function CreateInstanceForm() {
     diskSource: disks?.[0]?.value || '',
     sshPublicKeys: allKeys,
     bootDiskSize: nearest10(defaultImage?.size / GiB),
+    externalIps: [{ type: 'ephemeral', pool: defaultPool }],
   }
 
   const form = useForm({ defaultValues })
@@ -518,7 +527,11 @@ export function CreateInstanceForm() {
       <SshKeysField control={control} isSubmitting={isSubmitting} />
       <FormDivider />
       <Form.Heading id="advanced">Advanced</Form.Heading>
-      <AdvancedAccordion control={control} isSubmitting={isSubmitting} />
+      <AdvancedAccordion
+        control={control}
+        isSubmitting={isSubmitting}
+        allPools={allPools.items}
+      />
       <Form.Actions>
         <Form.Submit loading={createInstance.isPending}>Create instance</Form.Submit>
         <Form.Cancel onClick={() => navigate(pb.instances({ project }))} />
@@ -530,23 +543,20 @@ export function CreateInstanceForm() {
 const AdvancedAccordion = ({
   control,
   isSubmitting,
+  allPools,
 }: {
   control: Control<InstanceCreateInput>
   isSubmitting: boolean
+  allPools: Array<{ name: string; isDefault: boolean }>
 }) => {
   // we track this state manually for the sole reason that we need to be able to
   // tell, inside AccordionItem, when an accordion is opened so we can scroll its
   // contents into view
   const [openItems, setOpenItems] = useState<string[]>([])
-  const { data: allPools } = useApiQuery('projectIpPoolList', { query: { limit: 1000 } })
-  const defaultPool = useMemo(
-    () => (allPools ? allPools.items.find((p) => p.isDefault)?.name : undefined),
-    [allPools]
-  )
 
   const [assignEphemeralIp, setAssignEphemeralIp] = useState(true)
   const [selectedPool, setSelectedPool] = useState<SetStateAction<string | null>>(
-    defaultPool || null
+    (allPools.find((p) => p.isDefault) || allPools[0]).name
   )
 
   useEffect(() => {
@@ -555,12 +565,6 @@ const AdvancedAccordion = ({
       : []
   }, [assignEphemeralIp, selectedPool, control._formValues])
 
-  useEffect(() => {
-    if (defaultPool) {
-      setSelectedPool(defaultPool)
-    }
-  }, [defaultPool])
-
   return (
     <Accordion.Root
       type="multiple"
@@ -602,7 +606,7 @@ const AdvancedAccordion = ({
           // this cannot be right
           selected={`${selectedPool}`}
           items={
-            allPools?.items.map((pool) =>
+            allPools.map((pool) =>
               // according to the designs, we need to indicate the default pool, but we don't have that info pulled in yet
               ({
                 label: `${pool.name}${pool.isDefault ? ' (default)' : ''}`,

From d938d5c4c79c511311e1bea36b6c49805dfda2e3 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Fri, 3 May 2024 15:08:05 -0700
Subject: [PATCH 06/18] move away from using controls

---
 app/forms/instance-create.tsx | 49 ++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index f7a3ea6835..19849a2cf8 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -6,8 +6,8 @@
  * Copyright Oxide Computer Company
  */
 import * as Accordion from '@radix-ui/react-accordion'
-import { useEffect, useMemo, useState, type SetStateAction } from 'react'
-import { useWatch, type Control } from 'react-hook-form'
+import { useEffect, useMemo, useState } from 'react'
+import { useController, useWatch, type Control } from 'react-hook-form'
 import { useNavigate, type LoaderFunctionArgs } from 'react-router-dom'
 import type { SetRequired } from 'type-fest'
 
@@ -553,17 +553,10 @@ const AdvancedAccordion = ({
   // tell, inside AccordionItem, when an accordion is opened so we can scroll its
   // contents into view
   const [openItems, setOpenItems] = useState<string[]>([])
-
-  const [assignEphemeralIp, setAssignEphemeralIp] = useState(true)
-  const [selectedPool, setSelectedPool] = useState<SetStateAction<string | null>>(
-    (allPools.find((p) => p.isDefault) || allPools[0]).name
-  )
-
-  useEffect(() => {
-    control._formValues.externalIps = assignEphemeralIp
-      ? [{ type: 'ephemeral', pool: selectedPool }]
-      : []
-  }, [assignEphemeralIp, selectedPool, control._formValues])
+  const externalIps = useController({ control, name: 'externalIps' })
+  const ephemeralIp = externalIps.field.value?.find((ip) => ip.type === 'ephemeral')
+  const assignEphemeralIp = !!ephemeralIp
+  const selectedPool = ephemeralIp && 'pool' in ephemeralIp ? ephemeralIp.pool : undefined
 
   return (
     <Accordion.Root
@@ -589,11 +582,18 @@ const AdvancedAccordion = ({
         <div className="max-w-lg space-y-2">
           <h2>External IP</h2>
           <div className="flex items-center gap-2.5">
-            {/* This is currently set as a checkbox, but I'm thinking it should be a radio button, with the options being "ephemeral" or "floating" */}
             <Checkbox
               id="assignEphemeralIp"
               checked={assignEphemeralIp}
-              onChange={() => setAssignEphemeralIp(!assignEphemeralIp)}
+              onChange={() => {
+                const newExternalIps = assignEphemeralIp
+                  ? externalIps.field.value?.filter((ip) => ip.type !== 'ephemeral')
+                  : [
+                      ...(externalIps.field.value || []),
+                      { type: 'ephemeral', pool: selectedPool },
+                    ]
+                externalIps.field.onChange(newExternalIps)
+              }}
             />
             <label htmlFor="assignEphemeralIp" className="text-sans-md text-secondary">
               Assign an ephemeral IP address
@@ -603,20 +603,21 @@ const AdvancedAccordion = ({
         <Listbox
           name="pools"
           label="Assign ephemeral IP from pool"
-          // this cannot be right
           selected={`${selectedPool}`}
           items={
-            allPools.map((pool) =>
-              // according to the designs, we need to indicate the default pool, but we don't have that info pulled in yet
-              ({
-                label: `${pool.name}${pool.isDefault ? ' (default)' : ''}`,
-                value: pool.name,
-              })
-            ) || []
+            allPools.map((pool) => ({
+              label: `${pool.name}${pool.isDefault ? ' (default)' : ''}`,
+              value: pool.name,
+            })) || []
           }
           disabled={!assignEphemeralIp || isSubmitting}
           required
-          onChange={(value) => setSelectedPool(value)}
+          onChange={(value) => {
+            const newExternalIps = externalIps.field.value?.map((ip) =>
+              ip.type === 'ephemeral' ? { ...ip, pool: value } : ip
+            )
+            externalIps.field.onChange(newExternalIps)
+          }}
         />
       </AccordionItem>
       <AccordionItem

From 83e4da5f334da5292d387df0c2e34fe14dd90213 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Fri, 3 May 2024 17:04:18 -0700
Subject: [PATCH 07/18] Update tests

---
 app/forms/instance-create.tsx   | 45 +++++++++++++++++----------------
 test/e2e/instance-create.e2e.ts | 16 ++++++++++++
 2 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index 19849a2cf8..e783c4b089 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -557,6 +557,7 @@ const AdvancedAccordion = ({
   const ephemeralIp = externalIps.field.value?.find((ip) => ip.type === 'ephemeral')
   const assignEphemeralIp = !!ephemeralIp
   const selectedPool = ephemeralIp && 'pool' in ephemeralIp ? ephemeralIp.pool : undefined
+  const defaultPool = allPools.find((pool) => pool.isDefault)?.name
 
   return (
     <Accordion.Root
@@ -580,7 +581,7 @@ const AdvancedAccordion = ({
         />
 
         <div className="max-w-lg space-y-2">
-          <h2>External IP</h2>
+          <h2 className="text-sans-md">External IP</h2>
           <div className="flex items-center gap-2.5">
             <Checkbox
               id="assignEphemeralIp"
@@ -590,35 +591,35 @@ const AdvancedAccordion = ({
                   ? externalIps.field.value?.filter((ip) => ip.type !== 'ephemeral')
                   : [
                       ...(externalIps.field.value || []),
-                      { type: 'ephemeral', pool: selectedPool },
+                      { type: 'ephemeral', pool: selectedPool || defaultPool },
                     ]
                 externalIps.field.onChange(newExternalIps)
               }}
             />
-            <label htmlFor="assignEphemeralIp" className="text-sans-md text-secondary">
+            <label htmlFor="assignEphemeralIp" className="text-sans-md">
               Assign an ephemeral IP address
             </label>
           </div>
+          <Listbox
+            name="pools"
+            label="Assign ephemeral IP from pool"
+            selected={`${allPools.find((pool) => pool.name === selectedPool)?.name}`}
+            items={
+              allPools.map((pool) => ({
+                label: `${pool.name}${pool.isDefault ? ' (default)' : ''}`,
+                value: pool.name,
+              })) || []
+            }
+            disabled={!assignEphemeralIp || isSubmitting}
+            required
+            onChange={(value) => {
+              const newExternalIps = externalIps.field.value?.map((ip) =>
+                ip.type === 'ephemeral' ? { ...ip, pool: value } : ip
+              )
+              externalIps.field.onChange(newExternalIps)
+            }}
+          />
         </div>
-        <Listbox
-          name="pools"
-          label="Assign ephemeral IP from pool"
-          selected={`${selectedPool}`}
-          items={
-            allPools.map((pool) => ({
-              label: `${pool.name}${pool.isDefault ? ' (default)' : ''}`,
-              value: pool.name,
-            })) || []
-          }
-          disabled={!assignEphemeralIp || isSubmitting}
-          required
-          onChange={(value) => {
-            const newExternalIps = externalIps.field.value?.map((ip) =>
-              ip.type === 'ephemeral' ? { ...ip, pool: value } : ip
-            )
-            externalIps.field.onChange(newExternalIps)
-          }}
-        />
       </AccordionItem>
       <AccordionItem
         value="configuration"
diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts
index 13b82d208b..9353f2da0b 100644
--- a/test/e2e/instance-create.e2e.ts
+++ b/test/e2e/instance-create.e2e.ts
@@ -51,6 +51,22 @@ test('can create an instance', async ({ page }) => {
   await page.getByRole('button', { name: 'Networking' }).click()
   await page.getByRole('button', { name: 'Configuration' }).click()
 
+  // verify that the ip pool selector is visible and default is selected
+  await expect(
+    page.getByRole('checkbox', { name: 'Assign an ephemeral IP address' })
+  ).toBeChecked()
+  await page.getByRole('button', { name: 'ip-pool-1 (default)' }).click()
+  await expect(page.getByRole('option', { name: 'ip-pool-1 (default)' })).toBeEnabled()
+
+  // unchecking the box should disable the selector
+  await page.getByRole('checkbox', { name: 'Assign an ephemeral IP address' }).uncheck()
+  await expect(page.getByRole('button', { name: 'Select an option' })).toBeDisabled()
+
+  // re-checking the box should re-enable the selector, and other options should be selectable
+  await page.getByRole('checkbox', { name: 'Assign an ephemeral IP address' }).check()
+  await page.getByRole('button', { name: 'ip-pool-1 (default)' }).click()
+  await page.getByRole('option', { name: 'ip-pool-2' }).click()
+
   // should be visible in accordion
   await expectVisible(page, [
     'role=radiogroup[name="Network interface"]',

From b944bb68535b1ae3823035811bc0fb9332f6b205 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 6 May 2024 14:59:17 -0700
Subject: [PATCH 08/18] Update tests

---
 app/forms/instance-create.tsx   | 10 ++++++++--
 test/e2e/instance-create.e2e.ts | 21 +++++++++++++--------
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index e783c4b089..f9f3860a1b 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -558,6 +558,7 @@ const AdvancedAccordion = ({
   const assignEphemeralIp = !!ephemeralIp
   const selectedPool = ephemeralIp && 'pool' in ephemeralIp ? ephemeralIp.pool : undefined
   const defaultPool = allPools.find((pool) => pool.isDefault)?.name
+  const defaultPoolLabel = defaultPool ? `${defaultPool} (default)` : 'No pools available'
 
   return (
     <Accordion.Root
@@ -596,13 +597,18 @@ const AdvancedAccordion = ({
                 externalIps.field.onChange(newExternalIps)
               }}
             />
-            <label htmlFor="assignEphemeralIp" className="text-sans-md">
+            <label
+              htmlFor="assignEphemeralIp"
+              className="text-sans-md"
+              aria-label="Assign an ephemeral IP address"
+            >
               Assign an ephemeral IP address
             </label>
           </div>
           <Listbox
             name="pools"
-            label="Assign ephemeral IP from pool"
+            aria-label="Assign ephemeral IP from pool"
+            placeholder={defaultPoolLabel}
             selected={`${allPools.find((pool) => pool.name === selectedPool)?.name}`}
             items={
               allPools.map((pool) => ({
diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts
index 9353f2da0b..980df8ae3f 100644
--- a/test/e2e/instance-create.e2e.ts
+++ b/test/e2e/instance-create.e2e.ts
@@ -51,20 +51,25 @@ test('can create an instance', async ({ page }) => {
   await page.getByRole('button', { name: 'Networking' }).click()
   await page.getByRole('button', { name: 'Configuration' }).click()
 
+  const assignEphemeralIpCheckbox = page.getByRole('checkbox', {
+    name: 'Assign an ephemeral IP address',
+  })
+  const assignEphemeralIpButton = page.getByRole('button', {
+    name: 'Assign ephemeral IP from pool',
+  })
+
   // verify that the ip pool selector is visible and default is selected
-  await expect(
-    page.getByRole('checkbox', { name: 'Assign an ephemeral IP address' })
-  ).toBeChecked()
-  await page.getByRole('button', { name: 'ip-pool-1 (default)' }).click()
+  await expect(assignEphemeralIpCheckbox).toBeChecked()
+  await assignEphemeralIpButton.click()
   await expect(page.getByRole('option', { name: 'ip-pool-1 (default)' })).toBeEnabled()
 
   // unchecking the box should disable the selector
-  await page.getByRole('checkbox', { name: 'Assign an ephemeral IP address' }).uncheck()
-  await expect(page.getByRole('button', { name: 'Select an option' })).toBeDisabled()
+  await assignEphemeralIpCheckbox.uncheck()
+  await expect(assignEphemeralIpButton).toBeDisabled()
 
   // re-checking the box should re-enable the selector, and other options should be selectable
-  await page.getByRole('checkbox', { name: 'Assign an ephemeral IP address' }).check()
-  await page.getByRole('button', { name: 'ip-pool-1 (default)' }).click()
+  await assignEphemeralIpCheckbox.check()
+  await assignEphemeralIpButton.click()
   await page.getByRole('option', { name: 'ip-pool-2' }).click()
 
   // should be visible in accordion

From 0440d039c2069611af2333fbba910531285358d7 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Mon, 6 May 2024 17:54:48 -0700
Subject: [PATCH 09/18] copy update

---
 app/forms/instance-create.tsx | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index f9f3860a1b..0e5192751d 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -558,7 +558,6 @@ const AdvancedAccordion = ({
   const assignEphemeralIp = !!ephemeralIp
   const selectedPool = ephemeralIp && 'pool' in ephemeralIp ? ephemeralIp.pool : undefined
   const defaultPool = allPools.find((pool) => pool.isDefault)?.name
-  const defaultPoolLabel = defaultPool ? `${defaultPool} (default)` : 'No pools available'
 
   return (
     <Accordion.Root
@@ -608,7 +607,7 @@ const AdvancedAccordion = ({
           <Listbox
             name="pools"
             aria-label="Assign ephemeral IP from pool"
-            placeholder={defaultPoolLabel}
+            placeholder={defaultPool ? `${defaultPool} (default)` : 'Select pool'}
             selected={`${allPools.find((pool) => pool.name === selectedPool)?.name}`}
             items={
               allPools.map((pool) => ({

From e8e04f017de98be322ce409c0fba8f82b4d72c2f Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 7 May 2024 11:24:56 -0700
Subject: [PATCH 10/18] update to use badge in dropdown; add description
 microcopy

---
 app/components/AccordionItem.tsx |  2 +-
 app/forms/instance-create.tsx    | 79 +++++++++++++++++++++-----------
 app/util/links.ts                |  2 +
 3 files changed, 54 insertions(+), 29 deletions(-)

diff --git a/app/components/AccordionItem.tsx b/app/components/AccordionItem.tsx
index 8bf2365120..227d8583ae 100644
--- a/app/components/AccordionItem.tsx
+++ b/app/components/AccordionItem.tsx
@@ -37,7 +37,7 @@ export const AccordionItem = ({ children, isOpen, label, value }: AccordionItemP
       <Accordion.Content
         ref={contentRef}
         forceMount
-        className={cn('ox-accordion-content overflow-hidden py-8', { hidden: !isOpen })}
+        className={cn('ox-accordion-content py-8', { hidden: !isOpen })}
       >
         {children}
       </Accordion.Content>
diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index 0e5192751d..9a4a16952f 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -30,6 +30,7 @@ import {
 } from '@oxide/design-system/icons/react'
 
 import { AccordionItem } from '~/components/AccordionItem'
+import { ExternalLink } from '~/components/ExternalLink'
 import { CheckboxField } from '~/components/form/fields/CheckboxField'
 import { DescriptionField } from '~/components/form/fields/DescriptionField'
 import { DiskSizeField } from '~/components/form/fields/DiskSizeField'
@@ -50,6 +51,7 @@ import { Form } from '~/components/form/Form'
 import { FullPageForm } from '~/components/form/FullPageForm'
 import { getProjectSelector, useForm, useProjectSelector } from '~/hooks'
 import { addToast } from '~/stores/toast'
+import { Badge } from '~/ui/lib/Badge'
 import { Checkbox } from '~/ui/lib/Checkbox'
 import { FormDivider } from '~/ui/lib/Divider'
 import { EmptyMessage } from '~/ui/lib/EmptyMessage'
@@ -581,8 +583,17 @@ const AdvancedAccordion = ({
         />
 
         <div className="max-w-lg space-y-2">
-          <h2 className="text-sans-md">External IP</h2>
-          <div className="flex items-center gap-2.5">
+          <h2 className="text-sans-md">
+            Ephemeral IP{' '}
+            <TextInputHint id="ephemeral-ip-description" className="mb-2">
+              Ephemeral IPs are non-permanent, randomly-assigned addresses, dynamically
+              allocated from a pool of IPs when the instance is created.{' '}
+              <ExternalLink href={links.externalAddresses}>
+                Learn more about ephemeral IPs.
+              </ExternalLink>
+            </TextInputHint>
+          </h2>
+          <div className="flex items-start gap-2.5">
             <Checkbox
               id="assignEphemeralIp"
               checked={assignEphemeralIp}
@@ -596,34 +607,46 @@ const AdvancedAccordion = ({
                 externalIps.field.onChange(newExternalIps)
               }}
             />
-            <label
-              htmlFor="assignEphemeralIp"
-              className="text-sans-md"
-              aria-label="Assign an ephemeral IP address"
-            >
-              Assign an ephemeral IP address
+            <label htmlFor="assignEphemeralIp" className="text-sans-md">
+              Automatically assign an ephemeral IP address
             </label>
           </div>
-          <Listbox
-            name="pools"
-            aria-label="Assign ephemeral IP from pool"
-            placeholder={defaultPool ? `${defaultPool} (default)` : 'Select pool'}
-            selected={`${allPools.find((pool) => pool.name === selectedPool)?.name}`}
-            items={
-              allPools.map((pool) => ({
-                label: `${pool.name}${pool.isDefault ? ' (default)' : ''}`,
-                value: pool.name,
-              })) || []
-            }
-            disabled={!assignEphemeralIp || isSubmitting}
-            required
-            onChange={(value) => {
-              const newExternalIps = externalIps.field.value?.map((ip) =>
-                ip.type === 'ephemeral' ? { ...ip, pool: value } : ip
-              )
-              externalIps.field.onChange(newExternalIps)
-            }}
-          />
+          {assignEphemeralIp && (
+            <Listbox
+              className="pt-1"
+              name="pools"
+              aria-label="ephemeral IP pool"
+              placeholder={defaultPool ? `${defaultPool} (default)` : 'Select pool'}
+              selected={`${allPools.find((pool) => pool.name === selectedPool)?.name}`}
+              items={
+                allPools.map((pool) => ({
+                  label: (
+                    <div className="flex items-center gap-1">
+                      {pool.name}
+                      {pool.isDefault && (
+                        <Badge
+                          color={selectedPool === pool.name ? 'default' : 'neutral'}
+                          variant={selectedPool === pool.name ? 'solid' : 'default'}
+                        >
+                          default
+                        </Badge>
+                      )}
+                    </div>
+                  ),
+                  labelString: pool.name,
+                  value: pool.name,
+                })) || []
+              }
+              disabled={!assignEphemeralIp || isSubmitting}
+              required
+              onChange={(value) => {
+                const newExternalIps = externalIps.field.value?.map((ip) =>
+                  ip.type === 'ephemeral' ? { ...ip, pool: value } : ip
+                )
+                externalIps.field.onChange(newExternalIps)
+              }}
+            />
+          )}
         </div>
       </AccordionItem>
       <AccordionItem
diff --git a/app/util/links.ts b/app/util/links.ts
index 9fa5cabbae..b9b5a4298a 100644
--- a/app/util/links.ts
+++ b/app/util/links.ts
@@ -10,6 +10,8 @@ export const links: Record<string, string> = {
   cloudInitFormat: 'https://cloudinit.readthedocs.io/en/latest/explanation/format.html',
   cloudInitExamples: 'https://cloudinit.readthedocs.io/en/latest/reference/examples.html',
   disksDocs: 'https://docs.oxide.computer/guides/managing-disks-and-snapshots',
+  externalAddresses:
+    'https://docs.oxide.computer/guides/operator/ip-pool-management#_external_address_categorization',
   firewallRulesDocs:
     'https://docs.oxide.computer/guides/configuring-guest-networking#_firewall_rules',
   floatingIpsDocs: 'https://docs.oxide.computer/guides/managing-floating-ips',

From 139dad685890e3e7c3f03095214266b65efbbc5b Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 7 May 2024 11:29:04 -0700
Subject: [PATCH 11/18] Fix tests

---
 app/forms/instance-create.tsx   | 4 +---
 test/e2e/instance-create.e2e.ts | 8 ++++----
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index 9a4a16952f..da108d068e 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -588,9 +588,7 @@ const AdvancedAccordion = ({
             <TextInputHint id="ephemeral-ip-description" className="mb-2">
               Ephemeral IPs are non-permanent, randomly-assigned addresses, dynamically
               allocated from a pool of IPs when the instance is created.{' '}
-              <ExternalLink href={links.externalAddresses}>
-                Learn more about ephemeral IPs.
-              </ExternalLink>
+              <ExternalLink href={links.externalAddresses}>Learn more.</ExternalLink>
             </TextInputHint>
           </h2>
           <div className="flex items-start gap-2.5">
diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts
index 980df8ae3f..d33a743635 100644
--- a/test/e2e/instance-create.e2e.ts
+++ b/test/e2e/instance-create.e2e.ts
@@ -52,20 +52,20 @@ test('can create an instance', async ({ page }) => {
   await page.getByRole('button', { name: 'Configuration' }).click()
 
   const assignEphemeralIpCheckbox = page.getByRole('checkbox', {
-    name: 'Assign an ephemeral IP address',
+    name: 'Automatically assign an ephemeral IP address',
   })
   const assignEphemeralIpButton = page.getByRole('button', {
-    name: 'Assign ephemeral IP from pool',
+    name: 'ephemeral IP pool',
   })
 
   // verify that the ip pool selector is visible and default is selected
   await expect(assignEphemeralIpCheckbox).toBeChecked()
   await assignEphemeralIpButton.click()
-  await expect(page.getByRole('option', { name: 'ip-pool-1 (default)' })).toBeEnabled()
+  await expect(page.getByRole('option', { name: 'ip-pool-1' })).toBeEnabled()
 
   // unchecking the box should disable the selector
   await assignEphemeralIpCheckbox.uncheck()
-  await expect(assignEphemeralIpButton).toBeDisabled()
+  await expect(assignEphemeralIpButton).toBeHidden()
 
   // re-checking the box should re-enable the selector, and other options should be selectable
   await assignEphemeralIpCheckbox.check()

From 05e577d313a9cae6bd8e97a353cb824d98517d6f Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 7 May 2024 17:33:58 -0700
Subject: [PATCH 12/18] Incorporate badge in select button

---
 app/forms/instance-create.tsx | 26 ++++++++++----------------
 app/ui/lib/Listbox.tsx        | 12 +++---------
 2 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index da108d068e..38d9dc5e79 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -561,6 +561,13 @@ const AdvancedAccordion = ({
   const selectedPool = ephemeralIp && 'pool' in ephemeralIp ? ephemeralIp.pool : undefined
   const defaultPool = allPools.find((pool) => pool.isDefault)?.name
 
+  const poolLabel = (pool: { name: string; isDefault: boolean }) => (
+    <div className="flex items-center gap-1">
+      {pool.name}
+      {pool.isDefault && <Badge>default</Badge>}
+    </div>
+  )
+
   return (
     <Accordion.Root
       type="multiple"
@@ -586,8 +593,8 @@ const AdvancedAccordion = ({
           <h2 className="text-sans-md">
             Ephemeral IP{' '}
             <TextInputHint id="ephemeral-ip-description" className="mb-2">
-              Ephemeral IPs are non-permanent, randomly-assigned addresses, dynamically
-              allocated from a pool of IPs when the instance is created.{' '}
+              Ephemeral IPs are allocated when the instance is created and deallocated when
+              it is deleted.{' '}
               <ExternalLink href={links.externalAddresses}>Learn more.</ExternalLink>
             </TextInputHint>
           </h2>
@@ -618,20 +625,7 @@ const AdvancedAccordion = ({
               selected={`${allPools.find((pool) => pool.name === selectedPool)?.name}`}
               items={
                 allPools.map((pool) => ({
-                  label: (
-                    <div className="flex items-center gap-1">
-                      {pool.name}
-                      {pool.isDefault && (
-                        <Badge
-                          color={selectedPool === pool.name ? 'default' : 'neutral'}
-                          variant={selectedPool === pool.name ? 'solid' : 'default'}
-                        >
-                          default
-                        </Badge>
-                      )}
-                    </div>
-                  ),
-                  labelString: pool.name,
+                  label: poolLabel(pool),
                   value: pool.name,
                 })) || []
               }
diff --git a/app/ui/lib/Listbox.tsx b/app/ui/lib/Listbox.tsx
index 81c622aea1..f61c4aadcd 100644
--- a/app/ui/lib/Listbox.tsx
+++ b/app/ui/lib/Listbox.tsx
@@ -26,13 +26,7 @@ import { TextInputHint } from './TextInput'
 
 export type ListboxItem<Value extends string = string> = {
   value: Value
-} & (
-  | { label: string; labelString?: never }
-  // labelString is required when `label` is a `ReactElement` because we
-  // need need a one-line string to display in the button when the item is
-  // selected.
-  | { label: ReactNode; labelString: string }
-)
+} & { label?: string | ReactNode; labelString?: string }
 
 export interface ListboxProps<Value extends string = string> {
   // null is allowed as a default empty value, but onChange will never be called with null
@@ -44,9 +38,9 @@ export interface ListboxProps<Value extends string = string> {
   disabled?: boolean
   hasError?: boolean
   name?: string
-  label?: string
+  label?: React.ReactNode
   tooltipText?: string
-  description?: string | React.ReactNode
+  description?: React.ReactNode
   required?: boolean
   isLoading?: boolean
 }

From c2c04869da94083b78e40905ffba9830de5ae592 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 7 May 2024 17:34:37 -0700
Subject: [PATCH 13/18] slightly more space

---
 app/forms/instance-create.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index 38d9dc5e79..a32fb9a16f 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -562,7 +562,7 @@ const AdvancedAccordion = ({
   const defaultPool = allPools.find((pool) => pool.isDefault)?.name
 
   const poolLabel = (pool: { name: string; isDefault: boolean }) => (
-    <div className="flex items-center gap-1">
+    <div className="flex items-center gap-2">
       {pool.name}
       {pool.isDefault && <Badge>default</Badge>}
     </div>

From 6d2f8203050d4d3ff43c3f471c42d1c488500d69 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Tue, 7 May 2024 21:51:18 -0700
Subject: [PATCH 14/18] add label back in for listbox

---
 app/forms/instance-create.tsx   | 13 ++++++-------
 test/e2e/instance-create.e2e.ts |  4 ++--
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index a32fb9a16f..85bf7ba7cc 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -30,7 +30,6 @@ import {
 } from '@oxide/design-system/icons/react'
 
 import { AccordionItem } from '~/components/AccordionItem'
-import { ExternalLink } from '~/components/ExternalLink'
 import { CheckboxField } from '~/components/form/fields/CheckboxField'
 import { DescriptionField } from '~/components/form/fields/DescriptionField'
 import { DiskSizeField } from '~/components/form/fields/DiskSizeField'
@@ -60,6 +59,7 @@ import { Message } from '~/ui/lib/Message'
 import { RadioCard } from '~/ui/lib/Radio'
 import { Tabs } from '~/ui/lib/Tabs'
 import { TextInputHint } from '~/ui/lib/TextInput'
+import { TipIcon } from '~/ui/lib/TipIcon'
 import { readBlobAsBase64 } from '~/util/file'
 import { links } from '~/util/links'
 import { nearest10 } from '~/util/math'
@@ -592,11 +592,10 @@ const AdvancedAccordion = ({
         <div className="max-w-lg space-y-2">
           <h2 className="text-sans-md">
             Ephemeral IP{' '}
-            <TextInputHint id="ephemeral-ip-description" className="mb-2">
+            <TipIcon>
               Ephemeral IPs are allocated when the instance is created and deallocated when
-              it is deleted.{' '}
-              <ExternalLink href={links.externalAddresses}>Learn more.</ExternalLink>
-            </TextInputHint>
+              it is deleted
+            </TipIcon>
           </h2>
           <div className="flex items-start gap-2.5">
             <Checkbox
@@ -613,14 +612,14 @@ const AdvancedAccordion = ({
               }}
             />
             <label htmlFor="assignEphemeralIp" className="text-sans-md">
-              Automatically assign an ephemeral IP address
+              Allocate and attach an ephemeral IP address
             </label>
           </div>
           {assignEphemeralIp && (
             <Listbox
               className="pt-1"
               name="pools"
-              aria-label="ephemeral IP pool"
+              label="IP pool for ephemeral IP"
               placeholder={defaultPool ? `${defaultPool} (default)` : 'Select pool'}
               selected={`${allPools.find((pool) => pool.name === selectedPool)?.name}`}
               items={
diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts
index d33a743635..da8e27cf6e 100644
--- a/test/e2e/instance-create.e2e.ts
+++ b/test/e2e/instance-create.e2e.ts
@@ -52,10 +52,10 @@ test('can create an instance', async ({ page }) => {
   await page.getByRole('button', { name: 'Configuration' }).click()
 
   const assignEphemeralIpCheckbox = page.getByRole('checkbox', {
-    name: 'Automatically assign an ephemeral IP address',
+    name: 'Allocate and attach an ephemeral IP address',
   })
   const assignEphemeralIpButton = page.getByRole('button', {
-    name: 'ephemeral IP pool',
+    name: 'IP pool for ephemeral IP',
   })
 
   // verify that the ip pool selector is visible and default is selected

From 74c05e65d405fe016b0875ddf73e6fdb1858e950 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Wed, 8 May 2024 09:49:47 -0700
Subject: [PATCH 15/18] New test - no ephemeral IP attached

---
 test/e2e/instance-create.e2e.ts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts
index da8e27cf6e..d8b8ed006d 100644
--- a/test/e2e/instance-create.e2e.ts
+++ b/test/e2e/instance-create.e2e.ts
@@ -314,3 +314,15 @@ test('maintains selected values even when changing tabs', async ({ page }) => {
   // so this checks to make sure that the arch-based image — with ID `bd6aa051…` — was used
   await expectVisible(page, [`text=${instanceName}-bd6aa051`])
 })
+
+test('does not attach an ephemeral IP when the checkbox is unchecked', async ({ page }) => {
+  await page.goto('/projects/mock-project/instances-new')
+  await page.getByRole('textbox', { name: 'Name', exact: true }).fill('no-ephemeral-ip')
+  await page.getByRole('button', { name: 'Networking' }).click()
+  await page
+    .getByRole('checkbox', { name: 'Allocate and attach an ephemeral IP address' })
+    .uncheck()
+  await page.getByRole('button', { name: 'Create instance' }).click()
+  await expect(page).toHaveURL('/projects/mock-project/instances/no-ephemeral-ip/storage')
+  await expect(page.getByText('External IPs—')).toBeVisible()
+})

From c46c1c796c26055eccece354c5d3018548d15eeb Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Wed, 8 May 2024 10:46:33 -0700
Subject: [PATCH 16/18] Slight rename for clarity

---
 app/forms/instance-create.tsx | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index 85bf7ba7cc..11d1686f6b 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -193,12 +193,12 @@ export function CreateInstanceForm() {
   const { data: sshKeys } = usePrefetchedApiQuery('currentUserSshKeyList', {})
   const allKeys = useMemo(() => sshKeys.items.map((key) => key.id), [sshKeys])
 
-  const { data: allPools } = usePrefetchedApiQuery('projectIpPoolList', {
+  const { data: siloPools } = usePrefetchedApiQuery('projectIpPoolList', {
     query: { limit: 1000 },
   })
   const defaultPool = useMemo(
-    () => (allPools ? allPools.items.find((p) => p.isDefault)?.name : undefined),
-    [allPools]
+    () => (siloPools ? siloPools.items.find((p) => p.isDefault)?.name : undefined),
+    [siloPools]
   )
 
   const defaultSource =
@@ -532,7 +532,7 @@ export function CreateInstanceForm() {
       <AdvancedAccordion
         control={control}
         isSubmitting={isSubmitting}
-        allPools={allPools.items}
+        siloPools={siloPools.items}
       />
       <Form.Actions>
         <Form.Submit loading={createInstance.isPending}>Create instance</Form.Submit>
@@ -545,11 +545,11 @@ export function CreateInstanceForm() {
 const AdvancedAccordion = ({
   control,
   isSubmitting,
-  allPools,
+  siloPools,
 }: {
   control: Control<InstanceCreateInput>
   isSubmitting: boolean
-  allPools: Array<{ name: string; isDefault: boolean }>
+  siloPools: Array<{ name: string; isDefault: boolean }>
 }) => {
   // we track this state manually for the sole reason that we need to be able to
   // tell, inside AccordionItem, when an accordion is opened so we can scroll its
@@ -559,7 +559,7 @@ const AdvancedAccordion = ({
   const ephemeralIp = externalIps.field.value?.find((ip) => ip.type === 'ephemeral')
   const assignEphemeralIp = !!ephemeralIp
   const selectedPool = ephemeralIp && 'pool' in ephemeralIp ? ephemeralIp.pool : undefined
-  const defaultPool = allPools.find((pool) => pool.isDefault)?.name
+  const defaultPool = siloPools.find((pool) => pool.isDefault)?.name
 
   const poolLabel = (pool: { name: string; isDefault: boolean }) => (
     <div className="flex items-center gap-2">
@@ -621,9 +621,9 @@ const AdvancedAccordion = ({
               name="pools"
               label="IP pool for ephemeral IP"
               placeholder={defaultPool ? `${defaultPool} (default)` : 'Select pool'}
-              selected={`${allPools.find((pool) => pool.name === selectedPool)?.name}`}
+              selected={`${siloPools.find((pool) => pool.name === selectedPool)?.name}`}
               items={
-                allPools.map((pool) => ({
+                siloPools.map((pool) => ({
                   label: poolLabel(pool),
                   value: pool.name,
                 })) || []

From f9f35118d1fb5852d9829bff4ed1f259d20516c9 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Wed, 8 May 2024 10:46:49 -0700
Subject: [PATCH 17/18] oops; meant to add this comment

---
 app/forms/instance-create.tsx | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index 11d1686f6b..6e78fe5e99 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -193,6 +193,7 @@ export function CreateInstanceForm() {
   const { data: sshKeys } = usePrefetchedApiQuery('currentUserSshKeyList', {})
   const allKeys = useMemo(() => sshKeys.items.map((key) => key.id), [sshKeys])
 
+  // projectIpPoolList fetches the pools linked to the current silo
   const { data: siloPools } = usePrefetchedApiQuery('projectIpPoolList', {
     query: { limit: 1000 },
   })

From 63331170702d835c0d4b45e75c56ed61b50a9a62 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Wed, 8 May 2024 11:35:58 -0700
Subject: [PATCH 18/18] A few spacing tweaks

---
 app/forms/instance-create.tsx | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx
index 6e78fe5e99..9e74ab96da 100644
--- a/app/forms/instance-create.tsx
+++ b/app/forms/instance-create.tsx
@@ -562,13 +562,6 @@ const AdvancedAccordion = ({
   const selectedPool = ephemeralIp && 'pool' in ephemeralIp ? ephemeralIp.pool : undefined
   const defaultPool = siloPools.find((pool) => pool.isDefault)?.name
 
-  const poolLabel = (pool: { name: string; isDefault: boolean }) => (
-    <div className="flex items-center gap-2">
-      {pool.name}
-      {pool.isDefault && <Badge>default</Badge>}
-    </div>
-  )
-
   return (
     <Accordion.Root
       type="multiple"
@@ -583,14 +576,16 @@ const AdvancedAccordion = ({
       >
         <NetworkInterfaceField control={control} disabled={isSubmitting} />
 
-        <TextField
-          name="hostname"
-          tooltipText="Will be generated if not provided"
-          control={control}
-          disabled={isSubmitting}
-        />
+        <div className="py-2">
+          <TextField
+            name="hostname"
+            tooltipText="Will be generated if not provided"
+            control={control}
+            disabled={isSubmitting}
+          />
+        </div>
 
-        <div className="max-w-lg space-y-2">
+        <div className="flex flex-1 flex-col gap-4">
           <h2 className="text-sans-md">
             Ephemeral IP{' '}
             <TipIcon>
@@ -612,20 +607,24 @@ const AdvancedAccordion = ({
                 externalIps.field.onChange(newExternalIps)
               }}
             />
-            <label htmlFor="assignEphemeralIp" className="text-sans-md">
+            <label htmlFor="assignEphemeralIp" className="text-sans-md text-secondary">
               Allocate and attach an ephemeral IP address
             </label>
           </div>
           {assignEphemeralIp && (
             <Listbox
-              className="pt-1"
               name="pools"
               label="IP pool for ephemeral IP"
               placeholder={defaultPool ? `${defaultPool} (default)` : 'Select pool'}
               selected={`${siloPools.find((pool) => pool.name === selectedPool)?.name}`}
               items={
                 siloPools.map((pool) => ({
-                  label: poolLabel(pool),
+                  label: (
+                    <div className="flex items-center gap-2">
+                      {pool.name}
+                      {pool.isDefault && <Badge>default</Badge>}
+                    </div>
+                  ),
                   value: pool.name,
                 })) || []
               }