Skip to content

Comments

fix(clickhouse): honour cluster mode off when using cloud#252

Closed
pjhampton wants to merge 2 commits intolangfuse:mainfrom
pjhampton:pjhampton/fix-clickhouse-cluster-env
Closed

fix(clickhouse): honour cluster mode off when using cloud#252
pjhampton wants to merge 2 commits intolangfuse:mainfrom
pjhampton:pjhampton/fix-clickhouse-cluster-env

Conversation

@pjhampton
Copy link

@pjhampton pjhampton commented Oct 10, 2025

If you try to use ClickHouse Cloud, you cannot currently set clickhouse.clusterEnabled: false because of this block. You should be able to set it regardless.

{{- if and .Values.clickhouse.deploy ($.Values.clickhouse.replicaCount | int | eq 1) }}
- name: CLICKHOUSE_CLUSTER_ENABLED
  value: "false"
{{- end }}

The workaround is:

langfuse:
  additionalEnv:
    - name: CLICKHOUSE_CLUSTER_ENABLED
      value: "false"

and this PR updates the chart.

IMPORTANT: unit test vibe coded. Please review.


Important

Updates Helm chart to allow disabling ClickHouse cluster mode in cloud environments and adds tests for this behavior.

  • Behavior:
    • Updates condition in _helpers.tpl to set CLICKHOUSE_CLUSTER_ENABLED to false when clickhouse.deploy is false or clickhouse.replicaCount is 1.
    • Ensures CLICKHOUSE_CLUSTER_ENABLED is not set for multi-replica deployments.
  • Tests:
    • Adds clickhouse-cluster_test.yaml to verify CLICKHOUSE_CLUSTER_ENABLED is false for external ClickHouse and single replica deployments.
    • Verifies CLICKHOUSE_CLUSTER_ENABLED is not set for multi-replica deployments.

This description was created by Ellipsis for 807e0eb. You can customize this summary. It will automatically update as commits are pushed.

@pjhampton pjhampton requested a review from Steffen911 as a code owner October 10, 2025 13:14
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Oct 10, 2025
@CLAassistant
Copy link

CLAassistant commented Oct 10, 2025

CLA assistant check
All committers have signed the CLA.

@Steffen911
Copy link
Member

@pjhampton Thank you a lot for raising this. I've implemented another approach via #255 where we evaluate the clusterEnabled flag of the values.yaml to make the decision.

I agree that the opt-out using the additional env is unintuitive, but I'd like to keep the default behaviour that assumes an external ClickHouse is clustered.

With the alternative in place, I'll close this PR.

@Steffen911 Steffen911 closed this Oct 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants