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.

Sign up for GitHub

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

Jump to bottom

staging/publishing: move rules here from publishing bot repo #73023

Merged
merged 2 commits into from Jan 23, 2019
Merged

staging/publishing: move rules here from publishing bot repo #73023

merged 2 commits into from Jan 23, 2019

Conversation

sttts
Copy link
Contributor

@sttts sttts commented Jan 17, 2019

In order to make publishing more transparent for the community (no manual publishing bot redeploy needed anymore) and to split the Kubernetes config from the machinery that does the publishing, we move the rules here.

@k8s-ci-robot
Copy link
Contributor

@sttts: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jan 17, 2019
@sttts sttts added release-note-none Denotes a PR that doesn't merit a release note. sig/release Categorizes an issue or PR as relevant to SIG Release. labels Jan 17, 2019
@k8s-ci-robot k8s-ci-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Jan 17, 2019
@sttts sttts added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jan 17, 2019
@k8s-ci-robot k8s-ci-robot removed the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Jan 17, 2019
@sttts sttts added the area/release-eng Issues or PRs related to the Release Engineering subproject label Jan 17, 2019
@sttts
Copy link
Contributor Author

sttts commented Jan 17, 2019

/assign @nikhita @dims

@sttts sttts mentioned this pull request Jan 17, 2019
Merged
@dims
Copy link
Member

dims commented Jan 17, 2019

@sttts since publishing is a sig-release sub project, should we use https://github.com/kubernetes/sig-release ? Another option is https://github.com/kubernetes/k8s.io where we started adding DNS related stuff and other configuration information.

@sttts
Copy link
Contributor Author

sttts commented Jan 17, 2019

@dims the idea was to keep that near to the staging/ repos in k/k. If we separate rules and staging, we can leave them in publishing-bot IMO.

@nikhita
Copy link
Member

nikhita commented Jan 17, 2019

/kind feature

Just want to explicitly say that THIS IS AWESOME!

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Jan 17, 2019
nikhita
nikhita reviewed Jan 17, 2019
View reviewed changes
staging/publishing/rules.yaml Outdated
dir: staging/src/k8s.io/code-generator
name: master
# - source:
# branch: release-1.8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sttts can we remove these or do you want to keep these commented? IMO we can add these back if we ever need to publish them again (super super critical security fix). Because right now this yaml file is huge. :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely non-critical, but maybe remove them in a separate commit (even in the same PR) so we have them in the git history of k/k?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hoegaarden good idea.

Btw, judging from the last apimachinery CVE, we don't backport that far anyway. @liggitt what's the rule?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also note, we have: skip-source-branches: to skip old branches. That's easier to use than commenting tons of yaml lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove everything up to 1.9.

@dims
Copy link
Member

dims commented Jan 17, 2019

LGTM will let sig release folks bless this 👍 ( cc @tpepper @justaugustus )

@kubernetes/sig-release

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 17, 2019
@nikhita
Copy link
Member

nikhita commented Jan 17, 2019 via email

@nikhita
Copy link
Member

nikhita commented Jan 17, 2019

/retest

❄️

@sttts
Copy link
Contributor Author

sttts commented Jan 18, 2019 •
edited

While we want to make the presubmits better, I would suggest considering the options @dims mentioned if only to make PRs to this easier while still separating the config.

The deployment config is separate anyway.

This is about the semantics of the staging repos. Now we have import restrictions in k/k and dependency information in k/publishing-bot. Both are directly related and it's redundant information which diverges (and probably already has), and then breaks the bot which must be manually fixed. We would like to keep this just in k/k and verify it before any merge. This ensures that staging repos are consistent and reduces the risk that publishing is broken.

@sttts
Copy link
Contributor Author

sttts commented Jan 23, 2019

@tpepper @justaugustus can we make progress here?

@sttts
Copy link
Contributor Author

sttts commented Jan 23, 2019

#73071 was yet another example of a PR that should have never merged. A publishing smoke test would have uncovered that.

@dims
Copy link
Member

dims commented Jan 23, 2019

/approve
/lgtm

/assign @smarterclayton @lavalamp

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 23, 2019
nikhita
nikhita reviewed Jan 23, 2019
View reviewed changes
staging/publishing/OWNERS Outdated
- nikhita
labels:
- sig/release
- area/release-infra
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one more small change is needed - this label is now called area/release-eng. 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 23, 2019
@dims
Copy link
Member

dims commented Jan 23, 2019

/lgtm

Thanks @sttts

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 23, 2019
@nikhita
Copy link
Member

nikhita commented Jan 23, 2019 •
edited

Re last commit about skipping publishing of 1.10 branch...I think we are going to cut a patch release for 1.10 soon.

@dims @liggitt @BenTheElder @spiffxp can any of you confirm?

@smarterclayton
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, smarterclayton, sttts

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 Jan 23, 2019
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 23, 2019
@sttts
Copy link
Contributor Author

sttts commented Jan 23, 2019 •
edited

@nikhita Moved the last commit to #73223. We can wait with that.

@dims
Copy link
Member

dims commented Jan 23, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 23, 2019
@sttts
Copy link
Contributor Author

sttts commented Jan 23, 2019

/retest

@nikhita nikhita mentioned this pull request Jan 23, 2019
Closed
@k8s-ci-robot k8s-ci-robot merged commit 9f4a495 into kubernetes:master Jan 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoegaarden hoegaarden hoegaarden left review comments

@nikhita nikhita nikhita left review comments

@liggitt liggitt Awaiting requested review from liggitt

@mikedanese mikedanese Awaiting requested review from mikedanese

Assignees

@dims dims

@lavalamp lavalamp

@smarterclayton smarterclayton

@nikhita nikhita

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/release-eng Issues or PRs related to the Release Engineering subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/release Categorizes an issue or PR as relevant to SIG Release. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@sttts @k8s-ci-robot @dims @nikhita @BenTheElder @smarterclayton @hoegaarden @lavalamp