clarify some parts of the cherry pick process #1980

tpepper · 2018-03-28T20:09:57Z

The cherry-pick instruction guide had some readability issues in
its formatting, with numbered bullet lists not rendering correctly.
It also referenced an issue which was resolved, so I've added links
to the resulting tools and information. Finally, based on watching
a new user attempt a cherry pick (a common occurance around release
time with the revolving staffing of the release team) a few
clarification and setup issues are described around the
hack/cherry_pick_pull.sh script.

Signed-off-by: Tim Pepper tpepper@vmware.com

tpepper · 2018-03-28T20:14:09Z

PTAL
/assign @dims

dims · 2018-03-28T21:01:44Z

contributors/devel/cherry-picks.md

+#### Individual Cherrypicks
+ * Run the [cherry pick script](https://git.k8s.io/kubernetes/hack/cherry_pick_pull.sh). This example applies a master branch PR #98765 to the remote branch `upstream/release-3.14`: `hack/cherry_pick_pull.sh upstream/release-3.14 98765`
+ * NOTE:
+    * For this script to work you need the normal git and GitHub configured shell environment for pushing to your kubernetes `origin` fork on GitHub and making a pull request against a configured remote `upstream` that tracks "http://git.k8s.io/kubernetes".


is http://git.k8s.io/kubernetes legit?

@cblecker is this the preferred way to reference it versus https://github.com/kubernetes/kubernetes/ ?

when making a markdown link, using the shortener would be preferred. However, when you're talking about git configuration, you're either tracking https://github.com/kubernetes/kubernetes.git for HTTPS or git@github.com:kubernetes/kubernetes.git for SSH.

Thanks! Amended.

cblecker · 2018-03-29T19:04:19Z

/cc @cblecker @spiffxp

spiffxp · 2018-04-02T22:32:45Z

taking a look, untangling how cherry picks actually work is hurting my head

I'm also unsure whether we should have this instead document the prow-based cherrypicker that's being worked on

cjwagner · 2018-04-02T23:40:25Z

Please let me know of any pain points or feature requests related to the mungegithub cherrypick mungers. They are due for a revamp and migration to Prow.

spiffxp · 2018-04-02T21:28:47Z

contributors/devel/cherry-picks.md

@@ -7,39 +7,42 @@ depending on the point in the release cycle.
 ## Propose a Cherry Pick

 1. Cherrypicks are [managed with labels and milestones](/contributors/guide/release-notes.md)


not that you changed this, but the linked content doesn't really talk about labels nor milestones

Oof indeed. I should've caught that.

spiffxp · 2018-04-02T22:22:36Z

contributors/devel/cherry-picks.md

+ * PRs batched up and merged to the release branch get a `cherrypick-approved` label and lose the `cherrypick-candidate` label.
+ * PRs that won't be merged to the release branch, lose the `cherrypick-candidate` label.
+
+There is a helper tool [`branchff`](https://git.k8s.io/release/branchff) and


I would remove the mention of branchff, it's unrelated to cherry picking. Its used to keep the release-1.x branch in sync with master while code freeze is in effect. Not exactly a fast-forward merge, but conceptually similar in terms of the commits it picks up.

Do folks think it would make sense to merge this cherry-pick.md and the https://github.com/kubernetes/release/blob/master/docs/branching.md ?

Maybe if you want to rescope into "the lifecycle of a release branch and how commits get into it", which also sorta overlaps with https://github.com/kubernetes/sig-release/blob/master/release-process-documentation/release-team-guides/patch-release-manager-playbook.md and https://github.com/kubernetes/sig-release#during-a-patch-release and help I can't stop why do we have so many docs that also say different versions and facets of the same thing

for now I'd rather see this PR land to fixup this one doc to be the authoritative source for this one mechanical part of the release process

spiffxp · 2018-04-02T23:01:07Z

contributors/devel/cherry-picks.md

-  * See the [PR template](https://git.k8s.io/kubernetes/.github/PULL_REQUEST_TEMPLATE.md) for more details.
-  * PR titles and body comments are mutable and can be modified at any time
-    prior to the release to reflect a release note friendly message.
+


This is more of a note to self as I work through this:

submit-queue is actually the thing responsible for merging PR's to release-1.x

submit-queue won't bother if there is a do-not-merge/cherry-pick-unapproved label

the label-pick-unapproved munger run as part of the misc-mungers deployment applies this label

it removes the label if there is a cherrypick-approved label

it applies the label if a PR is not for master

the cherrypick-approved label can either be applied by a human, or by the cherrypick-auto-approve munger

the cherrypick-auto-approve munger

only looks at PR's not for master

skips PR's that have cherrypick-approved and a milestone

skips PR's that don't mention other PR's (hereafter called "parents") in their description

skips PR's that are for release-1.x but have milestone v1.y

skips PR's whose parents don't have the cherrypick-approved label

skips PR's that are for release-1.x whose parents have milestone v1.y or none

applies cherrypick-approved and milestone 1.x if it's made it this far

various mungers will remove the cherrypick-candidate label

from "parent" PR's that have successfully merged

from "parent" PR's that lack a milestone

etc.

Some thoughts:

the current auto-approve process assumes one PR to master == one cherrypick, and discounts that some fixes may need to be backported to multiple release branches

I had to double-check, but it sure looks like cherrypick-candidate is just used for display purposes, I'm not sure if there is automation that consumes the cherrypick queue

Given that there is no bot-driven way to apply the cherrypick-candidate label, it's more likely a PR author without repo write access would just run the cherypick script themselves

I dislike this whole process and suggest that it's way easier to just get a human to apply the cherrypick-approved label

Re: multi-branch pick tracking/process, see suggestions in https://groups.google.com/forum/m/#!topic/kubernetes-wg-contribex/XxYn02YQcV0

thanks for reminding me of that thread, replied over there

spiffxp · 2018-04-02T23:45:31Z

contributors/devel/cherry-picks.md

+[branching](https://git.k8s.io/release/docs/branching.md) documentation.
+
+#### Individual Cherrypicks
+ * Run the [cherry pick script](https://git.k8s.io/kubernetes/hack/cherry_pick_pull.sh). This example applies a master branch PR #98765 to the remote branch `upstream/release-3.14`: `hack/cherry_pick_pull.sh upstream/release-3.14 98765`


I would move the description of this script way, way up. AFAICT it's the sole answer for how cherrypick-candidates turn into cherrypick PR's with the current setup. A branch manager looks at the cherry pick queue, and calls this with N PR's for "batch" mode, or PR authors can do this on their own.

spiffxp · 2018-04-02T23:47:28Z

contributors/devel/cherry-picks.md

+
+#### Individual Cherrypicks
+ * Run the [cherry pick script](https://git.k8s.io/kubernetes/hack/cherry_pick_pull.sh). This example applies a master branch PR #98765 to the remote branch `upstream/release-3.14`: `hack/cherry_pick_pull.sh upstream/release-3.14 98765`
+ * NOTE:


I think the script also needs the GITHUB_USER environment variable to be set to wherever your fork of kubernetes lives

tpepper

Thanks for all the additional info @spiffxp

tpepper · 2018-04-03T00:54:29Z

contributors/devel/cherry-picks.md

@@ -7,39 +7,42 @@ depending on the point in the release cycle.
 ## Propose a Cherry Pick

 1. Cherrypicks are [managed with labels and milestones](/contributors/guide/release-notes.md)


Oof indeed. I should've caught that.

tpepper · 2018-04-03T00:56:04Z

contributors/devel/cherry-picks.md

+ * PRs batched up and merged to the release branch get a `cherrypick-approved` label and lose the `cherrypick-candidate` label.
+ * PRs that won't be merged to the release branch, lose the `cherrypick-candidate` label.
+
+There is a helper tool [`branchff`](https://git.k8s.io/release/branchff) and


Do folks think it would make sense to merge this cherry-pick.md and the https://github.com/kubernetes/release/blob/master/docs/branching.md ?

spiffxp · 2018-04-03T17:48:51Z

This may address #1894

tpepper · 2018-04-11T22:02:22Z

@spiffxp I've chopped most of the doc now. I'm not marking this as fixing #1894. I feel like the full fix is moving to prow and if there are any then also changes to the "cherrypick-*" label management and syncing docs to that.

cblecker · 2018-04-19T19:34:07Z

contributors/devel/cherry-picks.md

+   `upstream/release-3.14`: `hack/cherry_pick_pull.sh upstream/release-3.14
+   98765`
+ * Your cherrypick PR (targeted to the branch) will immediately get
+   the `do-not-merge/cherry-pick-not-approved` label. The branch owner


The branch owner will approve the cherry pick, but they won't LGTM it. Standard lgtm/approval rules still apply, even for cherry picks.

I guess I'm happy the way I updated this doc made the few pieces of original text look like new text too. I hadn't changed this line's contents but it is nevertheless unclear. I've tweaked the text as suggested. Thanks!

cblecker · 2018-04-19T21:07:29Z

contributors/devel/cherry-picks.md

+   `do-not-merge/cherry-pick-not-approved` label. The release branch owner
+   will triage PRs targeted to the branch.  Normal rules apply for code
+   reviewers applying the `/lgtm` label as appropriate, with the branch
+   owner applying the `/approve` label.


Sorry, I should have been clearer.. the branch manager will apply "cherrypick-approved", but OWNERS will still need to "/approve" the PR.

So that makes sense, but...is that what's actually implemented? I'm not seeing that in prow.

The cherrypick-approved label doesn't have an associated prow command. It's added manually by the branch manager.

Example PR that shows the process: kubernetes/kubernetes#62847

Got it! Amended.

cblecker · 2018-04-20T17:05:43Z

contributors/devel/cherry-picks.md

+   will triage PRs targeted to the branch.  Normal rules apply for code merge.
+   Reviewers apply the `/lgtm` label and owners `/approve` as they deem
+   appropriate. The approving release branch owner is responsible for applying
+   the `/cherrypick-approved` label.


I'm gonna get real nit picky here.. when you're talking about commands, the command is /lgtm and the label it applies is label. So if you're talking about the context of the command someone will use, it's /lgtm and /approve, but if you're talking about the labels they apply, it's lgtm, approve, and cherrypick-approved. cherrypick-approved should never have a slash, as there is no slash command.

Not at all not picking. This is important to have clear and consistent, especially here since parts can be done with commands and the cherrypick-approve is not. I inadvertently left the / in the cherrypick-approved and definitely will remove. On the other two: since things are drifting toward bot commands more and more as the normal workflow path would you bias to saying to label with the (non-slash-preceded) label names? Or to giving the command name to trigger the labeling?

It all depends on the context and how you write it. When you say "Reviewers apply the /lgtm label", this is incorrect because the label is "lgtm". You can either say "Reviewers apply the lgtm label" or "Reviewers use the /lgtm command". I don't really have a preference as long as we are consistent.

Amended to simplified language on the commands and removed the errant slash on the non-comman cherrypick-approved label.

The cherry-pick instruction guide was quite out of date. This commit dramatically simplifies the doc. Also, based on watching a new user attempt a cherry pick (a common occurance around release time with the revolving staffing of the release team), a few clarification and setup issues are addressed around the hack/cherry_pick_pull.sh script pre-requisites. This is an interim fix as the doc will need updated again to capture the pending prow based method when that becomes preferred and the file should be merged with the various other branch and release maintenance documents that are out there. Signed-off-by: Tim Pepper <tpepper@vmware.com>

cblecker

/lgtm
/approve

k8s-ci-robot · 2018-04-20T19:51:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cblecker

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:

~~contributors/devel/OWNERS~~ [cblecker]

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 added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 28, 2018

k8s-ci-robot requested review from grodrigues3 and Phillels March 28, 2018 20:10

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 28, 2018

k8s-ci-robot assigned dims Mar 28, 2018

dims reviewed Mar 28, 2018

View reviewed changes

k8s-ci-robot requested review from cblecker and spiffxp March 29, 2018 19:04

tpepper force-pushed the master branch from 32cb6bc to 809e417 Compare March 29, 2018 19:19

spiffxp mentioned this pull request Apr 2, 2018

Add cherrypicker to prow deployment bundle kubernetes/test-infra#7393

Closed

spiffxp reviewed Apr 2, 2018

View reviewed changes

tpepper commented Apr 3, 2018

View reviewed changes

This was referenced Apr 3, 2018

Improve cherry-pick process to better track them kubernetes/test-infra#1795

Closed

Automated cherry pick of #61942: fix mirror-pod hash race condition kubernetes/kubernetes#61954

Closed

tpepper force-pushed the master branch from 809e417 to 144c6ba Compare April 11, 2018 21:56

cblecker requested changes Apr 19, 2018

View reviewed changes

tpepper force-pushed the master branch from 144c6ba to db1bebe Compare April 19, 2018 20:58

cblecker reviewed Apr 19, 2018

View reviewed changes

tpepper force-pushed the master branch 2 times, most recently from 16dc02c to 15c2679 Compare April 20, 2018 14:59

cblecker reviewed Apr 20, 2018

View reviewed changes

tpepper force-pushed the master branch from 15c2679 to 17f2c35 Compare April 20, 2018 19:48

cblecker approved these changes Apr 20, 2018

View reviewed changes

k8s-ci-robot assigned cblecker Apr 20, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 20, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 20, 2018

k8s-ci-robot merged commit afc5171 into kubernetes:master Apr 20, 2018

spiffxp mentioned this pull request Apr 26, 2018

cherry-picks.md in /contributors/devel is a duplicate #1760

Closed

nikhita mentioned this pull request Nov 22, 2018

Other unanswered questions in cherry-picks.md #1894

Closed

nikhita mentioned this pull request Dec 3, 2018

How to cherry pick a fix to multiple release branches? #994

Closed

		@@ -7,39 +7,42 @@ depending on the point in the release cycle.
		## Propose a Cherry Pick

		1. Cherrypicks are [managed with labels and milestones](/contributors/guide/release-notes.md)

clarify some parts of the cherry pick process #1980

clarify some parts of the cherry pick process #1980

Conversation

tpepper commented Mar 28, 2018

tpepper commented Mar 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cblecker commented Mar 29, 2018

spiffxp commented Apr 2, 2018

cjwagner commented Apr 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tpepper left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spiffxp commented Apr 3, 2018

tpepper commented Apr 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cblecker Apr 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cblecker left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 20, 2018

cblecker Apr 19, 2018 •

edited

Loading