Disallow disk snapshots for disks attached to stopped instances

[augustuswm]

Currently the web console allows for attempting to snapshot a disk that is attached to a stopped instance. This is currently not supported in nexus and either the instance needs to be running, or the disk needs to be detached for us to be able to snapshot it. We should not present the option to snapshot under these conditions.

[david-crespo]

Found what looks like the relevant API-side check. It has been there since snapshots were introduced.

https://github.com/oxidecomputer/omicron/blob/a5430a6465f457175ca1cf0c0053ffc47f25bf62/sled-agent/src/instance.rs#L1183-L1203

Unfortunately the client-side check is written in a way that is hard to extend to include instance state. Not a big deal, but it won't be a one-line fix.

console/libs/api/util.ts

Lines 112 to 126 in cfda163

	export const diskCan = mapValues(
	{
	// https://github.com/oxidecomputer/omicron/blob/4970c71e/nexus/db-queries/src/db/datastore/disk.rs#L578-L582.
	delete: ['detached', 'creating', 'faulted'],
	// TODO: link to API source
	snapshot: ['attached', 'detached'],
	},
	(states: DiskState['state'][]) => {
	// only have to Pick because we want this to work for both Disk and
	// Json<Disk>, which we pass to it in the MSW handlers
	const test = (d: Pick<Disk, 'state'>) => states.includes(d.state.state)
	test.states = states
	return test
	}
	)

[david-crespo]

Tested snapshotting a disk attached to a stopped instance on dogfood with #1948 deployed.

It's not great, but it would be better if the API error message was better.

{
  "request_id": "f14e26d3-f122-4f12-b284-daf99c2386c3",
  "error_code": "ServiceNotAvailable",
  "message": "Service Unavailable"
}

[david-crespo]

I was wrong, the error comes from Nexus, right in the saga.

https://github.com/oxidecomputer/omicron/blob/915276d47096a9ce076453eb75aaa9de29477e80/nexus/src/app/sagas/snapshot_create.rs#L841-L851

[augustuswm]

That is unfortunate. Maybe snapshot_create needs to check that invariant before issuing the saga given that we know it will fail. As it should really be using a 4xx status code like the other invariant failures as opposed to a 503, since no amount of calling the endpoint will succeed.

Eventually we will need to back it out once this functionality is working.

[david-crespo]

I think it might be a bug, as there is such a check before starting the saga. The logic might not be quite right.

oxidecomputer/omicron#5108

[askfongjojo]

We probably want to hold this ticket for a bit as oxidecomputer/omicron#3289 is being fixed by oxidecomputer/omicron#5221. Hopefully we no longer have to deal with the 503 error for much longer.

[askfongjojo]

I just confirmed on rack2 that we can now snapshot disks attached to stopped instance. The change requested here is no longer necessary.