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

ls-apis apis should show all dep paths and include Crucible's notify-nexus feature #6812

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

davepacheco
Copy link
Collaborator

This PR makes three logical changes:

  • Changes the ls-apis apis --show-deps subcommand to show all Rust dependency paths that led it to infer an API dependency, instead of just an arbitrary one. This is needed to show that ls-apis cannot see dependency from Propolis/Crucible upstairs to Nexus internal API #6811 exists before this change (because the expected path is missing) and is fixed after this change (because the expected path is present). Prior to fixing ls-apis cannot see dependency from Propolis/Crucible upstairs to Nexus internal API #6811, the output was correct anyway (for the wrong reasons) unless you looked at this level of detail. We could have the other subcommands print this detail but I didn't need it for this.
  • Fixes ls-apis cannot see dependency from Propolis/Crucible upstairs to Nexus internal API #6811, primarily by using --features omicron-build when loading the Propolis workspace metadata with cargo workspace.
  • Fixes Rust dependency traversal to not fully skip a package we've seen before. Previously, when traversing the package dependency tree, we'd immediately check if we'd seen this pkgid before and skip it if so. The point of this was to avoid re-traversing all of its dependencies again for no reason. But it had the extra effect of also preventing the caller from seeing a second path to the same transitive dependency. I changed this code to invoke the caller's callback (so that it sees this second path), but we still don't re-traverse the package's dependencies if we'd seen it before. All of this was necessary because even after doing the first thing above, I still didn't see the new path because of this issue.

Fixes #6811.

@davepacheco
Copy link
Collaborator Author

davepacheco commented Oct 9, 2024

I did this testing from a version of this work based on main @ 469522d.

I tested this by running:

cargo xtask ls-apis apis --show-deps
cargo xtask ls-apis apis --show-deps --filter all

on a few different versions of the code and seeing how those changed.

Between "main" and the first commit (which only changes the apis command output), almost every line changed but only in the expected way: it now prints out the number of paths, like this:

2,5c2,5
<     consumed by: omicron-sled-agent (omicron/sled-agent)
<         via path+file:///home/dap/omicron-merge/sled-agent#[email protected]
<     consumed by: wicketd (omicron/wicketd)
<         via path+file:///home/dap/omicron-merge/wicketd#0.1.0
---
>     consumed by: omicron-sled-agent (omicron/sled-agent) via paths: 1
>         via: path+file:///home/dap/omicron-merge/sled-agent#[email protected]
>     consumed by: wicketd (omicron/wicketd) via paths: 1
>         via: path+file:///home/dap/omicron-merge/wicketd#0.1.0

Between the first and second commit is where the interesting changes are. That's where the program now finds a whole bunch of new paths to things, including the one that we care about. Using the default output, I used diff -U30 to get the whole file here:

@@ -1,115 +1,158 @@
 Bootstrap Agent (client: bootstrap-agent-client)
     consumed by: omicron-sled-agent (omicron/sled-agent) via paths: 1
         via: path+file:///home/dap/omicron-merge/sled-agent#[email protected]
-    consumed by: wicketd (omicron/wicketd) via paths: 1
-        via: path+file:///home/dap/omicron-merge/wicketd#0.1.0
+    consumed by: wicketd (omicron/wicketd) via paths: 2
+        via path 1: path+file:///home/dap/omicron-merge/wicketd#0.1.0
+        via path 2: path+file:///home/dap/omicron-merge/wicketd-api#0.1.0
+        via path 2: path+file:///home/dap/omicron-merge/wicketd#0.1.0
 
 CockroachDB Cluster Admin (client: cockroach-admin-client)
-    consumed by: omicron-nexus (omicron/nexus) via paths: 1
-        via: path+file:///home/dap/omicron-merge/nexus#[email protected]
+    consumed by: omicron-nexus (omicron/nexus) via paths: 2
+        via path 1: path+file:///home/dap/omicron-merge/nexus#[email protected]
+        via path 2: path+file:///home/dap/omicron-merge/nexus/reconfigurator/execution#[email protected]
+        via path 2: path+file:///home/dap/omicron-merge/nexus#[email protected]
 
 Crucible Agent (client: crucible-agent-client)
     consumed by: omicron-nexus (omicron/nexus) via paths: 1
         via: path+file:///home/dap/omicron-merge/nexus#[email protected]
 
 Crucible Control (for testing only) (client: crucible-control-client)
 
 Crucible Pantry (client: crucible-pantry-client)
     consumed by: omicron-nexus (omicron/nexus) via paths: 1
         via: path+file:///home/dap/omicron-merge/nexus#[email protected]
 
 Maghemite DDM Admin (client: ddm-admin-client)
     consumed by: installinator (omicron/installinator) via paths: 1
         via: path+file:///home/dap/omicron-merge/clients/ddm-admin-client#[email protected]
         via: path+file:///home/dap/omicron-merge/installinator#0.1.0
     consumed by: mgd (maghemite/mgd) via paths: 1
         via: path+file:///home/dap/.cargo/git/checkouts/maghemite-de41bdd6c14939ab/b13b5b2/mg-lower#0.1.0
         via: path+file:///home/dap/.cargo/git/checkouts/maghemite-de41bdd6c14939ab/b13b5b2/mgd#0.1.0
     consumed by: omicron-sled-agent (omicron/sled-agent) via paths: 1
         via: path+file:///home/dap/omicron-merge/clients/ddm-admin-client#[email protected]
         via: path+file:///home/dap/omicron-merge/sled-agent#[email protected]
     consumed by: wicketd (omicron/wicketd) via paths: 1
         via: path+file:///home/dap/omicron-merge/clients/ddm-admin-client#[email protected]
         via: path+file:///home/dap/omicron-merge/wicketd#0.1.0
 
 DNS Server (client: dns-service-client)
-    consumed by: omicron-nexus (omicron/nexus) via paths: 1
-        via: path+file:///home/dap/omicron-merge/nexus#[email protected]
+    consumed by: omicron-nexus (omicron/nexus) via paths: 2
+        via path 1: path+file:///home/dap/omicron-merge/nexus#[email protected]
+        via path 2: path+file:///home/dap/omicron-merge/nexus/reconfigurator/execution#[email protected]
+        via path 2: path+file:///home/dap/omicron-merge/nexus#[email protected]
     consumed by: omicron-sled-agent (omicron/sled-agent) via paths: 1
         via: path+file:///home/dap/omicron-merge/sled-agent#[email protected]
 
 Dendrite DPD (client: dpd-client)
-    consumed by: ddmd (maghemite/ddmd) via paths: 1
-        via: path+file:///home/dap/.cargo/git/checkouts/maghemite-de41bdd6c14939ab/b13b5b2/ddmd#0.1.0
+    consumed by: ddmd (maghemite/ddmd) via paths: 2
+        via path 1: path+file:///home/dap/.cargo/git/checkouts/maghemite-de41bdd6c14939ab/b13b5b2/ddmd#0.1.0
+        via path 2: path+file:///home/dap/.cargo/git/checkouts/maghemite-de41bdd6c14939ab/b13b5b2/ddm#0.1.0
+        via path 2: path+file:///home/dap/.cargo/git/checkouts/maghemite-de41bdd6c14939ab/b13b5b2/ddmd#0.1.0
     consumed by: mgd (maghemite/mgd) via paths: 1
         via: path+file:///home/dap/.cargo/git/checkouts/maghemite-de41bdd6c14939ab/b13b5b2/mg-lower#0.1.0
         via: path+file:///home/dap/.cargo/git/checkouts/maghemite-de41bdd6c14939ab/b13b5b2/mgd#0.1.0
     consumed by: omicron-nexus (omicron/nexus) via paths: 1
         via: path+file:///home/dap/omicron-merge/nexus#[email protected]
     consumed by: omicron-sled-agent (omicron/sled-agent) via paths: 1
         via: path+file:///home/dap/omicron-merge/sled-agent#[email protected]
     consumed by: tfportd (dendrite/tfportd) via paths: 1
         via: path+file:///home/dap/.cargo/git/checkouts/dendrite-627b239a911c6241/76c735d/tfportd#0.1.0
-    consumed by: wicketd (omicron/wicketd) via paths: 1
-        via: path+file:///home/dap/omicron-merge/wicketd#0.1.0
+    consumed by: wicketd (omicron/wicketd) via paths: 2
+        via path 1: path+file:///home/dap/omicron-merge/wicketd#0.1.0
+        via path 2: path+file:///home/dap/omicron-merge/wicket-common#0.1.0
+        via path 2: path+file:///home/dap/omicron-merge/wicketd#0.1.0
 
 Downstairs Controller (debugging only) (client: dsc-client)
 
 Management Gateway Service (client: gateway-client)
     consumed by: dpd (dendrite/dpd) via paths: 1
         via: path+file:///home/dap/.cargo/git/checkouts/dendrite-627b239a911c6241/76c735d/dpd#0.2.0
-    consumed by: omicron-nexus (omicron/nexus) via paths: 1
-        via: path+file:///home/dap/omicron-merge/nexus#[email protected]
+    consumed by: omicron-nexus (omicron/nexus) via paths: 3
+        via path 1: path+file:///home/dap/omicron-merge/nexus#[email protected]
+        via path 2: path+file:///home/dap/omicron-merge/nexus/reconfigurator/planning#[email protected]
+        via path 2: path+file:///home/dap/omicron-merge/nexus#[email protected]
+        via path 3: path+file:///home/dap/omicron-merge/nexus/inventory#[email protected]
+        via path 3: path+file:///home/dap/omicron-merge/nexus#[email protected]
     consumed by: omicron-sled-agent (omicron/sled-agent) via paths: 1
         via: path+file:///home/dap/omicron-merge/sled-agent#[email protected]
-    consumed by: wicketd (omicron/wicketd) via paths: 1
-        via: path+file:///home/dap/omicron-merge/wicketd#0.1.0
+    consumed by: wicketd (omicron/wicketd) via paths: 3
+        via path 1: path+file:///home/dap/omicron-merge/wicketd#0.1.0
+        via path 2: path+file:///home/dap/omicron-merge/wicketd-api#0.1.0
+        via path 2: path+file:///home/dap/omicron-merge/wicketd#0.1.0
+        via path 3: path+file:///home/dap/omicron-merge/wicket-common#0.1.0
+        via path 3: path+file:///home/dap/omicron-merge/wicketd#0.1.0
 
 Wicketd Installinator (client: installinator-client)
     consumed by: installinator (omicron/installinator) via paths: 1
         via: path+file:///home/dap/omicron-merge/installinator#0.1.0
 
 Maghemite MG Admin (client: mg-admin-client)
     consumed by: omicron-nexus (omicron/nexus) via paths: 1
         via: path+file:///home/dap/omicron-merge/nexus#[email protected]
     consumed by: omicron-sled-agent (omicron/sled-agent) via paths: 1
         via: path+file:///home/dap/omicron-merge/sled-agent#[email protected]
 
 Nexus Internal API (client: nexus-client)
     consumed by: dpd (dendrite/dpd) via paths: 1
         via: path+file:///home/dap/.cargo/git/checkouts/dendrite-627b239a911c6241/76c735d/dpd#0.2.0
     consumed by: omicron-nexus (omicron/nexus) via paths: 1
         via: path+file:///home/dap/omicron-merge/nexus#[email protected]
-    consumed by: omicron-sled-agent (omicron/sled-agent) via paths: 1
-        via: path+file:///home/dap/omicron-merge/sled-agent#[email protected]
+    consumed by: omicron-sled-agent (omicron/sled-agent) via paths: 2
+        via path 1: path+file:///home/dap/omicron-merge/sled-agent#[email protected]
+        via path 2: path+file:///home/dap/omicron-merge/sled-agent/bootstrap-agent-api#0.1.0
+        via path 2: path+file:///home/dap/omicron-merge/sled-agent#[email protected]
     consumed by: oximeter-collector (omicron/oximeter/collector) via paths: 1
         via: path+file:///home/dap/omicron-merge/oximeter/collector#[email protected]
-    consumed by: propolis-server (propolis/bin/propolis-server) via paths: 1
-        via: path+file:///home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/bin/propolis-server#0.1.0
+    consumed by: propolis-server (propolis/bin/propolis-server) via paths: 3
+        via path 1: path+file:///home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/bin/propolis-server#0.1.0
+        via path 2: path+file:///home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/lib/propolis#0.1.0
+        via path 2: path+file:///home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/bin/propolis-server#0.1.0
+        via path 3: git+https://github.com/oxidecomputer/crucible?rev=2b88ab88461fb06aaf2aab11c5e381a3cad25eac#0.0.1
+        via path 3: path+file:///home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/lib/propolis#0.1.0
+        via path 3: path+file:///home/dap/.cargo/git/checkouts/propolis-12517f89d3d9f483/fae5334/bin/propolis-server#0.1.0
 
 External API (client: oxide-client)
 
 Oximeter (client: oximeter-client)
-    consumed by: omicron-nexus (omicron/nexus) via paths: 1
-        via: path+file:///home/dap/omicron-merge/nexus#[email protected]
+    consumed by: omicron-nexus (omicron/nexus) via paths: 2
+        via path 1: path+file:///home/dap/omicron-merge/nexus#[email protected]
+        via path 2: path+file:///home/dap/omicron-merge/nexus/metrics-producer-gc#[email protected]
+        via path 2: path+file:///home/dap/omicron-merge/nexus#[email protected]
 
 Propolis (client: propolis-client)
-    consumed by: omicron-nexus (omicron/nexus) via paths: 1
-        via: path+file:///home/dap/omicron-merge/nexus#[email protected]
+    consumed by: omicron-nexus (omicron/nexus) via paths: 2
+        via path 1: path+file:///home/dap/omicron-merge/nexus#[email protected]
+        via path 2: git+https://github.com/oxidecomputer/falcon?branch=main#[email protected]
+        via path 2: path+file:///home/dap/omicron-merge/test-utils#[email protected]
+        via path 2: path+file:///home/dap/omicron-merge/nexus/db-queries#[email protected]
+        via path 2: path+file:///home/dap/omicron-merge/nexus#[email protected]
     consumed by: omicron-sled-agent (omicron/sled-agent) via paths: 1
         via: path+file:///home/dap/omicron-merge/sled-agent#[email protected]
 
 Crucible Repair (client: repair-client)
     consumed by: crucible-downstairs (crucible/downstairs) via paths: 1
         via: path+file:///home/dap/.cargo/git/checkouts/crucible-f3b5bdecdc6486d6/2b88ab8/downstairs#[email protected]
 
 Sled Agent (client: sled-agent-client)
     consumed by: dpd (dendrite/dpd) via paths: 1
         via: path+file:///home/dap/.cargo/git/checkouts/dendrite-627b239a911c6241/76c735d/dpd#0.2.0
-    consumed by: omicron-nexus (omicron/nexus) via paths: 1
-        via: path+file:///home/dap/omicron-merge/nexus#[email protected]
+    consumed by: omicron-nexus (omicron/nexus) via paths: 7
+        via path 1: path+file:///home/dap/omicron-merge/nexus#[email protected]
+        via path 2: path+file:///home/dap/omicron-merge/nexus/reconfigurator/planning#[email protected]
+        via path 2: path+file:///home/dap/omicron-merge/nexus#[email protected]
+        via path 3: path+file:///home/dap/omicron-merge/nexus/reconfigurator/execution#[email protected]
+        via path 3: path+file:///home/dap/omicron-merge/nexus#[email protected]
+        via path 4: path+file:///home/dap/omicron-merge/nexus/networking#[email protected]
+        via path 4: path+file:///home/dap/omicron-merge/nexus#[email protected]
+        via path 5: path+file:///home/dap/omicron-merge/nexus/inventory#[email protected]
+        via path 5: path+file:///home/dap/omicron-merge/nexus#[email protected]
+        via path 6: path+file:///home/dap/omicron-merge/nexus/db-queries#[email protected]
+        via path 6: path+file:///home/dap/omicron-merge/nexus#[email protected]
+        via path 7: path+file:///home/dap/omicron-merge/nexus/db-model#[email protected]
+        via path 7: path+file:///home/dap/omicron-merge/nexus#[email protected]
     consumed by: omicron-sled-agent (omicron/sled-agent) via paths: 1
         via: path+file:///home/dap/omicron-merge/sled-agent#[email protected]
 
 Wicketd (client: wicketd-client)
 

I didn't verify all these paths but I spot-checked them and they seemed plausible. Importantly, there are no new API dependencies identified nor any that are missing. And the path we were looking for is present (under "Nexus Internal API", consumed by propolis-server, via crucible).

@davepacheco
Copy link
Collaborator Author

I updated this to "main" but I think pulled in some breakage -- maybe that's what #6834 is fixing?

In terms of (re-)testing:

  • The new expectorate test shows that the set of dependencies hasn't changed (though the output has because it now prints the number of paths).
  • I've manually confirmed that the same output for --filter all also doesn't find any new or removed deps. Only the output format has changed to show the number of paths.

Copy link
Contributor

@sunshowers sunshowers left a comment

Choose a reason for hiding this comment

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

Looks great! Sorry about the disruption re internal-dns :)

@davepacheco davepacheco enabled auto-merge (squash) October 11, 2024 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ls-apis cannot see dependency from Propolis/Crucible upstairs to Nexus internal API
2 participants