Skip to content

Commit 41f22f3

Browse files
authored
[nexus-test-utils] replace build.rs with a new setup script (#4150)
Nextest now has support for setup scripts, which means that we can replace the existing `build.rs` with a small binary. I also took the liberty of switching some of the related code over to camino.
1 parent 0a5b91a commit 41f22f3

File tree

11 files changed

+156
-56
lines changed

11 files changed

+156
-56
lines changed

.config/nextest.toml

+10-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,16 @@
33
#
44
# The required version should be bumped up if we need new features, performance
55
# improvements or bugfixes that are present in newer versions of nextest.
6-
nextest-version = { required = "0.9.55", recommended = "0.9.57" }
6+
nextest-version = { required = "0.9.59", recommended = "0.9.59" }
7+
8+
experimental = ["setup-scripts"]
9+
10+
[[profile.default.scripts]]
11+
filter = 'rdeps(nexus-test-utils)'
12+
setup = 'crdb-seed'
713

814
[profile.ci]
915
fail-fast = false
16+
17+
[script.crdb-seed]
18+
command = 'cargo run -p crdb-seed'

.github/buildomat/build-and-test.sh

+6-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ set -o xtrace
77
# NOTE: This version should be in sync with the recommended version in
88
# .config/nextest.toml. (Maybe build an automated way to pull the recommended
99
# version in the future.)
10-
NEXTEST_VERSION='0.9.57'
10+
NEXTEST_VERSION='0.9.59'
1111

1212
cargo --version
1313
rustc --version
@@ -66,6 +66,11 @@ ptime -m timeout 2h cargo nextest run --profile ci --locked --verbose
6666
banner doctest
6767
ptime -m timeout 1h cargo test --doc --locked --verbose --no-fail-fast
6868

69+
# We expect the seed CRDB to be placed here, so we explicitly remove it so the
70+
# rmdir check below doesn't get triggered. Nextest doesn't have support for
71+
# teardown scripts so this is the best we've got.
72+
rm -rf "$TEST_TMPDIR/crdb-base"
73+
6974
#
7075
# Make sure that we have left nothing around in $TEST_TMPDIR. The easiest way
7176
# to check is to try to remove it with `rmdir`.

Cargo.lock

+15-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ members = [
66
"caboose-util",
77
"certificates",
88
"common",
9+
"crdb-seed",
910
"ddm-admin-client",
1011
"deploy",
1112
"dev-tools/omdb",

crdb-seed/Cargo.toml

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
[package]
2+
name = "crdb-seed"
3+
version = "0.1.0"
4+
edition = "2021"
5+
license = "MPL-2.0"
6+
7+
[dependencies]
8+
camino.workspace = true
9+
camino-tempfile.workspace = true
10+
dropshot.workspace = true
11+
hex.workspace = true
12+
omicron-test-utils.workspace = true
13+
ring.workspace = true
14+
slog.workspace = true
15+
tokio.workspace = true

crdb-seed/src/main.rs

+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
use camino::Utf8PathBuf;
2+
use dropshot::{test_util::LogContext, ConfigLogging, ConfigLoggingLevel};
3+
use omicron_test_utils::dev;
4+
use slog::Logger;
5+
use std::io::Write;
6+
7+
// Creates a string identifier for the current DB schema and version.
8+
//
9+
// The goal here is to allow to create different "seed" directories
10+
// for each revision of the DB.
11+
fn digest_unique_to_schema() -> String {
12+
let schema = include_str!("../../schema/crdb/dbinit.sql");
13+
let crdb_version = include_str!("../../tools/cockroachdb_version");
14+
let mut ctx = ring::digest::Context::new(&ring::digest::SHA256);
15+
ctx.update(&schema.as_bytes());
16+
ctx.update(&crdb_version.as_bytes());
17+
let digest = ctx.finish();
18+
hex::encode(digest.as_ref())
19+
}
20+
21+
enum SeedDirectoryStatus {
22+
Created,
23+
Existing,
24+
}
25+
26+
async fn ensure_seed_directory_exists(
27+
log: &Logger,
28+
) -> (Utf8PathBuf, SeedDirectoryStatus) {
29+
let base_seed_dir = Utf8PathBuf::from_path_buf(std::env::temp_dir())
30+
.expect("Not a UTF-8 path")
31+
.join("crdb-base");
32+
std::fs::create_dir_all(&base_seed_dir).unwrap();
33+
let desired_seed_dir = base_seed_dir.join(digest_unique_to_schema());
34+
35+
if desired_seed_dir.exists() {
36+
return (desired_seed_dir, SeedDirectoryStatus::Existing);
37+
}
38+
39+
// The directory didn't exist when we started, so try to create it.
40+
//
41+
// Nextest will execute it just once, but it is possible for a user to start
42+
// up multiple nextest processes to be running at the same time. So we
43+
// should consider it possible for another caller to create this seed
44+
// directory before we finish setting it up ourselves.
45+
let tmp_seed_dir =
46+
camino_tempfile::Utf8TempDir::new_in(base_seed_dir).unwrap();
47+
dev::test_setup_database_seed(log, tmp_seed_dir.path()).await;
48+
49+
// If we can successfully perform the rename, there was either no
50+
// contention or we won a creation race.
51+
//
52+
// If we couldn't perform the rename, the directory might already exist.
53+
// Check that this is the error we encountered -- otherwise, we're
54+
// struggling.
55+
if let Err(err) = std::fs::rename(tmp_seed_dir.path(), &desired_seed_dir) {
56+
if !desired_seed_dir.exists() {
57+
panic!("Cannot rename seed directory for CockroachDB: {err}");
58+
}
59+
}
60+
61+
(desired_seed_dir, SeedDirectoryStatus::Created)
62+
}
63+
64+
#[tokio::main]
65+
async fn main() {
66+
// TODO: dropshot is v heavyweight for this, we should be able to pull in a
67+
// smaller binary
68+
let logctx = LogContext::new(
69+
"crdb_seeding",
70+
&ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Info },
71+
);
72+
let (dir, status) = ensure_seed_directory_exists(&logctx.log).await;
73+
match status {
74+
SeedDirectoryStatus::Created => {
75+
slog::info!(logctx.log, "Created seed directory: `{dir}`");
76+
}
77+
SeedDirectoryStatus::Existing => {
78+
slog::info!(logctx.log, "Using existing seed directory: `{dir}`");
79+
}
80+
}
81+
if let Ok(env_path) = std::env::var("NEXTEST_ENV") {
82+
let mut file = std::fs::File::create(&env_path)
83+
.expect("failed to open NEXTEST_ENV file");
84+
writeln!(file, "CRDB_SEED_DIR={dir}")
85+
.expect("failed to write to NEXTEST_ENV file");
86+
} else {
87+
slog::warn!(
88+
logctx.log,
89+
"NEXTEST_ENV not set (is this script running under nextest?)"
90+
);
91+
}
92+
}

nexus/test-utils/Cargo.toml

-5
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,3 @@ tempfile.workspace = true
3838
trust-dns-proto.workspace = true
3939
trust-dns-resolver.workspace = true
4040
uuid.workspace = true
41-
42-
[build-dependencies]
43-
dropshot.workspace = true
44-
omicron-test-utils.workspace = true
45-
tokio.workspace = true

nexus/test-utils/build.rs

-37
This file was deleted.

nexus/test-utils/src/db.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44

55
//! Database testing facilities.
66
7+
use camino::Utf8PathBuf;
78
use omicron_test_utils::dev;
89
use slog::Logger;
9-
use std::path::PathBuf;
1010

1111
/// Path to the "seed" CockroachDB directory.
1212
///
@@ -16,8 +16,11 @@ use std::path::PathBuf;
1616
/// By creating a "seed" version of the database, we can cut down
1717
/// on the time spent performing this operation. Instead, we opt
1818
/// to copy the database from this seed location.
19-
fn seed_dir() -> PathBuf {
20-
PathBuf::from(concat!(env!("OUT_DIR"), "/crdb-base"))
19+
fn seed_dir() -> Utf8PathBuf {
20+
// The setup script should set this environment variable.
21+
let seed_dir = std::env::var("CRDB_SEED_DIR")
22+
.expect("CRDB_SEED_DIR missing -- are you running this test with `cargo nextest run`?");
23+
seed_dir.into()
2124
}
2225

2326
/// Wrapper around [`dev::test_setup_database`] which uses a a

test-utils/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ license = "MPL-2.0"
66

77
[dependencies]
88
anyhow.workspace = true
9+
camino.workspace = true
910
dropshot.workspace = true
1011
futures.workspace = true
1112
headers.workspace = true

test-utils/src/dev/mod.rs

+10-8
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,14 @@ pub mod poll;
1212
pub mod test_cmds;
1313

1414
use anyhow::Context;
15+
use camino::Utf8Path;
16+
use camino::Utf8PathBuf;
1517
pub use dropshot::test_util::LogContext;
1618
use dropshot::ConfigLogging;
1719
use dropshot::ConfigLoggingIfExists;
1820
use dropshot::ConfigLoggingLevel;
1921
use slog::Logger;
20-
use std::path::{Path, PathBuf};
22+
use std::path::Path;
2123

2224
// Helper for copying all the files in one directory to another.
2325
fn copy_dir(
@@ -77,22 +79,22 @@ pub enum StorageSource {
7779
/// Do not populate anything. This is primarily used for migration testing.
7880
DoNotPopulate,
7981
/// Populate the latest version of the database.
80-
PopulateLatest { output_dir: PathBuf },
82+
PopulateLatest { output_dir: Utf8PathBuf },
8183
/// Copy the database from a seed directory, which has previously
8284
/// been created with `PopulateLatest`.
83-
CopyFromSeed { input_dir: PathBuf },
85+
CopyFromSeed { input_dir: Utf8PathBuf },
8486
}
8587

8688
/// Creates a [`db::CockroachInstance`] with a populated storage directory.
8789
///
8890
/// This is intended to optimize subsequent calls to [`test_setup_database`]
8991
/// by reducing the latency of populating the storage directory.
90-
pub async fn test_setup_database_seed(log: &Logger, dir: &Path) {
91-
let _ = std::fs::remove_dir_all(&dir);
92-
std::fs::create_dir_all(&dir).unwrap();
92+
pub async fn test_setup_database_seed(log: &Logger, dir: &Utf8Path) {
93+
let _ = std::fs::remove_dir_all(dir);
94+
std::fs::create_dir_all(dir).unwrap();
9395
let mut db = setup_database(
9496
log,
95-
StorageSource::PopulateLatest { output_dir: dir.to_path_buf() },
97+
StorageSource::PopulateLatest { output_dir: dir.to_owned() },
9698
)
9799
.await;
98100
db.cleanup().await.unwrap();
@@ -148,7 +150,7 @@ async fn setup_database(
148150
StorageSource::CopyFromSeed { input_dir } => {
149151
info!(&log,
150152
"cockroach: copying from seed directory ({}) to storage directory ({})",
151-
input_dir.to_string_lossy(), starter.store_dir().to_string_lossy(),
153+
input_dir, starter.store_dir().to_string_lossy(),
152154
);
153155
copy_dir(input_dir, starter.store_dir())
154156
.expect("Cannot copy storage from seed directory");

0 commit comments

Comments
 (0)