Skip to content

[Issue 5992][Helm chart] Helm chart bug fixes#6565

Closed
nicknezis wants to merge 2 commits intoapache:masterfrom
nicknezis:helm-fixes
Closed

[Issue 5992][Helm chart] Helm chart bug fixes#6565
nicknezis wants to merge 2 commits intoapache:masterfrom
nicknezis:helm-fixes

Conversation

@nicknezis
Copy link

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

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 }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@sijie
Copy link
Member

sijie commented Jun 10, 2020

Close this issue now since the helm chart has been moved to https://github.com/apache/pulsar-helm-chart

@sijie sijie closed this Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants