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

Option to write generated prow config, and automatically create pull request against jetstack/testing #89

Merged
merged 5 commits into from
Aug 17, 2022

Conversation

JoshVanL
Copy link
Contributor

@JoshVanL JoshVanL commented Aug 11, 2022

This PR adds a new flag --output to generate-prow. The default stdout value prints to stdout as it does currently. file will write the prow config to a directory and file in form <branch>/cert-manager-<branch>.yaml to mimic the existing path structure.

The branches flag now also accepts a value of * which will loop over all existing known branches.


A github workflow is added which (hopefully) runs the generate command when a cert-manager/release PR is merged. If the output is different to what is currently in jetstack/testing, then a PR will be opened with the changes.

This new workflow uses peter-evans/create-pull-request@v4. I don't know how we using a third party action like this.


An informational verify verify / show-prow-config-changes workflow is added which will show the diff to the jetstack/testing repo.

@jetstack-bot jetstack-bot added the dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. label Aug 11, 2022
@jetstack-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoshVanL

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jetstack-bot jetstack-bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 11, 2022
@inteon
Copy link
Member

inteon commented Aug 12, 2022

Can you confirm that we want to update the prow definitions for all branches?
There is an argument to be made to leave the "release-1.8" and "release-1.9" branches unchanged.

@JoshVanL
Copy link
Contributor Author

Can you confirm that we want to update the prow definitions for all branches? There is an argument to be made to leave the "release-1.8" and "release-1.9" branches unchanged.

Yep, I hear you on not changing those tests. Ideally we should be codifying that these tests don't change using the release program (which is what this PR achieves). If we do want to update the tests in future (for example if we want to delete them and add a new branch), this PR will do that too.

@JoshVanL
Copy link
Contributor Author

Added a workflow to show what the changes are going to be made on the jetstack/testing repo

https://github.com/cert-manager/release/runs/7802831243?check_suite_focus=true

files.

Signed-off-by: joshvanl <me@joshvanl.dev>
jetstack/testing when the cert-manager prow config changes

Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: joshvanl <me@joshvanl.dev>
Signed-off-by: joshvanl <me@joshvanl.dev>
@JoshVanL
Copy link
Contributor Author

JoshVanL commented Aug 17, 2022

Please see https://github.com/cert-manager/release/runs/7875736927?check_suite_focus=true which shows that only master branch prow jobs have updated images.

@inteon
Copy link
Member

inteon commented Aug 17, 2022

@JoshVanL thank you for fixing this.
However, as shown in the diff, the current CommonTestImage value in this repo seems outdated.
Maybe we should merge while keeping this outdated value, so we can test if a PR gets created? (& not merge the created PR, since it is outdated)

@JoshVanL
Copy link
Contributor Author

@JoshVanL thank you for fixing this.
However, as shown in the diff, the current CommonTestImage value in this repo seems outdated.
Maybe we should merge while keeping this outdated value, so we can test if a PR gets created? (& not merge the created PR, since it is outdated)

Yes, the value of the image itself is not to do with this PR. Will follow up with a separate PR that fixes that.

@inteon
Copy link
Member

inteon commented Aug 17, 2022

/lgtm

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 17, 2022
@jetstack-bot jetstack-bot merged commit 4c17d76 into cert-manager:master Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants