Use data-bearing enum variants pattern in negotiation (#1727)

During negotiation, there are certain pieces of data which are only
relevant in particular states:

- Before connecting, the client has a `oneshot::Sender<()>` which kicks
off the connection when triggered
- When waiting for a quorum, each client stores a `RegionMetadata`, to
be used when computing reconciliation

Previously, these two pieces of data were always present in the
`DownstairsClient` object, and were checked (or unwrapped) at the
relevant points in negotiation.

This PR moves them into the relevant `NegotiationState` variants, so
they are _guaranteed_ to be present at the right times.

Unfortunately, `NegotiationState` is also used (as a member of
`DsState`) in serialization, for both OpenAPI calls and DTrace probes.
We can't serialize or log a oneshot sender (or region metadata), which
is a problem!

The **vast majority** of this PR is separating the
serialization-friendly subset of data (`DsState` and `NegotiationState`)
from the data-bearing variants (new `DsStateData` and
`NegotiationStateData`).

There's also one subtlety: the PR adds a new
`NegotiationState::WaitConnect(oneshot::Sender<()>)`. Previously,
`NegotiationState::Start` represented both "waiting for connection
oneshot to fire" and "waiting for connection to the Downstairs"
(depending on whether `ClientTaskHandle::client_connect_tx` was present
or not). This change makes those two sub-states distinct.

Author: mkeeter

Cargo.lock

@@ -99,7 +99,7 @@ slog-term = { version = "2.9" }
 static_assertions = "1.1.0"
 statistical = "1.0.0"
 subprocess = "0.2.9"
-strum = "0.27"
+strum = { version = "0.27", features = ["derive"] }
 strum_macros = "0.27"
 tempfile = "3"
 test-strategy = "0.4.1"

Cargo.toml

@@ -86,7 +86,7 @@ enum Action {
     Repair,
 }
 
-// Translate a DsState into a three letter string for printing.
+/// Translate a [`DsState`] into a three letter string for printing.
 fn short_state(dss: DsState) -> String {
     match dss {
         DsState::Connecting {

cmon/src/main.rs

@@ -280,7 +280,7 @@
         ]
       },
       "DsState": {
-        "description": "High-level states for a Downstairs\n\nThe state machine for a Downstairs is relatively simple:\n\n```text ┌────────────┐ ┌────► LiveRepair ├─────┐ start ┌─────────┴┐   └─────┬──────┘   ┌─▼──────┐ ────►│Connecting│         │          │Stopping│ └─▲───────┬┘   ┌─────▼──────┐   └─▲────┬─┘ │       └────►   Active   ├─────┘    │ │            └─────┬──────┘          │ │                  │                 │ └─────────────────◄┴─────────────────┘ ```\n\nComplexity is hidden in the `Connecting` state, which wraps a [`NegotiationState`] implementing the negotiation state machine.",
+        "description": "High-level states for a Downstairs\n\nThe state machine for a Downstairs is relatively simple:\n\n```text ┌────────────┐ ┌────► LiveRepair ├─────┐ start ┌─────────┴┐   └─────┬──────┘   ┌─▼──────┐ ────►│Connecting│         │          │Stopping│ └─▲───────┬┘   ┌─────▼──────┐   └─▲────┬─┘ │       └────►   Active   ├─────┘    │ │            └─────┬──────┘          │ │                  │                 │ └─────────────────◄┴─────────────────┘ ```\n\nComplexity is hidden in the `Connecting` state, which wraps a [`NegotiationStateData`] implementing the negotiation state machine.\n\nThis is a *generic* downstairs state; it's specialized to either [`DsStateData`] (which is data-bearing) or [`DsState`] (which is serializable).  The `#[derive(..)]` annotations for serialization only apply if the generic parameter `N` is serializable, so it only applies to `DsState`.",
         "oneOf": [
           {
             "description": "New connection",
@@ -384,8 +384,23 @@
         ]
       },
       "NegotiationState": {
-        "description": "Tracks client negotiation progress\n\nThe exact path through negotiation depends on the [`ConnectionMode`].\n\nThere are three main paths, shown below:\n\n```text ┌───────┐ │ Start │ └───┬───┘ │ ┌───────▼────────┐ │ WaitForPromote │ └───────┬────────┘ │ ┌────────▼──────────┐ │ WaitForRegionInfo │ └──┬──────────────┬─┘ Offline │              │ New / Faulted / Replaced (replay) │         ┌────▼────────────┐ │         │GetExtentVersions│ │         └─┬─────────────┬─┘ │           │ New         │ Faulted / Replaced │    ┌──────▼───┐    ┌────▼──────────┐ │    │WaitQuorum│    │LiveRepairReady│ │    └────┬─────┘    └────┬──────────┘ │         │               │ │    ┌────▼────┐          │ │    │Reconcile│          │ │    └────┬────┘          │ │         │               │ │     ┌───▼──┐            │ └─────► Done ◄────────────┘ └──────┘ ```\n\n`Done` isn't actually present in the state machine; it's indicated by returning a [`NegotiationResult`] other than [`NegotiationResult::NotDone`].",
+        "description": "Auto-generated discriminant enum variants",
         "oneOf": [
+          {
+            "description": "One-shot sender to ask the client to open its connection\n\nThis is used to hold the client (without connecting) in cases where we have deliberately deactivated this client.",
+            "type": "object",
+            "properties": {
+              "type": {
+                "type": "string",
+                "enum": [
+                  "wait_connect"
+                ]
+              }
+            },
+            "required": [
+              "type"
+            ]
+          },
           {
             "description": "After connecting, waiting to hear `YesItsMe` from the client\n\nOnce this message is heard, sends `PromoteToActive` and transitions to `WaitForPromote`",
             "type": "object",

openapi/crucible-control.json

@@ -43,13 +43,14 @@ schemars.workspace = true
 semver.workspace = true
 serde.workspace = true
 serde_json.workspace = true
-slog.workspace = true
 slog-async.workspace = true
 slog-dtrace.workspace = true
 slog-term.workspace = true
-tokio.workspace = true
-tokio-util.workspace = true
+slog.workspace = true
+strum.workspace = true
 tokio-rustls.workspace = true
+tokio-util.workspace = true
+tokio.workspace = true
 toml.workspace = true
 tracing.workspace = true
 usdt.workspace = true