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

Embed Helm to change ordering of polling #21

Merged
merged 4 commits into from Feb 6, 2019

Conversation

yorinasub17
Copy link
Contributor

Motivation

When testing kubergrunt on EKS, I ran into a bug where the --wait parameter of helm init causes trouble. When working off of a clean helm home directory, it appears that in order to poll for Tiller, helm requires the working TLS configuration to exist and so it fails the polling step of the deploy.

I still don't know why this works fine in minikube.


Solution

The solution implemented here is to break the polling step to be after the local client is configured. Since we require an RBAC entity to be passed during the deploy, it is guaranteed that we will always setup a new local helm client, so we can push the polling to be after this step.

However, in order to do this, we need more control over the deployment process. As such, I decided to embed helm into kubergrunt so we can call out to the underlying library functions. Unfortunately, much of it was hidden in private functions so I had to copy paste them here (FWIW, terraform-helm-provider does similar things).


Additional Notes

  • Many of the kubernetes libraries don't implement dep. As a result, there is some hard coded dependency overrides in here that were discovered through dependency grunt work, manually going through Godeps.json and glide.lock files.
  • Not all commands have been adapted to use the embedded helm library (namely undeploy). As a result, we still need the helm client to be installed. I punted on the work to adapt undeploy for now, since I think getting a deployable service helm chart is higher priority.
  • There is a bugfix being sneaked in: the helm home directory will now be initialized as part of configure, so you can pass in a helm home directory that does not yet exist. This fixes an annoying issue where you had to run helm repo update after configure in order to actually use the client to install stuff.
  • A lot of the refactored functionality is tested through the integration test. It is on my radar to pull the integration test out into its own file.

Copy link
Contributor

@autero1 autero1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@yorinasub17
Copy link
Contributor Author

Merging. I'm going to implement another feature before releasing.

@yorinasub17 yorinasub17 merged commit 4eff3f9 into master Feb 6, 2019
@yorinasub17 yorinasub17 deleted the yori-embedded-helm-take-2 branch February 6, 2019 17:10
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.

None yet

3 participants