[GitHub] [pulsar] nicknezis opened a new pull request #6565: [Issue 5992][Helm chart] Helm chart bug fixes

[GitBox <gi...@apache.org>]

nicknezis opened a new pull request #6565: [Issue 5992][Helm chart] Helm chart bug fixes
URL: https://github.com/apache/pulsar/pull/6565
 
 
   Master Issue: #5992 
   
   ### Motivation
   
   The Helm chart failed to work because of the two typos fixed in this PR.
   
   ### Modifications
   
   *Describe the modifications you've done.*
   
   This change is a trivial rework / code cleanup without any test coverage.
   
   ### Does this pull request potentially affect one of the following parts:
   
   *If `yes` was chosen, please highlight the changes*
   
     - Dependencies (does it add or upgrade a dependency): no
     - The public API: no
     - The schema: no
     - The default values of configurations: no
     - The wire protocol: no
     - The rest endpoints: no
     - The admin cli options: no
     - Anything that affects deployment: yes
   
   ### Documentation
   
     - Does this pull request introduce a new feature? no
     - If yes, how is the feature documented? not applicable
     - If a feature is not applicable for documentation, explain why?
     - If a feature is not documented yet in this PR, please create a followup issue for adding the documentation
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [pulsar] sijie commented on a change in pull request #6565: [Issue 5992][Helm chart] Helm chart bug fixes

[GitBox <gi...@apache.org>]

sijie commented on a change in pull request #6565: [Issue 5992][Helm chart] Helm chart bug fixes
URL: https://github.com/apache/pulsar/pull/6565#discussion_r403744582
 
 

 ##########
 File path: deployment/kubernetes/helm/pulsar/templates/bookkeeper-configmap.yaml
 ##########
 @@ -32,7 +32,7 @@ metadata:
 data:
   zkServers:
     {{- $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 }}.{{ template "pulsar.fullname" $global }}-{{ $global.Values.zookeeper.component }}{{ end }}
+    {{ range $i, $e := until (.Values.zookeeper.replicaCount | int) }}{{ if ne $i 0 }},{{ end }}{{ template "pulsar.fullname" $global }}-{{ $global.Values.zookeeper.component }}-{{ printf "%d" $i }},{{ template "pulsar.fullname" $global }}-{{ $global.Values.zookeeper.component }}{{ end }}
 
 Review comment:
   I don't think this change is correct.
   
   Before this change, it generates `zkServers` as the following settings:
   ```
   pulsar-dev-zookeeper-0.pulsar-dev-zookeeper,pulsar-dev-zookeeper-1.pulsar-dev-zookeeper,pulsar-dev-zookeeper-2.pulsar-dev-zookeeper
   ```
   
   But after your change, it generates a wrong list:
   
   ```
   pulsar-dev-zookeeper-0,pulsar-dev-zookeeper,pulsar-dev-zookeeper-1,pulsar-dev-zookeeper,pulsar-dev-zookeeper-2,pulsar-dev-zookeeper
   ```
   
   Although the wrong list can be used without problems, it doesn't seem to the right fix.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services