prototype conversion to new dropshot builder interface

Author: davepacheco

Cargo.lock

@@ -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"]

Cargo.toml

@@ -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;
 
@@ -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>>;
@@ -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)
 }

clickhouse-admin/src/lib.rs

@@ -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;
 
@@ -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>>;
@@ -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)
 }

cockroach-admin/src/lib.rs

@@ -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))

dns-server/src/lib.rs

@@ -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(())

gateway/src/lib.rs

@@ -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;
@@ -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)
-}

installinator-api/src/lib.rs

@@ -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]

internal-dns/src/resolver.rs

@@ -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();
 

nexus/src/app/external_endpoints.rs

@@ -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,
@@ -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

nexus/src/lib.rs

@@ -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;
@@ -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 {
@@ -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.

oximeter/collector/src/lib.rs

@@ -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;
@@ -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";