Simplify 'image add' mechanism

[taspelund]

Target component

• CLI
• SDK
• Something else
• Not sure

Overview

A CLI user should be able to upload an image file "directly" as an image, the same way a Console user can.

Today the console does the work behind the scenes to do the creation of a temp disk, bulk writes of the image to that temp tdisk, taking the disk out of import mode, creating a temp snapshot of the disk, creating an image from the snapshot, then deleting the temp disk and temp snapshot... all of which makes the image upload process effectively transparent to the user.

It would seem the CLI doesn't have this user-friendly abstraction. What is available doesn't quite give the same nice experience:

• oxide image create: expects a json body and has no docs for what should be in it
• oxide disk import: convoluted (must first be run w/ arguments but w/o a command, then must be run with commands later?) and results in a disk that still needs to be snapshotted and have an image created from it, then cleaned up

Implementation details

No response

Anything else you would like to add?

No response

[david-crespo]

oxide disk import does do the snapshot and image create for you, but you have to give --image and --snapshot args.

https://docs.oxide.computer/cli/disk/import

oxide.rs/cli/src/cmd_disk.rs

Lines 492 to 545 in 8fc14f8

	// Finalize the disk, optionally making a snapshot
	let request = client
	.disk_finalize_import()
	.project(&self.project)
	.disk(self.disk.clone())
	.body(FinalizeDisk {
	snapshot_name: self.snapshot.clone(),
	});
	let finalize_response = request.send().await;
	if let Err(e) = finalize_response {
	eprintln!("finalizing the disk failed with {:?}", e);
	// Attempt to unwind the disk, although it will probably fail - the
	// first step is to finalize the disk!
	self.unwind_disk_finalize(client).await?;
	self.unwind_disk_delete(client).await?;
	return Err(e.into());
	}
	// optionally, make an image out of that snapshot
	if let Some(image_name) = &self.image {
	let snapshot_name = self.snapshot.as_ref().unwrap();
	let image_description = self.image_description.as_ref().unwrap();
	let image_os = self.image_os.as_ref().unwrap();
	let image_version = self.image_version.as_ref().unwrap();
	// at this point, unwinding is not as important as before. the user
	// has uploaded their file to a disk and finalized that disk, making
	// a snapshot out of it. if this step fails, they can always
	// manually make an image out of the snapshot later and be sure that
	// the snapshot's contents are the same.
	let snapshot = client
	.snapshot_view()
	.project(&self.project)
	.snapshot(NameOrId::Name(snapshot_name.clone()))
	.send()
	.await?;
	client
	.image_create()
	.project(&self.project)
	.body(ImageCreate {
	name: image_name.clone(),
	description: image_description.clone(),
	os: image_os.clone(),
	version: image_version.clone(),
	source: ImageSource::Snapshot(snapshot.id),
	})
	.send()
	.await?;
	}

However, you are 100% right to raise this issue because the docs are worse than useless for figuring that out, and the flags API is also not very intuitive. We can do a lot better.

[citrus-it]

The cli does support this, but it isn't entirely intuitive:

oxide disk import \
        --project myproject \
        --description "Some image" \
        --path /tmp/image.raw \
        --disk image \
        --disk-size 80g \
        --snapshot image-snap \
        --image "My image" \
        --image-description "Some image" \
        --image-os linux \
        --image-version 1234

there could definitely be a wrapper around this to make it simpler.

[david-crespo]

That is the wrapper @citrus-it. I think we just have to improve the flags and the docs. We should also make clearer on stuff like image create that that is not the thing if you're trying to upload an image?

Though maybe you're right that we should have a version of this under image like image upload.

Another note, something I've never noticed before: oxide disk import is one of the rare (maybe unique?) parent commands that groups other commands but also does something itself.

Other parent commands will usually just list their children and nothing else. Maybe we should stick to that pattern and possibly even enforce it programmatically.

[taspelund]

I would also call out that the oxide disk import wrapper doesn't delete the temp disk or snapshot, while the console workflow does. So there are intermediate artifacts that the user needs to 1) be aware of, and 2) manage themselves.

[karencfv]

Today the console does the work behind the scenes to do the creation of a temp disk, bulk writes of the image to that temp tdisk, taking the disk out of import mode, creating a temp snapshot of the disk, creating an image from the snapshot, then deleting the temp disk and temp snapshot... all of which makes the image upload process effectively transparent to the user.

Would it be possible for this workflow to be integrated into the API instead? It'd be pretty sweet to have this in the API so each client doesn't have to implement it's version of the workflow.

[david-crespo]

The bulk write loop can't really be integrated that way, but I bet we could figure out how to get the before and after steps into a single API call each, which would still simplify things quite a bit on the client side.

[karencfv]

That sounds like a really good idea! Thanks @david-crespo

Just for my own understanding, why can't the bulk write loop be integrated?

[david-crespo]

You have to send the file to the server somehow. You could do one big upload of the entire file and let it feed the chunks to crucible in some kind of async process, but

• the upload part would not be parallelizable, or if you made it parallelizable you're basically doing the same thing but with fewer chunks
• if it fails you have to start over
• you still need to monitor the server-side process to know when it's done

So, the way I'm picturing it, at least, the alternative is not really simpler. Though we may end up doing something more integrated we want to put URL-based uploads back in. I do think bigger rethinks are on the table.