Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update to Dropshot 0.13.0 (minimum changes) #7050

Merged
merged 16 commits into from
Nov 14, 2024
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
195 changes: 132 additions & 63 deletions Cargo.lock

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ dns-server = { path = "dns-server" }
dns-server-api = { path = "dns-server-api" }
dns-service-client = { path = "clients/dns-service-client" }
dpd-client = { path = "clients/dpd-client" }
dropshot = { version = "0.12.0", features = [ "usdt-probes" ] }
dropshot = { version = "0.13.0", features = [ "usdt-probes" ] }
dyn-clone = "1.0.17"
either = "1.13.0"
expectorate = "1.1.0"
Expand Down Expand Up @@ -520,10 +520,10 @@ prettyplease = { version = "0.2.25", features = ["verbatim"] }
proc-macro2 = "1.0"
progenitor = "0.8.0"
progenitor-client = "0.8.0"
bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "86101eaf80b55e7f405b5cafe9b0de0e9f331656" }
propolis_api_types = { git = "https://github.com/oxidecomputer/propolis", rev = "86101eaf80b55e7f405b5cafe9b0de0e9f331656" }
propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "86101eaf80b55e7f405b5cafe9b0de0e9f331656" }
propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "86101eaf80b55e7f405b5cafe9b0de0e9f331656" }
bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "aadc0998c0f07f08ab15a95c006074291734800f" }
propolis_api_types = { git = "https://github.com/oxidecomputer/propolis", rev = "aadc0998c0f07f08ab15a95c006074291734800f" }
propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "aadc0998c0f07f08ab15a95c006074291734800f" }
propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "aadc0998c0f07f08ab15a95c006074291734800f" }
proptest = "1.5.0"
qorb = "0.2.0"
quote = "1.0"
Expand Down Expand Up @@ -809,7 +809,7 @@ opt-level = 3
# It's common during development to use a local copy of various complex
# dependencies. If you want to use those, uncomment one of these blocks.
#
#[patch."https://github.com/oxidecomputer/dropshot"]
#[patch."crates-io"]
#dropshot = { path = "../dropshot/dropshot" }
#[patch.crates-io]
#steno = { path = "../steno" }
Expand Down
1 change: 1 addition & 0 deletions dev-tools/openapi-manager/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ openapiv3.workspace = true
owo-colors.workspace = true
oximeter-api.workspace = true
repo-depot-api.workspace = true
semver.workspace = true
serde_json.workspace = true
similar.workspace = true
sled-agent-api.workspace = true
Expand Down
31 changes: 16 additions & 15 deletions dev-tools/openapi-manager/src/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub fn all_apis() -> Vec<ApiSpec> {
vec![
ApiSpec {
title: "Bootstrap Agent API",
version: "0.0.1",
version: semver::Version::new(0, 0, 1),
description: "Per-sled API for setup and teardown",
boundary: ApiBoundary::Internal,
api_description:
Expand All @@ -28,7 +28,7 @@ pub fn all_apis() -> Vec<ApiSpec> {
},
ApiSpec {
title: "ClickHouse Cluster Admin Keeper API",
version: "0.0.1",
version: semver::Version::new(0, 0, 1),
description: "API for interacting with the Oxide \
control plane's ClickHouse cluster keepers",
boundary: ApiBoundary::Internal,
Expand All @@ -39,7 +39,7 @@ pub fn all_apis() -> Vec<ApiSpec> {
},
ApiSpec {
title: "ClickHouse Cluster Admin Server API",
version: "0.0.1",
version: semver::Version::new(0, 0, 1),
description: "API for interacting with the Oxide \
control plane's ClickHouse cluster replica servers",
boundary: ApiBoundary::Internal,
Expand All @@ -50,7 +50,7 @@ pub fn all_apis() -> Vec<ApiSpec> {
},
ApiSpec {
title: "CockroachDB Cluster Admin API",
version: "0.0.1",
version: semver::Version::new(0, 0, 1),
description: "API for interacting with the Oxide \
control plane's CockroachDB cluster",
boundary: ApiBoundary::Internal,
Expand All @@ -61,7 +61,7 @@ pub fn all_apis() -> Vec<ApiSpec> {
},
ApiSpec {
title: "Oxide Management Gateway Service API",
version: "0.0.1",
version: semver::Version::new(0, 0, 1),
description: "API for interacting with the Oxide \
control plane's gateway service",
boundary: ApiBoundary::Internal,
Expand All @@ -72,7 +72,7 @@ pub fn all_apis() -> Vec<ApiSpec> {
},
ApiSpec {
title: "Internal DNS",
version: "0.0.1",
version: semver::Version::new(0, 0, 1),
description: "API for the internal DNS server",
boundary: ApiBoundary::Internal,
api_description:
Expand All @@ -82,7 +82,7 @@ pub fn all_apis() -> Vec<ApiSpec> {
},
ApiSpec {
title: "Installinator API",
version: "0.0.1",
version: semver::Version::new(0, 0, 1),
description: "API for installinator to fetch artifacts \
and report progress",
boundary: ApiBoundary::Internal,
Expand All @@ -93,7 +93,7 @@ pub fn all_apis() -> Vec<ApiSpec> {
},
ApiSpec {
title: "Oxide Region API",
version: "20241204.0",
version: semver::Version::new(20241204, 0, 0),
description: "API for interacting with the Oxide control plane",
boundary: ApiBoundary::External,
api_description:
Expand All @@ -103,7 +103,7 @@ pub fn all_apis() -> Vec<ApiSpec> {
},
ApiSpec {
title: "Nexus internal API",
version: "0.0.1",
version: semver::Version::new(0, 0, 1),
description: "Nexus internal API",
boundary: ApiBoundary::Internal,
api_description:
Expand All @@ -113,7 +113,7 @@ pub fn all_apis() -> Vec<ApiSpec> {
},
ApiSpec {
title: "Oxide Oximeter API",
version: "0.0.1",
version: semver::Version::new(0, 0, 1),
description: "API for interacting with oximeter",
boundary: ApiBoundary::Internal,
api_description:
Expand All @@ -123,7 +123,7 @@ pub fn all_apis() -> Vec<ApiSpec> {
},
ApiSpec {
title: "Oxide TUF Repo Depot API",
version: "0.0.1",
version: semver::Version::new(0, 0, 1),
description: "API for fetching update artifacts",
boundary: ApiBoundary::Internal,
api_description: repo_depot_api::repo_depot_api_mod::stub_api_description,
Expand All @@ -132,7 +132,7 @@ pub fn all_apis() -> Vec<ApiSpec> {
},
ApiSpec {
title: "Oxide Sled Agent API",
version: "0.0.1",
version: semver::Version::new(0, 0, 1),
description: "API for interacting with individual sleds",
boundary: ApiBoundary::Internal,
api_description:
Expand All @@ -142,7 +142,7 @@ pub fn all_apis() -> Vec<ApiSpec> {
},
ApiSpec {
title: "Oxide Technician Port Control Service",
version: "0.0.1",
version: semver::Version::new(0, 0, 1),
description: "API for use by the technician port TUI: wicket",
boundary: ApiBoundary::Internal,
api_description: wicketd_api::wicketd_api_mod::stub_api_description,
Expand All @@ -158,7 +158,7 @@ pub struct ApiSpec {
pub title: &'static str,

/// The version.
pub version: &'static str,
pub version: semver::Version,

/// The description string.
pub description: &'static str,
Expand Down Expand Up @@ -251,7 +251,8 @@ impl ApiSpec {
// impl formats the errors appropriately.
anyhow::anyhow!("{}", error)
})?;
let mut openapi_def = description.openapi(&self.title, &self.version);
let mut openapi_def =
description.openapi(&self.title, self.version.clone());
openapi_def
.description(&self.description)
.contact_url("https://oxide.computer")
Expand Down
1 change: 1 addition & 0 deletions internal-dns/resolver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ expectorate.workspace = true
omicron-test-utils.workspace = true
omicron-uuid-kinds.workspace = true
progenitor.workspace = true
semver.workspace = true
serde = { workspace = true, features = ["derive"] }
serde_json.workspace = true
sled.workspace = true
Expand Down
2 changes: 1 addition & 1 deletion internal-dns/resolver/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ mod test {
// Progenitor client.
fn expect_openapi_json_valid_for_test_server() {
let api = api();
let openapi = api.openapi("Test Server", "v0.1.0");
let openapi = api.openapi("Test Server", semver::Version::new(0, 1, 0));
let mut output = std::io::Cursor::new(Vec::new());
openapi.write(&mut output).unwrap();
expectorate::assert_contents(
Expand Down
2 changes: 1 addition & 1 deletion internal-dns/resolver/tests/output/test-server.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"openapi": "3.0.3",
"info": {
"title": "Test Server",
"version": "v0.1.0"
"version": "0.1.0"
},
"paths": {
"/test": {
Expand Down
2 changes: 1 addition & 1 deletion nexus/external-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use omicron_common::api::external::{
use openapi_manager_types::ValidationContext;
use openapiv3::OpenAPI;

pub const API_VERSION: &str = "20241204.0";
pub const API_VERSION: &str = "20241204.0.0";

// API ENDPOINT FUNCTION NAMING CONVENTIONS
//
Expand Down
1 change: 1 addition & 0 deletions nexus/test-utils/src/http_testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ impl<'a> RequestBuilder<'a> {
body: dropshot::Body::empty(),
expected_status: None,
allowed_headers: Some(vec![
http::header::CONNECTION,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This curious change appears to be required due to this change in hyper v1.5. Without this fix, I found an update test failed, reporting that it expected a 500. In fact it did get a 500, but it failed the validation we do on all client responses that only these allowed headers were present. It failed that validation because there was a connection header present.

Copy link
Contributor

Choose a reason for hiding this comment

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

interesting

http::header::CONTENT_ENCODING,
http::header::CONTENT_LENGTH,
http::header::CONTENT_TYPE,
Expand Down
2 changes: 1 addition & 1 deletion openapi/nexus.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"url": "https://oxide.computer",
"email": "[email protected]"
},
"version": "20241204.0"
"version": "20241204.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

I hadn't considered this consequence. I can't see any problems with it; this version string does appear in CLI/SDK stuff...

},
"paths": {
"/device/auth": {
Expand Down
1 change: 1 addition & 0 deletions oximeter/collector/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ qorb.workspace = true
rand.workspace = true
reqwest = { workspace = true, features = [ "json" ] }
schemars.workspace = true
semver.workspace = true
serde.workspace = true
slog.workspace = true
slog-async.workspace = true
Expand Down
2 changes: 1 addition & 1 deletion oximeter/collector/src/bin/oximeter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use uuid::Uuid;

pub fn run_standalone_openapi() -> Result<(), String> {
standalone_nexus_api()
.openapi("Oxide Nexus API", "0.0.1")
.openapi("Oxide Nexus API", semver::Version::new(0, 0, 1))
.description("API for interacting with Nexus")
.contact_url("https://oxide.computer")
.contact_email("[email protected]")
Expand Down
4 changes: 2 additions & 2 deletions package-manifest.toml
Original file line number Diff line number Diff line change
Expand Up @@ -621,10 +621,10 @@ service_name = "propolis-server"
only_for_targets.image = "standard"
source.type = "prebuilt"
source.repo = "propolis"
source.commit = "86101eaf80b55e7f405b5cafe9b0de0e9f331656"
source.commit = "aadc0998c0f07f08ab15a95c006074291734800f"
# The SHA256 digest is automatically posted to:
# https://buildomat.eng.oxide.computer/public/file/oxidecomputer/propolis/image/<commit>/propolis-server.sha256.txt
source.sha256 = "8dd411d6f2db23f93c2340cce11aa194da8dcb8cfd20081a614a5722ffbfe255"
source.sha256 = "3cd889201aaa8cc5b916fc8f8176ab5529e2fc1d5d57165ad9a660eb48affef9"
output.type = "zone"

[package.mg-ddm-gz]
Expand Down
1 change: 1 addition & 0 deletions sled-agent/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ omicron-test-utils.workspace = true
pretty_assertions.workspace = true
rcgen.workspace = true
reqwest = { workspace = true, features = ["blocking"] }
semver.workspace = true
subprocess.workspace = true
slog-async.workspace = true
slog-term.workspace = true
Expand Down
20 changes: 4 additions & 16 deletions sled-agent/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2102,29 +2102,17 @@ mod tests {
// TODO: factor out, this is also in sled-agent-sim.
fn propolis_mock_server(
log: &Logger,
) -> (HttpServer<Arc<propolis_mock_server::Context>>, PropolisClient) {
) -> (propolis_mock_server::Server, PropolisClient) {
let propolis_bind_address =
SocketAddr::new(Ipv6Addr::LOCALHOST.into(), 0); // allocate port
let dropshot_config = dropshot::ConfigDropshot {
let dropshot_config = propolis_mock_server::Config {
bind_address: propolis_bind_address,
..Default::default()
};
let propolis_log = log.new(o!("component" => "propolis-server-mock"));
let private =
Arc::new(propolis_mock_server::Context::new(propolis_log));
info!(log, "Starting mock propolis-server...");
let dropshot_log = log.new(o!("component" => "dropshot"));
let mock_api = propolis_mock_server::api();

let srv = dropshot::HttpServerStarter::new(
&dropshot_config,
mock_api,
private,
&dropshot_log,
)
.expect("couldn't create mock propolis-server")
.start();

let srv = propolis_mock_server::start(dropshot_config, log.clone())
.expect("couldn't create mock propolis-server");
let client = propolis_client::Client::new(&format!(
"http://{}",
srv.local_addr()
Expand Down
5 changes: 4 additions & 1 deletion sled-agent/src/sim/http_entrypoints_pantry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,10 @@ mod tests {
let Value::String(ref version) = real_api["info"]["version"] else {
unreachable!();
};
let sim_api = super::api().openapi(title, version).json().unwrap();
let sim_api = super::api()
.openapi(title, version.parse().unwrap())
.json()
.unwrap();

// We'll assert that anything which apppears in the simulated API must
// appear exactly as-is in the real API. I.e., the simulated is a subset
Expand Down
29 changes: 10 additions & 19 deletions sled-agent/src/sim/sled_agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::sim::simulatable::Simulatable;
use crate::updates::UpdateManager;
use anyhow::bail;
use anyhow::Context;
use dropshot::{HttpError, HttpServer};
use dropshot::HttpError;
use futures::lock::Mutex;
use nexus_sled_agent_shared::inventory::{
Inventory, InventoryDataset, InventoryDisk, InventoryZpool,
Expand All @@ -43,7 +43,6 @@ use oxnet::Ipv6Net;
use propolis_client::{
types::VolumeConstructionRequest, Client as PropolisClient,
};
use propolis_mock_server::Context as PropolisContext;
use sled_agent_types::disk::DiskStateRequested;
use sled_agent_types::early_networking::{
EarlyNetworkConfig, EarlyNetworkConfigBody,
Expand Down Expand Up @@ -81,7 +80,7 @@ pub struct SledAgent {
disk_id_to_region_ids: Mutex<HashMap<String, Vec<Uuid>>>,
pub v2p_mappings: Mutex<HashSet<VirtualNetworkInterfaceHost>>,
mock_propolis:
Mutex<Option<(HttpServer<Arc<PropolisContext>>, PropolisClient)>>,
Mutex<Option<(propolis_mock_server::Server, PropolisClient)>>,
/// lists of external IPs assigned to instances
pub external_ips:
Mutex<HashMap<PropolisUuid, HashSet<InstanceExternalIpBody>>>,
Expand Down Expand Up @@ -797,26 +796,18 @@ impl SledAgent {
}
let propolis_bind_address =
SocketAddr::new(Ipv6Addr::LOCALHOST.into(), 0);
let dropshot_config = dropshot::ConfigDropshot {
let dropshot_config = propolis_mock_server::Config {
bind_address: propolis_bind_address,
..Default::default()
};
let propolis_log = log.new(o!("component" => "propolis-server-mock"));
let private = Arc::new(PropolisContext::new(propolis_log));
info!(log, "Starting mock propolis-server...");
let dropshot_log = log.new(o!("component" => "dropshot"));
let mock_api = propolis_mock_server::api();

let srv = dropshot::HttpServerStarter::new(
&dropshot_config,
mock_api,
private,
&dropshot_log,
)
.map_err(|error| {
Error::unavail(&format!("initializing propolis-server: {}", error))
})?
.start();
let srv = propolis_mock_server::start(dropshot_config, log.clone())
.map_err(|error| {
Error::unavail(&format!(
"initializing propolis-server: {}",
error
))
})?;
let addr = srv.local_addr();
let client = propolis_client::Client::new(&format!("http://{}", addr));
*mock_lock = Some((srv, client));
Expand Down
Loading
Loading