rebase

Created using spr 1.3.6-beta.1

.config/nextest.toml

@@ -3,7 +3,7 @@
 #
 # The required version should be bumped up if we need new features, performance
 # improvements or bugfixes that are present in newer versions of nextest.
-nextest-version = { required = "0.9.77", recommended = "0.9.78" }
+nextest-version = { required = "0.9.77", recommended = "0.9.86" }
 
 experimental = ["setup-scripts"]
 

.github/buildomat/build-and-test.sh

@@ -18,7 +18,7 @@ target_os=$1
 # NOTE: This version should be in sync with the recommended version in
 # .config/nextest.toml. (Maybe build an automated way to pull the recommended
 # version in the future.)
-NEXTEST_VERSION='0.9.78'
+NEXTEST_VERSION='0.9.86'
 
 cargo --version
 rustc --version
@@ -69,7 +69,8 @@ banner ls-apis
     source ./tools/include/force-git-over-https.sh;
     ptime -m cargo xtask ls-apis apis &&
         ptime -m cargo xtask ls-apis deployment-units &&
-        ptime -m cargo xtask ls-apis servers
+        ptime -m cargo xtask ls-apis servers &&
+        ptime -m cargo xtask ls-apis check
 )
 
 #

Cargo.toml

@@ -363,7 +363,7 @@ derive_more = "0.99.18"
 derive-where = "1.2.7"
 # Having the i-implement-... feature here makes diesel go away from the workspace-hack
 diesel = { version = "2.2.4", features = ["i-implement-a-third-party-backend-and-opt-into-breaking-changes", "postgres", "r2d2", "chrono", "serde_json", "network-address", "uuid"] }
-diesel-dtrace = "0.4.0"
+diesel-dtrace = "0.4.2"
 dns-server = { path = "dns-server" }
 dns-server-api = { path = "dns-server-api" }
 dns-service-client = { path = "clients/dns-service-client" }

clippy.toml

@@ -15,5 +15,5 @@ disallowed-methods = [
     # and can fail spuriously.
     # Instead, the "transaction_retry_wrapper" should be preferred, as it
     # automatically retries transactions experiencing contention.
-    { path = "async_bb8_diesel::AsyncConnection::transaction_async", reason = "Prefer to use transaction_retry_wrapper, if possible. Feel free to override this for tests and nested transactions." },
+    { path = "async_bb8_diesel::AsyncConnection::transaction_async", reason = "Prefer to use transaction_retry_wrapper, if possible. For tests and nested transactions, use transaction_non_retry_wrapper to at least get dtrace probes" },
 ]

dev-tools/ls-apis/README.adoc

@@ -104,6 +104,30 @@ You can generate a PNG image of the graph like this:
 $ dot -T png -o deployment-units.png deployment-units.dot
 ```
 
+=== Checking the upgrade DAG
+
+As of this writing:
+
+* All Dropshot/Progenitor APIs in the system are identified with this tool (as far as we know).
+* Every API is annotated with metadata in `api-manifest.toml`.
+* The API metadata specifies whether versioning for the API is managed on the server side exclusively or using client-side versioning as well.  We've made this choice for every existing API.
+* There are no cycles in the server-side-managed-API dependency graph.
+
+This tool verifies these properties.  If you add a new API or a new API dependency without adding valid metadata, or if that metadata breaks these constraints, you will have to fix this up before you can land the change to Omicron.
+
+You can verify the versioning metadata using:
+
+```
+$ cargo xtask ls-apis check
+```
+
+You might see any of the following errors:
+
+* `+missing field `versioned_how`+`: this probably means that you added an API to the metadata but neglected to specify the `versioned_how` field.  This must be either `client` or `server`.  In general, prefer `server`.  The tool should tell you if you try to use `server` but that can't work.  If in doubt, ask the update team.
+* `at least one API has unknown version strategy (see above)`: this probably means you added an API with `version_how = "unknown"`.  (The specific API(s) with this problem are listed immediately above this error.)  You need to instead decide if it should be client-side or server-side versioned.  If in doubt, ask the update team.
+* `+missing field `versioned_how_reason`+`: this probably means that you added an API with `versioned_how = "client"`, but did not add a `versioned_how_reason`.  This field is required.  It's a free-text field that explains why we couldn't use server-side versioning for this API.
+* `graph of server-managed components has a cycle (includes node: "omicron-nexus")`: this probably means that you added an API with `versioned_how = "server"`, but that created a circular dependency with other server-side-versioned components.  You'll need to break the cycle by changing one of these components (probably your new one) to be client-managed instead.
+* `API identified by client package "nexus-client" (Nexus Internal API) is the "client" in a "non-dag" dependency rule, but its "versioned_how" is not "client"` (the specific APIs may be different): this (unlikely) condition means just what it says: the API manifest file contains a dependency rule saying that some API is *not* part of the update DAG, but that API is not marked accordingly.  See the documentation below on filter rules.
 
 == Details
 

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

@@ -132,6 +132,28 @@ packages = [ "oximeter-collector" ]
 #
 # `server_package_name`: the package that contains the Dropshot API definition.
 #
+# `versioned_how`: one of:
+#
+#   "server":  this component uses server-side versioning only, meaning that the
+#              update system can ensure that servers are always updated before
+#              clients.
+#
+#   "client":  this component uses client-side versioning, meaning that the
+#              update system cannot ensure that servers are always updated
+#              before clients, so clients need to deal with multiple different
+#              server versions.
+#
+#              You must also specify `versioned_how_reason` if you use "client".
+#
+#   "unknown": this has not been determined.  This will break tests.  But the
+#              tooling supports this value so that during development you can
+#              relax these constraints.
+#
+# [`versioned_how_reason`]: free text string explaining why `versioned_how` must
+# be "client" for this API.  This is printed in documentation and command output
+# and serves as documentation for developers to understand why we made this
+# choice.  This should be present only if `versioned_how = "client"`.
+#
 # [`notes`]: optional free-form human-readable summary documentation about this
 # API
 ################################################################################
@@ -140,11 +162,14 @@ packages = [ "oximeter-collector" ]
 client_package_name = "bootstrap-agent-client"
 label = "Bootstrap Agent"
 server_package_name = "bootstrap-agent-api"
+versioned_how = "client"
+versioned_how_reason = "depends on itself (i.e., instances call each other)"
 
 [[apis]]
 client_package_name = "clickhouse-admin-keeper-client"
 label = "Clickhouse Cluster Admin for Keepers"
 server_package_name = "clickhouse-admin-api"
+versioned_how = "server"
 notes = """
 This is the server running inside multi-node Clickhouse keeper zones that's \
 responsible for local configuration and monitoring.
@@ -154,6 +179,7 @@ responsible for local configuration and monitoring.
 client_package_name = "clickhouse-admin-server-client"
 label = "Clickhouse Cluster Admin for Servers"
 server_package_name = "clickhouse-admin-api"
+versioned_how = "server"
 notes = """
 This is the server running inside multi-node Clickhouse server zones that's \
 responsible for local configuration and monitoring.
@@ -163,6 +189,7 @@ responsible for local configuration and monitoring.
 client_package_name = "clickhouse-admin-single-client"
 label = "Clickhouse Single-Node Cluster Admin"
 server_package_name = "clickhouse-admin-api"
+versioned_how = "server"
 notes = """
 This is the server running inside single-node Clickhouse server zones that's \
 responsible for local configuration and monitoring.
@@ -172,6 +199,7 @@ responsible for local configuration and monitoring.
 client_package_name = "cockroach-admin-client"
 label = "CockroachDB Cluster Admin"
 server_package_name = "cockroach-admin-api"
+versioned_how = "server"
 notes = """
 This is the server running inside CockroachDB zones that performs \
 configuration and monitoring that requires the `cockroach` CLI.
@@ -181,11 +209,14 @@ configuration and monitoring that requires the `cockroach` CLI.
 client_package_name = "crucible-agent-client"
 label = "Crucible Agent"
 server_package_name = "crucible-agent"
+versioned_how = "server"
 
 [[apis]]
 client_package_name = "repair-client"
 label = "Crucible Repair"
 server_package_name = "crucible-downstairs"
+versioned_how = "client"
+versioned_how_reason = "depends on itself (i.e., instances call each other)"
 notes = """
 The repair service offered by a crucible-downstairs supports both repairing \
 one downstairs from another, and making a clone of a read-only downstairs \
@@ -196,11 +227,13 @@ when creating a new region in the crucible agent.
 client_package_name = "crucible-pantry-client"
 label = "Crucible Pantry"
 server_package_name = "crucible-pantry"
+versioned_how = "server"
 
 [[apis]]
 client_package_name = "ddm-admin-client"
 label = "Maghemite DDM Admin"
 server_package_name = "ddmd"
+versioned_how = "server"
 notes = """
 The `ddmd` server runs in each sled GZ and each switch zone. These daemons \
 provide an interface for advertising network prefixes, and observing what \
@@ -216,11 +249,17 @@ observability APIs in the future.
 client_package_name = "dns-service-client"
 label = "DNS Server"
 server_package_name = "dns-server-api"
+versioned_how = "server"
 
 [[apis]]
 client_package_name = "dpd-client"
 label = "Dendrite DPD"
 server_package_name = "dpd"
+# TODO We might need to pick this apart more carefully.  DPD invokes and is
+# invoked by several different services.  It's possible that some of them can
+# treat it as server-managed.
+versioned_how = "client"
+versioned_how_reason = "Sled Agent calling DPD creates a circular dependency"
 notes = """
 Dendrite's data plane daemon (`dpd`) is the interface to configure and manage \
 the rack switches. It's consumed by sled-agent to get the rack off the \
@@ -235,17 +274,29 @@ repo is not currently open source.
 client_package_name = "gateway-client"
 label = "Management Gateway Service"
 server_package_name = "gateway-api"
+versioned_how = "server"
 notes = "Wicketd is deployed in a unit with MGS so we can ignore that one."
 
 [[apis]]
 client_package_name = "installinator-client"
 label = "Wicketd Installinator"
 server_package_name = "installinator-api"
+# The installinator-client is used only by Installinator.  This is part of the
+# recovery OS image.  Today, this is not really "shipped" in the traditional
+# sense.  The only way it gets used today is by an operator uploading it to
+# Wicket and then performing a MUPdate.  In this sense, we do not control the
+# client, so we mark this client-managed -- it's up to the operator to make sure
+# they're using a TUF repo whose recovery image contains an Installinator that's
+# compatible with the API served by Wicket on the deployed system.  In practice,
+# we're almost certainly just going to avoid changing this API.
+versioned_how = "client"
+versioned_how_reason = "client is provided implicitly by the operator"
 
 [[apis]]
 client_package_name = "mg-admin-client"
 label = "Maghemite MG Admin"
 server_package_name = "mgd"
+versioned_how = "server"
 notes = """
 The `mgd` daemon runs in each switch zone. This daemon is responsible for all \
 external route management for a switch. It provides interfaces for static \
@@ -258,17 +309,25 @@ bring the rack up.
 client_package_name = "nexus-client"
 label = "Nexus Internal API"
 server_package_name = "nexus-internal-api"
+# nexus-client has to be client-versioned today because it's got a cyclic
+# dependency with sled-agent-client, which is server-versioned.
+versioned_how = "client"
+versioned_how_reason = "Circular dependencies between Nexus and other services"
 
 [[apis]]
 client_package_name = "oxide-client"
 label = "External API"
 server_package_name = "nexus-external-api"
+# The versioning strategy for the external API is outside the scope of this
+# tool.  It doesn't matter what we put here.
+versioned_how = "server"
 notes = "Special case, since we don't fully control all clients"
 
 [[apis]]
 client_package_name = "oximeter-client"
 label = "Oximeter"
 server_package_name = "oximeter-api"
+versioned_how = "server"
 notes = """
 Shared types for this interface are in `omicron-common`. The producer makes \
 requests to Nexus, and receives them from `oximeter-collector`. \
@@ -281,6 +340,7 @@ and makes the periodic renewal requests to `oximeter-collector`.
 client_package_name = "propolis-client"
 label = "Propolis"
 server_package_name = "propolis-server"
+versioned_how = "server"
 notes = """
 Sled Agent is deployed in a unit with Propolis so we can ignore that one.
 """
@@ -289,16 +349,20 @@ Sled Agent is deployed in a unit with Propolis so we can ignore that one.
 client_package_name = "sled-agent-client"
 label = "Sled Agent"
 server_package_name = "sled-agent-api"
+versioned_how = "server"
 
 [[apis]]
 client_package_name = "repo-depot-client"
 label = "Repo Depot API"
 server_package_name = "repo-depot-api"
+versioned_how = "client"
+versioned_how_reason = "depends on itself (i.e., instances call each other)"
 
 [[apis]]
 client_package_name = "wicketd-client"
 label = "Wicketd"
 server_package_name = "wicketd-api"
+versioned_how = "server"
 notes = """
 wicketd-client is only used by wicket, which is deployed in a unit with wicketd.
 """
@@ -307,6 +371,7 @@ wicketd-client is only used by wicket, which is deployed in a unit with wicketd.
 client_package_name = "crucible-control-client"
 label = "Crucible Control (for testing only)"
 server_package_name = "crucible"
+versioned_how = "server"
 notes = """
 Exposed by Crucible upstairs for debugging via the `cmon` debugging tool.
 """
@@ -315,6 +380,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"
+versioned_how = "server"
 dev_only = true
 notes = """
 `dsc` is a control program for spinning up and controlling instances of Crucible
@@ -400,6 +466,16 @@ DNS server depends on itself only to provide TransientServer, which is not a
 deployed component.
 """
 
+[[dependency_filter_rules]]
+ancestor = "omicron-sled-agent"
+client = "sled-agent-client"
+evaluation = "not-deployed"
+note = """
+Sled Agent uses a Sled Agent client in two ways: RSS, which we don't care about
+for the purpose of upgrade, and in the `zone-bundle` binary, which is not
+deployed.
+"""
+
 #
 # "Bogus" dependencies.  See above for details.
 #