diff --git a/.github/buildomat/jobs/build-and-test.sh b/.github/buildomat/jobs/build-and-test.sh index 183697e1f1f..769520b805e 100644 --- a/.github/buildomat/jobs/build-and-test.sh +++ b/.github/buildomat/jobs/build-and-test.sh @@ -14,18 +14,6 @@ set -o xtrace cargo --version rustc --version -banner clickhouse -ptime -m ./tools/ci_download_clickhouse - -banner cockroach -ptime -m bash ./tools/ci_download_cockroachdb - -# -# Put "./cockroachdb/bin" and "./clickhouse" on the PATH for the test -# suite. -# -export PATH="$PATH:$PWD/cockroachdb/bin:$PWD/clickhouse" - # # We build with: # @@ -51,6 +39,18 @@ ptime -m cargo +'nightly-2021-11-24' build --locked --all-targets --verbose banner deploy-check ptime -m cargo run --bin omicron-package -- check +banner clickhouse +ptime -m ./tools/ci_download_clickhouse + +banner cockroach +ptime -m bash ./tools/ci_download_cockroachdb + +# +# Put "./cockroachdb/bin" and "./clickhouse" on the PATH for the test +# suite. +# +export PATH="$PATH:$PWD/cockroachdb/bin:$PWD/clickhouse" + # # NOTE: We're using using the same RUSTFLAGS and RUSTDOCFLAGS as above to avoid # having to rebuild here. diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 148cf9391bf..bf91caf30d1 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -108,12 +108,6 @@ jobs: with: key: ${{ runner.os }}-clickhouse-binary-${{ hashFiles('tools/clickhouse_checksums') }} path: "clickhouse" - - name: Download ClickHouse - if: steps.cache-clickhouse.outputs.cache-hit != 'true' - run: ./tools/ci_download_clickhouse - - name: Download CockroachDB binary - if: steps.cache-cockroachdb.outputs.cache-hit != 'true' - run: bash ./tools/ci_download_cockroachdb - name: Build # We build with: # - RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings": disallow warnings @@ -125,7 +119,13 @@ jobs: # also gives us a record of which dependencies were used for each CI # run. Building with `--locked` ensures that the checked-in Cargo.lock # is up to date. - run: PATH="$PATH:$PWD/cockroachdb/bin:$PWD/clickhouse" RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings" cargo build --locked --all-targets --verbose + run: RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings" cargo build --locked --all-targets --verbose + - name: Download ClickHouse + if: steps.cache-clickhouse.outputs.cache-hit != 'true' + run: ./tools/ci_download_clickhouse + - name: Download CockroachDB binary + if: steps.cache-cockroachdb.outputs.cache-hit != 'true' + run: bash ./tools/ci_download_cockroachdb - name: Run tests # Use the same RUSTFLAGS and RUSTDOCFLAGS as above to avoid having to # rebuild here. diff --git a/Cargo.lock b/Cargo.lock index 7d0c748d377..87f892d06f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1804,7 +1804,6 @@ dependencies = [ "serde", "serde_json", "slog", - "tokio", "uuid", ] diff --git a/Cargo.toml b/Cargo.toml index 5853d11a0b3..3f3302fa519 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ default-members = [ "common", "nexus", "nexus/src/db/db-macros", + "nexus/test-utils", "package", "rpaths", "sled-agent", diff --git a/README.adoc b/README.adoc index 85bbb73caa6..6eaa8520b50 100644 --- a/README.adoc +++ b/README.adoc @@ -83,7 +83,7 @@ example, on Helios, you'd want `/usr/bin` on your PATH. -- . CockroachDB v21.1.10. + -The build and test suite expects to be able to start a single-node CockroachDB cluster using the `cockroach` executable on your PATH. +The test suite expects to be able to start a single-node CockroachDB cluster using the `cockroach` executable on your PATH. On illumos, MacOS, and Linux, you should be able to use the `tools/ci_download_cockroachdb` script to fetch the official CockroachDB binary. It will be put into `./cockroachdb/bin/cockroach`. Alternatively, you can follow the https://www.cockroachlabs.com/docs/stable/install-cockroachdb.html[official CockroachDB installation instructions for your platform]. diff --git a/nexus/test-utils/Cargo.toml b/nexus/test-utils/Cargo.toml index b53469d4b27..18d767dd89d 100644 --- a/nexus/test-utils/Cargo.toml +++ b/nexus/test-utils/Cargo.toml @@ -24,8 +24,3 @@ serde = { version = "1.0", features = [ "derive" ] } serde_json = "1.0" slog = { version = "2.7", features = [ "max_level_trace", "release_max_level_debug" ] } uuid = { version = "0.8", features = [ "serde", "v4" ] } - -[build-dependencies] -dropshot = { git = "https://github.com/oxidecomputer/dropshot", branch = "main", features = [ "usdt-probes" ] } -omicron-test-utils = { path = "../../test-utils" } -tokio = { version = "1.14" } diff --git a/nexus/test-utils/build.rs b/nexus/test-utils/build.rs deleted file mode 100644 index 3d7306bf87c..00000000000 --- a/nexus/test-utils/build.rs +++ /dev/null @@ -1,32 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// 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 dropshot::{test_util::LogContext, ConfigLogging, ConfigLoggingLevel}; -use omicron_test_utils::dev::test_setup_database_seed; - -// Creates a "pre-populated" CockroachDB storage directory, which -// subsequent tests can copy instead of creating themselves. -// -// Is it critical this happens at build-time? No. However, it -// makes it more convenient for tests to assume this seeded -// directory exists, rather than all attempting to create it -// concurrently. -// -// Refer to the documentation of [`test_setup_database_seed`] for -// more context. -#[tokio::main] -async fn main() { - println!("cargo:rerun-if-changed=build.rs"); - println!("cargo:rerun-if-changed=../../common/src/sql/dbinit.sql"); - println!("cargo:rerun-if-changed=../../tools/cockroachdb_checksums"); - println!("cargo:rerun-if-changed=../../tools/cockroachdb_version"); - - let logctx = LogContext::new( - "crdb_seeding", - &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Info }, - ); - - test_setup_database_seed(&logctx.log).await; - logctx.cleanup_successful(); -} diff --git a/test-utils/src/dev/db.rs b/test-utils/src/dev/db.rs index 88361210bfa..50793d2c02a 100644 --- a/test-utils/src/dev/db.rs +++ b/test-utils/src/dev/db.rs @@ -204,7 +204,7 @@ impl CockroachStarterBuilder { CockroachStarterBuilder::temp_path(&temp_dir, "listen-url"); let listen_arg = format!("127.0.0.1:{}", self.listen_port); self.arg("--store") - .arg(&store_dir) + .arg(store_dir) .arg("--listen-addr") .arg(&listen_arg) .arg("--listening-url-file") @@ -222,7 +222,6 @@ impl CockroachStarterBuilder { Ok(CockroachStarter { temp_dir, - store_dir: store_dir.into(), listen_url_file, args: self.args, cmd_builder: self.cmd_builder, @@ -261,8 +260,6 @@ impl CockroachStarterBuilder { pub struct CockroachStarter { /// temporary directory used for URL file and potentially data storage temp_dir: TempDir, - /// path to storage directory - store_dir: PathBuf, /// path to listen URL file (inside temp_dir) listen_url_file: PathBuf, /// command-line arguments, mirrored here for reporting to the user @@ -286,11 +283,6 @@ impl CockroachStarter { self.temp_dir.path() } - /// Returns the path to the storage directory created for this execution. - pub fn store_dir(&self) -> &Path { - self.store_dir.as_path() - } - /** * Spawns a new process to run the configured command * diff --git a/test-utils/src/dev/mod.rs b/test-utils/src/dev/mod.rs index 7749ed1ce70..48fa7e9f3fc 100644 --- a/test-utils/src/dev/mod.rs +++ b/test-utils/src/dev/mod.rs @@ -12,63 +12,11 @@ pub mod db; pub mod poll; pub mod test_cmds; -use anyhow::Context; use dropshot::test_util::LogContext; use dropshot::ConfigLogging; use dropshot::ConfigLoggingIfExists; use dropshot::ConfigLoggingLevel; use slog::Logger; -use std::path::{Path, PathBuf}; - -/// Path to the "seed" CockroachDB directory. -/// -/// Populating CockroachDB unfortunately isn't free - creation of -/// tables, indices, and users takes several seconds to complete. -/// -/// By creating a "seed" version of the database, we can cut down -/// on the time spent performing this operation. Instead, we opt -/// to copy the database from this seed location. -fn seed_dir() -> PathBuf { - std::env::temp_dir().join("crdb-base") -} - -// Helper for copying all the files in one directory to another. -fn copy_dir( - src: impl AsRef, - dst: impl AsRef, -) -> Result<(), anyhow::Error> { - let src = src.as_ref(); - let dst = dst.as_ref(); - std::fs::create_dir_all(&dst) - .with_context(|| format!("Failed to create dst {}", dst.display()))?; - for entry in std::fs::read_dir(src) - .with_context(|| format!("Failed to read_dir {}", src.display()))? - { - let entry = entry.with_context(|| { - format!("Failed to read entry in {}", src.display()) - })?; - let ty = entry.file_type().context("Failed to access file type")?; - let target = dst.join(entry.file_name()); - if ty.is_dir() { - copy_dir(entry.path(), &target).with_context(|| { - format!( - "Failed to copy subdirectory {} to {}", - entry.path().display(), - target.display() - ) - })?; - } else { - std::fs::copy(entry.path(), &target).with_context(|| { - format!( - "Failed to copy file at {} to {}", - entry.path().display(), - target.display() - ) - })?; - } - } - Ok(()) -} /** * Set up a [`dropshot::test_util::LogContext`] appropriate for a test named @@ -87,39 +35,11 @@ pub fn test_setup_log(test_name: &str) -> LogContext { LogContext::new(test_name, &log_config) } -enum StorageSource { - Populate, - CopyFromSeed, -} - -/// Creates a [`db::CockroachInstance`] with a populated storage directory. -/// -/// This is intended to optimize subsequent calls to [`test_setup_database`] -/// by reducing the latency of populating the storage directory. -pub async fn test_setup_database_seed(log: &Logger) { - let dir = seed_dir(); - let _ = std::fs::remove_dir_all(&dir); - std::fs::create_dir_all(&dir).unwrap(); - let mut db = setup_database(log, Some(&dir), StorageSource::Populate).await; - db.cleanup().await.unwrap(); -} - -/// Set up a [`db::CockroachInstance`] for running tests. +/** + * Set up a [`db::CockroachInstance`] for running tests against. + */ pub async fn test_setup_database(log: &Logger) -> db::CockroachInstance { - setup_database(log, None, StorageSource::CopyFromSeed).await -} - -async fn setup_database( - log: &Logger, - store_dir: Option<&Path>, - storage_source: StorageSource, -) -> db::CockroachInstance { - let builder = db::CockroachStarterBuilder::new(); - let mut builder = if let Some(store_dir) = store_dir { - builder.store_dir(store_dir) - } else { - builder - }; + let mut builder = db::CockroachStarterBuilder::new(); builder.redirect_stdio_to_files(); let starter = builder.build().unwrap(); info!( @@ -127,31 +47,14 @@ async fn setup_database( "cockroach temporary directory: {}", starter.temp_dir().display() ); - - // If we're going to copy the storage directory from the seed, - // it is critical we do so before starting the DB. - if matches!(storage_source, StorageSource::CopyFromSeed) { - info!(&log, - "cockroach: copying from seed directory ({}) to storage directory ({})", - seed_dir().to_string_lossy(), starter.store_dir().to_string_lossy(), - ); - copy_dir(seed_dir(), starter.store_dir()) - .expect("Cannot copy storage from seed directory"); - } - info!(&log, "cockroach command line: {}", starter.cmdline()); let database = starter.start().await.unwrap(); info!(&log, "cockroach pid: {}", database.pid()); let db_url = database.pg_config(); info!(&log, "cockroach listen URL: {}", db_url); - - // If we populate the storage directory by importing the '.sql' - // file, we must do so after the DB has started. - if matches!(storage_source, StorageSource::Populate) { - info!(&log, "cockroach: populating"); - database.populate().await.expect("failed to populate database"); - info!(&log, "cockroach: populated"); - } + info!(&log, "cockroach: populating"); + database.populate().await.expect("failed to populate database"); + info!(&log, "cockroach: populated"); database }