You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/06/08 19:53:22 UTC

[GitHub] [pulsar-helm-chart] rdhabalia opened a new pull request, #269: Support mechanism to provide external zookeeper-server list to build global/configuration zookeeper

rdhabalia opened a new pull request, #269:
URL: https://github.com/apache/pulsar-helm-chart/pull/269

   Fixes #268
   
   ### Motivation
   Right now, [chart dynamically](https://github.com/apache/pulsar-helm-chart/blob/master/charts/pulsar/templates/zookeeper-statefulset.yaml#L140) creates zk cluster with zk pods initialized in the same namespace. However, for global/configuration zookeeper, user requires to build zk clusters with pods deployed in different namespaces. Therefore, user needs a mechanism to pass an external list of zk-servers to the chart and build zk-cluster with pods across different namespaces.
   
   ### Modification
   - Chart should be considering zk-value's configuration for external zookeeper and generate zk-configuration file with appropriate zk-server list and unique id of that zookeeper.
   
   This PR sets `ZOOKEEPER_SERVERS` value provided by user and also sets override-value flag which will be used by [generate-zookeeper-config.sh](https://github.com/apache/pulsar/blob/master/docker/pulsar/scripts/generate-zookeeper-config.sh) to override external zk list in config file and assign appropriate id to the host.
   
   
   ### Result
   - User can add `ZOOKEEPER_SERVERS` string into `zookeeper.configData` in [Values.yaml](https://github.com/apache/pulsar-helm-chart/blob/master/charts/pulsar/values.yaml#L385) file to override external zk-server list.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-helm-chart] rdhabalia commented on a diff in pull request #269: Support mechanism to provide external zookeeper-server list to build global/configuration zookeeper

Posted by GitBox <gi...@apache.org>.
rdhabalia commented on code in PR #269:
URL: https://github.com/apache/pulsar-helm-chart/pull/269#discussion_r907003823


##########
charts/pulsar/templates/zookeeper-statefulset.yaml:
##########
@@ -134,10 +134,19 @@ spec:
           containerPort: {{ .Values.zookeeper.ports.clientTls }}
         {{- end }}
         env:
-        - name: ZOOKEEPER_SERVERS
-          value:
-            {{- $global := . }}
-            {{ range $i, $e := until (.Values.zookeeper.replicaCount | int) }}{{ if ne $i 0 }},{{ end }}{{ template "pulsar.fullname" $global }}-{{ $global.Values.zookeeper.component }}-{{ printf "%d" $i }}{{ end }}
+         - name: ZOOKEEPER_SERVERS
+        {{- if .Values.zookeeper.configData.ZOOKEEPER_SERVERS }}

Review Comment:
   yes, I have added sample-example values in `examples/values-zookeeper-aws.yaml`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-helm-chart] michaeljmarshall commented on a diff in pull request #269: Support mechanism to provide external zookeeper-server list to build global/configuration zookeeper

Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on code in PR #269:
URL: https://github.com/apache/pulsar-helm-chart/pull/269#discussion_r906755486


##########
charts/pulsar/templates/zookeeper-statefulset.yaml:
##########
@@ -134,10 +134,19 @@ spec:
           containerPort: {{ .Values.zookeeper.ports.clientTls }}
         {{- end }}
         env:
-        - name: ZOOKEEPER_SERVERS
-          value:
-            {{- $global := . }}
-            {{ range $i, $e := until (.Values.zookeeper.replicaCount | int) }}{{ if ne $i 0 }},{{ end }}{{ template "pulsar.fullname" $global }}-{{ $global.Values.zookeeper.component }}-{{ printf "%d" $i }}{{ end }}
+         - name: ZOOKEEPER_SERVERS
+        {{- if .Values.zookeeper.configData.ZOOKEEPER_SERVERS }}

Review Comment:
   @rdhabalia - are you able to update the values.yaml file so that this configuration is documented? I plan to release an updated version of the helm chart next week, and it'd be great to include this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-helm-chart] michaeljmarshall commented on pull request #269: Support mechanism to provide external zookeeper-server list to build global/configuration zookeeper

Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on PR #269:
URL: https://github.com/apache/pulsar-helm-chart/pull/269#issuecomment-1283102030

   I missed a minor detail with how the feature is configured. I proposed a fix here https://github.com/apache/pulsar-helm-chart/pull/308.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-helm-chart] MarvinCai commented on a diff in pull request #269: Support mechanism to provide external zookeeper-server list to build global/configuration zookeeper

Posted by GitBox <gi...@apache.org>.
MarvinCai commented on code in PR #269:
URL: https://github.com/apache/pulsar-helm-chart/pull/269#discussion_r901590241


##########
charts/pulsar/templates/zookeeper-statefulset.yaml:
##########
@@ -134,10 +134,19 @@ spec:
           containerPort: {{ .Values.zookeeper.ports.clientTls }}
         {{- end }}
         env:
-        - name: ZOOKEEPER_SERVERS
-          value:
-            {{- $global := . }}
-            {{ range $i, $e := until (.Values.zookeeper.replicaCount | int) }}{{ if ne $i 0 }},{{ end }}{{ template "pulsar.fullname" $global }}-{{ $global.Values.zookeeper.component }}-{{ printf "%d" $i }}{{ end }}
+         - name: ZOOKEEPER_SERVERS
+        {{- if .Values.zookeeper.configData.ZOOKEEPER_SERVERS }}

Review Comment:
   probably add it to value file with an example and comment it out? so user can know how to use this feature.
   As from the script change, seems the provided server list need to contain election port and follower port.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-helm-chart] rdhabalia commented on a diff in pull request #269: Support mechanism to provide external zookeeper-server list to build global/configuration zookeeper

Posted by GitBox <gi...@apache.org>.
rdhabalia commented on code in PR #269:
URL: https://github.com/apache/pulsar-helm-chart/pull/269#discussion_r907003823


##########
charts/pulsar/templates/zookeeper-statefulset.yaml:
##########
@@ -134,10 +134,19 @@ spec:
           containerPort: {{ .Values.zookeeper.ports.clientTls }}
         {{- end }}
         env:
-        - name: ZOOKEEPER_SERVERS
-          value:
-            {{- $global := . }}
-            {{ range $i, $e := until (.Values.zookeeper.replicaCount | int) }}{{ if ne $i 0 }},{{ end }}{{ template "pulsar.fullname" $global }}-{{ $global.Values.zookeeper.component }}-{{ printf "%d" $i }}{{ end }}
+         - name: ZOOKEEPER_SERVERS
+        {{- if .Values.zookeeper.configData.ZOOKEEPER_SERVERS }}

Review Comment:
   yes, I have added sample-example values in `examples/values-zookeeper-aws.yaml`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-helm-chart] rdhabalia commented on pull request #269: Support mechanism to provide external zookeeper-server list to build global/configuration zookeeper

Posted by GitBox <gi...@apache.org>.
rdhabalia commented on PR #269:
URL: https://github.com/apache/pulsar-helm-chart/pull/269#issuecomment-1171522763

   ping @MarvinCai @michaeljmarshall 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar-helm-chart] michaeljmarshall merged pull request #269: Support mechanism to provide external zookeeper-server list to build global/configuration zookeeper

Posted by GitBox <gi...@apache.org>.
michaeljmarshall merged PR #269:
URL: https://github.com/apache/pulsar-helm-chart/pull/269


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org