Skip to content

fix: correct helm binary in install script for environment that have both helm 2 and 3 installed - #270

Merged
mumoshu merged 1 commit into
databus23:masterfrom
astorath:fix/install
May 18, 2021
Merged

mumoshu merged 1 commit into
databus23:masterfrom
astorath:fix/install

Conversation

@astorath

Copy link
Copy Markdown
Contributor

This fixes install-binary.sh to use correct helm binary

@deiga

deiga commented Mar 21, 2021

Copy link
Copy Markdown
Contributor

@astorath Can you describe what the problem is right now? Why is this fix needed?

@astorath

Copy link
Copy Markdown
Contributor Author

@deiga Sure, the problems are, when you have helm binary named differently (helm2 or helm3 for instance):

  1. Installation does not work sometimes (HELM_PLUGIN_DIR resolves to /plugins/helm-diff) because root fs might be readonly
  2. /plugins/helm-diff is created polluting filesystem
  3. The latest version is installed always (because git describe fails on empty /plugins/helm-diff)

PS: I also have a question about that. Would you be so kind to release special v2.11.0+6 tagged v2 release containing the same fix. I have github fork for that, but it would be nice to have the same issue fixed for helm2 in original repo.

@mumoshu mumoshu changed the title fix: correct helm binary in install script fix: correct helm binary in install script for environment that have both helm 2 and 3 installed May 18, 2021

@mumoshu mumoshu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good catch! I've tried my best to rephrase the PR title to better capture the purpose of this fix. Thanks for your contribution @astorath ☺️

@mumoshu
mumoshu merged commit 7f10f25 into databus23:master May 18, 2021
@mumoshu

mumoshu commented May 18, 2021

Copy link
Copy Markdown
Collaborator

@deiga Thanks for your help on communication! I wouldn't have understood this without your conversation with @astorath

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.

3 participants