fix(hosting): deploy ClickHouse from the official image instead of Bitnami - #4249
Conversation
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughClickHouse configuration is updated across self-hosting documentation, Docker Compose, and Helm. Docker now uses the official image, updated credentials and paths, file limits, and a compatible data-path configuration. Helm replaces the Bitnami dependency with custom ClickHouse resources, pinned image settings, explicit ports, probes, persistence, security settings, generated URLs, and updated tests. Documentation records the ClickHouse 25.8+ requirement and external service port key. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🧭 Helm Chart Prerelease PublishedVersion: Install: helm upgrade --install trigger \
oci://ghcr.io/triggerdotdev/charts/trigger \
--version "4.5.9-pr4249.8e28375"
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
e2e7e33 to
4608012
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
ea85282 to
228c4a3
Compare
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…tnami The Bitnami free image catalog is EOL and its frozen bitnamilegacy archive tops out at ClickHouse 25.7.5, below the 25.8 floor the platform requires since v4.5.0. The Docker Compose stack and the Helm chart now run the official clickhouse/clickhouse-server image at 26.2, the same version the platform is developed and tested against. The Helm chart deploys ClickHouse with a chart-owned StatefulSet instead of the Bitnami subchart. Existing deployments keep their data with no manual steps: a config override keeps the on-disk layout compatible with volumes created by the Bitnami-based setup, Compose reuses the same named volume, and the Helm chart automatically adopts the data PVC left behind by the old subchart.
…e inline credentials The chart-deployed ClickHouse now reads CLICKHOUSE_PASSWORD from a chart-owned Secret instead of a plaintext env value in the pod spec, and the CLICKHOUSE_URL helpers percent-encode inline usernames and passwords so special characters no longer produce an unparseable URL.
…ntials urlquery encodes spaces as plus signs, which URL userinfo decoding keeps literal. A shared urlencode helper rewrites them to %20 (a real plus already encodes as %2B), so passwords containing spaces now work.
… restart checksum The data-paths.xml default now lives in a shared helper used by both the ConfigMap and the pod checksum annotation, so changing it in a future chart version restarts the pod (subPath mounts do not update in place).
…lickHouse chart Keeps existing self-hosted deployments working without manual steps: - Default resource requests/limits match the preset the Bitnami subchart applied, so the ClickHouse pod doesn't silently become BestEffort. - nodeSelector, tolerations, and affinity pass through to the StatefulSet for deployments that schedule ClickHouse onto dedicated nodes. - Optional volumePermissions init container fixes data-volume ownership on storage without fsGroup support (NFS, hostPath). - The helm test reads the password from the datastore secret instead of the usually-empty values key, and the pinned-password restart checksum is omitted when the password is auto-generated. - Docs cover updating a pinned CLICKHOUSE_IMAGE_TAG to an official tag and the GitOps existingClaim step in more detail.
Adds the recovery path for GitOps renders that missed existingClaim, the pinned Bitnami image note for the Helm chart, the single-node clustering note, and the one-way rollback caveat for Docker Compose.
…rtup probe The webapp now resolves CLICKHOUSE_PASSWORD through the same auth.existingSecret/existingSecretKey values the bundled server uses, so pointing the chart at a custom credentials secret keeps both sides on the same password. A startup probe gives ClickHouse up to ten minutes to load metadata on first boot with a large adopted data volume before liveness checks begin.
Production upgrade feedback (Tillör, chart
|
… keys When secrets.existingSecret is set the chart generates nothing and reads every application and control-plane key from that Secret. A chart version that starts consuming a new key therefore breaks the webapp rollout with a CreateContainerConfigError partway through, leaving the release half-applied. The pre-install/pre-upgrade validation now looks the Secret up and fails with the full list of missing keys, so a running release is left untouched. The lookup returns nothing under helm template and client-side dry-run, where the check is skipped rather than guessing.
Collects the chart changes that need action before an upgrade: the two secret keys the webapp started reading in 4.5.6, the move of the bundled datastore credentials into the chart-managed Secret (with the old-to-new mapping for consumers outside the chart), and a pointer to the ClickHouse image notes. Also corrects the existingSecret key list, which named the S3 environment variables rather than the keys the chart actually reads.
f7da3f8 to
fbf811c
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… volume Upgrading the bundled ClickHouse from the Bitnami subchart to the official image adopts the old data volume, whose files are owned by the previous uid. The official server runs non-root (uid 101), so on storage where fsGroup can't relabel the volume (hostPath, NFS, local-path - common for self-hosters) it crashes with 'cd: /var/lib/clickhouse/data/: Permission denied'. Run the one-time chown init automatically whenever a volume is adopted (existingClaim set, explicitly or auto-detected); fresh installs create their own data and skip it, and volumePermissions.enabled still forces it. Verified live in kind: a bitnami->official upgrade on local-path storage now comes up with data and auth intact, with no manual flag.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…SE_USER The bundled ClickHouse creates only the user named by CLICKHOUSE_USER (the official image removes the built-in default user when it differs), but the webapp CLICKHOUSE_URL and RUN_REPLICATION_CLICKHOUSE_URL hardcoded 'default', so a custom CLICKHOUSE_USER left the app unable to authenticate while the healthcheck (which already honours the var) stayed green. Derive the username in both URLs from CLICKHOUSE_USER. Also document, for the Helm chart, that inline external ClickHouse credentials are now percent-encoded (store the raw value), and that existingClaim should be pinned once a bitnami volume is adopted so client-side renders stay deterministic.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… not a fixed uid The volume-permissions init container hardcoded chown -R 101:101, ignoring clickhouse.securityContext.runAsUser/runAsGroup. Anyone who overrides the run-as user (e.g. for a restricted cluster policy) got a data directory owned by an account the server no longer runs as, so it failed to start on the adopted volume. Derive the uid/gid from securityContext (defaulting to 101). Also document the transient ReadWriteOnce multi-attach warning during the upgrade window so operators don't abort mid-migration.
|
Ran a full local validation of the ClickHouse migration - docker bitnami→official on the same volume, and a real bitnami→official
Validation, all live: docker migration keeps data + auth (empty/wrong passwords rejected); helm fresh install comes up; helm upgrade in kind on local-path reproduced the crash, then confirmed the auto-init fix - CH came up with the seeded rows intact and the retained auto-generated password authenticating. Auto-gen integration is intact ( Nice PR - the bitnami-off migration is well handled (data-paths compat, PVC adoption, fsGroup). Replies to the individual comments inline. |
|
Also thanks for the review @iFlyinq! ❤️ |
…-walk on every restart Because the adopted-volume condition stays true for the life of the release, the ownership init container runs on every pod restart, and an unconditional recursive chown re-walks the whole database each time - minutes of delay on large or network-backed data. Skip the chown when the volume root is already owned by the run-as user (mirroring fsGroupChangePolicy: OnRootMismatch), so only the first mount after adoption pays the cost.
3 participants
Summary
Self-hosted deployments now run ClickHouse from the official
clickhouse/clickhouse-serverimage instead ofbitnamilegacy/clickhouse. Bitnami's free image catalog is EOL and the frozen legacy archive tops out at ClickHouse 25.7.5, below the 25.8 minimum the platform requires since v4.5.0, which broke every ClickHouse insert on chart-bundled deployments. Both stacks now default to 26.2, the same version the platform is developed and tested against.Existing deployments keep their ClickHouse data with no manual migration.
Fixes #4197.
Details
Docker Compose: the
clickhouseservice uses the official image with its native env vars, plus the recommendednofileulimits. It reuses the same named volume as before: adata-paths.xmlconfig override points ClickHouse at thedata/subdirectory of the volume, which is exactly the layout the Bitnami image used, so old volumes work in place (including SQL-created users) and fresh installs get the identical layout. The service follows the required-secrets model:CLICKHOUSE_PASSWORDmust be set, matching the other services.Helm chart: the Bitnami ClickHouse subchart is replaced by a chart-owned single-node StatefulSet and Service running the official image (non-root, HTTP
/pingprobes, config overrides mounted intoconfig.d, and the samedata-paths.xmllayout compatibility). On upgrade, the chart automatically adopts the data PVC left behind by the old subchart (data-<release>-clickhouse-shard0-0) vialookup, andfsGrouprelabeling handles the uid change on first mount. Both the ClickHouse server and the webapp read the password from the same chart-managed datastore secret (auto-generated and retained across upgrades), so the server credential and the app's connection URL always match. Existingclickhouse.*values keep working:auth(includingexistingSecret/existingSecretKey),persistence(includingglobal.storageClass),resources,secure,external.*,configdFiles, and nownodeSelector/tolerations/affinity. Bitnami-only keys (shards,replicaCount,keeper,resourcesPreset) are gone; defaultresourcesrequests/limits match what the old preset applied. The docs state the 25.8 minimum for bring-your-own ClickHouse.Upgrade caveats
An adversarial review of the upgrade path found a few cohorts that need awareness (all documented):
helm template(no cluster access): PVC auto-detection can't run, soclickhouse.persistence.existingClaimmust be set to the old PVC name or ClickHouse starts on a fresh empty volume. Documented in the values file and the Kubernetes self-hosting docs. Tools that run real helm installs (e.g. Flux) adopt automatically.CLICKHOUSE_IMAGE_TAGpointing at a Bitnami tag must be updated to an official image tag; documented in the Docker self-hosting docs.fsGroupsupport (NFS, hostPath): setclickhouse.volumePermissions.enabled: truefor a one-time ownership-fixing init container.Verification
main(Bitnami), wrote thousands of rows, then brought the same project up with this branch's compose file. The official 26.2 server came up healthy on the same volume with all rows intact, SQL-created users working, and writes succeeding.helm lint,helm template(default values,existingClaimset, external ClickHouse, volumePermissions/scheduling toggles, and the production example) and kubeconform all pass, mirroring the release CI steps. The rendered webapp Deployment and ClickHouse StatefulSet resolve to the same datastore secret key.input_format_json_infer_array_of_dynamic_from_array_of_different_types(the setting that fails on 25.7.5) succeed on the upgraded volume.Upgrade preflight and docs
A production upgrade report on this branch surfaced two hazards that predate this PR — both landed in chart 4.5.6 (#4316) — so they are fixed here rather than left for the next person to hit.
secrets.existingSecretgained two required keys. The webapp started readingPROVIDER_SECRETandCOORDINATOR_SECRET, and whenexistingSecretis set the chart generates nothing, so a missing key only surfaced as aCreateContainerConfigErrorpartway through the webapp rollout. The pre-install/pre-upgrade validation now looks the Secret up and fails with the complete list of missing keys, leaving the running release untouched. It is skipped underhelm templateand client-side dry-run, wherelookupcannot read the cluster.Bundled datastore credentials moved into the chart-managed Secret (
<release>-clickhouse/admin-password→trigger-datastore/clickhouse-admin-password). The chart wires both ends itself, but consumers outside it — maintenance CronJobs, Grafana datasources, secret syncs — have to be repointed. A new## Upgradingsection in the Kubernetes docs carries the old→new mapping, the two new keys, and a pointer to the ClickHouse image notes.The existingSecret key list in the docs also named
OBJECT_STORE_ACCESS_KEY_ID/OBJECT_STORE_SECRET_ACCESS_KEY, which are env var names rather than keys the chart reads; corrected to the real key names and the condition under which they apply.Verified on a throwaway kind cluster with
--dry-run=server: a pre-4.5.6 Secret fails with both key names listed, the documentedkubectl patchclears it, and default values,existingClaim, external ClickHouse, volumePermissions/scheduling and the production example all still render. A realhelm installfollowed by an upgrade against an incomplete Secret aborts with the release still at revision 1 anddeployed.helm lint, the CI render and kubeconform (59 resources, 0 invalid) pass.