ClickHouse / ClickHouse/clickhouse-operator
Reload-safe path for named_collections — typed CRD field or a reload-safe settings category
- Dominant language
- Go
- Stars
- 292
- Forks
- 42
- Avg merge
- 10h 18m
- Merged PRs (30d)
- 23
Description
## Problem
Any change to `spec.settings.extraConfig` rolls the ClickHouse pods, even when the change is a
`named_collections` entry — a section ClickHouse itself applies via `SYSTEM RELOAD CONFIG`
without a restart (verified on 26.7.1). On single-replica clusters that turns a URL tweak into
a full ClickHouse outage window.
The cause is that `extraConfig` is one opaque raw blob, hardcoded restart-required in
`internal/controller/clickhouse/config.go`:
```go
Name: ExtraConfigFileName,
requiresRestart: true,
Getter: func(r *clickhouseReconciler) []byte { return r.Cluster.Spec.Settings.ExtraConfig.Raw },
```
We understand why (#15): arbitrary user YAML can't be classified, so restart-always is the only
safe default *given the current API shape*. This issue asks for an API shape that lets the
operator know a setting is reload-safe — with `named_collections` as the concrete, common case.
## Why named_collections specifically
- They're a natural fit for CRs: per-environment endpoints/credentials for `url()`/S3/dictionary
sources (our case: a refreshable materialized view pulling a dimension table from an internal
REST API — the collection holds the per-env URL).
- They're reload-safe by ClickHouse's own semantics, and the operator already trusts that: the
config-reload revision tracking introduced in #115 reads its marker back **through a reserved
named collection**. The reload plumbing (restartable vs reloadable hash split,
`checksum/reloadable-configuration` on the ConfigMap, operator-driven `SYSTEM RELOAD CONFIG`)
already exists and already carries `extraUsersConfig` — named collections just can't reach it.
## Proposal
**Option A (preferred): a first-class typed field**, e.g.
```yaml
spec:
settings:
namedCollections:
ctx_customers:
url: "http://my-service.my-ns.svc:5400/Customers"
format: "JSONEachRow"
```
rendered into its own `config.d` file whose generator sets `requiresRestart: false`, riding the
existing #115 reload path. Being typed, it also becomes validatable (see bonus below) and could
later support `valueFrom` secret refs for credentials.
**Option B (more general): an explicitly reload-safe raw section**, e.g.
`spec.settings.extraReloadableConfig` — same pass-through semantics as `extraConfig`, but the
user asserts reload-safety by choosing the field, and the operator classifies it into the
reloadable bucket. This covers the whole *category* of reload-safe server settings without the
operator having to classify anything itself.
**Option C (least preferred): an opt-out annotation** excluding `extraConfig` from the restart
hash — mentioned only for completeness; it's a footgun compared to A/B.
## Bonus: the misplacement trap A would also close
Trying to dodge the restart by putting `named_collections` under `extraUsersConfig` fails
*silently*: the users-config parser ignores server-tree sections — server starts clean, no log
warning, `system.named_collections` empty (verified on `clickhouse-server:26.7.1.1315-alpine`;
the same YAML via `config.d` registers fine). Until a typed field exists, a validating-webhook
check rejecting server-tree keys inside `extraUsersConfig` would convert that silent no-op into
a loud error.
## Environment
- operator `v0.0.7` (ghcr.io), behavior re-verified unchanged on `main` @ `fc046cc1`
- ClickHouse `26.7.1.1315-alpine`, Replicated database, 1–2 replicas per cluster, AKS
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in internal/controller/clickhouse/config.go, then trace the reload plumbing and revision tracking introduced by #115, including the reloadable-configuration ConfigMap marker and operator-driven SYSTEM RELOAD CONFIG path. Decide which proposed API shape fits the existing design, then verify that named_collections changes use the reloadable path without rolling pods while extraConfig remains restart-required.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes
- Domain
- database, devops
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100