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/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.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..bb34d5ef5 100644 --- a/internal/controller/clickhouse/templates_test.go +++ b/internal/controller/clickhouse/templates_test.go @@ -318,6 +318,62 @@ 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..688e4da42 100644 --- a/internal/controller/keeper/templates_test.go +++ b/internal/controller/keeper/templates_test.go @@ -345,3 +345,51 @@ 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())) + }) +})