prometheus-community / community

Prometheus & The Ecosystem Community Meeting Information
20 stars 3 forks source link

Create a new git repo for prometheus-community helm charts #28

Closed scottrigby closed 3 years ago

scottrigby commented 3 years ago

Hello ๐Ÿ‘‹

Problem

Helm stable and incubator repos are EOL on Nov 13, 2020 (See support plan and Deprecation Timeline). The community (chart OWNERS, organizations, groups or individuals who want to host charts) are moving charts to new Helm repos, and will list these new repos on the Helm Hub before stable and incubator are de-listed there.

Proposed solution

From there, these remaining checklist items can move to an issue in the new repo. I'm listing them here so everyone can see the end-to-end plan:

Additional context

~Open question about prometheus-operator chart~

Update: it was decided that the prometheus-operator chart should move to the prometheus-community org, per https://github.com/prometheus-operator/prometheus-operator/issues/3169#issuecomment-669793279

~@paulfantom pointed me to the open issue at https://github.com/prometheus-operator/prometheus-operator/issues/3169 and I have commented there.~

Relevant notes from IRC

From chat in Freenode #prometheus-dev IRC today

I'm part of the Helm team, focusing largely on charts, repos and related tooling. Just caught up on the "Helm charts home" mailing list topic https://groups.google.com/g/prometheus-developers/c/Gh8GMscELao/m/2zZFPwdLBwAJ?pli=1 and would like to help however I can to get questions resolved for the new home(s) for stable prometheus charts before the current repo deprecation.

@brancz and I had an earlier Slack thread with Naseem in February/March about this. I think the mailing thread covered most of the topics we discussed.

In general, an idiomatic approach for an org hosting multiple Helm charts is a monorepo. If on GitHub, Helm has created Actions for consistent chart testing during CI and chart packaging releasing for CD (listed in the mail list thread). For history sake, we normally recommend preserving git history (using filter-branch) when moving charts, so that it's clear to future devs why past changes were made, and to credit those who made them.

That thread bike-shedded a bit on details like naming (really just keep it simple, no prefixes or suffixes needed etc). But there were two other points that I think are legit potential blockers to discuss:

  1. how the prom charts should be ideally structured vs how the stable ones people depend on now are structured
  2. whether there should be several categories of charts, clearly separated somehow for end users (example: a. charts owned by prometheus to be better architected and named, b. charts maintained by the community moved from stable as-is and refactored over time, c. charts focused on external projects like the prom jira-alerts. Or maybe b and c should be combined etc)

My suggestion to keep things as simple as possible is to take a multi-stage approach:

  • First ask a Prometheus GitHub admin to create a new git repo https://github.com/prometheus-community/helm-charts. And invite a few collaborators as repo admins to help set it up (I'm happy to help), as well as write access for current chart maintainers (we can follow up about github usernames)
  • Then we can bring in the existing charts as-is, with a goal of refactoring, and deprecating/renaming as needed from there according to ongoing discussion about more appropriate architecture/naming

โ˜๏ธ This would still allow specific refactored charts to later be moved to https://github.com/prometheus/helm-charts if prometheus developers prefer an easier to distinguish charts maintained internally vs those maintained by the community. But they could also remain in prometheus-community if this isn't desired. My main goal is to unblock the 17 stable community prometheus-related Helm charts currently in use by end users today. Here is an issue we're using to track progress on moving all existing stable charts https://github.com/helm/charts/issues/21103

@SuperQ said:

The first step for that is to open an issue on https://github.com/prometheus-community/community/issues

@brian-brazil said:

I don't think it should ever be in the main prometheus org

Personally I agree. Some people in the mailing list suggested that so I just want to clearly note that the community repo wouldn't preclude individual charts moving elsewhere later, for any technical reasons related to helm repos, hub aggregation etc.

SuperQ commented 3 years ago

Anyone volunteer to be the maintainer sponsor? @bwplotka @brancz @gouthamve ?

hickeyma commented 3 years ago

Just to add some feedback that we have received in the Helm community in the last few months. There has been a lot of questions wrt prometheus Helm charts and if they will be supported by the prometheus community going forward. The other question been asked is, when will the charts be Helm 3 compatible.

stuart-c commented 3 years ago

@hickeyma what do you mean by Helm 3 compatible? I've been using Helm 3 with the charts for 6 months so far...

hickeyma commented 3 years ago

IIRC, someone mentioned that it was not compatible at some stage.

Ah, my bad. Checking the operator chart, I can see that it has indeed been updated for Helm 3 CRDs. Thanks for the heads up @stuart-c

brancz commented 3 years ago

I'm happy with sponsoring the chart for the prometheus-community org, under the condition that:

stuart-c commented 3 years ago

@brancz when you say "the chart" are you referring to the prometheus operator chart, as the repo would be home to all the prometheus related charts (prom operator, node exporter, alertmanager, other exporters, etc)?

SuperQ commented 3 years ago

I've created @prometheus-community/helm-charts GitHub team to manage things.

brancz commented 3 years ago

I'm specifically talking about what is in the upstream charts repo referred to as the prometheus-operator chart.

stuart-c commented 3 years ago

I'd be happy to lend a hand if you would like an additional maintainer...

Nexucis commented 3 years ago

It is renamed to kube-prometheus-chart to actually reflect its content, and confuse users less

Sorry by advance for the dummy question, but what is the point of putting kube in the name of the repo ?

Helm-charts is for sure for kube. People that doesn't make the link probably shouldn't use it.

And you already have the name prometheus in prometheus-community. IMHO prometheus-community/helm-charts is obviously the different helm charts relative to the different prometheus repositories which then should be used into kube

scottrigby commented 3 years ago

@brancz OK, I updated the issue description to say it was decided that the prometheus-operator chart should move to the prometheus-community org, per https://github.com/prometheus-operator/prometheus-operator/issues/3169#issuecomment-669793279 ๐Ÿ‘Œ

Simplifying charts

Regarding this note from that thread:

Aside from that, I'm excited to see how what you mentioned @scottrigby works out in practice around minimal helm charts, and hope to see the chart evolve in that direction.

Yes! I'm happy to continue the discussion with chart maintainers to help this happen. One potentially valuable direction could be to create new, minimal charts at some point once refactoring/consolidation/renaming is agreed upon as chart API v2 charts (the default when using helm create with Helm 3). These could be a breaking MAJOR version to the same charts after Helm 2 is officially deprecated in November, or we could start earlier and these could live alongside the legacy ones until the old charts are deprecated. The only thing we can't do is have two charts with the same name in the same helm repo.

Multi-step process

Clarifying, do you agree we should follow the multi-step process I summarized in this issue description above? If it's not clear, I'll expand on it a bit here:

Step 1. Move over all existing stable prometheus-related charts to a new space: github.com/prometheus-community/helm-charts

These are the prometheus related charts:

  1. stable/prometheus
  2. stable/prometheus-adapter
  3. stable/prometheus-blackbox-exporter
  4. stable/prometheus-cloudwatch-exporter
  5. stable/prometheus-consul-exporter
  6. stable/prometheus-couchdb-exporter
  7. stable/prometheus-mongodb-exporter
  8. stable/prometheus-mysql-exporter
  9. stable/prometheus-nats-exporter
  10. stable/prometheus-node-exporter
  11. stable/prometheus-operator
  12. stable/prometheus-postgres-exporter
  13. stable/prometheus-pushgateway
  14. stable/prometheus-rabbitmq-exporter
  15. stable/prometheus-redis-exporter
  16. stable/prometheus-snmp-exporter
  17. stable/prometheus-to-sd

Step 2. Collaborate with prometheus devs to refactor/rename and simplify charts as appropriate

Does this make sense?

scottrigby commented 3 years ago

@SuperQ re https://github.com/prometheus-community/community/issues/28#issuecomment-669915094 if you would like to add me to the @prometheus-community/helm-charts GitHub team I'm more than happy to help configure the initial repo, move over the initial version of the charts, set up the standard helm repo CI/CD etc. There may also be other helm maintainers who would be willing to help with ongoing repo-level administration help, though I think after initial setup the prometheus-community members can also take over this part.

We can also discuss the process of adding chart maintainers, who will require git repo write access. I would recommend asking the current maintainers of the existing charts if they would like to be added to this. If there is a reason to differentiate maintainers of each of the individual charts, this can be done with a GitHub CODEOWNERS file (I'm not assuming that's necessary, just noting this is a good solution if it becomes a need through discussion about chart maintenance moving forward).

Please just lmk when this is ready and I will hop on it. If I miss replies about that here I'd appreciate if someone wouldn't mind pinging me in Slack or IRC. Thanks!

stuart-c commented 3 years ago

As part of this will the existing charts in the helm org be deprecated and their README files updated to point here?

matthiasr commented 3 years ago

I think this is a good move, from an ecosystem standpoint it makes sense. I'm not 100% sure what the governance model is now (prometheus-community/community#7) but if we need a formal sponsor from -team I am willing to take that. Unless we have someone who actually uses Helm day-to-day, that would be even better then?

brancz commented 3 years ago

@Nexucis kube-prometheus was created without ever intending to build a helm chart out of it :)

@scottrigby As I said before, my condition is that the "prometheus-operator" chart as it exists today with that name must change. It does not just deploy the prometheus operator, it deploys an entire cluster monitoring stack, that includes many many other components. I'm not against having a chart like that, but it must not be called prometheus operator, this naming has hurt the prometheus operator maintainers tremendously over the years as everyone opens issues directly on the prometheus operator repository about things that have nothing to do with the prometheus operator. I am ok with that chart being called kube-prometheus, because that is exactly what that helm chart was derived from. I am also ok with having a minimal helm chart called prometheus-operator, like the bitnami one, that truly just deploys the prometheus operator. I actually have exactly the same issue with the "prometheus" chart as it is based off of a very old version of kube-prometheus, that predates kube-prometheus using the prometheus operator.

I am perfectly happy with having all of these charts in general, as well as the two "meta" charts (prometheus and prometheus-operator as they are called today), but their name must accurately reflect their content.

scottrigby commented 3 years ago

@stuart-c Yes that would be the process, along with steps in https://github.com/helm/charts/blob/master/PROCESSES.md#deprecating-a-chart ๐Ÿ‘ Then in https://github.com/helm/charts/issues/21103 we'll link to the helm/charts repo PR where that deprecation and notes to the new repo are added, just to help users keep track all in one place.

@matthiasr great ๐Ÿ™Œ

@brancz Those conditions make sense. I can also understand the frustration that came over time from that chart naming choice. I agree this seems like a good time to correct that once and for all. If we deprecate the chart before it's published to the new helm repo, it won't appear in the hub aggregators either.

One path forward is I could create a temporary git repo under my namespace, so that you can look it over before approving, then transfer it to the https://github.com/prometheus-community. That way we can adjust names as needed without confusing end users before fully approving and moving over the repo as the new starting point. What do you think?

naseemkullah commented 3 years ago

On a related note, I notice there is no alertmanager chart in stable repo, I've prepared one here: https://github.com/naseemkullah/naseem-charts/tree/master/charts/alertmanager and would be glad to move it to the new repo in efforts to modularize the components into subcharts.

Though this would be more for vanilla prometheus, as I think alertmanager is a CRD in prometheus-operator.

SuperQ commented 3 years ago

@scottrigby Setting up the new github repo and then moving it sounds like a good solution.

brancz commented 3 years ago

Thank you @scottrigby! This gives me hope! :)

You have my approval and sponsorship. (I'm soon on vacation so unfortunately I don't think I will be very helpful with actually doing the move, but it appears that @SuperQ might be able to help :) )

scottrigby commented 3 years ago

I set up an initial version of this a few days ago, and took a first stab at renaming the prometheus-operator chart to kube-prometheus (also linked above) https://github.com/scottrigby/prometheus-helm-charts/pull/1

Would someone be willing to review/test/fix whatever I broke by doing this? I added some detailed questions in the PR. Thanks!

Iโ€™d like to put PRs to at least stable/prometheus on hold in the stable until this is resolved, but we donโ€™t have any good automated way to ensure that. So hopefully we can get this done ASAP. Thanks in advance for any help on this ๐Ÿ™‚

scottrigby commented 3 years ago

Iโ€™ve added hold/do-not-merge label to all open PRs for stable/prometheus-operator. If anyone sees a new PR there that I miss, please feel free to /hold it and link to this issue. Thanks!

naseemkullah commented 3 years ago

@brancz Can you confirm that kube-prometheus should be able to swap in vanilla prometheus/alertmanager as a replacement to prometheus-operator?

brancz commented 3 years ago

@naseemkullah kube-prometheus is literally a subproject of prometheus-operator. Of course you can change that, but that wouldn't be kube-prometheus anymore. Let's keep discussions about changes to the helm charts out of this thread I would say.

Hokwang commented 3 years ago

looking forward this done

zanhsieh commented 3 years ago

Let me know what else I could do with stable/prometheus chart.

scottrigby commented 3 years ago

Let's keep discussions about changes to the helm charts out of this thread I would say.

@brancz I agree this makes sense. Want to move discussions about chart changes over to https://github.com/scottrigby/prometheus-helm-charts?

I added issues and an (automated) kanban project board to help everyone track steps and progress. Everyone: please let me know if I've missed anything there.

Current chart maintainers (how you can help):

  1. I'll be adding you as collaborators now per https://github.com/scottrigby/prometheus-helm-charts/issues/12 Please see the maintainers poll on that issue and weigh in. I'll start by reproducing the same access we have on the stable repo prometheus charts, unless there's overwhelming consensus not to (we can always change it afterwards). Once this is done, we can enable branch protection requiring reviews on PRs
  2. Also please see another maintainers poll about README update pattern here https://github.com/scottrigby/prometheus-helm-charts/pull/14

Thanks everyone!

gkarthiks commented 3 years ago

@scottrigby @brancz @SuperQ this is awesome initiative, count me in if you need additional maintainers to the new Prometheus Community repo.

scottrigby commented 3 years ago

@gkarthiks Do you mean for additional charts? Since you're already added to the initial git repo as the maintainer of prometheus-couchdb-exporter you will continue to be added once the repo moves back to the prometheus-community org (see https://github.com/scottrigby/prometheus-helm-charts/issues/20 for next steps ๐Ÿ˜„ We are nearly there!). For helping maintain other prometheus charts, you would need to ask the existing maintainers of those the chart(s) you want to help with, and have that convo. I've made an issue for discussion on thisย  https://github.com/scottrigby/prometheus-helm-charts/issues/21 so we can follow-up there.

QuentinBisson commented 3 years ago

I can help as well if a chart maintainer is needed

scottrigby commented 3 years ago

@omegas27 Would you mind also replying on this issue? https://github.com/scottrigby/prometheus-helm-charts/issues/21

For everyone else as well: mainly I'm trying to respect @brancz request to move discussion related to the charts to that repo. The discussion will persist when the repo is transferred to the prometheus-community org. Thanks!

scottrigby commented 3 years ago

Update: I believe we are ready to transfer! See https://github.com/scottrigby/prometheus-helm-charts/issues/11 for a maintainers poll.

@brancz your final approval is especially important here as you're sponsoring the move. I outlined the questions as I see them in that issue.

@SuperQ In order to transfer I'll DM you about team/permission details in IRC

sachinmsft commented 3 years ago

@scottrigby Is migration of prometheus-operator chart complete ? I wanted to shoot a PR to add optional Windows dashboards and rules once migration is done.

scottrigby commented 3 years ago

@sachinmsft There is a pull request in progress for the rename https://github.com/scottrigby/prometheus-helm-charts/pull/1. If you wish you can make a PR against the PR branch there (rename-prometheus-operator-kube-prometheus). Let's move further discussion about that one either to an issue in that repo or on your Pull Request itself.

Note: merging that PR has not been identified as a blocker to transferring the repo to prometheus-community. See https://github.com/scottrigby/prometheus-helm-charts/issues/11

sachinmsft commented 3 years ago

Thanks @scottrigby . I can wait couple of days till this is done.

scottrigby commented 3 years ago

This is done! ๐ŸŽ‰ https://github.com/prometheus-community/helm-charts

For maintainers, there's this GH "project" if you want to see the remaining post-transfer steps: https://github.com/prometheus-community/helm-charts/projects/2

Closing this now. Thanks to @brancz and @SuperQ as well as all the prometheus community charts maintainers ๐Ÿฆ„ ๐Ÿ’–

brancz commented 3 years ago

Thank you so much @scottrigby for taking the lead on this! I think we're in a much better place now than we were before.

scottrigby commented 3 years ago

My pleasure!