Skip to content

Commit

Permalink
convert to new dropshot builder interface (#6730)
Browse files Browse the repository at this point in the history
  • Loading branch information
davepacheco authored Nov 14, 2024
1 parent 41b78f1 commit 7cf2b90
Show file tree
Hide file tree
Showing 18 changed files with 167 additions and 215 deletions.
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

0 comments on commit 7cf2b90

Please sign in to comment.