[ls-apis] support APIs exposed by multiple servers

hawkw · hawkw · commit 564ce9638958 · 2025-03-20T10:48:50.000-07:00
Currently, `cargo xtask ls-apis` will fail when it encounters an API that is served by multiple entities. For example, when adding the `ereporter-api` in #7833, which is served by both MGS and sled-agent, `ls-apis` currently fails with an error like this one: ```console eliza@theseus ~/Code/oxide/omicron $ cargo xtask ls-apis apis Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.45s Running `target/debug/xtask ls-apis apis` Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.01s Running `target/debug/ls-apis apis` loading metadata for workspace omicron from current workspace loading metadata for workspace lldp from /home/eliza/.cargo/git/checkouts/lldp-d47de417041f191b/ce952e6/Cargo.toml loading metadata for workspace propolis from /home/eliza/.cargo/git/checkouts/propolis-d68c8bd1bc59c9bd/6b5f2af/Cargo.toml loading metadata for workspace crucible from /home/eliza/.cargo/git/checkouts/crucible-0a48bd218bc2bbbc/81a3528/Cargo.toml loading metadata for workspace maghemite from /home/eliza/.cargo/git/checkouts/maghemite-c0236f0fd3d582b6/caafd88/Cargo.toml loading metadata for workspace dendrite from /home/eliza/.cargo/git/checkouts/dendrite-ae9f1715c17fc765/a66561e/Cargo.toml note: ignoring Cargo dependency from crucible-pantry -> ... -> crucible-control-client note: ignoring Cargo dependency from omicron-sled-agent -> dns-server error: API for client ereporter-client appears to be exported by multiple components: at least omicron-sled-agent and omicron-gateway (DepPath([PackageId { repr: "path+file:///home/eliza/Code/oxide/omicron/gateway#omicron-gateway@0.1.0" }])) Error: found at least one API exported by multiple servers # ... backtrace snipped for brevity ... ``` This is unfortunate, as it would be nice to be able to have such APIs without breaking `ls-apis`. Therefore, this commit adds support for APIs with multiple server components. This change ends up being pretty straightforward; it's mainly just changing the `api_producers` map from a map of client package names to server components to a map of client package names to *a map of* server components, and updating the code that consumes this information to loop over those server components as appropriate. Now, the various `ls-apis` commands can handle the new `ereproter-api`. For example: ```console eliza@theseus ~/Code/oxide/omicron $ cargo xtask ls-apis apis Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.37s Running `target/debug/xtask ls-apis apis` Compiling omicron-ls-apis v0.1.0 (/home/eliza/Code/oxide/omicron/dev-tools/ls-apis) Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.69s Running `target/debug/ls-apis apis` loading metadata for workspace omicron from current workspace loading metadata for workspace lldp from /home/eliza/.cargo/git/checkouts/lldp-d47de417041f191b/ce952e6/Cargo.toml loading metadata for workspace maghemite from /home/eliza/.cargo/git/checkouts/maghemite-c0236f0fd3d582b6/caafd88/Cargo.toml loading metadata for workspace propolis from /home/eliza/.cargo/git/checkouts/propolis-d68c8bd1bc59c9bd/6b5f2af/Cargo.toml loading metadata for workspace crucible from /home/eliza/.cargo/git/checkouts/crucible-0a48bd218bc2bbbc/81a3528/Cargo.toml loading metadata for workspace dendrite from /home/eliza/.cargo/git/checkouts/dendrite-ae9f1715c17fc765/a66561e/Cargo.toml note: ignoring Cargo dependency from crucible-pantry -> ... -> crucible-control-client note: ignoring Cargo dependency from omicron-sled-agent -> dns-server Bootstrap Agent (client: bootstrap-agent-client) consumed by: omicron-sled-agent (omicron/sled-agent) via 1 path consumed by: wicketd (omicron/wicketd) via 2 paths # ... snipped irrelevant APIs for brevity ... Ereporter (client: ereporter-client) consumed by: omicron-nexus (omicron/nexus) via 1 path # ... snipped irrelevant APIs for brevity ... eliza@theseus ~/Code/oxide/omicron $ cargo xtask ls-apis check Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.32s Running `target/debug/xtask ls-apis check` Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.52s Running `target/debug/ls-apis check` loading metadata for workspace omicron from current workspace loading metadata for workspace lldp from /home/eliza/.cargo/git/checkouts/lldp-d47de417041f191b/ce952e6/Cargo.toml loading metadata for workspace maghemite from /home/eliza/.cargo/git/checkouts/maghemite-c0236f0fd3d582b6/caafd88/Cargo.toml loading metadata for workspace propolis from /home/eliza/.cargo/git/checkouts/propolis-d68c8bd1bc59c9bd/6b5f2af/Cargo.toml loading metadata for workspace crucible from /home/eliza/.cargo/git/checkouts/crucible-0a48bd218bc2bbbc/81a3528/Cargo.toml loading metadata for workspace dendrite from /home/eliza/.cargo/git/checkouts/dendrite-ae9f1715c17fc765/a66561e/Cargo.toml note: ignoring Cargo dependency from crucible-pantry -> ... -> crucible-control-client note: ignoring Cargo dependency from omicron-sled-agent -> dns-server Server-managed APIs: Clickhouse Cluster Admin for Keepers (clickhouse-admin-keeper-client, exposed by omicron-clickhouse-admin) Clickhouse Cluster Admin for Servers (clickhouse-admin-server-client, exposed by omicron-clickhouse-admin) Clickhouse Single-Node Cluster Admin (clickhouse-admin-single-client, exposed by omicron-clickhouse-admin) CockroachDB Cluster Admin (cockroach-admin-client, exposed by omicron-cockroach-admin) Crucible Agent (crucible-agent-client, exposed by crucible-agent) Crucible Control (for testing only) (crucible-control-client, exposed by propolis-server) Crucible Pantry (crucible-pantry-client, exposed by crucible-pantry) Maghemite DDM Admin (ddm-admin-client, exposed by ddmd) DNS Server (dns-service-client, exposed by dns-server) Ereporter (ereporter-client, exposed by omicron-gateway, omicron-sled-agent) Management Gateway Service (gateway-client, exposed by omicron-gateway) LLDP daemon (lldpd-client, exposed by lldpd) Maghemite MG Admin (mg-admin-client, exposed by mgd) External API (oxide-client, exposed by omicron-nexus) Oximeter (oximeter-client, exposed by oximeter-collector) Propolis (propolis-client, exposed by propolis-server) Sled Agent (sled-agent-client, exposed by omicron-sled-agent) Wicketd (wicketd-client, exposed by wicketd) Client-managed API: Bootstrap Agent (bootstrap-agent-client, exposed by omicron-sled-agent) reason: depends on itself (i.e., instances call each other) Dendrite DPD (dpd-client, exposed by dpd) reason: Sled Agent calling DPD creates a circular dependency Wicketd Installinator (installinator-client, exposed by wicketd) reason: client is provided implicitly by the operator Nexus Internal API (nexus-client, exposed by omicron-nexus) reason: Circular dependencies between Nexus and other services Crucible Repair (repair-client, exposed by crucible-downstairs) reason: depends on itself (i.e., instances call each other) Repo Depot API (repo-depot-client, exposed by omicron-sled-agent) reason: depends on itself (i.e., instances call each other) APIs with unknown version management: none ````
diff --git a/dev-tools/ls-apis/src/bin/ls-apis.rs b/dev-tools/ls-apis/src/bin/ls-apis.rs
@@ -8,8 +8,8 @@ use anyhow::{Context, Result, bail};
 use camino::Utf8PathBuf;
 use clap::{Args, Parser, Subcommand};
 use omicron_ls_apis::{
-    AllApiMetadata, ApiDependencyFilter, LoadArgs, ServerComponentName,
-    SystemApis, VersionedHow,
+    AllApiMetadata, ApiDependencyFilter, ApiMetadata, LoadArgs,
+    ServerComponentName, SystemApis, VersionedHow,
 };
 use parse_display::{Display, FromStr};
 
@@ -108,34 +108,33 @@ fn run_adoc(apis: &SystemApis) -> Result<()> {
 
     let metadata = apis.api_metadata();
     for api in metadata.apis() {
-        let Some(server_component) =
-            apis.api_producer(&api.client_package_name)
-        else {
-            continue;
-        };
-        println!("// DO NOT EDIT.  This table is auto-generated. See above.");
-        println!("|{}", api.label);
-        println!("|{}", apis.adoc_label(server_component)?);
-        println!("|{}", apis.adoc_label(&api.client_package_name)?);
-        println!("|");
-
-        for (c, _) in apis.api_consumers(
-            &api.client_package_name,
-            ApiDependencyFilter::default(),
-        )? {
-            println!("* {}", apis.adoc_label(c)?);
-        }
-
-        match &api.versioned_how {
-            VersionedHow::Unknown => println!("|TBD"),
-            VersionedHow::Server => println!("|Server-side only"),
-            VersionedHow::Client(reason) => {
-                println!("|Client-side ({})", reason);
+        for server_component in apis.api_producers(&api.client_package_name) {
+            println!(
+                "// DO NOT EDIT.  This table is auto-generated. See above."
+            );
+            println!("|{}", api.label);
+            println!("|{}", apis.adoc_label(server_component)?);
+            println!("|{}", apis.adoc_label(&api.client_package_name)?);
+            println!("|");
+
+            for (c, _) in apis.api_consumers(
+                &api.client_package_name,
+                ApiDependencyFilter::default(),
+            )? {
+                println!("* {}", apis.adoc_label(c)?);
             }
-        };
 
-        print!("|{}", api.notes.as_deref().unwrap_or("-\n"));
-        println!("");
+            match &api.versioned_how {
+                VersionedHow::Unknown => println!("|TBD"),
+                VersionedHow::Server => println!("|Server-side only"),
+                VersionedHow::Client(reason) => {
+                    println!("|Client-side ({})", reason);
+                }
+            };
+
+            print!("|{}", api.notes.as_deref().unwrap_or("-\n"));
+            println!("");
+        }
     }
 
     println!("|===\n");
@@ -214,12 +213,10 @@ fn print_server_components<'a>(
     for s in server_components.into_iter() {
         let (repo_name, pkg_path) = apis.package_label(s)?;
         println!("{}{} ({}/{})", prefix, s, repo_name, pkg_path);
-        for api in metadata.apis().filter(|a| {
-            matches!(
-                apis.api_producer(&a.client_package_name),
-                Some (name) if name == s
-            )
-        }) {
+        for api in metadata
+            .apis()
+            .filter(|a| apis.is_producer_of(s, &a.client_package_name))
+        {
             println!(
                 "{}    exposes: {} (client = {})",
                 prefix, api.label, api.client_package_name
@@ -301,31 +298,33 @@ fn run_check(apis: &SystemApis) -> Result<()> {
         );
     }
 
+    fn print_api_and_producers(api: &ApiMetadata, apis: &SystemApis) {
+        print!("    {} ({}", api.label, api.client_package_name,);
+        let mut producers = apis.api_producers(&api.client_package_name);
+        if let Some(producer) = producers.next() {
+            print!(", exposed by {producer}");
+            for producer in producers {
+                print!(", {producer}")
+            }
+        }
+        println!(")");
+    }
+
     println!("\n");
     println!("Server-managed APIs:\n");
     for api in apis
         .api_metadata()
         .apis()
         .filter(|f| f.deployed() && f.versioned_how == VersionedHow::Server)
     {
-        println!(
-            "    {} ({}, exposed by {})",
-            api.label,
-            api.client_package_name,
-            apis.api_producer(&api.client_package_name).unwrap()
-        );
+        print_api_and_producers(api, apis);
     }
 
     println!("\n");
     println!("Client-managed API:\n");
     for api in apis.api_metadata().apis().filter(|f| f.deployed()) {
         if let VersionedHow::Client(reason) = &api.versioned_how {
-            println!(
-                "    {} ({}, exposed by {})",
-                api.label,
-                api.client_package_name,
-                apis.api_producer(&api.client_package_name).unwrap()
-            );
+            print_api_and_producers(api, apis);
             println!("        reason: {}", reason);
         }
     }
@@ -342,12 +341,7 @@ fn run_check(apis: &SystemApis) -> Result<()> {
     } else {
         println!("\n");
         for api in unknown {
-            println!(
-                "    {} ({}, exposed by {})",
-                api.label,
-                api.client_package_name,
-                apis.api_producer(&api.client_package_name).unwrap()
-            );
+            print_api_and_producers(api, apis);
         }
         bail!("at least one API has unknown version strategy (see above)");
     }
diff --git a/dev-tools/ls-apis/src/lib.rs b/dev-tools/ls-apis/src/lib.rs
@@ -10,6 +10,7 @@ mod system_apis;
 mod workspaces;
 
 pub use api_metadata::AllApiMetadata;
+pub use api_metadata::ApiMetadata;
 pub use api_metadata::VersionedHow;
 pub use system_apis::ApiDependencyFilter;
 pub use system_apis::SystemApis;
diff --git a/dev-tools/ls-apis/src/system_apis.rs b/dev-tools/ls-apis/src/system_apis.rs
