-
Notifications
You must be signed in to change notification settings - Fork 175
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
Automate updates of existing helm charts #983
Automate updates of existing helm charts #983
Conversation
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.
Sorry @adamkpickering, I have no idea what this is about, could you provide a description?
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.
We should also remove the .github/workflows/sync-fork.yml
file given that it will not be applicable anymore.
.github/workflows/release.yaml
Outdated
# checkout action only fetches main-source, so we need to fetch main branch also | ||
git fetch origin main --depth 1 | ||
git checkout main | ||
git checkout main-source -- index.yaml assets |
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 original action also removes the assets
folder and the index.yaml
file before executing the checkout. Have you tested that it works?
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.
I think I should have left that in. I'll add it back, if it fits with creating an auto-merged PR that is.
.github/workflows/release.yaml
Outdated
run: | | ||
scripts/pull-ci-scripts | ||
bin/partner-charts-ci auto | ||
git push origin main-source |
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.
Instead of pushing directly to the branches, wouldn't it be better to create PRs for each individual change, with auto-merge enabled?
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.
What do you mean by "each individual change"? Are you suggesting that this automation should create a PR that is merged automatically every time it runs? If so, I can't think of a benefit to doing this - can you please elaborate?
If this isn't what you meant, can you clarify?
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.
By creating a PR (even with auto merge enabled), it forces an execution of the CI scripts to validate the change. Currently, this executes partner-charts-ci validate
(see .github/workflows/pull-request.yml). If something went wrong, we would have the ability to stop the change from getting merged.
.github/workflows/release.yaml
Outdated
git checkout main-source -- assets index.yaml | ||
git commit -m "Update partner charts" | ||
git push origin | ||
TITLE="Update partner charts" |
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.
For automated pull requests we usually use [AUTOMATED] Title
.
c98acc0
to
cf786cc
Compare
f86f826
to
33fdb42
Compare
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.
LGTM!
|
||
- name: Update main-source branch | ||
env: | ||
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
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.
@adamkpickering 🤔 Which one?
Without this option, xargs will run the `gh pr close` command with no arguments when `gh pr list` outputs no PRs. Adding this option prevents the error that can result.
git diff --quiet main-source "$BRANCH" && exit 0 | ||
|
||
# close all existing PRs from branches starting with "auto-update" | ||
gh pr --repo "$GITHUB_REPOSITORY" list --search 'head:auto-update' --json 'headRefName' --jq '.[] | join("\n")' | \ |
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.
One possible issue is that you should always make sure that this Ubuntu latest image has JQ installed; otherwise, it will break.
My Ubuntu did not come with it installed.
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.
I don't think this command requires jq
- it's just an option. Looking at the dependencies of the gh
package, only git
is listed. So they must build jq
, or its syntax, into gh
somehow?
rancher/partner-charts
adds a Rancher-specific layer to a variety of upstream charts. These charts are updated frequently, and the Rancher versions of these charts don't get updated unless we go through a manual process to do so. This process has most recently been done by @marcosbc. We want to automate this process so that updates happen more frequently (i.e. daily) and don't sap our attention.This PR introduces two new workflows:
bin/partner-charts-ci auto
daily in order to update themain-source
branchassets/
andindex.yaml
from themain-source
to themain
branch when changes are pushed to themain-source
branchThere are a lot of things with this repo that I'd like to change. However, this PR is just about automating what we have. We can make further improvements later.