From 062fa0d771da127d62e6545bde104ff015e2f596 Mon Sep 17 00:00:00 2001 From: ganga h Date: Sun, 13 Sep 2026 18:44:39 -0500 Subject: [PATCH 1/2] feat: add TopologyMinDomains opt-in field to PodTemplateSpec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With demand-driven autoscalers (Karpenter, Cluster Autoscaler), the Kubernetes scheduler counts only topology domains (AZs) that already have eligible nodes. When topologyZoneKey generates a maxSkew:1/DoNotSchedule constraint and nodes exist in only two AZs at scheduling time, the constraint is satisfied with a 2/1 split — the autoscaler never provisions into a third AZ, leaving one replica unspread. Setting minDomains on the TopologySpreadConstraint tells the scheduler to treat N zones as domains regardless of whether nodes currently exist in all of them, forcing the autoscaler to provision into the missing zone. This commit adds an opt-in TopologyMinDomains *int32 field to PodTemplateSpec. When nil (the default), behaviour is unchanged. When set (e.g. 3 for a three-AZ cluster), minDomains is forwarded to the generated constraint for both ClickHouseCluster and KeeperCluster. Example usage: podTemplate: topologyZoneKey: topology.kubernetes.io/zone topologyMinDomains: 3 --- api/v1alpha1/common.go | 13 +++++ api/v1alpha1/zz_generated.deepcopy.go | 5 ++ .../clickhouse.com_clickhouseclusters.yaml | 14 +++++ .../bases/clickhouse.com_keeperclusters.yaml | 14 +++++ .../clickhouseclusters.clickhouse.com.yaml | 14 +++++ .../crd/keeperclusters.clickhouse.com.yaml | 14 +++++ internal/controller/clickhouse/templates.go | 1 + .../controller/clickhouse/templates_test.go | 53 +++++++++++++++++++ internal/controller/keeper/templates.go | 1 + internal/controller/keeper/templates_test.go | 46 ++++++++++++++++ 10 files changed, 175 insertions(+) diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go index 436ddb7ac..aa7b2c066 100644 --- a/api/v1alpha1/common.go +++ b/api/v1alpha1/common.go @@ -262,6 +262,19 @@ type PodTemplateSpec struct { // +optional TopologyZoneKey *string `json:"topologyZoneKey,omitempty"` + // TopologyMinDomains sets the minDomains field on the TopologySpreadConstraint generated by TopologyZoneKey. + // The Kubernetes scheduler counts only zones that already have eligible nodes as topology domains. + // With demand-driven autoscalers (e.g. Karpenter, Cluster Autoscaler) this means that if nodes + // exist in only two AZs at scheduling time, a maxSkew:1/DoNotSchedule constraint is satisfied with + // a 2/1 split and the autoscaler never provisions a node in the third AZ. + // Setting TopologyMinDomains to the expected number of zones (typically 3) tells the scheduler to + // treat that many zones as domains regardless of whether nodes exist in all of them, forcing the + // autoscaler to provision into the missing zone. + // Only takes effect when TopologyZoneKey is set. Omit (nil) to preserve the default behaviour + // of the scheduler counting only zones with existing eligible nodes. + // +optional + TopologyMinDomains *int32 `json:"topologyMinDomains,omitempty"` + // NodeHostnameKey is the key of node labels. // Nodes that have a label with this key and identical values are considered to be on the same node. // Set it to enforce exclusive node occupancy: a required PodAntiAffinity rule across all pods of the cluster, diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index b332b647a..04fceab59 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -772,6 +772,11 @@ func (in *PodTemplateSpec) DeepCopyInto(out *PodTemplateSpec) { *out = new(string) **out = **in } + if in.TopologyMinDomains != nil { + in, out := &in.TopologyMinDomains, &out.TopologyMinDomains + *out = new(int32) + **out = **in + } if in.NodeHostnameKey != nil { in, out := &in.NodeHostnameKey, &out.NodeHostnameKey *out = new(string) diff --git a/config/crd/bases/clickhouse.com_clickhouseclusters.yaml b/config/crd/bases/clickhouse.com_clickhouseclusters.yaml index 00c7fd005..8fcab14f0 100644 --- a/config/crd/bases/clickhouse.com_clickhouseclusters.yaml +++ b/config/crd/bases/clickhouse.com_clickhouseclusters.yaml @@ -4361,6 +4361,20 @@ spec: type: object type: array x-kubernetes-list-type: atomic + topologyMinDomains: + description: |- + TopologyMinDomains sets the minDomains field on the TopologySpreadConstraint generated by TopologyZoneKey. + The Kubernetes scheduler counts only zones that already have eligible nodes as topology domains. + With demand-driven autoscalers (e.g. Karpenter, Cluster Autoscaler) this means that if nodes + exist in only two AZs at scheduling time, a maxSkew:1/DoNotSchedule constraint is satisfied with + a 2/1 split and the autoscaler never provisions a node in the third AZ. + Setting TopologyMinDomains to the expected number of zones (typically 3) tells the scheduler to + treat that many zones as domains regardless of whether nodes exist in all of them, forcing the + autoscaler to provision into the missing zone. + Only takes effect when TopologyZoneKey is set. Omit (nil) to preserve the default behaviour + of the scheduler counting only zones with existing eligible nodes. + format: int32 + type: integer topologySpreadConstraints: description: |- TopologySpreadConstraints describes how a group of pods ought to spread across topology diff --git a/config/crd/bases/clickhouse.com_keeperclusters.yaml b/config/crd/bases/clickhouse.com_keeperclusters.yaml index 7ae46a115..94df150ba 100644 --- a/config/crd/bases/clickhouse.com_keeperclusters.yaml +++ b/config/crd/bases/clickhouse.com_keeperclusters.yaml @@ -4051,6 +4051,20 @@ spec: type: object type: array x-kubernetes-list-type: atomic + topologyMinDomains: + description: |- + TopologyMinDomains sets the minDomains field on the TopologySpreadConstraint generated by TopologyZoneKey. + The Kubernetes scheduler counts only zones that already have eligible nodes as topology domains. + With demand-driven autoscalers (e.g. Karpenter, Cluster Autoscaler) this means that if nodes + exist in only two AZs at scheduling time, a maxSkew:1/DoNotSchedule constraint is satisfied with + a 2/1 split and the autoscaler never provisions a node in the third AZ. + Setting TopologyMinDomains to the expected number of zones (typically 3) tells the scheduler to + treat that many zones as domains regardless of whether nodes exist in all of them, forcing the + autoscaler to provision into the missing zone. + Only takes effect when TopologyZoneKey is set. Omit (nil) to preserve the default behaviour + of the scheduler counting only zones with existing eligible nodes. + format: int32 + type: integer topologySpreadConstraints: description: |- TopologySpreadConstraints describes how a group of pods ought to spread across topology diff --git a/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml b/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml index ab82197e4..57194acff 100644 --- a/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml +++ b/dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml @@ -4364,6 +4364,20 @@ spec: type: object type: array x-kubernetes-list-type: atomic + topologyMinDomains: + description: |- + TopologyMinDomains sets the minDomains field on the TopologySpreadConstraint generated by TopologyZoneKey. + The Kubernetes scheduler counts only zones that already have eligible nodes as topology domains. + With demand-driven autoscalers (e.g. Karpenter, Cluster Autoscaler) this means that if nodes + exist in only two AZs at scheduling time, a maxSkew:1/DoNotSchedule constraint is satisfied with + a 2/1 split and the autoscaler never provisions a node in the third AZ. + Setting TopologyMinDomains to the expected number of zones (typically 3) tells the scheduler to + treat that many zones as domains regardless of whether nodes exist in all of them, forcing the + autoscaler to provision into the missing zone. + Only takes effect when TopologyZoneKey is set. Omit (nil) to preserve the default behaviour + of the scheduler counting only zones with existing eligible nodes. + format: int32 + type: integer topologySpreadConstraints: description: |- TopologySpreadConstraints describes how a group of pods ought to spread across topology diff --git a/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml b/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml index f4dcc347f..58e10829a 100644 --- a/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml +++ b/dist/chart/templates/crd/keeperclusters.clickhouse.com.yaml @@ -4054,6 +4054,20 @@ spec: type: object type: array x-kubernetes-list-type: atomic + topologyMinDomains: + description: |- + TopologyMinDomains sets the minDomains field on the TopologySpreadConstraint generated by TopologyZoneKey. + The Kubernetes scheduler counts only zones that already have eligible nodes as topology domains. + With demand-driven autoscalers (e.g. Karpenter, Cluster Autoscaler) this means that if nodes + exist in only two AZs at scheduling time, a maxSkew:1/DoNotSchedule constraint is satisfied with + a 2/1 split and the autoscaler never provisions a node in the third AZ. + Setting TopologyMinDomains to the expected number of zones (typically 3) tells the scheduler to + treat that many zones as domains regardless of whether nodes exist in all of them, forcing the + autoscaler to provision into the missing zone. + Only takes effect when TopologyZoneKey is set. Omit (nil) to preserve the default behaviour + of the scheduler counting only zones with existing eligible nodes. + format: int32 + type: integer topologySpreadConstraints: description: |- TopologySpreadConstraints describes how a group of pods ought to spread across topology diff --git a/internal/controller/clickhouse/templates.go b/internal/controller/clickhouse/templates.go index ccb57109c..af868252e 100644 --- a/internal/controller/clickhouse/templates.go +++ b/internal/controller/clickhouse/templates.go @@ -398,6 +398,7 @@ func templatePodSpec(r *clickhouseReconciler, id v1.ClickHouseReplicaID) (corev1 MaxSkew: 1, TopologyKey: zoneKey, WhenUnsatisfiable: corev1.DoNotSchedule, + MinDomains: cr.Spec.PodTemplate.TopologyMinDomains, LabelSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ controllerutil.LabelAppKey: cr.SpecificName(), diff --git a/internal/controller/clickhouse/templates_test.go b/internal/controller/clickhouse/templates_test.go index b5fc76f8c..ca8d5eea0 100644 --- a/internal/controller/clickhouse/templates_test.go +++ b/internal/controller/clickhouse/templates_test.go @@ -318,6 +318,59 @@ var _ = Describe("SecurityContext defaults", func() { }) }) +var _ = Describe("TopologySpreadConstraints", func() { + newCluster := func(zoneKey string, minDomains *int32) *v1.ClickHouseCluster { + spec := v1.PodTemplateSpec{} + if zoneKey != "" { + spec.TopologyZoneKey = &zoneKey + } + spec.TopologyMinDomains = minDomains + return &v1.ClickHouseCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Spec: v1.ClickHouseClusterSpec{PodTemplate: spec}, + } + } + int32p := func(v int32) *int32 { return &v } + + It("should not set MinDomains when topologyZoneKey is unset", func() { + r := &clickhouseReconciler{Cluster: newCluster("", nil)} + podSpec, err := templatePodSpec(r, v1.ClickHouseReplicaID{}) + Expect(err).NotTo(HaveOccurred()) + Expect(podSpec.TopologySpreadConstraints).To(BeEmpty()) + }) + + It("should set the zone TopologySpreadConstraint without MinDomains when topologyMinDomains is unset", func() { + r := &clickhouseReconciler{Cluster: newCluster("topology.kubernetes.io/zone", nil)} + podSpec, err := templatePodSpec(r, v1.ClickHouseReplicaID{ShardID: 1}) + Expect(err).NotTo(HaveOccurred()) + Expect(podSpec.TopologySpreadConstraints).To(HaveLen(1)) + tsc := podSpec.TopologySpreadConstraints[0] + Expect(tsc.TopologyKey).To(Equal("topology.kubernetes.io/zone")) + Expect(tsc.MaxSkew).To(BeEquivalentTo(1)) + Expect(tsc.WhenUnsatisfiable).To(Equal(corev1.DoNotSchedule)) + Expect(tsc.MinDomains).To(BeNil()) + Expect(tsc.LabelSelector.MatchLabels).To(HaveKeyWithValue("clickhouse.com/shard-id", "1")) + }) + + It("should set MinDomains on the zone constraint when topologyMinDomains is specified", func() { + r := &clickhouseReconciler{Cluster: newCluster("topology.kubernetes.io/zone", int32p(3))} + podSpec, err := templatePodSpec(r, v1.ClickHouseReplicaID{ShardID: 0}) + Expect(err).NotTo(HaveOccurred()) + Expect(podSpec.TopologySpreadConstraints).To(HaveLen(1)) + tsc := podSpec.TopologySpreadConstraints[0] + Expect(tsc.MinDomains).NotTo(BeNil()) + Expect(*tsc.MinDomains).To(BeEquivalentTo(3)) + }) + + It("should scope the zone constraint to the specific shard so cross-shard spread is independent", func() { + r := &clickhouseReconciler{Cluster: newCluster("topology.kubernetes.io/zone", int32p(3))} + podSpec0, _ := templatePodSpec(r, v1.ClickHouseReplicaID{ShardID: 0}) + podSpec2, _ := templatePodSpec(r, v1.ClickHouseReplicaID{ShardID: 2}) + Expect(podSpec0.TopologySpreadConstraints[0].LabelSelector.MatchLabels).To(HaveKeyWithValue("clickhouse.com/shard-id", "0")) + Expect(podSpec2.TopologySpreadConstraints[0].LabelSelector.MatchLabels).To(HaveKeyWithValue("clickhouse.com/shard-id", "2")) + }) +}) + var _ = Describe("Service templates", func() { cr := &v1.ClickHouseCluster{ Name: "test", diff --git a/internal/controller/keeper/templates.go b/internal/controller/keeper/templates.go index fb7cc3741..111fe0e84 100644 --- a/internal/controller/keeper/templates.go +++ b/internal/controller/keeper/templates.go @@ -425,6 +425,7 @@ func templatePodSpec(cr *v1.KeeperCluster, id v1.KeeperReplicaID) (corev1.PodSpe MaxSkew: 1, TopologyKey: *podTemplate.TopologyZoneKey, WhenUnsatisfiable: corev1.DoNotSchedule, + MinDomains: podTemplate.TopologyMinDomains, LabelSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ controllerutil.LabelAppKey: cr.SpecificName(), diff --git a/internal/controller/keeper/templates_test.go b/internal/controller/keeper/templates_test.go index 82c81aaf5..9deb21cb3 100644 --- a/internal/controller/keeper/templates_test.go +++ b/internal/controller/keeper/templates_test.go @@ -345,3 +345,49 @@ var _ = Describe("TemplateNetworkPolicy", func() { Expect(templateNetworkPolicy(cluster, nil)).To(Equal(templateNetworkPolicy(cluster, nil))) }) }) + +var _ = Describe("TopologySpreadConstraints", func() { + newKeeperCluster := func(zoneKey string, minDomains *int32) *v1.KeeperCluster { + spec := v1.PodTemplateSpec{} + if zoneKey != "" { + spec.TopologyZoneKey = &zoneKey + } + spec.TopologyMinDomains = minDomains + return &v1.KeeperCluster{ + Name: "test", + Spec: v1.KeeperClusterSpec{PodTemplate: spec}, + } + } + int32p := func(v int32) *int32 { return &v } + + It("should not set topology constraints when topologyZoneKey is unset", func() { + cr := newKeeperCluster("", nil) + podSpec, err := templatePodSpec(cr, v1.KeeperReplicaID(0)) + Expect(err).NotTo(HaveOccurred()) + Expect(podSpec.TopologySpreadConstraints).To(BeEmpty()) + }) + + It("should set zone TopologySpreadConstraint without MinDomains when topologyMinDomains is unset", func() { + cr := newKeeperCluster("topology.kubernetes.io/zone", nil) + podSpec, err := templatePodSpec(cr, v1.KeeperReplicaID(0)) + Expect(err).NotTo(HaveOccurred()) + Expect(podSpec.TopologySpreadConstraints).To(HaveLen(1)) + tsc := podSpec.TopologySpreadConstraints[0] + Expect(tsc.TopologyKey).To(Equal("topology.kubernetes.io/zone")) + Expect(tsc.MaxSkew).To(BeEquivalentTo(1)) + Expect(tsc.WhenUnsatisfiable).To(Equal(corev1.DoNotSchedule)) + Expect(tsc.MinDomains).To(BeNil()) + Expect(tsc.LabelSelector.MatchLabels).To(HaveKeyWithValue(controllerutil.LabelRoleKey, controllerutil.LabelKeeperValue)) + }) + + It("should set MinDomains on the zone constraint when topologyMinDomains is specified", func() { + cr := newKeeperCluster("topology.kubernetes.io/zone", int32p(3)) + podSpec, err := templatePodSpec(cr, v1.KeeperReplicaID(0)) + Expect(err).NotTo(HaveOccurred()) + Expect(podSpec.TopologySpreadConstraints).To(HaveLen(1)) + tsc := podSpec.TopologySpreadConstraints[0] + Expect(tsc.MinDomains).NotTo(BeNil()) + Expect(*tsc.MinDomains).To(BeEquivalentTo(3)) + Expect(tsc.LabelSelector.MatchLabels).To(HaveKeyWithValue(controllerutil.LabelAppKey, cr.SpecificName())) + }) +}) From 9cf17ab7898ace98901d676d9c2bbf5297eb669e Mon Sep 17 00:00:00 2001 From: ganga h Date: Mon, 14 Sep 2026 10:07:55 -0500 Subject: [PATCH 2/2] fix: add missing blank lines for wsl_v5 and regenerate API reference docs Fixes golangci-lint wsl_v5 failures: adds required blank lines before assignments that follow a closing block and before return statements in templates_test.go for both clickhouse and keeper controllers. Also adds the missing topologyMinDomains row to docs/reference/api-reference.mdx so the API Reference Generated CI check passes. --- docs/reference/api-reference.mdx | 1 + internal/controller/clickhouse/templates_test.go | 3 +++ internal/controller/keeper/templates_test.go | 2 ++ 3 files changed, 6 insertions(+) diff --git a/docs/reference/api-reference.mdx b/docs/reference/api-reference.mdx index 8751ee9a3..280cad1af 100644 --- a/docs/reference/api-reference.mdx +++ b/docs/reference/api-reference.mdx @@ -474,6 +474,7 @@ PodTemplateSpec describes the pod configuration overrides for the cluster's pods | `volumes` | [Volume](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#volume-v1-core) array | Volumes defines the list of volumes that can be mounted by containers belonging to the pod.
More info: https://kubernetes.io/docs/concepts/storage/volumes
Merged with operator defaults by name; a user volume replaces any operator volume with the same name. | false | | | `securityContext` | [PodSecurityContext](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#podsecuritycontext-v1-core) | SecurityContext holds pod-level security attributes and common container settings.
A non-nil SecurityContext fully replaces operator defaults; the user owns the
entire struct. When nil, operator defaults are preserved. | false | | | `topologyZoneKey` | string | TopologyZoneKey is the key of node labels.
Nodes that have a label with this key and identical values are considered to be in the same topology domain,
e.g. the same availability zone for "topology.kubernetes.io/zone" or the same node for "kubernetes.io/hostname".
Set it to balance pods holding the same data (replicas of one shard for ClickHouse, all replicas for Keeper)
across domains: a required TopologySpreadConstraint (maxSkew 1, DoNotSchedule) plus a preferred PodAntiAffinity rule.
Pods of different shards may share a domain, so multiple pods per domain are allowed.
With fewer domains than replicas, several replicas share a domain, still evenly balanced.
ClickHouse pods additionally prefer domains hosting the Keeper pods they use.
Recommended to be set to "topology.kubernetes.io/zone". | false | | +| `topologyMinDomains` | integer | TopologyMinDomains sets the minDomains field on the TopologySpreadConstraint generated by TopologyZoneKey.
The Kubernetes scheduler counts only zones that already have eligible nodes as topology domains.
With demand-driven autoscalers (e.g. Karpenter, Cluster Autoscaler) this means that if nodes
exist in only two AZs at scheduling time, a maxSkew:1/DoNotSchedule constraint is satisfied with
a 2/1 split and the autoscaler never provisions a node in the third AZ.
Setting TopologyMinDomains to the expected number of zones (typically 3) tells the scheduler to
treat that many zones as domains regardless of whether nodes exist in all of them, forcing the
autoscaler to provision into the missing zone.
Only takes effect when TopologyZoneKey is set. Omit (nil) to preserve the default behaviour
of the scheduler counting only zones with existing eligible nodes. | false | | | `nodeHostnameKey` | string | NodeHostnameKey is the key of node labels.
Nodes that have a label with this key and identical values are considered to be on the same node.
Set it to enforce exclusive node occupancy: a required PodAntiAffinity rule across all pods of the cluster,
at most one pod per node regardless of shard. Requires at least as many nodes as pods;
with fewer nodes the excess pods stay Pending.
To spread replicas of the same shard across nodes while packing several pods per node,
set TopologyZoneKey to "kubernetes.io/hostname" instead.
Recommended to be set to "kubernetes.io/hostname". | false | | | `initContainers` | [Container](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#container-v1-core) array | InitContainers is the list of init containers to run before the main server container starts.
Merged with operator defaults by name.
with the same name. | false | | diff --git a/internal/controller/clickhouse/templates_test.go b/internal/controller/clickhouse/templates_test.go index ca8d5eea0..bb34d5ef5 100644 --- a/internal/controller/clickhouse/templates_test.go +++ b/internal/controller/clickhouse/templates_test.go @@ -324,7 +324,9 @@ var _ = Describe("TopologySpreadConstraints", func() { if zoneKey != "" { spec.TopologyZoneKey = &zoneKey } + spec.TopologyMinDomains = minDomains + return &v1.ClickHouseCluster{ ObjectMeta: metav1.ObjectMeta{Name: "test"}, Spec: v1.ClickHouseClusterSpec{PodTemplate: spec}, @@ -366,6 +368,7 @@ var _ = Describe("TopologySpreadConstraints", func() { r := &clickhouseReconciler{Cluster: newCluster("topology.kubernetes.io/zone", int32p(3))} podSpec0, _ := templatePodSpec(r, v1.ClickHouseReplicaID{ShardID: 0}) podSpec2, _ := templatePodSpec(r, v1.ClickHouseReplicaID{ShardID: 2}) + Expect(podSpec0.TopologySpreadConstraints[0].LabelSelector.MatchLabels).To(HaveKeyWithValue("clickhouse.com/shard-id", "0")) Expect(podSpec2.TopologySpreadConstraints[0].LabelSelector.MatchLabels).To(HaveKeyWithValue("clickhouse.com/shard-id", "2")) }) diff --git a/internal/controller/keeper/templates_test.go b/internal/controller/keeper/templates_test.go index 9deb21cb3..688e4da42 100644 --- a/internal/controller/keeper/templates_test.go +++ b/internal/controller/keeper/templates_test.go @@ -352,7 +352,9 @@ var _ = Describe("TopologySpreadConstraints", func() { if zoneKey != "" { spec.TopologyZoneKey = &zoneKey } + spec.TopologyMinDomains = minDomains + return &v1.KeeperCluster{ Name: "test", Spec: v1.KeeperClusterSpec{PodTemplate: spec},