Skip to content

CORE-1505: ci: bump e2e ClickHouse to 25.8 and fix its healthcheck - #2382

Merged
EladBarkay merged 3 commits into
masterfrom
core-1505-clickhouse-e2e-bump-server-image-243-rejects-dbt-clickhouses
Oct 5, 2026
Merged

EladBarkay merged 3 commits into
masterfrom
core-1505-clickhouse-e2e-bump-server-image-243-rejects-dbt-clickhouses

Conversation

@EladBarkay

@EladBarkay EladBarkay commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Fixes CORE-1505 (part of CORE-1502).

test (clickhouse) fails on dbt seed:

Failed to create elementary_tests database due to ClickHouse exception

dbt-clickhouse hides the underlying error, which is:

Code: 115. DB::Exception: Setting lightweight_deletes_sync is neither a builtin setting ... (UNKNOWN_SETTING) (version 24.3.18.7)

dbt-clickhouse 1.10.3 sets lightweight_deletes_sync=3 on its connection. The e2e image clickhouse/clickhouse-server:24.3 (out of support) doesn't know that setting.

Changes

  • Bump the e2e ClickHouse image to 25.8 (current LTS).
  • The healthcheck used curl, which isn't in the image, so it always reported unhealthy. It now uses wget, and the CI start step waits on it with --wait.
  • Changing docker-compose.yml busts the seed cache, so this run seeds from scratch.

Tested locally with dbt-core 1.12.5, dbt-clickhouse 1.10.3 and dbt-data-reliability master:

  • On 24.3, dbt seed reproduces the CI failure.
  • On 25.8, dbt seed passes (22/22), and dbt run fails only on error_model, which matches the workflow's validation.

edr monitor and the report run in CI.

The trino, dremio and sqlserver jobs stay red here until #2380 and #2381 merge.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Updated the automated warehouse test environment to use a newer ClickHouse version and verify that the service is ready before tests proceed.

dbt-clickhouse 1.10.3 sends lightweight_deletes_sync on every query, which
ClickHouse 24.3 (EOL) rejects with UNKNOWN_SETTING, so dbt seed fails with
'Failed to create elementary_tests database due to ClickHouse exception'.
Also replace the curl healthcheck (curl isn't in the image) with wget and
wait for it in CI.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

👋 @EladBarkay
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in this pull request.

@linear

linear Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

CORE-1505

CORE-1502

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1121cfe6-0a83-46c1-92d2-0d67922e7d26
📥 Commits

Reviewing files that changed from the base of the PR and between aec5dca and f8500fc.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: dd487654-5036-410f-80a6-736cd69e9fb8
📥 Commits

Reviewing files that changed from the base of the PR and between 3a2ef5d and 96c311b.

📒 Files selected for processing (2)
  • .github/workflows/test-warehouse.yml
  • tests/e2e_dbt_project/docker-compose.yml

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

The end-to-end project now uses ClickHouse 25.8 with a wget-based ping healthcheck. The warehouse test workflow waits for ClickHouse to become healthy before continuing.

Changes

ClickHouse test setup

Layer / File(s) Summary
ClickHouse image and readiness
.github/workflows/test-warehouse.yml, tests/e2e_dbt_project/docker-compose.yml
The project uses ClickHouse 25.8 and checks its ping endpoint with wget -qO-. The workflow waits for the service to become healthy.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: haritamar

Merge Risk: ⚪ Minimal · up to 96c31

The workflow now waits for ClickHouse readiness using a working healthcheck. The reviewed changes present no identified merge-blocking risk.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 96c31

The inspected changes remain within the end-to-end test environment. Startup now waits for ClickHouse health without changing configured network exposure, credentials, or workflow permissions. No material security regression was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed execution path directly affects the CI runner's ClickHouse test container, its published ports, and its persisted test volume. The inspected diff adds no mounts, capabilities, credentials, or workflow permissions; production reachability was not established.

Trust Boundaries and Controls

  • inferred — The new gate checks service liveness, not downstream identity or authorization. A successful ping cannot prove that the profile's credentials match an externally overridden password or reused volume. This distinction already existed in the probes and credential configuration; the PR does not demonstrate weakened authentication.

Resilience and Maintainability Implications

  • observed — The startup command has no failure-suppression operator. After cache saving, the script independently retries ClickHouse ping up to 30 times and exits with failure if readiness is never observed, despite tolerating the preceding restart command's failure.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the ClickHouse version bump and healthcheck fix, which are the main changes in the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@EladBarkay
EladBarkay enabled auto-merge October 5, 2026 12:18
@EladBarkay
EladBarkay merged commit f8e17b1 into master Oct 5, 2026
32 checks passed
@EladBarkay
EladBarkay deleted the core-1505-clickhouse-e2e-bump-server-image-243-rejects-dbt-clickhouses branch October 5, 2026 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants