ClickHouse / ClickHouse/clickhouse-operator

Reload-safe path for named_collections — typed CRD field or a reload-safe settings category

Open
#308 2 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.