Skip to content

fix(clickhouse): keep the Keeper name inside the StatefulSet volume-name limit - #4133

Merged
Aleksei Sviridkin (lexfrei) merged 1 commit into
cozystack:mainfrom
yankawai:fix/clickhouse-keeper-name
Sep 17, 2026
Merged

Aleksei Sviridkin (lexfrei) merged 1 commit into
cozystack:mainfrom
yankawai:fix/clickhouse-keeper-name

Conversation

@yankawai

@yankawai europrinter (yankawai) commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

What this PR does

Fixes #4083.

clickhouse-operator names the Keeper config volume chk-<chk>-deploy-confd-<cluster>-<shard>-<replica>. With the CHK called <release>-keeper and Release.Name already prefixed with clickhouse-, an application name longer than 15 characters pushes that volume name past 63 characters. The StatefulSet is rejected on every reconcile, no Keeper pod appears, and the CHK still reports Completed; the only visible signal is the Keeper WorkloadMonitor staying at operational: false.

templates/_keeper.tpl now computes the CHK name: <release>-keeper as before when it fits, otherwise the release name is truncated and a six-character hash of the release name is appended so two long names cannot collide. The budget accounts for the width of the replica index. chkeeper.yaml and the zookeeper nodes list in clickhouse.yaml both use the helper, so the hosts ClickHouse connects to always match the CHK the operator creates. The pod label, the VMPodScrape selector and the WorkloadMonitor stay on <release>-keeper.

Names up to 15 characters render byte-for-byte as before. A release with a longer name never had a Keeper pod; on upgrade its pod-less CHK is replaced by the shortened one and the Keeper comes up.

helm unittest: 30/30 in the chart, including the new tests/keeper_name_test.yaml (short name unchanged, the exact 63-character boundary, one character past it, a very long name, a two-digit replica index, no trailing dash after truncation).

Downstream repositories

Templates and tests only: no values, schema, README, defaults, ApplicationDefinition or tenant-facing Secret/Service names change.

Release note

fix(clickhouse): shorten the Keeper name for application names longer than 15 characters so the Keeper StatefulSet can be created

Summary by CodeRabbit

  • Bug Fixes

    • Fixed ClickHouse Keeper resource naming for long release names so generated names remain within Kubernetes’ 63-character limit.
    • Updated ClickHouse Keeper hostnames to consistently match the generated resource name, including when names are shortened.
    • Preserved identifying labels and monitoring selectors while applying the corrected Keeper naming behavior.
  • Tests

    • Added coverage for standard, boundary-length, and truncated Keeper names, including replica-specific naming scenarios.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: d3e80408-2fa6-458f-ac3c-d36989c77780

📥 Commits

Reviewing files that changed from the base of the PR and between cd675e1 and 03a7af7.

📒 Files selected for processing (4)
  • packages/apps/clickhouse/templates/_keeper.tpl
  • packages/apps/clickhouse/templates/chkeeper.yaml
  • packages/apps/clickhouse/templates/clickhouse.yaml
  • packages/apps/clickhouse/tests/keeper_name_test.yaml

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The ClickHouse chart now generates a bounded Keeper name. Keeper metadata and ClickHouse zookeeper hostnames use this name. Helm tests cover length boundaries, truncation, replica counts, hostname alignment, and selector behavior.

Changes

ClickHouse Keeper naming

Layer / File(s) Summary
Keeper name helper
packages/apps/clickhouse/templates/_keeper.tpl
Adds clickhouse.keeperName, which preserves valid names and truncates oversized names with a six-character hash suffix.
Template name wiring
packages/apps/clickhouse/templates/chkeeper.yaml, packages/apps/clickhouse/templates/clickhouse.yaml
Uses the helper for Keeper metadata and ClickHouse zookeeper hostnames.
Name budget validation
packages/apps/clickhouse/tests/keeper_name_test.yaml
Tests boundary lengths, truncation, dash trimming, replica counts, hostname alignment, and selector values.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 03a7a

The chart now creates Keeper resources for longer application names while preserving existing names for short releases and keeping ClickHouse endpoints aligned. No current merge-blocking risk remains.

Sequence Diagram(s)

sequenceDiagram
  participant HelmRelease
  participant clickhouse.keeperName
  participant ClickHouseKeeperInstallation
  participant ClickHouseInstallation
  HelmRelease->>clickhouse.keeperName: provide release name and replica count
  clickhouse.keeperName->>ClickHouseKeeperInstallation: render bounded metadata.name
  clickhouse.keeperName->>ClickHouseInstallation: render bounded zookeeper hostname
  ClickHouseKeeperInstallation->>ClickHouseInstallation: use matching Keeper name
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary change: keeping the generated ClickHouse Keeper name within the StatefulSet volume-name limit.
Linked Issues check ✅ Passed The changes address issue [#4083] by shortening the Keeper name only when the generated volume name can exceed 63 characters, adding a collision-resistant hash suffix, accounting for replica index wid…
Out of Scope Changes check ✅ Passed All code and test changes support the linked issue [#4083]. The helper, resource references, hostname references, and boundary tests are directly related to preventing invalid Keeper StatefulSets.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added size/L This PR changes 100-499 lines, ignoring generated files area/database Issues or PRs related to managed databases (postgres, mariadb, redis, etcd, kafka, clickhouse) kind/bug Categorizes issue or PR as related to a bug labels Sep 7, 2026
…ame limit

clickhouse-operator derives the Keeper config volume name as
chk-<chk>-deploy-confd-<cluster>-<shard>-<replica>. The CHK is named
<release>-keeper and Release.Name already carries the clickhouse- prefix,
so an application name longer than 15 characters pushes that volume name
past 63 characters: the StatefulSet is rejected, no Keeper pod appears,
and the CHK still reports Completed.

The CHK name is now shortened only when it would overflow, with a short
hash of the release name so two long names stay distinct, and the
ClickHouse zookeeper hosts are built from the same helper. Short names
render exactly as before.

Signed-off-by: Yan Bondarenko <[email protected]>

@scooby87 scooby87 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (independent cozy-review pass).

The truncation threshold is exactly the previous failure condition, so no healthy install (name <= 63) is renamed — short release names keep <release>-keeper unchanged and only already-broken names (> 63) are truncated, which makes the upgrade safe for existing keeper PVCs. The truncated name targets exactly 63 chars and the boundary case is tested. Cross-file consistency is right: object names use the truncated $keeper, while pod-label app and its selectors stay on the full name and remain internally consistent.

Verified: helm unittest 7/7; mutating the threshold turns 4 tests RED (non-vacuous); the sha256 hashes in the tests check out.

Non-blocking: in the truncation branch the name depends on the digit count of replicas, so scaling keeper across a power-of-ten boundary (9->10) recomputes the budget and renames the CHK; narrow trigger, worth a comment.

@lexfrei
Aleksei Sviridkin (lexfrei) merged commit bd38922 into cozystack:main Sep 17, 2026
15 checks passed
@lexfrei Aleksei Sviridkin (lexfrei) added the kind/backport Categorizes issue or PR as requiring a backport to the current release line label Sep 18, 2026
@github-actions

Copy link
Copy Markdown

myasnikovdaniil added a commit that referenced this pull request Sep 23, 2026
…he StatefulSet volume-name limit (#4314)

# Description
Backport of #4133 to `release-1.6`.
Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Nov 13, 2026
…4945)

## What this PR does

The apps API admits release names of up to 53 characters, the release
prefix included (less for Kafka and Kubernetes, which cap their own
names). Kubernetes copies a Job name into the `job-name` label of its
pods, and a label value takes at most 63 characters, so the apiserver
refuses a Job with a longer name. A CronJob name is refused over 52
characters, because the controller names each Job it starts
`<cronjob>-<timestamp>`. I rendered every Job and CronJob in
`packages/apps` and `packages/extra` with the longest release name each
application admits, and eight of them go over.

| Application | Object | Name | Longest on main | After |
| --- | --- | --- | --- | --- |
| bucket | post-delete Job | `<release>-cleanup-acme` | 66 | 63 |
| qdrant | post-delete Job | `<release>-qdrant-cleanup` | 68 | 63 |
| kafka | pre-install and pre-upgrade Job | `<release>-kafka-migration`
| 66 (release capped at 50) | 63 |
| vm-disk | resize Job, rendered when the disk grows |
`<release>-volume-resize-hook` | 72 | 63 |
| vm-instance | update Job and the `app` label of its pod |
`<release>-update-hook` | 65 | 63 |
| mariadb | backup CronJob | `<release>-backup` | 60 | 52 |
| clickhouse | legacy backup CronJob | `<release>-backup` | 60 | 52 |
| tenant | pre-delete Job in `cozy-system` | `<tenant
namespace>-cleanup` | 71 (nested namespace of 63) | 63 |

Counted in characters of the application name, a Kafka of 42 to 44
cannot be installed today, a VMDisk of 37 or more cannot grow, and a
VMInstance of 40 or 41 cannot take an instance type, profile or external
change. Deleting a Bucket of 44 or more or a Qdrant of 42 or more fails
in the post-delete hook. With the backup CronJob enabled, a MariaDB of
38 or more and a ClickHouse of 35 or more fail the whole release. A
nested tenant whose namespace is longer than 55 characters cannot be
deleted.

The Jobs of bucket, qdrant, kafka, vm-disk and vm-instance are now named
`printf "%s-<suffix>" .Release.Name | trunc 63 | trimSuffix "-"`, and
vm-instance uses the same value for the pod label. A release name is
never longer than 53, so only the fixed suffix is cut, and a brute-force
check over every pair of release name lengths found no two release names
that render the same Job name.

A plain cut to 52 would reach into the release name of a CronJob, and
two 53-character names that differ in the last character would share
one. So a CronJob name that does not fit becomes the release name cut to
38 characters, the first six hex characters of its sha256 and `-backup`,
the shape #4133 gave the ClickHouse Keeper name. The tenant cleanup Job
takes the same form with the namespace cut to 48, because it runs in
`cozy-system` next to the cleanup Job of every other tenant. There a
plain cut gives a parent and a child named `clea` the same name, and the
child's `before-hook-creation` would delete the parent's Job while it
runs.

Names that fit render exactly as before. The hook documents rendered
from main and from this branch with the short release names the tests
use are identical in all eight charts. A name that did not fit never
produced an object, so nothing is renamed on upgrade. The
ServiceAccounts, Roles and RoleBindings of these hooks keep their names,
since they have no such limit, so the ones a pre-install hook left
behind are replaced rather than orphaned. Helm runs the delete hooks
stored with a release, so an existing Bucket, Qdrant or tenant picks up
the fix with its next upgrade.

Every other Job and CronJob fits at the longest name. The cleanup Jobs
of clickhouse, harbor, mariadb and opensearch and the Kafka pre-delete
Job reach 61, the Postgres init Job 62, and the Kubernetes and
KubernetesNodes Jobs either stay under 63 or already cut their names.
The etcd and monitoring modules require their chart name as the release
name, and the gateway and seaweedfs cleanup Jobs reach 61.

Each of the eight charts gets a new helm-unittest suite. It pins a short
name, a name that lands exactly on the limit and the longest admitted
name, a cut that would end in a dash wherever one can happen, and for
the hooks the unchanged ServiceAccount and RBAC names. The CronJob
suites also pin two 53-character names that differ in the last
character, and the tenant suite pins the parent and child pair. Against
the templates on main, 18 of the 41 new cases fail, every one with a
name over the limit. With the hash forms replaced by a plain cut, the
tenant parent and child both render
`tenant-abcdefghijklmnopqrstuvwxyz-0123456789abcdefghijklmn-clea` and
the two MariaDB names both render
`mariadb-abcdefghijklmnopqrstuvwxyz0123456789abcdefgh`.
`hack/helm-unit-tests.sh` passes over all 106 packages with tests. The
Bats files that exercise these hooks and charts
(`app-cleanup-hooks_test`, `kafka-migration-hook`,
`kafka-pre-delete-hook`, `tenant-pre-delete-hook`,
`check-hook-request-timeout`, `pod-label-census_test`,
`mariadb-first-boot-waits`, `check-render-matrix`) give 96 ok on main
and on this branch. `make generate` with cozyvalues-gen v1.7.0 in the
eight packages changes nothing. A local kube-apiserver v1.35.0 refuses
the longest name each chart renders on main, with `must be no more than
63 bytes` on the job-name label or `must be no more than 52 characters`
on the CronJob, and creates the name from this branch. For vm-disk and
vm-instance, whose hooks render only after a live lookup, that check
used a Job with the same name and labels.

This is related to #4875 but does not depend on it. #4875 adds a
cozy-lib cleanup Job and cuts its name to 63 the same way, while this PR
covers the Jobs already on main. Neither needs the other, and the two
merge cleanly.

### Screenshots

Not a UI change.

### Downstream repositories

Templates and tests only. No values, schema, README, defaults,
`ApplicationDefinition`, Secret or Service name changes, and none of the
changed Job or CronJob names appears in the website, the Terraform
provider, the Ansible collection or the other repositories on the list.

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:

### Release note

```release-note
fix(apps): Job and CronJob names now fit the Kubernetes limits for the longest application names. A Kafka with a name of 42 to 44 characters can now be installed, long VMDisk and VMInstance names no longer fail a resize or an instance type change, long Bucket and Qdrant names and long nested tenant namespaces no longer fail deletion, and long MariaDB and ClickHouse names no longer fail the release with the backup CronJob enabled.
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Cleanup, migration, resize, and update Jobs now use names that fit
Kubernetes naming limits, including for long release or tenant names.
* ClickHouse and MariaDB backup CronJobs now use names within
Kubernetes’ length limit. Longer names are shortened with a hash suffix
to help distinguish them.
* Short names and names at the supported limit retain their existing
naming format.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/database Issues or PRs related to managed databases (postgres, mariadb, redis, etcd, kafka, clickhouse) kind/backport Categorizes issue or PR as requiring a backport to the current release line kind/bug Categorizes issue or PR as related to a bug size/L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

clickhouse: Keeper is never created when the application name is longer than 15 characters

3 participants