Repository: pravega/zookeeper-operator (go)

Issue:
Add support for more zoo.cfg configuration parameters
### Description

In our environment we are looking into tweaking `clientPortListenBacklog` and `serverCnxnFactory` zoo.cfg parameters to tune the zookeeper server(s) listening sockets.

### Importance

Should-have


### Suggestions for an improvement

we could extend the existing support to add these parameters.


Pull request description:
ISSUE-356 Add support for more zoo.cfg configuration parameters
### Change log description

Added a new `spec.config.additionalConfig` field to allov custom zoo.cfg parameters to passed to zookeeper


### Purpose of the change

Fixes #356


Patch:
diff --git a/charts/zookeeper-operator/templates/_helpers.tpl b/charts/zookeeper-operator/templates/_helpers.tpl
index 2b3db99..125c98b 100644
--- a/charts/zookeeper-operator/templates/_helpers.tpl
+++ b/charts/zookeeper-operator/templates/_helpers.tpl
@@ -161,6 +161,13 @@ openAPIV3Schema:
                 is provided required default values will be provided, and optional
                 values will be excluded.
               properties:
+                additionalConfig:
+                  additionalProperties:
+                    type: string
+                  description: key-value map of additional zookeeper
+                    configuration parameters
+                  type: object
+                  x-kubernetes-preserve-unknown-fields: true
                 autoPurgePurgeInterval:
                   description: "The time interval in hours for which the purge task
                     has to be triggered \n Disabled by default"
diff --git a/charts/zookeeper/README.md b/charts/zookeeper/README.md
index 4264063..3596442 100644
--- a/charts/zookeeper/README.md
+++ b/charts/zookeeper/README.md
@@ -104,6 +104,7 @@ The following table lists the configurable parameters of the zookeeper chart and
 | `config.autoPurgeSnapRetainCount` | The number of snapshots to be retained | `3`
 | `config.autoPurgePurgeInterval` | The time interval in hours for which the purge task has to be triggered | `1`
 | `config.quorumListenOnAllIPs` | Whether Zookeeper server will listen for connections from its peers on all available IP addresses | `false` |
+| `config.additionalConfig` | Additional zookeeper coniguration parameters that should be defined in generated zoo.cfg file | `{}` |
 | `storageType` | Type of storage that can be used it can take either ephemeral or persistence as value | `persistence` |
 | `persistence.reclaimPolicy` | Reclaim policy for persistent volumes | `Delete` |
 | `persistence.annotations` | Specifies the annotations to attach to pvcs | `{}` |`
diff --git a/charts/zookeeper/templates/zookeeper.yaml b/charts/zookeeper/templates/zookeeper.yaml
index 496682a..8e5502a 100644
--- a/charts/zookeeper/templates/zookeeper.yaml
+++ b/charts/zookeeper/templates/zookeeper.yaml
@@ -169,6 +169,10 @@ spec:
     {{- if .Values.config.quorumListenOnAllIPs }}
     quorumListenOnAllIPs: {{ .Values.config.quorumListenOnAllIPs }}
     {{- end }}
+    {{- if .Values.config.additionalConfig }}
+    additionalConfig:
+    {{- toYaml .Values.config.additionalConfig | nindent 6 }}
+    {{- end }}
   {{- end }}
   storageType: {{ $storageType }}
   {{- if eq $storageType "ephemeral" }}
diff --git a/charts/zookeeper/values.yaml b/charts/zookeeper/values.yaml
index 1824812..7f5088d 100644
--- a/charts/zookeeper/values.yaml
+++ b/charts/zookeeper/values.yaml
@@ -66,6 +66,7 @@ config: {}
   # autoPurgeSnapRetainCount: 3
   # autoPurgePurgeInterval: 1
   # quorumListenOnAllIPs: false
+  # additionalConfig: {}
 
 ## configure the storage type
 ## accepted values : persistence/ephemeral
diff --git a/deploy/all_ns/rbac.yaml b/deploy/all_ns/rbac.yaml
index f96e124..dd5d1ce 100644
--- a/deploy/all_ns/rbac.yaml
+++ b/deploy/all_ns/rbac.yaml
@@ -26,6 +26,7 @@ rules:
   - events
   - configmaps
   - secrets
+  - serviceaccounts
   verbs:
   - "*"
 - apiGroups:
diff --git a/deploy/crds/zookeeper.pravega.io_zookeeperclusters_crd.yaml b/deploy/crds/zookeeper.pravega.io_zookeeperclusters_crd.yaml
index f7dbc17..29ef8d2 100644
--- a/deploy/crds/zookeeper.pravega.io_zookeeperclusters_crd.yaml
+++ b/deploy/crds/zookeeper.pravega.io_zookeeperclusters_crd.yaml
@@ -92,6 +92,13 @@ spec:
                 is provided required default values will be provided, and optional
                 values will be excluded.
               properties:
+                additionalConfig:
+                  additionalProperties:
+                    type: string
+                  description: key-value map of additional zookeeper configuration
+                    parameters
+                  type: object
+                  x-kubernetes-preserve-unknown-fields: true
                 autoPurgePurgeInterval:
                   description: "The time interval in hours for which the purge task
                     has to be triggered \n Disabled by default"
diff --git a/deploy/default_ns/rbac.yaml b/deploy/default_ns/rbac.yaml
index 6db143b..28ef903 100644
--- a/deploy/default_ns/rbac.yaml
+++ b/deploy/default_ns/rbac.yaml
@@ -26,6 +26,7 @@ rules:
   - events
   - configmaps
   - secrets
+  - serviceaccounts
   verbs:
   - "*"
 - apiGroups:
diff --git a/pkg/apis/zookeeper/v1beta1/zookeepercluster_types.go b/pkg/apis/zookeeper/v1beta1/zookeepercluster_types.go
index b8cd760..55aef77 100644
--- a/pkg/apis/zookeeper/v1beta1/zookeepercluster_types.go
+++ b/pkg/apis/zookeeper/v1beta1/zookeepercluster_types.go
@@ -642,6 +642,11 @@ type ZookeeperConfig struct {
 	//
 	// The default value is false.
 	QuorumListenOnAllIPs bool `json:"quorumListenOnAllIPs,omitempty"`
+
+	// key-value map of additional zookeeper configuration parameters
+	// +kubebuilder:pruning:PreserveUnknownFields
+	// +optional
+	AdditionalConfig map[string]string `json:"additionalConfig,omitempty"`
 }
 
 func (c *ZookeeperConfig) withDefaults() (changed bool) {
diff --git a/pkg/apis/zookeeper/v1beta1/zz_generated.deepcopy.go b/pkg/apis/zookeeper/v1beta1/zz_generated.deepcopy.go
index 5ac67ab..2b4ca25 100644
--- a/pkg/apis/zookeeper/v1beta1/zz_generated.deepcopy.go
+++ b/pkg/apis/zookeeper/v1beta1/zz_generated.deepcopy.go
@@ -1,6 +1,6 @@
 // +build !ignore_autogenerated
 
-// Code generated by operator-sdk. DO NOT EDIT.
+// Code generated by operator-sdk-v0.17.0-x86_64-apple-darwin. DO NOT EDIT.
 
 package v1beta1
 
@@ -393,7 +393,7 @@ func (in *ZookeeperClusterSpec) DeepCopyInto(out *ZookeeperClusterSpec) {
 		*out = new(Ephemeral)
 		(*in).DeepCopyInto(*out)
 	}
-	out.Conf = in.Conf
+	in.Conf.DeepCopyInto(&out.Conf)
 	if in.Containers != nil {
 		in, out := &in.Containers, &out.Containers
 		*out = make([]v1.Container, len(*in))
@@ -465,6 +465,13 @@ func (in *ZookeeperClusterStatus) DeepCopy() *ZookeeperClusterStatus {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ZookeeperConfig) DeepCopyInto(out *ZookeeperConfig) {
 	*out = *in
+	if in.AdditionalConfig != nil {
+		in, out := &in.AdditionalConfig, &out.AdditionalConfig
+		*out = make(map[string]string, len(*in))
+		for key, val := range *in {
+			(*out)[key] = val
+		}
+	}
 	return
 }
 
diff --git a/pkg/zk/generators.go b/pkg/zk/generators.go
index 142ebf6..0294327 100644
--- a/pkg/zk/generators.go
+++ b/pkg/zk/generators.go
@@ -247,7 +247,12 @@ func MakeHeadlessService(z *v1beta1.ZookeeperCluster) *v1.Service {
 
 func makeZkConfigString(z *v1beta1.ZookeeperCluster) string {
 	ports := z.ZookeeperPorts()
-	return "4lw.commands.whitelist=cons, envi, conf, crst, srvr, stat, mntr, ruok\n" +
+
+	var zkConfig = ""
+	for key, value := range z.Spec.Conf.AdditionalConfig {
+		zkConfig = zkConfig + fmt.Sprintf("%s=%s\n", key, value)
+	}
+	return zkConfig + "4lw.commands.whitelist=cons, envi, conf, crst, srvr, stat, mntr, ruok\n" +
 		"dataDir=/data\n" +
 		"standaloneEnabled=false\n" +
 		"reconfigEnabled=true\n" +


Test patch:
diff --git a/pkg/apis/zookeeper/v1beta1/deepcopy_test.go b/pkg/apis/zookeeper/v1beta1/deepcopy_test.go
index 8f5846e..0a4f234 100644
--- a/pkg/apis/zookeeper/v1beta1/deepcopy_test.go
+++ b/pkg/apis/zookeeper/v1beta1/deepcopy_test.go
@@ -58,7 +58,7 @@ var _ = Describe("ZookeeperCluster DeepCopy", func() {
 					},
 				},
 				InitContainers: []v1.Container{
-					v1.Container{
+					{
 						Name:    "testing",
 						Image:   "dummy-image",
 						Command: []string{"sh", "-c", "ls;pwd"},
@@ -224,6 +224,15 @@ var _ = Describe("ZookeeperCluster DeepCopy", func() {
 			zooconfig2 := zooconfig.DeepCopy()
 			Ω(zooconfig2).To(BeNil())
 		})
+		It("checking for deepcopy for zookeeperconfig", func() {
+			var zkConfig = v1beta1.ZookeeperConfig{
+				AdditionalConfig: map[string]string{
+					"tcpKeepAlive": "true",
+				},
+			}
+			zkConfig2 := zkConfig.DeepCopy()
+			Ω(zkConfig2.AdditionalConfig["tcpKeepAlive"]).To(Equal("true"))
+		})
 		It("checking for nil clusterlist", func() {
 			var clusterlist *v1beta1.ZookeeperClusterList
 			clusterlist2 := clusterlist.DeepCopy()
diff --git a/pkg/zk/generators_test.go b/pkg/zk/generators_test.go
index 94488c0..2690190 100644
--- a/pkg/zk/generators_test.go
+++ b/pkg/zk/generators_test.go
@@ -53,6 +53,11 @@ var _ = Describe("Generators Spec", func() {
 						Labels: map[string]string{
 							"exampleLabel": "exampleValue",
 						},
+						Conf: v1beta1.ZookeeperConfig{
+							AdditionalConfig: map[string]string{
+								"tcpKeepAlive": "true",
+							},
+						},
 					},
 				}
 				z.WithDefaults()
@@ -98,6 +103,10 @@ var _ = Describe("Generators Spec", func() {
 					Ω(cfg).To(ContainSubstring("syncLimit=2\n"))
 				})
 
+				It("should set additional configuration tcpKeepAlive to 'true'", func() {
+					Ω(cfg).To(ContainSubstring("tcpKeepAlive=true\n"))
+				})
+
 				It("should have a dynamicConfigFile", func() {
 					Ω(cfg).
 						To(ContainSubstring(