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

Add Kops required status checks #17857

Merged
merged 1 commit into from
Jun 11, 2020

Conversation

mikesplain
Copy link
Contributor

Adding kops to branch-protection for github actions, as discussed in kubernetes/kops#9005.

I believe this will also remove the existing travis ci block so we'll need to add that here if we wish for it to continue to block.

Thoughts @rifelpet, @hakman, @johngmyers ?

/hold

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. area/config Issues or PRs related to code in /config labels Jun 5, 2020
@k8s-ci-robot k8s-ci-robot added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label Jun 5, 2020
@mikesplain
Copy link
Contributor Author

/assign @rifelpet

@rifelpet
Copy link
Member

rifelpet commented Jun 7, 2020

two thoughts:

  1. We should consider adding the travis ci check here. Once we make this blocking we'll be able to remove all of the travis jobs except the ARM64 build. I know @hakman made that individual job non-blocking but eventually we'd want to make it blocking as ARM64 support stabilizes. If we add travis here and the only travis job is configured to allow failure, will tide still treat PRs as mergeable if the ARM64 job fails?

  2. It'd be nice if we had more stable names for the github action jobs so that as we upgrade the OS or go versions we don't need to update these references (since they require test-infra approvals) Any idea if github actions support customizing the names from jobs' matrixes to make them more stable? I don't have a good suggestion on what to name them though. Maybe this isn't a big deal since we dont update either OS or go versions very often.

@mikesplain mikesplain force-pushed the kops_required_contexts branch from 287e52f to 566fea3 Compare June 8, 2020 14:52
@mikesplain
Copy link
Contributor Author

two thoughts:

  1. We should consider adding the travis ci check here. Once we make this blocking we'll be able to remove all of the travis jobs except the ARM64 build. I know @hakman made that individual job non-blocking but eventually we'd want to make it blocking as ARM64 support stabilizes. If we add travis here and the only travis job is configured to allow failure, will tide still treat PRs as mergeable if the ARM64 job fails?

Agreed, just added this as well. This should block on travis the way it does now. I believe this will continue to work even when we disable the old jobs, since the travis job should still pass since the allow failure flag is set. I went with continuous-integration/travis-ci/pr though it's possible just continuous-integration/travis-ci would work.

  1. It'd be nice if we had more stable names for the github action jobs so that as we upgrade the OS or go versions we don't need to update these references (since they require test-infra approvals) Any idea if github actions support customizing the names from jobs' matrixes to make them more stable? I don't have a good suggestion on what to name them though. Maybe this isn't a big deal since we dont update either OS or go versions very often.

I agree but I wasn't able to come up with a better way at the moment. There is something nice about being explicit about the block, for instance if we want to start testing on a new OS and start blocking later. How about I add a comment to our actions mentioning that an update is required here in order to start blocking when action names/matrix change?

@rifelpet
Copy link
Member

rifelpet commented Jun 8, 2020

Adding a comment to our actions sounds great 👍
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 8, 2020
@mikesplain
Copy link
Contributor Author

Comment added in PR, will unhold kubernetes/kops#9305 when this is merged and confirmed.

Ready for review!
/assign @chases2

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 8, 2020
@johngmyers
Copy link
Member

/lgtm

@mikesplain
Copy link
Contributor Author

/assign @cblecker

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chases2, mikesplain

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 10, 2020
@k8s-ci-robot k8s-ci-robot merged commit cba5718 into kubernetes:master Jun 11, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.19 milestone Jun 11, 2020
@k8s-ci-robot
Copy link
Contributor

@mikesplain: Updated the config configmap in namespace default at cluster default using the following files:

  • key config.yaml using file config/prow/config.yaml

In response to this:

Adding kops to branch-protection for github actions, as discussed in kubernetes/kops#9005.

I believe this will also remove the existing travis ci block so we'll need to add that here if we wish for it to continue to block.

Thoughts @rifelpet, @hakman, @johngmyers ?

/hold

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@hakman
Copy link
Member

hakman commented Jun 11, 2020

Small problem, I think this applies to all Kops branches, which pretty much blocks 1.6 and 1.17.
kubernetes/kops#9331

@mikesplain
Copy link
Contributor Author

@hakman Good point. I’ll create a follow up. Thanks!

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. area/config Issues or PRs related to code in /config cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants