Replace self-upgrade workflow with Renovate JSONata manager#636
Conversation
cert-manager-prow
Bot
added
dco-signoff: yes
There was a problem hiding this comment.
Pull request overview
This PR replaces the repository “self-upgrade” automation (previously implemented as a scheduled GitHub Actions workflow) with a Renovate custom JSONata manager that updates klone.yaml, aiming to spread upgrade PRs over time and reduce CI load spikes.
Changes:
- Add a Renovate
customManagersJSONata configuration to extract dependencies fromklone.yaml. - Group
klone.yamlupdates and run post-upgrademaketasks after Renovate updates. - Remove the generated
make-self-upgradeGitHub workflow and its Chainguard STS permissions file; add cleanup ingenerate-basefor downstream repos.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| renovate-config.json5 | Adds Renovate JSONata custom manager and package rule for klone.yaml; removes ignore for old workflow. |
| modules/repository-base/base/.github/workflows/make-self-upgrade.yaml | Removes the generated scheduled self-upgrade workflow. |
| modules/repository-base/base/.github/chainguard/make-self-upgrade.sts.yaml | Removes the Chainguard STS permissions file used by the deleted workflow. |
| modules/repository-base/01_mod.mk | Stops templating repository placeholders for base files and removes legacy self-upgrade files in downstream repos. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "make vendor-go", | ||
| "make generate-klone", | ||
| "make generate" |
There was a problem hiding this comment.
note: I think there's something interesting here I'd not thought enough about before.
vendor-go is in the tools module and generate-klone is in the klone module. generate is a target built from several modules. In this PR we're changing the repository-base module.
I think the implication is that to use this self upgrade flow (maybe to use makefile modules at all?) you need at least the repository-base, tools and klone modules.
Makes me think that our separation of modules isn't quite right - we have really important cross dependencies across these three core modules. I wonder if it would make sense to condense these modules down into a single base module, conceptually.
(Not a blocker or anything because this is only for our consumption and we use those modules everywhere anyway - I'm just thinking out loud)
There was a problem hiding this comment.
There is actually a meaningful implication of this - the make-self-upgrade GH action used the setup-go action to provision go, so the repository-base + klone modules were all that were required for self-upgrades. Now in this PR we add a (hard) dependency on the tools module because we're using make vendor-go instead.
I don't think that's a blocker either, but it's worth calling out.
There was a problem hiding this comment.
Good point! Do we really need to have an exact Go version when running make generate-klone and make generate? Or would it be sufficient to have a Go version newer than version X?
There was a problem hiding this comment.
I had another look, and while I agree on your problem statement about module separation, I think this could be fixed in a follow-up PR I'll vote in favour of moving the vendor-go and unvendor-go targets to the repository-base module. And also merge the klone module into the repository-base module. WDYT, @inteon?
But in the context of this PR, I propose to keep it as proposed. I just tested this PR in the website project, which is our project with less Go content, and it works.
SgtCoDFish
left a comment
SgtCoDFish
left a comment
There was a problem hiding this comment.
/lgtm
/approve
/hold
Hold because of the potential finding, but if you think it's bogus just unhold and merge.
| extends: [ | ||
| 'github>cert-manager/renovate-config:default.json5', | ||
| ], | ||
| "customManagers": [ |
There was a problem hiding this comment.
suggestion(claude): Found by AI, edited by me. I've not dug into checking if this is legit, so I'm not going to consider it a blocker or anything - but if you think it's legit + worth fixing, I'm happy to re-review next week!
Missing executionMode: 'branch'
The new packageRules entry for custom.jsonata is missing executionMode: 'branch', which every other rule with postUpgradeTasks in the repo uses.
Without it, Renovate defaults to per-update mode, meaning the three make commands run once per updated module entry. For a typical klone.yaml with several modules all pinned to the same hash those commands would run 7 times on a single branch. With executionMode: 'branch', they run once for the whole branch.
There was a problem hiding this comment.
Ok, this is interesting. I am going to POC this in the helm-tool project.
There was a problem hiding this comment.
There was a problem hiding this comment.
Ok, this works, and I have updated the PR to include executionMode: 'branch'.
cert-manager-prow
Bot
added
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: SgtCoDFish The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
cert-manager-prow
Bot
added
the
approved
Signed-off-by: Erik Godding Boye <egboye@gmail.com>
erikgb
force-pushed
the
renovate-self-upgrade
branch
from
8f7d6c0 to
36c614a
Compare
cert-manager-prow
Bot
removed
the
lgtm
|
New changes are detected. LGTM label has been removed. |
The JSONata experiment added by @ThatsMrTalbot in https://github.com/cert-manager/helm-tool appears to be working now. This is how the self-upgrade PRs might look like after this change: cert-manager/helm-tool#272.
We currently schedule all self-upgrade jobs at the same cron schedule, which makes the Prow jobs fail regularly - probably because of overload in the Prow cluster. With this change, Renovate will ensure the changes are distributed in a more randomly timed fashion.
We probably want to automerge the makefile-module upgrades, as we currently do, but I think we can do it in a follow-up PR. But I am a bit worried about noise, since Renovate will kick in more often than once a day - if not configured to do something else.