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

feat: helm tiller integration #449

Closed
astorath opened this issue Jan 23, 2019 · 10 comments
Closed

feat: helm tiller integration #449

astorath opened this issue Jan 23, 2019 · 10 comments

Comments

@astorath
Copy link
Contributor

astorath commented Jan 23, 2019

Proposal: helm tiller plugin integration

Motivation

Provide a clean solution to run tillerless kubernetes using helmfile.
See #381

Problems

  • Using helm tiller with helmfile requires separate setup of execution scripts. This separates helmfile setup and helmfile execution setup which is somewhat error-prone.

Proposal

  • Add an option to execute helm commands via helm-tiller addon to helmfile spec (helmDefaults), e.g.:
helm tiller run tiller-namespace -- helm diff ...
  • Provide settings for: HELM_TILLER_SILENT, HELM_TILLER_PORT, HELM_TILLER_STORAGE, tiller_namespace, HELM_TILLER_HISTORY_MAX
  • Provide an option to workaround --kube-context problem in helm-tiller addon (running helm with helm-tiller addon does not respect --kube-context option)
  • Optionally provide settings for HELM_TILLER_LOGS_DIR
  • Optionally support helm tiller start and helm tiller start-ci, but I don't think this is necessary
@mumoshu
Copy link
Collaborator

mumoshu commented Jan 24, 2019

Hey! Thanks for writing this. I believe this would be a great addition to helmfile.

helm-tiller seems like a practical way that works today to achieve Tilerless Helm without waiting until Helm v3.

Let me add a few notes on your proposal:

Provide settings for: HELM_TILLER_SILENT, HELM_TILLER_PORT, HELM_TILLER_STORAGE, tiller_namespace, HELM_TILLER_HISTORY_MAX

Making tiller_namespace per per each relelase would be nice.

I don't have strong opinions on fine-tuning HELM_TILLER_SILENT, HELM_TILLER_PORT, HELM_TILLER_STORAGE, HELM_TILLER_HISTORY_MAX, so I'd just use envvars to globally set em.

HELM_TILLER_PORT can be automatically computed per a group of releases sharing the same tillerNamespace (and HELM_TILLER_STORAGE if we allow fine-tuning it per release). Otherwise we can start by bringing up a dedicated tiller per release, just for ease of implementation. But it would add moderate overhead to helmfile when the helm-tiller integration is enabled on many releases.

I'd set HELM_TILLER_STORAGE to the secret backend by default when the helm-tiller integration is enabled, as long as there's no strong opinion to use configmaps.

Provide an option to workaround --kube-context problem in helm-tiller addon (running helm with helm-tiller addon does not respect --kube-context option)

Yep, this makes sense. But I'm unsure how we could do it reliably and universally. Wrap every helm command calls into its own bash process with the KUBECONFIG envvar, KUBECONFIG=tempkubeconfig bash -c 'helm tiller run -- helm upgrade ...., as that's the proposed workaround?

Optionally provide settings for HELM_TILLER_LOGS_DIR

I'm interested. What would be the use case for this? Debugging infunctional tiller?

Optionally support helm tiller start and helm tiller start-ci, but I don't think this is necessary

Yep. I don't have strong desired to support those as well.

@osterman
Copy link
Contributor

Also, for reference, I think is referring to the helm plugin called helm-tiller

@osterman
Copy link
Contributor

I think this is a neat suggestion. That said, because we use codefresh and enjoy the tight integration with helm that it provides, I don't think we're ready to adopt helm-tiller because it would break the functionality.

@astorath
Copy link
Contributor Author

I don't have strong opinions on fine-tuning HELM_TILLER_SILENT, HELM_TILLER_PORT, HELM_TILLER_STORAGE, HELM_TILLER_HISTORY_MAX, so I'd just use envvars to globally set em.

Agreed, but implementing HELM_TILLER_HISTORY_MAX seems ok though...

Provide an option to workaround --kube-context problem in helm-tiller addon (running helm with helm-tiller addon does not respect --kube-context option)

Yep, this makes sense. But I'm unsure how we could do it reliably and universally. Wrap every helm command calls into its own bash process with the KUBECONFIG envvar, KUBECONFIG=tempkubeconfig bash -c 'helm tiller run -- helm upgrade ...., as that's the proposed workaround?

This is a hack, but helmfile uses the same hack with secrets (decrypting them and deleting afterwards), so seems ok. If I were to implement this - I would use bash process substitution to keep updated version of kubeconfig in memory.

The second option (simple one) is to prevent setting kube-context with helm-tiller, to make this clear at least.

@danielezer
Copy link

+1

@pvalsecc
Copy link
Contributor

pvalsecc commented Apr 2, 2019

@osterman, I was considering working on a PR for the feature (at least parts of it), but your comment about codefresh made me doubt about the feasibility. But I see no reason why. Could you, please, explain a bit more?

pvalsecc pushed a commit to pvalsecc/helmfile that referenced this issue Apr 2, 2019
pvalsecc pushed a commit to pvalsecc/helmfile that referenced this issue Apr 2, 2019
pvalsecc pushed a commit to pvalsecc/helmfile that referenced this issue Apr 2, 2019
pvalsecc pushed a commit to pvalsecc/helmfile that referenced this issue Apr 2, 2019
pvalsecc pushed a commit to pvalsecc/helmfile that referenced this issue Apr 2, 2019
pvalsecc pushed a commit to pvalsecc/helmfile that referenced this issue Apr 2, 2019
@mumoshu
Copy link
Collaborator

mumoshu commented May 22, 2019

@pvalsecc I think the latest helm release contains my patch that adds a tiller flag for customing probeAddr, so that we can avoid the port collision issue when helmfile --concurrency is larger than 1.

Would it be a good time to continue working on this? 😄

@pvalsecc
Copy link
Contributor

@mumoshu, since the next version of helm (3.0) won't use tiller anymore, I don't know if it's worth the time to add support for this option. I'd say we should wait for the next release.

@mumoshu
Copy link
Collaborator

mumoshu commented May 23, 2019

@pvalsecc Hey! Thanks for your response.

I just wanted to encourage anyone to contribute this, as helm v3 is still alpha1 and several big additions are planned(like Lua hooks) until it reaches the final version.

Seeing the hundreds of commits not yet cherry-picked to the v3 branch seems like a huge blocker for me. helm-tiller + helm2 will remain a viable option for months(or maybe a year or so). Just my two cents!

@mumoshu
Copy link
Collaborator

mumoshu commented Dec 11, 2019

Closing this as Helm v3 has been released and Helmfile already supports v3. Please feel free to open dedicated issues to improve helm-tiller integration for helm v2 mode!

@mumoshu mumoshu closed this as completed Dec 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants