Skip to content

Commit 7635a00

Browse files
scooby87scooby87
authored
feat(clickhouse): add configurable version parameter (#3476)
## What this PR does The ClickHouse server and Keeper images were hardcoded to `24.9.2.42` in the chart templates, so the chart exposed no top-level `version` values key. The Cozystack API returns a managed database's engine version by passing the HelmRelease's `spec.values.version` straight through into the Application object, so ClickHouse instances carried no `spec.version` in the API — unlike postgres, mariadb, mongodb, opensearch, rabbitmq and redis, which all ship a `version` parameter. This closes the ClickHouse half of the gap tracked in #1246. This adds the same version-selection scaffolding the other managed-database charts already use: - `files/versions.yaml` maps a `major.minor` key to the full image tag, shared by the ClickHouse server and Keeper images (both are pinned to one version). - `templates/_versions.tpl` (`clickhouse.versionMap`) resolves the tag and fails the render on an unsupported version. - `values.yaml` gains a `version` enum (`v25.8`, `v25.3`, `v24.9`), consumed by `templates/clickhouse.yaml` and `templates/chkeeper.yaml`. - `hack/update-versions.sh` + `make update` refresh the map from Docker Hub, keeping only tags published for *both* the server and Keeper images and preserving the current default so a regeneration never bumps the major of existing deployments. The default stays `v24.9` → `24.9.2.42`, so existing installations render byte-for-byte identically and are never upgraded to a new major implicitly. The generated `values.schema.json`, `README.md`, the Go API type and the `clickhouse-rd` `openAPISchema` are regenerated with `cozyvalues-gen` v1.6.0. A new `tests/version_test.yaml` helm-unittest suite covers the server/Keeper mapping for each version and the schema-level rejection of an unsupported value. ### Downstream repositories The diff changes `packages/apps/clickhouse/values.schema.json` by adding a new `version` field with an `enum`. Per the trigger map, this reaches `terraform-provider-cozystack`, whose ClickHouse resource is hand-written and would need a matching `version` attribute with a `stringvalidator.OneOf` list and its expand/flatten pair. There is no existing PR or issue there for it yet. This is left for a maintainer decision rather than a speculative cross-repo PR, so the box below is intentionally left unticked and flagged here. No other downstream repository is reached (the website reference page is regenerated from `README.md` by the release docs bot; ClickHouse is already in its app list). - [ ] 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: needs a `version` attribute on the ClickHouse resource (maintainer decision — see note above) - [ ] [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: ### Release note ```release-note feat(clickhouse): add a configurable `version` parameter (v25.8, v25.3, v24.9; default v24.9) so the deployed ClickHouse version is selectable and returned through the API. Existing installations are unchanged. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a configurable ClickHouse version setting for both server and Keeper images, defaulting to `v24.9`. * Supported versions are `v25.8`, `v25.3`, and `v24.9`, with compatible image patches selected automatically. * **Documentation** * Documented the version setting, default, and supported values. * **Validation** * Unsupported versions are rejected with an error listing allowed options. * **Tests** * Added coverage for version mapping, defaults, invalid values, and update behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents 713ae1f + 7ff9e07 commit 7635a00

14 files changed

Lines changed: 505 additions & 14 deletions

File tree

‎api/apps/v1alpha1/clickhouse/types.go‎

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎hack/clickhouse-update-versions_test.bats‎

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
#!/usr/bin/env bats
2+
# Unit test: packages/apps/clickhouse/hack/update-versions.sh (the ClickHouse
3+
# version-map generator).
4+
#
5+
# The generator turns the Docker Hub tag lists of clickhouse/clickhouse-server
6+
# and clickhouse/clickhouse-keeper into files/versions.yaml (a major -> full
7+
# tag map) and rewrites the version enum in values.yaml. These properties are
8+
# load-bearing and each guards a specific failure the review of #3476 caught:
9+
#
10+
# 1. The server/keeper tag intersection must be computed with a byte
11+
# (lexicographic) collation, not a version collation. `comm` walks its
12+
# inputs assuming byte order; feeding it `sort -V` output silently drops
13+
# common tags at every X.9 -> X.10 boundary.
14+
# 2. The default line is frozen to the exact tag it already ships (read from
15+
# the committed versions.yaml) so a regeneration never bumps the image of
16+
# existing default-version installs. The freeze follows whichever major is
17+
# the default, not a hardcoded one.
18+
# 3. Dropping the current default's major from the supported set is a hard
19+
# error, not a silent promotion of the newest major to default.
20+
# 4. A configured major with no tag common to both images is a hard error,
21+
# and the generator is atomic: on any error the committed files are left
22+
# untouched.
23+
# 5. The values.yaml splice is bounded: a malformed section (enum header
24+
# present, `version:` line absent) does not swallow the rest of the file.
25+
#
26+
# The generator is exercised offline: CH_SERVER_TAGS_FILE / CH_KEEPER_TAGS_FILE
27+
# inject the tag lists (no curl), and VALUES_FILE / VERSIONS_FILE stay in a
28+
# scratch dir. Written in the portable subset hack/cozytest.sh understands (no
29+
# `run`/`$status`/`$lines`, no setup/teardown, no standalone `}` inside a test
30+
# body): each assertion exits non-zero on failure and the body relies on
31+
# `set -e`.
32+
33+
REPO_ROOT="$(cd "$(dirname "${BATS_TEST_FILENAME:-$0}")/.." && pwd)"
34+
GEN="$REPO_ROOT/packages/apps/clickhouse/hack/update-versions.sh"
35+
36+
# Minimal values.yaml with the version section the generator rewrites. $2 is
37+
# the default version written into both the enum and the `version:` line.
38+
seed_values() {
39+
cat > "$1" <<EOF
40+
## @param {string} storageClass - StorageClass used to store the data.
41+
storageClass: ""
42+
43+
## @enum {string} Version
44+
## @value $2
45+
## @param {Version} version - ClickHouse major.minor version to deploy.
46+
version: $2
47+
48+
##
49+
## @section Application-specific parameters
50+
##
51+
## @param {int} logTTL - TTL.
52+
logTTL: 15
53+
EOF
54+
}
55+
56+
init_case() {
57+
WORK="$(mktemp -d)"
58+
SERVER="$WORK/server.txt"
59+
KEEPER="$WORK/keeper.txt"
60+
export VERSIONS_FILE="$WORK/versions.yaml"
61+
export VALUES_FILE="$WORK/values.yaml"
62+
export CH_SERVER_TAGS_FILE="$SERVER"
63+
export CH_KEEPER_TAGS_FILE="$KEEPER"
64+
: > "$VERSIONS_FILE"
65+
}
66+
67+
@test "intersection keeps tags across the X.9 -> X.10 boundary" {
68+
init_case
69+
seed_values "$VALUES_FILE" v24.9
70+
printf '%s\n' 24.9.2.42 24.9.3.128 24.10.1.1 > "$SERVER"
71+
cp "$SERVER" "$KEEPER"
72+
CH_SUPPORTED_MAJORS="24.10 24.9" bash "$GEN"
73+
grep -q '^"v24.10": "24.10.1.1"$' "$VERSIONS_FILE" || { echo "24.10 dropped from intersection" >&2; exit 1; }
74+
grep -q '^"v24.9":' "$VERSIONS_FILE" || { echo "24.9 dropped from intersection" >&2; exit 1; }
75+
}
76+
77+
@test "the default is frozen to its committed tag, not bumped to the latest patch" {
78+
init_case
79+
seed_values "$VALUES_FILE" v24.9
80+
printf '%s\n' '"v24.9": "24.9.2.42"' > "$VERSIONS_FILE"
81+
printf '%s\n' 24.9.2.42 24.9.3.128 > "$SERVER"
82+
cp "$SERVER" "$KEEPER"
83+
CH_SUPPORTED_MAJORS="24.9" bash "$GEN"
84+
grep -q '^"v24.9": "24.9.2.42"$' "$VERSIONS_FILE" || { echo "default not frozen to 24.9.2.42" >&2; exit 1; }
85+
if grep -q '24.9.3.128' "$VERSIONS_FILE"; then echo "default bumped to newer patch" >&2; exit 1; fi
86+
}
87+
88+
@test "the freeze follows the default major, not a hardcoded one" {
89+
init_case
90+
seed_values "$VALUES_FILE" v25.3
91+
printf '%s\n' '"v25.3": "25.3.14.14"' '"v24.9": "24.9.2.42"' > "$VERSIONS_FILE"
92+
printf '%s\n' 24.9.2.42 24.9.3.128 25.3.14.14 25.3.99.9 25.8.28.1 > "$SERVER"
93+
cp "$SERVER" "$KEEPER"
94+
CH_SUPPORTED_MAJORS="25.8 25.3 24.9" bash "$GEN"
95+
# v25.3 is the default -> frozen to its committed 25.3.14.14 despite 25.3.99.9
96+
grep -q '^"v25.3": "25.3.14.14"$' "$VERSIONS_FILE" || { echo "default v25.3 not frozen" >&2; exit 1; }
97+
# v24.9 is no longer the default -> free to move to its latest common patch
98+
grep -q '^"v24.9": "24.9.3.128"$' "$VERSIONS_FILE" || { echo "non-default v24.9 not on latest" >&2; exit 1; }
99+
}
100+
101+
@test "dropping the current default's major from the supported set is a hard error" {
102+
init_case
103+
seed_values "$VALUES_FILE" v24.9
104+
printf '%s\n' 25.3.14.14 25.8.28.1 > "$SERVER"
105+
cp "$SERVER" "$KEEPER"
106+
if CH_SUPPORTED_MAJORS="25.8 25.3" bash "$GEN" >/dev/null 2>&1; then echo "expected hard error on dropped default" >&2; exit 1; fi
107+
}
108+
109+
@test "non-default majors resolve to their latest common patch" {
110+
init_case
111+
seed_values "$VALUES_FILE" v25.8
112+
printf '%s\n' 25.3.14.14 25.3.15.1 25.8.28.1 > "$SERVER"
113+
cp "$SERVER" "$KEEPER"
114+
CH_SUPPORTED_MAJORS="25.8 25.3" bash "$GEN"
115+
grep -q '^"v25.3": "25.3.15.1"$' "$VERSIONS_FILE" || { echo "25.3 not latest patch" >&2; exit 1; }
116+
grep -q '^"v25.8": "25.8.28.1"$' "$VERSIONS_FILE" || { echo "25.8 missing" >&2; exit 1; }
117+
}
118+
119+
@test "a tag present in only one image is not selected" {
120+
init_case
121+
seed_values "$VALUES_FILE" v25.8
122+
printf '%s\n' 25.8.28.1 25.8.29.9 > "$SERVER"
123+
printf '%s\n' 25.8.28.1 > "$KEEPER"
124+
CH_SUPPORTED_MAJORS="25.8" bash "$GEN"
125+
grep -q '^"v25.8": "25.8.28.1"$' "$VERSIONS_FILE" || { echo "server-only tag was selected" >&2; exit 1; }
126+
}
127+
128+
@test "an unresolvable configured major is a hard error" {
129+
init_case
130+
seed_values "$VALUES_FILE" v24.9
131+
printf '%s\n' 24.9.2.42 > "$SERVER"
132+
cp "$SERVER" "$KEEPER"
133+
if CH_SUPPORTED_MAJORS="26.99 24.9" bash "$GEN" >/dev/null 2>&1; then echo "expected non-zero exit" >&2; exit 1; fi
134+
}
135+
136+
@test "a frozen default tag missing from the registry is a hard error" {
137+
init_case
138+
seed_values "$VALUES_FILE" v24.9
139+
printf '%s\n' '"v24.9": "24.9.2.42"' > "$VERSIONS_FILE"
140+
printf '%s\n' 24.9.3.128 > "$SERVER" # the pinned 24.9.2.42 is absent
141+
cp "$SERVER" "$KEEPER"
142+
cp "$VERSIONS_FILE" "$WORK/versions.before"
143+
cp "$VALUES_FILE" "$WORK/values.before"
144+
# Must NOT silently fall back to 24.9.3.128 and move the default image.
145+
if CH_SUPPORTED_MAJORS="24.9" bash "$GEN" >/dev/null 2>&1; then echo "expected hard error on missing frozen tag" >&2; exit 1; fi
146+
cmp -s "$VERSIONS_FILE" "$WORK/versions.before" || { echo "versions.yaml changed on failure" >&2; exit 1; }
147+
cmp -s "$VALUES_FILE" "$WORK/values.before" || { echo "values.yaml changed on failure" >&2; exit 1; }
148+
}
149+
150+
@test "no files are written when a configured major fails to resolve" {
151+
init_case
152+
seed_values "$VALUES_FILE" v24.9
153+
# The default (v24.9) is valid and 25.8 resolves, so the run passes the
154+
# default check and gets deep into the resolution loop before 26.99 fails —
155+
# exercising that nothing is written after a partial resolution, not just on
156+
# an early exit.
157+
printf '%s\n' 24.9.2.42 25.8.28.1 > "$SERVER"
158+
cp "$SERVER" "$KEEPER"
159+
printf '%s\n' '"v24.9": "24.9.2.42"' > "$VERSIONS_FILE"
160+
cp "$VERSIONS_FILE" "$WORK/versions.before"
161+
cp "$VALUES_FILE" "$WORK/values.before"
162+
if CH_SUPPORTED_MAJORS="25.8 24.9 26.99" bash "$GEN" >/dev/null 2>&1; then echo "expected non-zero exit" >&2; exit 1; fi
163+
cmp -s "$VERSIONS_FILE" "$WORK/versions.before" || { echo "versions.yaml changed on failure" >&2; exit 1; }
164+
cmp -s "$VALUES_FILE" "$WORK/values.before" || { echo "values.yaml changed on failure" >&2; exit 1; }
165+
}
166+
167+
@test "values.yaml enum is rewritten newest-first with the default preserved" {
168+
init_case
169+
seed_values "$VALUES_FILE" v24.9
170+
printf '%s\n' '"v24.9": "24.9.2.42"' > "$VERSIONS_FILE"
171+
printf '%s\n' 24.9.2.42 25.3.14.14 25.8.28.1 > "$SERVER"
172+
cp "$SERVER" "$KEEPER"
173+
CH_SUPPORTED_MAJORS="25.8 25.3 24.9" bash "$GEN"
174+
first="$(grep -m1 '@value' "$VALUES_FILE")"
175+
[ "$first" = "## @value v25.8" ] || { echo "enum not newest-first, got: $first" >&2; exit 1; }
176+
grep -q '^version: v24.9$' "$VALUES_FILE" || { echo "default not preserved as v24.9" >&2; exit 1; }
177+
grep -q '^## @section Application-specific parameters$' "$VALUES_FILE" || { echo "section boundary lost" >&2; exit 1; }
178+
grep -q '^logTTL: 15$' "$VALUES_FILE" || { echo "trailing content lost" >&2; exit 1; }
179+
}
180+
181+
@test "a malformed version section does not swallow the rest of the file" {
182+
init_case
183+
cat > "$VALUES_FILE" <<'EOF'
184+
## @enum {string} Version
185+
## @value v99.9
186+
## @section Application-specific parameters
187+
SENTINEL_KEEP: yes
188+
EOF
189+
printf '%s\n' 25.8.28.1 > "$SERVER"
190+
cp "$SERVER" "$KEEPER"
191+
CH_SUPPORTED_MAJORS="25.8" bash "$GEN"
192+
grep -q '^SENTINEL_KEEP: yes$' "$VALUES_FILE" || { echo "content after a malformed section was swallowed" >&2; exit 1; }
193+
grep -q '^## @section Application-specific parameters$' "$VALUES_FILE" || { echo "section boundary swallowed" >&2; exit 1; }
194+
}

‎packages/apps/clickhouse/.helmignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
.helmignore
22
/logos
33
/Makefile
4+
/hack

‎packages/apps/clickhouse/Makefile‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ generate:
77
cozyvalues-gen -m 'clickhouse' -v values.yaml -s values.schema.json -r README.md -g ../../../api/apps/v1alpha1/clickhouse/types.go
88
../../../hack/update-crd.sh
99

10+
update:
11+
hack/update-versions.sh
12+
make generate
13+
1014
.PHONY: test
1115
test:
1216
helm unittest .

‎packages/apps/clickhouse/README.md‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -108,16 +108,17 @@ chart-emitted CronJob renders only when `backup.schedule` is non-empty.
108108

109109
### Common parameters
110110

111-
| Name | Description | Type | Value |
112-
| ------------------ | ------------------------------------------------------------------------------------------------------------------------------------ | ---------- | ---------- |
113-
| `replicas` | Number of ClickHouse replicas. | `int` | `2` |
114-
| `shards` | Number of ClickHouse shards. | `int` | `1` |
115-
| `resources` | Explicit CPU and memory configuration for each ClickHouse replica. When omitted, the preset defined in `resourcesPreset` is applied. | `object` | `{}` |
116-
| `resources.cpu` | CPU available to each replica. | `quantity` | `""` |
117-
| `resources.memory` | Memory (RAM) available to each replica. | `quantity` | `""` |
118-
| `resourcesPreset` | Default sizing preset used when `resources` is omitted. | `string` | `t1.small` |
119-
| `size` | Persistent Volume Claim size available for application data. | `quantity` | `10Gi` |
120-
| `storageClass` | StorageClass used to store the data. | `string` | `""` |
111+
| Name | Description | Type | Value |
112+
| ------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | ---------- |
113+
| `replicas` | Number of ClickHouse replicas. | `int` | `2` |
114+
| `shards` | Number of ClickHouse shards. | `int` | `1` |
115+
| `resources` | Explicit CPU and memory configuration for each ClickHouse replica. When omitted, the preset defined in `resourcesPreset` is applied. | `object` | `{}` |
116+
| `resources.cpu` | CPU available to each replica. | `quantity` | `""` |
117+
| `resources.memory` | Memory (RAM) available to each replica. | `quantity` | `""` |
118+
| `resourcesPreset` | Default sizing preset used when `resources` is omitted. | `string` | `t1.small` |
119+
| `size` | Persistent Volume Claim size available for application data. | `quantity` | `10Gi` |
120+
| `storageClass` | StorageClass used to store the data. | `string` | `""` |
121+
| `version` | ClickHouse major.minor version to deploy. Applies to both the ClickHouse server and ClickHouse Keeper images. Downgrading to an older major is unsafe (ClickHouse cannot read data written by a newer server and Keeper snapshots are not backward compatible) — only increase this value. | `string` | `v24.9` |
121122

122123

123124
### Application-specific parameters

‎packages/apps/clickhouse/files/versions.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
"v25.8": "25.8.32.4"
2+
"v25.3": "25.3.14.14"
3+
"v24.9": "24.9.2.42"

0 commit comments

Comments
 (0)