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.

Sign up for GitHub

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

[ls-apis] support APIs exposed by multiple servers #7843

Merged
merged 3 commits into from
Mar 20, 2025
+179 −168
Merged

[ls-apis] support APIs exposed by multiple servers #7843

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
564ce96
[ls-apis] support APIs exposed by multiple servers
hawkw Mar 20, 2025
adf046e
[ls-apis] output bulleted list of servers in table cell
hawkw Mar 20, 2025
dc84a1b
[ls-apis] allow multiple dep paths for servers
hawkw Mar 20, 2025
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
98 changes: 46 additions & 52 deletions dev-tools/ls-apis/src/bin/ls-apis.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you post to the PR the output of this when there are multiple producers of an API?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added the output for ls-apis servers to the PR description. Here's the relevant bits (omicron-gateway and omicron-sled-agent both include the ereporter API):

...
omicron-sled-agent (omicron/sled-agent)
    exposes: Bootstrap Agent (client = bootstrap-agent-client)
    exposes: Ereporter (client = ereporter-client)
    exposes: Repo Depot API (client = repo-depot-client)
    exposes: Sled Agent (client = sled-agent-client)
    consumes: bootstrap-agent-client
    consumes: ddm-admin-client
    consumes: dns-service-client
    consumes: dpd-client
    consumes: gateway-client
    consumes: mg-admin-client
    consumes: nexus-client
    consumes: propolis-client
    consumes: repo-depot-client

...

omicron-gateway (omicron/gateway)
    exposes: Ereporter (client = ereporter-client)
    exposes: Management Gateway Service (client = gateway-client)
 
...

.apis()
.filter(|a| apis.is_producer_of(s, &a.client_package_name))
{
println!(
"{} exposes: {} (client = {})",
prefix, api.label, api.client_package_name
Expand Down Expand Up @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same -- would love to see the change in output for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I included the output from ls-apis check in the PR description here. It's a bit easy to miss the relevant bit in that output (the API with multiple producers), so I'll extract just that bit:

Server-managed APIs:

     ...
    Ereporter (ereporter-client, exposed by omicron-gateway, omicron-sled-agent)
     ...

}

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);
}
}
Expand All @@ -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)");
}
Expand Down
1 change: 1 addition & 0 deletions dev-tools/ls-apis/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Loading