Poll instance state when instance is transitioning (#2360)

* poll instance state on instance detail if instance is transitioning * make instance list polling not do weirdo stuff to the row actions menu * poll on the serial console page and try to connect * show a fun little message while auto-refreshing * Revert "poll on the serial console page and try to connect" 7f5c752 * don't poll on list view, use spinner with tooltip as loading indicator * e2es caught a real bug! * fix overzealous dep arrays * try to make serial test less flaky
oxidecomputer · Aug 16, 2024 · 33b7a50 · 33b7a50
1 parent 1a2cb52
commit 33b7a50
Show file tree

Hide file tree

Showing 8 changed files with 110 additions and 61 deletions.
diff --git a/app/api/util.ts b/app/api/util.ts
@@ -108,11 +108,20 @@ const instanceActions: Record<string, InstanceState[]> = {
 // while also making the states available directly
 
 export const instanceCan = R.mapValues(instanceActions, (states) => {
-  const test = (i: Instance) => states.includes(i.runState)
+  const test = (i: { runState: InstanceState }) => states.includes(i.runState)
   test.states = states
   return test
 })
 
+export function instanceTransitioning({ runState }: Instance) {
+  return (
+    runState === 'creating' ||
+    runState === 'starting' ||
+    runState === 'stopping' ||
+    runState === 'rebooting'
+  )
+}
+
 const diskActions: Record<string, DiskState['state'][]> = {
   // https://github.com/oxidecomputer/omicron/blob/4970c71e/nexus/db-queries/src/db/datastore/disk.rs#L578-L582.
   delete: ['detached', 'creating', 'faulted'],

diff --git a/app/pages/project/instances/actions.tsx b/app/pages/project/instances/actions.tsx
@@ -29,29 +29,35 @@ type Options = {
 }
 
 export const useMakeInstanceActions = (
-  projectSelector: { project: string },
+  { project }: { project: string },
   options: Options = {}
 ): MakeActions<Instance> => {
   const navigate = useNavigate()
 
   // if you also pass onSuccess to mutate(), this one is not overridden — this
-  // one runs first, then the one passed to mutate()
+  // one runs first, then the one passed to mutate().
+  //
+  // We pull out the mutate functions because they are referentially stable,
+  // while the whole useMutation result object is not. The async ones are used
+  // when we need to confirm because the confirm modals want that.
   const opts = { onSuccess: options.onSuccess }
-  const startInstance = useApiMutation('instanceStart', opts)
-  const stopInstance = useApiMutation('instanceStop', opts)
-  const rebootInstance = useApiMutation('instanceReboot', opts)
+  const { mutate: startInstance } = useApiMutation('instanceStart', opts)
+  const { mutateAsync: stopInstanceAsync } = useApiMutation('instanceStop', opts)
+  const { mutate: rebootInstance } = useApiMutation('instanceReboot', opts)
   // delete has its own
-  const deleteInstance = useApiMutation('instanceDelete', { onSuccess: options.onDelete })
+  const { mutateAsync: deleteInstanceAsync } = useApiMutation('instanceDelete', {
+    onSuccess: options.onDelete,
+  })
 
   return useCallback(
     (instance) => {
-      const instanceSelector = { ...projectSelector, instance: instance.name }
-      const instanceParams = { path: { instance: instance.name }, query: projectSelector }
+      const instanceSelector = { project, instance: instance.name }
+      const instanceParams = { path: { instance: instance.name }, query: { project } }
       return [
         {
           label: 'Start',
           onActivate() {
-            startInstance.mutate(instanceParams, {
+            startInstance(instanceParams, {
               onSuccess: () => addToast({ title: `Starting instance '${instance.name}'` }),
               onError: (error) =>
                 addToast({
@@ -71,7 +77,7 @@ export const useMakeInstanceActions = (
             confirmAction({
               actionType: 'danger',
               doAction: () =>
-                stopInstance.mutateAsync(instanceParams, {
+                stopInstanceAsync(instanceParams, {
                   onSuccess: () =>
                     addToast({ title: `Stopping instance '${instance.name}'` }),
                 }),
@@ -93,7 +99,7 @@ export const useMakeInstanceActions = (
         {
           label: 'Reboot',
           onActivate() {
-            rebootInstance.mutate(instanceParams, {
+            rebootInstance(instanceParams, {
               onSuccess: () => addToast({ title: `Rebooting instance '${instance.name}'` }),
               onError: (error) =>
                 addToast({
@@ -117,7 +123,7 @@ export const useMakeInstanceActions = (
           label: 'Delete',
           onActivate: confirmDelete({
             doDelete: () =>
-              deleteInstance.mutateAsync(instanceParams, {
+              deleteInstanceAsync(instanceParams, {
                 onSuccess: () =>
                   addToast({ title: `Deleting instance '${instance.name}'` }),
               }),
@@ -132,6 +138,13 @@ export const useMakeInstanceActions = (
         },
       ]
     },
-    [projectSelector, deleteInstance, navigate, rebootInstance, startInstance, stopInstance]
+    [
+      project,
+      navigate,
+      deleteInstanceAsync,
+      rebootInstance,
+      startInstance,
+      stopInstanceAsync,
+    ]
   )
 }
diff --git a/app/pages/project/instances/instance/InstancePage.tsx b/app/pages/project/instances/instance/InstancePage.tsx
@@ -17,6 +17,7 @@ import {
 } from '@oxide/api'
 import { Instances16Icon, Instances24Icon } from '@oxide/design-system/icons/react'
 
+import { instanceTransitioning } from '~/api/util'
 import { DocsPopover } from '~/components/DocsPopover'
 import { ExternalIps } from '~/components/ExternalIps'
 import { MoreActionsMenu } from '~/components/MoreActionsMenu'
@@ -28,6 +29,8 @@ import { EmptyCell } from '~/table/cells/EmptyCell'
 import { DateTime } from '~/ui/lib/DateTime'
 import { PageHeader, PageTitle } from '~/ui/lib/PageHeader'
 import { PropertiesTable } from '~/ui/lib/PropertiesTable'
+import { Spinner } from '~/ui/lib/Spinner'
+import { Tooltip } from '~/ui/lib/Tooltip'
 import { Truncate } from '~/ui/lib/Truncate'
 import { docLinks } from '~/util/links'
 import { pb } from '~/util/path-builder'
@@ -94,10 +97,19 @@ export function InstancePage() {
     onDelete: () => navigate(pb.instances(instanceSelector)),
   })
 
-  const { data: instance } = usePrefetchedApiQuery('instanceView', {
-    path: { instance: instanceSelector.instance },
-    query: { project: instanceSelector.project },
-  })
+  const { data: instance } = usePrefetchedApiQuery(
+    'instanceView',
+    {
+      path: { instance: instanceSelector.instance },
+      query: { project: instanceSelector.project },
+    },
+    {
+      refetchInterval: ({ state: { data: instance } }) =>
+        instance && instanceTransitioning(instance) ? 1000 : false,
+    }
+  )
+
+  const polling = instanceTransitioning(instance)
 
   const { data: nics } = usePrefetchedApiQuery('instanceNetworkInterfaceList', {
     query: {
@@ -157,7 +169,16 @@ export function InstancePage() {
             <span className="ml-1 text-quaternary"> {memory.unit}</span>
           </PropertiesTable.Row>
           <PropertiesTable.Row label="status">
-            <InstanceStatusBadge status={instance.runState} />
+            <div className="flex">
+              <InstanceStatusBadge status={instance.runState} />
+              {polling && (
+                <Tooltip content="Auto-refreshing while state changes" delay={150}>
+                  <button type="button">
+                    <Spinner className="ml-2" />
+                  </button>
+                </Tooltip>
+              )}
+            </div>
           </PropertiesTable.Row>
           <PropertiesTable.Row label="vpc">
             {vpc ? (

diff --git a/app/pages/project/instances/instance/tabs/NetworkingTab.tsx b/app/pages/project/instances/instance/tabs/NetworkingTab.tsx
@@ -133,6 +133,31 @@ const staticCols = [
 
 const updateNicStates = fancifyStates(instanceCan.updateNic.states)
 
+const ipColHelper = createColumnHelper<ExternalIp>()
+const staticIpCols = [
+  ipColHelper.accessor('ip', {
+    cell: (info) => <CopyableIp ip={info.getValue()} />,
+  }),
+  ipColHelper.accessor('kind', {
+    header: () => (
+      <>
+        Kind
+        <TipIcon className="ml-2">
+          Floating IPs can be detached from this instance and attached to another.
+        </TipIcon>
+      </>
+    ),
+    cell: (info) => <Badge color="neutral">{info.getValue()}</Badge>,
+  }),
+  ipColHelper.accessor('name', {
+    cell: (info) => (info.getValue() ? info.getValue() : <EmptyCell />),
+  }),
+  ipColHelper.accessor((row) => ('description' in row ? row.description : undefined), {
+    header: 'description',
+    cell: (info) => <DescriptionCell text={info.getValue()} />,
+  }),
+]
+
 export function NetworkingTab() {
   const instanceSelector = useInstanceSelector()
   const { instance: instanceName, project } = instanceSelector
@@ -157,13 +182,13 @@ export function NetworkingTab() {
       setCreateModalOpen(false)
     },
   })
-  const deleteNic = useApiMutation('instanceNetworkInterfaceDelete', {
+  const { mutateAsync: deleteNic } = useApiMutation('instanceNetworkInterfaceDelete', {
     onSuccess() {
       queryClient.invalidateQueries('instanceNetworkInterfaceList')
       addToast({ content: 'Network interface deleted' })
     },
   })
-  const editNic = useApiMutation('instanceNetworkInterfaceUpdate', {
+  const { mutate: editNic } = useApiMutation('instanceNetworkInterfaceUpdate', {
     onSuccess() {
       queryClient.invalidateQueries('instanceNetworkInterfaceList')
     },
@@ -180,7 +205,7 @@ export function NetworkingTab() {
       {
         label: 'Make primary',
         onActivate() {
-          editNic.mutate({
+          editNic({
             path: { interface: nic.name },
             query: instanceSelector,
             body: { ...nic, primary: true },
@@ -211,7 +236,7 @@ export function NetworkingTab() {
         label: 'Delete',
         onActivate: confirmDelete({
           doDelete: () =>
-            deleteNic.mutateAsync({
+            deleteNic({
               path: { interface: nic.name },
               query: instanceSelector,
             }),
@@ -243,32 +268,7 @@ export function NetworkingTab() {
     query: { project },
   })
 
-  const ipColHelper = createColumnHelper<ExternalIp>()
-  const staticIpCols = [
-    ipColHelper.accessor('ip', {
-      cell: (info) => <CopyableIp ip={info.getValue()} />,
-    }),
-    ipColHelper.accessor('kind', {
-      header: () => (
-        <>
-          Kind
-          <TipIcon className="ml-2">
-            Floating IPs can be detached from this instance and attached to another.
-          </TipIcon>
-        </>
-      ),
-      cell: (info) => <Badge color="neutral">{info.getValue()}</Badge>,
-    }),
-    ipColHelper.accessor('name', {
-      cell: (info) => (info.getValue() ? info.getValue() : <EmptyCell />),
-    }),
-    ipColHelper.accessor((row) => ('description' in row ? row.description : undefined), {
-      header: 'description',
-      cell: (info) => <DescriptionCell text={info.getValue()} />,
-    }),
-  ]
-
-  const ephemeralIpDetach = useApiMutation('instanceEphemeralIpDetach', {
+  const { mutateAsync: ephemeralIpDetach } = useApiMutation('instanceEphemeralIpDetach', {
     onSuccess() {
       queryClient.invalidateQueries('instanceExternalIpList')
       addToast({ content: 'Your ephemeral IP has been detached' })
@@ -278,7 +278,7 @@ export function NetworkingTab() {
     },
   })
 
-  const floatingIpDetach = useApiMutation('floatingIpDetach', {
+  const { mutateAsync: floatingIpDetach } = useApiMutation('floatingIpDetach', {
     onSuccess() {
       queryClient.invalidateQueries('floatingIpList')
       queryClient.invalidateQueries('instanceExternalIpList')
@@ -301,12 +301,12 @@ export function NetworkingTab() {
       const doAction =
         externalIp.kind === 'floating'
           ? () =>
-              floatingIpDetach.mutateAsync({
+              floatingIpDetach({
                 path: { floatingIp: externalIp.name },
                 query: { project },
               })
           : () =>
-              ephemeralIpDetach.mutateAsync({
+              ephemeralIpDetach({
                 path: { instance: instanceName },
                 query: { project },
               })

diff --git a/app/pages/project/instances/instance/tabs/StorageTab.tsx b/app/pages/project/instances/instance/tabs/StorageTab.tsx
@@ -73,7 +73,7 @@ export function StorageTab() {
     [instanceName, project]
   )
 
-  const detachDisk = useApiMutation('instanceDiskDetach', {
+  const { mutate: detachDisk } = useApiMutation('instanceDiskDetach', {
     onSuccess() {
       queryClient.invalidateQueries('instanceDiskList')
       addToast({ content: 'Disk detached' })
@@ -86,7 +86,7 @@ export function StorageTab() {
       })
     },
   })
-  const createSnapshot = useApiMutation('snapshotCreate', {
+  const { mutate: createSnapshot } = useApiMutation('snapshotCreate', {
     onSuccess() {
       queryClient.invalidateQueries('snapshotList')
       addToast({ content: 'Snapshot created' })
@@ -112,7 +112,7 @@ export function StorageTab() {
           </>
         ),
         onActivate() {
-          createSnapshot.mutate({
+          createSnapshot({
             query: { project },
             body: {
               name: genName(disk.name),
@@ -124,18 +124,20 @@ export function StorageTab() {
       },
       {
         label: 'Detach',
-        disabled: !instanceCan.detachDisk(instance) && (
+        disabled: !instanceCan.detachDisk({ runState: instance.runState }) && (
           <>
             Instance must be <span className="text-default">stopped</span> before disk can
             be detached
           </>
         ),
         onActivate() {
-          detachDisk.mutate({ body: { disk: disk.name }, ...instancePathQuery })
+          detachDisk({ body: { disk: disk.name }, ...instancePathQuery })
         },
       },
     ],
-    [detachDisk, instance, instancePathQuery, createSnapshot, project]
+    // important to pass instance.runState because instance is not referentially
+    // stable when we are polling when instance is in transition
+    [detachDisk, instance.runState, instancePathQuery, createSnapshot, project]
   )
 
   const attachDisk = useApiMutation('instanceDiskAttach', {

diff --git a/app/ui/styles/components/spinner.css b/app/ui/styles/components/spinner.css
@@ -46,6 +46,10 @@
   stroke: var(--content-default);
 }
 
+.spinner-secondary .path {
+  stroke: var(--content-secondary);
+}
+
 .spinner-primary .bg {
   stroke: var(--content-accent);
 }

diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts
@@ -673,7 +673,7 @@ export const handlers = makeHandlers({
 
     setTimeout(() => {
       instance.run_state = 'running'
-    }, 1000)
+    }, 3000)
 
     return json(instance, { status: 202 })
   },

diff --git a/test/e2e/instance-serial.e2e.ts b/test/e2e/instance-serial.e2e.ts
@@ -22,8 +22,8 @@ test('serial console can connect while starting', async ({ page }) => {
   await page.getByRole('link', { name: 'Connect' }).click()
 
   // The message goes from creating to starting and then disappears once
-  // the instance is running
-  await expect(page.getByText('The instance is creating')).toBeVisible()
+  // the instance is running. skip the check for "creating" because it can
+  // go by quickly
   await expect(page.getByText('Waiting for the instance to start')).toBeVisible()
   await expect(page.getByText('The instance is starting')).toBeVisible()
   await expect(page.getByText('The instance is')).toBeHidden()