[kube-prometheus-stack] Chart bump conflicts

[GMartinez-Sisti]

Is your feature request related to a problem ?

The problem I'm trying to resolve is related to conflicts with chart version on PRs. This is focused on kube-prometheus-stack but it's applicable to any of the helm charts on this repository.

The problem: Contributors need to patch the chart version when they submit PRs. Most of the times, merging other PRs causes conflicts on the chart version and we (maintainers) need to ask contributors to bump the version. Most often than not, they do it, but then another PR is merged and we have to ask again. This is highly frustrating to all the parties involved and it slows down the process a lot.

Describe the solution you'd like.

Bumping a chart version should be automated.

We can use labels or require the conventional commit format to infer the required bump. The bump then can happen on the PR automatically before the ct linter job runs (preferred), or after merging to main.

Describe alternatives you've considered.

NONE

Additional context.

No response

[GMartinez-Sisti]

@QuentinBisson FYI - trying to start a discussion 😄

[QuentinBisson]

I wholefully agree especially on highly used/changed charts as this is really painful for kube-prometheus-stack

[GMartinez-Sisti]

@jkroepke FYI if you have ideas too!

[jkroepke]

I had this issue multiple times in context of software development where we had tons of merge conflicts because the version number was part of the VCS.

My proposal would be that we leave the version number to 0.0.0 in Chart.yaml at VCS and set the version number only on publish. The helm package command supports this scenario, we don not have to use sed hacks to modify Chart.yaml version.

If this fine for everyone, I would look deeper how Github Action could identify the "next version". I personally would go with labels "major", "minor", "bugfix" on the Pull Requests, if this is possible.

This avoid infinite loops on PRs.. between author and reviewer. Additionally, I expect that the CI which runs in context of promethues-community does not have any permissions in the forked repository.

[GMartinez-Sisti]

My proposal would be that we leave the version number to 0.0.0 in Chart.yaml

I use this logic for internal repositories, but for public ones I think users are expecting to see the latest version on the main branch.

To prevent further issues along the line (after merging), I would prefer that the PR would be merged with the correct bump already. So a job would run before the lint/install to check the label and bump automatically. That way the linter can also check that it was bumped properly. If something is merged we would just press the "Update Branch" button and the job would run again.

The only bad consequence of the above is that we would have multiple bumps, but we already have that. And we also need to check if it's going to create any issues with verified commits.

WDYT?

[jkroepke]

and bump automatically

We may have to test this. As mention earlier I don't expect that Github Actions in our repository context does have push permissions to a forked repository.

Additional, some logic is requires in case of merge conflicts + still we need the a logic what is the next version?

but for public ones I think users are expecting

I may agree with you, but I have the feeling that resolving our maintenance issues should have an higher priority than what public users may expect. The version number is still reachable through git tags

Removing the requirement to maintenance the version maybe would the easiest one.

[jkroepke]

What about, if we as maintainer resolve the merge conflict manually before do the merge, if it's only the version itself?

In most cases, PR authors enabled commits from maintainers.

GitHub provides a good WebUI for this use-case to resolve an conflict without having the requirement of a local checkout.

It does requires zero complex scripts to archive it

[jkroepke]

While this issue is still not fully resolved, got inspired by

https://github.com/terraform-aws-modules/terraform-aws-eks/

The maintainer at https://github.com/terraform-aws-modules/ are using semantic-release as bare bone.

It bases on conventional commits where a commit message prefix indicates the version bump. renovate supports to setup PRs with such an prefix.

Kindly request to all contributor to use conventional commits for commits would be an endless story. But if we always use squad merges for PRs, than the PR title is used as commit message. As result, we only have to enforce the PR title here.

https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/.github/workflows/pr-title.yml

based on that, end-users must not longer bump the chart version on PRs.

And we could provide a CHANGELOG.md to end users.

Not sure, if semantic-release support mono-repos, but I guess it would be.

WDYT? @GMartinez-Sisti @QuentinBisson @monotek

[GMartinez-Sisti]

I like the concept, but I think this needs to be enabled for all the charts on the repository to work. I would suggest we create a POC with a repository to try and find all the caveats.

If this works I have other places where this model would improve the workflows too 😄

[monotek]

@jkroepke sounds good to me :)

[jkroepke]

It turns out semantic-release does not support multi project repos: https://github.com/semantic-release/semantic-release/issues/193

However, I will try to setup it with an different tool. Since it would also resolve the renovate issue (not bumping the chart version).