Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
8 changes: 6 additions & 2 deletions dev-tools/ls-apis/api-manifest.toml
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,14 @@ label = "Crucible Pantry"
packages = [ "crucible-pantry" ]

[[deployment_units]]
label = "Cockroach Admin"
label = "Cockroach"
packages = [ "omicron-cockroach-admin" ]

# These are really three distinct deployment units, but they behave the same for
# our purposes, and the tooling doesn't support multiple deployment units
# that each expose a particular service.
[[deployment_units]]
label = "Clickhouse Admin"
label = "Clickhouse (single-node) / Clickhouse Server (multi-node) / Clickhouse Keeper (multi-node)"
packages = [ "omicron-clickhouse-admin" ]

[[deployment_units]]
Expand Down Expand Up @@ -312,6 +315,7 @@ Exposed by Crucible upstairs for debugging via the `cmon` debugging tool.
client_package_name = "dsc-client"
label = "Downstairs Controller (debugging only)"
server_package_name = "dsc"
dev_only = true
notes = """
`dsc` is a control program for spinning up and controlling instances of Crucible
downstairs for testing. You can use the same program to control a running `dsc`
Expand Down
8 changes: 8 additions & 0 deletions dev-tools/ls-apis/src/api_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,14 @@ pub struct ApiMetadata {
pub server_package_name: ServerPackageName,
/// human-readable notes about this API
pub notes: Option<String>,
/// whether this package is ever expected to be deployed in a real system
dev_only: Option<bool>,
Copy link
Contributor

Choose a reason for hiding this comment

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

What does None mean?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated the comment to reflect that None means an API is deployed on real systems.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah thanks. Worth using serde's default for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We definitely could. I don't have a strong feeling either way.

}

impl ApiMetadata {
pub fn deployed(&self) -> bool {
!(self.dev_only.unwrap_or(false))
}
}

/// Describes a unit that combines one or more servers that get deployed
Expand Down
53 changes: 43 additions & 10 deletions dev-tools/ls-apis/src/system_apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,15 @@ impl SystemApis {
);
}

// Create an index of server package names, mapping each one to the API
// that it corresponds to.
let server_packages: BTreeMap<_, _> = api_metadata
.apis()
.map(|api| (api.server_package_name.clone(), api))
.collect();
// Create an index of server package names, mapping each one to the list
// of APIs that it produces.
let mut server_packages = BTreeMap::new();
for api in api_metadata.apis() {
server_packages
.entry(api.server_package_name.clone())
.or_insert_with(Vec::new)
.push(api);
}

// Walk the deployment units, then walk each one's list of packages, and
// then walk all of its dependencies. Along the way, record whenever we
Expand Down Expand Up @@ -160,6 +163,30 @@ impl SystemApis {
let (apis_consumed, api_consumers) =
(deps_tracker.apis_consumed, deps_tracker.api_consumers);

// Make sure that each API is produced by at least one producer.
for api in api_metadata.apis() {
let found_producer = api_producers.get(&api.client_package_name);
if api.deployed() {
if found_producer.is_none() {
bail!(
"error: found no producer for API with client package \
name {:?} in any deployment unit (should have been \
one that contains server package {:?})",
api.client_package_name,
api.server_package_name,
);
}
} else if let Some(found) = found_producer {
bail!(
"error: metadata says there should be no deployed \
producer for API with client package name {:?}, but found \
one: {:?}",
api.client_package_name,
found
);
}
}

Ok(SystemApis {
server_component_units,
unit_server_components,
Expand Down Expand Up @@ -375,7 +402,8 @@ impl SystemApis {
/// See `SystemApis::load()` for how this is used.
struct ServerComponentsTracker<'a> {
// inputs
known_server_packages: &'a BTreeMap<ServerPackageName, &'a ApiMetadata>,
known_server_packages:
&'a BTreeMap<ServerPackageName, Vec<&'a ApiMetadata>>,

// outputs (structures that we're building up)
errors: Vec<anyhow::Error>,
Expand All @@ -387,7 +415,10 @@ struct ServerComponentsTracker<'a> {

impl<'a> ServerComponentsTracker<'a> {
pub fn new(
known_server_packages: &'a BTreeMap<ServerPackageName, &'a ApiMetadata>,
known_server_packages: &'a BTreeMap<
ServerPackageName,
Vec<&'a ApiMetadata>,
>,
) -> ServerComponentsTracker<'a> {
ServerComponentsTracker {
known_server_packages,
Expand Down Expand Up @@ -470,11 +501,13 @@ impl<'a> ServerComponentsTracker<'a> {
pkgname: &str,
dep_path: &DepPath,
) {
let Some(api) = self.known_server_packages.get(pkgname) else {
let Some(apis) = self.known_server_packages.get(pkgname) else {
return;
};

self.found_api_producer(api, dunit_pkgname, dep_path);
for api in apis {
self.found_api_producer(api, dunit_pkgname, dep_path);
}
}

/// Record that the given package is one of the deployment unit's top-level
Expand Down
Loading