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

Import Helm Charts #65

Closed
eladb opened this issue Mar 10, 2020 · 18 comments · Fixed by #346
Closed

Import Helm Charts #65

eladb opened this issue Mar 10, 2020 · 18 comments · Fixed by #346
Assignees
Labels
effort/medium 1 week tops

Comments

@eladb
Copy link
Contributor

eladb commented Mar 10, 2020

No description provided.

@eladb eladb added the effort/medium 1 week tops label Jun 1, 2020
@mbonig
Copy link
Contributor

mbonig commented Jun 3, 2020

I recently went through the manual process of converting a helm chart (MySQL) to a cdk8s construct. Not sure how you'd do it automatically unless there is a nice way of parsing go-templates...

https://www.openconstructfoundation.org/helm-to-cdk8s/

@pgollucci
Copy link

how is this going to be different from cluster.addChart() ?

@manast
Copy link

manast commented Sep 17, 2020

no helm charts support is kind of no-go in current kubernetes deployment operations, it would be great if you could prioritize it up :).

@mbonig
Copy link
Contributor

mbonig commented Sep 17, 2020

no helm charts support is kind of no-go in current kubernetes deployment operations, it would be great if you could prioritize it up :).

Can you describe the use-case and how you imagine the solution might work?

@manast
Copy link

manast commented Sep 17, 2020

well most services that we would like to install are most conveniently installed using helm charts, cert-manager, redis, contour, etc.

@mbonig
Copy link
Contributor

mbonig commented Sep 17, 2020

Importing helm charts, with all their templating, could be very difficult. Generally you'd likely have to rewrite any existing helm charts into constructs. I wrote up a guide when I did this for a MySQL helm chart to constructs, maybe it'll help:

https://www.openconstructfoundation.org/helm-to-cdk8s/

@eladb
Copy link
Contributor Author

eladb commented Sep 21, 2020

I was thinking to simply execute helm template during synth.

@manast
Copy link

manast commented Sep 21, 2020

@mbonig thats gonna be a no-go, maintaining constructs for every third-party helm chart we depend on is impracticable.
Perhaps it is more difficult to implement helm charts support in the CDK, but Pulumi seems to get this right: https://www.pulumi.com/docs/guides/adopting/from_kubernetes/

@mbonig
Copy link
Contributor

mbonig commented Sep 23, 2020

@mbonig thats gonna be a no-go, maintaining constructs for every third-party helm chart we depend on is impracticable.
Perhaps it is more difficult to implement helm charts support in the CDK, but Pulumi seems to get this right: https://www.pulumi.com/docs/guides/adopting/from_kubernetes/

I'd love to dig into their code and see how they manage importing a helm chart. It would be a powerful feature to have in the cdk8s.

@eladb
Copy link
Contributor Author

eladb commented Sep 23, 2020

I'd love to dig into their code and see how they manage importing a helm chart. It would be a powerful feature to have in the cdk8s.

I expect this to be something like helm template under the hood

@mbonig
Copy link
Contributor

mbonig commented Sep 26, 2020

I took a quick stab at this:

https://github.com/mbonig/helm-chart

Let me know what you think, @manast?

I purposely didn't look too much at what pulumi was doing, just tried to build something simple and easy to work with.

@manast
Copy link

manast commented Sep 26, 2020

Looks great. Will test it next week :).

@RichiCoder1
Copy link

Grafana's tanka just added an analogous feature: https://github.com/grafana/tanka/blob/master/CHANGELOG.md#wheel_of_dharma-helm-support

Also uses helm template, but has the addition of requiring you vendor the charts.

@mbonig
Copy link
Contributor

mbonig commented Oct 7, 2020

Grafana's tanka just added an analogous feature: https://github.com/grafana/tanka/blob/master/CHANGELOG.md#wheel_of_dharma-helm-support

Also uses helm template, but has the addition of requiring you vendor the charts.

I'm not familiar with tanka... could you offer a comparison between their method and the one I created (https://github.com/mbonig/helm-chart)?

@RichiCoder1
Copy link

RichiCoder1 commented Oct 7, 2020

Looking at it, two big differences:

  1. Tanka requires the helm tool be installed, while yours appears to call into a docker image to template out the chart. (I actually prefer tanka's approach here, invoking docker for this seems a bit heavy)
  2. Tanka has a feature that will download and manage local "vendored" charts for you, so you don't have to untar them yourself: https://tanka.dev/helm#vendoring-helm-charts

A minor difference is that the produced objects are named ${kind}_${metadata.name}.

@mbonig
Copy link
Contributor

mbonig commented Oct 7, 2020

Looking at it, two big differences:

  1. Tanka requires the helm tool be installed, while yours appears to call into a docker image to template out the chart. (I actually prefer tanka's approach here, invoking docker for this seems a bit heavy)

I'm going to change this so it works similar to the NodejsFunction in the CDK, where it'll use docker if 'helm' isn't in the PATH or just execute helm directly if it is.

  1. Tanka has a feature that will download and manage local "vendored" charts for you, so you don't have to untar them yourself: https://tanka.dev/helm#vendoring-helm-charts

A minor difference is that the produced objects are named ${kind}_${metadata.name}.

I'll look into building similar functionality into it.

@mbonig
Copy link
Contributor

mbonig commented Oct 16, 2020

Submitted a PR: #346

eladb pushed a commit that referenced this issue Oct 19, 2020
Closes #65

This introduces a new construct that can be used to include an existing Helm chart into your constructs.
@sunshineo
Copy link

@mbonig Great job. I found something maybe a bug and filed #547

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/medium 1 week tops
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants