kubewarden / helm-charts

Helm charts for the Kubewarden project
Apache License 2.0
25 stars 16 forks source link

Automation: improve quality of update-charts automation #389

Open flavio opened 6 months ago

flavio commented 6 months ago

The PRs produce by the update-charts automation are not useful.

These are the main pain points:

  1. The contain LOTS of commits (see for example this one)
  2. They are out of sync from the main branch. When looking at the diff the comparison is not made between the current main branch and the PR branch. On the left side of the diff there's the commit that used to be HEAD inside of the main branch.
  3. Handling of RCs is broken (but there's already https://github.com/kubewarden/helm-charts/issues/222 to keep track of that)

I just cleaned up a PR created by the bot, I had to go through a 24 commits rebase with lots of conflicts that needed to be solved manually. It would have been easier - and quicker - to just create a new PR by hand, from scratch :disappointed:

Note: personally I find the 2nd pain point the most annoying one

How should we tackle that

Let's just ignore the 3rd pain point, for which we have a dedicated issue.

The 2nd pain point seems to be a limitation of the GitHub API (or something like that, @olblak gave me an explanation sometimes ago, but I forgot :sweat_smile:).

I've seen other bots, like dependabot, using the following approach:

Maybe we could do something like that? This could also reduce the number of commits inside of these PRs (the 1st point)

olblak commented 6 months ago

I am sorry to hear your pain points. All of them are identified on Updatecli side, just missing time to fix them.

The contain LOTS of commits (see for example https://github.com/kubewarden/helm-charts/pull/377)

They are two different open issues that could solve your pain point, one to improve the yaml plugin to allow multiple yamlpath queries suggested by @jvanz https://github.com/updatecli/updatecli/issues/1056. It would reduce the number of target needed and therefor the number of commit as commit title are generated by Updatecli target name.

And the second one related to https://github.com/updatecli/updatecli/issues/465 which would move all the git operation to a separated action that would allow more customization like either one commit per target or one commit per group of target.

The first approach suggested by Jose would definitely be easier to implement. The second one is part of a larger refactoring which we struggle to work on the side so we move slowly bits by bits 😞

They are out of sync from the main branch. When looking at the diff the comparison is not made between the current main branch and the PR branch. On the left side of the diff there's the commit that used to be HEAD inside of the main branch.

They are two problems, one on Updatecli side, I have an issue here https://github.com/updatecli/updatecli/issues/1747 and another one on GitHub side (I don't find anymore the GitHub issue tracking that problem :D ). The problem related to GitHub, is a git push can randomly reopen an old deleted branch which lead to wrong pullrequest information either with merge conflicts that shouldn't exist or wrong information like this one https://github.com/civo/kubernetes-marketplace/pull/545#discussion_r1303929371

When this happen, I close the pullrequest and delete the branch so Updatecli can reopen another one from the correct base branch

I think the GitHub issues should disappear or at least be mitigated once I implement the merge/rebase functionality.

Adding merge/rebase is pretty high on my priority list, I missing some feature in the go-git library so it's not trivial to fix but to me it's probably the most annoying bug on Updatecli today.

I've seen other bots, like dependabot, using the following approach:

I've been scratching my head around that problem too but it's a lot more difficult to handle in Updatecli because Updatecli can update a lot of different type of information. For example this pullrequest related to Kubernetes on CIVO contains more than one type of changes https://github.com/civo/kubernetes-marketplace/pull/685

https://github.com/updatecli/updatecli/issues/1363 and https://github.com/updatecli/updatecli/issues/1364

This problem prevents me to make progress on the Udash project. I would like to be back on that project too but I am struggling to find enough time and first I want to improve the different git operation like rebase or merge in Updatecli.

flavio commented 5 months ago

@olblak thanks for the detailed explanation.

I get the feeling that, until a better solution is available, the easiest workaround for us would be to:

In this way we would have PRs that are easier to review.

What do the other @kubewarden/kubewarden-developers think about that? If you agree with this workaround, then we need to update this GH action to expose a manual trigger

flavio commented 5 months ago

Not actionable, we're waiting for updatecli to implement the fixes mentioned by @olblak

olblak commented 4 months ago

For info I just released v0.75.0 which address pain point 2 :)