From f66afff0fae47359d359c8297cb3b038d52e0287 Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Thu, 9 Jan 2025 11:53:44 +0000 Subject: [PATCH 01/32] First pass instance auto-restart --- app/components/DocsPopover.tsx | 4 +- app/components/InstanceAutoRestartPopover.tsx | 158 ++++++++++++++++++ .../instances/instance/InstancePage.tsx | 14 +- .../instances/instance/tabs/SettingsTab.tsx | 147 ++++++++++++++++ app/routes.tsx | 2 + app/ui/lib/DropdownMenu.tsx | 2 +- app/ui/lib/Listbox.tsx | 2 +- app/ui/lib/SettingsGroup.tsx | 6 +- app/ui/styles/components/menu-button.css | 10 +- app/util/path-builder.spec.ts | 1 + app/util/path-builder.ts | 1 + mock-api/instance.ts | 1 + mock-api/msw/handlers.ts | 4 + tailwind.config.ts | 3 + 14 files changed, 343 insertions(+), 12 deletions(-) create mode 100644 app/components/InstanceAutoRestartPopover.tsx create mode 100644 app/pages/project/instances/instance/tabs/SettingsTab.tsx diff --git a/app/components/DocsPopover.tsx b/app/components/DocsPopover.tsx index 82362feb14..2f00aff171 100644 --- a/app/components/DocsPopover.tsx +++ b/app/components/DocsPopover.tsx @@ -48,8 +48,8 @@ export const DocsPopover = ({ heading, icon, summary, links }: DocsPopoverProps)
diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx new file mode 100644 index 0000000000..2e7a0c61ea --- /dev/null +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -0,0 +1,158 @@ +import { CloseButton, Popover, PopoverButton, PopoverPanel } from '@headlessui/react' +import cn from 'classnames' +import { formatDistanceToNow } from 'date-fns' +import { useEffect, useState, type ReactNode } from 'react' +import { Link } from 'react-router' + +import { NextArrow12Icon, OpenLink12Icon } from '@oxide/design-system/icons/react' + +import type { InstanceAutoRestartPolicy } from '~/api' +import { HL } from '~/components/HL' +import { useInstanceSelector } from '~/hooks/use-params' +import { Badge } from '~/ui/lib/Badge' +import { Spinner } from '~/ui/lib/Spinner' +import { pb } from '~/util/path-builder' + +const helpText = { + enabled: ( + <>The control plane will attempt to automatically restart instance this instance. + ), + disabled: ( + <> + The control plane will not attempt to automatically restart it after entering the{' '} + failed state. + + ), + never: ( + <> + Instance auto-restart policy is set to never. The control plane will not attempt to + automatically restart it after entering the failed state. + + ), + starting: ( + <> + Instance auto-restart policy is queued to start. The control plane will begin the + restart process shortly. + + ), +} + +export const InstanceAutoRestartPopover = ({ + enabled, + policy, + cooldownExpiration, +}: { + enabled: boolean + policy: InstanceAutoRestartPolicy + cooldownExpiration: Date | undefined +}) => { + const instanceSelector = useInstanceSelector() + const [now, setNow] = useState(new Date()) + + useEffect(() => { + const timer = setInterval(() => setNow(new Date()), 1000) + return () => clearInterval(timer) + }, []) + + const isQueued = cooldownExpiration && new Date(cooldownExpiration) < now + + // todo: untangle this web + const helpTextState = isQueued + ? 'starting' + : policy === 'never' + ? 'never' + : enabled + ? 'enabled' + : ('disabled' as const) + + return ( + + + + + + + {enabled ? Enabled : Disabled} + + + + {policy ? ( + policy === 'never' ? ( + + never + + ) : ( + best effort + ) + ) : ( + Default + )} +
+ +
+ + + {cooldownExpiration && ( + + {isQueued ? ( + <> + Queued for restart… + + ) : ( +
+ Waiting{' '} + + ({formatDistanceToNow(cooldownExpiration)}) + +
+ )} + + )} +
+

{helpText[helpTextState]}

+ + + Learn about Instance Auto-Restart + + + +
+ + + ) +} + +const AutoRestartIcon12 = ({ className }: { className?: string }) => ( + + + +) + +const PopoverRow = ({ label, children }: { label: string; children: ReactNode }) => ( +
+
{label}
+
+ {children} +
+
+) diff --git a/app/pages/project/instances/instance/InstancePage.tsx b/app/pages/project/instances/instance/InstancePage.tsx index 22acc7c97e..d882615bdb 100644 --- a/app/pages/project/instances/instance/InstancePage.tsx +++ b/app/pages/project/instances/instance/InstancePage.tsx @@ -29,6 +29,7 @@ import { import { ExternalIps } from '~/components/ExternalIps' import { NumberField } from '~/components/form/fields/NumberField' import { HL } from '~/components/HL' +import { InstanceAutoRestartPopover } from '~/components/InstanceAutoRestartPopover' import { InstanceDocsPopover } from '~/components/InstanceDocsPopover' import { MoreActionsMenu } from '~/components/MoreActionsMenu' import { RefreshButton } from '~/components/RefreshButton' @@ -168,6 +169,9 @@ export function InstancePage() { const memory = filesize(instance.memory, { output: 'object', base: 2 }) + // Document when this popover is showing + const hasAutoRestart = !!instance.autoRestartCooldownExpiration + return ( <> @@ -203,7 +207,7 @@ export function InstancePage() { {memory.unit} -
+
{polling && ( @@ -212,6 +216,13 @@ export function InstancePage() { )} + {hasAutoRestart && ( + + )}
@@ -241,6 +252,7 @@ export function InstancePage() { Metrics Networking Connect + Settings {resizeInstance && ( { + const timer = setInterval(() => setNow(new Date()), 1000) + return () => clearInterval(timer) + }, []) + + const { data: instance } = usePrefetchedApiQuery('instanceView', { + path: { instance: instanceSelector.instance }, + query: { project: instanceSelector.project }, + }) + + const instanceUpdate = useApiMutation('instanceUpdate', { + onSuccess() { + apiQueryClient.invalidateQueries('instanceView') + addToast({ content: 'Instance auto-restart policy updated' }) + }, + }) + + const form = useForm({ + defaultValues: { + autoRestartPolicy: instance.autoRestartPolicy, + }, + }) + const { isDirty } = form.formState + + const onSubmit = form.handleSubmit((values) => { + instanceUpdate.mutate({ + path: { instance: instanceSelector.instance }, + query: { project: instanceSelector.project }, + body: { + ncpus: instance.ncpus, + memory: instance.memory, + bootDisk: instance.bootDiskId, + autoRestartPolicy: values.autoRestartPolicy, + }, + }) + }) + + return ( +
+ + + Auto-restart +

The auto-restart policy configured for this instance

+ + + + + {instance.autoRestartEnabled ? 'True' : 'False'}{' '} + {instance.autoRestartEnabled && !instance.autoRestartPolicy && ( + (Project default) + )} + + + {instance.autoRestartCooldownExpiration ? ( + <> + {format( + new Date(instance.autoRestartCooldownExpiration), + 'MMM d, yyyy HH:mm:ss zz' + )}{' '} + {new Date(instance.autoRestartCooldownExpiration) > now && ( + + ({formatDistanceToNow(new Date(instance.autoRestartCooldownExpiration))} + ) + + )} + + ) : ( + n/a + )} + + + +
+ +
+ + + +
+ ) +} + +const FormMeta = ({ + label, + helpText, + children, +}: { + label: string + helpText?: string + children: ReactNode +}) => { + const id = useId() + return ( +
+
+ + {label} + + {helpText && {helpText}} +
+ {children} +
+ ) +} diff --git a/app/routes.tsx b/app/routes.tsx index 780e603b5c..d71ffea989 100644 --- a/app/routes.tsx +++ b/app/routes.tsx @@ -62,6 +62,7 @@ import * as SerialConsole from './pages/project/instances/instance/SerialConsole import * as ConnectTab from './pages/project/instances/instance/tabs/ConnectTab' import * as MetricsTab from './pages/project/instances/instance/tabs/MetricsTab' import * as NetworkingTab from './pages/project/instances/instance/tabs/NetworkingTab' +import * as SettingsTab from './pages/project/instances/instance/tabs/SettingsTab' import * as StorageTab from './pages/project/instances/instance/tabs/StorageTab' import { InstancesPage } from './pages/project/instances/InstancesPage' import { SnapshotsPage } from './pages/project/snapshots/SnapshotsPage' @@ -295,6 +296,7 @@ export const routes = createRoutesFromElements( /> + diff --git a/app/ui/lib/DropdownMenu.tsx b/app/ui/lib/DropdownMenu.tsx index a6cf724d3c..f70954c30c 100644 --- a/app/ui/lib/DropdownMenu.tsx +++ b/app/ui/lib/DropdownMenu.tsx @@ -35,7 +35,7 @@ export function Content({ className, children, anchor = 'bottom end', gap }: Con anchor={anchor} // goofy gap because tailwind hates string interpolation className={cn( - 'DropdownMenuContent elevation-2', + 'dropdown-menu-content elevation-2', gap === 8 && `[--anchor-gap:8px]`, className )} diff --git a/app/ui/lib/Listbox.tsx b/app/ui/lib/Listbox.tsx index 6cd7af0106..c1617bb758 100644 --- a/app/ui/lib/Listbox.tsx +++ b/app/ui/lib/Listbox.tsx @@ -82,7 +82,7 @@ export const Listbox = ({ {({ open }) => ( <> {label && ( -
+
{ "floatingIpsNew": "/projects/p/floating-ips-new", "instance": "/projects/p/instances/i/storage", "instanceConnect": "/projects/p/instances/i/connect", + "instanceSettings": "/projects/p/instances/i/settings", "instanceMetrics": "/projects/p/instances/i/metrics", "instanceNetworking": "/projects/p/instances/i/networking", "instanceStorage": "/projects/p/instances/i/storage", diff --git a/app/util/path-builder.ts b/app/util/path-builder.ts index 5b5ab823f9..d227511a18 100644 --- a/app/util/path-builder.ts +++ b/app/util/path-builder.ts @@ -44,6 +44,7 @@ export const pb = { instanceConnect: (params: PP.Instance) => `${instanceBase(params)}/connect`, instanceNetworking: (params: PP.Instance) => `${instanceBase(params)}/networking`, serialConsole: (params: PP.Instance) => `${instanceBase(params)}/serial-console`, + instanceSettings: (params: PP.Instance) => `${instanceBase(params)}/settings`, disksNew: (params: PP.Project) => `${projectBase(params)}/disks-new`, disks: (params: PP.Project) => `${projectBase(params)}/disks`, diff --git a/mock-api/instance.ts b/mock-api/instance.ts index c274e1f16c..0f8a833e0b 100644 --- a/mock-api/instance.ts +++ b/mock-api/instance.ts @@ -42,6 +42,7 @@ const failedInstance: Json = { hostname: 'oxide.com', project_id: project.id, run_state: 'failed', + auto_restart_cooldown_expiration: new Date(Date.now() + 5 * 60 * 1000).toISOString(), // 5 mins in the future } const startingInstance: Json = { diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 376ad49714..6cca78755e 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -613,6 +613,10 @@ export const handlers = makeHandlers({ instance.boot_disk_id = undefined } + if (body.auto_restart_policy !== undefined) { + instance.auto_restart_policy = body.auto_restart_policy + } + // always present on the body, always set them instance.ncpus = body.ncpus instance.memory = body.memory diff --git a/tailwind.config.ts b/tailwind.config.ts index 4203962afd..e68c978a34 100644 --- a/tailwind.config.ts +++ b/tailwind.config.ts @@ -61,6 +61,9 @@ export default { transparent: 'transparent', current: 'currentColor', }, + animation: { + 'spin-slow': 'spin 5s linear infinite', + }, }, plugins: [ plugin(({ addVariant, addUtilities }) => { From 3be850038ff7b6ea97c27dd23c80521e56292163 Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Thu, 9 Jan 2025 17:37:11 +0000 Subject: [PATCH 02/32] Add comma to settings help text Co-authored-by: Eliza Weisman --- app/pages/project/instances/instance/tabs/SettingsTab.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/pages/project/instances/instance/tabs/SettingsTab.tsx b/app/pages/project/instances/instance/tabs/SettingsTab.tsx index f77b70c1b3..cf354b3e02 100644 --- a/app/pages/project/instances/instance/tabs/SettingsTab.tsx +++ b/app/pages/project/instances/instance/tabs/SettingsTab.tsx @@ -75,7 +75,7 @@ export function Component() { control={form.control} name="autoRestartPolicy" label="Policy" - description="If unconfigured this instance uses the default auto-restart policy, which may or may not allow it to be restarted." + description="If unconfigured, this instance uses the default auto-restart policy, which may or may not allow it to be restarted." placeholder="Default" items={[ { value: '', label: 'None (Default)' }, From 8d40e2fd2cac0fbe6520b2ab3399f443918dbcc2 Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Tue, 14 Jan 2025 11:48:36 +0000 Subject: [PATCH 03/32] Add cooldown and add copy tweaks --- app/components/InstanceAutoRestartPopover.tsx | 4 +-- .../instances/instance/tabs/SettingsTab.tsx | 25 +++++++++++++++++-- mock-api/instance.ts | 1 + 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx index 2e7a0c61ea..68d79b5491 100644 --- a/app/components/InstanceAutoRestartPopover.tsx +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -103,7 +103,7 @@ export const InstanceAutoRestartPopover = ({ {cooldownExpiration && ( - + {isQueued ? ( <> Queued for restart… @@ -151,7 +151,7 @@ const AutoRestartIcon12 = ({ className }: { className?: string }) => ( const PopoverRow = ({ label, children }: { label: string; children: ReactNode }) => (
{label}
-
+
{children}
diff --git a/app/pages/project/instances/instance/tabs/SettingsTab.tsx b/app/pages/project/instances/instance/tabs/SettingsTab.tsx index cf354b3e02..76d5248f04 100644 --- a/app/pages/project/instances/instance/tabs/SettingsTab.tsx +++ b/app/pages/project/instances/instance/tabs/SettingsTab.tsx @@ -85,13 +85,19 @@ export function Component() { required className="max-w-none" /> - + {instance.autoRestartEnabled ? 'True' : 'False'}{' '} {instance.autoRestartEnabled && !instance.autoRestartPolicy && ( (Project default) )} - + {instance.autoRestartCooldownExpiration ? ( <> {format( @@ -109,6 +115,21 @@ export function Component() { n/a )} + + {instance.timeLastAutoRestarted ? ( + <> + {format( + new Date(instance.timeLastAutoRestarted), + 'MMM d, yyyy HH:mm:ss zz' + )} + + ) : ( + n/a + )} +
diff --git a/mock-api/instance.ts b/mock-api/instance.ts index 0f8a833e0b..3f93b6a4f5 100644 --- a/mock-api/instance.ts +++ b/mock-api/instance.ts @@ -43,6 +43,7 @@ const failedInstance: Json = { project_id: project.id, run_state: 'failed', auto_restart_cooldown_expiration: new Date(Date.now() + 5 * 60 * 1000).toISOString(), // 5 mins in the future + time_last_auto_restarted: new Date(Date.now() - 30 * 24 * 60 * 60 * 1000).toISOString(), // one month ago } const startingInstance: Json = { From bf37c96d0c2d0efb850e321bfa90fcd13f26ea9b Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Tue, 14 Jan 2025 12:02:36 +0000 Subject: [PATCH 04/32] Add license --- app/components/InstanceAutoRestartPopover.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx index 68d79b5491..7e9ee94e4d 100644 --- a/app/components/InstanceAutoRestartPopover.tsx +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -1,3 +1,10 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ import { CloseButton, Popover, PopoverButton, PopoverPanel } from '@headlessui/react' import cn from 'classnames' import { formatDistanceToNow } from 'date-fns' From d81c471eed79b6ae9c7684f13e1586dd84e47405 Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Tue, 14 Jan 2025 12:03:41 +0000 Subject: [PATCH 05/32] Change none to default in listbox text --- app/pages/project/instances/instance/tabs/SettingsTab.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/pages/project/instances/instance/tabs/SettingsTab.tsx b/app/pages/project/instances/instance/tabs/SettingsTab.tsx index 76d5248f04..46bc43cd01 100644 --- a/app/pages/project/instances/instance/tabs/SettingsTab.tsx +++ b/app/pages/project/instances/instance/tabs/SettingsTab.tsx @@ -78,7 +78,7 @@ export function Component() { description="If unconfigured, this instance uses the default auto-restart policy, which may or may not allow it to be restarted." placeholder="Default" items={[ - { value: '', label: 'None (Default)' }, + { value: '', label: 'Default' }, { value: 'never', label: 'Never' }, { value: 'best_effort', label: 'Best effort' }, ]} From 06400f890da98c1f5d44e94c5081e3ee26a46fef Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Tue, 14 Jan 2025 12:10:39 +0000 Subject: [PATCH 06/32] Update mock handler to match omicron --- mock-api/msw/handlers.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 6cca78755e..36e0e7a263 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -584,7 +584,23 @@ export const handlers = makeHandlers({ return json(newInstance, { status: 201 }) }, - instanceView: ({ path, query }) => lookup.instance({ ...path, ...query }), + instanceView: ({ path, query }) => { + const instance = lookup.instance({ ...path, ...query }) + + // if empty uses default auto-restart behaviour + // if set, is based off of the policy + // https://github.com/oxidecomputer/omicron/blob/f63ed095e744fb8d2383fda6799eb0b2d6dfbd3c/nexus/db-queries/src/db/datastore/instance.rs#L228C26-L239 + if (instance.auto_restart_policy === 'never') { + instance.auto_restart_enabled = false + } else if ( + instance.auto_restart_policy === 'best_effort' || + !instance.auto_restart_policy // included for posterity but this has to be set in the mock data anyway + ) { + instance.auto_restart_enabled = true + } + + return instance + }, instanceUpdate({ path, query, body }) { const instance = lookup.instance({ ...path, ...query }) From a11042a7417a933fe8b9c7edb9020d0ad2c60ee1 Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Tue, 14 Jan 2025 12:56:59 +0000 Subject: [PATCH 07/32] Use `useInterval` hook --- app/components/InstanceAutoRestartPopover.tsx | 8 +++----- app/pages/project/instances/instance/tabs/SettingsTab.tsx | 8 +++----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx index 7e9ee94e4d..57e5d16a1f 100644 --- a/app/components/InstanceAutoRestartPopover.tsx +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -8,7 +8,7 @@ import { CloseButton, Popover, PopoverButton, PopoverPanel } from '@headlessui/react' import cn from 'classnames' import { formatDistanceToNow } from 'date-fns' -import { useEffect, useState, type ReactNode } from 'react' +import { useState, type ReactNode } from 'react' import { Link } from 'react-router' import { NextArrow12Icon, OpenLink12Icon } from '@oxide/design-system/icons/react' @@ -18,6 +18,7 @@ import { HL } from '~/components/HL' import { useInstanceSelector } from '~/hooks/use-params' import { Badge } from '~/ui/lib/Badge' import { Spinner } from '~/ui/lib/Spinner' +import { useInterval } from '~/ui/lib/use-interval' import { pb } from '~/util/path-builder' const helpText = { @@ -56,10 +57,7 @@ export const InstanceAutoRestartPopover = ({ const instanceSelector = useInstanceSelector() const [now, setNow] = useState(new Date()) - useEffect(() => { - const timer = setInterval(() => setNow(new Date()), 1000) - return () => clearInterval(timer) - }, []) + useInterval({ fn: () => setNow(new Date()), delay: 1000 }) const isQueued = cooldownExpiration && new Date(cooldownExpiration) < now diff --git a/app/pages/project/instances/instance/tabs/SettingsTab.tsx b/app/pages/project/instances/instance/tabs/SettingsTab.tsx index 46bc43cd01..90a89dbe54 100644 --- a/app/pages/project/instances/instance/tabs/SettingsTab.tsx +++ b/app/pages/project/instances/instance/tabs/SettingsTab.tsx @@ -7,7 +7,7 @@ */ import { format, formatDistanceToNow } from 'date-fns' -import { useEffect, useId, useState, type ReactNode } from 'react' +import { useId, useState, type ReactNode } from 'react' import { useForm } from 'react-hook-form' import { apiQueryClient, useApiMutation, usePrefetchedApiQuery } from '~/api' @@ -18,6 +18,7 @@ import { Button } from '~/ui/lib/Button' import { FieldLabel } from '~/ui/lib/FieldLabel' import { LearnMore, SettingsGroup } from '~/ui/lib/SettingsGroup' import { TipIcon } from '~/ui/lib/TipIcon' +import { useInterval } from '~/ui/lib/use-interval' import { links } from '~/util/links' Component.displayName = 'SettingsTab' @@ -26,10 +27,7 @@ export function Component() { const [now, setNow] = useState(new Date()) - useEffect(() => { - const timer = setInterval(() => setNow(new Date()), 1000) - return () => clearInterval(timer) - }, []) + useInterval({ fn: () => setNow(new Date()), delay: 1000 }) const { data: instance } = usePrefetchedApiQuery('instanceView', { path: { instance: instanceSelector.instance }, From c03afd2cce5bffd995aa1a69102636aea2289f68 Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Tue, 14 Jan 2025 14:04:59 +0000 Subject: [PATCH 08/32] Use `design-system` icon --- app/components/InstanceAutoRestartPopover.tsx | 24 +-- package-lock.json | 152 +----------------- package.json | 2 +- 3 files changed, 12 insertions(+), 166 deletions(-) diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx index 57e5d16a1f..6af803984f 100644 --- a/app/components/InstanceAutoRestartPopover.tsx +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -11,7 +11,11 @@ import { formatDistanceToNow } from 'date-fns' import { useState, type ReactNode } from 'react' import { Link } from 'react-router' -import { NextArrow12Icon, OpenLink12Icon } from '@oxide/design-system/icons/react' +import { + AutoRestart12Icon, + NextArrow12Icon, + OpenLink12Icon, +} from '@oxide/design-system/icons/react' import type { InstanceAutoRestartPolicy } from '~/api' import { HL } from '~/components/HL' @@ -73,7 +77,7 @@ export const InstanceAutoRestartPopover = ({ return ( - @@ -137,22 +141,6 @@ export const InstanceAutoRestartPopover = ({ ) } -const AutoRestartIcon12 = ({ className }: { className?: string }) => ( - - - -) - const PopoverRow = ({ label, children }: { label: string; children: ReactNode }) => (
{label}
diff --git a/package-lock.json b/package-lock.json index 5159e7bc15..a7715e75a6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,7 @@ "dependencies": { "@floating-ui/react": "^0.26.23", "@headlessui/react": "^2.2.0", - "@oxide/design-system": "^1.7.3", + "@oxide/design-system": "^3.0.1--canary.0de862a.0", "@radix-ui/react-accordion": "^1.2.0", "@radix-ui/react-dialog": "^1.0.5", "@radix-ui/react-focus-guards": "1.0.1", @@ -1547,9 +1547,9 @@ "license": "MIT" }, "node_modules/@oxide/design-system": { - "version": "1.8.3", - "resolved": "https://registry.npmjs.org/@oxide/design-system/-/design-system-1.8.3.tgz", - "integrity": "sha512-N14ayGhpw2W/hJgqH8QTKns/HWPOJ3A/ZkfpfwEvx8MtjUEgkl8gTfNciukwe5hR4ZU0oFxN7cEurjffYeJCgg==", + "version": "3.0.1--canary.0de862a.0", + "resolved": "https://registry.npmjs.org/@oxide/design-system/-/design-system-3.0.1--canary.0de862a.0.tgz", + "integrity": "sha512-d2LLzwJt3s7IIS6+ghi/7NfRqhGGS+udAuPsfHABM9wJV1LIl8kISCy9jYp2gEvZMCsfVJl2zJO2F9XfKkJhOQ==", "license": "MPL 2.0", "dependencies": { "@floating-ui/react": "^0.25.1", @@ -1562,7 +1562,6 @@ "peerDependencies": { "@asciidoctor/core": "^3.0.0", "@oxide/react-asciidoc": "^1.0.0", - "@remix-run/react": "2.13.1", "react": "^18.2.0", "react-dom": "^18.2.0" } @@ -4064,96 +4063,6 @@ "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0" } }, - "node_modules/@remix-run/react": { - "version": "2.13.1", - "resolved": "https://registry.npmjs.org/@remix-run/react/-/react-2.13.1.tgz", - "integrity": "sha512-kZevCoKMz0ZDOOzTnG95yfM7M9ju38FkWNY1wtxCy+NnUJYrmTerGQtiBsJgMzYD6i29+w4EwoQsdqys7DmMSg==", - "license": "MIT", - "peer": true, - "dependencies": { - "@remix-run/router": "1.20.0", - "@remix-run/server-runtime": "2.13.1", - "react-router": "6.27.0", - "react-router-dom": "6.27.0", - "turbo-stream": "2.4.0" - }, - "engines": { - "node": ">=18.0.0" - }, - "peerDependencies": { - "react": "^18.0.0", - "react-dom": "^18.0.0", - "typescript": "^5.1.0" - }, - "peerDependenciesMeta": { - "typescript": { - "optional": true - } - } - }, - "node_modules/@remix-run/react/node_modules/react-router": { - "version": "6.27.0", - "resolved": "https://registry.npmjs.org/react-router/-/react-router-6.27.0.tgz", - "integrity": "sha512-YA+HGZXz4jaAkVoYBE98VQl+nVzI+cVI2Oj/06F5ZM+0u3TgedN9Y9kmMRo2mnkSK2nCpNQn0DVob4HCsY/WLw==", - "license": "MIT", - "peer": true, - "dependencies": { - "@remix-run/router": "1.20.0" - }, - "engines": { - "node": ">=14.0.0" - }, - "peerDependencies": { - "react": ">=16.8" - } - }, - "node_modules/@remix-run/router": { - "version": "1.20.0", - "resolved": "https://registry.npmjs.org/@remix-run/router/-/router-1.20.0.tgz", - "integrity": "sha512-mUnk8rPJBI9loFDZ+YzPGdeniYK+FTmRD1TMCz7ev2SNIozyKKpnGgsxO34u6Z4z/t0ITuu7voi/AshfsGsgFg==", - "license": "MIT", - "peer": true, - "engines": { - "node": ">=14.0.0" - } - }, - "node_modules/@remix-run/server-runtime": { - "version": "2.13.1", - "resolved": "https://registry.npmjs.org/@remix-run/server-runtime/-/server-runtime-2.13.1.tgz", - "integrity": "sha512-2DfBPRcHKVzE4bCNsNkKB50BhCCKF73x+jiS836OyxSIAL+x0tguV2AEjmGXefEXc5AGGzoxkus0AUUEYa29Vg==", - "license": "MIT", - "peer": true, - "dependencies": { - "@remix-run/router": "1.20.0", - "@types/cookie": "^0.6.0", - "@web3-storage/multipart-parser": "^1.0.0", - "cookie": "^0.6.0", - "set-cookie-parser": "^2.4.8", - "source-map": "^0.7.3", - "turbo-stream": "2.4.0" - }, - "engines": { - "node": ">=18.0.0" - }, - "peerDependencies": { - "typescript": "^5.1.0" - }, - "peerDependenciesMeta": { - "typescript": { - "optional": true - } - } - }, - "node_modules/@remix-run/server-runtime/node_modules/cookie": { - "version": "0.6.0", - "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.6.0.tgz", - "integrity": "sha512-U71cyTamuh1CRNCfpGY6to28lxvNwPG4Guz/EVjgf3Jmzv0vlDp1atT9eS5dDjMYHucpHbWns6Lwf3BKz6svdw==", - "license": "MIT", - "peer": true, - "engines": { - "node": ">= 0.6" - } - }, "node_modules/@rollup/pluginutils": { "version": "4.2.1", "resolved": "https://registry.npmjs.org/@rollup/pluginutils/-/pluginutils-4.2.1.tgz", @@ -5649,13 +5558,6 @@ "url": "https://opencollective.com/vitest" } }, - "node_modules/@web3-storage/multipart-parser": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/@web3-storage/multipart-parser/-/multipart-parser-1.0.0.tgz", - "integrity": "sha512-BEO6al7BYqcnfX15W2cnGR+Q566ACXAT9UQykORCWW80lmkpWsnEob6zJS1ZVBKsSJC8+7vJkHwlp+lXG1UCdw==", - "license": "(Apache-2.0 AND MIT)", - "peer": true - }, "node_modules/@xterm/addon-fit": { "version": "0.10.0", "resolved": "https://registry.npmjs.org/@xterm/addon-fit/-/addon-fit-0.10.0.tgz", @@ -12475,40 +12377,6 @@ } } }, - "node_modules/react-router-dom": { - "version": "6.27.0", - "resolved": "https://registry.npmjs.org/react-router-dom/-/react-router-dom-6.27.0.tgz", - "integrity": "sha512-+bvtFWMC0DgAFrfKXKG9Fc+BcXWRUO1aJIihbB79xaeq0v5UzfvnM5houGUm1Y461WVRcgAQ+Clh5rdb1eCx4g==", - "license": "MIT", - "peer": true, - "dependencies": { - "@remix-run/router": "1.20.0", - "react-router": "6.27.0" - }, - "engines": { - "node": ">=14.0.0" - }, - "peerDependencies": { - "react": ">=16.8", - "react-dom": ">=16.8" - } - }, - "node_modules/react-router-dom/node_modules/react-router": { - "version": "6.27.0", - "resolved": "https://registry.npmjs.org/react-router/-/react-router-6.27.0.tgz", - "integrity": "sha512-YA+HGZXz4jaAkVoYBE98VQl+nVzI+cVI2Oj/06F5ZM+0u3TgedN9Y9kmMRo2mnkSK2nCpNQn0DVob4HCsY/WLw==", - "license": "MIT", - "peer": true, - "dependencies": { - "@remix-run/router": "1.20.0" - }, - "engines": { - "node": ">=14.0.0" - }, - "peerDependencies": { - "react": ">=16.8" - } - }, "node_modules/react-router/node_modules/cookie": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/cookie/-/cookie-1.0.2.tgz", @@ -13414,16 +13282,6 @@ "react": ">=16.8.0" } }, - "node_modules/source-map": { - "version": "0.7.4", - "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.7.4.tgz", - "integrity": "sha512-l3BikUxvPOcn5E74dZiq5BGsTb5yEwhaTSzccU6t4sDOH8NWJCstKO5QT2CvtFoK6F0saL7p9xHAqHOlCPJygA==", - "license": "BSD-3-Clause", - "peer": true, - "engines": { - "node": ">= 8" - } - }, "node_modules/source-map-js": { "version": "1.2.1", "resolved": "https://registry.npmjs.org/source-map-js/-/source-map-js-1.2.1.tgz", @@ -14436,7 +14294,7 @@ "version": "5.7.3", "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.7.3.tgz", "integrity": "sha512-84MVSjMEHP+FQRPy3pX9sTVV/INIex71s9TL2Gm5FG/WG1SqXeKyZ0k7/blY/4FdOzI12CBy1vGc4og/eus0fw==", - "devOptional": true, + "dev": true, "license": "Apache-2.0", "bin": { "tsc": "bin/tsc", diff --git a/package.json b/package.json index 4dc0dbeab8..531ce0e52c 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "dependencies": { "@floating-ui/react": "^0.26.23", "@headlessui/react": "^2.2.0", - "@oxide/design-system": "^1.7.3", + "@oxide/design-system": "^3.0.1--canary.0de862a.0", "@radix-ui/react-accordion": "^1.2.0", "@radix-ui/react-dialog": "^1.0.5", "@radix-ui/react-focus-guards": "1.0.1", From 3212357d41013b1a45bfdffa10935db7149252fe Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Tue, 14 Jan 2025 14:28:31 +0000 Subject: [PATCH 09/32] Text tweak --- app/components/InstanceAutoRestartPopover.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx index 6af803984f..ff138536d1 100644 --- a/app/components/InstanceAutoRestartPopover.tsx +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -27,7 +27,10 @@ import { pb } from '~/util/path-builder' const helpText = { enabled: ( - <>The control plane will attempt to automatically restart instance this instance. + <> + The control plane will attempt to automatically restart instance this instance after + entering the failed state. + ), disabled: ( <> From 7ada68ea3e020ac38fb3e51e97d4ae77696c1ee1 Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Tue, 14 Jan 2025 14:39:08 +0000 Subject: [PATCH 10/32] Simplify `helpTextState` --- app/components/InstanceAutoRestartPopover.tsx | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx index ff138536d1..a6330e5aa2 100644 --- a/app/components/InstanceAutoRestartPopover.tsx +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -68,14 +68,10 @@ export const InstanceAutoRestartPopover = ({ const isQueued = cooldownExpiration && new Date(cooldownExpiration) < now - // todo: untangle this web - const helpTextState = isQueued - ? 'starting' - : policy === 'never' - ? 'never' - : enabled - ? 'enabled' - : ('disabled' as const) + let helpTextState: keyof typeof helpText = 'disabled' + if (isQueued) helpTextState = 'starting' // Expiration is in the past and queued for restart + if (policy === 'never') helpTextState = 'never' // Will never auto-restart + if (enabled) helpTextState = 'enabled' // Restart enabled and cooldown as not expired return ( From a5997452ccda637549ade8c1340eea5cbd46a08d Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Tue, 14 Jan 2025 14:39:16 +0000 Subject: [PATCH 11/32] `InstanceAutoRestartPolicy` can be undefined --- app/components/InstanceAutoRestartPopover.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx index a6330e5aa2..daca276832 100644 --- a/app/components/InstanceAutoRestartPopover.tsx +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -58,7 +58,7 @@ export const InstanceAutoRestartPopover = ({ cooldownExpiration, }: { enabled: boolean - policy: InstanceAutoRestartPolicy + policy?: InstanceAutoRestartPolicy cooldownExpiration: Date | undefined }) => { const instanceSelector = useInstanceSelector() From 467b58e874a572d48add1ace06da232e4d1d302b Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Tue, 14 Jan 2025 14:50:00 +0000 Subject: [PATCH 12/32] Vitest fixes --- .../__snapshots__/path-builder.spec.ts.snap | 22 +++++++++++++++++++ app/util/path-builder.spec.ts | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/app/util/__snapshots__/path-builder.spec.ts.snap b/app/util/__snapshots__/path-builder.spec.ts.snap index 7142b88c84..0682007ae5 100644 --- a/app/util/__snapshots__/path-builder.spec.ts.snap +++ b/app/util/__snapshots__/path-builder.spec.ts.snap @@ -171,6 +171,28 @@ exports[`breadcrumbs 2`] = ` "path": "/projects/p/instances/i/networking", }, ], + "instanceSettings (/projects/p/instances/i/settings)": [ + { + "label": "Projects", + "path": "/projects", + }, + { + "label": "p", + "path": "/projects/p/instances", + }, + { + "label": "Instances", + "path": "/projects/p/instances", + }, + { + "label": "i", + "path": "/projects/p/instances/i/storage", + }, + { + "label": "Settings", + "path": "/projects/p/instances/i/settings", + }, + ], "instanceStorage (/projects/p/instances/i/storage)": [ { "label": "Projects", diff --git a/app/util/path-builder.spec.ts b/app/util/path-builder.spec.ts index 3a24da37e2..f094d10312 100644 --- a/app/util/path-builder.spec.ts +++ b/app/util/path-builder.spec.ts @@ -48,9 +48,9 @@ test('path builder', () => { "floatingIpsNew": "/projects/p/floating-ips-new", "instance": "/projects/p/instances/i/storage", "instanceConnect": "/projects/p/instances/i/connect", - "instanceSettings": "/projects/p/instances/i/settings", "instanceMetrics": "/projects/p/instances/i/metrics", "instanceNetworking": "/projects/p/instances/i/networking", + "instanceSettings": "/projects/p/instances/i/settings", "instanceStorage": "/projects/p/instances/i/storage", "instances": "/projects/p/instances", "instancesNew": "/projects/p/instances-new", From ebe8dc08906128eb43b64993abae05acf798027f Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Tue, 21 Jan 2025 10:42:52 +0000 Subject: [PATCH 13/32] Upgrade `@oxide/design-system` --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1726a1239d..a56de5dfb0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,7 @@ "dependencies": { "@floating-ui/react": "^0.26.23", "@headlessui/react": "^2.2.0", - "@oxide/design-system": "^3.0.1--canary.0de862a.0", + "@oxide/design-system": "^2.1.0", "@radix-ui/react-accordion": "^1.2.0", "@radix-ui/react-dialog": "^1.0.5", "@radix-ui/react-focus-guards": "1.0.1", @@ -1547,9 +1547,9 @@ "license": "MIT" }, "node_modules/@oxide/design-system": { - "version": "3.0.1--canary.0de862a.0", - "resolved": "https://registry.npmjs.org/@oxide/design-system/-/design-system-3.0.1--canary.0de862a.0.tgz", - "integrity": "sha512-d2LLzwJt3s7IIS6+ghi/7NfRqhGGS+udAuPsfHABM9wJV1LIl8kISCy9jYp2gEvZMCsfVJl2zJO2F9XfKkJhOQ==", + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/@oxide/design-system/-/design-system-2.1.0.tgz", + "integrity": "sha512-aQpD+vZQIgkBzi9jcX35m1z1milAf4PzewVH9FlMMGniwUNCXw6G6KT8t3Unnwy3egUQ/qvhdQqm9VSdoDQwsA==", "license": "MPL 2.0", "dependencies": { "@floating-ui/react": "^0.25.1", diff --git a/package.json b/package.json index 4d7d10d423..0544ff0e31 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "dependencies": { "@floating-ui/react": "^0.26.23", "@headlessui/react": "^2.2.0", - "@oxide/design-system": "^3.0.1--canary.0de862a.0", + "@oxide/design-system": "^2.1.0", "@radix-ui/react-accordion": "^1.2.0", "@radix-ui/react-dialog": "^1.0.5", "@radix-ui/react-focus-guards": "1.0.1", From 72f3ea3b80489fd18ee1b06d4c9106df967467b7 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 31 Jan 2025 16:35:11 -0600 Subject: [PATCH 14/32] uncontroversial copy tweaks --- app/components/InstanceAutoRestartPopover.tsx | 6 +++--- app/pages/project/instances/instance/InstancePage.tsx | 5 +---- app/pages/project/instances/instance/tabs/SettingsTab.tsx | 8 ++++---- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx index daca276832..04b7ca94ed 100644 --- a/app/components/InstanceAutoRestartPopover.tsx +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -28,8 +28,8 @@ import { pb } from '~/util/path-builder' const helpText = { enabled: ( <> - The control plane will attempt to automatically restart instance this instance after - entering the failed state. + The control plane will attempt to automatically restart this instance after entering + the failed state. ), disabled: ( @@ -66,7 +66,7 @@ export const InstanceAutoRestartPopover = ({ useInterval({ fn: () => setNow(new Date()), delay: 1000 }) - const isQueued = cooldownExpiration && new Date(cooldownExpiration) < now + const isQueued = cooldownExpiration && cooldownExpiration < now let helpTextState: keyof typeof helpText = 'disabled' if (isQueued) helpTextState = 'starting' // Expiration is in the past and queued for restart diff --git a/app/pages/project/instances/instance/InstancePage.tsx b/app/pages/project/instances/instance/InstancePage.tsx index d882615bdb..01d0259c1f 100644 --- a/app/pages/project/instances/instance/InstancePage.tsx +++ b/app/pages/project/instances/instance/InstancePage.tsx @@ -169,9 +169,6 @@ export function InstancePage() { const memory = filesize(instance.memory, { output: 'object', base: 2 }) - // Document when this popover is showing - const hasAutoRestart = !!instance.autoRestartCooldownExpiration - return ( <> @@ -216,7 +213,7 @@ export function InstancePage() { )} - {hasAutoRestart && ( + {instance.autoRestartCooldownExpiration && ( {instance.autoRestartEnabled ? 'True' : 'False'}{' '} {instance.autoRestartEnabled && !instance.autoRestartPolicy && ( @@ -94,7 +94,7 @@ export function Component() { {instance.autoRestartCooldownExpiration ? ( <> @@ -110,7 +110,7 @@ export function Component() { )} ) : ( - n/a + N/A )} ) : ( - n/a + N/A )} From de849ff15bfb9240ef1fddf34e1d590c9f7ddeb5 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 31 Jan 2025 17:33:48 -0600 Subject: [PATCH 15/32] fix restart policy form unset bug --- app/components/InstanceAutoRestartPopover.tsx | 36 ++++++++---- .../instances/instance/tabs/SettingsTab.tsx | 55 +++++++++++++------ app/util/links.ts | 2 + mock-api/msw/handlers.ts | 5 +- test/e2e/instance-auto-restart.e2e.ts | 42 ++++++++++++++ 5 files changed, 109 insertions(+), 31 deletions(-) create mode 100644 test/e2e/instance-auto-restart.e2e.ts diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx index 04b7ca94ed..4bf8d9b705 100644 --- a/app/components/InstanceAutoRestartPopover.tsx +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -52,6 +52,24 @@ const helpText = { ), } +// ts-pattern could make this less ugly +const PolicyBadge = ({ policy }: { policy?: InstanceAutoRestartPolicy }) => { + if (policy === 'never') { + return ( + + never + + ) + } else if (policy === 'best_effort') { + return best effort + } else if (policy === undefined) { + return Default + } else { + const _exhaustiveCheck: never = policy + return _exhaustiveCheck + } +} + export const InstanceAutoRestartPopover = ({ enabled, policy, @@ -75,9 +93,13 @@ export const InstanceAutoRestartPopover = ({ return ( - + - {policy ? ( - policy === 'never' ? ( - - never - - ) : ( - best effort - ) - ) : ( - Default - )} +
diff --git a/app/pages/project/instances/instance/tabs/SettingsTab.tsx b/app/pages/project/instances/instance/tabs/SettingsTab.tsx index be8c7640b1..68426e4ae4 100644 --- a/app/pages/project/instances/instance/tabs/SettingsTab.tsx +++ b/app/pages/project/instances/instance/tabs/SettingsTab.tsx @@ -10,23 +10,50 @@ import { format, formatDistanceToNow } from 'date-fns' import { useId, useState, type ReactNode } from 'react' import { useForm } from 'react-hook-form' -import { apiQueryClient, useApiMutation, usePrefetchedApiQuery } from '~/api' +import { + apiQueryClient, + useApiMutation, + usePrefetchedApiQuery, + type InstanceUpdate, +} from '~/api' import { ListboxField } from '~/components/form/fields/ListboxField' import { useInstanceSelector } from '~/hooks/use-params' import { addToast } from '~/stores/toast' import { Button } from '~/ui/lib/Button' import { FieldLabel } from '~/ui/lib/FieldLabel' +import { type ListboxItem } from '~/ui/lib/Listbox' import { LearnMore, SettingsGroup } from '~/ui/lib/SettingsGroup' import { TipIcon } from '~/ui/lib/TipIcon' import { useInterval } from '~/ui/lib/use-interval' import { links } from '~/util/links' +type FormPolicy = 'default' | 'never' | 'best_effort' +type ApiPolicy = InstanceUpdate['autoRestartPolicy'] + +const formPolicyToApiPolicy: Record = { + default: undefined, + never: 'never', + best_effort: 'best_effort', +} + +const restartPolicyItems: ListboxItem[] = [ + { value: 'default', label: 'Default' }, + { value: 'never', label: 'Never' }, + { value: 'best_effort', label: 'Best effort' }, +] + +type FormValues = { + autoRestartPolicy: FormPolicy +} + Component.displayName = 'SettingsTab' export function Component() { const instanceSelector = useInstanceSelector() const [now, setNow] = useState(new Date()) + // TODO: see if we can tweak the polling condition to include + // failed + cooling down and just rely on polling useInterval({ fn: () => setNow(new Date()), delay: 1000 }) const { data: instance } = usePrefetchedApiQuery('instanceView', { @@ -41,12 +68,12 @@ export function Component() { }, }) - const form = useForm({ - defaultValues: { - autoRestartPolicy: instance.autoRestartPolicy, - }, - }) - const { isDirty } = form.formState + const autoRestartPolicy = instance.autoRestartPolicy || 'default' + const defaultValues: FormValues = { autoRestartPolicy } + + const form = useForm({ defaultValues }) + + const disableSubmit = form.watch('autoRestartPolicy') === autoRestartPolicy const onSubmit = form.handleSubmit((values) => { instanceUpdate.mutate({ @@ -56,7 +83,7 @@ export function Component() { ncpus: instance.ncpus, memory: instance.memory, bootDisk: instance.bootDiskId, - autoRestartPolicy: values.autoRestartPolicy, + autoRestartPolicy: formPolicyToApiPolicy[values.autoRestartPolicy], }, }) }) @@ -66,7 +93,7 @@ export function Component() { Auto-restart -

The auto-restart policy configured for this instance

+

The auto-restart policy for this instance

 @@ -131,9 +154,9 @@ export function Component() {
- +
- diff --git a/app/util/links.ts b/app/util/links.ts index b3bc96256f..c067cc5070 100644 --- a/app/util/links.ts +++ b/app/util/links.ts @@ -26,6 +26,8 @@ export const links = { instanceActionsDocs: 'https://docs.oxide.computer/guides/managing-instances', // TODO: link to section instanceBootDiskDocs: 'https://docs.oxide.computer/guides/deploying-workloads', + instanceUpdateDocs: + 'https://docs.oxide.computer/guides/managing-instances#_update_instances', keyConceptsIamPolicyDocs: 'https://docs.oxide.computer/guides/key-entities-and-concepts#iam-policy', keyConceptsProjectsDocs: diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 61fcdfdf11..1d640e1e23 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -629,9 +629,8 @@ export const handlers = makeHandlers({ instance.boot_disk_id = undefined } - if (body.auto_restart_policy !== undefined) { - instance.auto_restart_policy = body.auto_restart_policy - } + // undefined or missing is meaningful, unsets the value + instance.auto_restart_policy = body.auto_restart_policy // always present on the body, always set them instance.ncpus = body.ncpus diff --git a/test/e2e/instance-auto-restart.e2e.ts b/test/e2e/instance-auto-restart.e2e.ts new file mode 100644 index 0000000000..3e7fd64006 --- /dev/null +++ b/test/e2e/instance-auto-restart.e2e.ts @@ -0,0 +1,42 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ +import { expect, test } from '@playwright/test' + +import { expectToast } from './utils' + +test('Instance auto restart policy', async ({ page }) => { + await page.goto('/projects/mock-project/instances/you-fail') + + // check popover + const indicator = page.getByRole('button', { name: 'Auto-restart status' }) + await indicator.click() + await expect(page.getByText('Auto RestartEnabled')).toBeVisible() + await expect(page.getByText('PolicyDefault')).toBeVisible() + await expect(page.getByText('CooldownWaiting (5 minutes)')).toBeVisible() + + // now go to settings tab by clicking link in popover + await page.getByRole('link', { name: 'Default' }).click() + + // assert contents of table-like thing showing the state + + // await expect(page.getByRole('button', { name: 'Policy' })) + const save = page.getByRole('button', { name: 'Save' }) + await expect(save).toBeDisabled() + + const policyListbox = page.getByRole('button', { name: 'Policy' }) + await expect(policyListbox).toContainText('Default') + + await page.getByRole('button', { name: 'Policy' }).click() + await page.getByRole('option', { name: 'Never' }).click() + await save.click() + + await expectToast(page, 'Instance auto-restart policy updated') + await expect(policyListbox).toContainText('Never') +}) + +// TODO: test that polling updates the relevant stuff From 9f9d9db642d97072e22002255e3688201ac739b1 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Sat, 1 Feb 2025 11:46:33 -0600 Subject: [PATCH 16/32] ts-pattern for nicer exhaustiveness --- app/components/InstanceAutoRestartPopover.tsx | 29 +++++++------------ .../instances/instance/tabs/SettingsTab.tsx | 21 +++++--------- package-lock.json | 8 ++--- package.json | 1 + 4 files changed, 22 insertions(+), 37 deletions(-) diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx index 4bf8d9b705..abd25a10e2 100644 --- a/app/components/InstanceAutoRestartPopover.tsx +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -10,6 +10,7 @@ import cn from 'classnames' import { formatDistanceToNow } from 'date-fns' import { useState, type ReactNode } from 'react' import { Link } from 'react-router' +import { match } from 'ts-pattern' import { AutoRestart12Icon, @@ -52,24 +53,6 @@ const helpText = { ), } -// ts-pattern could make this less ugly -const PolicyBadge = ({ policy }: { policy?: InstanceAutoRestartPolicy }) => { - if (policy === 'never') { - return ( - - never - - ) - } else if (policy === 'best_effort') { - return best effort - } else if (policy === undefined) { - return Default - } else { - const _exhaustiveCheck: never = policy - return _exhaustiveCheck - } -} - export const InstanceAutoRestartPopover = ({ enabled, policy, @@ -116,7 +99,15 @@ export const InstanceAutoRestartPopover = ({ to={pb.instanceSettings(instanceSelector)} className="group -m-1 flex w-full items-center justify-between rounded px-1" > - + {match(policy) + .with('never', () => ( + + never + + )) + .with('best_effort', () => best effort) + .with(undefined, () => Default) + .exhaustive()}
diff --git a/app/pages/project/instances/instance/tabs/SettingsTab.tsx b/app/pages/project/instances/instance/tabs/SettingsTab.tsx index 68426e4ae4..422b6e6f7d 100644 --- a/app/pages/project/instances/instance/tabs/SettingsTab.tsx +++ b/app/pages/project/instances/instance/tabs/SettingsTab.tsx @@ -9,13 +9,9 @@ import { format, formatDistanceToNow } from 'date-fns' import { useId, useState, type ReactNode } from 'react' import { useForm } from 'react-hook-form' +import { match } from 'ts-pattern' -import { - apiQueryClient, - useApiMutation, - usePrefetchedApiQuery, - type InstanceUpdate, -} from '~/api' +import { apiQueryClient, useApiMutation, usePrefetchedApiQuery } from '~/api' import { ListboxField } from '~/components/form/fields/ListboxField' import { useInstanceSelector } from '~/hooks/use-params' import { addToast } from '~/stores/toast' @@ -28,13 +24,6 @@ import { useInterval } from '~/ui/lib/use-interval' import { links } from '~/util/links' type FormPolicy = 'default' | 'never' | 'best_effort' -type ApiPolicy = InstanceUpdate['autoRestartPolicy'] - -const formPolicyToApiPolicy: Record = { - default: undefined, - never: 'never', - best_effort: 'best_effort', -} const restartPolicyItems: ListboxItem[] = [ { value: 'default', label: 'Default' }, @@ -83,7 +72,11 @@ export function Component() { ncpus: instance.ncpus, memory: instance.memory, bootDisk: instance.bootDiskId, - autoRestartPolicy: formPolicyToApiPolicy[values.autoRestartPolicy], + autoRestartPolicy: match(values.autoRestartPolicy) + .with('default', () => undefined) + .with('never', () => 'never' as const) + .with('best_effort', () => 'best_effort' as const) + .exhaustive(), }, }) }) diff --git a/package-lock.json b/package-lock.json index 62957da1db..9fc131ca3f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -46,6 +46,7 @@ "recharts": "^2.12.7", "remeda": "^2.14.0", "simplebar-react": "^3.2.6", + "ts-pattern": "^5.6.2", "tslib": "^2.7.0", "tunnel-rat": "0.0.4", "uuid": "^10.0.0", @@ -14061,10 +14062,9 @@ "license": "Apache-2.0" }, "node_modules/ts-pattern": { - "version": "5.3.1", - "resolved": "https://registry.npmjs.org/ts-pattern/-/ts-pattern-5.3.1.tgz", - "integrity": "sha512-1RUMKa8jYQdNfmnK4jyzBK3/PS/tnjcZ1CW0v1vWDeYe5RBklc/nquw03MEoB66hVBm4BnlCfmOqDVxHyT1DpA==", - "dev": true, + "version": "5.6.2", + "resolved": "https://registry.npmjs.org/ts-pattern/-/ts-pattern-5.6.2.tgz", + "integrity": "sha512-d4IxJUXROL5NCa3amvMg6VQW2HVtZYmUTPfvVtO7zJWGYLJ+mry9v2OmYm+z67aniQoQ8/yFNadiEwtNS9qQiw==", "license": "MIT" }, "node_modules/tsconfck": { diff --git a/package.json b/package.json index ec9ebbbabf..96b100e764 100644 --- a/package.json +++ b/package.json @@ -68,6 +68,7 @@ "recharts": "^2.12.7", "remeda": "^2.14.0", "simplebar-react": "^3.2.6", + "ts-pattern": "^5.6.2", "tslib": "^2.7.0", "tunnel-rat": "0.0.4", "uuid": "^10.0.0", From b86cf88a4249a5bd6ce980dabccee2288f09b54e Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Mon, 3 Feb 2025 10:35:48 +0000 Subject: [PATCH 17/32] Stronger highlight --- app/components/HL.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/HL.tsx b/app/components/HL.tsx index fd34f00d5c..7c38b3ab99 100644 --- a/app/components/HL.tsx +++ b/app/components/HL.tsx @@ -10,7 +10,7 @@ import { classed } from '~/util/classed' // note parent with secondary text color must have 'group' on it for // this to work. see Toast for an example export const HL = classed.span` - text-sans-md text-raise + text-semi-md text-raise group-[.text-accent-secondary]:text-accent group-[.text-error-secondary]:text-error group-[.text-info-secondary]:text-info From 740fb8b753dd8d525640481eab54acf10b8db8f3 Mon Sep 17 00:00:00 2001 From: Benjamin Leonard Date: Mon, 3 Feb 2025 10:35:58 +0000 Subject: [PATCH 18/32] Stop icon spin --- app/components/InstanceAutoRestartPopover.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx index abd25a10e2..48521b07e0 100644 --- a/app/components/InstanceAutoRestartPopover.tsx +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -6,7 +6,6 @@ * Copyright Oxide Computer Company */ import { CloseButton, Popover, PopoverButton, PopoverPanel } from '@headlessui/react' -import cn from 'classnames' import { formatDistanceToNow } from 'date-fns' import { useState, type ReactNode } from 'react' import { Link } from 'react-router' @@ -80,10 +79,7 @@ export const InstanceAutoRestartPopover = ({ className="group flex h-6 w-6 items-center justify-center rounded border border-default hover:bg-hover" aria-label="Auto-restart status" > - + Date: Mon, 3 Feb 2025 10:53:52 +0000 Subject: [PATCH 19/32] Add instance popover link state --- app/components/InstanceAutoRestartPopover.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx index 48521b07e0..6d0f9d4483 100644 --- a/app/components/InstanceAutoRestartPopover.tsx +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -23,6 +23,7 @@ import { useInstanceSelector } from '~/hooks/use-params' import { Badge } from '~/ui/lib/Badge' import { Spinner } from '~/ui/lib/Spinner' import { useInterval } from '~/ui/lib/use-interval' +import { links } from '~/util/links' import { pb } from '~/util/path-builder' const helpText = { @@ -127,9 +128,12 @@ export const InstanceAutoRestartPopover = ({ )}

{helpText[helpTextState]}

- + - Learn about Instance Auto-Restart + Learn about{' '} + + Instance Auto-Restart + From d1a41b1e6a11058fe6445072512eab02c817bf82 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Sat, 1 Feb 2025 12:40:06 -0600 Subject: [PATCH 20/32] use instance polling instead of interval --- app/api/util.ts | 5 +++ app/components/InstanceAutoRestartPopover.tsx | 26 +++++-------- .../instances/instance/InstancePage.tsx | 39 +++++++++++-------- 3 files changed, 38 insertions(+), 32 deletions(-) diff --git a/app/api/util.ts b/app/api/util.ts index 954cd77b0e..3121c8b176 100644 --- a/app/api/util.ts +++ b/app/api/util.ts @@ -140,6 +140,11 @@ export function instanceTransitioning({ runState }: Instance) { ) } +/** Cooling down after failed auto-restart */ +export function instanceCoolingDown(i: Instance) { + return i.runState === 'failed' && i.autoRestartCooldownExpiration +} + const diskActions = { // this is a weird one because the list of states is dynamic and it includes // 'creating' in the unwind of the disk create saga, but does not include diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx index 6d0f9d4483..e518b3edd0 100644 --- a/app/components/InstanceAutoRestartPopover.tsx +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -7,7 +7,7 @@ */ import { CloseButton, Popover, PopoverButton, PopoverPanel } from '@headlessui/react' import { formatDistanceToNow } from 'date-fns' -import { useState, type ReactNode } from 'react' +import { type ReactNode } from 'react' import { Link } from 'react-router' import { match } from 'ts-pattern' @@ -17,12 +17,11 @@ import { OpenLink12Icon, } from '@oxide/design-system/icons/react' -import type { InstanceAutoRestartPolicy } from '~/api' +import type { Instance } from '~/api' import { HL } from '~/components/HL' import { useInstanceSelector } from '~/hooks/use-params' import { Badge } from '~/ui/lib/Badge' import { Spinner } from '~/ui/lib/Spinner' -import { useInterval } from '~/ui/lib/use-interval' import { links } from '~/util/links' import { pb } from '~/util/path-builder' @@ -53,20 +52,15 @@ const helpText = { ), } -export const InstanceAutoRestartPopover = ({ - enabled, - policy, - cooldownExpiration, -}: { - enabled: boolean - policy?: InstanceAutoRestartPolicy - cooldownExpiration: Date | undefined -}) => { - const instanceSelector = useInstanceSelector() - const [now, setNow] = useState(new Date()) - - useInterval({ fn: () => setNow(new Date()), delay: 1000 }) +export const InstanceAutoRestartPopover = ({ instance }: { instance: Instance }) => { + const { + autoRestartCooldownExpiration: cooldownExpiration, + autoRestartPolicy: policy, + autoRestartEnabled: enabled, + } = instance + const instanceSelector = useInstanceSelector() + const now = new Date() const isQueued = cooldownExpiration && cooldownExpiration < now let helpTextState: keyof typeof helpText = 'disabled' diff --git a/app/pages/project/instances/instance/InstancePage.tsx b/app/pages/project/instances/instance/InstancePage.tsx index 01d0259c1f..3a0db96ec9 100644 --- a/app/pages/project/instances/instance/InstancePage.tsx +++ b/app/pages/project/instances/instance/InstancePage.tsx @@ -9,6 +9,7 @@ import { filesize } from 'filesize' import { useId, useMemo, useState } from 'react' import { useForm } from 'react-hook-form' import { Link, useNavigate, type LoaderFunctionArgs } from 'react-router' +import { match } from 'ts-pattern' import { apiQueryClient, @@ -24,6 +25,7 @@ import { INSTANCE_MAX_CPU, INSTANCE_MAX_RAM_GiB, instanceCan, + instanceCoolingDown, instanceTransitioning, } from '~/api/util' import { ExternalIps } from '~/components/ExternalIps' @@ -107,6 +109,20 @@ InstancePage.loader = async ({ params }: LoaderFunctionArgs) => { const POLL_INTERVAL = 1000 +function shouldPoll(instance: Instance) { + if (instanceTransitioning(instance)) return 'transition' + if (instanceCoolingDown(instance)) return 'cooldown' + return null +} + +const PollingSpinner = () => ( + + + +) + export function InstancePage() { const instanceSelector = useInstanceSelector() const [resizeInstance, setResizeInstance] = useState(false) @@ -131,11 +147,11 @@ export function InstancePage() { }, { refetchInterval: ({ state: { data: instance } }) => - instance && instanceTransitioning(instance) ? POLL_INTERVAL : false, + instance && shouldPoll(instance) ? POLL_INTERVAL : false, } ) - const polling = instanceTransitioning(instance) + const pollReason = shouldPoll(instance) const { data: nics } = usePrefetchedApiQuery('instanceNetworkInterfaceList', { query: { @@ -206,20 +222,11 @@ export function InstancePage() {
- {polling && ( - - - - )} - {instance.autoRestartCooldownExpiration && ( - - )} + {match(pollReason) + .with('transition', () => ) + .with('cooldown', () => ) + .with(null, () => null) + .exhaustive()}
From cbe936255c9faca5ba420c6d698428d5eafd63e1 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 5 Feb 2025 15:45:43 -0600 Subject: [PATCH 21/32] also rely on polling on settings tab --- .../instances/instance/tabs/SettingsTab.tsx | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/app/pages/project/instances/instance/tabs/SettingsTab.tsx b/app/pages/project/instances/instance/tabs/SettingsTab.tsx index 422b6e6f7d..4c03f679b8 100644 --- a/app/pages/project/instances/instance/tabs/SettingsTab.tsx +++ b/app/pages/project/instances/instance/tabs/SettingsTab.tsx @@ -7,7 +7,7 @@ */ import { format, formatDistanceToNow } from 'date-fns' -import { useId, useState, type ReactNode } from 'react' +import { useId, type ReactNode } from 'react' import { useForm } from 'react-hook-form' import { match } from 'ts-pattern' @@ -20,7 +20,6 @@ import { FieldLabel } from '~/ui/lib/FieldLabel' import { type ListboxItem } from '~/ui/lib/Listbox' import { LearnMore, SettingsGroup } from '~/ui/lib/SettingsGroup' import { TipIcon } from '~/ui/lib/TipIcon' -import { useInterval } from '~/ui/lib/use-interval' import { links } from '~/util/links' type FormPolicy = 'default' | 'never' | 'best_effort' @@ -39,12 +38,6 @@ Component.displayName = 'SettingsTab' export function Component() { const instanceSelector = useInstanceSelector() - const [now, setNow] = useState(new Date()) - - // TODO: see if we can tweak the polling condition to include - // failed + cooling down and just rely on polling - useInterval({ fn: () => setNow(new Date()), delay: 1000 }) - const { data: instance } = usePrefetchedApiQuery('instanceView', { path: { instance: instanceSelector.instance }, query: { project: instanceSelector.project }, @@ -118,10 +111,9 @@ export function Component() { new Date(instance.autoRestartCooldownExpiration), 'MMM d, yyyy HH:mm:ss zz' )}{' '} - {new Date(instance.autoRestartCooldownExpiration) > now && ( + {instance.autoRestartCooldownExpiration > new Date() && ( - ({formatDistanceToNow(new Date(instance.autoRestartCooldownExpiration))} - ) + ({formatDistanceToNow(instance.autoRestartCooldownExpiration)}) )} From d9a97c65a16208743591b2e9a1524b2c701214ce Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 6 Feb 2025 23:44:26 -0600 Subject: [PATCH 22/32] bump API client generator for date parsing fix --- app/api/__generated__/util.ts | 11 ++++++++++- package-lock.json | 8 ++++---- package.json | 2 +- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/app/api/__generated__/util.ts b/app/api/__generated__/util.ts index 3aaf5cb07f..a1aba6bb77 100644 --- a/app/api/__generated__/util.ts +++ b/app/api/__generated__/util.ts @@ -43,8 +43,17 @@ export const mapObj = return newObj } +const isoDateRegex = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d{1,6})?Z$/ + export const parseIfDate = (k: string | undefined, v: unknown) => { - if (typeof v === 'string' && (k?.startsWith('time_') || k === 'timestamp')) { + if ( + typeof v === 'string' && + isoDateRegex.test(v) && + (k?.startsWith('time_') || + k?.endsWith('_time') || + k?.endsWith('_expiration') || + k === 'timestamp') + ) { const d = new Date(v) if (isNaN(d.getTime())) return v return d diff --git a/package-lock.json b/package-lock.json index 6826555ac0..f971d1b54d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -56,7 +56,7 @@ "devDependencies": { "@ianvs/prettier-plugin-sort-imports": "^4.4.1", "@mswjs/http-middleware": "^0.10.3", - "@oxide/openapi-gen-ts": "~0.5.0", + "@oxide/openapi-gen-ts": "~0.6.0", "@playwright/test": "^1.50.0", "@testing-library/dom": "^10.4.0", "@testing-library/jest-dom": "^6.6.3", @@ -1606,9 +1606,9 @@ } }, "node_modules/@oxide/openapi-gen-ts": { - "version": "0.5.0", - "resolved": "https://registry.npmjs.org/@oxide/openapi-gen-ts/-/openapi-gen-ts-0.5.0.tgz", - "integrity": "sha512-2dgDlJRrU0TXuZ4pMSpSwLGwljH/d22BB1btKnSVcxq8ChL2i/77lHtpfvbBQlVo9rP4Q6tDrRawBUI9495cnw==", + "version": "0.6.0", + "resolved": "https://registry.npmjs.org/@oxide/openapi-gen-ts/-/openapi-gen-ts-0.6.0.tgz", + "integrity": "sha512-oGu/CRrp859bsvcg74otwIHBQfs0ve3F2xJGpFq+I1H5ExCKtO4sDc++TXuvTCoUY9nlxzObocn0vg9kDMEhfg==", "dev": true, "license": "MPL-2.0", "dependencies": { diff --git a/package.json b/package.json index 2b0426fbf3..9d29162ec9 100644 --- a/package.json +++ b/package.json @@ -78,7 +78,7 @@ "devDependencies": { "@ianvs/prettier-plugin-sort-imports": "^4.4.1", "@mswjs/http-middleware": "^0.10.3", - "@oxide/openapi-gen-ts": "~0.5.0", + "@oxide/openapi-gen-ts": "~0.6.0", "@playwright/test": "^1.50.0", "@testing-library/dom": "^10.4.0", "@testing-library/jest-dom": "^6.6.3", From db52d7759941bc3030309149b5d15b53d94523d7 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 5 Feb 2025 15:52:55 -0600 Subject: [PATCH 23/32] format dates for locale with existing helper --- .../instances/instance/tabs/SettingsTab.tsx | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/app/pages/project/instances/instance/tabs/SettingsTab.tsx b/app/pages/project/instances/instance/tabs/SettingsTab.tsx index 4c03f679b8..4778151cd3 100644 --- a/app/pages/project/instances/instance/tabs/SettingsTab.tsx +++ b/app/pages/project/instances/instance/tabs/SettingsTab.tsx @@ -6,7 +6,7 @@ * Copyright Oxide Computer Company */ -import { format, formatDistanceToNow } from 'date-fns' +import { formatDistanceToNow } from 'date-fns' import { useId, type ReactNode } from 'react' import { useForm } from 'react-hook-form' import { match } from 'ts-pattern' @@ -20,6 +20,7 @@ import { FieldLabel } from '~/ui/lib/FieldLabel' import { type ListboxItem } from '~/ui/lib/Listbox' import { LearnMore, SettingsGroup } from '~/ui/lib/SettingsGroup' import { TipIcon } from '~/ui/lib/TipIcon' +import { toLocaleDateTimeString } from '~/util/date' import { links } from '~/util/links' type FormPolicy = 'default' | 'never' | 'best_effort' @@ -107,10 +108,7 @@ export function Component() { > {instance.autoRestartCooldownExpiration ? ( <> - {format( - new Date(instance.autoRestartCooldownExpiration), - 'MMM d, yyyy HH:mm:ss zz' - )}{' '} + {toLocaleDateTimeString(instance.autoRestartCooldownExpiration)}{' '} {instance.autoRestartCooldownExpiration > new Date() && ( ({formatDistanceToNow(instance.autoRestartCooldownExpiration)}) @@ -126,12 +124,7 @@ export function Component() { helpText="When this instance was last automatically restarted. Empty if never auto-restarted." > {instance.timeLastAutoRestarted ? ( - <> - {format( - new Date(instance.timeLastAutoRestarted), - 'MMM d, yyyy HH:mm:ss zz' - )} - + <>{toLocaleDateTimeString(instance.timeLastAutoRestarted)} ) : ( N/A )} From 2547eccd4fa47cdda86c11ae8ecd35128bc877fd Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 6 Feb 2025 23:59:57 -0600 Subject: [PATCH 24/32] take enabled out of settings form --- .../instances/instance/tabs/SettingsTab.tsx | 41 ++++++++----------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/app/pages/project/instances/instance/tabs/SettingsTab.tsx b/app/pages/project/instances/instance/tabs/SettingsTab.tsx index 4778151cd3..256a3f8afc 100644 --- a/app/pages/project/instances/instance/tabs/SettingsTab.tsx +++ b/app/pages/project/instances/instance/tabs/SettingsTab.tsx @@ -87,37 +87,32 @@ export function Component() { control={form.control} name="autoRestartPolicy" label="Policy" - description="If unconfigured, this instance uses the default auto-restart policy, which may or may not allow it to be restarted." + description="The global default is currently best effort, but this may change in the future." placeholder="Default" items={restartPolicyItems} required className="max-w-none" /> - - {instance.autoRestartEnabled ? 'True' : 'False'}{' '} - {instance.autoRestartEnabled && !instance.autoRestartPolicy && ( - (Project default) - )} - - {instance.autoRestartCooldownExpiration ? ( - <> - {toLocaleDateTimeString(instance.autoRestartCooldownExpiration)}{' '} - {instance.autoRestartCooldownExpiration > new Date() && ( - - ({formatDistanceToNow(instance.autoRestartCooldownExpiration)}) - - )} - - ) : ( - N/A - )} + { + // TODO: show preview of how the time would change on update when + // policy is changed? + instance.autoRestartCooldownExpiration ? ( + <> + {toLocaleDateTimeString(instance.autoRestartCooldownExpiration)}{' '} + {instance.autoRestartCooldownExpiration > new Date() && ( + + ({formatDistanceToNow(instance.autoRestartCooldownExpiration)}) + + )} + + ) : ( + N/A + ) + } Date: Thu, 6 Feb 2025 23:59:57 -0600 Subject: [PATCH 25/32] handle auto restart stuff in msw at update time --- mock-api/instance.ts | 6 +++-- mock-api/msw/handlers.ts | 54 ++++++++++++++++++++++++---------------- 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/mock-api/instance.ts b/mock-api/instance.ts index 3f93b6a4f5..c44fd814ad 100644 --- a/mock-api/instance.ts +++ b/mock-api/instance.ts @@ -5,6 +5,8 @@ * * Copyright Oxide Computer Company */ +import { addMinutes } from 'date-fns' + import type { Instance } from '@oxide/api' import { GiB } from '~/util/units' @@ -42,8 +44,8 @@ const failedInstance: Json = { hostname: 'oxide.com', project_id: project.id, run_state: 'failed', - auto_restart_cooldown_expiration: new Date(Date.now() + 5 * 60 * 1000).toISOString(), // 5 mins in the future - time_last_auto_restarted: new Date(Date.now() - 30 * 24 * 60 * 60 * 1000).toISOString(), // one month ago + auto_restart_cooldown_expiration: addMinutes(new Date(), 5).toISOString(), // 5 minutes from now + time_last_auto_restarted: addMinutes(new Date(), -55).toISOString(), // 55 minutes ago } const startingInstance: Json = { diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 78b2570db9..d8b32a3399 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -5,8 +5,10 @@ * * Copyright Oxide Computer Company */ +import { addHours } from 'date-fns' import { delay } from 'msw' import * as R from 'remeda' +import { match } from 'ts-pattern' import { validate as isUuid, v4 as uuid } from 'uuid' import { @@ -591,23 +593,7 @@ export const handlers = makeHandlers({ return json(newInstance, { status: 201 }) }, - instanceView: ({ path, query }) => { - const instance = lookup.instance({ ...path, ...query }) - - // if empty uses default auto-restart behaviour - // if set, is based off of the policy - // https://github.com/oxidecomputer/omicron/blob/f63ed095e744fb8d2383fda6799eb0b2d6dfbd3c/nexus/db-queries/src/db/datastore/instance.rs#L228C26-L239 - if (instance.auto_restart_policy === 'never') { - instance.auto_restart_enabled = false - } else if ( - instance.auto_restart_policy === 'best_effort' || - !instance.auto_restart_policy // included for posterity but this has to be set in the mock data anyway - ) { - instance.auto_restart_enabled = true - } - - return instance - }, + instanceView: ({ path, query }) => lookup.instance({ ...path, ...query }), instanceUpdate({ path, query, body }) { const instance = lookup.instance({ ...path, ...query }) @@ -616,6 +602,10 @@ export const handlers = makeHandlers({ throw `Instance can only be updated if ${commaSeries(states, 'or')}` } + // always present on the body, always set them + instance.ncpus = body.ncpus + instance.memory = body.memory + if (body.boot_disk) { // Only include project if it's a name, otherwise lookup will error. // This will 404 if the disk doesn't exist, which I think is right. @@ -636,12 +626,34 @@ export const handlers = makeHandlers({ instance.boot_disk_id = undefined } - // undefined or missing is meaningful, unsets the value + // AUTO RESTART + + // Undefined or missing is meaningful: it unsets the value instance.auto_restart_policy = body.auto_restart_policy - // always present on the body, always set them - instance.ncpus = body.ncpus - instance.memory = body.memory + // We depart here from nexus in that nexus does both of the following + // calculations at view time (when converting model to view). We can't + // do that/don't need because our mock DB stores and returns the view + // representation directly. + + // https://github.com/oxidecomputer/omicron/blob/0c6ab099e/nexus/db-queries/src/db/datastore/instance.rs#L228-L239 + instance.auto_restart_enabled = match(instance.auto_restart_policy) + .with(undefined, () => true) + .with('best_effort', () => true) + .with('never', () => false) + .exhaustive() + + // Nexus has something slightly more complicated because it's possible the + // default cooldown of one hour can be overridden at the instance level, but + // that is currently only used in tests, so we should assume all instances + // have the default of 1 hour. It's worth noting this may never come into + // effect unless we deliberately set time_last_auto_restarted on a mock + // instance because the mock API has no ability to actually auto-restart + // an instance. + // https://github.com/oxidecomputer/omicron/blob/0c6ab099e/nexus/db-queries/src/db/datastore/instance.rs#L206-L226 + instance.auto_restart_cooldown_expiration = instance.time_last_auto_restarted + ? addHours(instance.time_last_auto_restarted, 1).toISOString() + : undefined return instance }, From 690b788f07d5bcbcde97dbce714052f5fbb9d7ab Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 7 Feb 2025 12:07:52 -0600 Subject: [PATCH 26/32] update mock API to allow policy update any time --- app/api/util.ts | 15 +++++++++-- app/pages/project/instances/actions.tsx | 4 +-- .../instances/instance/InstancePage.tsx | 2 +- .../instances/instance/tabs/SettingsTab.tsx | 10 +++++++ .../instances/instance/tabs/StorageTab.tsx | 4 +-- mock-api/msw/handlers.ts | 26 ++++++++++++++++--- test/e2e/instance-auto-restart.e2e.ts | 1 + 7 files changed, 52 insertions(+), 10 deletions(-) diff --git a/app/api/util.ts b/app/api/util.ts index 3121c8b176..a1ae7d6146 100644 --- a/app/api/util.ts +++ b/app/api/util.ts @@ -99,8 +99,19 @@ const instanceActions = { // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/instance.rs#L865 delete: ['stopped', 'failed'], - // https://github.com/oxidecomputer/omicron/blob/3093818/nexus/db-queries/src/db/datastore/instance.rs#L1030-L1043 - update: ['stopped', 'failed', 'creating'], + // not all updates are restricted by state + + // resize means changes to ncpus and memory + // https://github.com/oxidecomputer/omicron/blob/0c6ab099e/nexus/db-queries/src/db/datastore/instance.rs#L1282-L1288 + // https://github.com/oxidecomputer/omicron/blob/0c6ab099e/nexus/db-model/src/instance_state.rs#L39-L42 + resize: ['stopped', 'failed', 'creating'], + + // https://github.com/oxidecomputer/omicron/blob/0c6ab099e/nexus/db-queries/src/db/datastore/instance.rs#L1209-L1215 + updateBootDisk: ['stopped', 'failed', 'creating'], + + // there are no state restrictions on setting the auto restart policy, so we + // don't have a helper for it + // https://github.com/oxidecomputer/omicron/blob/0c6ab099e/nexus/db-queries/src/db/datastore/instance.rs#L1050-L1058 // reboot and stop are kind of weird! // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/src/app/instance.rs#L790-L798 diff --git a/app/pages/project/instances/actions.tsx b/app/pages/project/instances/actions.tsx index 748a46eb3f..476c08ae83 100644 --- a/app/pages/project/instances/actions.tsx +++ b/app/pages/project/instances/actions.tsx @@ -147,8 +147,8 @@ export const useMakeInstanceActions = ( { label: 'Resize', onActivate: () => onResizeClick?.(instance), - disabled: !instanceCan.update(instance) && ( - <>Only {fancifyStates(instanceCan.update.states)} instances can be resized + disabled: !instanceCan.resize(instance) && ( + <>Only {fancifyStates(instanceCan.resize.states)} instances can be resized ), }, { diff --git a/app/pages/project/instances/instance/InstancePage.tsx b/app/pages/project/instances/instance/InstancePage.tsx index 3a0db96ec9..a6a7c08db4 100644 --- a/app/pages/project/instances/instance/InstancePage.tsx +++ b/app/pages/project/instances/instance/InstancePage.tsx @@ -314,7 +314,7 @@ export function ResizeInstanceModal({ mode: 'onChange', }) - const canResize = instanceCan.update(instance) + const canResize = instanceCan.resize(instance) const willChange = form.watch('ncpus') !== instance.ncpus || form.watch('memory') !== instance.memory / GiB const isDisabled = !form.formState.isValid || !canResize || !willChange diff --git a/app/pages/project/instances/instance/tabs/SettingsTab.tsx b/app/pages/project/instances/instance/tabs/SettingsTab.tsx index 256a3f8afc..cd5b166fb6 100644 --- a/app/pages/project/instances/instance/tabs/SettingsTab.tsx +++ b/app/pages/project/instances/instance/tabs/SettingsTab.tsx @@ -49,6 +49,13 @@ export function Component() { apiQueryClient.invalidateQueries('instanceView') addToast({ content: 'Instance auto-restart policy updated' }) }, + onError(err) { + addToast({ + title: 'Could not update auto-restart policy', + content: err.message, + variant: 'error', + }) + }, }) const autoRestartPolicy = instance.autoRestartPolicy || 'default' @@ -56,6 +63,9 @@ export function Component() { const form = useForm({ defaultValues }) + // note there are no instance state-based restrictions on updating auto + // restart, so there is no instanceCan helper for it + // https://github.com/oxidecomputer/omicron/blob/0c6ab099e/nexus/db-queries/src/db/datastore/instance.rs#L1050-L1058 const disableSubmit = form.watch('autoRestartPolicy') === autoRestartPolicy const onSubmit = form.handleSubmit((values) => { diff --git a/app/pages/project/instances/instance/tabs/StorageTab.tsx b/app/pages/project/instances/instance/tabs/StorageTab.tsx index b53c167080..8b790aa1ee 100644 --- a/app/pages/project/instances/instance/tabs/StorageTab.tsx +++ b/app/pages/project/instances/instance/tabs/StorageTab.tsx @@ -151,7 +151,7 @@ export function Component() { getSnapshotAction(disk), { label: 'Unset as boot disk', - disabled: !instanceCan.update({ runState: disk.instanceState }) && ( + disabled: !instanceCan.updateBootDisk({ runState: disk.instanceState }) && ( <> Instance must be stopped before boot disk can be changed @@ -213,7 +213,7 @@ export function Component() { getSnapshotAction(disk), { label: 'Set as boot disk', - disabled: !instanceCan.update({ runState: disk.instanceState }) && ( + disabled: !instanceCan.updateBootDisk({ runState: disk.instanceState }) && ( <> Instance must be stopped before boot disk can be changed diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index d8b32a3399..efb684441f 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -597,15 +597,18 @@ export const handlers = makeHandlers({ instanceUpdate({ path, query, body }) { const instance = lookup.instance({ ...path, ...query }) - if (!instanceCan.update({ runState: instance.run_state })) { - const states = instanceCan.update.states - throw `Instance can only be updated if ${commaSeries(states, 'or')}` + const resize = body.ncpus !== instance.ncpus || body.memory !== instance.memory + if (resize && !instanceCan.resize({ runState: instance.run_state })) { + const states = instanceCan.resize.states + throw `Instance can only be resized if ${commaSeries(states, 'or')}` } // always present on the body, always set them instance.ncpus = body.ncpus instance.memory = body.memory + const rejectSetBootDisk = `Boot disk can only be changed if instance is ${commaSeries(instanceCan.updateBootDisk.states, 'or')}` + if (body.boot_disk) { // Only include project if it's a name, otherwise lookup will error. // This will 404 if the disk doesn't exist, which I think is right. @@ -614,6 +617,14 @@ export const handlers = makeHandlers({ project: isUuid(body.boot_disk) ? undefined : query.project, }) + // blow up if we're trying to change the boot disk but instance isn't stopped + if ( + disk.id !== instance.boot_disk_id && + !instanceCan.updateBootDisk({ runState: instance.run_state }) + ) { + throw rejectSetBootDisk + } + const isAttached = disk.state.state === 'attached' && disk.state.instance === instance.id if (!(diskCan.setAsBootDisk(disk) && isAttached)) { @@ -623,6 +634,15 @@ export const handlers = makeHandlers({ instance.boot_disk_id = disk.id } else { // we're clearing the boot disk! + + // if we already have a boot disk, the request is trying to unset it, so blow + // up if that's not allowed + if ( + instance.boot_disk_id && + !instanceCan.updateBootDisk({ runState: instance.run_state }) + ) { + throw rejectSetBootDisk + } instance.boot_disk_id = undefined } diff --git a/test/e2e/instance-auto-restart.e2e.ts b/test/e2e/instance-auto-restart.e2e.ts index 3e7fd64006..8a0c655e7b 100644 --- a/test/e2e/instance-auto-restart.e2e.ts +++ b/test/e2e/instance-auto-restart.e2e.ts @@ -40,3 +40,4 @@ test('Instance auto restart policy', async ({ page }) => { }) // TODO: test that polling updates the relevant stuff +// TODO: test updating policy on a running instance From fad694b1af5a80fd71414163540b6f4016fda46d Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 7 Feb 2025 14:15:02 -0600 Subject: [PATCH 27/32] copy tweaks, fill in tests --- .../instances/instance/tabs/SettingsTab.tsx | 7 ++-- test/e2e/instance-auto-restart.e2e.ts | 40 +++++++++++++++++-- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/app/pages/project/instances/instance/tabs/SettingsTab.tsx b/app/pages/project/instances/instance/tabs/SettingsTab.tsx index cd5b166fb6..8f09b83e77 100644 --- a/app/pages/project/instances/instance/tabs/SettingsTab.tsx +++ b/app/pages/project/instances/instance/tabs/SettingsTab.tsx @@ -98,14 +98,13 @@ export function Component() { name="autoRestartPolicy" label="Policy" description="The global default is currently best effort, but this may change in the future." - placeholder="Default" items={restartPolicyItems} required className="max-w-none" /> { // TODO: show preview of how the time would change on update when @@ -126,10 +125,10 @@ export function Component() { {instance.timeLastAutoRestarted ? ( - <>{toLocaleDateTimeString(instance.timeLastAutoRestarted)} + toLocaleDateTimeString(instance.timeLastAutoRestarted) ) : ( N/A )} diff --git a/test/e2e/instance-auto-restart.e2e.ts b/test/e2e/instance-auto-restart.e2e.ts index 8a0c655e7b..b441a61039 100644 --- a/test/e2e/instance-auto-restart.e2e.ts +++ b/test/e2e/instance-auto-restart.e2e.ts @@ -9,7 +9,7 @@ import { expect, test } from '@playwright/test' import { expectToast } from './utils' -test('Instance auto restart policy', async ({ page }) => { +test('Auto restart policy on failed instance', async ({ page }) => { await page.goto('/projects/mock-project/instances/you-fail') // check popover @@ -22,9 +22,11 @@ test('Instance auto restart policy', async ({ page }) => { // now go to settings tab by clicking link in popover await page.getByRole('link', { name: 'Default' }).click() - // assert contents of table-like thing showing the state + // assert contents of table-like thing showing the state. leave date underspecified + // because it's always 5 minutes of whatever now is + await expect(page.getByText(/Cooldown expiration.+, 202\d.+\(5 minutes\)/)).toBeVisible() + await expect(page.getByText(/Last auto-restarted.+, 202\d/)).toBeVisible() - // await expect(page.getByRole('button', { name: 'Policy' })) const save = page.getByRole('button', { name: 'Save' }) await expect(save).toBeDisabled() @@ -37,7 +39,37 @@ test('Instance auto restart policy', async ({ page }) => { await expectToast(page, 'Instance auto-restart policy updated') await expect(policyListbox).toContainText('Never') + await expect(save).toBeDisabled() }) // TODO: test that polling updates the relevant stuff -// TODO: test updating policy on a running instance + +// unlike the other instance update things, you can change auto restart policy +// regardless of state +test('Auto restart policy on running instance', async ({ page }) => { + await page.goto('/projects/mock-project/instances/db1') + + await expect(page.getByText('Running')).toBeVisible() // it's running. we know + + // go to settings tab + await page.getByRole('tab', { name: 'settings' }).click() + + // assert contents of table-like thing showing the state + await expect(page.getByText('Cooldown expirationN/A')).toBeVisible() + await expect(page.getByText('Last auto-restartedN/A')).toBeVisible() + + // await expect(page.getByRole('button', { name: 'Policy' })) + const save = page.getByRole('button', { name: 'Save' }) + await expect(save).toBeDisabled() + + const policyListbox = page.getByRole('button', { name: 'Policy' }) + await expect(policyListbox).toContainText('Default') + + await page.getByRole('button', { name: 'Policy' }).click() + await page.getByRole('option', { name: 'Never' }).click() + await save.click() + + await expectToast(page, 'Instance auto-restart policy updated') + await expect(policyListbox).toContainText('Never') + await expect(save).toBeDisabled() +}) From d707200e49db8bf169313d9440ee9370426be70b Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 7 Feb 2025 14:15:02 -0600 Subject: [PATCH 28/32] poll slower for failed instances --- .../instances/instance/InstancePage.tsx | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/app/pages/project/instances/instance/InstancePage.tsx b/app/pages/project/instances/instance/InstancePage.tsx index a6a7c08db4..678c6fb3a4 100644 --- a/app/pages/project/instances/instance/InstancePage.tsx +++ b/app/pages/project/instances/instance/InstancePage.tsx @@ -107,8 +107,15 @@ InstancePage.loader = async ({ params }: LoaderFunctionArgs) => { return null } -const POLL_INTERVAL = 1000 +const sec = 1000 // ms, obviously +const POLL_INTERVAL_FAST = 1 * sec +const POLL_INTERVAL_SLOW = 60 * sec +// We're using this logic here on instance detail, but not on instance list. +// Instance list will only poll for the transitioning case. We don't show +// anything about cooldown on the instance list, so the only point of polling +// would be to catch a restart when it happens. But with the cooldown period +// being an hour, we'd be doing a _lot_ of unnecessary polling on the list page. function shouldPoll(instance: Instance) { if (instanceTransitioning(instance)) return 'transition' if (instanceCoolingDown(instance)) return 'cooldown' @@ -146,8 +153,14 @@ export function InstancePage() { query: { project: instanceSelector.project }, }, { - refetchInterval: ({ state: { data: instance } }) => - instance && shouldPoll(instance) ? POLL_INTERVAL : false, + refetchInterval: ({ state: { data: instance } }) => { + const pollReason = instance ? shouldPoll(instance) : null + return match(pollReason) + .with('transition', () => POLL_INTERVAL_FAST) + .with('cooldown', () => POLL_INTERVAL_SLOW) + .with(null, () => false as const) + .exhaustive() + }, } ) From a3871c23fa7035ee9731ea75c606e7de59db7bd9 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Mon, 10 Feb 2025 15:12:11 -0600 Subject: [PATCH 29/32] simplify FormMeta, no Label needed --- .../instances/instance/InstancePage.tsx | 3 +- .../instances/instance/tabs/SettingsTab.tsx | 38 ++++++++----------- app/ui/lib/TipIcon.tsx | 1 + 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/app/pages/project/instances/instance/InstancePage.tsx b/app/pages/project/instances/instance/InstancePage.tsx index 678c6fb3a4..65386d92f6 100644 --- a/app/pages/project/instances/instance/InstancePage.tsx +++ b/app/pages/project/instances/instance/InstancePage.tsx @@ -107,9 +107,10 @@ InstancePage.loader = async ({ params }: LoaderFunctionArgs) => { return null } +// both a little faster than the default on the list view const sec = 1000 // ms, obviously const POLL_INTERVAL_FAST = 1 * sec -const POLL_INTERVAL_SLOW = 60 * sec +const POLL_INTERVAL_SLOW = 30 * sec // We're using this logic here on instance detail, but not on instance list. // Instance list will only poll for the transitioning case. We don't show diff --git a/app/pages/project/instances/instance/tabs/SettingsTab.tsx b/app/pages/project/instances/instance/tabs/SettingsTab.tsx index 8f09b83e77..cc3fbe4275 100644 --- a/app/pages/project/instances/instance/tabs/SettingsTab.tsx +++ b/app/pages/project/instances/instance/tabs/SettingsTab.tsx @@ -7,7 +7,7 @@ */ import { formatDistanceToNow } from 'date-fns' -import { useId, type ReactNode } from 'react' +import { type ReactNode } from 'react' import { useForm } from 'react-hook-form' import { match } from 'ts-pattern' @@ -16,7 +16,6 @@ import { ListboxField } from '~/components/form/fields/ListboxField' import { useInstanceSelector } from '~/hooks/use-params' import { addToast } from '~/stores/toast' import { Button } from '~/ui/lib/Button' -import { FieldLabel } from '~/ui/lib/FieldLabel' import { type ListboxItem } from '~/ui/lib/Listbox' import { LearnMore, SettingsGroup } from '~/ui/lib/SettingsGroup' import { TipIcon } from '~/ui/lib/TipIcon' @@ -104,7 +103,7 @@ export function Component() { /> { // TODO: show preview of how the time would change on update when @@ -125,7 +124,7 @@ export function Component() { {instance.timeLastAutoRestarted ? ( toLocaleDateTimeString(instance.timeLastAutoRestarted) @@ -147,25 +146,18 @@ export function Component() { ) } -const FormMeta = ({ - label, - helpText, - children, -}: { +type FormMetaProps = { label: string - helpText?: string + tip?: string children: ReactNode -}) => { - const id = useId() - return ( -
-
- - {label} - - {helpText && {helpText}} -
- {children} -
- ) } + +const FormMeta = ({ label, tip, children }: FormMetaProps) => ( +
+
+
{label}
+ {tip && {tip}} +
+ {children} +
+) diff --git a/app/ui/lib/TipIcon.tsx b/app/ui/lib/TipIcon.tsx index 91e6f33ec6..dec07d18f3 100644 --- a/app/ui/lib/TipIcon.tsx +++ b/app/ui/lib/TipIcon.tsx @@ -21,6 +21,7 @@ export function TipIcon({ children, className }: TipIconProps) { From a641b898279ecd33b18ea107ca5e6fd31df1fd30 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Mon, 10 Feb 2025 22:36:13 -0600 Subject: [PATCH 30/32] fix up polling and popover logic --- app/api/util.ts | 5 -- .../instances/instance/InstancePage.tsx | 52 +++++++++---------- 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/app/api/util.ts b/app/api/util.ts index a1ae7d6146..9207914631 100644 --- a/app/api/util.ts +++ b/app/api/util.ts @@ -151,11 +151,6 @@ export function instanceTransitioning({ runState }: Instance) { ) } -/** Cooling down after failed auto-restart */ -export function instanceCoolingDown(i: Instance) { - return i.runState === 'failed' && i.autoRestartCooldownExpiration -} - const diskActions = { // this is a weird one because the list of states is dynamic and it includes // 'creating' in the unwind of the disk create saga, but does not include diff --git a/app/pages/project/instances/instance/InstancePage.tsx b/app/pages/project/instances/instance/InstancePage.tsx index 65386d92f6..1b34e906dc 100644 --- a/app/pages/project/instances/instance/InstancePage.tsx +++ b/app/pages/project/instances/instance/InstancePage.tsx @@ -9,7 +9,6 @@ import { filesize } from 'filesize' import { useId, useMemo, useState } from 'react' import { useForm } from 'react-hook-form' import { Link, useNavigate, type LoaderFunctionArgs } from 'react-router' -import { match } from 'ts-pattern' import { apiQueryClient, @@ -25,7 +24,6 @@ import { INSTANCE_MAX_CPU, INSTANCE_MAX_RAM_GiB, instanceCan, - instanceCoolingDown, instanceTransitioning, } from '~/api/util' import { ExternalIps } from '~/components/ExternalIps' @@ -109,20 +107,9 @@ InstancePage.loader = async ({ params }: LoaderFunctionArgs) => { // both a little faster than the default on the list view const sec = 1000 // ms, obviously -const POLL_INTERVAL_FAST = 1 * sec +const POLL_INTERVAL_FAST = 2 * sec const POLL_INTERVAL_SLOW = 30 * sec -// We're using this logic here on instance detail, but not on instance list. -// Instance list will only poll for the transitioning case. We don't show -// anything about cooldown on the instance list, so the only point of polling -// would be to catch a restart when it happens. But with the cooldown period -// being an hour, we'd be doing a _lot_ of unnecessary polling on the list page. -function shouldPoll(instance: Instance) { - if (instanceTransitioning(instance)) return 'transition' - if (instanceCoolingDown(instance)) return 'cooldown' - return null -} - const PollingSpinner = () => (
- {cooldownExpiration && ( + {enabled && cooldownExpiration && ( - {isQueued ? ( + {restartingSoon ? ( <> - Queued for restart… + Restarting soon… ) : (
@@ -121,8 +96,19 @@ export const InstanceAutoRestartPopover = ({ instance }: { instance: Instance }) )}
-

{helpText[helpTextState]}

- +

+ {enabled + ? restartingSoon + ? 'This instance will automatically restart soon.' + : 'This instance will automatically restart after the cooldown period.' + : 'This instance will not automatically restart.'} +

+ Learn about{' '} diff --git a/app/pages/project/instances/instance/InstancePage.tsx b/app/pages/project/instances/instance/InstancePage.tsx index 1b34e906dc..c0854dde30 100644 --- a/app/pages/project/instances/instance/InstancePage.tsx +++ b/app/pages/project/instances/instance/InstancePage.tsx @@ -236,9 +236,7 @@ export function InstancePage() {
{instanceTransitioning(instance) && } - {instance.runState === 'failed' && ( - - )} +
diff --git a/app/pages/project/instances/instance/tabs/SettingsTab.tsx b/app/pages/project/instances/instance/tabs/SettingsTab.tsx index cc3fbe4275..fd087e79c8 100644 --- a/app/pages/project/instances/instance/tabs/SettingsTab.tsx +++ b/app/pages/project/instances/instance/tabs/SettingsTab.tsx @@ -105,22 +105,22 @@ export function Component() { label="Cooldown expiration" tip="When this instance will next restart (if in a failed state and the policy allows it). If N/A, then either the instance has never been automatically restarted, or the cooldown period has expired." > - { - // TODO: show preview of how the time would change on update when - // policy is changed? - instance.autoRestartCooldownExpiration ? ( - <> - {toLocaleDateTimeString(instance.autoRestartCooldownExpiration)}{' '} - {instance.autoRestartCooldownExpiration > new Date() && ( - - ({formatDistanceToNow(instance.autoRestartCooldownExpiration)}) - - )} - - ) : ( - N/A - ) - } + {instance.autoRestartCooldownExpiration ? ( + <> + {toLocaleDateTimeString(instance.autoRestartCooldownExpiration)}{' '} + {instance.runState === 'failed' && instance.autoRestartEnabled && ( + + ( + {instance.autoRestartCooldownExpiration < new Date() + ? 'restarting soon' + : formatDistanceToNow(instance.autoRestartCooldownExpiration)} + ) + + )} + + ) : ( + N/A + )} = { ...base, id: '935499b3-fd96-432a-9c21-83a3dc1eece4', name: 'db1', - ncpus: 2, - memory: 4 * GiB, description: 'an instance', hostname: 'oxide.com', project_id: project.id, @@ -60,4 +60,57 @@ const startingInstance: Json = { run_state: 'starting', } -export const instances: Json[] = [instance, failedInstance, startingInstance] +// failed instances in other-project for testing auto restart popover + +const failedRestartingSoon: Json = { + ...base, + id: 'a2fddf77-c053-4648-9cda-19976c395059', + name: 'failed-restarting-soon', + description: 'a failed instance', + hostname: 'oxide.com', + project_id: project2.id, + + auto_restart_enabled: true, + run_state: 'failed', + auto_restart_cooldown_expiration: addMinutes(new Date(), -0.5).toISOString(), // 30 seconds ago + time_last_auto_restarted: addMinutes(new Date(), -60.5).toISOString(), // hour and 30 secs ago +} + +const failedRestartNever: Json = { + ...base, + id: 'ad8d6f3f-d0c9-4a7f-aecf-2af267483dab', + name: 'failed-restart-never', + description: 'a failed instance', + hostname: 'oxide.com', + project_id: project2.id, + + auto_restart_enabled: false, + auto_restart_policy: 'never', + run_state: 'failed', + auto_restart_cooldown_expiration: addMinutes(new Date(), 5).toISOString(), // 5 minutes from now + time_last_auto_restarted: addMinutes(new Date(), -55).toISOString(), // 55 minutes ago +} + +const failedCooledRestartNever: Json = { + ...base, + id: '32a0249f-3a5c-4d30-a154-2476e372aa62', + name: 'failed-cooled-restart-never', + description: 'a failed instance', + hostname: 'oxide.com', + project_id: project2.id, + + auto_restart_enabled: false, + auto_restart_policy: 'never', + run_state: 'failed', + auto_restart_cooldown_expiration: addMinutes(new Date(), -5).toISOString(), // 5 minutes ago + time_last_auto_restarted: addMinutes(new Date(), -65).toISOString(), // 65 minutes ago +} + +export const instances: Json[] = [ + instance, + failedInstance, + startingInstance, + failedRestartingSoon, + failedRestartNever, + failedCooledRestartNever, +] diff --git a/test/e2e/instance-auto-restart.e2e.ts b/test/e2e/instance-auto-restart.e2e.ts index b441a61039..37a171154c 100644 --- a/test/e2e/instance-auto-restart.e2e.ts +++ b/test/e2e/instance-auto-restart.e2e.ts @@ -42,8 +42,6 @@ test('Auto restart policy on failed instance', async ({ page }) => { await expect(save).toBeDisabled() }) -// TODO: test that polling updates the relevant stuff - // unlike the other instance update things, you can change auto restart policy // regardless of state test('Auto restart policy on running instance', async ({ page }) => { @@ -73,3 +71,73 @@ test('Auto restart policy on running instance', async ({ page }) => { await expect(policyListbox).toContainText('Never') await expect(save).toBeDisabled() }) + +test('Auto restart popover, restarting soon', async ({ page }) => { + await page.goto('/projects/other-project/instances/failed-restarting-soon') + + // check popover + const indicator = page.getByRole('button', { name: 'Auto-restart status' }) + await indicator.click() + await expect(page.getByText('Auto RestartEnabled')).toBeVisible() + await expect(page.getByText('PolicyDefault')).toBeVisible() + await expect(page.getByText('Restarting soon…')).toBeVisible() + await expect(page.getByText('instance will automatically restart soon')).toBeVisible() + + // go to settings tab + await page.getByRole('link', { name: 'Default' }).click() + + // assert contents of table-like thing showing the state + await expect( + page.getByText(/Cooldown expiration.+, 202\d.+\(restarting soon\)/) + ).toBeVisible() + await expect(page.getByText(/Last auto-restarted.+, 202\d/)).toBeVisible() + + const policyListbox = page.getByRole('button', { name: 'Policy' }) + await expect(policyListbox).toContainText('Default') + await expect(page.getByRole('button', { name: 'Save' })).toBeDisabled() +}) + +test('Auto restart popover, policy never', async ({ page }) => { + await page.goto('/projects/other-project/instances/failed-restart-never') + + // check popover + const indicator = page.getByRole('button', { name: 'Auto-restart status' }) + await indicator.click() + await expect(page.getByText('Auto RestartDisabled')).toBeVisible() + await expect(page.getByText('PolicyNever')).toBeVisible() + await expect(page.getByText('Cooldown')).toBeHidden() + await expect(page.getByText('instance will not automatically restart')).toBeVisible() + + // go to settings tab + await page.getByRole('link', { name: 'never', exact: true }).click() + + await expect(page.getByText(/Cooldown expiration.+, 202\d.+/)).toBeVisible() + await expect(page.getByText(/Last auto-restarted.+, 202\d/)).toBeVisible() + + const policyListbox = page.getByRole('button', { name: 'Policy' }) + await expect(policyListbox).toContainText('Never') + await expect(page.getByRole('button', { name: 'Save' })).toBeDisabled() +}) + +test('Auto restart popover, cooled, policy never, cooled', async ({ page }) => { + await page.goto('/projects/other-project/instances/failed-cooled-restart-never') + + // check popover + const indicator = page.getByRole('button', { name: 'Auto-restart status' }) + await indicator.click() + await expect(page.getByText('Auto RestartDisabled')).toBeVisible() + await expect(page.getByText('PolicyNever')).toBeVisible() + await expect(page.getByText('Cooldown')).toBeHidden() + await expect(page.getByText('instance will not automatically restart')).toBeVisible() + + // go to settings tab + await page.getByRole('link', { name: 'never', exact: true }).click() + + await expect(page.getByText(/Cooldown expiration.+, 202\d.+/)).toBeVisible() + await expect(page.getByText('restarting soon')).toBeHidden() + await expect(page.getByText(/Last auto-restarted.+, 202\d/)).toBeVisible() + + const policyListbox = page.getByRole('button', { name: 'Policy' }) + await expect(policyListbox).toContainText('Never') + await expect(page.getByRole('button', { name: 'Save' })).toBeDisabled() +}) From 6c413e9067a8d473b4c1dc854ebf727af581b7ba Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 11 Feb 2025 12:42:51 -0600 Subject: [PATCH 32/32] instanceAutoRestartingSoon helper --- app/api/util.ts | 18 ++++++++++++++++++ app/components/InstanceAutoRestartPopover.tsx | 9 +++------ .../instances/instance/InstancePage.tsx | 13 ++++--------- .../instances/instance/tabs/SettingsTab.tsx | 9 +++++++-- 4 files changed, 32 insertions(+), 17 deletions(-) diff --git a/app/api/util.ts b/app/api/util.ts index 9207914631..9cbb13ba40 100644 --- a/app/api/util.ts +++ b/app/api/util.ts @@ -151,6 +151,24 @@ export function instanceTransitioning({ runState }: Instance) { ) } +/** + * True if instance is failed, auto restart enabled, and either we've never + * restarted or we just expired and we're waiting to get restarted. + * + * There can be a short window (up to a minute) after expiration where the + * instance hasn't been picked up for auto-restart yet. + * + * https://github.com/oxidecomputer/omicron/blob/b6ada022a/nexus/src/app/background/init.rs#L726-L745 + * https://github.com/oxidecomputer/omicron/blob/b6ada022a/smf/nexus/multi-sled/config-partial.toml#L70-L71 + */ +export function instanceAutoRestartingSoon(i: Instance) { + return ( + i.runState === 'failed' && + i.autoRestartEnabled && + (!i.autoRestartCooldownExpiration || i.autoRestartCooldownExpiration < new Date()) + ) +} + const diskActions = { // this is a weird one because the list of states is dynamic and it includes // 'creating' in the unwind of the disk create saga, but does not include diff --git a/app/components/InstanceAutoRestartPopover.tsx b/app/components/InstanceAutoRestartPopover.tsx index 7e29753e4a..6cc7b3eb45 100644 --- a/app/components/InstanceAutoRestartPopover.tsx +++ b/app/components/InstanceAutoRestartPopover.tsx @@ -18,6 +18,7 @@ import { } from '@oxide/design-system/icons/react' import type { Instance } from '~/api' +import { instanceAutoRestartingSoon } from '~/api/util' import { useInstanceSelector } from '~/hooks/use-params' import { Badge } from '~/ui/lib/Badge' import { Spinner } from '~/ui/lib/Spinner' @@ -28,6 +29,7 @@ import { pb } from '~/util/path-builder' * Appears if and only if the instance is failed. */ export const InstanceAutoRestartPopover = ({ instance }: { instance: Instance }) => { + const instanceSelector = useInstanceSelector() if (instance.runState !== 'failed') return null // important! const { @@ -36,12 +38,7 @@ export const InstanceAutoRestartPopover = ({ instance }: { instance: Instance }) autoRestartEnabled: enabled, } = instance - const instanceSelector = useInstanceSelector() - - // true if either we've never restarted or we just expired and we're waiting - // to get restarted (should always be under a minute because that's how often - // the cleanup job runs) - const restartingSoon = enabled && (!cooldownExpiration || cooldownExpiration < new Date()) + const restartingSoon = instanceAutoRestartingSoon(instance) return ( diff --git a/app/pages/project/instances/instance/InstancePage.tsx b/app/pages/project/instances/instance/InstancePage.tsx index c0854dde30..60d0a50a1d 100644 --- a/app/pages/project/instances/instance/InstancePage.tsx +++ b/app/pages/project/instances/instance/InstancePage.tsx @@ -23,6 +23,7 @@ import { Instances24Icon } from '@oxide/design-system/icons/react' import { INSTANCE_MAX_CPU, INSTANCE_MAX_RAM_GiB, + instanceAutoRestartingSoon, instanceCan, instanceTransitioning, } from '~/api/util' @@ -152,15 +153,9 @@ export function InstancePage() { if (instanceTransitioning(instance)) return POLL_INTERVAL_FAST if (instance.runState === 'failed' && instance.autoRestartEnabled) { - // There can be a short window (up to a minute, based on the links - // below) after expiration where the instance hasn't been picked up - // for auto-restart yet, during which time we still want to poll. - // https://github.com/oxidecomputer/omicron/blob/b6ada022a/nexus/src/app/background/init.rs#L726-L745 - // https://github.com/oxidecomputer/omicron/blob/b6ada022a/smf/nexus/multi-sled/config-partial.toml#L70-L71 - const restartingSoon = - !instance.autoRestartCooldownExpiration || - instance.autoRestartCooldownExpiration < new Date() - return restartingSoon ? POLL_INTERVAL_FAST : POLL_INTERVAL_SLOW + return instanceAutoRestartingSoon(instance) + ? POLL_INTERVAL_FAST + : POLL_INTERVAL_SLOW } }, } diff --git a/app/pages/project/instances/instance/tabs/SettingsTab.tsx b/app/pages/project/instances/instance/tabs/SettingsTab.tsx index fd087e79c8..c75492fc86 100644 --- a/app/pages/project/instances/instance/tabs/SettingsTab.tsx +++ b/app/pages/project/instances/instance/tabs/SettingsTab.tsx @@ -11,7 +11,12 @@ import { type ReactNode } from 'react' import { useForm } from 'react-hook-form' import { match } from 'ts-pattern' -import { apiQueryClient, useApiMutation, usePrefetchedApiQuery } from '~/api' +import { + apiQueryClient, + instanceAutoRestartingSoon, + useApiMutation, + usePrefetchedApiQuery, +} from '~/api' import { ListboxField } from '~/components/form/fields/ListboxField' import { useInstanceSelector } from '~/hooks/use-params' import { addToast } from '~/stores/toast' @@ -111,7 +116,7 @@ export function Component() { {instance.runState === 'failed' && instance.autoRestartEnabled && ( ( - {instance.autoRestartCooldownExpiration < new Date() + {instanceAutoRestartingSoon(instance) ? 'restarting soon' : formatDistanceToNow(instance.autoRestartCooldownExpiration)} )