Skip to content

Add ClickHouse settings round-trip to service integration test #176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sdairs merged 3 commits into from
May 18, 2026
Merged

Add ClickHouse settings round-trip to service integration test #176

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c828cdc
Add ClickHouse settings round-trip phase to service integration test
sdairs May 15, 2026
283fa79
Make ClickHouse settings round-trip resilient to schema variance
sdairs May 15, 2026
290b6bd
Merge remote-tracking branch 'origin/main' into issue-161-clickhouse-…
sdairs May 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions crates/clickhouse-cloud-api/tests/common/support.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,13 @@ impl FailureRecorder {

// ── Cleanup Registry ─────────────────────────────────────────────────

#[derive(Debug, Clone)]
pub struct ClickhouseSettingRestore {
pub service_id: String,
pub setting_name: String,
pub original_value: String,
}

#[derive(Default)]
pub struct CleanupRegistry {
service_ids: Vec<String>,
Expand All @@ -372,6 +379,7 @@ pub struct CleanupRegistry {
/// being deleted, table drops are redundant but harmless.
tables: Vec<String>,
api_key_ids: Vec<String>,
clickhouse_setting_restores: Vec<ClickhouseSettingRestore>,
}

impl CleanupRegistry {
Expand Down Expand Up @@ -433,6 +441,25 @@ impl CleanupRegistry {
.retain(|registered| registered != key_id);
}

pub fn register_clickhouse_setting_restore(
&mut self,
service_id: impl Into<String>,
setting_name: impl Into<String>,
original_value: impl Into<String>,
) {
self.clickhouse_setting_restores.push(ClickhouseSettingRestore {
service_id: service_id.into(),
setting_name: setting_name.into(),
original_value: original_value.into(),
});
}

pub fn unregister_clickhouse_setting_restore(&mut self, service_id: &str, setting_name: &str) {
self.clickhouse_setting_restores.retain(|restore| {
!(restore.service_id == service_id && restore.setting_name == setting_name)
});
}

pub async fn cleanup(
&mut self,
client: &Client,
Expand All @@ -443,6 +470,41 @@ impl CleanupRegistry {
) -> Result<(), String> {
let mut failures = Vec::new();

// Restore any ClickHouse settings before deleting the service that owns
// them. If the service is already gone (e.g. test deleted it as part of
// its body) the restore call will 404 and is skipped.
while let Some(restore) = self.clickhouse_setting_restores.pop() {
// The `settings` field on the API is a JSON-encoded string; build
// it with serde_json so quotes / backslashes in the original value
// round-trip correctly.
let inner = match serde_json::to_string(&serde_json::json!({
restore.setting_name.clone(): restore.original_value.clone(),
})) {
Ok(s) => s,
Err(e) => {
failures.push(format!(
"serialize clickhouse setting restore body for {} on {}: {}",
restore.setting_name, restore.service_id, e
));
continue;
}
};
let body = ServiceClickhouseSettingsPatchRequest {
settings: Some(inner),
};
match client
.service_clickhouse_settings_update(org_id, &restore.service_id, &body)
.await
{
Ok(_) => {}
Err(clickhouse_cloud_api::Error::Api { status: 404, .. }) => {}
Err(e) => failures.push(format!(
"restore clickhouse setting {} on service {}: {}",
restore.setting_name, restore.service_id, e
)),
}
}

// API keys are cleaned up first; they belong to the org, not a
// specific service, so they outlive service deletion if leaked.
while let Some(key_id) = self.api_key_ids.pop() {
Expand Down
279 changes: 276 additions & 3 deletions crates/clickhouse-cloud-api/tests/integration_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,280 @@ async fn cloud_service_crud_lifecycle() -> TestResult<()> {
})
.await?;

// ── 5. IP Access ─────────────────────────────────────────────
// ── 5. ClickHouse Settings ───────────────────────────────────
//
// Round-trip a service-level ClickHouse setting through the four
// settings endpoints (`schema`, `list`, `update`, `get`) and capture
// the original value so cleanup can restore it.
//
// The schema endpoint does not expose a "restartRequired" flag, so we
// pick from a curated allowlist of well-known runtime-changeable
// settings (no restart required). We intersect the allowlist with the
// schema returned at test time, so we only touch a setting the cloud
// control plane currently advertises as configurable. If none of the
// allowlisted settings appear in the schema this phase records a
// non-blocking failure rather than guessing at an unknown setting.

log_phase("ClickHouse Settings");

let settings_schema = failures
.run(
&ctx,
StepKind::NonBlocking,
"clickhouse settings schema get",
|| {
let client = client.clone();
let org_id = ctx.org_id.clone();
let service_id = service_id.clone();
async move {
let resp = client
.service_clickhouse_settings_schema_get(&org_id, &service_id)
.await?;
let schema = resp
.result
.ok_or("clickhouse settings schema returned no result")?;
if schema.settings.is_empty() {
return Err("clickhouse settings schema returned no entries".into());
}
Ok(schema)
}
},
)
.await?;

if let Some(schema) = settings_schema {
// Curated allowlist of ClickHouse settings that are safe to mutate
// on a running service and do not require a restart. All of these
// are per-query / per-user runtime knobs — changing them does not
// bounce the server. Ordered by preference; first match wins.
//
// Hardcoded because the schema endpoint does not carry a
// restart-required marker today; using a curated list is the
// conservative alternative to picking blindly. The cloud schema
// exposes a curated subset of OSS settings, so we list multiple
// alternatives — if the cloud control plane stops exposing one,
// the next match is used.
const NO_RESTART_CANDIDATES: &[&str] = &[
"max_concurrent_queries_for_user",
"max_threads",
"max_memory_usage_for_user",
"min_insert_block_size_rows",
"min_insert_block_size_bytes",
"max_insert_block_size",
"max_partitions_per_insert_block",
"max_block_size",
"max_concurrent_queries",
"max_concurrent_select_queries",
"max_concurrent_insert_queries",
"max_execution_time",
"max_result_rows",
];

let chosen = NO_RESTART_CANDIDATES.iter().find_map(|name| {
schema
.settings
.iter()
.find(|entry| entry.name == *name)
.cloned()
});

if let Some(entry) = chosen {
let setting_name = entry.name.clone();
eprintln!(" chose setting: {setting_name} (type: {})", entry.r#type);

let list_resp = failures
.run(
&ctx,
StepKind::NonBlocking,
"clickhouse settings list get",
|| {
let client = client.clone();
let org_id = ctx.org_id.clone();
let service_id = service_id.clone();
async move {
let resp = client
.service_clickhouse_settings_list_get(
&org_id,
&service_id,
)
.await?;
let list = resp.result.ok_or(
"clickhouse settings list returned no result",
)?;
if list.settings.is_empty() {
return Err(
"clickhouse settings list returned no entries".into(),
);
}
Ok(list)
}
},
)
.await?;

let original_value = list_resp.as_ref().and_then(|list| {
list.settings
.iter()
.find(|s| s.name == setting_name)
.map(|s| s.value.clone())
});

if let Some(original) = original_value {
eprintln!(" current value: {original}");

// Pick a new numeric value that differs from the current
// one. The candidates are all integer-typed settings, so
// we parse the current value as an integer; if parsing
// fails we bail to the next pre-set safe value below.
let new_value = match original.parse::<u64>() {
Ok(0) => "1".to_string(),
Ok(n) => (n.saturating_add(1)).to_string(),
Err(_) => "1".to_string(),
};

// Register the restore BEFORE attempting the mutation so
// a failed mid-mutation still triggers a cleanup attempt.
cleanup.register_clickhouse_setting_restore(
service_id.clone(),
setting_name.clone(),
original.clone(),
);

// The `settings` field on the API is a JSON-encoded string
// (the spec example is "{\"compatibility\":\"24.8\"}"). Build
// it with serde_json so the inner JSON escapes correctly
// regardless of what the setting name/value look like.
let patch_body_settings = serde_json::to_string(
&serde_json::json!({ setting_name.clone(): new_value.clone() }),
)?;
let update_ok = failures
.run(
&ctx,
StepKind::NonBlocking,
"clickhouse settings update",
|| {
let client = client.clone();
let org_id = ctx.org_id.clone();
let service_id = service_id.clone();
let body = ServiceClickhouseSettingsPatchRequest {
settings: Some(patch_body_settings.clone()),
};
async move {
let resp = client
.service_clickhouse_settings_update(
&org_id,
&service_id,
&body,
)
.await?;
if resp.result.is_none() {
return Err(
"clickhouse settings update returned no result"
.into(),
);
}
Ok(())
}
},
)
.await?;

if update_ok.is_some() {
failures
.run(
&ctx,
StepKind::NonBlocking,
"clickhouse setting get reflects update",
|| {
let client = client.clone();
let org_id = ctx.org_id.clone();
let service_id = service_id.clone();
let setting_name = setting_name.clone();
let expected = new_value.clone();
let interval = ctx.poll_interval;
async move {
// The control plane may take a few
// seconds to propagate the change to
// the per-setting GET endpoint, so
// poll briefly rather than asserting
// on the first read.
poll_until(
"clickhouse setting reflects update",
std::time::Duration::from_secs(60),
interval,
|| {
let client = client.clone();
let org_id = org_id.clone();
let service_id = service_id.clone();
let setting_name = setting_name.clone();
let expected = expected.clone();
async move {
let resp = client
.service_clickhouse_setting_get(
&org_id,
&service_id,
&setting_name,
)
.await?;
let got = resp.result.ok_or(
"clickhouse setting get returned no result",
)?;
if got.value == expected {
Ok(Some(()))
} else {
Ok(None)
}
}
},
)
.await
}
},
)
.await?;
}
} else {
failures
.run(
&ctx,
StepKind::NonBlocking,
"clickhouse settings round-trip: capture original value",
|| {
let setting_name = setting_name.clone();
async move {
let err: Box<dyn std::error::Error> = format!(
"setting {setting_name} not present in settings list — \
cannot capture original value for round-trip"
)
.into();
Err::<(), _>(err)
}
},
)
.await?;
}
} else {
// The schema endpoint was reachable (proven by the prior
// step) but none of the curated no-restart-required
// candidates are exposed. Rather than recording a hard
// failure that would abort the run in fail-fast mode, log
// the schema's setting names so the allowlist can be
// updated, and skip the mutation phase. The earlier
// `clickhouse settings schema get` step still records
// coverage of the schema endpoint.
let exposed: Vec<&str> =
schema.settings.iter().map(|s| s.name.as_str()).collect();
eprintln!(
" SKIP clickhouse settings round-trip: none of {:?} matched the \
{} settings the cloud schema currently exposes: {:?}",
NO_RESTART_CANDIDATES,
exposed.len(),
exposed,
);
}
}

// ── 6. IP Access ─────────────────────────────────────────────

log_phase("IP Access");
failures
Expand Down Expand Up @@ -1114,7 +1387,7 @@ async fn cloud_service_crud_lifecycle() -> TestResult<()> {
)
.await?;

// ── 6. Scaling ───────────────────────────────────────────────
// ── 7. Scaling ───────────────────────────────────────────────

log_phase("Scaling");
failures
Expand Down Expand Up @@ -1213,7 +1486,7 @@ async fn cloud_service_crud_lifecycle() -> TestResult<()> {
})
.await?;

// ── 7. Delete ────────────────────────────────────────────────
// ── 8. Delete ────────────────────────────────────────────────

log_phase("Delete");

Expand Down
Loading