Skip to content

Commit

Permalink
Refactor clickhouse admin integration tests (#7017)
Browse files Browse the repository at this point in the history
## Overview

This commit changes the way the clickhouse clusters are spun up for
clickhouse-admin integration tests. Instead of multiple clusters being
spun up for every test, now we start up a single cluster via nextest's
"setup scripts" functionality.
This way, we no longer need to manually hard code so many ports per
test.

## Caveats

Sadly, we still have to hardcode some ports, but at least this way it's
only a set of ports, and we won't need to add more every time we want to
test something new.

In order to get the teardown function working, I had to limit the tests
to run on a single thread. They're a bit slower than before, but nothing
too terrible. Setting up the cluster, running the tests and tearing down
takes ~7s. The time to run the tests themselves is ~3s
  • Loading branch information
karencfv authored Nov 14, 2024
1 parent 59ab88a commit 63bf8cc
Show file tree
Hide file tree
Showing 12 changed files with 286 additions and 178 deletions.
9 changes: 8 additions & 1 deletion .config/nextest.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ fail-fast = false
# invocations of nextest happen.
command = 'cargo run -p crdb-seed --profile test'

[[profile.default.scripts]]
filter = 'package(omicron-clickhouse-admin)'
setup = 'clickhouse-cluster'

[script.clickhouse-cluster]
command = 'cargo run -p clickhouse-cluster-dev'

[test-groups]
# The ClickHouse cluster tests currently rely on a hard-coded set of ports for
# the nodes in the cluster. We would like to relax this in the future, at which
Expand All @@ -39,7 +46,7 @@ live-tests = { max-threads = 1 }
default-filter = 'all() - package(omicron-live-tests) - package(end-to-end-tests)'

[[profile.default.overrides]]
filter = 'package(oximeter-db) and test(replicated)'
filter = 'package(oximeter-db) and test(replicated) + package(omicron-clickhouse-admin)'
test-group = 'clickhouse-cluster'

[[profile.default.overrides]]
Expand Down
29 changes: 28 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ members = [
"certificates",
"clickhouse-admin",
"clickhouse-admin/api",
"clickhouse-admin/test-utils",
"clients/bootstrap-agent-client",
"clients/clickhouse-admin-keeper-client",
"clients/clickhouse-admin-server-client",
Expand All @@ -25,6 +26,7 @@ members = [
"cockroach-admin/types",
"common",
"dev-tools/cert-dev",
"dev-tools/clickhouse-cluster-dev",
"dev-tools/ch-dev",
"dev-tools/crdb-seed",
"dev-tools/db-dev",
Expand Down Expand Up @@ -130,6 +132,7 @@ default-members = [
"clickhouse-admin",
"clickhouse-admin/api",
"clickhouse-admin/types",
"clickhouse-admin/test-utils",
"clients/bootstrap-agent-client",
"clients/clickhouse-admin-keeper-client",
"clients/clickhouse-admin-server-client",
Expand All @@ -150,6 +153,7 @@ default-members = [
"cockroach-admin/types",
"common",
"dev-tools/cert-dev",
"dev-tools/clickhouse-cluster-dev",
"dev-tools/ch-dev",
"dev-tools/crdb-seed",
"dev-tools/db-dev",
Expand Down Expand Up @@ -324,6 +328,7 @@ clickhouse-admin-api = { path = "clickhouse-admin/api" }
clickhouse-admin-keeper-client = { path = "clients/clickhouse-admin-keeper-client" }
clickhouse-admin-server-client = { path = "clients/clickhouse-admin-server-client" }
clickhouse-admin-types = { path = "clickhouse-admin/types" }
clickhouse-admin-test-utils = { path = "clickhouse-admin/test-utils" }
clickward = { git = "https://github.com/oxidecomputer/clickward", rev = "a1b342c2558e835d09e6e39a40d3de798a29c2f" }
cockroach-admin-api = { path = "cockroach-admin/api" }
cockroach-admin-client = { path = "clients/cockroach-admin-client" }
Expand Down
3 changes: 2 additions & 1 deletion clickhouse-admin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,16 @@ omicron-workspace-hack.workspace = true

[dev-dependencies]
clickward.workspace = true
clickhouse-admin-test-utils.workspace = true
dropshot.workspace = true
expectorate.workspace = true
nexus-test-utils.workspace = true
omicron-test-utils.workspace = true
oximeter-db.workspace = true
oximeter-test-utils.workspace = true
openapi-lint.workspace = true
openapiv3.workspace = true
serde_json.workspace = true
slog-term.workspace = true
subprocess.workspace = true
url.workspace = true

Expand Down
16 changes: 16 additions & 0 deletions clickhouse-admin/test-utils/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[package]
name = "clickhouse-admin-test-utils"
version = "0.1.0"
edition = "2021"
license = "MPL-2.0"

[lints]
workspace = true

[dependencies]
camino.workspace = true
clickhouse-admin-types.workspace = true
clickward.workspace = true
dropshot.workspace = true

omicron-workspace-hack.workspace = true
43 changes: 43 additions & 0 deletions clickhouse-admin/test-utils/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// 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/.

//! Integration testing facilities for clickhouse-admin

use camino::Utf8PathBuf;
use clickhouse_admin_types::OXIMETER_CLUSTER;
use clickward::{BasePorts, Deployment, DeploymentConfig};
use dropshot::test_util::{log_prefix_for_test, LogContext};
use dropshot::{ConfigLogging, ConfigLoggingLevel};

pub const DEFAULT_CLICKHOUSE_ADMIN_BASE_PORTS: BasePorts = BasePorts {
keeper: 29000,
raft: 29100,
clickhouse_tcp: 29200,
clickhouse_http: 29300,
clickhouse_interserver_http: 29400,
};

pub fn default_clickhouse_cluster_test_deployment(
path: Utf8PathBuf,
) -> Deployment {
let config = DeploymentConfig {
path,
base_ports: DEFAULT_CLICKHOUSE_ADMIN_BASE_PORTS,
cluster_name: OXIMETER_CLUSTER.to_string(),
};

Deployment::new(config)
}

pub fn default_clickhouse_log_ctx_and_path() -> (LogContext, Utf8PathBuf) {
let logctx = LogContext::new(
"clickhouse_cluster",
&ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Info },
);

let (parent_dir, _prefix) = log_prefix_for_test("clickhouse_cluster");
let path = parent_dir.join("clickward_test");

(logctx, path)
}
Loading

0 comments on commit 63bf8cc

Please sign in to comment.