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

propolis-mock-server should expose more types #808

Merged
merged 4 commits into from
Nov 13, 2024
Merged
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
32 changes: 31 additions & 1 deletion bin/mock-server/src/lib/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use dropshot::{
TypedBody, WebsocketConnection,
};
use futures::SinkExt;
use slog::{error, info, Logger};
use slog::{error, info, o, Logger};
use thiserror::Error;
use tokio::sync::{watch, Mutex};
use tokio_tungstenite::tungstenite::protocol::{Role, WebSocketConfig};
Expand Down Expand Up @@ -654,6 +654,10 @@ mod serial {

/// Returns a Dropshot [`ApiDescription`] object to launch a mock Propolis
/// server.
///
/// This function should be avoided in favor of `start()` because using this
/// function requires that the consumer and Propolis update Dropshot
/// dependencies in lockstep due to the sharing of various types.
Comment on lines +657 to +660
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just remove the pub instead? AFAIK (from a global GitHub search) the only user of this crate who calls this is simulated sled agent, which is presumably the consumer we want to switch to using start.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to do that but would prefer to do it in a separate PR to minimize the chance of breakage on the critical path here.

pub fn api() -> ApiDescription<Arc<Context>> {
let mut api = ApiDescription::new();
api.register(instance_ensure).unwrap();
Expand All @@ -664,3 +668,29 @@ pub fn api() -> ApiDescription<Arc<Context>> {
api.register(instance_serial_history_get).unwrap();
api
}

// These types need to be exposed so that consumers have names for them without
// having to maintain a dropshot dependency in lockstep with their dependency on
// this crate.

/// configuration for the dropshot server
pub type Config = dropshot::ConfigDropshot;
/// the dropshot server itself
pub type Server = dropshot::HttpServer<Arc<Context>>;
/// errors returned from attempting to start a dropshot server
// Dropshot should expose this, but it's going to be removed anyway.
pub type ServerStartError = Box<dyn std::error::Error + Send + Sync>;

/// Starts a Propolis mock server
pub fn start(config: Config, log: Logger) -> Result<Server, ServerStartError> {
let propolis_log = log.new(o!("component" => "propolis-server-mock"));
let dropshot_log = log.new(o!("component" => "dropshot"));
let private = Arc::new(Context::new(propolis_log));
let starter = dropshot::HttpServerStarter::new(
&config,
api(),
private,
&dropshot_log,
)?;
Ok(starter.start())
}
Loading