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

convert to new dropshot builder interface #6730

Merged
merged 21 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
c0ea923
prototype conversion to new dropshot builder interface
davepacheco Sep 30, 2024
c7c0f3e
test build for dropshot + API versioning
davepacheco Nov 13, 2024
fe3331e
rustfmt
davepacheco Nov 13, 2024
3a17ac3
fix expectorate
davepacheco Nov 13, 2024
fd95b56
fix test failure in integration_tests::updates::test_update_uninitial…
davepacheco Nov 13, 2024
c4e7281
minimum changes for Dropshot v0.13.0
davepacheco Nov 13, 2024
6806e0b
avoid tight coupling with propolis Dropshot version
davepacheco Nov 13, 2024
97d7d4d
does not need to be async
davepacheco Nov 13, 2024
3da800b
update dep
davepacheco Nov 13, 2024
294caf7
update Cargo.lock
davepacheco Nov 13, 2024
5183381
Merge remote-tracking branch 'origin/dap/fix-dropshot-coupling' into …
davepacheco Nov 13, 2024
25d67e7
update Propolis for mock server updates
davepacheco Nov 13, 2024
afa4f8b
Merge branch 'dap/propolis-update' into dap/fix-dropshot-coupling
davepacheco Nov 13, 2024
d94c953
update Cargo.lock too
davepacheco Nov 13, 2024
9380f0b
Merge branch 'dap/propolis-update' into dap/fix-dropshot-coupling
davepacheco Nov 13, 2024
35f0467
Merge remote-tracking branch 'origin/dap/fix-dropshot-coupling' into …
davepacheco Nov 13, 2024
892047b
hakari
davepacheco Nov 13, 2024
5895d1d
Merge remote-tracking branch 'origin/dap/drafts/dropshot-update' into…
davepacheco Nov 13, 2024
ce04ed2
fix up stuff that had conflicted with main
davepacheco Nov 13, 2024
3ddde59
Merge branch 'main' into dap/dropshot-builder
davepacheco Nov 14, 2024
1d40776
Merge branch 'main' into dap/dropshot-builder
davepacheco Nov 14, 2024
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
27 changes: 13 additions & 14 deletions clickhouse-admin/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use omicron_common::FileKv;
use slog::{debug, error, Drain};
use slog_dtrace::ProbeRegistration;
use slog_error_chain::SlogInlineError;
use std::error::Error;
use std::io;
use std::sync::Arc;

Expand All @@ -28,7 +27,7 @@ pub enum StartError {
#[error("failed to register dtrace probes: {0}")]
RegisterDtraceProbes(String),
#[error("failed to initialize HTTP server")]
InitializeHttpServer(#[source] Box<dyn Error + Send + Sync>),
InitializeHttpServer(#[source] dropshot::BuildError),
}

pub type Server = dropshot::HttpServer<Arc<ServerContext>>;
Expand Down Expand Up @@ -64,15 +63,15 @@ pub async fn start_server_admin_server(
.with_log(log.new(slog::o!("component" => "ClickhouseCli"))),
log.new(slog::o!("component" => "ServerContext")),
);
let http_server_starter = dropshot::HttpServerStarter::new(
&server_config.dropshot,

dropshot::ServerBuilder::new(
http_entrypoints::clickhouse_admin_server_api(),
Arc::new(context),
&log.new(slog::o!("component" => "dropshot")),
log.new(slog::o!("component" => "dropshot")),
)
.map_err(StartError::InitializeHttpServer)?;

Ok(http_server_starter.start())
.config(server_config.dropshot)
.start()
.map_err(StartError::InitializeHttpServer)
}

/// Start the dropshot server for `clickhouse-admin-server` which
Expand Down Expand Up @@ -106,13 +105,13 @@ pub async fn start_keeper_admin_server(
.with_log(log.new(slog::o!("component" => "ClickhouseCli"))),
log.new(slog::o!("component" => "ServerContext")),
);
let http_server_starter = dropshot::HttpServerStarter::new(
&server_config.dropshot,

dropshot::ServerBuilder::new(
http_entrypoints::clickhouse_admin_keeper_api(),
Arc::new(context),
&log.new(slog::o!("component" => "dropshot")),
log.new(slog::o!("component" => "dropshot")),
)
.map_err(StartError::InitializeHttpServer)?;

Ok(http_server_starter.start())
.config(server_config.dropshot)
.start()
.map_err(StartError::InitializeHttpServer)
}
14 changes: 6 additions & 8 deletions cockroach-admin/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use slog::error;
use slog::Drain;
use slog_dtrace::ProbeRegistration;
use slog_error_chain::SlogInlineError;
use std::error::Error;
use std::io;
use std::sync::Arc;

Expand All @@ -30,7 +29,7 @@ pub enum StartError {
#[error("failed to register dtrace probes: {0}")]
RegisterDtraceProbes(String),
#[error("failed to initialize HTTP server")]
InitializeHttpServer(#[source] Box<dyn Error + Send + Sync>),
InitializeHttpServer(#[source] dropshot::BuildError),
}

pub type Server = dropshot::HttpServer<Arc<ServerContext>>;
Expand Down Expand Up @@ -64,13 +63,12 @@ pub async fn start_server(
cockroach_cli,
log.new(slog::o!("component" => "ServerContext")),
);
let http_server_starter = dropshot::HttpServerStarter::new(
&server_config.dropshot,
dropshot::ServerBuilder::new(
http_entrypoints::api(),
Arc::new(context),
&log.new(slog::o!("component" => "dropshot")),
log.new(slog::o!("component" => "dropshot")),
)
.map_err(StartError::InitializeHttpServer)?;

Ok(http_server_starter.start())
.config(server_config.dropshot)
.start()
.map_err(StartError::InitializeHttpServer)
}
8 changes: 4 additions & 4 deletions dns-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,14 @@ pub async fn start_servers(
let http_api = http_server::api();
let http_api_context = http_server::Context::new(store);

dropshot::HttpServerStarter::new(
dropshot_config,
dropshot::ServerBuilder::new(
http_api,
http_api_context,
&log.new(o!("component" => "http")),
log.new(o!("component" => "http")),
)
.map_err(|error| anyhow!("setting up HTTP server: {:#}", error))?
.config(dropshot_config.clone())
.start()
.map_err(|error| anyhow!("setting up HTTP server: {:#}", error))?
};

Ok((dns_server, dropshot_server))
Expand Down
38 changes: 21 additions & 17 deletions gateway/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,25 +87,29 @@ fn start_dropshot_server(
all_servers_shutdown: &FuturesUnordered<ShutdownWaitFuture>,
log: &Logger,
) -> Result<(), String> {
let dropshot = ConfigDropshot {
bind_address: SocketAddr::V6(addr),
request_body_max_bytes,
default_handler_task_mode: HandlerTaskMode::Detached,
log_headers: vec![],
};
let http_server_starter = dropshot::HttpServerStarter::new(
&dropshot,
http_entrypoints::api(),
Arc::clone(apictx),
&log.new(o!("component" => "dropshot")),
)
.map_err(|error| {
format!("initializing http server listening at {addr}: {}", error)
})?;

match http_servers.entry(addr) {
Entry::Vacant(slot) => {
let http_server = http_server_starter.start();
let dropshot = ConfigDropshot {
bind_address: SocketAddr::V6(addr),
request_body_max_bytes,
default_handler_task_mode: HandlerTaskMode::Detached,
log_headers: vec![],
};

let http_server = dropshot::ServerBuilder::new(
http_entrypoints::api(),
Arc::clone(apictx),
log.new(o!("component" => "dropshot")),
)
.config(dropshot)
.start()
.map_err(|error| {
format!(
"initializing http server listening at {addr}: {}",
error
)
})?;

all_servers_shutdown.push(http_server.wait_for_shutdown());
slot.insert(http_server);
Ok(())
Expand Down
39 changes: 3 additions & 36 deletions installinator-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
//! named by the client, since it is expected that multiple services will
//! implement it.
use anyhow::{anyhow, Result};
use anyhow::Result;
use dropshot::{
Body, ConfigDropshot, FreeformBody, HandlerTaskMode, HttpError,
HttpResponseHeaders, HttpResponseOk, HttpResponseUpdatedNoContent,
HttpServerStarter, Path, RequestContext, TypedBody,
HttpResponseHeaders, HttpResponseOk, HttpResponseUpdatedNoContent, Path,
RequestContext, TypedBody,
};
use hyper::{header, StatusCode};
use installinator_common::EventReport;
Expand Down Expand Up @@ -134,36 +134,3 @@ pub fn default_config(bind_address: std::net::SocketAddr) -> ConfigDropshot {
log_headers: vec![],
}
}

/// Make an `HttpServerStarter` for the installinator API with default settings.
pub fn make_server_starter<T: InstallinatorApi>(
context: T::Context,
bind_address: std::net::SocketAddr,
log: &slog::Logger,
) -> Result<HttpServerStarter<T::Context>> {
let dropshot_config = dropshot::ConfigDropshot {
bind_address,
// Even though the installinator sets an upper bound on the number
// of items in a progress report, they can get pretty large if they
// haven't gone through for a bit. Ensure that hitting the max
// request size won't cause a failure by setting a generous upper
// bound for the request size.
//
// TODO: replace with an endpoint-specific option once
// https://github.com/oxidecomputer/dropshot/pull/618 lands and is
// available in omicron.
request_body_max_bytes: 4 * 1024 * 1024,
default_handler_task_mode: HandlerTaskMode::Detached,
log_headers: vec![],
};

let api = crate::installinator_api_mod::api_description::<T>()?;
let server =
dropshot::HttpServerStarter::new(&dropshot_config, api, context, &log)
.map_err(|error| {
anyhow!(error)
.context("failed to create installinator artifact server")
})?;

Ok(server)
}
5 changes: 3 additions & 2 deletions internal-dns/resolver/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -816,9 +816,10 @@ mod test {
bind_address: "[::1]:0".parse().unwrap(),
..Default::default()
};
dropshot::HttpServerStarter::new(&config_dropshot, api(), label, &log)
.unwrap()
dropshot::ServerBuilder::new(api(), label, log)
.config(config_dropshot)
.start()
.unwrap()
}

#[tokio::test]
Expand Down
11 changes: 3 additions & 8 deletions nexus/src/app/external_endpoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1323,14 +1323,9 @@ mod test {
let logctx = omicron_test_utils::dev::test_setup_log("test_authority");
let mut api = dropshot::ApiDescription::new();
api.register(echo_server_name).unwrap();
let server = dropshot::HttpServerStarter::new(
&dropshot::ConfigDropshot::default(),
api,
(),
&logctx.log,
)
.expect("failed to create dropshot server")
.start();
let server = dropshot::ServerBuilder::new(api, (), logctx.log.clone())
.start()
.expect("failed to create dropshot server");
let local_addr = server.local_addr();
let port = local_addr.port();

Expand Down
54 changes: 26 additions & 28 deletions nexus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,13 @@ impl InternalServer {
.await?;

// Launch the internal server.
let server_starter_internal = match dropshot::HttpServerStarter::new(
&config.deployment.dropshot_internal,
let http_server_internal = match dropshot::ServerBuilder::new(
internal_api(),
context.clone(),
&log.new(o!("component" => "dropshot_internal")),
log.new(o!("component" => "dropshot_internal")),
)
.config(config.deployment.dropshot_internal.clone())
.start()
.map_err(|error| format!("initializing internal server: {}", error))
{
Ok(server) => server,
Expand All @@ -97,7 +98,6 @@ impl InternalServer {
return Err(err);
}
};
let http_server_internal = server_starter_internal.start();

Ok(Self {
apictx: context,
Expand Down Expand Up @@ -157,32 +157,30 @@ impl Server {
};

let http_server_external = {
let server_starter_external =
dropshot::HttpServerStarter::new_with_tls(
&config.deployment.dropshot_external.dropshot,
external_api(),
apictx.for_external(),
&log.new(o!("component" => "dropshot_external")),
tls_config.clone().map(dropshot::ConfigTls::Dynamic),
)
.map_err(|error| {
format!("initializing external server: {}", error)
})?;
server_starter_external.start()
dropshot::ServerBuilder::new(
external_api(),
apictx.for_external(),
log.new(o!("component" => "dropshot_external")),
)
.config(config.deployment.dropshot_external.dropshot.clone())
.tls(tls_config.clone().map(dropshot::ConfigTls::Dynamic))
.start()
.map_err(|error| {
format!("initializing external server: {}", error)
})?
};
let http_server_techport_external = {
let server_starter_external_techport =
dropshot::HttpServerStarter::new_with_tls(
&techport_server_config,
external_api(),
apictx.for_techport(),
&log.new(o!("component" => "dropshot_external_techport")),
tls_config.map(dropshot::ConfigTls::Dynamic),
)
.map_err(|error| {
format!("initializing external techport server: {}", error)
})?;
server_starter_external_techport.start()
dropshot::ServerBuilder::new(
external_api(),
apictx.for_techport(),
log.new(o!("component" => "dropshot_external_techport")),
)
.config(techport_server_config)
.tls(tls_config.map(dropshot::ConfigTls::Dynamic))
.start()
.map_err(|error| {
format!("initializing external techport server: {}", error)
})?
};

// Start the metric producer server that oximeter uses to fetch our
Expand Down
34 changes: 17 additions & 17 deletions oximeter/collector/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use dropshot::ConfigDropshot;
use dropshot::ConfigLogging;
use dropshot::HttpError;
use dropshot::HttpServer;
use dropshot::HttpServerStarter;
use dropshot::ServerBuilder;
use internal_dns_types::names::ServiceName;
use omicron_common::address::get_internal_dns_server_addresses;
use omicron_common::address::DNS_PORT;
Expand Down Expand Up @@ -311,17 +311,17 @@ impl Oximeter {
.expect("Expected an infinite retry loop initializing the timeseries database");

let dropshot_log = log.new(o!("component" => "dropshot"));
let server = HttpServerStarter::new(
&ConfigDropshot {
bind_address: SocketAddr::V6(args.address),
..Default::default()
},
let server = ServerBuilder::new(
oximeter_api(),
Arc::clone(&agent),
&dropshot_log,
dropshot_log,
)
.map_err(|e| Error::Server(e.to_string()))?
.start();
.config(ConfigDropshot {
bind_address: SocketAddr::V6(args.address),
..Default::default()
})
.start()
.map_err(|e| Error::Server(e.to_string()))?;

// Notify Nexus that this oximeter instance is available.
let our_info = nexus_client::types::OximeterInfo {
Expand Down Expand Up @@ -422,17 +422,17 @@ impl Oximeter {
);

let dropshot_log = log.new(o!("component" => "dropshot"));
let server = HttpServerStarter::new(
&ConfigDropshot {
bind_address: SocketAddr::V6(args.address),
..Default::default()
},
let server = ServerBuilder::new(
oximeter_api(),
Arc::clone(&agent),
&dropshot_log,
dropshot_log,
)
.map_err(|e| Error::Server(e.to_string()))?
.start();
.config(ConfigDropshot {
bind_address: SocketAddr::V6(args.address),
..Default::default()
})
.start()
.map_err(|e| Error::Server(e.to_string()))?;
info!(log, "started oximeter standalone server");

// Notify the standalone nexus.
Expand Down
Loading
Loading