Repository: pravega/zookeeper-operator (go)

Issue:
Add support for topologySearchConstraints in Zookeeper helm charts
### Description

_Add support for [topologySpreadConstraints](https://kubernetes.io/docs/concepts/workloads/pods/pod-topology-spread-constraints/) in Zookeeper helm charts

### Importance

_should-have_

### Location

[_zookeeper.yaml_](https://github.com/pravega/zookeeper-operator/blob/master/charts/zookeeper/templates/zookeeper.yaml)

### Suggestions for an improvement

_Very similar to how support is provided for POD anti-affinity_


Pull request:
Add topologySpreadConstraints to PodSpec
### Change log description

Adds `topologySpreadConstraints` support to pod deployment definition.

### Purpose of the change

Fixes #427

### What the code does

Adds an existing specification to the Spec.Pod to aid in topology spread.

### How to verify it

Add a `topologySpreadConstraints` and see if it shows up in the Zookeeper StatefulSet pod spec.

Patch:
diff --git a/api/v1beta1/zookeepercluster_types.go b/api/v1beta1/zookeepercluster_types.go
index 3f5da83..8985dab 100644
--- a/api/v1beta1/zookeepercluster_types.go
+++ b/api/v1beta1/zookeepercluster_types.go
@@ -445,6 +445,9 @@ type PodPolicy struct {
 	// The scheduling constraints on pods.
 	Affinity *v1.Affinity `json:"affinity,omitempty"`
 
+	// TopologySpreadConstraints to apply to the pods
+	TopologySpreadConstraints []v1.TopologySpreadConstraint `json:"topologySpreadConstraints,omitempty"`
+
 	// Resources is the resource requirements for the container.
 	// This field cannot be updated once the cluster is created.
 	Resources v1.ResourceRequirements `json:"resources,omitempty"`
diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go
index 409d989..e712f6c 100644
--- a/api/v1beta1/zz_generated.deepcopy.go
+++ b/api/v1beta1/zz_generated.deepcopy.go
@@ -192,6 +192,13 @@ func (in *PodPolicy) DeepCopyInto(out *PodPolicy) {
 		*out = new(v1.Affinity)
 		(*in).DeepCopyInto(*out)
 	}
+	if in.TopologySpreadConstraints != nil {
+		in, out := &in.TopologySpreadConstraints, &out.TopologySpreadConstraints
+		*out = make([]v1.TopologySpreadConstraint, len(*in))
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
+		}
+	}
 	in.Resources.DeepCopyInto(&out.Resources)
 	if in.Tolerations != nil {
 		in, out := &in.Tolerations, &out.Tolerations
diff --git a/charts/zookeeper-operator/templates/zookeeper.pravega.io_zookeeperclusters_crd.yaml b/charts/zookeeper-operator/templates/zookeeper.pravega.io_zookeeperclusters_crd.yaml
index 03555d8..c38f3a4 100644
--- a/charts/zookeeper-operator/templates/zookeeper.pravega.io_zookeeperclusters_crd.yaml
+++ b/charts/zookeeper-operator/templates/zookeeper.pravega.io_zookeeperclusters_crd.yaml
@@ -4133,6 +4133,144 @@ spec:
                           type: string
                       type: object
                     type: array
+                  topologySpreadConstraints:
+                    description: TopologySpreadConstraints to apply to the pods
+                    items:
+                      description: TopologySpreadConstraint specifies how to spread
+                        matching pods among the given topology.
+                      properties:
+                        labelSelector:
+                          description: LabelSelector is used to find matching pods.
+                            Pods that match this label selector are counted to determine
+                            the number of pods in their corresponding topology domain.
+                          properties:
+                            matchExpressions:
+                              description: matchExpressions is a list of label selector
+                                requirements. The requirements are ANDed.
+                              items:
+                                description: A label selector requirement is a selector
+                                  that contains values, a key, and an operator that
+                                  relates the key and values.
+                                properties:
+                                  key:
+                                    description: key is the label key that the selector
+                                      applies to.
+                                    type: string
+                                  operator:
+                                    description: operator represents a key's relationship
+                                      to a set of values. Valid operators are In,
+                                      NotIn, Exists and DoesNotExist.
+                                    type: string
+                                  values:
+                                    description: values is an array of string values.
+                                      If the operator is In or NotIn, the values array
+                                      must be non-empty. If the operator is Exists
+                                      or DoesNotExist, the values array must be empty.
+                                      This array is replaced during a strategic merge
+                                      patch.
+                                    items:
+                                      type: string
+                                    type: array
+                                required:
+                                - key
+                                - operator
+                                type: object
+                              type: array
+                            matchLabels:
+                              additionalProperties:
+                                type: string
+                              description: matchLabels is a map of {key,value} pairs.
+                                A single {key,value} in the matchLabels map is equivalent
+                                to an element of matchExpressions, whose key field
+                                is "key", the operator is "In", and the values array
+                                contains only "value". The requirements are ANDed.
+                              type: object
+                          type: object
+                        maxSkew:
+                          description: 'MaxSkew describes the degree to which pods
+                            may be unevenly distributed. When `whenUnsatisfiable=DoNotSchedule`,
+                            it is the maximum permitted difference between the number
+                            of matching pods in the target topology and the global
+                            minimum. The global minimum is the minimum number of matching
+                            pods in an eligible domain or zero if the number of eligible
+                            domains is less than MinDomains. For example, in a 3-zone
+                            cluster, MaxSkew is set to 1, and pods with the same labelSelector
+                            spread as 2/2/1: In this case, the global minimum is 1.
+                            | zone1 | zone2 | zone3 | |  P P  |  P P  |   P   | -
+                            if MaxSkew is 1, incoming pod can only be scheduled to
+                            zone3 to become 2/2/2; scheduling it onto zone1(zone2)
+                            would make the ActualSkew(3-1) on zone1(zone2) violate
+                            MaxSkew(1). - if MaxSkew is 2, incoming pod can be scheduled
+                            onto any zone. When `whenUnsatisfiable=ScheduleAnyway`,
+                            it is used to give higher precedence to topologies that
+                            satisfy it. It''s a required field. Default value is 1
+                            and 0 is not allowed.'
+                          format: int32
+                          type: integer
+                        minDomains:
+                          description: "MinDomains indicates a minimum number of eligible
+                            domains. When the number of eligible domains with matching
+                            topology keys is less than minDomains, Pod Topology Spread
+                            treats \"global minimum\" as 0, and then the calculation
+                            of Skew is performed. And when the number of eligible
+                            domains with matching topology keys equals or greater
+                            than minDomains, this value has no effect on scheduling.
+                            As a result, when the number of eligible domains is less
+                            than minDomains, scheduler won't schedule more than maxSkew
+                            Pods to those domains. If value is nil, the constraint
+                            behaves as if MinDomains is equal to 1. Valid values are
+                            integers greater than 0. When value is not nil, WhenUnsatisfiable
+                            must be DoNotSchedule. \n For example, in a 3-zone cluster,
+                            MaxSkew is set to 2, MinDomains is set to 5 and pods with
+                            the same labelSelector spread as 2/2/2: | zone1 | zone2
+                            | zone3 | |  P P  |  P P  |  P P  | The number of domains
+                            is less than 5(MinDomains), so \"global minimum\" is treated
+                            as 0. In this situation, new pod with the same labelSelector
+                            cannot be scheduled, because computed skew will be 3(3
+                            - 0) if new Pod is scheduled to any of the three zones,
+                            it will violate MaxSkew. \n This is an alpha field and
+                            requires enabling MinDomainsInPodTopologySpread feature
+                            gate."
+                          format: int32
+                          type: integer
+                        topologyKey:
+                          description: TopologyKey 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. We consider each
+                            <key, value> as a "bucket", and try to put balanced number
+                            of pods into each bucket. We define a domain as a particular
+                            instance of a topology. Also, we define an eligible domain
+                            as a domain whose nodes match the node selector. e.g.
+                            If TopologyKey is "kubernetes.io/hostname", each Node
+                            is a domain of that topology. And, if TopologyKey is "topology.kubernetes.io/zone",
+                            each zone is a domain of that topology. It's a required
+                            field.
+                          type: string
+                        whenUnsatisfiable:
+                          description: 'WhenUnsatisfiable indicates how to deal with
+                            a pod if it doesn''t satisfy the spread constraint. -
+                            DoNotSchedule (default) tells the scheduler not to schedule
+                            it. - ScheduleAnyway tells the scheduler to schedule the
+                            pod in any location, but giving higher precedence to topologies
+                            that would help reduce the skew. A constraint is considered
+                            "Unsatisfiable" for an incoming pod if and only if every
+                            possible node assignment for that pod would violate "MaxSkew"
+                            on some topology. For example, in a 3-zone cluster, MaxSkew
+                            is set to 1, and pods with the same labelSelector spread
+                            as 3/1/1: | zone1 | zone2 | zone3 | | P P P |   P   |   P   |
+                            If WhenUnsatisfiable is set to DoNotSchedule, incoming
+                            pod can only be scheduled to zone2(zone3) to become 3/2/1(3/1/2)
+                            as ActualSkew(2-1) on zone2(zone3) satisfies MaxSkew(1).
+                            In other words, the cluster can still be imbalanced, but
+                            scheduler won''t make it *more* imbalanced. It''s a required
+                            field.'
+                          type: string
+                      required:
+                      - maxSkew
+                      - topologyKey
+                      - whenUnsatisfiable
+                      type: object
+                    type: array
                 type: object
               ports:
                 items:
diff --git a/charts/zookeeper/templates/zookeeper.yaml b/charts/zookeeper/templates/zookeeper.yaml
index 6a877a8..902440e 100644
--- a/charts/zookeeper/templates/zookeeper.yaml
+++ b/charts/zookeeper/templates/zookeeper.yaml
@@ -76,6 +76,10 @@ spec:
     {{- if .Values.pod.affinity }}
     affinity:
 {{ toYaml .Values.pod.affinity | indent 6 }}
+    {{- end }}
+    {{- if .Values.pod.topologySpreadConstraints }}
+    topologySpreadConstraints:
+{{ toYaml .Values.pod.topologySpreadConstraints | indent 6 }}
     {{- end }}
     {{- if .Values.pod.resources }}
     resources:
diff --git a/charts/zookeeper/values.yaml b/charts/zookeeper/values.yaml
index 0209149..d7977ec 100644
--- a/charts/zookeeper/values.yaml
+++ b/charts/zookeeper/values.yaml
@@ -36,6 +36,7 @@ pod:
   # labels: {}
   # nodeSelector: {}
   # affinity: {}
+  # topologySpreadConstraints: {}
   # resources: {}
   # tolerations: []
   # env: []
diff --git a/config/crd/bases/zookeeper.pravega.io_zookeeperclusters.yaml b/config/crd/bases/zookeeper.pravega.io_zookeeperclusters.yaml
index 46f114d..af5084e 100644
--- a/config/crd/bases/zookeeper.pravega.io_zookeeperclusters.yaml
+++ b/config/crd/bases/zookeeper.pravega.io_zookeeperclusters.yaml
@@ -4132,6 +4132,144 @@ spec:
                           type: string
                       type: object
                     type: array
+                  topologySpreadConstraints:
+                    description: TopologySpreadConstraints to apply to the pods
+                    items:
+                      description: TopologySpreadConstraint specifies how to spread
+                        matching pods among the given topology.
+                      properties:
+                        labelSelector:
+                          description: LabelSelector is used to find matching pods.
+                            Pods that match this label selector are counted to determine
+                            the number of pods in their corresponding topology domain.
+                          properties:
+                            matchExpressions:
+                              description: matchExpressions is a list of label selector
+                                requirements. The requirements are ANDed.
+                              items:
+                                description: A label selector requirement is a selector
+                                  that contains values, a key, and an operator that
+                                  relates the key and values.
+                                properties:
+                                  key:
+                                    description: key is the label key that the selector
+                                      applies to.
+                                    type: string
+                                  operator:
+                                    description: operator represents a key's relationship
+                                      to a set of values. Valid operators are In,
+                                      NotIn, Exists and DoesNotExist.
+                                    type: string
+                                  values:
+                                    description: values is an array of string values.
+                                      If the operator is In or NotIn, the values array
+                                      must be non-empty. If the operator is Exists
+                                      or DoesNotExist, the values array must be empty.
+                                      This array is replaced during a strategic merge
+                                      patch.
+                                    items:
+                                      type: string
+                                    type: array
+                                required:
+                                - key
+                                - operator
+                                type: object
+                              type: array
+                            matchLabels:
+                              additionalProperties:
+                                type: string
+                              description: matchLabels is a map of {key,value} pairs.
+                                A single {key,value} in the matchLabels map is equivalent
+                                to an element of matchExpressions, whose key field
+                                is "key", the operator is "In", and the values array
+                                contains only "value". The requirements are ANDed.
+                              type: object
+                          type: object
+                        maxSkew:
+                          description: 'MaxSkew describes the degree to which pods
+                            may be unevenly distributed. When `whenUnsatisfiable=DoNotSchedule`,
+                            it is the maximum permitted difference between the number
+                            of matching pods in the target topology and the global
+                            minimum. The global minimum is the minimum number of matching
+                            pods in an eligible domain or zero if the number of eligible
+                            domains is less than MinDomains. For example, in a 3-zone
+                            cluster, MaxSkew is set to 1, and pods with the same labelSelector
+                            spread as 2/2/1: In this case, the global minimum is 1.
+                            | zone1 | zone2 | zone3 | |  P P  |  P P  |   P   | -
+                            if MaxSkew is 1, incoming pod can only be scheduled to
+                            zone3 to become 2/2/2; scheduling it onto zone1(zone2)
+                            would make the ActualSkew(3-1) on zone1(zone2) violate
+                            MaxSkew(1). - if MaxSkew is 2, incoming pod can be scheduled
+                            onto any zone. When `whenUnsatisfiable=ScheduleAnyway`,
+                            it is used to give higher precedence to topologies that
+                            satisfy it. It''s a required field. Default value is 1
+                            and 0 is not allowed.'
+                          format: int32
+                          type: integer
+                        minDomains:
+                          description: "MinDomains indicates a minimum number of eligible
+                            domains. When the number of eligible domains with matching
+                            topology keys is less than minDomains, Pod Topology Spread
+                            treats \"global minimum\" as 0, and then the calculation
+                            of Skew is performed. And when the number of eligible
+                            domains with matching topology keys equals or greater
+                            than minDomains, this value has no effect on scheduling.
+                            As a result, when the number of eligible domains is less
+                            than minDomains, scheduler won't schedule more than maxSkew
+                            Pods to those domains. If value is nil, the constraint
+                            behaves as if MinDomains is equal to 1. Valid values are
+                            integers greater than 0. When value is not nil, WhenUnsatisfiable
+                            must be DoNotSchedule. \n For example, in a 3-zone cluster,
+                            MaxSkew is set to 2, MinDomains is set to 5 and pods with
+                            the same labelSelector spread as 2/2/2: | zone1 | zone2
+                            | zone3 | |  P P  |  P P  |  P P  | The number of domains
+                            is less than 5(MinDomains), so \"global minimum\" is treated
+                            as 0. In this situation, new pod with the same labelSelector
+                            cannot be scheduled, because computed skew will be 3(3
+                            - 0) if new Pod is scheduled to any of the three zones,
+                            it will violate MaxSkew. \n This is an alpha field and
+                            requires enabling MinDomainsInPodTopologySpread feature
+                            gate."
+                          format: int32
+                          type: integer
+                        topologyKey:
+                          description: TopologyKey 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. We consider each
+                            <key, value> as a "bucket", and try to put balanced number
+                            of pods into each bucket. We define a domain as a particular
+                            instance of a topology. Also, we define an eligible domain
+                            as a domain whose nodes match the node selector. e.g.
+                            If TopologyKey is "kubernetes.io/hostname", each Node
+                            is a domain of that topology. And, if TopologyKey is "topology.kubernetes.io/zone",
+                            each zone is a domain of that topology. It's a required
+                            field.
+                          type: string
+                        whenUnsatisfiable:
+                          description: 'WhenUnsatisfiable indicates how to deal with
+                            a pod if it doesn''t satisfy the spread constraint. -
+                            DoNotSchedule (default) tells the scheduler not to schedule
+                            it. - ScheduleAnyway tells the scheduler to schedule the
+                            pod in any location, but giving higher precedence to topologies
+                            that would help reduce the skew. A constraint is considered
+                            "Unsatisfiable" for an incoming pod if and only if every
+                            possible node assignment for that pod would violate "MaxSkew"
+                            on some topology. For example, in a 3-zone cluster, MaxSkew
+                            is set to 1, and pods with the same labelSelector spread
+                            as 3/1/1: | zone1 | zone2 | zone3 | | P P P |   P   |   P   |
+                            If WhenUnsatisfiable is set to DoNotSchedule, incoming
+                            pod can only be scheduled to zone2(zone3) to become 3/2/1(3/1/2)
+                            as ActualSkew(2-1) on zone2(zone3) satisfies MaxSkew(1).
+                            In other words, the cluster can still be imbalanced, but
+                            scheduler won''t make it *more* imbalanced. It''s a required
+                            field.'
+                          type: string
+                      required:
+                      - maxSkew
+                      - topologyKey
+                      - whenUnsatisfiable
+                      type: object
+                    type: array
                 type: object
               ports:
                 items:
diff --git a/pkg/zk/generators.go b/pkg/zk/generators.go
index 07720c1..fddb487 100644
--- a/pkg/zk/generators.go
+++ b/pkg/zk/generators.go
@@ -172,9 +172,10 @@ func makeZkPodSpec(z *v1beta1.ZookeeperCluster, volumes []v1.Volume) v1.PodSpec
 
 	zkContainer.Env = append(zkContainer.Env, z.Spec.Pod.Env...)
 	podSpec := v1.PodSpec{
-		Containers: append(z.Spec.Containers, zkContainer),
-		Affinity:   z.Spec.Pod.Affinity,
-		Volumes:    append(z.Spec.Volumes, volumes...),
+		Containers:                append(z.Spec.Containers, zkContainer),
+		Affinity:                  z.Spec.Pod.Affinity,
+		TopologySpreadConstraints: z.Spec.Pod.TopologySpreadConstraints,
+		Volumes:                   append(z.Spec.Volumes, volumes...),
 	}
 	if !reflect.DeepEqual(v1.PodSecurityContext{}, z.Spec.Pod.SecurityContext) {
 		podSpec.SecurityContext = z.Spec.Pod.SecurityContext


Test patch:
diff --git a/api/v1beta1/deepcopy_test.go b/api/v1beta1/deepcopy_test.go
index 52edca2..5743a5a 100644
--- a/api/v1beta1/deepcopy_test.go
+++ b/api/v1beta1/deepcopy_test.go
@@ -173,6 +173,9 @@ var _ = Describe("ZookeeperCluster DeepCopy", func() {
 		It("value of Tol should be example", func() {
 			Ω(z2.Spec.Pod.Tolerations[0].Key).To(Equal("tol"))
 		})
+		It("have empty topologySpreadConstraints", func() {
+			Ω(z2.Spec.Pod.TopologySpreadConstraints).To(HaveLen(0))
+		})
 		It("checking status conditions", func() {
 			Ω(z2.Status.Conditions[0].Reason).To(Equal(z1.Status.Conditions[0].Reason))
 		})
@@ -242,6 +245,14 @@ var _ = Describe("ZookeeperCluster DeepCopy", func() {
 			podpolicy2 := podpolicy.DeepCopy()
 			Ω(podpolicy2).To(BeNil())
 		})
+		It("checking for deepcopy podpolicy.topologyspreadconstraints", func() {
+			t := v1.TopologySpreadConstraint{}
+			podpolicy := v1beta1.PodPolicy{
+				TopologySpreadConstraints: []v1.TopologySpreadConstraint{t},
+			}
+			podpolicy2 := podpolicy.DeepCopy()
+			Ω(podpolicy2.TopologySpreadConstraints).To(HaveLen(1))
+		})
 		It("checking for nil zookeepercluster", func() {
 			var zk *v1beta1.ZookeeperCluster
 			zk2 := zk.DeepCopy()
diff --git a/pkg/zk/generators_test.go b/pkg/zk/generators_test.go
index a077bb9..f545ddb 100644
--- a/pkg/zk/generators_test.go
+++ b/pkg/zk/generators_test.go
@@ -209,6 +209,10 @@ var _ = Describe("Generators Spec", func() {
 					"exampleLabel",
 					"exampleValue"))
 			})
+
+			It("should have blank topologySpreadConstraints", func() {
+				Ω(sts.Spec.Template.Spec.TopologySpreadConstraints).To(HaveLen(0))
+			})
 		})
 
 		Context("with pod policy annotations", func() {