diff --git a/OMICRON_VERSION b/OMICRON_VERSION index 451cd1c94b..f7a1d4ae62 100644 --- a/OMICRON_VERSION +++ b/OMICRON_VERSION @@ -1 +1 @@ -d2e3f344e82fd7a1f45a39a9d1f867f9238d171d +c50cf019cd9be35f98266a7f4acacab0236b3a3d diff --git a/app/api/__generated__/Api.ts b/app/api/__generated__/Api.ts index 6a327138ef..e64990f1f9 100644 --- a/app/api/__generated__/Api.ts +++ b/app/api/__generated__/Api.ts @@ -1320,10 +1320,10 @@ export type DiskSource = */ export type DiskCreate = { description: string - /** initial source for this disk */ + /** The initial source for this disk */ diskSource: DiskSource name: Name - /** total size of the Disk in bytes */ + /** The total size of the Disk (in bytes) */ size: ByteCount } @@ -1755,6 +1755,14 @@ export type InstanceState = * View of an Instance */ export type Instance = { + /** The time at which the auto-restart cooldown period for this instance completes, permitting it to be automatically restarted again. If the instance enters the `Failed` state, it will not be restarted until after this time. + +If this is not present, then either the instance has never been automatically restarted, or the cooldown period has already expired, allowing the instance to be restarted immediately if it fails. */ + autoRestartCooldownExpiration?: Date + /** `true` if this instance's auto-restart policy will permit the control plane to automatically restart it if it enters the `Failed` state. */ + autoRestartEnabled: boolean + /** the ID of the disk used to boot this Instance, if a specific one is assigned. */ + bootDiskId?: string /** human-readable free-form text about a resource */ description: string /** RFC1035-compliant hostname for the Instance. */ @@ -1772,11 +1780,25 @@ export type Instance = { runState: InstanceState /** timestamp when this resource was created */ timeCreated: Date + /** The timestamp of the most recent time this instance was automatically restarted by the control plane. + +If this is not present, then this instance has not been automatically restarted. */ + timeLastAutoRestarted?: Date /** timestamp when this resource was last modified */ timeModified: Date timeRunStateUpdated: Date } +/** + * A policy determining when an instance should be automatically restarted by the control plane. + */ +export type InstanceAutoRestartPolicy = + /** The instance should not be automatically restarted by the control plane if it fails. */ + | 'never' + + /** If this instance is running and unexpectedly fails (e.g. due to a host software crash or unexpected host reboot), the control plane will make a best-effort attempt to restart it. The control plane may choose not to restart the instance to preserve the overall availability of the system. */ + | 'best_effort' + /** * Describe the instance's disks at creation time */ @@ -1784,10 +1806,10 @@ export type InstanceDiskAttachment = /** During instance creation, create and attach disks */ | { description: string - /** initial source for this disk */ + /** The initial source for this disk */ diskSource: DiskSource name: Name - /** total size of the Disk in bytes */ + /** The total size of the Disk (in bytes) */ size: ByteCount type: 'create' } @@ -1829,6 +1851,16 @@ If more than one interface is provided, then the first will be designated the pr * Create-time parameters for an `Instance` */ export type InstanceCreate = { + /** The auto-restart policy for this instance. + +This indicates whether the instance should be automatically restarted by the control plane on failure. If this is `null`, no auto-restart policy has been configured for this instance by the user. */ + autoRestartPolicy?: InstanceAutoRestartPolicy + /** The disk this instance should boot into. This disk can either be attached if it already exists, or created, if it should be a new disk. + +It is strongly recommended to either provide a boot disk at instance creation, or update the instance after creation to set a boot disk. + +An instance without an explicit boot disk can be booted: the options are as managed by UEFI, and as controlled by the guest OS, but with some risk. If this instance later has a disk attached or detached, it is possible that boot options can end up reordered, with the intended boot disk moved after the EFI shell in boot priority. This may result in an instance that only boots to the EFI shell until the desired disk is set as an explicit boot disk and the instance rebooted. */ + bootDisk?: InstanceDiskAttachment description: string /** The disks to be created or attached for this instance. */ disks?: InstanceDiskAttachment[] @@ -1836,9 +1868,12 @@ export type InstanceCreate = { By default, all instances have outbound connectivity, but no inbound connectivity. These external addresses can be used to provide a fixed, known IP address for making inbound connections to the instance. */ externalIps?: ExternalIpCreate[] + /** The hostname to be assigned to the instance */ hostname: Hostname + /** The amount of RAM (in bytes) to be allocated to the instance */ memory: ByteCount name: Name + /** The number of vCPUs to be allocated to the instance */ ncpus: InstanceCpuCount /** The network interfaces to be created for this instance. */ networkInterfaces?: InstanceNetworkInterfaceAttachment @@ -1937,6 +1972,16 @@ export type InstanceSerialConsoleData = { lastByteOffset: number } +/** + * Parameters of an `Instance` that can be reconfigured after creation. + */ +export type InstanceUpdate = { + /** Name or ID of the disk the instance should be instructed to boot from. + +If not provided, unset the instance's boot disk. */ + bootDisk?: NameOrId +} + /** * A collection of IP ranges. If a pool is linked to a silo, IP addresses from the pool can be allocated within that silo */ @@ -4196,6 +4241,14 @@ export interface InstanceViewQueryParams { project?: NameOrId } +export interface InstanceUpdatePathParams { + instance: NameOrId +} + +export interface InstanceUpdateQueryParams { + project?: NameOrId +} + export interface InstanceDeletePathParams { instance: NameOrId } @@ -5688,6 +5741,29 @@ export class Api extends HttpClient { ...params, }) }, + /** + * Update instance + */ + instanceUpdate: ( + { + path, + query = {}, + body, + }: { + path: InstanceUpdatePathParams + query?: InstanceUpdateQueryParams + body: InstanceUpdate + }, + params: FetchParams = {} + ) => { + return this.request({ + path: `/v1/instances/${path.instance}`, + method: 'PUT', + body, + query, + ...params, + }) + }, /** * Delete instance */ diff --git a/app/api/__generated__/OMICRON_VERSION b/app/api/__generated__/OMICRON_VERSION index ffdb941808..2c1de3ebde 100644 --- a/app/api/__generated__/OMICRON_VERSION +++ b/app/api/__generated__/OMICRON_VERSION @@ -1,2 +1,2 @@ # generated file. do not update manually. see docs/update-pinned-api.md -d2e3f344e82fd7a1f45a39a9d1f867f9238d171d +c50cf019cd9be35f98266a7f4acacab0236b3a3d diff --git a/app/api/__generated__/msw-handlers.ts b/app/api/__generated__/msw-handlers.ts index 27fb72c2f2..49df51a418 100644 --- a/app/api/__generated__/msw-handlers.ts +++ b/app/api/__generated__/msw-handlers.ts @@ -305,6 +305,14 @@ export interface MSWHandlers { req: Request cookies: Record }) => Promisable> + /** `PUT /v1/instances/:instance` */ + instanceUpdate: (params: { + path: Api.InstanceUpdatePathParams + query: Api.InstanceUpdateQueryParams + body: Json + req: Request + cookies: Record + }) => Promisable> /** `DELETE /v1/instances/:instance` */ instanceDelete: (params: { path: Api.InstanceDeletePathParams @@ -1593,6 +1601,14 @@ export function makeHandlers(handlers: MSWHandlers): HttpHandler[] { '/v1/instances/:instance', handler(handlers['instanceView'], schema.InstanceViewParams, null) ), + http.put( + '/v1/instances/:instance', + handler( + handlers['instanceUpdate'], + schema.InstanceUpdateParams, + schema.InstanceUpdate + ) + ), http.delete( '/v1/instances/:instance', handler(handlers['instanceDelete'], schema.InstanceDeleteParams, null) diff --git a/app/api/__generated__/validate.ts b/app/api/__generated__/validate.ts index 5b1902e28c..77402ef0df 100644 --- a/app/api/__generated__/validate.ts +++ b/app/api/__generated__/validate.ts @@ -1680,6 +1680,9 @@ export const InstanceState = z.preprocess( export const Instance = z.preprocess( processResponseBody, z.object({ + autoRestartCooldownExpiration: z.coerce.date().optional(), + autoRestartEnabled: SafeBoolean, + bootDiskId: z.string().uuid().optional(), description: z.string(), hostname: z.string(), id: z.string().uuid(), @@ -1689,11 +1692,20 @@ export const Instance = z.preprocess( projectId: z.string().uuid(), runState: InstanceState, timeCreated: z.coerce.date(), + timeLastAutoRestarted: z.coerce.date().optional(), timeModified: z.coerce.date(), timeRunStateUpdated: z.coerce.date(), }) ) +/** + * A policy determining when an instance should be automatically restarted by the control plane. + */ +export const InstanceAutoRestartPolicy = z.preprocess( + processResponseBody, + z.enum(['never', 'best_effort']) +) + /** * Describe the instance's disks at creation time */ @@ -1743,6 +1755,8 @@ export const InstanceNetworkInterfaceAttachment = z.preprocess( export const InstanceCreate = z.preprocess( processResponseBody, z.object({ + autoRestartPolicy: InstanceAutoRestartPolicy.default(null).optional(), + bootDisk: InstanceDiskAttachment.default(null).optional(), description: z.string(), disks: InstanceDiskAttachment.array().default([]).optional(), externalIps: ExternalIpCreate.array().default([]).optional(), @@ -1833,6 +1847,14 @@ export const InstanceSerialConsoleData = z.preprocess( z.object({ data: z.number().min(0).max(255).array(), lastByteOffset: z.number().min(0) }) ) +/** + * Parameters of an `Instance` that can be reconfigured after creation. + */ +export const InstanceUpdate = z.preprocess( + processResponseBody, + z.object({ bootDisk: NameOrId.optional() }) +) + /** * A collection of IP ranges. If a pool is linked to a silo, IP addresses from the pool can be allocated within that silo */ @@ -4148,6 +4170,18 @@ export const InstanceViewParams = z.preprocess( }) ) +export const InstanceUpdateParams = z.preprocess( + processResponseBody, + z.object({ + path: z.object({ + instance: NameOrId, + }), + query: z.object({ + project: NameOrId.optional(), + }), + }) +) + export const InstanceDeleteParams = z.preprocess( processResponseBody, z.object({ diff --git a/app/api/util.ts b/app/api/util.ts index 5d997c02c4..52bb61eb54 100644 --- a/app/api/util.ts +++ b/app/api/util.ts @@ -90,12 +90,18 @@ export const genName = (...parts: [string, ...string[]]) => { } const instanceActions: Record = { + // NoVmm maps to to Stopped: + // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-model/src/instance_state.rs#L55 + // https://github.com/oxidecomputer/omicron/blob/0496637/nexus/src/app/instance.rs#L2064 start: ['stopped', 'failed'], // 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'], + // reboot and stop are kind of weird! // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/src/app/instance.rs#L790-L798 // https://github.com/oxidecomputer/propolis/blob/b278193/bin/propolis-server/src/lib/vm/request_queue.rs @@ -103,9 +109,6 @@ const instanceActions: Record = { reboot: ['running'], // technically rebooting allowed but too weird to say it stop: ['running', 'starting', 'rebooting'], - // NoVmm maps to to Stopped: - // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-model/src/instance_state.rs#L55 - // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/disk.rs#L323-L327 detachDisk: ['creating', 'stopped', 'failed'], // only Creating and NoVmm @@ -148,6 +151,8 @@ const diskActions: Record = { attach: ['creating', 'detached'], // https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/disk.rs#L313-L314 detach: ['attached'], + // https://github.com/oxidecomputer/omicron/blob/3093818/nexus/db-queries/src/db/datastore/instance.rs#L1077-L1081 + setAsBootDisk: ['attached'], } export const diskCan = R.mapValues(diskActions, (states) => { diff --git a/app/components/form/fields/DisksTableField.tsx b/app/components/form/fields/DisksTableField.tsx index f8389ee650..4084c7eacd 100644 --- a/app/components/form/fields/DisksTableField.tsx +++ b/app/components/form/fields/DisksTableField.tsx @@ -15,7 +15,6 @@ import { CreateDiskSideModalForm } from '~/forms/disk-create' import type { InstanceCreateInput } from '~/forms/instance-create' import { Badge } from '~/ui/lib/Badge' import { Button } from '~/ui/lib/Button' -import { FieldLabel } from '~/ui/lib/FieldLabel' import * as MiniTable from '~/ui/lib/MiniTable' import { bytesToGiB } from '~/util/units' @@ -39,14 +38,13 @@ export function DisksTableField({ const { field: { value: items, onChange }, - } = useController({ control, name: 'disks' }) + } = useController({ control, name: 'otherDisks' }) return ( <>
- {/* this was empty */} {!!items.length && ( - + Name Type @@ -68,7 +66,7 @@ export function DisksTableField({ {item.type === 'attach' ? ( - '-' + '—' ) : ( <> {bytesToGiB(item.size)} diff --git a/app/forms/instance-create.tsx b/app/forms/instance-create.tsx index e0a33bc08c..fc5d86736b 100644 --- a/app/forms/instance-create.tsx +++ b/app/forms/instance-create.tsx @@ -103,7 +103,7 @@ export type InstanceCreateInput = Assign< SetRequired, { presetId: (typeof PRESETS)[number]['id'] - disks: DiskTableItem[] + otherDisks: DiskTableItem[] bootDiskName: string bootDiskSize: number @@ -139,7 +139,7 @@ const baseDefaultValues: InstanceCreateInput = { projectImageSource: '', diskSource: '', - disks: [], + otherDisks: [], networkInterfaces: { type: 'default' }, sshPublicKeys: [], @@ -317,7 +317,8 @@ export function CreateInstanceForm() { description: values.description, memory: instance.memory * GiB, ncpus: instance.ncpus, - disks: [bootDisk, ...values.disks], + disks: values.otherDisks, + bootDisk, externalIps: values.externalIps, start: values.start, networkInterfaces: values.networkInterfaces, diff --git a/app/pages/project/instances/instance/tabs/StorageTab.tsx b/app/pages/project/instances/instance/tabs/StorageTab.tsx index 5e05d34df0..1083b509e5 100644 --- a/app/pages/project/instances/instance/tabs/StorageTab.tsx +++ b/app/pages/project/instances/instance/tabs/StorageTab.tsx @@ -8,6 +8,7 @@ import { createColumnHelper, getCoreRowModel, useReactTable } from '@tanstack/react-table' import { useCallback, useMemo, useState } from 'react' import type { LoaderFunctionArgs } from 'react-router-dom' +import * as R from 'remeda' import { apiQueryClient, @@ -22,30 +23,24 @@ import { } from '@oxide/api' import { Storage24Icon } from '@oxide/design-system/icons/react' +import { HL } from '~/components/HL' import { DiskStateBadge } from '~/components/StateBadge' import { AttachDiskSideModalForm } from '~/forms/disk-attach' import { CreateDiskSideModalForm } from '~/forms/disk-create' import { getInstanceSelector, useInstanceSelector } from '~/hooks/use-params' +import { confirmAction } from '~/stores/confirm-action' import { addToast } from '~/stores/toast' import { useColsWithActions, type MenuAction } from '~/table/columns/action-col' import { Columns } from '~/table/columns/common' import { Table } from '~/table/Table' import { Button } from '~/ui/lib/Button' -import { EmptyMessage } from '~/ui/lib/EmptyMessage' -import { TableEmptyBox } from '~/ui/lib/Table' +import { CreateButton } from '~/ui/lib/CreateButton' +import { EMBody, EmptyMessage } from '~/ui/lib/EmptyMessage' +import { TableControls, TableEmptyBox, TableTitle } from '~/ui/lib/Table' +import { links } from '~/util/links' import { fancifyStates } from './common' -const EmptyState = () => ( - - } - title="No disks" - body="Attach a disk to this instance to see it here" - /> - -) - StorageTab.loader = async ({ params }: LoaderFunctionArgs) => { const { project, instance } = getInstanceSelector(params) const selector = { path: { instance }, query: { project } } @@ -120,29 +115,124 @@ export function StorageTab() { const { data: instance } = usePrefetchedApiQuery('instanceView', instancePathQuery) - const makeActions = useCallback( + const { mutateAsync: instanceUpdate } = useApiMutation('instanceUpdate', { + onSuccess() { + apiQueryClient.invalidateQueries('instanceView') + }, + }) + + // shared between boot and other disks + const getSnapshotAction = useCallback( + (disk: InstanceDisk) => ({ + label: 'Snapshot', + disabled: !diskCan.snapshot(disk) && ( + <>Only disks in state {fancifyStates(diskCan.snapshot.states)} can be snapshotted + ), + onActivate() { + createSnapshot({ + query: { project }, + body: { name: genName(disk.name), disk: disk.name, description: '' }, + }) + }, + }), + [createSnapshot, project] + ) + + const { data: disks } = usePrefetchedApiQuery('instanceDiskList', instancePathQuery) + + const [bootDisks, otherDisks] = useMemo( + () => R.partition(disks.items, (d) => d.id === instance.bootDiskId), + [disks.items, instance.bootDiskId] + ) + + const makeBootDiskActions = useCallback( (disk: InstanceDisk): MenuAction[] => [ + getSnapshotAction(disk), { - label: 'Snapshot', - disabled: !diskCan.snapshot(disk) && ( + label: 'Unset as boot disk', + disabled: !instanceCan.update({ runState: disk.instanceState }) && ( <> - Only disks in state {fancifyStates(diskCan.snapshot.states)} can be snapshotted + Instance must be stopped before boot disk + can be changed ), - onActivate() { - createSnapshot({ - query: { project }, - body: { - name: genName(disk.name), - disk: disk.name, - description: '', - }, + onActivate: () => + confirmAction({ + doAction: () => + instanceUpdate({ + path: { instance: instance.id }, + body: { bootDisk: undefined }, + }), + errorTitle: 'Could not unset boot disk', + modalTitle: 'Confirm unset boot disk', + // TODO: copy + link to docs + modalContent: ( +
+

+ Are you sure you want to unset {disk.name} as the boot disk? It + will remain attached to the instance. +

+

+ Setting a boot disk is recommended unless you intend to manage boot order + within the instance. +

+
+ ), + actionType: 'primary', + // TODO: add docs link to modal footer similar to LearnMore from + // SettingsGroup. + }), + }, + { + label: 'Detach', + disabled: 'Boot disk must be unset before it can be detached', + onActivate() {}, // it's always disabled, so noop is ok + }, + ], + [instanceUpdate, instance.id, getSnapshotAction] + ) + + const makeOtherDiskActions = useCallback( + (disk: InstanceDisk): MenuAction[] => [ + getSnapshotAction(disk), + { + label: 'Set as boot disk', + disabled: !instanceCan.update({ runState: disk.instanceState }) && ( + <> + Instance must be stopped before boot disk + can be changed + + ), + onActivate: () => { + const bootDiskName = bootDisks.length > 0 ? bootDisks[0].name : undefined + const verb = bootDiskName ? 'change' : 'set' + return confirmAction({ + doAction: () => + instanceUpdate({ + path: { instance: instance.id }, + body: { bootDisk: disk.id }, + }), + errorTitle: `Could not ${verb} boot disk`, + modalTitle: `Confirm ${verb} boot disk`, + modalContent: bootDiskName ? ( +

+ Are you sure you want to change the boot disk to {disk.name}? + Current boot disk {bootDiskName} will remain attached to the + instance. +

+ ) : ( +

+ Are you sure you want to set {disk.name} as the boot disk? +

+ ), + actionType: 'primary', + // TODO: add docs link to modal footer similar to LearnMore + // from SettingsGroup. This probably requires a change to + // `confirmAction`. }) }, }, { - // don't bother checking disk state: assume that if it is showing up - // in this list, it can be detached label: 'Detach', disabled: !instanceCan.detachDisk({ runState: disk.instanceState }) && ( <> @@ -151,11 +241,11 @@ export function StorageTab() { ), onActivate() { - detachDisk({ body: { disk: disk.name }, ...instancePathQuery }) + detachDisk({ body: { disk: disk.name }, path: { instance: instance.id } }) }, }, ], - [detachDisk, instancePathQuery, createSnapshot, project] + [detachDisk, instanceUpdate, instance.id, getSnapshotAction, bootDisks] ) const attachDisk = useApiMutation('instanceDiskAttach', { @@ -174,59 +264,74 @@ export function StorageTab() { }, }) - const { data: disks } = usePrefetchedApiQuery('instanceDiskList', instancePathQuery) - - const rows = useMemo( - () => disks.items.map((disk) => ({ ...disk, instanceState: instance.runState })), - [disks.items, instance.runState] - ) + const bootDisksTable = useReactTable({ + columns: useColsWithActions(staticCols, makeBootDiskActions), + data: useMemo( + () => bootDisks.map((disk) => ({ ...disk, instanceState: instance.runState })), + [bootDisks, instance.runState] + ), + getCoreRowModel: getCoreRowModel(), + }) - const table = useReactTable({ - columns: useColsWithActions(staticCols, makeActions), - data: rows, + const otherDisksTable = useReactTable({ + columns: useColsWithActions(staticCols, makeOtherDiskActions), + data: useMemo( + () => otherDisks.map((disk) => ({ ...disk, instanceState: instance.runState })), + [otherDisks, instance.runState] + ), getCoreRowModel: getCoreRowModel(), }) return ( <> - {disks.items.length > 0 ? : } -
-
- - -
- {!instanceCan.attachDisk(instance) && ( - - The instance must be stopped to add or - attach a disk. - - )} + + Boot disk + + {bootDisks.length > 0 ? ( +
+ ) : ( + + )} + + + Other disks + + + {otherDisks.length > 0 ? ( +
+ ) : ( + + )} + +
+ setShowDiskCreate(true)} + disabledReason={ + <> + Instance must be stopped to create and + attach a disk + + } + disabled={!instanceCan.attachDisk(instance)} + > + Create disk + +
+ {showDiskCreate && ( setShowDiskCreate(false)} @@ -250,3 +355,55 @@ export function StorageTab() { ) } + +function BootDiskEmptyState({ otherDisks }: { otherDisks: Disk[] }) { + return ( + + } + title="No boot disk set" + body={ + <> + {otherDisks.length > 1 ? ( + + Setting a boot disk is recommended unless you intend to manage boot order + within the instance. + + ) : otherDisks.length === 1 ? ( + + Instance will boot from {otherDisks[0].name} because it is the only + disk. + + ) : ( + Attach a disk to be able to set a boot disk. + )} + + Learn more in the{' '} + + Instances + {' '} + guide. + + + } + /> + + ) +} + +function OtherDisksEmptyState() { + return ( + + } + title="No other disks" + body="Attach a disk to see it here" + /> + + ) +} diff --git a/app/ui/lib/EmptyMessage.tsx b/app/ui/lib/EmptyMessage.tsx index f22371dfb5..c603b81ea9 100644 --- a/app/ui/lib/EmptyMessage.tsx +++ b/app/ui/lib/EmptyMessage.tsx @@ -6,9 +6,11 @@ * Copyright Oxide Computer Company */ import cn from 'classnames' -import type { ReactElement } from 'react' +import type { ReactElement, ReactNode } from 'react' import { Link } from 'react-router-dom' +import { classed } from '~/util/classed' + import { Button, buttonStyle } from './Button' const buttonStyleProps = { variant: 'ghost', size: 'sm', color: 'secondary' } as const @@ -16,7 +18,7 @@ const buttonStyleProps = { variant: 'ghost', size: 'sm', color: 'secondary' } as type Props = { icon?: ReactElement title: string - body?: string + body?: ReactNode } & ( // only require buttonTo or onClick if buttonText is present | { buttonText: string; buttonTo: string } | { buttonText: string; onClick: () => void } @@ -46,10 +48,10 @@ export function EmptyMessage(props: Props) { )}

{props.title}

- {props.body && ( -

{props.body}

- )} + {typeof props.body === 'string' ? {props.body} : props.body} {button} ) } + +export const EMBody = classed.p`mt-1 text-balance text-sans-md text-secondary` diff --git a/app/util/links.ts b/app/util/links.ts index 65e24673a2..48c88d5eed 100644 --- a/app/util/links.ts +++ b/app/util/links.ts @@ -20,6 +20,8 @@ export const links = { preparingImagesDocs: 'https://docs.oxide.computer/guides/creating-and-sharing-images#_preparing_images_for_import', instanceActionsDocs: 'https://docs.oxide.computer/guides/managing-instances', + // TODO: link to section + instanceBootDiskDocs: 'https://docs.oxide.computer/guides/deploying-workloads', keyConceptsIamPolicyDocs: 'https://docs.oxide.computer/guides/key-entities-and-concepts#iam-policy', keyConceptsProjectsDocs: diff --git a/mock-api/instance.ts b/mock-api/instance.ts index c519297c02..c274e1f16c 100644 --- a/mock-api/instance.ts +++ b/mock-api/instance.ts @@ -12,7 +12,15 @@ import { GiB } from '~/util/units' import type { Json } from './json-type' import { project } from './project' +const base = { + time_created: new Date().toISOString(), + time_modified: new Date().toISOString(), + time_run_state_updated: new Date().toISOString(), + auto_restart_enabled: true, +} + export const instance: Json = { + ...base, id: '935499b3-fd96-432a-9c21-83a3dc1eece4', name: 'db1', ncpus: 2, @@ -21,12 +29,11 @@ export const instance: Json = { hostname: 'oxide.com', project_id: project.id, run_state: 'running', - time_created: new Date().toISOString(), - time_modified: new Date().toISOString(), - time_run_state_updated: new Date().toISOString(), + boot_disk_id: '7f2309a5-13e3-47e0-8a4c-2a3b3bc992fd', // disk-1 } const failedInstance: Json = { + ...base, id: 'b5946edc-5bed-4597-88ab-9a8beb9d32a4', name: 'you-fail', ncpus: 4, @@ -35,12 +42,10 @@ const failedInstance: Json = { hostname: 'oxide.com', project_id: project.id, run_state: 'failed', - time_created: new Date().toISOString(), - time_modified: new Date().toISOString(), - time_run_state_updated: new Date().toISOString(), } const startingInstance: Json = { + ...base, id: '16737f54-1f76-4c96-8b7c-9d24971c1d62', name: 'not-there-yet', ncpus: 2, @@ -49,9 +54,6 @@ const startingInstance: Json = { hostname: 'oxide.com', project_id: project.id, run_state: 'starting', - time_created: new Date().toISOString(), - time_modified: new Date().toISOString(), - time_run_state_updated: new Date().toISOString(), } export const instances: Json[] = [instance, failedInstance, startingInstance] diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index d1d346615e..5278bf105c 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -17,6 +17,7 @@ import { INSTANCE_MIN_RAM_GiB, MAX_NICS_PER_INSTANCE, type ApiTypes as Api, + type InstanceDiskAttachment, type SamlIdentityProvider, } from '@oxide/api' @@ -417,12 +418,18 @@ export const handlers = makeHandlers({ * Eagerly check for disk errors. Execution will stop early and prevent orphaned disks from * being created if there's a failure. In omicron this is done automatically via an undo on the saga. */ - for (const diskParams of body.disks || []) { + const allDisks: Json[] = [] + if (body.disks) allDisks.push(...body.disks) + if (body.boot_disk) allDisks.push(body.boot_disk) + + for (const diskParams of allDisks) { if (diskParams.type === 'create') { errIfExists(db.disks, { name: diskParams.name, project_id: project.id }, 'disk') errIfInvalidDiskSize(diskParams) } else { - lookup.disk({ ...query, disk: diskParams.name }) + const disk = lookup.disk({ ...query, disk: diskParams.name }) + if (disk.state.state !== 'detached') + throw `Disk '${diskParams.name}' is already attached to an instance` } } @@ -440,7 +447,11 @@ export const handlers = makeHandlers({ }) } - for (const diskParams of body.disks || []) { + ///////////////////////////////////////////////////////////// + // DB write stuff starts here + ///////////////////////////////////////////////////////////// + + for (const diskParams of allDisks) { if (diskParams.type === 'create') { const { size, name, description, disk_source } = diskParams const newDisk: Json = { @@ -461,6 +472,11 @@ export const handlers = makeHandlers({ } } + // at this point, the boot disk has been created, so just retrieve it again + const bootDiskId = body.boot_disk?.name + ? lookup.disk({ disk: body.boot_disk.name, project: project.id }).id + : undefined + // just use the first VPC in the project and first subnet in the VPC. bit of // a hack but not very important const anyVpc = db.vpcs.find((v) => v.project_id === project.id) @@ -505,6 +521,8 @@ export const handlers = makeHandlers({ ...getTimestamps(), run_state: 'creating', time_run_state_updated: new Date().toISOString(), + boot_disk_id: bootDiskId, + auto_restart_enabled: true, } body.external_ips?.forEach((ip) => { @@ -542,6 +560,36 @@ export const handlers = makeHandlers({ return json(newInstance, { status: 201 }) }, instanceView: ({ path, query }) => lookup.instance({ ...path, ...query }), + 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')}` + } + + 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. + const disk = lookup.disk({ + disk: body.boot_disk, + project: isUuid(body.boot_disk) ? undefined : query.project, + }) + + const isAttached = + disk.state.state === 'attached' && disk.state.instance === instance.id + if (!(diskCan.setAsBootDisk(disk) && isAttached)) { + throw 'Boot disk must be attached to the instance' + } + + instance.boot_disk_id = disk.id + } else { + // we're clearing the boot disk! + instance.boot_disk_id = undefined + } + + return instance + }, instanceDelete({ path, query }) { const instance = lookup.instance({ ...path, ...query }) db.instances = db.instances.filter((i) => i.id !== instance.id) @@ -573,7 +621,11 @@ export const handlers = makeHandlers({ const states = commaSeries(instanceCan.detachDisk.states, 'or') throw `Can only detach disk from instance that is ${states}` } - const disk = lookup.disk({ ...projectParams, disk: body.disk }) + const disk = lookup.disk({ + disk: body.disk, + // use instance project ID because project may not be specified in params + project: isUuid(body.disk) ? undefined : instance.project_id, + }) if (!diskCan.detach(disk)) { const states = commaSeries(diskCan.detach.states, 'or') throw `Can only detach disk that is ${states}` diff --git a/test/e2e/click-everything.e2e.ts b/test/e2e/click-everything.e2e.ts index 95db0babe8..e2ead76cc8 100644 --- a/test/e2e/click-everything.e2e.ts +++ b/test/e2e/click-everything.e2e.ts @@ -23,7 +23,7 @@ test('Click through instance page', async ({ page }) => { 'role=cell[name="disk-1"]', 'role=cell[name="disk-2"]', // buttons disabled while instance is running - 'role=button[name="Create new disk"][disabled]', + 'role=button[name="Create disk"][disabled]', 'role=button[name="Attach existing disk"][disabled]', // TODO: assert minitable contents ]) diff --git a/test/e2e/instance-create.e2e.ts b/test/e2e/instance-create.e2e.ts index 5ccf320d3b..97af4ec0b5 100644 --- a/test/e2e/instance-create.e2e.ts +++ b/test/e2e/instance-create.e2e.ts @@ -114,6 +114,22 @@ test('can create an instance', async ({ page }) => { 'text=from space', ]) + // instance goes from creating to starting to running as we poll + const pollingSpinner = page.getByLabel('Spinner') + await expect(pollingSpinner).toBeVisible() + await expect(page.getByText('Creating')).toBeVisible() + await expect(page.getByText('Starting')).toBeVisible() + await expect(page.getByText('Running')).toBeVisible() + await expect(pollingSpinner).toBeHidden() + + // boot disk visible, no other disks attached + await expect( + page + .getByRole('table', { name: 'Boot disk' }) + .getByRole('cell', { name: 'my-boot-disk' }) + ).toBeVisible() + await expect(page.getByText('No other disk')).toBeVisible() + // network tab works await page.getByRole('tab', { name: 'Networking' }).click() const table = page.getByRole('table', { name: 'Network interfaces' }) @@ -280,7 +296,11 @@ test('create instance with existing disk', async ({ page }) => { await page.getByRole('button', { name: 'Create instance' }).click() await expect(page).toHaveURL(`/projects/mock-project/instances/${instanceName}/storage`) await expectVisible(page, [`h1:has-text("${instanceName}")`, 'text=8 GiB']) - await expectRowVisible(page.getByRole('table'), { Disk: 'disk-3' }) + + const bootDisks = page.getByRole('table', { name: 'Boot disk' }) + await expectRowVisible(bootDisks, { Disk: 'disk-3' }) + + await expect(page.getByText('No other disks')).toBeVisible() }) test('create instance with a silo image', async ({ page }) => { @@ -481,3 +501,45 @@ test('attaching additional disks allows for combobox filtering', async ({ page } await selectADisk.fill('asdf') await expect(page.getByRole('option', { name: 'No items match' })).toBeVisible() }) + +test('create instance with additional disks', async ({ page }) => { + const instanceName = 'more-disks' + await page.goto('/projects/mock-project/instances-new') + await page.getByRole('textbox', { name: 'Name', exact: true }).fill(instanceName) + await selectAProjectImage(page, 'image-1') + + // Create a new disk + await page.getByRole('button', { name: 'Create new disk' }).click() + + const createForm = page.getByRole('dialog', { name: 'Create disk' }) + await createForm.getByRole('textbox', { name: 'Name', exact: true }).fill('new-disk-1') + await createForm.getByRole('textbox', { name: 'Size (GiB)' }).fill('5') + await createForm.getByRole('button', { name: 'Create disk' }).click() + + const disksTable = page.getByRole('table', { name: 'Disks' }) + await expectRowVisible(disksTable, { Name: 'new-disk-1', Type: 'create', Size: '5GiB' }) + + // Attach an existing disk + await page.getByRole('button', { name: 'Attach existing disk' }).click() + await page.getByRole('button', { name: 'Disk name' }).click() + await page.getByRole('option', { name: 'disk-3' }).click() + await page.getByRole('button', { name: 'Attach disk' }).click() + + await expectRowVisible(disksTable, { Name: 'disk-3', Type: 'attach', Size: '—' }) + + // Create the instance + await page.getByRole('button', { name: 'Create instance' }).click() + + // Assert we're on the new instance's storage page + await expect(page).toHaveURL(`/projects/mock-project/instances/${instanceName}/storage`) + + // Check for the boot disk + const bootDiskTable = page.getByRole('table', { name: 'Boot disk' }) + // name is generated so it's gnarly + await expect(bootDiskTable.getByRole('cell', { name: /^more-disks-/ })).toBeVisible() + + // Check for the additional disks + const otherDisksTable = page.getByRole('table', { name: 'Other disks' }) + await expectRowVisible(otherDisksTable, { Disk: 'new-disk-1', size: '5 GiB' }) + await expectRowVisible(otherDisksTable, { Disk: 'disk-3', size: '6 GiB' }) +}) diff --git a/test/e2e/instance-disks.e2e.ts b/test/e2e/instance-disks.e2e.ts index 95981b277b..c0c0cfeafa 100644 --- a/test/e2e/instance-disks.e2e.ts +++ b/test/e2e/instance-disks.e2e.ts @@ -15,39 +15,68 @@ import { test, } from './utils' -test('Attach disk', async ({ page }) => { +test('Disabled actions', async ({ page }) => { await page.goto('/projects/mock-project/instances/db1') - const warning = 'The instance must be stopped to add or attach a disk.' - await expect(page.getByText(warning)).toBeVisible() + const bootDiskRow = page.getByRole('row', { name: 'disk-1', exact: false }) + await expect(bootDiskRow).toBeVisible() - const row = page.getByRole('row', { name: 'disk-1', exact: false }) - await expect(row).toBeVisible() + // Both buttons disabled, also test fancy tooltip + + await bootDiskRow.getByRole('button', { name: 'Row actions' }).click() + + const detachButton = page.getByRole('menuitem', { name: 'Detach' }) + const unsetButton = page.getByRole('menuitem', { name: 'Unset' }) + + await expect(unsetButton).toBeDisabled() + await page.getByRole('menuitem', { name: 'Unset' }).hover() + await expect( + page.getByText('Instance must be stopped before boot disk can be changed') + ).toBeVisible() + + await expect(detachButton).toBeDisabled() + await detachButton.hover() + await expect( + page.getByText('Boot disk must be unset before it can be detached') + ).toBeVisible() + await page.keyboard.press('Escape') // close menu + + const otherDiskRow = page.getByRole('row', { name: 'disk-2', exact: false }) + await expect(otherDiskRow).toBeVisible() + + await otherDiskRow.getByRole('button', { name: 'Row actions' }).click() + + const setButton = page.getByRole('menuitem', { name: 'Set as boot disk' }) + await expect(setButton).toBeDisabled() + await setButton.hover() + await expect( + page.getByText('Instance must be stopped before boot disk can be changed') + ).toBeVisible() - // can't detach, also test fancy construction of disabled tooltip - await row.getByRole('button', { name: 'Row actions' }).click() - await expect(page.getByRole('menuitem', { name: 'Detach' })).toBeDisabled() - await page.getByRole('menuitem', { name: 'Detach' }).hover() + await expect(detachButton).toBeDisabled() + await detachButton.hover() await expect( page.getByText('Instance must be stopped before disk can be detached') ).toBeVisible() await page.keyboard.press('Escape') // close menu - // Have to stop instance to edit disks + // confirm they become enabled when the instance is stopped await stopInstance(page) - await expect(page.getByText(warning)).toBeHidden() + await bootDiskRow.getByRole('button', { name: 'Row actions' }).click() + await expect(unsetButton).toBeEnabled() + await expect(detachButton).toBeDisabled() // detach is always disabled on boot disk - // New disk form - await page.click('role=button[name="Create new disk"]') - await expectVisible(page, [ - 'role=textbox[name="Name"]', - 'role=textbox[name="Description"]', - 'role=radiogroup[name="Block size (Bytes)"]', - 'role=textbox[name="Size (GiB)"]', - 'role=button[name="Create disk"]', - ]) - await page.click('role=button[name="Cancel"]') + await otherDiskRow.getByRole('button', { name: 'Row actions' }).click() + await expect(setButton).toBeEnabled() + await expect(detachButton).toBeEnabled() +}) + +test('Attach disk', async ({ page }) => { + await page.goto('/projects/mock-project/instances/db1') + + // Have to stop instance to edit disks + await stopInstance(page) // Attach existing disk form await page.click('role=button[name="Attach existing disk"]') @@ -67,6 +96,34 @@ test('Attach disk', async ({ page }) => { await expectVisible(page, ['role=cell[name="disk-3"]']) }) +test('Create disk', async ({ page }) => { + await page.goto('/projects/mock-project/instances/db1') + + const row = page.getByRole('cell', { name: 'created-disk' }) + await expect(row).toBeHidden() + + // Have to stop instance to edit disks + await stopInstance(page) + + // New disk form + const createForm = page.getByRole('dialog', { name: 'Create disk' }) + await expect(createForm).toBeHidden() + await page.getByRole('button', { name: 'Create disk' }).click() + await expect(createForm).toBeVisible() + + await createForm.getByRole('textbox', { name: 'Name' }).fill('created-disk') + await expect(createForm.getByRole('textbox', { name: 'Description' })).toBeVisible() + + await page.getByRole('radio', { name: 'Snapshot' }).click() + await page.getByRole('button', { name: 'Source snapshot' }).click() + await page.getByRole('option', { name: 'snapshot-heavy' }).click() + + await createForm.getByRole('button', { name: 'Create disk' }).click() + + const otherDisksTable = page.getByRole('table', { name: 'Other disks' }) + await expectRowVisible(otherDisksTable, { Disk: 'created-disk', size: '20 GiB' }) +}) + test('Detach disk', async ({ page }) => { await page.goto('/projects/mock-project/instances/db1') @@ -74,11 +131,11 @@ test('Detach disk', async ({ page }) => { await stopInstance(page) const successMsg = page.getByText('Disk detached').nth(0) - const row = page.getByRole('row', { name: 'disk-1' }) + const row = page.getByRole('row', { name: 'disk-2' }) await expect(row).toBeVisible() await expect(successMsg).toBeHidden() - await clickRowAction(page, 'disk-1', 'Detach') + await clickRowAction(page, 'disk-2', 'Detach') await expect(successMsg).toBeVisible() await expect(row).toBeHidden() // disk row goes away }) @@ -90,7 +147,7 @@ test('Snapshot disk', async ({ page }) => { const successMsg = page.getByText('Snapshot created').nth(0) await expect(successMsg).toBeHidden() - await clickRowAction(page, 'disk-2', 'Snapshot') + await clickRowAction(page, 'disk-1', 'Snapshot') await expect(successMsg).toBeVisible() // we see the toast! @@ -99,7 +156,69 @@ test('Snapshot disk', async ({ page }) => { await page.getByRole('button', { name: 'next' }).click() const table = page.getByRole('table') await expectRowVisible(table, { - name: expect.stringMatching(/^disk-2-/), - disk: 'disk-2', + name: expect.stringMatching(/^disk-1-/), + disk: 'disk-1', }) }) + +test('Change boot disk', async ({ page }) => { + await page.goto('/projects/mock-project/instances/db1') + + // assert disk-1 is boot disk, disk-2 also there + const bootDiskTable = page.getByRole('table', { name: 'Boot disk' }) + const otherDisksTable = page.getByRole('table', { name: 'Other disks' }) + const confirm = page.getByRole('button', { name: 'Confirm' }) + const noBootDisk = page.getByText('No boot disk set') + const noOtherDisks = page.getByText('No other disks') + + const disk1 = { Disk: 'disk-1', size: '2 GiB' } + const disk2 = { Disk: 'disk-2', size: '4 GiB' } + + await expectRowVisible(bootDiskTable, disk1) + await expectRowVisible(otherDisksTable, disk2) + + await stopInstance(page) + + // Set disk-2 as boot disk + await clickRowAction(page, 'disk-2', 'Set as boot disk') + await confirm.click() + + await expectRowVisible(bootDiskTable, disk2) + await expectRowVisible(otherDisksTable, disk1) + + // Unset boot disk + await expect(noBootDisk).toBeHidden() + + await clickRowAction(page, 'disk-2', 'Unset as boot disk') + await confirm.click() + + await expect(noBootDisk).toBeVisible() + await expectRowVisible(otherDisksTable, disk1) + await expectRowVisible(otherDisksTable, disk2) + + await expect(page.getByText('Setting a boot disk is recommended')).toBeVisible() + + // detach disk so there's only one + await clickRowAction(page, 'disk-2', 'Detach') + + await expect(page.getByText('Instance will boot from disk-1')).toBeVisible() + + // set disk-1 back as boot disk + await clickRowAction(page, 'disk-1', 'Set as boot disk') + await confirm.click() + + await expect(noBootDisk).toBeHidden() + await expect(noOtherDisks).toBeVisible() + + // Remove disk-1 altogether, no disks left + await clickRowAction(page, 'disk-1', 'Unset as boot disk') + await confirm.click() + + await expectRowVisible(otherDisksTable, disk1) + + await clickRowAction(page, 'disk-1', 'Detach') + await expect(noBootDisk).toBeVisible() + await expect(noOtherDisks).toBeVisible() + + await expect(page.getByText('Attach a disk to be able to set a boot disk')).toBeVisible() +})