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

prototype conversion to new dropshot builder interface #6730

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
105 changes: 51 additions & 54 deletions Cargo.lock

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -794,8 +794,9 @@ 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"]
#dropshot = { path = "../dropshot/dropshot" }
[patch.crates-io.dropshot]
path = "../dropshot-builder/dropshot"

#[patch.crates-io]
#steno = { path = "../steno" }
# [patch."https://github.com/oxidecomputer/propolis"]
Expand Down
14 changes: 6 additions & 8 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 @@ -63,13 +62,12 @@ pub async fn start_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::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 @@ -79,14 +79,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/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,9 +776,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,14 +83,14 @@ impl InternalServer {
.await?;

// Launch the internal server.
let server_starter_internal = dropshot::HttpServerStarter::new(
&config.deployment.dropshot_internal,
let http_server_internal = 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))?;
let http_server_internal = server_starter_internal.start();

Ok(Self {
apictx: context,
Expand Down Expand Up @@ -150,32 +150,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::resolver::ResolveError;
use internal_dns::resolver::Resolver;
use internal_dns::ServiceName;
Expand Down Expand Up @@ -239,17 +239,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 @@ -333,17 +333,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
12 changes: 6 additions & 6 deletions oximeter/collector/src/standalone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ use dropshot::HttpError;
use dropshot::HttpResponseCreated;
use dropshot::HttpResponseUpdatedNoContent;
use dropshot::HttpServer;
use dropshot::HttpServerStarter;
use dropshot::RequestContext;
use dropshot::ServerBuilder;
use dropshot::TypedBody;
use nexus_types::internal_api::params::OximeterInfo;
use omicron_common::api::internal::nexus::ProducerEndpoint;
Expand Down Expand Up @@ -251,14 +251,14 @@ impl Server {
let nexus = Arc::new(StandaloneNexus::new(
log.new(slog::o!("component" => "nexus-standalone")),
));
let server = HttpServerStarter::new(
&ConfigDropshot { bind_address: address, ..Default::default() },
let server = ServerBuilder::new(
standalone_nexus_api(),
Arc::clone(&nexus),
&log,
log.clone(),
)
.map_err(|e| Error::Server(e.to_string()))?
.start();
.config(ConfigDropshot { bind_address: address, ..Default::default() })
.start()
.map_err(|e| Error::Server(e.to_string()))?;
info!(
log,
"created standalone nexus server for metric collections";
Expand Down
Loading
Loading