diff --git a/Cargo.lock b/Cargo.lock index 4a21ff15cc..1929217356 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5633,6 +5633,7 @@ version = "0.1.0" dependencies = [ "anyhow", "camino", + "cargo_metadata", "clap", "expectorate", "futures", @@ -5654,7 +5655,6 @@ dependencies = [ "slog-term", "smf", "strum", - "swrite", "tar", "thiserror", "tokio", diff --git a/dev-tools/xtask/src/check_workspace_deps.rs b/dev-tools/xtask/src/check_workspace_deps.rs index 94de557a46..73d5643ffb 100644 --- a/dev-tools/xtask/src/check_workspace_deps.rs +++ b/dev-tools/xtask/src/check_workspace_deps.rs @@ -146,6 +146,23 @@ pub fn run_cmd() -> Result<()> { } } + let mut seen_bins = BTreeSet::new(); + for package in &workspace.packages { + if workspace.workspace_members.contains(&package.id) { + for target in &package.targets { + if target.is_bin() { + if !seen_bins.insert(&target.name) { + eprintln!( + "error: bin target {:?} seen multiple times", + target.name + ); + nerrors += 1; + } + } + } + } + } + eprintln!( "check-workspace-deps: errors: {}, warnings: {}", nerrors, nwarnings diff --git a/installinator/Cargo.toml b/installinator/Cargo.toml index ebdb6269b7..c21c3f2ee2 100644 --- a/installinator/Cargo.toml +++ b/installinator/Cargo.toml @@ -57,5 +57,3 @@ tokio-stream.workspace = true [features] image-standard = [] -image-trampoline = [] -rack-topology-single-sled = [] diff --git a/oximeter/db/src/client/mod.rs b/oximeter/db/src/client/mod.rs index 9a2b7b1bd3..0db71d195a 100644 --- a/oximeter/db/src/client/mod.rs +++ b/oximeter/db/src/client/mod.rs @@ -4185,4 +4185,242 @@ mod tests { db.cleanup().await.unwrap(); logctx.cleanup_successful(); } + + // The schema directory, used in tests. The actual updater uses the + // zone-image files copied in during construction. + const SCHEMA_DIR: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/schema"); + + #[tokio::test] + async fn check_actual_schema_upgrades_are_valid_single_node() { + check_actual_schema_upgrades_are_valid_impl(false).await; + } + + #[tokio::test] + async fn check_actual_schema_upgrades_are_valid_replicated() { + check_actual_schema_upgrades_are_valid_impl(true).await; + } + + // NOTE: This does not actually run the upgrades, only checks them for + // validity. + async fn check_actual_schema_upgrades_are_valid_impl(replicated: bool) { + let name = format!( + "check_actual_schema_upgrades_are_valid_{}", + if replicated { "replicated" } else { "single_node" } + ); + let logctx = test_setup_log(&name); + let log = &logctx.log; + + // We really started tracking the database version in 2. However, that + // set of files is not valid by construction, since we were just + // creating the full database from scratch as an "upgrade". So we'll + // start by applying version 3, and then do all later ones. + const FIRST_VERSION: u64 = 3; + for version in FIRST_VERSION..=OXIMETER_VERSION { + let upgrade_file_contents = Client::read_schema_upgrade_sql_files( + log, false, version, SCHEMA_DIR, + ) + .await + .expect("failed to read schema upgrade files"); + + if let Err(e) = + Client::verify_schema_upgrades(&upgrade_file_contents) + { + panic!( + "Schema update files for version {version} \ + are not valid: {e:?}" + ); + } + } + logctx.cleanup_successful(); + } + + // Either a cluster or single node. + // + // How does this not already exist... + enum ClickHouse { + Cluster(ClickHouseCluster), + Single(ClickHouseInstance), + } + + impl ClickHouse { + fn port(&self) -> u16 { + match self { + ClickHouse::Cluster(cluster) => cluster.replica_1.port(), + ClickHouse::Single(node) => node.port(), + } + } + + async fn cleanup(mut self) { + match &mut self { + ClickHouse::Cluster(cluster) => { + cluster + .keeper_1 + .cleanup() + .await + .expect("Failed to cleanup ClickHouse keeper 1"); + cluster + .keeper_2 + .cleanup() + .await + .expect("Failed to cleanup ClickHouse keeper 2"); + cluster + .keeper_3 + .cleanup() + .await + .expect("Failed to cleanup ClickHouse keeper 3"); + cluster + .replica_1 + .cleanup() + .await + .expect("Failed to cleanup ClickHouse server 1"); + cluster + .replica_2 + .cleanup() + .await + .expect("Failed to cleanup ClickHouse server 2"); + } + ClickHouse::Single(node) => node + .cleanup() + .await + .expect("Failed to cleanup single node ClickHouse"), + } + } + } + + #[tokio::test] + async fn check_db_init_is_sum_of_all_up_single_node() { + check_db_init_is_sum_of_all_up_impl(false).await; + } + + #[tokio::test] + async fn check_db_init_is_sum_of_all_up_replicated() { + check_db_init_is_sum_of_all_up_impl(true).await; + } + + // Check that the set of tables we arrive at through upgrades equals those + // we get by creating the latest version directly. + async fn check_db_init_is_sum_of_all_up_impl(replicated: bool) { + let name = format!( + "check_db_init_is_sum_of_all_up_{}", + if replicated { "replicated" } else { "single_node" } + ); + let logctx = test_setup_log(&name); + let log = &logctx.log; + let db = if replicated { + ClickHouse::Cluster(create_cluster(&logctx).await) + } else { + ClickHouse::Single( + ClickHouseInstance::new_single_node(&logctx, 0) + .await + .expect("Failed to start ClickHouse"), + ) + }; + let address = SocketAddr::new(Ipv6Addr::LOCALHOST.into(), db.port()); + let client = Client::new(address, &log); + + // Let's start with version 2, which is the first tracked and contains + // the full SQL files we need to populate the DB. + client + .initialize_db_with_version(replicated, 2) + .await + .expect("Failed to initialize timeseries database"); + + // Now let's apply all the SQL updates from here to the latest. + for version in 3..=OXIMETER_VERSION { + client + .ensure_schema(replicated, version, SCHEMA_DIR) + .await + .expect("Failed to ensure schema"); + } + + // Fetch all the tables as a JSON blob. + let tables_through_upgrades = + fetch_oximeter_table_details(&client).await; + + // We'll completely re-init the DB with the real version now. + if replicated { + client.wipe_replicated_db().await.unwrap() + } else { + client.wipe_single_node_db().await.unwrap() + } + client + .initialize_db_with_version(replicated, OXIMETER_VERSION) + .await + .expect("Failed to initialize timeseries database"); + + // Fetch the tables again and compare. + let tables = fetch_oximeter_table_details(&client).await; + + // This is an annoying comparison. Since the tables are quite + // complicated, we want to really be careful about what errors we show. + // Iterate through all the expected tables (from the direct creation), + // and check each expected field matches. Then we also check that the + // tables from the upgrade path don't have anything else in them. + for (name, json) in tables.iter() { + let upgrade_table = + tables_through_upgrades.get(name).unwrap_or_else(|| { + panic!("The tables via upgrade are missing table '{name}'") + }); + for (key, value) in json.iter() { + let other_value = upgrade_table.get(key).unwrap_or_else(|| { + panic!("Upgrade table is missing key '{key}'") + }); + assert_eq!( + value, + other_value, + "{} database table {name} disagree on the value \ + of the column {key} between the direct table creation \ + and the upgrade path.\nDirect:\n\n{value} \ + \n\nUpgrade:\n\n{other_value}", + if replicated { "Replicated" } else { "Single-node" }, + ); + } + } + + // Check there are zero keys in the upgrade path that don't appear in + // the direct path. + let extra_keys: Vec<_> = tables_through_upgrades + .keys() + .filter(|k| !tables.contains_key(k.as_str())) + .cloned() + .collect(); + assert!( + extra_keys.is_empty(), + "The oximeter database contains tables in the upgrade path \ + that are not in the direct path: {extra_keys:?}" + ); + + db.cleanup().await; + logctx.cleanup_successful(); + } + + // Read the relevant table details from the `oximeter` database, and return + // it keyed on the table name. + async fn fetch_oximeter_table_details( + client: &Client, + ) -> BTreeMap> { + let out = client + .execute_with_body( + "SELECT \ + name, + engine_full, + create_table_query, + sorting_key, + primary_key + FROM system.tables \ + WHERE database = 'oximeter'\ + FORMAT JSONEachRow;", + ) + .await + .unwrap() + .1; + out.lines() + .map(|line| { + let json: serde_json::Map = + serde_json::from_str(&line).unwrap(); + let name = json.get("name").unwrap().to_string(); + (name, json) + }) + .collect() + } } diff --git a/oximeter/producer/src/lib.rs b/oximeter/producer/src/lib.rs index 6bf8954ae0..36b05d7bb1 100644 --- a/oximeter/producer/src/lib.rs +++ b/oximeter/producer/src/lib.rs @@ -9,7 +9,6 @@ use dropshot::endpoint; use dropshot::ApiDescription; use dropshot::ConfigDropshot; -use dropshot::ConfigLogging; use dropshot::HttpError; use dropshot::HttpResponseOk; use dropshot::HttpServer; @@ -42,6 +41,13 @@ use std::time::Duration; use thiserror::Error; use uuid::Uuid; +// Our public interface depends directly or indirectly on these types; we +// export them so that consumers need not depend on dropshot themselves and +// to simplify how we stage incompatible upgrades. +pub use dropshot::ConfigLogging; +pub use dropshot::ConfigLoggingIfExists; +pub use dropshot::ConfigLoggingLevel; + #[derive(Debug, Clone, Error)] pub enum Error { #[error("Error running producer HTTP server: {0}")] diff --git a/package/Cargo.toml b/package/Cargo.toml index 4632e66731..b63a5ed96f 100644 --- a/package/Cargo.toml +++ b/package/Cargo.toml @@ -11,11 +11,13 @@ workspace = true [dependencies] anyhow.workspace = true camino.workspace = true +cargo_metadata.workspace = true clap.workspace = true futures.workspace = true hex.workspace = true illumos-utils.workspace = true indicatif.workspace = true +omicron-workspace-hack.workspace = true omicron-zone-package.workspace = true petgraph.workspace = true rayon.workspace = true @@ -30,13 +32,11 @@ slog-bunyan.workspace = true slog-term.workspace = true smf.workspace = true strum.workspace = true -swrite.workspace = true tar.workspace = true thiserror.workspace = true tokio = { workspace = true, features = [ "full" ] } toml.workspace = true walkdir.workspace = true -omicron-workspace-hack.workspace = true [dev-dependencies] expectorate.workspace = true diff --git a/package/src/bin/omicron-package.rs b/package/src/bin/omicron-package.rs index a8c5508b77..6db168c9f8 100644 --- a/package/src/bin/omicron-package.rs +++ b/package/src/bin/omicron-package.rs @@ -4,7 +4,7 @@ //! Utility for bundling target binaries as tarfiles. -use anyhow::{anyhow, bail, Context, Result}; +use anyhow::{anyhow, bail, ensure, Context, Result}; use camino::{Utf8Path, Utf8PathBuf}; use clap::{Parser, Subcommand}; use futures::stream::{self, StreamExt, TryStreamExt}; @@ -12,7 +12,7 @@ use illumos_utils::{zfs, zone}; use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; use omicron_package::target::KnownTarget; use omicron_package::{parse, BuildCommand, DeployCommand, TargetCommand}; -use omicron_zone_package::config::Config as PackageConfig; +use omicron_zone_package::config::{Config as PackageConfig, PackageMap}; use omicron_zone_package::package::{Package, PackageOutput, PackageSource}; use omicron_zone_package::progress::Progress; use omicron_zone_package::target::Target; @@ -24,13 +24,13 @@ use slog::o; use slog::Drain; use slog::Logger; use slog::{info, warn}; +use std::collections::{BTreeMap, BTreeSet}; use std::env; use std::fs::create_dir_all; use std::io::Write; use std::str::FromStr; use std::sync::{Arc, OnceLock}; use std::time::Duration; -use swrite::{swrite, SWrite}; use tokio::io::{AsyncReadExt, AsyncWriteExt, BufReader}; use tokio::process::Command; @@ -105,82 +105,117 @@ struct Args { subcommand: SubCommand, } -async fn run_cargo_on_packages( - subcmd: &str, - packages: I, +#[derive(Debug, Default)] +struct CargoPlan<'a> { + command: &'a str, + bins: BTreeSet<&'a String>, + features: BTreeSet<&'a String>, release: bool, - features: &str, -) -> Result<()> -where - I: IntoIterator, - S: AsRef, -{ - let mut cmd = Command::new("cargo"); - // We rely on the rust-toolchain.toml file for toolchain information, - // rather than specifying one within the packaging tool. - cmd.arg(subcmd); - for package in packages { - cmd.arg("-p").arg(package); - } - cmd.arg("--features").arg(features); - if release { - cmd.arg("--release"); - } - let status = cmd - .status() - .await - .context(format!("Failed to run command: ({:?})", cmd))?; - if !status.success() { - bail!("Failed to build packages"); - } +} - Ok(()) +impl<'a> CargoPlan<'a> { + async fn run(&self, log: &Logger) -> Result<()> { + if self.bins.is_empty() { + return Ok(()); + } + + let mut cmd = Command::new("cargo"); + // We rely on the rust-toolchain.toml file for toolchain information, + // rather than specifying one within the packaging tool. + cmd.arg(self.command); + for bin in &self.bins { + cmd.arg("--bin").arg(bin); + } + if !self.features.is_empty() { + cmd.arg("--features").arg(self.features.iter().fold( + String::new(), + |mut acc, s| { + if !acc.is_empty() { + acc.push(' '); + } + acc.push_str(s); + acc + }, + )); + } + if self.release { + cmd.arg("--release"); + } + info!(log, "running: {:?}", cmd.as_std()); + let status = cmd + .status() + .await + .context(format!("Failed to run command: ({:?})", cmd))?; + if !status.success() { + bail!("Failed to build packages"); + } + + Ok(()) + } } async fn do_for_all_rust_packages( config: &Config, command: &str, ) -> Result<()> { - // First, filter out all Rust packages from the configuration that should be - // built, and partition them into "release" and "debug" categories. - let (release_pkgs, debug_pkgs): (Vec<_>, _) = config - .package_config - .packages_to_build(&config.target) - .0 + // Collect a map of all of the workspace packages + let workspace = cargo_metadata::MetadataCommand::new().no_deps().exec()?; + let workspace_pkgs = workspace + .packages .into_iter() - .filter_map(|(name, pkg)| match &pkg.source { - PackageSource::Local { rust: Some(rust_pkg), .. } => { - Some((name, rust_pkg.release)) - } - _ => None, + .filter_map(|package| { + workspace + .workspace_members + .contains(&package.id) + .then_some((package.name.clone(), package)) }) - .partition(|(_, release)| *release); - - let features = - config.target.0.iter().fold(String::new(), |mut acc, (name, value)| { - swrite!(acc, "{}-{} ", name, value); - acc - }); + .collect::>(); - // Execute all the release / debug packages at the same time. - if !release_pkgs.is_empty() { - run_cargo_on_packages( - command, - release_pkgs.iter().map(|(name, _)| name), - true, - &features, - ) - .await?; - } - if !debug_pkgs.is_empty() { - run_cargo_on_packages( - command, - debug_pkgs.iter().map(|(name, _)| name), - false, - &features, - ) - .await?; + // Generate a list of all features we might want to request + let features = config + .target + .0 + .iter() + .map(|(name, value)| format!("{name}-{value}")) + .collect::>(); + + // We split the packages to be built into "release" and "debug" lists + let mut release = + CargoPlan { command, release: true, ..Default::default() }; + let mut debug = CargoPlan { command, release: false, ..Default::default() }; + + for (name, pkg) in config.packages_to_build().0 { + // If this is a Rust package... + if let PackageSource::Local { rust: Some(rust_pkg), .. } = &pkg.source { + let plan = if rust_pkg.release { &mut release } else { &mut debug }; + // Get the package metadata + let metadata = workspace_pkgs.get(name).with_context(|| { + format!("package '{name}' is not a workspace package") + })?; + // Add the binaries we want to build to the plan + let bins = metadata + .targets + .iter() + .filter_map(|target| target.is_bin().then_some(&target.name)) + .collect::>(); + for bin in &rust_pkg.binary_names { + ensure!( + bins.contains(bin), + "bin target '{bin}' does not belong to package '{name}'" + ); + plan.bins.insert(bin); + } + // Add all features we want to request to the plan + plan.features.extend( + features + .iter() + .filter(|feature| metadata.features.contains_key(*feature)), + ); + } } + + release.run(&config.log).await?; + debug.run(&config.log).await?; Ok(()) } @@ -205,9 +240,7 @@ async fn do_list_outputs( output_directory: &Utf8Path, intermediate: bool, ) -> Result<()> { - for (name, package) in - config.package_config.packages_to_build(&config.target).0 - { + for (name, package) in config.packages_to_build().0 { if !intermediate && package.output == (PackageOutput::Zone { intermediate_only: true }) @@ -508,7 +541,7 @@ async fn do_package( do_build(&config).await?; - let packages = config.package_config.packages_to_build(&config.target); + let packages = config.packages_to_build(); let package_iter = packages.build_order(); for batch in package_iter { @@ -912,6 +945,8 @@ struct Config { package_config: PackageConfig, // Description of the target we're trying to operate on. target: Target, + // The list of packages the user wants us to build (all, if empty) + only: Vec, // True if we should skip confirmations for destructive operations. force: bool, // Number of times to retry failed downloads. @@ -937,6 +972,67 @@ impl Config { _ => bail!("Aborting"), } } + + /// Returns target packages to be assembled on the builder machine, limited + /// to those specified in `only` (if set). + fn packages_to_build(&self) -> PackageMap<'_> { + let packages = self.package_config.packages_to_build(&self.target); + if self.only.is_empty() { + return packages; + } + + let mut filtered_packages = PackageMap(BTreeMap::new()); + let mut to_walk = PackageMap(BTreeMap::new()); + // add the requested packages to `to_walk` + for package_name in &self.only { + to_walk.0.insert( + package_name, + packages.0.get(package_name).unwrap_or_else(|| { + panic!( + "Explicitly-requested package '{}' does not exist", + package_name + ) + }), + ); + } + // dependencies are listed by output name, so create a lookup table to + // get a package by its output name. + let lookup_by_output = packages + .0 + .iter() + .map(|(name, package)| { + (package.get_output_file(name), (*name, *package)) + }) + .collect::>(); + // packages yet to be walked are added to `to_walk`. pop each entry and + // add its dependencies to `to_walk`, then add the package we finished + // walking to `filtered_packages`. + while let Some((package_name, package)) = to_walk.0.pop_first() { + if let PackageSource::Composite { packages } = &package.source { + for output in packages { + // find the package by output name + let (dep_name, dep_package) = + lookup_by_output.get(output).unwrap_or_else(|| { + panic!( + "Could not find a package which creates '{}'", + output + ) + }); + if dep_name.as_str() == package_name { + panic!("'{}' depends on itself", package_name); + } + // if we've seen this package already, it will be in + // `filtered_packages`. otherwise, add it to `to_walk`. + if !filtered_packages.0.contains_key(dep_name) { + to_walk.0.insert(dep_name, dep_package); + } + } + } + // we're done looking at this package's deps + filtered_packages.0.insert(package_name, package); + } + filtered_packages + } } #[tokio::main] @@ -989,6 +1085,7 @@ async fn main() -> Result<()> { log: log.clone(), package_config, target, + only: Vec::new(), force: args.force, retry_count: args.retry_count, retry_duration: args.retry_duration, @@ -1004,7 +1101,7 @@ async fn main() -> Result<()> { })?; } - match &args.subcommand { + match args.subcommand { SubCommand::Build(BuildCommand::Target { subcommand }) => { do_target(&args.artifact_dir, &args.target, &subcommand).await?; } @@ -1012,16 +1109,22 @@ async fn main() -> Result<()> { do_dot(&get_config()?).await?; } SubCommand::Build(BuildCommand::ListOutputs { intermediate }) => { - do_list_outputs(&get_config()?, &args.artifact_dir, *intermediate) + do_list_outputs(&get_config()?, &args.artifact_dir, intermediate) .await?; } - SubCommand::Build(BuildCommand::Package { disable_cache }) => { - do_package(&get_config()?, &args.artifact_dir, *disable_cache) - .await?; + SubCommand::Build(BuildCommand::Package { disable_cache, only }) => { + let mut config = get_config()?; + config.only = only; + do_package(&config, &args.artifact_dir, disable_cache).await?; } SubCommand::Build(BuildCommand::Stamp { package_name, version }) => { - do_stamp(&get_config()?, &args.artifact_dir, package_name, version) - .await?; + do_stamp( + &get_config()?, + &args.artifact_dir, + &package_name, + &version, + ) + .await?; } SubCommand::Build(BuildCommand::Check) => { do_check(&get_config()?).await? diff --git a/package/src/lib.rs b/package/src/lib.rs index 2b99cfbe07..2009de9dfe 100644 --- a/package/src/lib.rs +++ b/package/src/lib.rs @@ -103,6 +103,9 @@ pub enum BuildCommand { /// By default, the cache is used. #[clap(short, long)] disable_cache: bool, + /// Limit to building only these packages + #[clap(long)] + only: Vec, }, /// Stamps semver versions onto packages within a manifest Stamp { diff --git a/sled-agent/Cargo.toml b/sled-agent/Cargo.toml index 167ac987ca..b798ba783d 100644 --- a/sled-agent/Cargo.toml +++ b/sled-agent/Cargo.toml @@ -127,14 +127,7 @@ name = "sled-agent" doc = false [features] -image-standard = [] image-trampoline = [] -machine-gimlet = [] -machine-gimlet-standalone = [] -machine-non-gimlet = [] switch-asic = [] switch-stub = [] switch-softnpu = [] -switch-hypersoftnpu = [] -rack-topology-single-sled = [] -rack-topology-multi-sled = [] diff --git a/tools/permslip_staging b/tools/permslip_staging index e0c4b4ad70..bc9250ae93 100644 --- a/tools/permslip_staging +++ b/tools/permslip_staging @@ -1,5 +1,5 @@ -844c56d542700c4b613d9cd7aee5ab306c8d0b969e5dfe194b1b7468a6a9752b manifest-gimlet-v1.0.21.toml +c28eaa13638f55100a42916727227242ee02d18cebecb1412d6af5c8aa945b99 manifest-gimlet-v1.0.22.toml b973cc9feb20f7bba447e7f5291c4070387fa9992deab81301f67f0a3844cd0c manifest-oxide-rot-1-v1.0.11.toml -ca14a77639db3b71c60234e4edebd01ff31ba5a93a842100a991dbf3ad6e94fb manifest-psc-v1.0.20.toml -af0f6c7d0723db33a2972343cc42e4c2ee2ab8884c49808c9c3d8289c193f97b manifest-sidecar-v1.0.21.toml +6d53bfbfdd6baa3fc150153a003abfac6d4b46c34f61fa7a8ec2af8af19a7d5a manifest-psc-v1.0.21.toml +d608dba3fa5a1fce3592ff3f643319787218b84706134147e5918f5bd1c0345d manifest-sidecar-v1.0.22.toml c0fecaefac7674138337f3bd4ce4ce5b884053dead5ec27b575701471631ea2f manifest-bootleby-v1.3.0.toml