-
Notifications
You must be signed in to change notification settings - Fork 891
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
feat: Added Renovate to auto update helm-values and github-actions #979
Conversation
@timja What do you think of this direction? I had to make some major (breaking) changes for Renovate to detect updates correctly (https://docs.renovatebot.com/modules/manager/helm-values/#additional-information). This would of course be a major version bump. If you would like to include some other breaking changes let me know! |
9f1c56a
to
c1b3fa0
Compare
Signed-off-by: kvanzuijlen <[email protected]>
Signed-off-by: kvanzuijlen <[email protected]>
Signed-off-by: kvanzuijlen <[email protected]>
Signed-off-by: kvanzuijlen <[email protected]>
Signed-off-by: kvanzuijlen <[email protected]>
c1b3fa0
to
390836a
Compare
# Conflicts: # charts/jenkins/unittests/jcasc-config-test.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
general approach looks good, i've left a couple of comments.
.github/renovate.json5
Outdated
matchManagers: ["helm-values"], | ||
bumpVersion: "minor", | ||
postUpgradeTasks: { | ||
"commands": ["helm unittest --strict -f 'unittests/*.yaml' charts/jenkins -u"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this needed? won't CI just run this anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The -u
updates the snapshots, which is needed for kiwigrid/k8s-sidecar and jenkins/inbound-agent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah that's cool, much simpler.
We could also configure this to update plugins too right then?
helm-charts/charts/jenkins/values.yaml
Lines 250 to 253 in 762105a
- kubernetes:4029.v5712230ccb_f8 | |
- workflow-aggregator:596.v8c21c963d92d | |
- git:5.1.0 | |
- configuration-as-code:1670.v564dc8b_982d0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that could be a bit harder... We'd need to use the regex manager for that, so we can't use the bumpVersion functionality. I see 2 solutions for that;
- Use
postUpgradeTasks
for bumping the Chart version as well - Use a 2-step approach, where we'd first create a draft PR using renovate and then use another GitHub Action to do stuff like bumping the Chart version, adding to the changelog, etc.
I need to verify if the postUpgradeTasks approach would work, as I don't know if things like helm (and helm unittest) are available once installed on the runner or if we'd need to build a custom Renovate image (Renovate runs in a container, but could, for example, mount some stuff).
If a custom image is required we could opt for option 2 instead. If a custom image isn't a big deal (it should be quite easy, with Renovate auto-updating it, etc.) that would also be an option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we could maintain a plugins.txt
file and sync it in the renovate workflow using yq
to override what's in the values.yaml file in post upgrade tasks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be an option yeah. That still wouldn't solve the bumpVersion issue though. Also, postUpgradeTasks run within the image, so we'd have to install the tools on the runner and mount the executables/binaries, use a custom image, or use the 2-step solution I proposed.
Personally, I'm more of a fan of mounting the tools since it doesn't involve custom images (maintenance!) and it doesn't require a 2-step approach.
Hi, For kindest/node I'm working on a CLI tool that would automate selecting the most recent n versions. Let me know if that's something you'd be interested in. |
…te-autoupdate # Conflicts: # charts/jenkins/unittests/jcasc-config-test.yaml
# Conflicts: # charts/jenkins/unittests/jcasc-config-test.yaml # charts/jenkins/unittests/jenkins-controller-statefulset-test.yaml
Yes definitely |
@timja I'm planning on testing this by making a copy of my fork (import the fork as a new repository). I'll merge this branch on that repository and make sure I have the necessary (dummy) secrets set. |
@timja as I expected, I needed to make (a ton of) changes to get it to work. I'm getting closer and closer to getting it done. You can take a look at my progress here: https://github.com/kvanzuijlen/jenkins-helm-charts-renovate Once I've verified everything is working I'll update this PR and mark it as ready for review. Of course, I want to deliver something workable/maintainable, so please feel free to request any changes/clarifications where needed when the PR is ready for review. |
@timja still haven't forgotten about this. As far as I can tell the following we'd like to automate the following:
Am I missing anything here? Can you check the PRs I've added as examples to see if there's anything weird/wrong? If everything is as desired I'll be resolving the conflicts/updating the configuration. Right now I've left VALUES_SUMMARY.md out-of-scope, as I'd like to propose to start using https://github.com/norwoodj/helm-docs instead. Similar to how https://github.com/renovatebot/helm-charts does it. |
That all looks great thanks for all this, let me know when it's ready for a review! |
# Conflicts: # .github/workflows/sync-lts.yaml # charts/jenkins/CHANGELOG.md # charts/jenkins/Chart.yaml # charts/jenkins/unittests/jenkins-controller-statefulset-test.yaml # charts/jenkins/values.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timja should I also remove the backup
section in this major version?
platform: "github", | ||
repositories: ["jenkinsci/helm-charts"], | ||
allowedPostUpgradeCommands: ["^\.github\/renovate-postupgrade\.sh {{{depName}}} {{{newVersion}}}$"], | ||
prConcurrentLimit: 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default is 10, 0 is unlimited. Hourly limit is 2 by default.
.github/workflows/sync-lts.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced by Renovate
@@ -12,11 +12,39 @@ Use the following links to reference issues, PRs, and commits prior to v2.6.0. | |||
The changelog until v1.5.7 was auto-generated based on git commits. | |||
Those entries include a reference to the git commit to be able to get more details. | |||
|
|||
## 5.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to include the breaking changes to adminSecrets at the time of writing this comment.
@timja PR should be ready to review now. Please let me know if anything is unclear/wrong. I'm happy to make improvements where possible/add some explanation if needed. |
yes please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really great, thanks for all the amazing work here, a few nits / questions + there's the backup change for 5.0.0 as well but we'll be able to get this in very soon!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
TODO:
What does this PR do?
This PR adds the "self-hosted" Renovate Github Action.
If you modified files in the
./charts/jenkins/
directory, please also include the following:Submitter checklist
Special notes for your reviewer
I contributed to Renovate to support bumping the chart version with updates to a Helm values file. We have to wait until that gets merged. I also added support for upgrading the versions for tools that are installed within github-actions. If all of this works the only dependency changes that have to be made manually is the Kind version matrix. I might also be able to take a look at those as well in a future PR.