[HDX] Bump clickhouse-operator to v0.0.6 + fix ClickHouse OOMKill

[wrn14897]

Summary

Bumps clickhouse-operator-helm from v0.0.2 to v0.0.6 and fixes two resulting full-stack integration-test failures caused by new operator defaults that break the single-replica ClickHouse deployment.

Supersedes the upstream bump in ClickHouse/ClickStack-helm-charts#224.

Changes

Operator bump (from #224)

• clickhouse-operator-helm ~0.0.2 → ~0.0.6 (Chart.yaml, Chart.lock)
• values.yaml: rename enable → enabled for webhook / certManager / crd — required, since v0.0.6 renamed these keys. The old enable keys are silently ignored in v0.0.6 and would leave webhook/certManager enabled, breaking the install (no cert-manager present).
• integration-tests/full-stack/assert.sh: scope kubectl wait with --field-selector=status.phase!=Succeeded. Required by the bump: v0.0.6 introduces a version-probe Job (absent in v0.0.2) that leaves a Completed pod, which kubectl wait --for=condition=Ready pods --all can never satisfy → wait times out → suite fails.

ClickHouse hardening for operator v0.0.6 (new)

Two new operator defaults in v0.0.6 broke the single-replica ClickHouse deployment. The chart now overrides both in clickhouse.cluster.spec:

1. Explicit container resources (containerTemplate.resources: 2Gi memory, 500m CPU request)

The chart previously set no resources, relying on the operator default. v0.0.6 changed that default (release PR #206: "use the same req/limit for memory in default") to 512Mi, request == limit. At 512Mi, ClickHouse computes max_server_memory_usage ≈ 460 MiB; the full ClickStack schema plus ingestion and background merges exceeds it → OOMKilled (exit 137), crash-loop.

2. settings.enableDatabaseSync: false

v0.0.6 adds enableDatabaseSync defaulting to true (new CRD field). It creates the default database with the Replicated (DatabaseReplicated) engine, so table metadata lives in Keeper. Per the CRD docs it "Supports only Replicated and integration databases" and targets multi-replica clusters. In this single-replica deployment it only adds fragility: during ClickHouse startup a transient Keeper exception occurs —

Code: 999. Coordination::Exception: Cannot use any of provided ZooKeeper nodes. (KEEPER_EXCEPTION)   (exit 231)

— and the Replicated default database desyncs and silently drops every seeded table (otel_traces, otel_logs, …, and the goose tracking table). They never come back, so the collector loops on Table default.otel_traces does not exist and the smoke test's wait_for_table_queryable times out after 5 minutes → exit 1 (the failure seen on this PR's CI). Setting enableDatabaseSync: false keeps default Atomic, so seeded tables persist on the data volume across restarts.

Verification (local kind, fresh full-stack cluster)

For each fix, repro'd the failure on the bump and confirmed the fix end-to-end:

• Resources: repro'd ClickHouse OOMKill at 512Mi; after fix CH sees 2.0 GiB, max_server_memory_usage 1.80 GiB, stable.
• databaseSync: repro'd — default engine Replicated, all tables dropped after a startup Keeper hiccup, SHOW TABLES FROM default empty. After enableDatabaseSync: false: default engine Atomic, all otel_* / hyperdx_sessions / rollup tables present and queryable, ingestion succeeds (traces 173→218, logs 14→16).
• helm unittest charts/clickstack: 180/180 pass (added assertions for both new defaults).
• Both example values files (alb-ingress, api-only) render cleanly.

Note

2Gi raises the chart's baseline ClickHouse memory footprint vs. the operator's 512Mi default. Both settings remain overridable per environment via clickhouse.cluster.spec.

Ref

#224
#226

[changeset-bot]

🦋 Changeset detected

Latest commit: 0424d39

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

director_placeholder

[github-actions]

Deep Review

✅ No critical issues found. The four change sites — the ~0.0.6 semver bump (consistent with the pinned Chart.lock), the enable→enabled subchart-key rename, the verbatim-rendered containerTemplate.resources block, and the --field-selector=status.phase!=Succeeded scoping of kubectl wait — all trace clean through the rendering path, the helm-unittest suite, and the integration script. No rendering error, malformed manifest, or broken default introduced.

🟡 P2 — recommended

• charts/clickstack-operators/values.yaml:12 — the enable→enabled rename for webhook/certManager/crd has no test coverage, and Helm silently ignores unrecognized subchart value keys, so a wrong key name would no-op with no lint or render error and fall back to subchart defaults.
  • Fix: confirm the clickhouse-operator-helm v0.0.6 key names, then add a helm-unittest suite under charts/clickstack-operators (wired into the CI helm unittest invocation) that renders the subchart and asserts webhook/certManager disabled and crd created.
  • _{testing, correctness}

🔵 P3 nitpicks (1)

• charts/clickstack/values.yaml:331 — the new resources block introduces 2Gi/500m values and an intentional memory-request==limit / no-CPU-limit asymmetry with no inline comment, unlike the repo's "document all values with inline YAML comments" convention (the rationale currently lives only in the changeset, consumed at release).
  • Fix: add an inline comment above the block noting the operator default was too low (OOMKill) and why memory is capped request==limit while CPU has a request but no limit.

---

Reviewers (6): correctness, testing, maintainability, project-standards, reliability, learnings-researcher.

Testing gaps:

• No assertion that a Succeeded/Completed pod is excluded by kubectl wait; the --field-selector fix is exercised only at integration-test runtime, not by any unit test.
• The new resources test does not assert limits.cpu is intentionally unset, so a future accidental CPU limit (reintroducing throttling) would go uncaught.
• status.phase!=Succeeded still matches a Failed-phase pod, so a failed version-probe Job would make the final kubectl wait (no || true, under set -e) block the full 600s and misattribute the failure to pod readiness; consider excluding the probe Job by label instead.

[jordan-simonovski]

minimum CPU requests are definitely good to have. Overprovisioning a node is a much bigger problem than pod OOMs, so I like this. Glad we don't implement cpu limits.

[GrigoryPervakov]

enableDatabaseSync has been there since the first release and has always been enabled by default.
There were no changes regarding Atomic->Replicated recreation since then.

It makes sense even for single-node clusters, as it helps scale further and keeps the engine set the same for different topologies.

drops seeded tables on Keeper desync

This should not happen. It never drops tables or recreates the default database as Replicated if any tables have been created(code)

If smth really gets dropped by the operator(somehow) give a repro and I'll fix

[wrn14897]

@GrigoryPervakov, thanks for following up on this PR. I dug into the issue and found the root cause. The collector (clickhouseexporter) creates an atomic database (https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/277638be46c8653cc6574f2f9a6d2597bd5c12da/exporter/clickhouseexporter/internal/clickhouse.go#L60-L73). Later, the operator drops that database and recreates it as a Replicated database (https://github.com/ClickHouse/clickhouse-operator/blob/ecd71949243d71f37a717d21acbba20424af03a1/internal/controller/clickhouse/commands.go#L274-L290), which causes the integration test to fail.

What do you suggest as the best way to proceed here?

[wrn14897]

@GrigoryPervakov I've created a draft PR ClickHouse/clickhouse-operator#255. Let me know what you think. For the ClickStack Helm chart, we should use the Replicated engine going forward. That will be done in a follow-up PR.

[wrn14897]

@GrigoryPervakov Do you know when the next release is? It's currently blocking this PR.