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

Central helm charts repository for grafana? #2593

Closed
torstenwalter opened this issue Sep 3, 2020 · 28 comments · Fixed by #2720
Closed

Central helm charts repository for grafana? #2593

torstenwalter opened this issue Sep 3, 2020 · 28 comments · Fixed by #2720
Milestone

Comments

@torstenwalter
Copy link
Contributor

As the helm/charts repository is being deprecated many charts are moving to new location.

For example all Prometheus related charts are being moved to prometheus-community GitHub organisation.

At https://github.com/scottrigby/prometheus-helm-charts you find the current state of that migration. That repository hosts multiple helm charts, CI pipeline is set up using GitHub actions e.g. kind to spin up a Kubernetes cluster, chart-testing to verify helm charts and super-linter to check the syntax.

While doing that migration the discussion came up wat to do with dependent charts which are still in stable repository. See prometheus-community/helm-charts#30 and prometheus-community/helm-charts#1 (comment)

In the meantime @zanhsieh started migrating stable/grafana to https://github.com/grafana GitHub Org.

The goal would be to to have a similar setup to Prometheus or Jenkins.
Charts index.ysml is provided via GitHub Pages which even allows to use custom domains like https://charts.jenkins.io/. The chart packages (tgz) are normal GitHub releases. Check https://github.com/scottrigby/prometheus-helm-charts/releases for an example.

Now my question: Would you be interested in hosting the loki charts together with grafana chart in a similar way?

.

@slim-bean
Copy link
Collaborator

What you describe is already how the Loki helm charts are setup, but under the Loki repo:

https://grafana.github.io/loki/charts/

Back when setting up the Loki charts we decided not to use the helm/charts repo because even then it was being deprecated.

The chart sources are kept in the Loki repo and we publish them to a gh-pages branch which is what you see behind that URL I pasted above.

I don't think we would want to move the helm charts to a different repo I think that would be confusing to Loki users who wish to make changes to the charts, it would also make our maintenance burden higher as we would need to monitor multiple git repos for changes to the charts.

@monotek
Copy link
Contributor

monotek commented Sep 3, 2020

I think the question is not to move the charts to some third party but to have loki & grafana in the same repo, managed by grafana staff.

@slim-bean
Copy link
Collaborator

Understood, however I'm not sure that represents a significant advantage?

I'm not totally opposed to this but currently it's the Loki maintainers who are responsible for managing all the Helm PR's moving this to a separate repo means we have to now monitor another repo, which will also have a lot of non Loki PR's in it.

Perhaps I'm being a bit selfish here but the maintenance burden for Helm charts is already really high, adding another step for us feels undesirable until we get to a point where we have more dedicated team members managing Helm for both Grafana and Loki although I'm not sure this will ever be the case.

I also personally feel like while Loki and Grafana are maintained by Grafana Labs, they are separate projects and combining them really only seems to add the single benefit of one helm repo add command instead of two? (Not sure if there are other benefits I'm overlooking).

Given that other charts, you mention prometheus, are moving to separate repos I think the norm is always gonna look like:

  • helm repo add prometheus...
  • helm repo add grafana...
  • helm repo add loki...

It almost feels more intuitive to me to keep them separate, but again maybe I'm just biased here from the maintenance side of things :)

@monotek
Copy link
Contributor

monotek commented Sep 4, 2020

Single repo would mean you could share the same ci pipeline without the need to develop and maintain it twice.

I also think users expect to find all of the Grafana tools in the same repo.

If Grafana staff is not able to maintain all the Helm charts alone, you might want to have a community supported repo too? I've made already several contributions to the Grafana chart. The dashboard import feature is also coming from the company i work for, so i would be happy to help in a Grafana community repo under the Grafana umbrella too :)

@scottrigby
Copy link
Contributor

@slim-bean It's entirely up to org/project/chart maintainer preference 😄 Helm itself doesn't dictate any of this. That said Helm repos are intended to support multiple charts, and the helm tools for CI and CD (including the wrapping Helm GitHub Actions to help make this easier for chart maintainers) supports this as well. But it/s not required. Some app maintainers prefer to keep the chart coupled with the application codebase (especially in cases where chart manifests are auto-generated by app source changes, such as CRD updates), while some prefer logical grouping of charts together in a standalone git repo. So really up to you.

FWIW, I can see an advantage to end users in having all Grafana related charts in the same helm repo. Mainly so chart PRs, testing, and release schedule etc are all consistent. It may also be a help to you to have additional community maintainers for the charts, and as @monotek said to expand the grafana helm community with newly proposed charts. Control can be managed with GitHub CODEOWNERS (this is what other repos such as prometheus did, where there are 17 different charts with different contributors). Other that that, I'm not sure what the advantages are. The repo is there now, so you can always change your mind at any point in the future.

@monotek You can still offer to contribute that to https://github.com/grafana/helm-charts even if the Loki maintainers prefer the loki chart to remain separate.

@slim-bean
Copy link
Collaborator

Single repo would mean you could share the same ci pipeline without the need to develop and maintain it twice.

FWIW, I can see an advantage to end users in having all Grafana related charts in the same helm repo. Mainly so chart PRs, testing, and release schedule etc are all consistent.

This is extremely compelling.

It may also be a help to you to have additional community maintainers for the charts

As is this.

You have both put me firmly on the fence now :) I really appreciate the discussion!

Let me regroup with some folks internally between Grafana and other Loki maintainers to see if we can get a rough consensus on what we'd like to do with the Loki charts.

Thanks again for your patience and discussion, more to come! (but probably later next week I'm off for a few days)

@torstenwalter
Copy link
Contributor Author

Thank you for considering it. It's great to hear pros and cons in this discussion.

Enjoy your days off.

@unguiculus
Copy link
Contributor

@slim-bean I would also be in favor of hosting charts for all Grafana products in one repo. I would like to offer helping as a collaborator. I'm currently developing a Helm chart for Loki in microservices mode and would be happy to contribute that or to update the existing chart. I'm a Helm org and charts maintainer, am familiar with Helm best practices, and developed much of the CI tooling and GitHub actions now used in the new charts repo for the Grafana chart.

@scottrigby
Copy link
Contributor

Count me in as well, if that would be helpful.

@unguiculus
Copy link
Contributor

Linking #2529

@unguiculus
Copy link
Contributor

@slim-bean Have you had a chance to discuss this internally?

Here's what I'm currently working on:
https://github.com/unguiculus/loki-helm-chart

I'd appreciate feedback and would be happy to work on upstreaming the chart.

@owen-d
Copy link
Member

owen-d commented Sep 22, 2020

Wow, this is a great issue. Apologies that we haven't gotten around to discussing this much internally and ❤️ for all the volunteering. Sharing the CI/Release burden between charts of different projects does seem really attractive.

@torstenwalter
Copy link
Contributor Author

torstenwalter commented Sep 24, 2020

Wow, this is a great issue. Apologies that we haven't gotten around to discussing this much internally and ❤️ for all the volunteering. Sharing the CI/Release burden between charts of different projects does seem really attractive.

There are enough volunteers to make this happen ;-)
If you want this it should not be hard to find someone doing the migration.

@slim-bean
Copy link
Collaborator

I think all the necessary discussions have been had here and everyone is in support of this move.

Apologies for the slow response but everyone on the Loki project is heads down right now trying to finalize some really cool new features.

I would like to pick this up next week (hopefully, if not the week after) and see if we can come up with a plan to migrate the helm charts as well as talk about upstreaming @unguiculus hard work!

Thanks everyone, hold tight we will hopefully get this sorted out over the next few weeks and please bare with us as we juggle a bunch of stuff :)

@torstenwalter
Copy link
Contributor Author

No worries thanks for the feedback. I could create a PR for the helm charts repository to migrate the existing loki chart including it's history next week.

Afterwards one could deprecate the existing chart and upstream the work from @unguiculus.

For migration I also wrote a small guide: https://github.com/torstenwalter/helm-chart-hosting

If you want you could also set up a CNAME alias like charts.grafana.com for https://github.com/grafana/helm-charts

@unguiculus
Copy link
Contributor

@owen-d @slim-bean We should also discuss how we want to handle the existing chart vs. the one I've been working on. The existing one only supports the monolithic mode while mine is made for the microservices mode. I would tend to keep two separate charts instead of merging them into one. Otherwise, this would involve a whole lot more conditional logic in the chart which I'd rather like to avoid. Plus, it would complicate configuration.

@owen-d
Copy link
Member

owen-d commented Oct 1, 2020

That sounds reasonable to me.

@torstenwalter
Copy link
Contributor Author

torstenwalter commented Oct 3, 2020

It took a bit, but finally I created two PRs for the migration of the charts.

grafana/helm-charts#39 adds the existing charts from this repository to https://github.com/grafana/helm-charts. It keeps the complete history and has some changes which would be needed for a migration. PR contains more details on that. Would be great if maintainers of the existing chart could review and approve it.

Existing charts mention "Loki Maintainers" - which GitHub usernames are behind that?
Would it make sense to list them explicitly in the Chart? I am asking as I had to disable validate-maintainers in chart-testing. We could use those names to add them to a CODEOWNERS file and require reviews from CODEONWERS in the new repository.

Are you fine with https://grafana.github.io/helm-charts or do you prefer an alias like charts.grafana.com, if so then someone would need to set up a CNAME record which points to grafana.github.io. See also GitHub Pages documentation. Afterwards we would need to add a CNAME file like this one and update the READMEs. I could include that in the existing PR if desired.

Once the PR grafana/helm-charts#39 is merged we would need to verify that everything works as expected and could merge #2720, which deprecates charts in this repository. All charts in this repo should be published with this change so that people are informed about the move. Afterwards it should be safe to remove the helm directory completely.

Follow up steps would be:

@unguiculus
Copy link
Contributor

unguiculus commented Oct 4, 2020

Since we seem to agree that we should have two separate Loki charts, one for monolithic and one for a distributed setup, I'd suggest I rename my chart to loki-distributed before moving it here.

@owen-d @slim-bean Feel free to suggest a different name.

@owen-d
Copy link
Member

owen-d commented Oct 7, 2020

Sounds good to me, thanks!

@oleksandrsemak
Copy link

Hey @unguiculus do you mind merge your loki-distributed to https://github.com/grafana/helm-charts ?

@unguiculus
Copy link
Contributor

@alexandrsemak This is the plan. I will just have to get things sorted with the Grafana folks and do some adjustments to the charts tooling (helm/chart-releaser#81).

@owen-d
Copy link
Member

owen-d commented Oct 28, 2020

I've added this to our 2.1 milestone.

grafana/helm-charts#39 has a lot of work already done to enable moving this out of the Loki repo.

Thanks for adding support for GPG signing to the helm releaser: https://github.com/helm/chart-releaser/pull/81/files

Now that we've finished the 2.0 release, we should be able to free up some time to support this endeavor.

Are there any blocking issues besides

  1. Figuring out who will own/have access to grafana/helm-charts
  2. Associated with (1), how are we going to seed credentials for signing the charts?

/cc @slim-bean

@torstenwalter
Copy link
Contributor Author

For 2. one could use encrypted secrets for the repository.

In terms of GPG Key it would be the easiest if we could re-use the one you are already using for Loki. It would be best if whoever has access to that private GPG key sets up the encrypted secret.

How would we proceed with grafana/helm-charts#39?
I could migrate existing loki chart releases (tgz files) to that repository already. That would help as we could point dependencies to the new location without bumping versions in requirements.yaml.

@unguiculus
Copy link
Contributor

That's great news @owen-d @slim-bean. Before we can proceed moving the charts, a new release of chart-releaser must be cut and the corresponding GitHub action must be updated and released as well. I'll try and get that done shortly. Once this has happened, I'd suggest we proceed with the following steps:

  1. Configure GPG keyring as secret as suggested by @torstenwalter
  2. Update GitHub workflow to enable GPG signing for all charts in the repo
  3. Release a signed version of the Grafana chart to verify things work as expected
  4. Update and merge Add loki charts helm-charts#39
  5. Transfer charts from https://github.com/unguiculus/loki-helm-chart/ to this repo

I'd suggest we add a CODEOWNERS file and require reviews from the respective chart maintainers. Maintainers should be added as trusted collaborators.

@unguiculus
Copy link
Contributor

The tooling is now ready for GPG signing. The demo repo has been updated: https://github.com/helm/charts-repo-actions-demo

@owen-d @slim-bean We would need two secrets in the charts repo, one for the base64-encoded secret keyring (GPG_KEYRING_BASE64) and one for the passphrase (GPG_PASSPHRASE). Could you provide these? Feel free to ping me in Grafana Slack for further questions.

unguiculus added a commit to unguiculus/grafana-helm-charts that referenced this issue Nov 20, 2020
This updates GitHub actions and tools used in GitHub workflows to the
latest versions.

See https://github.com/helm/charts-repo-actions-demo for reference.

This update adds support for GPG signing which will be required for the
planned addition of Loki charts. See grafana/loki/issues/2593

Signed-off-by: Reinhard Nägele <unguiculus@gmail.com>
unguiculus added a commit to unguiculus/grafana-helm-charts that referenced this issue Nov 20, 2020
This updates GitHub actions and tools used in GitHub workflows to the
latest versions.

See helm/charts-repo-actions-demo for reference.

This update adds support for GPG signing which will be required for the
planned addition of Loki charts. See grafana/loki/issues/2593

Signed-off-by: Reinhard Nägele <unguiculus@gmail.com>
unguiculus added a commit to unguiculus/grafana-helm-charts that referenced this issue Nov 20, 2020
This updates GitHub actions and tools used in GitHub workflows to the
latest versions.

See github.com/helm/charts-repo-actions-demo for reference.

This update adds support for GPG signing which will be required for the
planned addition of Loki charts. See grafana/loki/issues/2593

Signed-off-by: Reinhard Nägele <unguiculus@gmail.com>
unguiculus added a commit to unguiculus/grafana-helm-charts that referenced this issue Nov 20, 2020
This updates GitHub actions and tools used in GitHub workflows to the
latest versions.

See https://github.com/helm/charts-repo-actions-demo for reference.

This update adds support for GPG signing which will be required for the
planned addition of Loki charts. See grafana/loki/issues/2593

Signed-off-by: Reinhard Nägele <unguiculus@gmail.com>
unguiculus added a commit to unguiculus/grafana-helm-charts that referenced this issue Nov 20, 2020
This updates GitHub actions and tools used in GitHub workflows to the
latest versions.

See https://github.com/helm/charts-repo-actions-demo for reference.

This update adds support for GPG signing which will be required for the
planned addition of Loki charts. See grafana/loki/issues/2593

Signed-off-by: Reinhard Nägele <unguiculus@gmail.com>
unguiculus added a commit to unguiculus/grafana-helm-charts that referenced this issue Nov 20, 2020
This updates GitHub actions and tools used in GitHub workflows to the
latest versions.

See https://github.com/helm/charts-repo-actions-demo for reference.

This update adds support for GPG signing which will be required for the
planned addition of Loki charts. See grafana/loki/issues/2593

Signed-off-by: Reinhard Nägele <unguiculus@gmail.com>
@torstenwalter
Copy link
Contributor Author

torstenwalter commented Nov 20, 2020

To give short update on this grafana/helm-charts#94 contains the requested CODEOWNERS.

@owen-d @slim-bean could you add the required secrets in https://github.com/grafana/helm-charts/settings/secrets/actions?

@owen-d @slim-bean We would need two secrets in the charts repo, one for the base64-encoded secret keyring (GPG_KEYRING_BASE64) and one for the passphrase (GPG_PASSPHRASE). Could you provide these? Feel free to ping me in Grafana Slack for further questions.

I could then continue with:

  • Update GitHub workflow to enable GPG signing for all charts in the repo
  • Release a signed version of the Grafana chart to verify things work as expected

@slim-bean I could redo the work done in grafana/helm-charts#39 and move the charts. Just need a ping from you to do so. It only makes sense if you then stop publishing new version of the charts to https://github.com/grafana/loki/tree/master/production/helm. Otherwise charts with the same version, but different content might exist in two repos...

@torstenwalter
Copy link
Contributor Author

Charts are migrated only thing left is to deprecated the charts in their previous location and adjust CI pipeline to publish future changes to the new location.

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 a pull request may close this issue.

7 participants