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

Automation: improve quality of update-charts automation #389

Open
flavio opened this issue Mar 1, 2024 · 4 comments
Open

Automation: improve quality of update-charts automation #389

flavio opened this issue Mar 1, 2024 · 4 comments

Comments

@flavio
Copy link
Member

flavio commented Mar 1, 2024

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 Feature Request: Handle updates from -rcX to -rcX+1 in Update chart job #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 😞

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 😅).

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

  • Create a PR 123 that bumps component foo to version x.y.z
  • A new version of foo is created, but PR 123 is still open
  • Create new PR 124 that bumps component foo to version x.y.(z+1)
  • Close PR 123

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

@olblak
Copy link
Member

olblak commented Mar 1, 2024

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 #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 updatecli/updatecli#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 updatecli/updatecli#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 updatecli/updatecli#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 civo/kubernetes-marketplace#545 (comment)

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 civo/kubernetes-marketplace#685

updatecli/updatecli#1363 and updatecli/updatecli#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
Copy link
Member Author

flavio commented Mar 4, 2024

@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
Copy link
Member Author

flavio commented Mar 8, 2024

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

@olblak
Copy link
Member

olblak commented Apr 6, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

2 participants