From 1c7de76c3a9672dfb81313d15953b4bccb5dbd94 Mon Sep 17 00:00:00 2001 From: Rain Date: Wed, 10 Jan 2024 17:30:27 -0800 Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?= =?UTF-8?q?l=20version?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.5 --- .../buildomat/jobs/build-and-test-helios.sh | 2 +- .../buildomat/jobs/build-and-test-linux.sh | 2 +- Cargo.lock | 110 ++++++++++------ Cargo.toml | 5 +- dev-tools/omicron-dev/src/bin/omicron-dev.rs | 32 +++-- nexus/benches/setup_benchmark.rs | 6 +- nexus/test-utils/src/lib.rs | 10 +- oximeter/db/src/client.rs | 27 ++-- test-utils/Cargo.toml | 1 + test-utils/src/dev/clickhouse.rs | 121 ++++++++++++++++-- workspace-hack/Cargo.toml | 4 +- 11 files changed, 234 insertions(+), 86 deletions(-) diff --git a/.github/buildomat/jobs/build-and-test-helios.sh b/.github/buildomat/jobs/build-and-test-helios.sh index f9722a2b92..2c7a1f884d 100755 --- a/.github/buildomat/jobs/build-and-test-helios.sh +++ b/.github/buildomat/jobs/build-and-test-helios.sh @@ -5,7 +5,7 @@ #: target = "helios-2.0" #: rust_toolchain = "1.72.1" #: output_rules = [ -#: "/var/tmp/omicron_tmp/*", +#: "%/var/tmp/omicron_tmp/*", #: "!/var/tmp/omicron_tmp/crdb-base*", #: "!/var/tmp/omicron_tmp/rustc*", #: ] diff --git a/.github/buildomat/jobs/build-and-test-linux.sh b/.github/buildomat/jobs/build-and-test-linux.sh index 715effd080..4f4ebc1d8a 100755 --- a/.github/buildomat/jobs/build-and-test-linux.sh +++ b/.github/buildomat/jobs/build-and-test-linux.sh @@ -5,7 +5,7 @@ #: target = "ubuntu-22.04" #: rust_toolchain = "1.72.1" #: output_rules = [ -#: "/var/tmp/omicron_tmp/*", +#: "%/var/tmp/omicron_tmp/*", #: "!/var/tmp/omicron_tmp/crdb-base*", #: "!/var/tmp/omicron_tmp/rustc*", #: ] diff --git a/Cargo.lock b/Cargo.lock index a4157829af..ce2392e2d5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1836,7 +1836,7 @@ dependencies = [ "dns-service-client", "dropshot", "expectorate", - "http", + "http 0.2.11", "omicron-test-utils", "omicron-workspace-hack", "openapi-lint", @@ -1868,7 +1868,7 @@ name = "dns-service-client" version = "0.1.0" dependencies = [ "chrono", - "http", + "http 0.2.11", "omicron-workspace-hack", "progenitor", "reqwest", @@ -1906,7 +1906,7 @@ dependencies = [ "anyhow", "chrono", "futures", - "http", + "http 0.2.11", "ipnetwork", "omicron-workspace-hack", "omicron-zone-package", @@ -1928,7 +1928,7 @@ dependencies = [ [[package]] name = "dropshot" version = "0.9.1-dev" -source = "git+https://github.com/oxidecomputer/dropshot?branch=main#ff87a0175a6c8ce4462cfe7486edd7000f01be6e" +source = "git+https://github.com/oxidecomputer//dropshot?rev=6f912a16aa7137303531d595796e56f908ba8a4c#6f912a16aa7137303531d595796e56f908ba8a4c" dependencies = [ "async-stream", "async-trait", @@ -1941,7 +1941,7 @@ dependencies = [ "form_urlencoded", "futures", "hostname", - "http", + "http 0.2.11", "hyper", "indexmap 2.1.0", "multer", @@ -1950,7 +1950,7 @@ dependencies = [ "percent-encoding", "proc-macro2", "rustls", - "rustls-pemfile", + "rustls-pemfile 2.0.0", "schemars", "serde", "serde_json", @@ -1974,7 +1974,7 @@ dependencies = [ [[package]] name = "dropshot_endpoint" version = "0.9.1-dev" -source = "git+https://github.com/oxidecomputer/dropshot?branch=main#ff87a0175a6c8ce4462cfe7486edd7000f01be6e" +source = "git+https://github.com/oxidecomputer//dropshot?rev=6f912a16aa7137303531d595796e56f908ba8a4c#6f912a16aa7137303531d595796e56f908ba8a4c" dependencies = [ "proc-macro2", "quote", @@ -2112,7 +2112,7 @@ dependencies = [ "async-trait", "base64", "chrono", - "http", + "http 0.2.11", "hyper", "omicron-sled-agent", "omicron-test-utils", @@ -2749,7 +2749,7 @@ dependencies = [ "futures-core", "futures-sink", "futures-util", - "http", + "http 0.2.11", "indexmap 1.9.3", "slab", "tokio", @@ -2802,7 +2802,7 @@ dependencies = [ "base64", "bytes", "headers-core", - "http", + "http 0.2.11", "httpdate", "mime", "sha1", @@ -2814,7 +2814,7 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e7f66481bfee273957b1f20485a4ff3362987f85b2c236580d81b4eb7a326429" dependencies = [ - "http", + "http 0.2.11", ] [[package]] @@ -2930,6 +2930,17 @@ dependencies = [ "itoa", ] +[[package]] +name = "http" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b32afd38673a8016f7c9ae69e5af41a58f81b1d31689040f2f1959594ce194ea" +dependencies = [ + "bytes", + "fnv", + "itoa", +] + [[package]] name = "http-body" version = "0.4.5" @@ -2937,7 +2948,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d5f38f16d184e36f2408a55281cd658ecbd3ca05cce6d6510a176eca393e26d1" dependencies = [ "bytes", - "http", + "http 0.2.11", "pin-project-lite", ] @@ -2970,7 +2981,7 @@ dependencies = [ "crossbeam-channel", "form_urlencoded", "futures", - "http", + "http 0.2.11", "hyper", "log", "once_cell", @@ -3057,7 +3068,7 @@ dependencies = [ "futures-core", "futures-util", "h2", - "http", + "http 0.2.11", "http-body", "httparse", "httpdate", @@ -3077,7 +3088,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ec3efd23720e2049821a693cbc7e65ea87c72f1c58ff2f9522ff332b1491e590" dependencies = [ "futures-util", - "http", + "http 0.2.11", "hyper", "log", "rustls", @@ -3093,7 +3104,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "318ca89e4827e7fe4ddd2824f52337239796ae8ecc761a663324407dc3d8d7e7" dependencies = [ "futures-util", - "http", + "http 0.2.11", "http-range", "httpdate", "hyper", @@ -3302,7 +3313,7 @@ dependencies = [ "futures", "hex", "hex-literal", - "http", + "http 0.2.11", "illumos-utils", "installinator-artifact-client", "installinator-common", @@ -3999,14 +4010,14 @@ dependencies = [ [[package]] name = "multer" -version = "2.1.0" +version = "3.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01acbdc23469fd8fe07ab135923371d5f5a422fbf9c522158677c8eb15bc51c2" +checksum = "a15d522be0a9c3e46fd2632e272d178f56387bdb5c9fbb3a36c649062e9b5219" dependencies = [ "bytes", "encoding_rs", "futures-util", - "http", + "http 1.0.0", "httparse", "log", "memchr", @@ -4150,7 +4161,7 @@ dependencies = [ "futures", "gateway-client", "headers", - "http", + "http 0.2.11", "hyper", "hyper-rustls", "internal-dns", @@ -4269,7 +4280,7 @@ dependencies = [ "gateway-messages", "gateway-test-utils", "headers", - "http", + "http 0.2.11", "hyper", "internal-dns", "nexus-db-queries", @@ -4618,7 +4629,7 @@ dependencies = [ "expectorate", "futures", "hex", - "http", + "http 0.2.11", "ipnetwork", "libc", "macaddr", @@ -4709,7 +4720,7 @@ dependencies = [ "gateway-sp-comms", "gateway-test-utils", "hex", - "http", + "http 0.2.11", "hyper", "illumos-utils", "ipcc-key-value", @@ -4767,7 +4778,7 @@ dependencies = [ "gateway-test-utils", "headers", "hex", - "http", + "http 0.2.11", "httptest", "hubtools", "hyper", @@ -4979,7 +4990,7 @@ dependencies = [ "glob", "guppy", "hex", - "http", + "http 0.2.11", "hyper", "hyper-staticfile", "illumos-utils", @@ -5051,7 +5062,7 @@ dependencies = [ "filetime", "headers", "hex", - "http", + "http 0.2.11", "libc", "omicron-common", "omicron-workspace-hack", @@ -5069,6 +5080,7 @@ dependencies = [ "tokio", "tokio-postgres", "usdt", + "walkdir", ] [[package]] @@ -5384,7 +5396,7 @@ dependencies = [ "base64", "chrono", "futures", - "http", + "http 0.2.11", "hyper", "omicron-workspace-hack", "progenitor", @@ -5539,7 +5551,7 @@ dependencies = [ "chrono", "dropshot", "futures", - "http", + "http 0.2.11", "kstat-rs", "omicron-workspace-hack", "oximeter", @@ -6247,7 +6259,7 @@ source = "git+https://github.com/oxidecomputer/progenitor?branch=main#9339b57628 dependencies = [ "getopts", "heck 0.4.1", - "http", + "http 0.2.11", "indexmap 2.1.0", "openapiv3", "proc-macro2", @@ -6745,7 +6757,7 @@ dependencies = [ "futures-core", "futures-util", "h2", - "http", + "http 0.2.11", "http-body", "hyper", "hyper-rustls", @@ -6759,7 +6771,7 @@ dependencies = [ "percent-encoding", "pin-project-lite", "rustls", - "rustls-pemfile", + "rustls-pemfile 1.0.3", "serde", "serde_json", "serde_urlencoded", @@ -7097,7 +7109,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a9aace74cb666635c918e9c12bc0d348266037aa8eb599b5cba565709a8dff00" dependencies = [ "openssl-probe", - "rustls-pemfile", + "rustls-pemfile 1.0.3", "schannel", "security-framework", ] @@ -7111,6 +7123,22 @@ dependencies = [ "base64", ] +[[package]] +name = "rustls-pemfile" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35e4980fa29e4c4b212ffb3db068a564cbf560e51d3944b7c88bd8bf5bec64f4" +dependencies = [ + "base64", + "rustls-pki-types", +] + +[[package]] +name = "rustls-pki-types" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9e9d979b3ce68192e42760c7810125eb6cf2ea10efae545a156063e61f314e2a" + [[package]] name = "rustls-webpki" version = "0.101.7" @@ -7374,9 +7402,9 @@ dependencies = [ [[package]] name = "serde" -version = "1.0.194" +version = "1.0.195" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0b114498256798c94a0689e1a15fec6005dee8ac1f41de56404b67afc2a4b773" +checksum = "63261df402c67811e9ac6def069e4786148c4563f4b50fd4bf30aa370d626b02" dependencies = [ "serde_derive", ] @@ -7412,9 +7440,9 @@ dependencies = [ [[package]] name = "serde_derive" -version = "1.0.194" +version = "1.0.195" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a3385e45322e8f9931410f01b3031ec534c3947d0e94c18049af4d9f9907d4e0" +checksum = "46fe8f8603d81ba86327b23a2e9cdf49e1255fb94a4c5f297f6ee0547178ea2c" dependencies = [ "proc-macro2", "quote", @@ -7812,9 +7840,9 @@ dependencies = [ [[package]] name = "slog-bunyan" -version = "2.4.0" +version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "440fd32d0423c31e4f98d76c0b62ebdb847f905aa07357197e9b41ac620af97d" +checksum = "dcaaf6e68789d3f0411f1e72bc443214ef252a1038b6e344836e50442541f190" dependencies = [ "hostname", "slog", @@ -9184,7 +9212,7 @@ dependencies = [ "byteorder", "bytes", "data-encoding", - "http", + "http 0.2.11", "httparse", "log", "rand 0.8.5", @@ -9839,7 +9867,7 @@ dependencies = [ "gateway-messages", "gateway-test-utils", "hex", - "http", + "http 0.2.11", "hubtools", "hyper", "illumos-utils", diff --git a/Cargo.toml b/Cargo.toml index b2d0e406da..b20db5de7d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -559,8 +559,9 @@ opt-level = 3 # It's common during development to use a local copy of various complex # dependencies. If you want to use those, uncomment one of these blocks. # -#[patch."https://github.com/oxidecomputer/dropshot"] -#dropshot = { path = "../dropshot/dropshot" } +[patch."https://github.com/oxidecomputer/dropshot"] +# extra slash to trick Cargo into thinking this is a different source +dropshot = { git = "https://github.com/oxidecomputer//dropshot", rev = "6f912a16aa7137303531d595796e56f908ba8a4c" } #[patch.crates-io] #steno = { path = "../steno" } #[patch."https://github.com/oxidecomputer/propolis"] diff --git a/dev-tools/omicron-dev/src/bin/omicron-dev.rs b/dev-tools/omicron-dev/src/bin/omicron-dev.rs index bbff4f6fe5..0eb421478c 100644 --- a/dev-tools/omicron-dev/src/bin/omicron-dev.rs +++ b/dev-tools/omicron-dev/src/bin/omicron-dev.rs @@ -9,6 +9,7 @@ use camino::Utf8Path; use camino::Utf8PathBuf; use clap::Args; use clap::Parser; +use dropshot::test_util::LogContext; use futures::stream::StreamExt; use nexus_test_interface::NexusServer; use omicron_common::cmd::fatal; @@ -270,22 +271,32 @@ struct ChRunArgs { } async fn cmd_clickhouse_run(args: &ChRunArgs) -> Result<(), anyhow::Error> { + let logctx = LogContext::new( + "omicron-dev", + &dropshot::ConfigLogging::StderrTerminal { + level: dropshot::ConfigLoggingLevel::Info, + }, + ); if args.replicated { - start_replicated_cluster().await?; + start_replicated_cluster(&logctx).await?; } else { - start_single_node(args.port).await?; + start_single_node(&logctx, args.port).await?; } Ok(()) } -async fn start_single_node(port: u16) -> Result<(), anyhow::Error> { +async fn start_single_node( + logctx: &LogContext, + port: u16, +) -> Result<(), anyhow::Error> { // Start a stream listening for SIGINT let signals = Signals::new(&[SIGINT]).expect("failed to wait for SIGINT"); let mut signal_stream = signals.fuse(); // Start the database server process, possibly on a specific port let mut db_instance = - dev::clickhouse::ClickHouseInstance::new_single_node(port).await?; + dev::clickhouse::ClickHouseInstance::new_single_node(logctx, port) + .await?; println!( "omicron-dev: running ClickHouse with full command:\n\"clickhouse {}\"", db_instance.cmdline().join(" ") @@ -331,7 +342,9 @@ async fn start_single_node(port: u16) -> Result<(), anyhow::Error> { Ok(()) } -async fn start_replicated_cluster() -> Result<(), anyhow::Error> { +async fn start_replicated_cluster( + logctx: &LogContext, +) -> Result<(), anyhow::Error> { // Start a stream listening for SIGINT let signals = Signals::new(&[SIGINT]).expect("failed to wait for SIGINT"); let mut signal_stream = signals.fuse(); @@ -345,9 +358,12 @@ async fn start_replicated_cluster() -> Result<(), anyhow::Error> { .as_path() .join("../../oximeter/db/src/configs/keeper_config.xml"); - let mut cluster = - dev::clickhouse::ClickHouseCluster::new(replica_config, keeper_config) - .await?; + let mut cluster = dev::clickhouse::ClickHouseCluster::new( + logctx, + replica_config, + keeper_config, + ) + .await?; println!( "omicron-dev: running ClickHouse cluster with configuration files:\n \ replicas: {}\n keepers: {}", diff --git a/nexus/benches/setup_benchmark.rs b/nexus/benches/setup_benchmark.rs index 304ccc8325..5e87151512 100644 --- a/nexus/benches/setup_benchmark.rs +++ b/nexus/benches/setup_benchmark.rs @@ -28,8 +28,12 @@ async fn do_crdb_setup() { // Wraps exclusively the ClickhouseDB portion of setup/teardown. async fn do_clickhouse_setup() { + let cfg = nexus_test_utils::load_test_config(); + let logctx = LogContext::new("clickhouse_setup", &cfg.pkg.log); let mut clickhouse = - dev::clickhouse::ClickHouseInstance::new_single_node(0).await.unwrap(); + dev::clickhouse::ClickHouseInstance::new_single_node(&logctx, 0) + .await + .unwrap(); clickhouse.cleanup().await.unwrap(); } diff --git a/nexus/test-utils/src/lib.rs b/nexus/test-utils/src/lib.rs index c6dc9fefe9..19d5f747d8 100644 --- a/nexus/test-utils/src/lib.rs +++ b/nexus/test-utils/src/lib.rs @@ -387,10 +387,12 @@ impl<'a, N: NexusServer> ControlPlaneTestContextBuilder<'a, N> { pub async fn start_clickhouse(&mut self) { let log = &self.logctx.log; debug!(log, "Starting Clickhouse"); - let clickhouse = - dev::clickhouse::ClickHouseInstance::new_single_node(0) - .await - .unwrap(); + let clickhouse = dev::clickhouse::ClickHouseInstance::new_single_node( + &self.logctx, + 0, + ) + .await + .unwrap(); let port = clickhouse.port(); let zpool_id = Uuid::new_v4(); diff --git a/oximeter/db/src/client.rs b/oximeter/db/src/client.rs index d6ec01d9fc..aa8072e5f7 100644 --- a/oximeter/db/src/client.rs +++ b/oximeter/db/src/client.rs @@ -1399,6 +1399,7 @@ mod tests { use crate::query::field_table_name; use bytes::Bytes; use chrono::Utc; + use dropshot::test_util::LogContext; use omicron_test_utils::dev::clickhouse::{ ClickHouseCluster, ClickHouseInstance, }; @@ -1463,8 +1464,9 @@ mod tests { #[tokio::test] async fn test_single_node() { + let logctx = test_setup_log("test_single_node"); // Let the OS assign a port and discover it after ClickHouse starts - let mut db = ClickHouseInstance::new_single_node(0) + let mut db = ClickHouseInstance::new_single_node(&logctx, 0) .await .expect("Failed to start ClickHouse"); @@ -1637,20 +1639,21 @@ mod tests { db.cleanup().await.expect("Failed to cleanup ClickHouse server"); } - async fn create_cluster() -> ClickHouseCluster { + async fn create_cluster(logctx: &LogContext) -> ClickHouseCluster { let cur_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); let replica_config = cur_dir.as_path().join("src/configs/replica_config.xml"); let keeper_config = cur_dir.as_path().join("src/configs/keeper_config.xml"); - ClickHouseCluster::new(replica_config, keeper_config) + ClickHouseCluster::new(logctx, replica_config, keeper_config) .await .expect("Failed to initialise ClickHouse Cluster") } #[tokio::test] async fn test_replicated() { - let mut cluster = create_cluster().await; + let logctx = test_setup_log("test_replicated"); + let mut cluster = create_cluster(&logctx).await; // Tests that the expected error is returned on a wrong address bad_db_connection_test().await.unwrap(); @@ -4099,7 +4102,7 @@ mod tests { const TEST_NAME: &str = "test_apply_one_schema_upgrade_replicated"; let logctx = test_setup_log(TEST_NAME); let log = &logctx.log; - let mut cluster = create_cluster().await; + let mut cluster = create_cluster(&logctx).await; let address = cluster.replica_1.address; test_apply_one_schema_upgrade_impl(log, address, true).await; @@ -4138,7 +4141,7 @@ mod tests { const TEST_NAME: &str = "test_apply_one_schema_upgrade_single_node"; let logctx = test_setup_log(TEST_NAME); let log = &logctx.log; - let mut db = ClickHouseInstance::new_single_node(0) + let mut db = ClickHouseInstance::new_single_node(&logctx, 0) .await .expect("Failed to start ClickHouse"); let address = SocketAddr::new(Ipv6Addr::LOCALHOST.into(), db.port()); @@ -4152,7 +4155,7 @@ mod tests { let logctx = test_setup_log("test_ensure_schema_with_version_gaps_fails"); let log = &logctx.log; - let mut db = ClickHouseInstance::new_single_node(0) + let mut db = ClickHouseInstance::new_single_node(&logctx, 0) .await .expect("Failed to start ClickHouse"); let address = SocketAddr::new(Ipv6Addr::LOCALHOST.into(), db.port()); @@ -4195,7 +4198,7 @@ mod tests { "test_ensure_schema_with_missing_desired_schema_version_fails", ); let log = &logctx.log; - let mut db = ClickHouseInstance::new_single_node(0) + let mut db = ClickHouseInstance::new_single_node(&logctx, 0) .await .expect("Failed to start ClickHouse"); let address = SocketAddr::new(Ipv6Addr::LOCALHOST.into(), db.port()); @@ -4327,7 +4330,7 @@ mod tests { "test_ensure_schema_walks_through_multiple_steps_single_node"; let logctx = test_setup_log(TEST_NAME); let log = &logctx.log; - let mut db = ClickHouseInstance::new_single_node(0) + let mut db = ClickHouseInstance::new_single_node(&logctx, 0) .await .expect("Failed to start ClickHouse"); let address = SocketAddr::new(Ipv6Addr::LOCALHOST.into(), db.port()); @@ -4345,7 +4348,7 @@ mod tests { "test_ensure_schema_walks_through_multiple_steps_replicated"; let logctx = test_setup_log(TEST_NAME); let log = &logctx.log; - let mut cluster = create_cluster().await; + let mut cluster = create_cluster(&logctx).await; let address = cluster.replica_1.address; test_ensure_schema_walks_through_multiple_steps_impl( log, address, true, @@ -4448,7 +4451,7 @@ mod tests { let logctx = test_setup_log("test_select_all_field_types"); let log = &logctx.log; - let mut db = ClickHouseInstance::new_single_node(0) + let mut db = ClickHouseInstance::new_single_node(&logctx, 0) .await .expect("Failed to start ClickHouse"); let address = SocketAddr::new(Ipv6Addr::LOCALHOST.into(), db.port()); @@ -4479,7 +4482,7 @@ mod tests { async fn test_sql_query_output() { let logctx = test_setup_log("test_sql_query_output"); let log = &logctx.log; - let mut db = ClickHouseInstance::new_single_node(0) + let mut db = ClickHouseInstance::new_single_node(&logctx, 0) .await .expect("Failed to start ClickHouse"); let address = SocketAddr::new(Ipv6Addr::LOCALHOST.into(), db.port()); diff --git a/test-utils/Cargo.toml b/test-utils/Cargo.toml index 7f210134a2..48223e0291 100644 --- a/test-utils/Cargo.toml +++ b/test-utils/Cargo.toml @@ -30,6 +30,7 @@ usdt.workspace = true rcgen.workspace = true regex.workspace = true reqwest.workspace = true +walkdir.workspace = true omicron-workspace-hack.workspace = true [dev-dependencies] diff --git a/test-utils/src/dev/clickhouse.rs b/test-utils/src/dev/clickhouse.rs index c73161eec7..225923779c 100644 --- a/test-utils/src/dev/clickhouse.rs +++ b/test-utils/src/dev/clickhouse.rs @@ -4,13 +4,15 @@ //! Tools for managing ClickHouse during development +use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use std::process::Stdio; use std::time::Duration; use anyhow::{anyhow, Context}; use camino::{Utf8Path, Utf8PathBuf}; -use camino_tempfile::Utf8TempDir; +use camino_tempfile::{Builder, Utf8TempDir}; +use dropshot::test_util::{log_prefix_for_test, LogContext}; use std::net::{IpAddr, Ipv6Addr, SocketAddr}; use thiserror::Error; use tokio::{ @@ -63,8 +65,11 @@ pub enum ClickHouseError { impl ClickHouseInstance { /// Start a new single node ClickHouse server on the given IPv6 port. - pub async fn new_single_node(port: u16) -> Result { - let data_dir = ClickHouseDataDir::new()?; + pub async fn new_single_node( + logctx: &LogContext, + port: u16, + ) -> Result { + let data_dir = ClickHouseDataDir::new(logctx)?; let args = vec![ "server".to_string(), "--log-file".to_string(), @@ -115,6 +120,7 @@ impl ClickHouseInstance { /// Start a new replicated ClickHouse server on the given IPv6 port. pub async fn new_replicated( + logctx: &LogContext, port: u16, tcp_port: u16, interserver_port: u16, @@ -122,7 +128,7 @@ impl ClickHouseInstance { r_number: String, config_path: PathBuf, ) -> Result { - let data_dir = ClickHouseDataDir::new()?; + let data_dir = ClickHouseDataDir::new(logctx)?; let args = vec![ "server".to_string(), "--config-file".to_string(), @@ -181,6 +187,7 @@ impl ClickHouseInstance { /// Start a new ClickHouse keeper on the given IPv6 port. pub async fn new_keeper( + logctx: &LogContext, port: u16, k_id: u16, config_path: PathBuf, @@ -191,7 +198,7 @@ impl ClickHouseInstance { if ![1, 2, 3].contains(&k_id) { return Err(ClickHouseError::InvalidKeeperId.into()); } - let data_dir = ClickHouseDataDir::new()?; + let data_dir = ClickHouseDataDir::new(logctx)?; let args = vec![ "keeper".to_string(), @@ -262,7 +269,7 @@ impl ClickHouseInstance { child.wait().await.context("waiting for child")?; } if let Some(dir) = self.data_dir.take() { - dir.close()?; + dir.close_clean()?; } Ok(()) } @@ -294,10 +301,14 @@ struct ClickHouseDataDir { } impl ClickHouseDataDir { - fn new() -> Result { - // Keepers do not allow a dot in the beginning of the directory, so we must - // use a prefix. - let dir = Utf8TempDir::with_prefix("clickhouse-") + fn new(logctx: &LogContext) -> Result { + // TODO: replace with log_prefix_for_test once + // https://github.com/oxidecomputer/dropshot/pull/878 is merged. + let (parent_dir, prefix) = log_prefix_for_test(logctx.test_name()); + + let dir = Builder::new() + .prefix(&format!("{prefix}-clickhouse-")) + .tempdir_in(parent_dir) .context("failed to create tempdir for ClickHouse data")?; let ret = Self { dir }; @@ -375,9 +386,83 @@ impl ClickHouseDataDir { self.dir.path().join("snapshots/") } - fn close(self) -> Result<(), anyhow::Error> { + fn close_clean(self) -> Result<(), anyhow::Error> { self.dir.close().context("failed to delete ClickHouse data dir") } + + /// Closes this data directory during a test failure, or other unclean + /// shutdown. + /// + /// Removes all files except those in any of the log directories. + fn close_unclean(self) -> Result<(), anyhow::Error> { + let keep_prefixes = vec![ + self.log_path(), + self.err_log_path(), + self.keeper_log_path(), + self.keeper_err_log_path(), + self.keeper_log_storage_path(), + ]; + // Persist this temporary directory since we're going to be doing the + // cleanup ourselves. + let dir = self.dir.into_path(); + + let mut error_paths = BTreeMap::new(); + // contents_first = true ensures that we delete inner files before + // outer directories. + for entry in walkdir::WalkDir::new(&dir).contents_first(true) { + match entry { + Ok(entry) => { + // If it matches any of the prefixes, skip it. + if keep_prefixes + .iter() + .any(|prefix| entry.path().starts_with(prefix)) + { + continue; + } + if entry.file_type().is_dir() { + if let Err(error) = std::fs::remove_dir(entry.path()) { + // Ignore ENOTEMPTY errors because they're likely + // generated from parents of files we've kept, or + // were unable to delete for other reasons. + if error.raw_os_error() != Some(libc::ENOTEMPTY) { + error_paths.insert( + entry.path().to_owned(), + anyhow!(error), + ); + } + } + } else { + if let Err(error) = std::fs::remove_file(entry.path()) { + error_paths.insert( + entry.path().to_owned(), + anyhow!(error), + ); + } + } + } + Err(error) => { + if let Some(path) = error.path() { + error_paths.insert(path.to_owned(), anyhow!(error)); + } + } + } + } + + // Are there any error paths? + if !error_paths.is_empty() { + let error_paths = error_paths + .into_iter() + .map(|(path, error)| format!("- {}: {}", path.display(), error)) + .collect::>(); + let error_paths = error_paths.join("\n"); + return Err(anyhow!( + "failed to clean up ClickHouse data dir:\n{}", + error_paths + )); + } + + Ok(()) + } } impl Drop for ClickHouseInstance { @@ -392,7 +477,9 @@ impl Drop for ClickHouseInstance { let _ = child.start_kill(); } if let Some(dir) = self.data_dir.take() { - let _ = dir.close(); + if let Err(e) = dir.close_unclean() { + eprintln!("{}", e); + } } } } @@ -412,18 +499,20 @@ pub struct ClickHouseCluster { impl ClickHouseCluster { pub async fn new( + logctx: &LogContext, replica_config: PathBuf, keeper_config: PathBuf, ) -> Result { // Start all Keeper coordinator nodes let keeper_amount = 3; let mut keepers = - Self::new_keeper_set(keeper_amount, &keeper_config).await?; + Self::new_keeper_set(logctx, keeper_amount, &keeper_config).await?; // Start all replica nodes let replica_amount = 2; let mut replicas = - Self::new_replica_set(replica_amount, &replica_config).await?; + Self::new_replica_set(logctx, replica_amount, &replica_config) + .await?; let r1 = replicas.swap_remove(0); let r2 = replicas.swap_remove(0); @@ -443,6 +532,7 @@ impl ClickHouseCluster { } pub async fn new_keeper_set( + logctx: &LogContext, keeper_amount: u16, config_path: &PathBuf, ) -> Result, anyhow::Error> { @@ -453,6 +543,7 @@ impl ClickHouseCluster { let k_id = i; let k = ClickHouseInstance::new_keeper( + logctx, k_port, k_id, config_path.clone(), @@ -468,6 +559,7 @@ impl ClickHouseCluster { } pub async fn new_replica_set( + logctx: &LogContext, replica_amount: u16, config_path: &PathBuf, ) -> Result, anyhow::Error> { @@ -480,6 +572,7 @@ impl ClickHouseCluster { let r_name = format!("oximeter_cluster node {}", i); let r_number = format!("0{}", i); let r = ClickHouseInstance::new_replicated( + logctx, r_port, r_tcp_port, r_interserver_port, diff --git a/workspace-hack/Cargo.toml b/workspace-hack/Cargo.toml index 5688e133c0..8646e08c27 100644 --- a/workspace-hack/Cargo.toml +++ b/workspace-hack/Cargo.toml @@ -87,7 +87,7 @@ reqwest = { version = "0.11.22", features = ["blocking", "json", "rustls-tls", " ring = { version = "0.17.7", features = ["std"] } schemars = { version = "0.8.16", features = ["bytes", "chrono", "uuid1"] } semver = { version = "1.0.21", features = ["serde"] } -serde = { version = "1.0.194", features = ["alloc", "derive", "rc"] } +serde = { version = "1.0.195", features = ["alloc", "derive", "rc"] } serde_json = { version = "1.0.111", features = ["raw_value", "unbounded_depth"] } sha2 = { version = "0.10.8", features = ["oid"] } similar = { version = "2.3.0", features = ["inline", "unicode"] } @@ -190,7 +190,7 @@ reqwest = { version = "0.11.22", features = ["blocking", "json", "rustls-tls", " ring = { version = "0.17.7", features = ["std"] } schemars = { version = "0.8.16", features = ["bytes", "chrono", "uuid1"] } semver = { version = "1.0.21", features = ["serde"] } -serde = { version = "1.0.194", features = ["alloc", "derive", "rc"] } +serde = { version = "1.0.195", features = ["alloc", "derive", "rc"] } serde_json = { version = "1.0.111", features = ["raw_value", "unbounded_depth"] } sha2 = { version = "0.10.8", features = ["oid"] } similar = { version = "2.3.0", features = ["inline", "unicode"] }