Issue 307: Provide a way for customization of annotations for service… (

#308) * Issue 307: Provide a way for customization of annotations for services and for making LoadBalancer optional for AdminServer service * fix default values adminServerService, clientService, headlessService * improve checking optional fields under headlessService, clientService, clientService * sync crds to fix e2e tests * invert behaviour of AdminServerService - make it internal by default and add LoadBalancer only when adminServerService.external: true Co-authored-by: Alim Akbashev <[email protected]>
pravega · Mar 22, 2021 · b1519ae · b1519ae
1 parent c25d193
commit b1519ae
Show file tree

Hide file tree

Showing 8 changed files with 224 additions and 11 deletions.
diff --git a/charts/zookeeper-operator/templates/zookeeper.pravega.io_zookeeperclusters_crd.yaml b/charts/zookeeper-operator/templates/zookeeper.pravega.io_zookeeperclusters_crd.yaml
@@ -2400,6 +2400,44 @@ spec:
                     type: object
                   type: array
               type: object
+            clientService:
+              description: ClientService defines the policy to create client Service
+                for the zookeeper cluster.
+              properties:
+                annotations:
+                  additionalProperties:
+                    type: string
+                  description: Annotations specifies the annotations to attach to
+                    client Service the operator creates.
+                  type: object
+              type: object
+            headlessService:
+              description: HeadlessService defines the policy to create headless Service
+                for the zookeeper cluster.
+              properties:
+                annotations:
+                  additionalProperties:
+                    type: string
+                  description: Annotations specifies the annotations to attach to
+                    headless Service the operator creates.
+                  type: object
+              type: object
+            adminServerService:
+              description: AdminServerService defines the policy to create AdminServer Service
+                for the zookeeper cluster.
+              properties:
+                external:
+                  description: External specifies if LoadBalancer should be created for
+                    the AdminServer. True means LoadBalancer will be created, false means ClusterIP
+                    will be used. Default is false.
+                  type: boolean
+                annotations:
+                  additionalProperties:
+                    type: string
+                  description: Annotations specifies the annotations to attach to AdminServer
+                    Service the operator creates.
+                  type: object
+              type: object
             ports:
               items:
                 description: ContainerPort represents a network port in a single container.

diff --git a/charts/zookeeper/README.md b/charts/zookeeper/README.md
@@ -82,6 +82,13 @@ The following table lists the configurable parameters of the zookeeper chart and
 | `pod.terminationGracePeriodSeconds` | Amount of time given to the pod to shutdown normally | `30` |
 | `pod.serviceAccountName` | Name for the service account | `zookeeper` |
 | `pod.imagePullSecrets` | ImagePullSecrets is a list of references to secrets in the same namespace to use for pulling any images. | `[]` |
+| `clientService` | Defines the policy to create client Service for the zookeeper cluster. | {} |
+| `clientService.annotations` | Specifies the annotations to attach to client Service the operator creates. | {} |
+| `headlessService` | Defines the policy to create headless Service for the zookeeper cluster. | {} |
+| `headlessService.annotations` | Specifies the annotations to attach to headless Service the operator creates. | {} |
+| `adminServerService` | Defines the policy to create AdminServer Service for the zookeeper cluster. | {} |
+| `adminServerService.annotations` | Specifies the annotations to attach to AdminServer Service the operator creates. | {} |
+| `adminServerService.external` | Specifies if LoadBalancer should be created for the AdminServer. True means LoadBalancer will be created, false - only ClusterIP will be used. | false |
 | `config.initLimit` | Amount of time (in ticks) to allow followers to connect and sync to a leader | `10` |
 | `config.tickTime` | Length of a single tick which is the basic time unit used by Zookeeper (measured in milliseconds) | `2000` |
 | `config.syncLimit` | Amount of time (in ticks) to allow followers to sync with Zookeeper | `2` |

diff --git a/charts/zookeeper/templates/zookeeper.yaml b/charts/zookeeper/templates/zookeeper.yaml
@@ -87,6 +87,30 @@ spec:
     imagePullSecrets:
 {{ toYaml .Values.pod.imagePullSecrets | indent 6 }}
     {{- end }}
+  {{- if .Values.clientService }}
+  clientService:
+    {{- if .Values.clientService.annotations }}
+    annotations:
+{{ toYaml .Values.clientService.annotations | indent 6 }}
+    {{- end }}
+  {{- end }}
+  {{- if .Values.headlessService }}
+  headlessService:
+    {{- if .Values.headlessService.annotations }}
+    annotations:
+{{ toYaml .Values.headlessService.annotations | indent 6 }}
+    {{- end }}
+  {{- end }}
+  {{- if .Values.adminServerService }}
+  adminServerService:
+    {{- if .Values.adminServerService.annotations }}
+    annotations:
+{{ toYaml .Values.adminServerService.annotations | indent 6 }}
+    {{- end }}
+    {{- if .Values.adminServerService.external }}
+    external: {{ .Values.adminServerService.external }}
+    {{- end }}
+  {{- end }}
   {{- if .Values.config }}
   config:
     {{- if .Values.config.initLimit }}

diff --git a/charts/zookeeper/values.yaml b/charts/zookeeper/values.yaml
@@ -40,6 +40,16 @@ pod:
   serviceAccountName: zookeeper
   # imagePullSecrets: []
 
+adminServerService: {}
+  # annotations: {}
+  # external: false
+
+clientService: {}
+  # annotations: {}
+
+headlessService: {}
+  # annotations: {}
+
 config: {}
   # initLimit: 10
   # tickTime: 2000

diff --git a/deploy/crds/zookeeper.pravega.io_zookeeperclusters_crd.yaml b/deploy/crds/zookeeper.pravega.io_zookeeperclusters_crd.yaml
@@ -2396,6 +2396,44 @@ spec:
                     type: object
                   type: array
               type: object
+            clientService:
+              description: ClientService defines the policy to create client Service
+                for the zookeeper cluster.
+              properties:
+                annotations:
+                  additionalProperties:
+                    type: string
+                  description: Annotations specifies the annotations to attach to
+                    client Service the operator creates.
+                  type: object
+              type: object
+            headlessService:
+              description: HeadlessService defines the policy to create headless Service
+                for the zookeeper cluster.
+              properties:
+                annotations:
+                  additionalProperties:
+                    type: string
+                  description: Annotations specifies the annotations to attach to
+                    headless Service the operator creates.
+                  type: object
+              type: object
+            adminServerService:
+              description: AdminServerService defines the policy to create AdminServer Service
+                for the zookeeper cluster.
+              properties:
+                external:
+                  description: External specifies if LoadBalancer should be created for
+                    the AdminServer. True means LoadBalancer will be created, false means ClusterIP
+                    will be used. Default is false.
+                  type: boolean
+                annotations:
+                  additionalProperties:
+                    type: string
+                  description: Annotations specifies the annotations to attach to AdminServer
+                    Service the operator creates.
+                  type: object
+              type: object
             ports:
               items:
                 description: ContainerPort represents a network port in a single container.

diff --git a/pkg/apis/zookeeper/v1beta1/zookeepercluster_types.go b/pkg/apis/zookeeper/v1beta1/zookeepercluster_types.go
@@ -100,6 +100,12 @@ type ZookeeperClusterSpec struct {
 	// Updating the Pod does not take effect on any existing pods.
 	Pod PodPolicy `json:"pod,omitempty"`
 
+	AdminServerService AdminServerServicePolicy `json:"adminServerService,omitempty"`
+
+	ClientService ClientServicePolicy `json:"clientService,omitempty"`
+
+	HeadlessService HeadlessServicePolicy `json:"headlessService,omitempty"`
+
 	//StorageType is used to tell which type of storage we will be using
 	//It can take either Ephemeral or persistence
 	//Default StorageType is Persistence storage
@@ -488,6 +494,26 @@ func (p *PodPolicy) withDefaults(z *ZookeeperCluster) (changed bool) {
 	return changed
 }
 
+type AdminServerServicePolicy struct {
+	// Annotations specifies the annotations to attach to AdminServer service the operator
+	// creates.
+	Annotations map[string]string `json:"annotations,omitempty"`
+
+	External bool `json:"external,omitempty"`
+}
+
+type ClientServicePolicy struct {
+	// Annotations specifies the annotations to attach to client service the operator
+	// creates.
+	Annotations map[string]string `json:"annotations,omitempty"`
+}
+
+type HeadlessServicePolicy struct {
+	// Annotations specifies the annotations to attach to headless service the operator
+	// creates.
+	Annotations map[string]string `json:"annotations,omitempty"`
+}
+
 func (s *Probes) withDefaults() (changed bool) {
 	if s.ReadinessProbe == nil {
 		changed = true

diff --git a/pkg/zk/generators.go b/pkg/zk/generators.go
@@ -191,7 +191,7 @@ func MakeClientService(z *v1beta1.ZookeeperCluster) *v1.Service {
 	svcPorts := []v1.ServicePort{
 		{Name: "tcp-client", Port: ports.Client},
 	}
-	return makeService(z.GetClientServiceName(), svcPorts, true, false, z)
+	return makeService(z.GetClientServiceName(), svcPorts, true, false, z.Spec.ClientService.Annotations, z)
 }
 
 // MakeAdminServerService returns a service which provides an interface
@@ -201,7 +201,9 @@ func MakeAdminServerService(z *v1beta1.ZookeeperCluster) *v1.Service {
 	svcPorts := []v1.ServicePort{
 		{Name: "tcp-admin-server", Port: ports.AdminServer},
 	}
-	return makeService(z.GetAdminServerServiceName(), svcPorts, true, true, z)
+	external := z.Spec.AdminServerService.External
+	annotations := z.Spec.AdminServerService.Annotations
+	return makeService(z.GetAdminServerServiceName(), svcPorts, true, external, annotations, z)
 }
 
 // MakeConfigMap returns a zookeeper config map
@@ -236,7 +238,7 @@ func MakeHeadlessService(z *v1beta1.ZookeeperCluster) *v1.Service {
 		{Name: "tcp-metrics", Port: ports.Metrics},
 		{Name: "tcp-admin-server", Port: ports.AdminServer},
 	}
-	return makeService(headlessSvcName(z), svcPorts, false, false, z)
+	return makeService(headlessSvcName(z), svcPorts, false, false, z.Spec.HeadlessService.Annotations, z)
 }
 
 func makeZkConfigString(z *v1beta1.ZookeeperCluster) string {
@@ -300,19 +302,17 @@ func makeZkEnvConfigString(z *v1beta1.ZookeeperCluster) string {
 		"CLUSTER_SIZE=" + fmt.Sprint(z.Spec.Replicas) + "\n"
 }
 
-func makeService(name string, ports []v1.ServicePort, clusterIP bool, external bool, z *v1beta1.ZookeeperCluster) *v1.Service {
+func makeService(name string, ports []v1.ServicePort, clusterIP bool, external bool, annotations map[string]string, z *v1beta1.ZookeeperCluster) *v1.Service {
 	var dnsName string
-	var annotationMap map[string]string
+	var annotationMap = copyMap(annotations)
 	if !clusterIP && z.Spec.DomainName != "" {
 		domainName := strings.TrimSpace(z.Spec.DomainName)
 		if strings.HasSuffix(domainName, dot) {
 			dnsName = name + dot + domainName
 		} else {
 			dnsName = name + dot + domainName + dot
 		}
-		annotationMap = map[string]string{externalDNSAnnotationKey: dnsName}
-	} else {
-		annotationMap = map[string]string{}
+		annotationMap[externalDNSAnnotationKey] = dnsName
 	}
 	service := v1.Service{
 		TypeMeta: metav1.TypeMeta{
@@ -334,7 +334,7 @@ func makeService(name string, ports []v1.ServicePort, clusterIP bool, external b
 		},
 	}
 	if external {
-		service.Spec.Type = "LoadBalancer"
+		service.Spec.Type = v1.ServiceTypeLoadBalancer
 	}
 	if !clusterIP {
 		service.Spec.ClusterIP = v1.ClusterIPNone
@@ -388,3 +388,13 @@ func mergeLabels(l ...map[string]string) map[string]string {
 	}
 	return res
 }
+
+// Make a copy of map
+func copyMap(s map[string]string) map[string]string {
+	res := make(map[string]string)
+
+	for lKey, lValue := range s {
+		res[lKey] = lValue
+	}
+	return res
+}
diff --git a/pkg/zk/generators_test.go b/pkg/zk/generators_test.go
@@ -297,6 +297,11 @@ var _ = Describe("Generators Spec", func() {
 					Labels: map[string]string{
 						"exampleLabel": "exampleValue",
 					},
+					ClientService: v1beta1.ClientServicePolicy{
+						Annotations: map[string]string{
+							"exampleAnnotation": "exampleValue",
+						},
+					},
 				},
 			}
 			z.WithDefaults()
@@ -318,15 +323,20 @@ var _ = Describe("Generators Spec", func() {
 		})
 
 		It("should not set the dns annotation", func() {
-			mapLength := len(s.GetAnnotations())
-			Ω(mapLength).To(Equal(0))
+			Expect(s.GetAnnotations()).NotTo(HaveKey("external-dns.alpha.kubernetes.io/hostname"))
 		})
 
 		It("should have custom labels set", func() {
 			Ω(s.GetLabels()).To(HaveKeyWithValue(
 				"exampleLabel",
 				"exampleValue"))
 		})
+
+		It("should have custom annotations set", func() {
+			Ω(s.GetAnnotations()).To(HaveKeyWithValue(
+				"exampleAnnotation",
+				"exampleValue"))
+		})
 	})
 
 	Context("#MakeHeadlessService", func() {
@@ -345,6 +355,11 @@ var _ = Describe("Generators Spec", func() {
 					Labels: map[string]string{
 						"exampleLabel": "exampleValue",
 					},
+					HeadlessService: v1beta1.HeadlessServicePolicy{
+						Annotations: map[string]string{
+							"exampleAnnotation": "exampleValue",
+						},
+					},
 				},
 			}
 			z.WithDefaults()
@@ -394,6 +409,12 @@ var _ = Describe("Generators Spec", func() {
 				"exampleLabel",
 				"exampleValue"))
 		})
+
+		It("should have custom annotations set", func() {
+			Ω(s.GetAnnotations()).To(HaveKeyWithValue(
+				"exampleAnnotation",
+				"exampleValue"))
+		})
 	})
 
 	Context("#MakeHeadlessService dnsname without dot", func() {
@@ -435,6 +456,11 @@ var _ = Describe("Generators Spec", func() {
 					Labels: map[string]string{
 						"exampleLabel": "exampleValue",
 					},
+					AdminServerService: v1beta1.AdminServerServicePolicy{
+						Annotations: map[string]string{
+							"exampleAnnotation": "exampleValue",
+						},
+					},
 				},
 			}
 			z.WithDefaults()
@@ -460,6 +486,40 @@ var _ = Describe("Generators Spec", func() {
 				"exampleLabel",
 				"exampleValue"))
 		})
+
+		It("should have custom annotations set", func() {
+			Ω(s.GetAnnotations()).To(HaveKeyWithValue(
+				"exampleAnnotation",
+				"exampleValue"))
+		})
+
+		It("should have no LoadBalancer attached by default", func() {
+			Ω(s.Spec.Type).NotTo(Equal(v1.ServiceTypeLoadBalancer))
+		})
+	})
+
+	Context("#MakeAdminServerService external with LoadBalancer", func() {
+		var s *v1.Service
+
+		BeforeEach(func() {
+			z := &v1beta1.ZookeeperCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "example",
+					Namespace: "default",
+				},
+				Spec: v1beta1.ZookeeperClusterSpec{
+					AdminServerService: v1beta1.AdminServerServicePolicy{
+						External: true,
+					},
+				},
+			}
+			z.WithDefaults()
+			s = zk.MakeAdminServerService(z)
+		})
+
+		It("should have LoadBalancer attached", func() {
+			Ω(s.Spec.Type).To(Equal(v1.ServiceTypeLoadBalancer))
+		})
 	})
 
 	Context("#MakePodDisruptionBudget", func() {