From e68910806d6ad4cc9c7edc8fafd457dac1479613 Mon Sep 17 00:00:00 2001
From: Artemis Everfree <artemis@oxidecomputer.com>
Date: Fri, 29 Sep 2023 23:20:11 +0000
Subject: [PATCH] make rack_topology required to save me from myself

---
 package/src/lib.rs    | 20 ++++++++++++++++++--
 package/src/target.rs | 17 ++++++++++-------
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/package/src/lib.rs b/package/src/lib.rs
index 0d6627e714..395f3ed472 100644
--- a/package/src/lib.rs
+++ b/package/src/lib.rs
@@ -50,9 +50,25 @@ pub enum TargetCommand {
         #[clap(
             short,
             long,
-            default_value_if("image", "standard", "single-sled")
+            default_value_if("image", "trampoline", Some("single-sled")),
+
+            // This opt is required, and clap will enforce that even with
+            // `required = false`, since it's not an Option. But the
+            // default_value_if only works if we set `required` to false. It's
+            // jank, but it is what it is.
+            // https://github.com/clap-rs/clap/issues/4086
+            required = false
         )]
-        rack_topology: Option<crate::target::RackTopology>,
+        /// Specify whether nexus will run in a single-sled or multi-sled
+        /// environment.
+        ///
+        /// Set single-sled for dev purposes when you're running a single
+        /// sled-agent. Set multi-sled if you're running with mulitple sleds.
+        /// Currently this only affects the crucible disk allocation strategy-
+        /// VM disks will require 3 distinct sleds with `multi-sled`, which will
+        /// fail in a single-sled environment. `single-sled` relaxes this
+        /// requirement.
+        rack_topology: crate::target::RackTopology,
     },
     /// List all existing targets
     List,
diff --git a/package/src/target.rs b/package/src/target.rs
index 7829928e82..d5d5e92c46 100644
--- a/package/src/target.rs
+++ b/package/src/target.rs
@@ -68,7 +68,7 @@ pub struct KnownTarget {
     image: Image,
     machine: Option<Machine>,
     switch: Option<Switch>,
-    rack_topology: Option<RackTopology>,
+    rack_topology: RackTopology,
 }
 
 impl KnownTarget {
@@ -76,7 +76,7 @@ impl KnownTarget {
         image: Image,
         machine: Option<Machine>,
         switch: Option<Switch>,
-        rack_topology: Option<RackTopology>,
+        rack_topology: RackTopology,
     ) -> Result<Self> {
         if matches!(image, Image::Trampoline) {
             if machine.is_some() {
@@ -103,7 +103,7 @@ impl Default for KnownTarget {
             image: Image::Standard,
             machine: Some(Machine::NonGimlet),
             switch: Some(Switch::Stub),
-            rack_topology: Some(RackTopology::MultiSled),
+            rack_topology: RackTopology::MultiSled,
         }
     }
 }
@@ -118,9 +118,7 @@ impl From<KnownTarget> for Target {
         if let Some(switch) = kt.switch {
             map.insert("switch".to_string(), switch.to_string());
         }
-        if let Some(rack_topology) = kt.rack_topology {
-            map.insert("rack-topology".to_string(), rack_topology.to_string());
-        }
+        map.insert("rack-topology".to_string(), kt.rack_topology.to_string());
         Target(map)
     }
 }
@@ -170,6 +168,11 @@ impl std::str::FromStr for KnownTarget {
                 }
             }
         }
-        KnownTarget::new(image, machine, switch, rack_topology)
+        KnownTarget::new(
+            image,
+            machine,
+            switch,
+            rack_topology.unwrap_or(RackTopology::MultiSled),
+        )
     }
 }