[4/n] [omicron-package] add and use target presets (#7288)

The overall goal of this change is to ensure that all release-related configuration is present in `package-manifest.toml`. This will allow linting and SBOM generation based on this config, rather than the knowledge being scattered across omicron-package and the releng tool.
oxidecomputer · Jan 7, 2025 · 390ac60 · 390ac60
1 parent 12b65a3
commit 390ac60
Show file tree

Hide file tree

Showing 12 changed files with 531 additions and 176 deletions.
diff --git a/.github/buildomat/jobs/package.sh b/.github/buildomat/jobs/package.sh
@@ -28,7 +28,7 @@ ptime -m cargo xtask download softnpu
 # Build the test target
 export CARGO_INCREMENTAL=0
 ptime -m cargo run --locked --release --bin omicron-package -- \
-  -t test target create -i standard -m non-gimlet -s softnpu -r single-sled
+  -t test target create -p dev
 ptime -m cargo run --locked --release --bin omicron-package -- \
   -t test package
 mapfile -t packages \

diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -53,7 +53,7 @@ jobs:
     - name: Install Pre-Requisites
       run: ./tools/install_builder_prerequisites.sh -y
     - name: Set default target
-      run: cargo run --bin omicron-package -- -t default target create -r single-sled
+      run: cargo run --bin omicron-package -- -t default target create --preset dev
     - name: Check build of deployed Omicron packages
       run: cargo run --bin omicron-package -- -t default check
 

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/dev-tools/releng/src/main.rs b/dev-tools/releng/src/main.rs
@@ -454,8 +454,13 @@ async fn main() -> Result<()> {
                     artifacts_path.as_str(),
                     "target",
                     "create",
+                    "--preset",
+                    target.as_str(),
                 ])
-                .args(target.target_args())
+                // Note: Do not override the preset by adding arguments like
+                // `-m`/`--machine` here, or anywhere else in the releng
+                // tooling! All release targets must be configured entirely via
+                // the `target.preset` table in `package-manifest.toml`.
                 .env_remove("CARGO_MANIFEST_DIR"),
         )
         .after("omicron-package");
@@ -639,22 +644,6 @@ impl Target {
         }
     }
 
-    fn target_args(self) -> &'static [&'static str] {
-        match self {
-            Target::Host => &[
-                "--image",
-                "standard",
-                "--machine",
-                "gimlet",
-                "--switch",
-                "asic",
-                "--rack-topology",
-                "multi-sled",
-            ],
-            Target::Recovery => &["--image", "trampoline"],
-        }
-    }
-
     fn proto_packages(
         self,
     ) -> &'static [(&'static PackageName, InstallMethod)] {

diff --git a/docs/how-to-run.adoc b/docs/how-to-run.adoc
@@ -332,57 +332,88 @@ In some configurations (not the one described here), it may be necessary to upda
 
 The `omicron-package` tool builds Omicron and bundles all required files into _packages_ that can be copied to another system (if necessary) and installed there.  This tool acts on `package-manifest.toml`, which describes the contents of the packages.
 
-Packages have a notion of "build targets", which are used to select between different variants of certain components.  A build target is composed of an image type, a machine type, and a switch type:
+Packages have a notion of "build targets", which are used to select between different variants of certain components. For example, the Sled Agent can be built for a real Oxide system, for a standalone Gimlet, or for a non-Gimlet system. This choice is represented by the `--machine` setting here:
 
 [source,console]
 ----
-$ cargo run --release --bin omicron-package -- target create -h
-    Finished release [optimized] target(s) in 0.70s
-     Running `target/release/omicron-package target create -h`
+$ cargo run --release --bin omicron-package -- target create --help
+    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.55s
+     Running `target/release/omicron-package target create --help`
 Error: Creates a new build target, and sets it as "active"
 
-Usage: omicron-package target create [OPTIONS]
+Usage: omicron-package target create [OPTIONS] --preset <PRESET>
 
 Options:
+  -p, --preset <PRESET>
+          The preset to use as part of the build (use `dev` for development).
+
+          Presets are defined in the `target.preset` section of the config. The other configurations are layered on top of
+          the preset.
+
   -i, --image <IMAGE>
-          [default: standard]
+          The image to use for the target.
+
+          If specified, this configuration is layered on top of the preset.
 
           Possible values:
           - standard:   A typical host OS image
           - trampoline: A recovery host OS image, intended to bootstrap a Standard image
 
   -m, --machine <MACHINE>
+          The kind of machine to build for
+
           Possible values:
           - gimlet:            Use sled agent configuration for a Gimlet
           - gimlet-standalone: Use sled agent configuration for a Gimlet running in isolation
           - non-gimlet:        Use sled agent configuration for a device emulating a Gimlet
 
   -s, --switch <SWITCH>
+          The switch to use for the target
+
           Possible values:
           - asic:    Use the "real" Dendrite, that attempts to interact with the Tofino
           - stub:    Use a "stub" Dendrite that does not require any real hardware
           - softnpu: Use a "softnpu" Dendrite that uses the SoftNPU asic emulator
 
   -r, --rack-topology <RACK_TOPOLOGY>
+          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
+          multiple 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.
+
           Possible values:
           - multi-sled:  Use configurations suitable for a multi-sled deployment, such as dogfood and production racks
           - single-sled: Use configurations suitable for a single-sled deployment, such as CI and dev machines
 
+  -c, --clickhouse-topology <CLICKHOUSE_TOPOLOGY>
+          Specify whether clickhouse will be deployed as a replicated cluster or single-node configuration.
+
+          Replicated cluster configuration is an experimental feature to be used only for testing.
+
+          Possible values:
+          - replicated-cluster: Use configurations suitable for a replicated ClickHouse cluster deployment
+          - single-node:        Use configurations suitable for a single-node ClickHouse deployment
+
   -h, --help
           Print help (see a summary with '-h')
-
 ----
 
-To set up a build target for a non-Gimlet machine with simulated (but fully functional) external networking, you would run:
+Setting up a target is typically done by selecting a **preset**. Presets are defined in `package-manifest.toml` under `[target.preset]`.
+
+For development purposes, the recommended preset is `dev`. This preset sets up a build target for a non-Gimlet machine with simulated (but fully functional) external networking:
 
 [source,console]
 ----
-$ cargo run --release --bin omicron-package -- -t default target create -i standard -m non-gimlet -s softnpu -r single-sled
+$ cargo run --release --bin omicron-package -- -t default target create -p dev
     Finished release [optimized] target(s) in 0.66s
-     Running `target/release/omicron-package -t default target create -i standard -m non-gimlet -s softnpu -r single-sled`
+     Running `target/release/omicron-package -t default target create -p dev`
 Created new build target 'default' and set it as active
 ----
 
+To customize the target beyond the preset, use the other options (for example, `--image`). These options will override the settings in the preset.
+
 NOTE: The `target create` command will set the new target as active and thus let you omit the `-t` flag in subsequent commands.
 
 To kick off the build and package everything up, you can run:

diff --git a/package-manifest.toml b/package-manifest.toml
@@ -797,7 +797,7 @@ output.intermediate_only = true
 
 # To package and install the asic variant of the switch, do:
 #
-# $ cargo run --release --bin omicron-package -- -t default target create -i standard -m gimlet -s asic
+# $ cargo run --release --bin omicron-package -- -t default target create -p dev -m gimlet -s asic
 # $ cargo run --release --bin omicron-package -- package
 # $ pfexec ./target/release/omicron-package install
 [package.switch-asic]
@@ -825,7 +825,7 @@ output.type = "zone"
 
 # To package and install the stub variant of the switch, do the following:
 #
-# $ cargo run --release --bin omicron-package -- -t default target create -i standard -m <gimlet|gimlet-standalone|non-gimlet> -s stub
+# $ cargo run --release --bin omicron-package -- -t default target create -p dev -m <gimlet|gimlet-standalone|non-gimlet> -s stub
 # $ cargo run --release --bin omicron-package -- package
 # $ pfexec ./target/release/omicron-package install
 [package.switch-stub]
@@ -851,7 +851,7 @@ output.type = "zone"
 
 # To package and install the softnpu variant of the switch, do the following:
 #
-# $ cargo run --release --bin omicron-package -- -t default target create -i standard -m <gimlet|gimlet-standalone|non-gimlet> -s softnpu
+# $ cargo run --release --bin omicron-package -- -t default target create -p dev -m <gimlet|gimlet-standalone|non-gimlet>
 # $ cargo run --release --bin omicron-package -- package
 # $ pfexec ./target/release/omicron-package install
 [package.switch-softnpu]
@@ -934,4 +934,38 @@ source.type = "local"
 source.rust.binary_names = ["clickana"]
 source.rust.release = true
 output.type = "zone"
-output.intermediate_only = true
+output.intermediate_only = true
+
+# Target configuration
+# --------------------
+#
+# This section defines "targets" built by Omicron. A target is a map of keys and
+# values that are used to filter out packages (via `only_for_targets`) and for
+# other purposes.
+#
+# For what the individual keys mean, see the definition for `TargetCommand` in
+# `package/src/lib.rs`.
+
+# A preset for the host image built during release.
+[target.preset.host]
+image = "standard"
+machine = "gimlet"
+switch = "asic"
+rack-topology = "multi-sled"
+clickhouse-topology = "single-node"
+
+# A preset for the recovery image built during release.
+[target.preset.recovery]
+image = "trampoline"
+# The trampoline image doesn't execute sled-agent and doesn't contain the switch
+# zone, so neither "machine" nor "switch" are defined.
+rack-topology = "single-sled"
+clickhouse-topology = "single-node"
+
+# A preset for development.
+[target.preset.dev]
+image = "standard"
+machine = "non-gimlet"
+switch = "softnpu"
+rack-topology = "single-sled"
+clickhouse-topology = "single-node"
diff --git a/package/Cargo.toml b/package/Cargo.toml
@@ -16,6 +16,7 @@ clap.workspace = true
 futures.workspace = true
 hex.workspace = true
 illumos-utils.workspace = true
+indent_write.workspace = true
 indicatif.workspace = true
 omicron-workspace-hack.workspace = true
 omicron-zone-package.workspace = true

diff --git a/package/src/bin/omicron-package.rs b/package/src/bin/omicron-package.rs
@@ -10,11 +10,14 @@ use clap::{Parser, Subcommand};
 use futures::stream::{self, StreamExt, TryStreamExt};
 use illumos_utils::{zfs, zone};
 use indicatif::{MultiProgress, ProgressBar, ProgressStyle};
-use omicron_package::cargo_plan::build_cargo_plan;
-use omicron_package::config::{Config, ConfigArgs};
-use omicron_package::target::{target_command_help, KnownTarget};
-use omicron_package::{parse, BuildCommand, DeployCommand, TargetCommand};
-use omicron_zone_package::config::{Config as PackageConfig, PackageName};
+use omicron_package::cargo_plan::{
+    build_cargo_plan, do_show_cargo_commands_for_config,
+    do_show_cargo_commands_for_presets,
+};
+use omicron_package::config::{BaseConfig, Config, ConfigArgs};
+use omicron_package::target::target_command_help;
+use omicron_package::{BuildCommand, DeployCommand, TargetCommand};
+use omicron_zone_package::config::PackageName;
 use omicron_zone_package::package::{Package, PackageOutput, PackageSource};
 use omicron_zone_package::progress::Progress;
 use omicron_zone_package::target::TargetMap;
@@ -30,7 +33,6 @@ use std::fs::create_dir_all;
 use std::sync::{Arc, OnceLock};
 use std::time::Duration;
 use tokio::io::{AsyncReadExt, AsyncWriteExt, BufReader};
-use tokio::process::Command;
 
 const OMICRON_SLED_AGENT: PackageName =
     PackageName::new_const("omicron-sled-agent");
@@ -70,49 +72,6 @@ struct Args {
     subcommand: SubCommand,
 }
 
-async fn do_show_cargo_commands(config: &Config) -> Result<()> {
-    let metadata = cargo_metadata::MetadataCommand::new().no_deps().exec()?;
-    let features = config.cargo_features();
-    let cargo_plan =
-        build_cargo_plan(&metadata, config.packages_to_build(), &features)?;
-
-    let release_command = cargo_plan.release.build_command("build");
-    let debug_command = cargo_plan.debug.build_command("build");
-
-    print!("release command: ");
-    if let Some(command) = release_command {
-        println!("{}", command_to_string(&command));
-    } else {
-        println!("(none)");
-    }
-
-    print!("debug command: ");
-    if let Some(command) = debug_command {
-        println!("{}", command_to_string(&command));
-    } else {
-        println!("(none)");
-    }
-
-    Ok(())
-}
-
-fn command_to_string(command: &Command) -> String {
-    // Use shell-words to join the command and arguments into a single string.
-    let mut v = vec![command
-        .as_std()
-        .get_program()
-        .to_str()
-        .expect("program is valid UTF-8")];
-    v.extend(
-        command
-            .as_std()
-            .get_args()
-            .map(|arg| arg.to_str().expect("argument is valid UTF-8")),
-    );
-
-    shell_words::join(&v)
-}
-
 async fn do_for_all_rust_packages(
     config: &Config,
     command: &str,
@@ -159,6 +118,7 @@ async fn do_list_outputs(
 }
 
 async fn do_target(
+    base_config: &BaseConfig,
     artifact_dir: &Utf8Path,
     name: Option<&str>,
     subcommand: &TargetCommand,
@@ -169,13 +129,15 @@ async fn do_target(
     })?;
     match subcommand {
         TargetCommand::Create {
+            preset,
             image,
             machine,
             switch,
             rack_topology,
             clickhouse_topology,
         } => {
-            let target = KnownTarget::new(
+            let preset_target = base_config.get_preset(preset)?;
+            let target = preset_target.with_overrides(
                 image.clone(),
                 machine.clone(),
                 switch.clone(),
@@ -269,7 +231,7 @@ async fn replace_active_link(
 
     let dst = target_dir.join(Config::ACTIVE);
     if !target_dir.join(src).exists() {
-        bail!("TargetMap file {} does not exist", src);
+        bail!("Target file {} does not exist", src);
     }
     let _ = tokio::fs::remove_file(&dst).await;
     tokio::fs::symlink(src, &dst).await.with_context(|| {
@@ -869,7 +831,9 @@ impl Progress for PackageProgress {
 #[tokio::main]
 async fn main() -> Result<()> {
     let args = Args::try_parse()?;
-    let package_config = parse::<_, PackageConfig>(&args.manifest)?;
+    let base_config = BaseConfig::load(&args.manifest).with_context(|| {
+        format!("failed to load base config from {:?}", args.manifest)
+    })?;
 
     let mut open_options = std::fs::OpenOptions::new();
     open_options.write(true).create(true).truncate(true);
@@ -883,9 +847,9 @@ async fn main() -> Result<()> {
     let log = Logger::root(drain, o!());
 
     let get_config = || -> Result<Config> {
-        Config::get_config(
+        Config::load(
             &log,
-            package_config,
+            base_config.package_config(),
             &args.config_args,
             &args.artifact_dir,
         )
@@ -903,6 +867,7 @@ async fn main() -> Result<()> {
     match args.subcommand {
         SubCommand::Build(BuildCommand::Target { subcommand }) => {
             do_target(
+                &base_config,
                 &args.artifact_dir,
                 args.config_args.target.as_deref(),
                 &subcommand,
@@ -930,8 +895,15 @@ async fn main() -> Result<()> {
             )
             .await?;
         }
-        SubCommand::Build(BuildCommand::ShowCargoCommands) => {
-            do_show_cargo_commands(&get_config()?).await?;
+        SubCommand::Build(BuildCommand::ShowCargoCommands { presets }) => {
+            // If presets is empty, show the commands from the
+            // default configuration, otherwise show the commands
+            // for the specified presets.
+            if let Some(presets) = presets {
+                do_show_cargo_commands_for_presets(&base_config, &presets)?;
+            } else {
+                do_show_cargo_commands_for_config(&get_config()?)?;
+            }
         }
         SubCommand::Build(BuildCommand::Check) => {
             do_check(&get_config()?).await?