[2/n] [reconfigurator] test that Reconfigurator updates are in server-side topological order

Cargo.toml

@@ -45,6 +45,7 @@ members = [
     "dev-tools/omdb",
     "dev-tools/omicron-dev",
     "dev-tools/omicron-dev-lib",
+    "dev-tools/omicron-deployment-graph",
     "dev-tools/oxlog",
     "dev-tools/pins",
     "dev-tools/reconfigurator-cli",
@@ -226,6 +227,7 @@ default-members = [
     "dev-tools/omdb",
     "dev-tools/omicron-dev",
     "dev-tools/omicron-dev-lib",
+    "dev-tools/omicron-deployment-graph",
     "dev-tools/oxlog",
     "dev-tools/pins",
     "dev-tools/reconfigurator-cli",
@@ -590,6 +592,7 @@ linear-map = "1.2.0"
 live-tests-macros = { path = "live-tests/macros" }
 lldpd_client = { git = "https://github.com/oxidecomputer/lldp", package = "lldpd-client" }
 lldp_protocol = { git = "https://github.com/oxidecomputer/lldp", package = "protocol" }
+omicron-deployment-graph = { path = "dev-tools/omicron-deployment-graph" }
 macaddr = { version = "1.0.1", features = ["serde_std"] }
 maplit = "1.0.2"
 newtype_derive = "0.1.6"

dev-tools/dropshot-apis/src/main.rs

@@ -268,7 +268,11 @@ fn all_apis() -> anyhow::Result<ManagedApis> {
             },
             api_description: ntp_admin_api_mod::stub_api_description,
             ident: "ntp-admin",
-        }),
+        })
+        // The NTP Admin API is client-side-versioned and currently frozen. We
+        // allow trivial changes to go through. If we did not, we would need to
+        // unfreeze the API and bump the version number for trivial changes.
+        .allow_trivial_changes_for_latest(),
         ManagedApi::from(ManagedApiConfig {
             title: "Oxide Oximeter API",
             versions: Versions::new_versioned(

dev-tools/ls-apis/Cargo.toml

@@ -15,6 +15,7 @@ clap.workspace = true
 iddqd.workspace = true
 indent_write.workspace = true
 itertools.workspace = true
+omicron-deployment-graph.workspace = true
 newtype_derive.workspace = true
 parse-display.workspace = true
 petgraph.workspace = true

dev-tools/ls-apis/README.adoc

@@ -208,6 +208,9 @@ Filter rules always represent a determination that a human has made about one or
 |`non-dag`
 |Any matching dependency has been flagged as "will not be part of the DAG used for online upgrade".  This is primarily to help us keep track of the specific dependencies that we've looked at and made this determination for.  These are currently ignored by default.
 
+|`rss-only`
+|Any matching dependency is only used during RSS, not during normal operation or upgrade.  Like `non-dag`, these are excluded from the default view.  Unlike `non-dag`, this does not require the API to use client-side versioning.
+
 |`dag`
 |Any matching dependency has been flagged as "we want this to be part of the DAG used for online upgrade".
 
@@ -218,17 +221,17 @@ In summary:
 * All dependencies start as `unknown`.
 * All the known false positives have been flagged as `bogus`.
 * All the known dependencies from non-deployed programs inside deployed packages have been flagged as `not-deployed`.
-* What remains is to evaluate the rest of the edges and determine if they're going to be `dag` or `non-dag`.
+* What remains is to evaluate the rest of the edges and determine if they're going to be `dag`, `non-dag`, or `rss-only`.
 
 It is a runtime error for two filter rules to match any dependency chain.  This makes the evaluation unambiguous.  i.e., you can't have one rule match a dependency chain and say it's `bogus` while another says it's `dag`.
 
 ==== Applying different filters at runtime
 
-By default, this command shows dependencies that might be in the final graph.  This includes those labeled `dag` and `unknown`.  It excludes `bogus`, `non-dag`, and `not-deployed` dependencies.
+By default, this command shows dependencies that might be in the final graph.  This includes those labeled `dag` and `unknown`.  It excludes `bogus`, `non-dag`, `rss-only`, and `not-deployed` dependencies.
 
 You can select different subsets using the `--filter` option, which accepts:
 
-* `include-non-dag`: show non-`bogus`, non-`not-deployed` dependencies (i.e., all dependencies that do exist in the deployed system).
+* `include-non-dag`: show non-`bogus`, non-`not-deployed` dependencies (i.e., all dependencies that do exist in the deployed system).  This includes `non-dag` and `rss-only` dependencies.
 * `non-bogus`: show everything _except_ bogus dependencies
 * `bogus`: show only the bogus dependencies (useful for seeing all the false positives)
 * `all`: show everything, even bogus dependencies

dev-tools/ls-apis/api-manifest.toml

@@ -67,7 +67,7 @@ ignored_non_clients = [
 # The host OS includes Sled Agent, Propolis, and all the components that get
 # bundled into the switch zone.
 [[deployment_units]]
-label = "Host OS"
+name = "Host OS"
 packages = [
     "omicron-sled-agent",
     "propolis-server",
@@ -83,44 +83,49 @@ packages = [
 
 # Installinator gets packaged into its own host OS image.
 [[deployment_units]]
-label = "Installinator"
+name = "Installinator"
 packages = [ "installinator" ]
 
 # The rest of these get bundled by standard control plane zone images.
 
 [[deployment_units]]
-label = "Crucible"
+name = "Crucible"
 packages = [ "crucible-agent", "crucible-downstairs" ]
 
 [[deployment_units]]
-label = "Crucible Pantry"
+name = "Crucible Pantry"
 packages = [ "crucible-pantry" ]
 
 [[deployment_units]]
-label = "Cockroach"
+name = "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.
+# This is really three distinct deployment units:
+#
+# * single-node ClickHouse
+# * multi-node ClickHouse Server
+# * multi-node ClickHouse Keeper
+#
+# 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 (single-node) / Clickhouse Server (multi-node) / Clickhouse Keeper (multi-node)"
+name = "ClickHouse"
 packages = [ "omicron-clickhouse-admin" ]
 
 [[deployment_units]]
-label = "DNS Server"
+name = "DNS Server"
 packages = [ "dns-server" ]
 
 [[deployment_units]]
-label = "Nexus"
+name = "Nexus"
 packages = [ "omicron-nexus" ]
 
 [[deployment_units]]
-label = "NTP"
+name = "NTP"
 packages = [ "omicron-ntp-admin" ]
 
 [[deployment_units]]
-label = "Oximeter"
+name = "Oximeter"
 packages = [ "oximeter-collector" ]
 
 
@@ -239,7 +244,11 @@ configuration and monitoring that requires the `cockroach` CLI.
 client_package_name = "ntp-admin-client"
 label = "NTP Admin"
 server_package_name = "ntp-admin-api"
-versioned_how = "server"
+versioned_how = "client"
+versioned_how_reason = """
+Sled Agent (within the Host OS deployment unit) calls into NTP, and is updated \
+before NTP. This requires that the NTP Admin API be client-side versioned.
+"""
 notes = """
 This is the server running inside NTP zones that performs \
 monitoring on 'chrony'.
@@ -475,9 +484,34 @@ instance.  It's also used by `crutest` for testing.
 # edges should be in that graph.
 #
 
+#
+# RSS-only dependencies.  These are real dependencies that only exist during
+# RSS, not during normal operation or upgrade.
+#
+# At RSS time, the system is known to be at a single, consistent version.
+#
+[[dependency_filter_rules]]
+ancestor = "omicron-sled-agent"
+client = "cockroach-admin-client"
+evaluation = "rss-only"
+note = """
+Sled Agent only uses the Cockroach Admin client during RSS, which we don't care
+about for the purpose of upgrade.
+"""
+
+[[dependency_filter_rules]]
+ancestor = "omicron-sled-agent"
+client = "dns-service-client"
+evaluation = "rss-only"
+note = """
+Sled Agent only uses the DNS service client during RSS, which we don't care
+about for the purpose of upgrade.
+"""
+
 #
 # Non-DAG dependencies.  See above for details.
 #
+
 [[dependency_filter_rules]]
 ancestor = "oximeter-producer"
 client = "nexus-client"

dev-tools/ls-apis/src/api_metadata.rs

@@ -5,7 +5,6 @@
 //! Developer-maintained API metadata
 
 use crate::ClientPackageName;
-use crate::DeploymentUnitName;
 use crate::ServerComponentName;
 use crate::ServerPackageName;
 use crate::cargo::DepPath;
@@ -14,6 +13,7 @@ use anyhow::{Result, bail};
 use iddqd::IdOrdItem;
 use iddqd::IdOrdMap;
 use iddqd::id_upcast;
+use omicron_deployment_graph::DeploymentUnitName;
 use serde::Deserialize;
 use std::borrow::Borrow;
 use std::collections::BTreeMap;
@@ -27,7 +27,7 @@ use std::collections::BTreeSet;
 #[serde(try_from = "RawApiMetadata")]
 pub struct AllApiMetadata {
     apis: BTreeMap<ClientPackageName, ApiMetadata>,
-    deployment_units: BTreeMap<DeploymentUnitName, DeploymentUnitInfo>,
+    deployment_units: IdOrdMap<DeploymentUnitInfo>,
     dependency_rules: BTreeMap<ClientPackageName, Vec<DependencyFilterRule>>,
     ignored_non_clients: BTreeSet<ClientPackageName>,
     intra_deployment_unit_only_edges: Vec<IntraDeploymentUnitOnlyEdge>,
@@ -42,10 +42,18 @@ impl AllApiMetadata {
     /// Iterate over the deployment units defined in the metadata
     pub fn deployment_units(
         &self,
-    ) -> impl Iterator<Item = (&DeploymentUnitName, &DeploymentUnitInfo)> {
+    ) -> impl Iterator<Item = &DeploymentUnitInfo> {
         self.deployment_units.iter()
     }
 
+    /// Look up a deployment unit's info by its name
+    pub fn deployment_unit_info(
+        &self,
+        name: &DeploymentUnitName,
+    ) -> Option<&DeploymentUnitInfo> {
+        self.deployment_units.get(name)
+    }
+
     /// Iterate over the package names for all the APIs' clients
     pub fn client_pkgnames(&self) -> impl Iterator<Item = &ClientPackageName> {
         self.apis.keys()
@@ -55,7 +63,7 @@ impl AllApiMetadata {
     pub fn server_components(
         &self,
     ) -> impl Iterator<Item = &ServerComponentName> {
-        self.deployment_units.values().flat_map(|d| d.packages.iter())
+        self.deployment_units.iter().flat_map(|d| d.packages.iter())
     }
 
     /// Look up details about an API based on its client package name
@@ -166,14 +174,12 @@ impl TryFrom<RawApiMetadata> for AllApiMetadata {
             }
         }
 
-        let mut deployment_units = BTreeMap::new();
+        let mut deployment_units = IdOrdMap::new();
         for info in raw.deployment_units {
-            if let Some(previous) =
-                deployment_units.insert(info.label.clone(), info)
-            {
+            if let Err(e) = deployment_units.insert_unique(info) {
                 bail!(
-                    "duplicate deployment unit in API metadata: {}",
-                    &previous.label,
+                    "duplicate deployment unit name in API metadata: {}",
+                    e.new_item().name,
                 );
             }
         }
@@ -206,7 +212,7 @@ impl TryFrom<RawApiMetadata> for AllApiMetadata {
         // Validate that IDU-only edges reference only known server components
         // and APIs.
         let known_components: BTreeSet<_> =
-            deployment_units.values().flat_map(|u| u.packages.iter()).collect();
+            deployment_units.iter().flat_map(|u| u.packages.iter()).collect();
         for edge in &raw.intra_deployment_unit_only_edges {
             if !known_components.contains(&edge.server) {
                 bail!(
@@ -411,12 +417,20 @@ pub enum ApiConsumerStatus {
 #[derive(Deserialize)]
 #[serde(deny_unknown_fields)]
 pub struct DeploymentUnitInfo {
-    /// human-readable label, also used as primary key
-    pub label: DeploymentUnitName,
+    /// human-readable name (e.g. "Nexus", "DNS Server")
+    pub name: DeploymentUnitName,
     /// list of Rust packages that are shipped in this unit
     pub packages: Vec<ServerComponentName>,
 }
 
+impl IdOrdItem for DeploymentUnitInfo {
+    type Key<'a> = &'a DeploymentUnitName;
+    fn key(&self) -> Self::Key<'_> {
+        &self.name
+    }
+    id_upcast!();
+}
+
 #[derive(Deserialize)]
 #[serde(deny_unknown_fields)]
 pub struct DependencyFilterRule {
@@ -446,6 +460,11 @@ pub enum Evaluation {
     NotDeployed,
     /// This dependency should not be part of the update DAG
     NonDag,
+    /// This dependency is only used during RSS, not during normal operation or
+    /// upgrade. Like `NonDag`, this means the dependency should not be part of
+    /// the update DAG. Unlike `NonDag`, this does not require that the target
+    /// API uses client-side versioning.
+    RssOnly,
     /// This dependency should be part of the update DAG
     Dag,
 }