Skip to content

Commit 35124a3

Browse files
filimonovclaude
filimonov
and
claude
committed
feat(keeper): Raft-safe ClickHouseKeeperInstallation rescale
Make CHK scale-up/scale-down Raft-safe by replacing the fixed post-create sleep and the all-at-once XML publish with staged, verified membership changes. Problem: on rescale the operator published a raft_configuration with every desired server at once and only waited a fixed 7s, never confirming committed Raft membership. Two freshly-started Keeper nodes could form a quorum between themselves while the original node still believed it was the whole cluster — a split-brain window. Scale-down deleted StatefulSets/PVCs with no barrier confirming the member had actually left committed membership. Approach (hardened staged XML; source-verified against Keeper 23.8..master — versioned `reconfig` CAS does not exist in any version, and the XML-diff mode is the reliable control plane): - Publish new members one at a time, each behind a committed-membership barrier read from the `/keeper/config` znode plus a leader `mntr` synced-followers barrier. Never more than one not-yet-committed server in the published XML (split-brain guard). Emit `<start_as_follower>` for members joining an established cluster. - Gate CHK `Completed` on committed membership == desired. - Scale-down: transfer leadership (`rqld`) off departing members, then gate StatefulSet/PVC purge on confirmed committed removal. - Lease leader election for the CHK controller manager. - Fail-safe: any barrier timeout / quorum loss requeues without deleting or mutating anything. Also version-gates the `async_replication` / `use_xid_64` coordination settings (a pre-existing incompatibility that CrashLooped Keeper < 24.12) and fixes a shared ReconcileAttributes tag setter that was a silent no-op. Adds a keeper observe client (`pkg/model/chk/keeper`), the controller barriers (`pkg/controller/chk/worker-raft-membership.go`), and e2e coverage including a deterministic split-brain demonstration (RED on the pre-fix operator, GREEN on this branch, across Keeper 23.8/24.8/25.8). Full-rescale support floor is Keeper 24.8 (documented; < 24.3 is safety-covered only). See docs/chk-rescale-raft-safety-v3.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
1 parent c0f085f commit 35124a3

37 files changed

Lines changed: 2912 additions & 47 deletions

‎cmd/operator/app/thread_keeper.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package app
22

33
import (
44
"context"
5+
"os"
56

67
"github.com/go-logr/logr"
78

@@ -20,6 +21,7 @@ import (
2021
ctrlController "sigs.k8s.io/controller-runtime/pkg/controller"
2122

2223
api "github.com/altinity/clickhouse-operator/pkg/apis/clickhouse-keeper.altinity.com/v1"
24+
deployment "github.com/altinity/clickhouse-operator/pkg/apis/deployment"
2325
"github.com/altinity/clickhouse-operator/pkg/chop"
2426
controller "github.com/altinity/clickhouse-operator/pkg/controller/chk"
2527
)
@@ -67,6 +69,13 @@ func initKeeper(ctx context.Context) error {
6769
// pkg/metrics/operator, and the CHK reconcile counters worth exposing are surfaced
6870
// through that path, not through controller-runtime's manager-default exposition.
6971
Metrics: metricsserver.Options{BindAddress: "0"},
72+
// Serialize CHK reconciliation across operator replicas/restarts.
73+
// The Lease lives in the operator's own namespace (empty value falls
74+
// back to controller-runtime's in-cluster namespace detection).
75+
LeaderElection: true,
76+
LeaderElectionID: "clickhouse-keeper-operator.altinity.com",
77+
LeaderElectionNamespace: os.Getenv(deployment.OPERATOR_POD_NAMESPACE),
78+
LeaderElectionReleaseOnCancel: true,
7079
})
7180
if err != nil {
7281
logger.Error(err, "init keeper - unable to ctrlRuntime.NewManager")

‎config/chk/keeper_config.d/01-keeper-01-default-config.xml‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,20 @@
77
<clickhouse>
88
<asynchronous_metrics_keeper_metrics_only>1</asynchronous_metrics_keeper_metrics_only>
99
<keeper_server>
10+
<!--
11+
async_replication and use_xid_64 are intentionally NOT set here.
12+
They are version-gated coordination settings: older ClickHouse Keeper
13+
releases reject them with UNKNOWN_SETTING and CrashLoop at startup
14+
(async_replication: absent before 24.3; use_xid_64: absent before 24.12).
15+
The operator emits them per-CR from the config generator, only for
16+
Keeper versions known to support them. See
17+
pkg/model/chk/config/generator.go getCoordinationSettings.
18+
-->
1019
<coordination_settings>
11-
<async_replication>1</async_replication>
1220
<min_session_timeout_ms>10000</min_session_timeout_ms>
1321
<operation_timeout_ms>10000</operation_timeout_ms>
1422
<raft_logs_level>information</raft_logs_level>
1523
<session_timeout_ms>100000</session_timeout_ms>
16-
<use_xid_64>1</use_xid_64>
1724
</coordination_settings>
1825
<hostname_checks_enabled>true</hostname_checks_enabled>
1926
<log_storage_path>/var/lib/clickhouse-keeper/coordination/logs</log_storage_path>
@@ -35,7 +42,7 @@
3542
stricter list can override this value, but they must keep `ruok` if they
3643
also use the default operator probes.
3744
-->
38-
<four_letter_word_white_list>conf,cons,crst,envi,ruok,srst,srvr,stat,wchs,dirs,mntr,isro</four_letter_word_white_list>
45+
<four_letter_word_white_list>conf,cons,crst,envi,ruok,srst,srvr,stat,wchs,dirs,mntr,isro,rqld</four_letter_word_white_list>
3946
</keeper_server>
4047
<listen_host>::</listen_host>
4148
<listen_host>0.0.0.0</listen_host>

‎deploy/builder/templates-config/chk/keeper_config.d/01-keeper-01-default-config.xml‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
<clickhouse>
22
<asynchronous_metrics_keeper_metrics_only>1</asynchronous_metrics_keeper_metrics_only>
33
<keeper_server>
4+
<!--
5+
async_replication and use_xid_64 are intentionally NOT set here.
6+
They are version-gated coordination settings: older ClickHouse Keeper
7+
releases reject them with UNKNOWN_SETTING and CrashLoop at startup
8+
(async_replication: absent before 24.3; use_xid_64: absent before 24.12).
9+
The operator emits them per-CR from the config generator, only for
10+
Keeper versions known to support them. See
11+
pkg/model/chk/config/generator.go getCoordinationSettings.
12+
-->
413
<coordination_settings>
5-
<async_replication>1</async_replication>
614
<min_session_timeout_ms>10000</min_session_timeout_ms>
715
<operation_timeout_ms>10000</operation_timeout_ms>
816
<raft_logs_level>information</raft_logs_level>
917
<session_timeout_ms>100000</session_timeout_ms>
10-
<use_xid_64>1</use_xid_64>
1118
</coordination_settings>
1219
<hostname_checks_enabled>true</hostname_checks_enabled>
1320
<log_storage_path>/var/lib/clickhouse-keeper/coordination/logs</log_storage_path>
@@ -29,7 +36,7 @@
2936
stricter list can override this value, but they must keep `ruok` if they
3037
also use the default operator probes.
3138
-->
32-
<four_letter_word_white_list>conf,cons,crst,envi,ruok,srst,srvr,stat,wchs,dirs,mntr,isro</four_letter_word_white_list>
39+
<four_letter_word_white_list>conf,cons,crst,envi,ruok,srst,srvr,stat,wchs,dirs,mntr,isro,rqld</four_letter_word_white_list>
3340
</keeper_server>
3441
<listen_host>::</listen_host>
3542
<listen_host>0.0.0.0</listen_host>

‎deploy/builder/templates-install-bundle/clickhouse-operator-install-yaml-template-02-section-rbac-02-role.yaml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,23 @@ rules:
8787
- get
8888
- list
8989

90+
#
91+
# coordination.k8s.io resources
92+
#
93+
94+
- apiGroups:
95+
- coordination.k8s.io
96+
resources:
97+
- leases
98+
verbs:
99+
- get
100+
- list
101+
- watch
102+
- create
103+
- update
104+
- patch
105+
- delete
106+
90107
#
91108
# apps.* resources
92109
#

‎deploy/helm/clickhouse-operator/templates/generated/ClusterRole-clickhouse-operator-kube-system.yaml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,19 @@ metadata:
1414
labels: {{ include "altinity-clickhouse-operator.labels" . | nindent 4 }}
1515
annotations: {{ include "altinity-clickhouse-operator.annotations" . | nindent 4 }}
1616
rules:
17+
# coordination.k8s.io resources (leader election)
18+
- apiGroups:
19+
- coordination.k8s.io
20+
resources:
21+
- leases
22+
verbs:
23+
- get
24+
- list
25+
- watch
26+
- create
27+
- update
28+
- patch
29+
- delete
1730
#
1831
# Core API group
1932
#

‎deploy/helm/clickhouse-operator/templates/generated/Role-clickhouse-operator.yaml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,19 @@ metadata:
1414
labels: {{ include "altinity-clickhouse-operator.labels" . | nindent 4 }}
1515
annotations: {{ include "altinity-clickhouse-operator.annotations" . | nindent 4 }}
1616
rules:
17+
# coordination.k8s.io resources (leader election)
18+
- apiGroups:
19+
- coordination.k8s.io
20+
resources:
21+
- leases
22+
verbs:
23+
- get
24+
- list
25+
- watch
26+
- create
27+
- update
28+
- patch
29+
- delete
1730
#
1831
# Core API group
1932
#

‎deploy/operator/clickhouse-operator-install-ansible.yaml‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5496,6 +5496,23 @@ rules:
54965496
- get
54975497
- list
54985498

5499+
#
5500+
# coordination.k8s.io resources
5501+
#
5502+
5503+
- apiGroups:
5504+
- coordination.k8s.io
5505+
resources:
5506+
- leases
5507+
verbs:
5508+
- get
5509+
- list
5510+
- watch
5511+
- create
5512+
- update
5513+
- patch
5514+
- delete
5515+
54995516
#
55005517
# apps.* resources
55015518
#
@@ -6705,13 +6722,20 @@ data:
67056722
<clickhouse>
67066723
<asynchronous_metrics_keeper_metrics_only>1</asynchronous_metrics_keeper_metrics_only>
67076724
<keeper_server>
6725+
<!--
6726+
async_replication and use_xid_64 are intentionally NOT set here.
6727+
They are version-gated coordination settings: older ClickHouse Keeper
6728+
releases reject them with UNKNOWN_SETTING and CrashLoop at startup
6729+
(async_replication: absent before 24.3; use_xid_64: absent before 24.12).
6730+
The operator emits them per-CR from the config generator, only for
6731+
Keeper versions known to support them. See
6732+
pkg/model/chk/config/generator.go getCoordinationSettings.
6733+
-->
67086734
<coordination_settings>
6709-
<async_replication>1</async_replication>
67106735
<min_session_timeout_ms>10000</min_session_timeout_ms>
67116736
<operation_timeout_ms>10000</operation_timeout_ms>
67126737
<raft_logs_level>information</raft_logs_level>
67136738
<session_timeout_ms>100000</session_timeout_ms>
6714-
<use_xid_64>1</use_xid_64>
67156739
</coordination_settings>
67166740
<hostname_checks_enabled>true</hostname_checks_enabled>
67176741
<log_storage_path>/var/lib/clickhouse-keeper/coordination/logs</log_storage_path>
@@ -6733,7 +6757,7 @@ data:
67336757
stricter list can override this value, but they must keep `ruok` if they
67346758
also use the default operator probes.
67356759
-->
6736-
<four_letter_word_white_list>conf,cons,crst,envi,ruok,srst,srvr,stat,wchs,dirs,mntr,isro</four_letter_word_white_list>
6760+
<four_letter_word_white_list>conf,cons,crst,envi,ruok,srst,srvr,stat,wchs,dirs,mntr,isro,rqld</four_letter_word_white_list>
67376761
</keeper_server>
67386762
<listen_host>::</listen_host>
67396763
<listen_host>0.0.0.0</listen_host>

‎deploy/operator/clickhouse-operator-install-bundle-v1beta1.yaml‎

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5365,7 +5365,6 @@ metadata:
53655365
namespace: kube-system
53665366
labels:
53675367
clickhouse.altinity.com/chop: 0.27.2
5368-
53695368
# Template Parameters:
53705369
#
53715370
# NAMESPACE=kube-system
@@ -5452,6 +5451,21 @@ rules:
54525451
- get
54535452
- list
54545453
#
5454+
# coordination.k8s.io resources
5455+
#
5456+
- apiGroups:
5457+
- coordination.k8s.io
5458+
resources:
5459+
- leases
5460+
verbs:
5461+
- get
5462+
- list
5463+
- watch
5464+
- create
5465+
- update
5466+
- patch
5467+
- delete
5468+
#
54555469
# apps.* resources
54565470
#
54575471
- apiGroups:
@@ -5618,7 +5632,6 @@ subjects:
56185632
- kind: ServiceAccount
56195633
name: clickhouse-operator
56205634
namespace: kube-system
5621-
56225635
# Template Parameters:
56235636
#
56245637
# NAMESPACE=kube-system
@@ -5705,6 +5718,21 @@ rules:
57055718
- get
57065719
- list
57075720
#
5721+
# coordination.k8s.io resources
5722+
#
5723+
- apiGroups:
5724+
- coordination.k8s.io
5725+
resources:
5726+
- leases
5727+
verbs:
5728+
- get
5729+
- list
5730+
- watch
5731+
- create
5732+
- update
5733+
- patch
5734+
- delete
5735+
#
57085736
# apps.* resources
57095737
#
57105738
- apiGroups:
@@ -6895,13 +6923,20 @@ data:
68956923
<clickhouse>
68966924
<asynchronous_metrics_keeper_metrics_only>1</asynchronous_metrics_keeper_metrics_only>
68976925
<keeper_server>
6926+
<!--
6927+
async_replication and use_xid_64 are intentionally NOT set here.
6928+
They are version-gated coordination settings: older ClickHouse Keeper
6929+
releases reject them with UNKNOWN_SETTING and CrashLoop at startup
6930+
(async_replication: absent before 24.3; use_xid_64: absent before 24.12).
6931+
The operator emits them per-CR from the config generator, only for
6932+
Keeper versions known to support them. See
6933+
pkg/model/chk/config/generator.go getCoordinationSettings.
6934+
-->
68986935
<coordination_settings>
6899-
<async_replication>1</async_replication>
69006936
<min_session_timeout_ms>10000</min_session_timeout_ms>
69016937
<operation_timeout_ms>10000</operation_timeout_ms>
69026938
<raft_logs_level>information</raft_logs_level>
69036939
<session_timeout_ms>100000</session_timeout_ms>
6904-
<use_xid_64>1</use_xid_64>
69056940
</coordination_settings>
69066941
<hostname_checks_enabled>true</hostname_checks_enabled>
69076942
<log_storage_path>/var/lib/clickhouse-keeper/coordination/logs</log_storage_path>
@@ -6923,7 +6958,7 @@ data:
69236958
stricter list can override this value, but they must keep `ruok` if they
69246959
also use the default operator probes.
69256960
-->
6926-
<four_letter_word_white_list>conf,cons,crst,envi,ruok,srst,srvr,stat,wchs,dirs,mntr,isro</four_letter_word_white_list>
6961+
<four_letter_word_white_list>conf,cons,crst,envi,ruok,srst,srvr,stat,wchs,dirs,mntr,isro,rqld</four_letter_word_white_list>
69276962
</keeper_server>
69286963
<listen_host>::</listen_host>
69296964
<listen_host>0.0.0.0</listen_host>

‎deploy/operator/clickhouse-operator-install-bundle.yaml‎

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5489,6 +5489,23 @@ rules:
54895489
- get
54905490
- list
54915491

5492+
#
5493+
# coordination.k8s.io resources
5494+
#
5495+
5496+
- apiGroups:
5497+
- coordination.k8s.io
5498+
resources:
5499+
- leases
5500+
verbs:
5501+
- get
5502+
- list
5503+
- watch
5504+
- create
5505+
- update
5506+
- patch
5507+
- delete
5508+
54925509
#
54935510
# apps.* resources
54945511
#
@@ -5755,6 +5772,23 @@ rules:
57555772
- get
57565773
- list
57575774

5775+
#
5776+
# coordination.k8s.io resources
5777+
#
5778+
5779+
- apiGroups:
5780+
- coordination.k8s.io
5781+
resources:
5782+
- leases
5783+
verbs:
5784+
- get
5785+
- list
5786+
- watch
5787+
- create
5788+
- update
5789+
- patch
5790+
- delete
5791+
57585792
#
57595793
# apps.* resources
57605794
#
@@ -6964,13 +6998,20 @@ data:
69646998
<clickhouse>
69656999
<asynchronous_metrics_keeper_metrics_only>1</asynchronous_metrics_keeper_metrics_only>
69667000
<keeper_server>
7001+
<!--
7002+
async_replication and use_xid_64 are intentionally NOT set here.
7003+
They are version-gated coordination settings: older ClickHouse Keeper
7004+
releases reject them with UNKNOWN_SETTING and CrashLoop at startup
7005+
(async_replication: absent before 24.3; use_xid_64: absent before 24.12).
7006+
The operator emits them per-CR from the config generator, only for
7007+
Keeper versions known to support them. See
7008+
pkg/model/chk/config/generator.go getCoordinationSettings.
7009+
-->
69677010
<coordination_settings>
6968-
<async_replication>1</async_replication>
69697011
<min_session_timeout_ms>10000</min_session_timeout_ms>
69707012
<operation_timeout_ms>10000</operation_timeout_ms>
69717013
<raft_logs_level>information</raft_logs_level>
69727014
<session_timeout_ms>100000</session_timeout_ms>
6973-
<use_xid_64>1</use_xid_64>
69747015
</coordination_settings>
69757016
<hostname_checks_enabled>true</hostname_checks_enabled>
69767017
<log_storage_path>/var/lib/clickhouse-keeper/coordination/logs</log_storage_path>
@@ -6992,7 +7033,7 @@ data:
69927033
stricter list can override this value, but they must keep `ruok` if they
69937034
also use the default operator probes.
69947035
-->
6995-
<four_letter_word_white_list>conf,cons,crst,envi,ruok,srst,srvr,stat,wchs,dirs,mntr,isro</four_letter_word_white_list>
7036+
<four_letter_word_white_list>conf,cons,crst,envi,ruok,srst,srvr,stat,wchs,dirs,mntr,isro,rqld</four_letter_word_white_list>
69967037
</keeper_server>
69977038
<listen_host>::</listen_host>
69987039
<listen_host>0.0.0.0</listen_host>

0 commit comments

Comments
 (0)