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

Decouple helm chart releases from application release stream #1392

Closed
paulfantom opened this issue Feb 23, 2021 · 14 comments · Fixed by #1448
Closed

Decouple helm chart releases from application release stream #1392

paulfantom opened this issue Feb 23, 2021 · 14 comments · Fixed by #1448
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@paulfantom
Copy link
Contributor

What happened:
Repository is constantly spamming about new version updates of helm chart and not the application.

What you expected to happen:
I expect to have a notification when a new version of the application comes out and not of some opinionated contrib part in form of a helm chart.

How to reproduce it (as minimally and precisely as possible):
Just go to https://github.com/kubernetes/kube-state-metrics/releases

Anything else we need to know?:

I am working on automatic updates of applications based on github releases. This is something I would like to have in kube-prometheus project and as of today I have 99% of this complete, apart of KSM piece where constant releases break everything up.

Environment:

  • kube-state-metrics version:
  • Kubernetes version (use kubectl version):
  • Cloud provider or hardware configuration:
  • Other info:
@paulfantom paulfantom added the kind/bug Categorizes issue or PR as related to a bug. label Feb 23, 2021
@lilic
Copy link
Member

lilic commented Feb 23, 2021

Thanks, I agree its very spammy for me as well, as someone who does not use helm.

@tariq1890 think you mentioned that something can be done around this from helm? Would this solve the spammy issue?

cc @mrueg @brancz

@paulfantom
Copy link
Contributor Author

Note: this is only about releases, not tags.

@mrueg
Copy link
Member

mrueg commented Feb 23, 2021

I think charts might require releases as there is an asset generated by the github action and I am not aware that this can be added to a tag.

Chart releases are easily identifiable by a common prefix, maybe as a short-term solution filtering on the client-side is possible?

@brancz
Copy link
Member

brancz commented Feb 23, 2021

I agree, it makes up the vast majority of notifications of this repo for me now even though I don't use or work on the helm parts. I'd also appreciate if we could have a way where this wasn't the case.

@paulfantom
Copy link
Contributor Author

paulfantom commented Feb 23, 2021

maybe as a short-term solution filtering on the client-side is possible?

A short-term fix was to exclude ksm from the updater. We need a proper fix for this and not a hacky workaround. Automation aside, spamming people notifications is not OK and if your automation requires spamming people, then it is just wrong.

@brancz
Copy link
Member

brancz commented Feb 24, 2021

if your automation requires spamming people, then it is just wrong.

I agree. Clearly for everyone who regularly interacts with this repo, this is problematic. @tariq1890 @mrueg @scottrigby the agreement of including helm in this repo was that it would not negatively affect maintenance of non-helm parts, which this shows was not the case.

I would really appreciate if we could get some proposals going on how to solve this, otherwise I think we're going to need to move it out of this repo again (which as it currently stands is my preference but I'm open to other suggestions).

@mrueg
Copy link
Member

mrueg commented Feb 24, 2021

Here are the options that I see:

  • Publish helm charts as a pre-release (which should not update the latest endpoint)
  • Client-side filtering on /releases endpoint instead of /releases/latest and e.g. pipe to jq '.[] | select( .tag_name|test("v."))|.tag_name'
  • Tie helm chart releases to kube-state-metrics releases (this might make helm contributors/users unhappy if the release cadence on kube-state-metrics is too low, maybe a monthly release train model would work)
  • Move helm chart to e.g. https://github.com/prometheus-community/helm-charts (which is low effort since automation is set up and we probably don't want to set up the repo on kubernetes' github namespace as it took very long to get it running there)
  • Figure out how to get Github to improve their notification system to something that is usable

@paulfantom
Copy link
Contributor Author

Client-side filtering on /releases endpoint instead of /releases/latest and e.g. pipe to jq '.[] | select( .tag_name|test("v."))|.tag_name'

GitHub already solved it by implementing /releases/latest. Doing such filtration on the client-side is reinventing the wheel.

Figure out how to get Github to improve their notification system to something that is usable

They already did that in some way as we can set different channels for notifications:

  • Participating and mentions
  • All activity
  • Ignore
  • Custom/Issues
  • Custom/Pull Requests
  • Custom/Discussions
  • Custom/Releases

The thing is, the current automation is spamming the channel which should have the lowest volume of notifications - Custom/Releases

Tie helm chart releases to kube-state-metrics releases (this might make helm contributors/users unhappy if the release cadence on kube-state-metrics is too low, maybe a monthly release train model would work)

Personally, I see this as the best option, but that is only my opinion.

Publish helm charts as a pre-release (which should not update the latest endpoint)

sgtm

@tariq1890
Copy link
Contributor

tariq1890 commented Feb 25, 2021

@paulfantom can you retitle this issue? I understand that a huge uptick in release notifications that arent so useful to one can be frustrating, but this is certainly not "spam" to the users of helm charts. However, having said that, I do agree that it is too much to have a release sent out for every commit.

I propose the following. If we are to continue to maintain (some of the maintainers) the helm charts as a part of this repo, it is easier And more tractable to consider the binary and helm chart together as an artifact. So if we were to fix multiple issues in the helm chart, we would just cut a release for both the binaries as well as the chart. The same would apply vice versus.

@tariq1890
Copy link
Contributor

tariq1890 commented Feb 25, 2021

I had already discussed this idea with @lilic and @mrueg before. We could go forward this strategy v2.0 on wards.

This might mean cutting a release which may sometimes include only helm chart changes (given that the helm charts see more activity in general than the application). But I think this won't be too much of an issue as we wont be too frequent with our releases either.

Does this sound acceptable to you? @paulfantom @brancz @lilic

@brancz
Copy link
Member

brancz commented Feb 25, 2021

I get the sentiment @tariq1890, but to the extend that it happens right now, I also classify it as spam, there is nothing valuable for a human to get this notification.

I think I'm willing to try to only have releases on the same "schedule" as the binary (as in the very same release). We can re-evaluate a few months into trying that approach then to see if it had the effect we wanted.

Could you take the lead on this @tariq1890 ? I would appreciate if we could have a timeline on it, because as it stands I just mark all kube-state-metrics notifications as read, because having to cut through the noise is not pleasant.

@paulfantom paulfantom changed the title Stop spamming releases Decouple helm chart releases from application release stream Feb 25, 2021
@paulfantom
Copy link
Contributor Author

paulfantom commented Feb 25, 2021

I understand that a huge uptick in release notifications that arent so useful to one can be frustrating, but this is certainly not "spam" to the users of helm charts

Via Merriam-Webster dictionary:

Spam - unsolicited usually commercial messages (such as emails, text messages, or Internet postings) sent to a large number of recipients or posted in a large number of places

I subscribed to notifications about releases of the application in this repository, but I get helm chart releases in those notifications. Hence for me, this is an unsolicited message and from my perspective (and clearly not only mine) this is spam.

That said, I changed the title.

@lilic
Copy link
Member

lilic commented Feb 25, 2021

I think I'm willing to try to only have releases on the same "schedule" as the binary (as in the very same release). We can re-evaluate a few months into trying that approach then to see if it had the effect we wanted.

+1 on this, lets give it a try, sooner rather than later. Because if I disable notifications from this repo I can't really do my job as a maintainer correctly and we will end up missing a lot of notifications this way. Thank you all!

@tariq1890
Copy link
Contributor

@paulfantom @brancz, I understand that and I do sympathise. But what I am trying to say is that it isn't "spam" for a good chunk of users who use these helm charts. Also not justifying the volume of release notifications either. That definitely needs to be addressed and I say that as a helm user myself. We are on the same page there.

I think I'm willing to try to only have releases on the same "schedule" as the binary (as in the very same release). We can re-evaluate a few months into trying that approach then to see if it had the effect we wanted.

Great! So we can move forward with this plan.

Could you take the lead on this @tariq1890 ? I would appreciate if we could have a timeline on it, because as it stands I just mark all kube-state-metrics notifications as read, because having to cut through the noise is not pleasant.

Definitely! So the timeline is basically having this figured out before we publish 2.0. As of 2.0, we'd need to make sure that the helm chart release happens along with the binary release. This will have to be treated as a critical action item for ksm 2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
5 participants