8000 backup: generate clickhouse-backup config.yml into a Secret, drop env vars + liveness restarts by Slach · Pull Request #2118 · Altinity/clickhouse-operator · GitHub
[go: up one dir, main page]
Skip to content

backup: generate clickhouse-backup config.yml into a Secret, drop env vars + liveness restarts - #2118

Open
Slach wants to merge 4 commits into
0.28.0from
backup-config-file
Open

backup: generate clickhouse-backup config.yml into a Secret, drop env vars + liveness restarts#2118
Slach wants to merge 4 commits into
0.28.0from
backup-config-file

Conversation

@Slach
@Slach Slach commented Oct 27, 2026 •
edited
Loading
Copy link
Copy Markdown
Collaborator

Summary

Replaces the way the operator-managed clickhouse-backup sidecar gets its settings.

Before: the storage Secret's keys came in as env vars through envFrom, and the credentials as secretKeyRef env vars. Kubelet reads env vars only when it starts a container, so a liveness probe compared the mounted Secret files with the sidecar's start-up environment and failed, so that the sidecar got restarted, whenever they differed.

After:

  • The storage Secret (backup.storages[].secretRef) holds a key config.yml: a YAML fragment of the clickhouse-backup configuration (general:, s3:, gcs:, …).
  • The operator builds a per-cluster Secret <chi>-<cluster>-backup-config from it (pkg/model/chi/backupconfig). The Secret is owned by the CHI and holds:
    • the user fragment;
    • the API and ClickHouse credentials from <chi>-auto-backup-secret;
    • default path/object_disk_path for every storage type;
    • general.disable_environment_override: false;
    • the storage-path-check settings under their env-var names.
  • The Secret is mounted read-only at /etc/clickhouse-backup as a whole directory, not via subPath. clickhouse-backup server re-reads config.yml for every API operation (APIServer.ReloadConfig), so a change applies without a restart.
  • The settings the Jobs rely on (API_LISTEN, API_SECURE, API_ALLOW_PARALLEL, RESTORE_SCHEMA_ON_CLUSTER, RESTORE_*_MAPPING, CLICKHOUSE_RESTART_COMMAND) and the per-host ClickHouse port/TLS stay env vars, which take precedence over the file.
  • The operator drops these from the base container, because they would override or bypass the file:
    • the env vars API_USERNAME, API_PASSWORD, CLICKHOUSE_USERNAME, CLICKHOUSE_PASSWORD and CLICKHOUSE_BACKUP_CONFIG;
    • any mount at, below or above /etc/clickhouse-backup.
  • The sidecar's own command waits until the config file exists, so the REST API is never served without credentials. A custom command makes the volume mandatory.
  • The CHI reconcile creates and updates the config Secrets before StatefulSets. A 1-minute background sync keeps them in step with the storage Secrets between reconciles. It skips CHIs that are suspended, being deleted, reconcile in progress, or never completed. The operator still doesn't cache or watch user Secrets.
  • The Job storage-path check now reads its settings from the generated Secret instead of the user's storage Secret.
  • Removed: backup-sidecar-liveness.sh, the envFrom of the storage Secret, the Secret mounts under /var/run/secrets/clickhouse-backup, and the default-path logic in backup-sidecar.sh.
  • New event BackupStorageConfigInvalid. BackupStorageSecretMissing now also fires when the Secret has no config.yml key.

⚠️ Breaking (API is EXPERIMENTAL): storage Secrets with env-style keys (REMOTE_STORAGE, S3_BUCKET, …) are no longer read and must be converted to config.yml. Docs, examples and e2e manifests are updated.

How the problems of the previous mechanism are addressed

  1. A key removed from the storage Secret was never noticed. The probe only compared files that exist, so a deleted key such as S3_STORAGE_CLASS stayed in the sidecar's environment until something else restarted it. Now the config file is rebuilt from the whole Secret, and clickhouse-backup reloads it for every operation, so a removed key is gone from the next operation on.
  2. A base container with its own livenessProbe disabled the mechanism entirely. With such a probe, Secret changes never reached the sidecar until the pod restarted. Applying a change no longer needs a probe or a restart, so a user-defined probe is irrelevant here.
  3. The restart raced with the backup Jobs. busy() only saw the local sidecar's operations, so a sidecar on host B could be restarted while a Job was still working on host A and hit B mid-run. A restore is never resumed. Nothing restarts the sidecar any more: a running operation keeps the config it loaded, and the next operation picks up the new one.
  4. Every Secret change looked like a crash. It produced Unhealthy / liveness-probe-failed events and bumped restartCount, which triggers container-restart alerts during routine credential rotation. Now there are no probe failures or restarts, only a Secret update.
  5. Storage credentials sat in the process environment. They are no longer env vars of the sidecar, so they no longer show in /proc/<pid>/environ or in the container env. They live only in the mounted file, read-only, inside the sidecar container.

REST API credentials: clickhouse-backup checks them against the config it has loaded, and only an authenticated operation reloads it, so a change of the credentials in <chi>-auto-backup-secret, edited in place or recreated, would otherwise get every Job a 401. The generated Secret also holds api-credentials.sha256, the SHA-256 of the REST API username and password, mounted next to config.yml. The sidecar script now supervises clickhouse-backup server instead of exec-ing it and sends it SIGHUP when the fingerprint changes. That restarts the server process, not the container, and cancels the operations in progress, which Jobs started with the old credentials could not follow anyway. Storage and ClickHouse settings are still reloaded per operation without a restart. e2e test_010087 changes the password in place and checks that every sidecar accepts the new one and refuses the old one without a container restart.

Sync warnings: the once-a-minute sync emits BackupStorageSecretMissing and BackupStorageConfigInvalid like the reconcile does, once per change of state rather than on every pass.

Testing

  • New unit tests:
    • pkg/model/chi/backupconfig: merge, defaults, determinism, invalid input;
    • pkg/controller/chi: create, update only on change, invalid input leaves the Secret alone, registry, periodic sync and its skip conditions.
  • Reworked the sidecar and path-check tests. The kubelet simulation now builds the config Secrets the way the operator does.
  • go test ./pkg/... ./cmd/... passes in golang:1.27.2 (Linux). The script tests need /proc and GNU date, and are skipped or fail on macOS, as before.
  • e2e test_010087 was updated for the new Secret format: it asserts the config volume, the generated Secret and the absence of credential env vars, and waits for the regenerated config before the shared-path backup. test_010087 (11m) and test_010088 (6m) pass on a local OrbStack Kubernetes against the operator image built from this branch. The full e2e suite passed in CI (run 40454429451, 3h 16m); test_010009, test_010011_5, test_010036, test_010086_1 and test_090099 needed a retry.

🤖 Generated with Claude Code

@Slach Slach mentioned this pull request Nov 3, 2026
Closed
@Slach
Slach commented Nov 5, 2026
8000 Copy link
Copy Markdown
Collaborator Author

@alex-zaitsev latest changes (2aa659a, 9cf5e21):

  • API: backup.storages[] / backup.storage replaced by backup.configSecretRef, a Secret with the whole clickhouse-backup config.yml per cluster. status.clusters[].configSecret of backups, restores and schedules shows which Secret a cluster's backups use.
  • ClickHouse credential rotation: changing CLICKHOUSE_USERNAME / CLICKHOUSE_PASSWORD in <chi>-auto-backup-secret reconciles the CHI. The reconcile writes the users config first, then the sidecar config. The minute sync keeps the ClickHouse credentials of the sidecar config, so a sidecar never gets a password ClickHouse doesn't know yet. API credentials are still picked up by the minute sync, without a reconcile.
  • Backup username from the Secret: CLICKHOUSE_USERNAME, default clickhouse_backup. An invalid name, default or the operator's own user aborts the reconcile with BackupSecretUnresolved.
  • Sync scaling: each CHI gets its own 15s deadline instead of 30s for the whole pass, so slow installations don't delay the others.
  • Tests: unit tests for rotation, sync and username. e2e test_010087 (now also rotates the ClickHouse credentials) and test_010088 pass locally.

@Slach
Slach force-pushed the backup-config-file branch from 9cf5e21 to a6379b9 Compare November 5, 2026 19:30
@Slach @claude
backup: generate clickhouse-backup config.yml into a Secret, take sto…
2e5729b 
…rage settings from backup.configSecretRef

- backup.configSecretRef names a Secret whose key config.yml holds the clickhouse-backup configuration of a cluster
  (replaces backup.storages[]/backup.storage). The operator builds <chi>-<cluster>-backup-config from it and the
  credentials of <chi>-auto-backup-secret, mounts it into the sidecar, and keeps it in step with a reconcile and a
  sync once a minute, with a deadline per installation. No more env vars from the storage Secret and no liveness
  restarts. A storage Secret gone or emptied keeps the storage settings the sidecars have.
- <chi>-auto-backup-secret holds only the generated REST API credentials. CLICKHOUSE_USERNAME and CLICKHOUSE_PASSWORD
  are set by the user or not at all: without them the sidecar configuration has no ClickHouse credentials and
  clickhouse-backup uses its own defaults. With both set, the operator creates that backup user, reachable from the
  pod's loopback only; with only one set, it warns with BackupCredentialsIncomplete.
- The sidecar restarts its server on new REST API credentials (api-credentials.sha256 next to config.yml).
- A change of CLICKHOUSE_USERNAME or CLICKHOUSE_PASSWORD reconciles the CHI, which writes the users configuration
  before the sidecar configuration; the sync takes new backup user credentials only once the users configuration
  has them.
- Jobs and CronJobs carry the clickhouse.altinity.com/backup-config-secret annotation; backups, restores and
  schedules report status.clusters[].configSecret.
- Events BackupStorageSecretMissing and BackupStorageConfigInvalid, once per change of state in the sync.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Slach
Slach force-pushed the backup-config-file branch from a6379b9 to 2e5729b Compare November 6, 2026 05:13

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sunsingerus sunsingerus Awaiting requested review from sunsingerus
@alex-zaitsev alex-zaitsev Awaiting requested review from alex-zaitsev

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Slach
0