Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
329 changes: 272 additions & 57 deletions Cargo.lock

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ default-members = [
"crates/*",
"crates/*/sys",
"lib/*",
"bin/dropshot-apis",
"bin/propolis-cli",
"bin/propolis-server",
"bin/propolis-standalone",
Expand Down Expand Up @@ -113,6 +114,8 @@ const_format = "0.2"
crossbeam-channel = "0.5"
ctrlc = "3.2"
dropshot = "0.16.4"
dropshot-api-manager = "0.2.2"
dropshot-api-manager-types = "0.2.2"
erased-serde = "0.4"
errno = "0.2.8"
escargot = "0.5.8"
Expand Down
14 changes: 14 additions & 0 deletions bin/dropshot-apis/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[package]
name = "propolis-dropshot-apis"
version = "0.1.0"
edition = "2024"
license = "MPL-2.0"

[dependencies]
anyhow.workspace = true
camino.workspace = true
clap.workspace = true
dropshot-api-manager-types.workspace = true
dropshot-api-manager.workspace = true
propolis-server-api.workspace = true
semver.workspace = true
81 changes: 81 additions & 0 deletions bin/dropshot-apis/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// 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/.

use std::process::ExitCode;

use anyhow::Context;
use camino::Utf8PathBuf;
use clap::Parser;
use dropshot_api_manager::{Environment, ManagedApiConfig, ManagedApis};
use dropshot_api_manager_types::{ManagedApiMetadata, Versions};
use propolis_server_api::*;

pub fn environment() -> anyhow::Result<Environment> {
// The workspace root is two levels up from this crate's directory.
let workspace_root = Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.parent()
.unwrap()
.parent()
.unwrap()
.to_path_buf();
let env = Environment::new(
// This is the command used to run the OpenAPI manager.
"cargo xtask openapi",
workspace_root,
// This is the location within the workspace root where the OpenAPI
// documents are stored.
"openapi",
)?
.with_default_git_branch("origin/master".to_owned());
Ok(env)
}

/// The list of APIs managed by the OpenAPI manager.
pub fn all_apis() -> anyhow::Result<ManagedApis> {
let apis = vec![ManagedApiConfig {
ident: "propolis-server",
versions: Versions::Lockstep { version: semver::Version::new(0, 0, 1) },
Copy link
Member

@iximeow iximeow Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and presumably in the future we'll move this to Versioned with the latest version and whatever the previous release's API was, right? (i don't think this was explicitly written out for APIs outside Omicron before, and being able to write that is part of the motivation here aka oxidecomputer/omicron#8727?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, making these APIs versioned (as part of self-service update) is the overall motivation for the series of changes I've been making to all of these repos. See oxidecomputer/omicron#8922 for overall status tracking.

title: "Oxide Propolis Server API",
metadata: ManagedApiMetadata {
description: Some(
"API for interacting with the Propolis hypervisor frontend.",
),
contact_url: Some("https://oxide.computer"),
contact_email: Some("api@oxide.computer"),
..Default::default()
},
api_description: propolis_server_api_mod::stub_api_description,
extra_validation: None,
}];

let apis = ManagedApis::new(apis).context("error creating ManagedApis")?;
Ok(apis)
}

fn main() -> anyhow::Result<ExitCode> {
let app = dropshot_api_manager::App::parse();
let env = environment()?;
let apis = all_apis()?;

Ok(app.exec(&env, &apis))
}

#[cfg(test)]
mod test {
use dropshot_api_manager::test_util::check_apis_up_to_date;

use super::*;

// Also recommended: a test which ensures documents are up-to-date. The
// OpenAPI manager comes with a helper function for this, called
// `check_apis_up_to_date`.
#[test]
fn test_apis_up_to_date() -> anyhow::Result<ExitCode> {
let env = environment()?;
let apis = all_apis()?;

let result = check_apis_up_to_date(&env, &apis)?;
Ok(result.to_exit_code())
}
}
6 changes: 3 additions & 3 deletions bin/mock-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ async fn run_server(
Arc::new(context),
&log,
)
.map_err(|error| anyhow!("Failed to start server: {}", error))?
.map_err(|error| anyhow!("Failed to start server: {error}"))?
.start();

let server_res = server.await;
server_res.map_err(|e| anyhow!("Server exited with an error: {}", e))
server_res.map_err(|e| anyhow!("Server exited with an error: {e}"))
}

#[tokio::main]
Expand All @@ -105,7 +105,7 @@ async fn main() -> anyhow::Result<()> {

match args {
Args::OpenApi => run_openapi()
.map_err(|e| anyhow!("Cannot generate OpenAPI spec: {}", e)),
.map_err(|e| anyhow!("Cannot generate OpenAPI spec: {e}")),
Args::Run { cfg: _cfg, propolis_addr, metric_addr } => {
// Dropshot configuration.
let config_dropshot = ConfigDropshot {
Expand Down
6 changes: 2 additions & 4 deletions bin/propolis-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,9 +443,7 @@ fn resolve_host(server: &str) -> anyhow::Result<IpAddr> {
.to_socket_addrs()?
.map(|sock_addr| sock_addr.ip())
.next()
.ok_or_else(|| {
anyhow!("failed to resolve server argument '{}'", server)
})
.ok_or_else(|| anyhow!("failed to resolve server argument '{server}'"))
}

/// Create a top-level logger that outputs to stderr
Expand Down Expand Up @@ -689,7 +687,7 @@ async fn serial(
| CloseCode::Protocol
| CloseCode::Size
| CloseCode::Unsupported => {
anyhow::bail!("{}", reason);
anyhow::bail!("{reason}");
}
_ => break,
}
Expand Down
3 changes: 1 addition & 2 deletions bin/propolis-server/src/lib/migrate/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ impl TryFrom<ProtocolParts> for Protocol {
Self::RonV0
}
_ => anyhow::bail!(format!(
"no protocol matching definition: {:?}",
value
"no protocol matching definition: {value:?}"
)),
};

Expand Down
23 changes: 0 additions & 23 deletions bin/propolis-server/src/lib/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,26 +646,3 @@ fn not_created_error() -> HttpError {
"Server not initialized (no instance)".to_string(),
)
}

#[cfg(test)]
mod test {
#[test]
fn test_propolis_server_openapi() {
let mut buf: Vec<u8> = vec![];
propolis_server_api::propolis_server_api_mod::stub_api_description()
.unwrap()
.openapi("Oxide Propolis Server API", semver::Version::new(0, 0, 1))
.description(
"API for interacting with the Propolis hypervisor frontend.",
)
.contact_url("https://oxide.computer")
.contact_email("api@oxide.computer")
.write(&mut buf)
.unwrap();
let output = String::from_utf8(buf).unwrap();
expectorate::assert_contents(
"../../openapi/propolis-server.json",
&output,
);
}
}
20 changes: 2 additions & 18 deletions bin/propolis-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ fn parse_log_level(s: &str) -> anyhow::Result<slog::Level> {
#[clap(about, version)]
/// An HTTP server providing access to Propolis
enum Args {
/// Generates the OpenAPI specification.
OpenApi,
/// Runs the Propolis server.
Run {
#[clap(action)]
Expand Down Expand Up @@ -107,18 +105,6 @@ enum Args {
},
}

pub fn run_openapi() -> Result<(), String> {
server::api()
.openapi("Oxide Propolis Server API", semver::Version::new(0, 0, 1))
.description(
"API for interacting with the Propolis hypervisor frontend.",
)
.contact_url("https://oxide.computer")
.contact_email("api@oxide.computer")
.write(&mut std::io::stdout())
.map_err(|e| e.to_string())
}

fn run_server(
bootrom_path: PathBuf,
bootrom_version: Option<String>,
Expand Down Expand Up @@ -187,7 +173,7 @@ fn run_server(
Arc::new(context),
&log,
)
.map_err(|error| anyhow!("Failed to start server: {}", error))?
.map_err(|error| anyhow!("Failed to start server: {error}"))?
.start();

let result = api_runtime.block_on(server);
Expand All @@ -197,7 +183,7 @@ fn run_server(
api_runtime.block_on(async { vnc.halt().await });
}

result.map_err(|e| anyhow!("Server exited with an error: {}", e))
result.map_err(|e| anyhow!("Server exited with an error: {e}"))
}

fn build_logger(level: slog::Level) -> slog::Logger {
Expand Down Expand Up @@ -298,8 +284,6 @@ fn main() -> anyhow::Result<()> {
let args = Args::parse();

match args {
Args::OpenApi => run_openapi()
.map_err(|e| anyhow!("Cannot generate OpenAPI spec: {}", e)),
Args::Run {
bootrom_path,
bootrom_version,
Expand Down
12 changes: 4 additions & 8 deletions bin/propolis-standalone/src/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,7 @@ pub(crate) fn save(
let device_data = match dev.migrate() {
Migrator::NonMigratable => {
anyhow::bail!(
"Can't snapshot instance with non-migratable device ({})",
name
"Can't snapshot instance with non-migratable device ({name})"
);
}
Migrator::Empty => continue,
Expand Down Expand Up @@ -343,13 +342,12 @@ pub(crate) fn restore(
for snap_dev in device_data {
let name = &snap_dev.instance_name;
let dev = guard.inventory.devs.get(name).ok_or_else(|| {
anyhow::anyhow!("unknown device in snapshot {}", name)
anyhow::anyhow!("unknown device in snapshot {name}")
})?;

match dev.migrate() {
Migrator::NonMigratable => anyhow::bail!(
"can't restore snapshot with non-migratable device ({})",
name
"can't restore snapshot with non-migratable device ({name})"
),
Migrator::Empty => {
// There really shouldn't be a payload for this
Expand Down Expand Up @@ -413,9 +411,7 @@ pub(crate) fn restore(
let remain = offer.remaining().count();
if remain > 0 {
return Err(anyhow::anyhow!(
"Device {} had {} remaining payload(s)",
name,
remain
"Device {name} had {remain} remaining payload(s)"
));
}
}
Expand Down
25 changes: 9 additions & 16 deletions phd-tests/framework/src/artifacts/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl StoredArtifact {
if let Some(digest) = sha256 {
file_hash_equals(&path, digest)?;
} else if !path.is_file() {
anyhow::bail!("artifact path {} is not a file", path);
anyhow::bail!("artifact path {path} is not a file");
}

// The file is in the right place and has the right hash (if that
Expand Down Expand Up @@ -98,8 +98,7 @@ impl StoredArtifact {
}
} else if maybe_path.exists() {
anyhow::bail!(
"artifact path {} already exists but isn't a file",
maybe_path
"artifact path {maybe_path} already exists but isn't a file"
);
}

Expand Down Expand Up @@ -152,8 +151,7 @@ impl StoredArtifact {
// This artifact is a tarball, and a file must be extracted from it.
let filename = untar_path.file_name().ok_or_else(|| {
anyhow::anyhow!(
"untar path '{}' has no file name component",
untar_path
"untar path '{untar_path}' has no file name component"
)
})?;
let extracted_path = path.with_file_name(filename);
Expand Down Expand Up @@ -358,8 +356,7 @@ impl Store {
Ok((path, kind))
}
_ => Err(anyhow::anyhow!(
"artifact {} is not a guest OS image",
artifact_name
"artifact {artifact_name} is not a guest OS image"
)),
}
}
Expand All @@ -375,8 +372,7 @@ impl Store {
guard.ensure(&self.local_dir, &self.downloader).await
}
_ => Err(anyhow::anyhow!(
"artifact {} is not a bootrom",
artifact_name
"artifact {artifact_name} is not a bootrom"
)),
}
}
Expand All @@ -392,8 +388,7 @@ impl Store {
guard.ensure(&self.local_dir, &self.downloader).await
}
_ => Err(anyhow::anyhow!(
"artifact {} is not a Propolis server",
artifact_name
"artifact {artifact_name} is not a Propolis server"
)),
}
}
Expand All @@ -416,7 +411,7 @@ impl Store {
name: &str,
) -> anyhow::Result<&Mutex<StoredArtifact>> {
self.artifacts.get(name).ok_or_else(|| {
anyhow::anyhow!("artifact {} not found in store", name)
anyhow::anyhow!("artifact {name} not found in store")
})
}

Expand All @@ -435,14 +430,12 @@ impl Store {
let full_path = cmd.canonicalize_utf8()?;
let filename = full_path.file_name().ok_or_else(|| {
anyhow::anyhow!(
"local artifact command '{}' contains no file component",
cmd
"local artifact command '{cmd}' contains no file component"
)
})?;
let dir = full_path.parent().ok_or_else(|| {
anyhow::anyhow!(
"canonicalized local artifact path '{}' has no directory component",
full_path
"canonicalized local artifact path '{full_path}' has no directory component"
)
})?;

Expand Down
3 changes: 1 addition & 2 deletions phd-tests/framework/src/disk/fat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,7 @@ impl FatFilesystem {
// this by ensuring there's one track containing all the sectors.
let sectors_per_track: u16 = sectors.try_into().map_err(|_| {
anyhow::anyhow!(
"disk has {} sectors, which is too many for one FAT track",
sectors
"disk has {sectors} sectors, which is too many for one FAT track"
)
})?;

Expand Down
Loading
Loading