diff --git a/.github/buildomat/jobs/package.sh b/.github/buildomat/jobs/package.sh index 7a2cc2c369..53b2e960da 100755 --- 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 index 28a0342b39..deec372d85 100644 --- 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 index 39edd5f0b6..7206daedf5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7145,6 +7145,7 @@ dependencies = [ "futures", "hex", "illumos-utils", + "indent_write", "indicatif", "omicron-workspace-hack", "omicron-zone-package", diff --git a/dev-tools/releng/src/main.rs b/dev-tools/releng/src/main.rs index b20b7729fb..78efa3bbe6 100644 --- a/dev-tools/releng/src/main.rs +++ b/dev-tools/releng/src/main.rs @@ -454,8 +454,12 @@ async fn main() -> Result<()> { artifacts_path.as_str(), "target", "create", + "--preset", + target.as_str(), ]) - .args(target.target_args()) + // Note: Do not override the preset here! 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 +643,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 index 35efe8cafa..799a447496 100644 --- 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 Options: + -p, --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 - [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 + 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 + 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 + 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 + 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 index 8140ce43af..5c1dd1879d 100644 --- 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 -s stub +# $ cargo run --release --bin omicron-package -- -t default target create -p dev -m -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 -s softnpu +# $ cargo run --release --bin omicron-package -- -t default target create -p dev -m # $ 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 \ No newline at end of file +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 index ccc768bb8e..35cfaaaa9f 100644 --- 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 index a22312c42a..2b2d415284 100644 --- 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::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? diff --git a/package/src/cargo_plan.rs b/package/src/cargo_plan.rs index 88702452a2..1788d24ba1 100644 --- a/package/src/cargo_plan.rs +++ b/package/src/cargo_plan.rs @@ -4,12 +4,15 @@ use std::collections::BTreeMap; use std::collections::BTreeSet; +use std::fmt; +use std::io::Write; use anyhow::bail; use anyhow::ensure; use anyhow::Context; use anyhow::Result; use cargo_metadata::Metadata; +use indent_write::io::IndentWriter; use omicron_zone_package::config::PackageMap; use omicron_zone_package::config::PackageName; use omicron_zone_package::package::PackageSource; @@ -17,6 +20,11 @@ use slog::info; use slog::Logger; use tokio::process::Command; +use crate::config::cargo_features_for_target; +use crate::config::BaseConfig; +use crate::config::Config; +use crate::config::MultiPresetArg; + /// For a configuration, build a plan: the set of packages, binaries, and /// features to operate on in release and debug modes. pub fn build_cargo_plan<'a>( @@ -82,12 +90,18 @@ pub struct CargoPlan<'a> { pub debug: CargoTargets<'a>, } -impl CargoPlan<'_> { +impl<'a> CargoPlan<'a> { pub async fn run(&self, command: &str, log: &Logger) -> Result<()> { self.release.run(command, log).await?; self.debug.run(command, log).await?; Ok(()) } + + /// Displays a `CargoPlan` in a human-readable format with the provided + /// command name. + pub fn display_human(&'a self, command: &'a str) -> DisplayCargoPlan<'_> { + DisplayCargoPlan { plan: self, command } + } } /// A set of packages, binaries, and features to operate on. @@ -172,3 +186,92 @@ pub enum BuildKind { Release, Debug, } + +/// Show the Cargo commands that would be run for a configuration. +pub fn do_show_cargo_commands_for_config(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)?; + print!("{}", cargo_plan.display_human("build")); + + Ok(()) +} + +/// Show the Cargo commands that would be run for a set of presets. +pub fn do_show_cargo_commands_for_presets( + base_config: &BaseConfig, + presets: &MultiPresetArg, +) -> Result<()> { + let presets = base_config.get_presets(presets)?; + let metadata = cargo_metadata::MetadataCommand::new().no_deps().exec()?; + + for (preset, target) in presets { + let target_map = target.clone().into(); + let features = cargo_features_for_target(&target_map); + + // Build the cargo plan for this preset. + let cargo_plan = build_cargo_plan( + &metadata, + base_config.package_config().packages_to_build(&target_map), + &features, + ) + .with_context(|| { + format!("failed to build cargo plan for preset '{preset}'") + })?; + + // Print out the plan for this preset. + println!("for preset '{}':", preset); + let mut writer = IndentWriter::new(" * ", std::io::stdout().lock()); + writeln!(writer, "{}", cargo_plan.display_human("build"))?; + + writer.flush()?; + } + + Ok(()) +} + +/// A human-readable display of a `CargoPlan`. +/// +/// Created by calling [`CargoPlan::display_human`]. +pub struct DisplayCargoPlan<'a> { + plan: &'a CargoPlan<'a>, + command: &'a str, +} + +impl fmt::Display for DisplayCargoPlan<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "release command: ")?; + if let Some(command) = &self.plan.release.build_command(self.command) { + writeln!(f, "{}", command_to_string(&command))?; + } else { + writeln!(f, "(none)")?; + } + + write!(f, "debug command: ")?; + if let Some(command) = &self.plan.debug.build_command(self.command) { + writeln!(f, "{}", command_to_string(&command))?; + } else { + writeln!(f, "(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) +} diff --git a/package/src/config.rs b/package/src/config.rs index 1efd523496..9aad0bdef5 100644 --- a/package/src/config.rs +++ b/package/src/config.rs @@ -2,11 +2,14 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use anyhow::{bail, Result}; +use anyhow::{bail, Context, Result}; use camino::Utf8Path; use clap::Args; use omicron_zone_package::{ - config::{Config as PackageConfig, PackageMap, PackageName}, + config::{ + Config as PackageConfig, PackageMap, PackageName, PresetName, + TargetConfig, + }, package::PackageSource, target::TargetMap, }; @@ -44,6 +47,157 @@ fn parse_duration_ms(arg: &str) -> Result { Ok(Duration::from_millis(ms)) } +/// A specification for zero or more presets over the command line. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum MultiPresetArg { + /// A list of presets. + List(Vec), + /// All presets. + All, +} + +impl MultiPresetArg { + /// Returns true if there are no presets specified. + #[inline] + pub fn is_empty(&self) -> bool { + match self { + Self::List(list) => list.is_empty(), + Self::All => false, + } + } +} + +impl FromStr for MultiPresetArg { + type Err = anyhow::Error; + + fn from_str(s: &str) -> Result { + if s == "all" { + return Ok(Self::All); + } + + // TODO: it would be nice to collect all errors here. + let list = s + .split(',') + .map(|x| { + let p = x.parse::()?; + + // Ensure that p isn't "all". + if p.as_str() == "all" { + bail!("'all' must not be specified with other presets"); + } + + Ok(p) + }) + .collect::>>()?; + Ok(Self::List(list)) + } +} + +/// A base configuration. +/// +/// This is a thin wrapper around `PackageConfig` that has also validated that +/// the defined presets are valid. +#[derive(Debug)] +pub struct BaseConfig { + package_config: PackageConfig, + presets: BTreeMap, +} + +impl BaseConfig { + /// Loads the base config and ensures that all presets are valid. + pub fn load(manifest: &Utf8Path) -> Result { + let package_config = crate::parse::<_, PackageConfig>(manifest)?; + let presets = build_presets(&package_config.target)?; + + Ok(Self { package_config, presets }) + } + + /// Returns the package configuration. + #[inline] + pub fn package_config(&self) -> &PackageConfig { + &self.package_config + } + + /// Gets the map of all presets. + #[inline] + pub fn presets(&self) -> &BTreeMap { + &self.presets + } + + /// Gets a list of available presets as a string. + pub fn available_presets_str(&self) -> String { + self.presets.keys().map(|x| x.as_str()).collect::>().join(", ") + } + + /// Gets the preset with the given name. + pub fn get_preset(&self, name: &PresetName) -> Result<&KnownTarget> { + self.presets.get(name).with_context(|| { + format!( + "preset '{name}' not found\n(available presets: {})", + self.available_presets_str(), + ) + }) + } + + /// Resolves the specified presets, returning an error if any of them are + /// invalid. + /// + /// Presets are returned in the order they were specified (keeping the order + /// the user specified on the command line). + pub fn get_presets( + &self, + presets: &MultiPresetArg, + ) -> Result> { + match presets { + MultiPresetArg::List(list) => { + let mut valid = Vec::new(); + // Ensure that all specified presets are found in the base config. + let mut missing = Vec::new(); + + for preset in list { + if let Some((preset, target)) = + self.presets.get_key_value(preset) + { + valid.push((preset, target)); + } else { + missing.push(preset); + } + } + + if !missing.is_empty() { + let names = + missing.iter().map(|x| x.as_str()).collect::>(); + bail!( + "presets not found in base config: {}\n(available presets: {})", + names.join(", "), + self.available_presets_str(), + ); + } + + Ok(valid) + } + MultiPresetArg::All => Ok(self.presets.iter().collect()), + } + } +} + +fn build_presets( + config: &TargetConfig, +) -> Result> { + let mut presets = BTreeMap::new(); + + for (name, value) in &config.presets { + // TODO: it would be nice to collect all errors here. + let target = + KnownTarget::from_target_map(value).with_context(|| { + format!("error parsing config for preset '{name}'") + })?; + presets.insert(name.clone(), target); + } + + Ok(presets) +} + #[derive(Debug)] pub struct Config { log: Logger, @@ -66,9 +220,9 @@ impl Config { pub const ACTIVE: &str = "active"; /// Builds a new configuration. - pub fn get_config( + pub fn load( log: &Logger, - package_config: PackageConfig, + package_config: &PackageConfig, args: &ConfigArgs, artifact_dir: &Utf8Path, ) -> Result { @@ -104,7 +258,7 @@ impl Config { Ok(Config { log: log.clone(), - package_config, + package_config: package_config.clone(), target, only: Vec::new(), force: args.force, @@ -234,10 +388,42 @@ impl Config { /// Out of these, the features that actually get requested are determined by /// which features are available for the list of packages being built. pub fn cargo_features(&self) -> Vec { - self.target - .0 - .iter() - .map(|(name, value)| format!("{name}-{value}")) - .collect::>() + cargo_features_for_target(&self.target) + } +} + +/// Return a list of all possible Cargo features that could be requested for the +/// given target. +/// +/// Out of these, the features that actually get requested are determined by +/// which features are available for the list of packages being built. +pub fn cargo_features_for_target(target: &TargetMap) -> Vec { + target.0.iter().map(|(name, value)| format!("{name}-{value}")).collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn multi_preset_arg() { + let all = MultiPresetArg::from_str("all").unwrap(); + assert_eq!(all, MultiPresetArg::All); + + let list = MultiPresetArg::from_str("a,b,c").unwrap(); + assert_eq!( + list, + MultiPresetArg::List(vec![ + "a".parse().unwrap(), + "b".parse().unwrap(), + "c".parse().unwrap(), + ]) + ); + + let error = MultiPresetArg::from_str("a,b,all").unwrap_err(); + assert_eq!( + error.to_string(), + "'all' must not be specified with other presets" + ); } } diff --git a/package/src/lib.rs b/package/src/lib.rs index 969cf6db21..5b931e4218 100644 --- a/package/src/lib.rs +++ b/package/src/lib.rs @@ -2,7 +2,8 @@ use camino::{Utf8Path, Utf8PathBuf}; use clap::Subcommand; -use omicron_zone_package::config::PackageName; +use config::MultiPresetArg; +use omicron_zone_package::config::{PackageName, PresetName}; use serde::de::DeserializeOwned; use thiserror::Error; @@ -36,48 +37,40 @@ pub fn parse, C: DeserializeOwned>( pub enum TargetCommand { /// Creates a new build target, and sets it as "active". Create { - #[clap(short, long, default_value = "standard")] - image: crate::target::Image, + /// 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. + #[clap(short, long)] + preset: PresetName, + + /// The image to use for the target. + /// + /// If specified, this configuration is layered on top of the preset. + #[clap(short, long)] + image: Option, - #[clap( - short, - long, - default_value_if("image", "standard", "non-gimlet") - )] + /// The kind of machine to build for. + #[clap(short, long)] machine: Option, - #[clap(short, long, default_value_if("image", "standard", "stub"))] + /// The switch to use for the target. + #[clap(short, long)] switch: Option, - #[clap( - short, - long, - 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 - )] + #[clap(short, long)] /// 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. + /// 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. - rack_topology: crate::target::RackTopology, + rack_topology: Option, - #[clap( - short, - long, - default_value = Some("single-node"), - required = false - )] + #[clap(short, long)] // TODO (https://github.com/oxidecomputer/omicron/issues/4148): Remove // once single-node functionality is removed. /// Specify whether clickhouse will be deployed as a replicated cluster @@ -85,7 +78,7 @@ pub enum TargetCommand { /// /// Replicated cluster configuration is an experimental feature to be /// used only for testing. - clickhouse_topology: crate::target::ClickhouseTopology, + clickhouse_topology: Option, }, /// List all existing targets List, @@ -134,7 +127,14 @@ pub enum BuildCommand { version: semver::Version, }, /// Show the Cargo commands that would be run to build the packages. - ShowCargoCommands, + ShowCargoCommands { + /// Show cargo commands for the specified presets, or `all` for all + /// presets. + /// + /// If not specified, the active target's preset is used. + #[clap(short, long = "preset")] + presets: Option, + }, /// Checks the packages specified in a manifest, without building them. Check, } diff --git a/package/src/target.rs b/package/src/target.rs index bf2c151443..19a5a832a1 100644 --- a/package/src/target.rs +++ b/package/src/target.rs @@ -85,7 +85,88 @@ pub struct KnownTarget { } impl KnownTarget { - pub fn new( + /// Creates a new `KnownTarget` from a [`TargetMap`] defined in configuration. + /// + /// Real `KnownTarget` instances might have overrides applied to them via + /// the command line. + pub fn from_target_map(target: &TargetMap) -> Result { + let mut image = Self::default().image; + let mut machine = None; + let mut switch = None; + let mut rack_topology = None; + let mut clickhouse_topology = None; + + for (k, v) in &target.0 { + match k.as_str() { + "image" => { + image = v.parse()?; + } + "machine" => { + machine = Some(v.parse()?); + } + "switch" => { + switch = Some(v.parse()?); + } + "rack-topology" => { + rack_topology = Some(v.parse()?); + } + "clickhouse-topology" => { + clickhouse_topology = Some(v.parse()?); + } + _ => { + bail!( + "Unknown target key {k}\nValid keys include: [{}]", + TargetMap::from(Self::default()) + .0 + .keys() + .cloned() + .collect::>() + .join(", "), + ) + } + } + } + + Self::validate_and_create( + image, + machine, + switch, + rack_topology.unwrap_or(RackTopology::MultiSled), + clickhouse_topology.unwrap_or(ClickhouseTopology::SingleNode), + ) + } + + /// Applies overrides to the target, returning a new `KnownTarget` with the + /// parameters applied. + /// + /// Errors if the new target does not satisfy target constraints. + pub fn with_overrides( + &self, + image: Option, + machine: Option, + switch: Option, + rack_topology: Option, + clickhouse_topology: Option, + ) -> Result { + let image = image.unwrap_or(self.image.clone()); + let machine = machine.or(self.machine.clone()); + let switch = switch.or(self.switch.clone()); + let rack_topology = rack_topology.unwrap_or(self.rack_topology.clone()); + let clickhouse_topology = + clickhouse_topology.unwrap_or(self.clickhouse_topology.clone()); + + Self::validate_and_create( + image, + machine, + switch, + rack_topology, + clickhouse_topology, + ) + } + + /// Creates a new `KnownTarget` from the given parameters, validating + /// constraints. + fn validate_and_create( image: Image, machine: Option, switch: Option, @@ -154,50 +235,7 @@ impl std::str::FromStr for KnownTarget { fn from_str(s: &str) -> Result { let target = TargetMap::from_str(s)?; - - let mut image = Self::default().image; - let mut machine = None; - let mut switch = None; - let mut rack_topology = None; - let mut clickhouse_topology = None; - - for (k, v) in target.0.into_iter() { - match k.as_str() { - "image" => { - image = v.parse()?; - } - "machine" => { - machine = Some(v.parse()?); - } - "switch" => { - switch = Some(v.parse()?); - } - "rack-topology" => { - rack_topology = Some(v.parse()?); - } - "clickhouse-topology" => { - clickhouse_topology = Some(v.parse()?); - } - _ => { - bail!( - "Unknown target key {k}\nValid keys include: [{}]", - TargetMap::from(Self::default()) - .0 - .keys() - .cloned() - .collect::>() - .join(", "), - ) - } - } - } - KnownTarget::new( - image, - machine, - switch, - rack_topology.unwrap_or(RackTopology::MultiSled), - clickhouse_topology.unwrap_or(ClickhouseTopology::SingleNode), - ) + Self::from_target_map(&target) } }