Add helm chart #10

mehyedes · 2022-02-02T13:30:44Z

This adds a helm chart for cluster-api-state-metrics

Create Helm chart
Add make targets for helm
~~Set default image repo (related to Find a proper destination registry #3)~~
~~Add additional CI steps for the helm chart~~ Will be done in a separate PR
~~Set helm repo for publishing the chart~~ Will be done in a separate PR

It would be great if you provide some feedback on how to proceed with the points above 😁

In the chart, there are also serviceMonitor and GrafanaDashboard templates. We thought they might be useful for anyone out there using the prometheus and grafana operators additionally.

Solves: #9

deploy/chart/templates/deployment.yaml

chrischdi · 2022-02-03T11:41:54Z

Looks good so far 😀 thanks for your Pull Request 🎉

Regarding your points:

Add additional CI steps for the helm chart

The rbac role is getting generated and updated via kubebuilder.

Should we integrate something to the makefile to also update the rbac role in the helm chart? (sed or something like that to generate the helm variant from the kubebuilder generated one?)

Set default image repo (related to #3)

I hope that I'll be able to finish the process by next week so we can publish images to quay.io.

Set helm repo for publishing the chart

What do we have for options here?
Is a github release a valid source or could this also be done at quay.io then?

deploy/chart/templates/grafanaDashboard.yaml

chrischdi

I think we should split out the grafana dashboard to either a separate location or another PR.

Not everyone is using the integreatly.org/v1alpha1 CRs and the dashboard would be way more reusable then I think.

Added first version 0.1.0 of the cluster-api-state-metrics helm chart. Signed-off-by: Mehdi Yedes <[email protected]>

mehyedes · 2022-02-03T13:25:00Z

I think we should split out the grafana dashboard to either a separate location or another PR.

Not everyone is using the integreatly.org/v1alpha1 CRs and the dashboard would be way more reusable then I think.

Agree, would be better to leave it out of this PR for now and then do it in another iteration.

mehyedes · 2022-02-03T14:14:01Z

Add additional CI steps for the helm chart

The rbac role is getting generated and updated via kubebuilder.

Should we integrate something to the makefile to also update the rbac role in the helm chart? (sed or something like that to generate the helm variant from the kubebuilder generated one?)

Yes, that would make sense. However, we should keep in mind that we need to publish a new helm chart release whenever the role(s) are changed. Of course, that would be ideally automated but we just need to keep that in mind.

Set default image repo (related to #3)

I hope that I'll be able to finish the process by next week so we can publish images to quay.io.

Awesome!

Set helm repo for publishing the chart

What do we have for options here? Is a github release a valid source or could this also be done at quay.io then?

I think Quay already supports helm chart repos(never worked with it though). But simply using github releases might also be an option for now. We could use chart-releaser, but I am not sure if it's possible to use it for this repo, since it's not dedicated to only storing helm charts.

chrischdi · 2022-02-04T13:33:25Z

Add additional CI steps for the helm chart

The rbac role is getting generated and updated via kubebuilder.
Should we integrate something to the makefile to also update the rbac role in the helm chart? (sed or something like that to generate the helm variant from the kubebuilder generated one?)

Yes, that would make sense. However, we should keep in mind that we need to publish a new helm chart release whenever the role(s) are changed. Of course, that would be ideally automated but we just need to keep that in mind.

I think publishing a chart release should be fine using chart releaser's github action: https://github.com/helm/chart-releaser-action :-) but let's do this in a follow-up PR then to limit scope here.

Set default image repo (related to #3)

I hope that I'll be able to finish the process by next week so we can publish images to quay.io.

Awesome!

Set helm repo for publishing the chart

What do we have for options here? Is a github release a valid source or could this also be done at quay.io then?

I think Quay already supports helm chart repos(never worked with it though). But simply using github releases might also be an option for now. We could use chart-releaser, but I am not sure if it's possible to use it for this repo, since it's not dedicated to only storing helm charts.

What would you prefer as chart repo: github release or quay.io? I think github release would be better?!

mehyedes · 2022-02-08T19:10:43Z

What would you prefer as chart repo: github release or quay.io? I think github release would be better?!

I agree.

I'll try to update my PR in the upcoming few days.

mehyedes · 2022-02-24T08:13:06Z

I have updated the PR now.

The failing checks should be resolved and the role templates for the helm chart are now generated from the ones under config/rbac

chrischdi · 2022-03-10T12:56:44Z

Hi @mehyedes sorry for the late reply.

We are currently working on a proposal to contribute CASM to github.com/kubernetes-sigs/cluster-api .
I don't know if the contribution would include a helm chart if we have it in here.

So I see some options here:

Add the helm chart here and also further maintain it after the CASM code got contributed to cluster-api
Create a seperate repository for the helm chart (which could be also published directly by you if you would prefer that), this way the chart would be independent.
Later on maybe contribute a helm chart to github.com/kubernetes-sigs/cluster-api if they want/accept helm-charts there (e.g. somewhere in hack/observability)

What do you think?

mehyedes · 2022-03-11T08:55:27Z

Hey @chrischdi, thanks for your feedback.

I think it depends on how the contribution of CASM into cluster-api would be. If CASM will be integrated into the cluster-api codebase, I believe there would be no need for a separate helm chart at all in that case. AFAIK, there is no "official" helm chart for cluster-api for now, but if there was one, then that's where I would add the CASM config.
And since CAPI project is using clusterctl to install its components, I don't see that there will be a helm chart, at least not any time soon.
Maybe we could add the helm chart into this repo, until the project gets contributed to CAPI. Then, there will be no need for it anymore I think so it wouldn't need to be further maintained.

chrischdi

one nit

/lgtm

@tobiasgiese maybe you have some time to take a look at it too?

Makefile

chrischdi · 2022-03-11T09:49:07Z

Maybe we could add the helm chart into this repo, until the project gets contributed to CAPI. Then, there will be no need for it anymore I think so it wouldn't need to be further maintained.

Sounds fair to me 👍 Just didn't want to raise the expectation that it will be integrated to the core repo automatically :-) So we have the same opinion here 😄

tobiasgiese

One tiny nit, rest looks really fine to me. Thanks for your contribution! 🙂🎉

deploy/chart/templates/tests/test-connection.yaml

Makefile

seanschneeweiss

/lgtm

seanschneeweiss

Thank you a lot for the contribution.

Signed-off-by: Mehdi Yedes <[email protected]>

fixed indentation in deployment template removed grafanaDashboard template updated chart README Signed-off-by: Mehdi Yedes <[email protected]>

- Added script to generate role templates for the helm chart. The script is executed with `make manifests` - Added missing SPDX headers Signed-off-by: Mehdi Yedes <[email protected]>

Added helm-lint to the lint target Signed-off-by: Mehdi Yedes <[email protected]>

Use YAML instead JSON Co-authored-by: Tobias Giese <[email protected]>

johannesfrey

Awesome. Thank you very much.

chrischdi · 2022-03-15T15:42:25Z

One last nit, the Readme seems not to be happy 🤔

Updated make targets docs

chrischdi · 2022-03-15T17:48:45Z

Thank you very much for your contribution 👍 If I remember correctly: the first external contribution for a Mercedes Benz repo on GitHub 🎉

dimatha reviewed Feb 2, 2022

View reviewed changes

deploy/chart/templates/deployment.yaml Outdated Show resolved Hide resolved

dimatha reviewed Feb 2, 2022

View reviewed changes

deploy/chart/templates/deployment.yaml Outdated Show resolved Hide resolved

chrischdi reviewed Feb 3, 2022

View reviewed changes

deploy/chart/templates/grafanaDashboard.yaml Outdated Show resolved Hide resolved

chrischdi reviewed Feb 3, 2022

View reviewed changes

add helm chart

93bcc77

Added first version 0.1.0 of the cluster-api-state-metrics helm chart. Signed-off-by: Mehdi Yedes <[email protected]>

mehyedes force-pushed the add-helm-chart branch from 9a5f20d to 29a8f6b Compare February 3, 2022 12:59

mehyedes force-pushed the add-helm-chart branch from 29a8f6b to cda7366 Compare February 4, 2022 08:18

mehyedes force-pushed the add-helm-chart branch from 99d9e69 to 93d7254 Compare February 24, 2022 07:59

chrischdi reviewed Mar 11, 2022

View reviewed changes

Makefile Show resolved Hide resolved

tobiasgiese approved these changes Mar 11, 2022

View reviewed changes

deploy/chart/templates/tests/test-connection.yaml Outdated Show resolved Hide resolved

Makefile Show resolved Hide resolved

seanschneeweiss reviewed Mar 11, 2022

View reviewed changes

seanschneeweiss approved these changes Mar 11, 2022

View reviewed changes

Mehdi Yedes added 4 commits March 15, 2022 13:41

add make targets for helm

b1c0664

Signed-off-by: Mehdi Yedes <[email protected]>

update helm chart templates

7d9f70a

fixed indentation in deployment template removed grafanaDashboard template updated chart README Signed-off-by: Mehdi Yedes <[email protected]>

update helm chart

6acd8b4

- Added script to generate role templates for the helm chart. The script is executed with `make manifests` - Added missing SPDX headers Signed-off-by: Mehdi Yedes <[email protected]>

update Makefile

89e42cb

Added helm-lint to the lint target Signed-off-by: Mehdi Yedes <[email protected]>

mehyedes force-pushed the add-helm-chart branch from 374ba9f to 89e42cb Compare March 15, 2022 12:42

Update deploy/chart/templates/tests/test-connection.yaml

38bc0ae

Use YAML instead JSON Co-authored-by: Tobias Giese <[email protected]>

tobiasgiese approved these changes Mar 15, 2022

View reviewed changes

johannesfrey approved these changes Mar 15, 2022

View reviewed changes

update README

bcfd87d

Updated make targets docs

chrischdi merged commit 412d937 into mercedes-benz:main Mar 15, 2022

mehyedes deleted the add-helm-chart branch March 15, 2022 18:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add helm chart #10

Add helm chart #10

mehyedes commented Feb 2, 2022 •

edited

Loading

chrischdi commented Feb 3, 2022

chrischdi left a comment

mehyedes commented Feb 3, 2022

mehyedes commented Feb 3, 2022 •

edited

Loading

chrischdi commented Feb 4, 2022

mehyedes commented Feb 8, 2022 •

edited

Loading

mehyedes commented Feb 24, 2022

chrischdi commented Mar 10, 2022 •

edited

Loading

mehyedes commented Mar 11, 2022 •

edited

Loading

chrischdi left a comment

chrischdi commented Mar 11, 2022

tobiasgiese left a comment

seanschneeweiss left a comment

seanschneeweiss left a comment

johannesfrey left a comment

chrischdi commented Mar 15, 2022

chrischdi commented Mar 15, 2022

Add helm chart #10

Add helm chart #10

Conversation

mehyedes commented Feb 2, 2022 • edited Loading

chrischdi commented Feb 3, 2022

chrischdi left a comment

Choose a reason for hiding this comment

mehyedes commented Feb 3, 2022

mehyedes commented Feb 3, 2022 • edited Loading

chrischdi commented Feb 4, 2022

mehyedes commented Feb 8, 2022 • edited Loading

mehyedes commented Feb 24, 2022

chrischdi commented Mar 10, 2022 • edited Loading

mehyedes commented Mar 11, 2022 • edited Loading

chrischdi left a comment

Choose a reason for hiding this comment

chrischdi commented Mar 11, 2022

tobiasgiese left a comment

Choose a reason for hiding this comment

seanschneeweiss left a comment

Choose a reason for hiding this comment

seanschneeweiss left a comment

Choose a reason for hiding this comment

johannesfrey left a comment

Choose a reason for hiding this comment

chrischdi commented Mar 15, 2022

chrischdi commented Mar 15, 2022

mehyedes commented Feb 2, 2022 •

edited

Loading

mehyedes commented Feb 3, 2022 •

edited

Loading

mehyedes commented Feb 8, 2022 •

edited

Loading

chrischdi commented Mar 10, 2022 •

edited

Loading

mehyedes commented Mar 11, 2022 •

edited

Loading