Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow the node hostpath to be set from helm chart env var #3

Merged
merged 2 commits into from
Oct 17, 2019
Merged

Allow the node hostpath to be set from helm chart env var #3

merged 2 commits into from
Oct 17, 2019

Conversation

j14s
Copy link
Contributor

@j14s j14s commented Oct 16, 2019

Hello!

I came across your helm chart for hostpath provisioner. It's great. But I have a need to custom set the actual path on the node.

I modified the chart first, and then found it was acting weird(e.g., the chart was setting it correctly, and a directory for the pvc directory was showing up where it should, but nothing was being written to it. After some investigation, I found it was also still using /mnt/hostpath and creating a pvc directory there too! =)

Once I dug into your code in this repo, I saw that it was hard coded here.

I am not sure you were looking to take outside changes, but I thought I would send you the PR in case you don't mind.

This change should not effect your current helm chart as it defaults to what was hard coded.

Also expect a PR for the chart after I send this. That on the other hand is dependent on this change to the code and an updated "latest" docker image.

Kindest Regards!

@j14s
Copy link
Contributor Author

j14s commented Oct 16, 2019

Sorry, missed the changes in version in the makefile. bumped to 0.2.2

@rimusz
Copy link
Owner

rimusz commented Oct 17, 2019

cool, thank you

@rimusz rimusz merged commit c2db56c into rimusz:master Oct 17, 2019
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