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

Automate cherry-pick batching process #23347

Closed
david-mcmahon opened this issue Mar 22, 2016 · 12 comments · Fixed by kubernetes/release#9
Closed

Automate cherry-pick batching process #23347

david-mcmahon opened this issue Mar 22, 2016 · 12 comments · Fixed by kubernetes/release#9
Labels
area/release-eng Issues or PRs related to the Release Engineering subproject lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience.

Comments

@david-mcmahon
Copy link
Contributor

Since 1.2 we have moved away from individual cherrypicks and instead batch them up. This should be turned into a button/lever/whatever. Some kind of automated (I don't care about the details) mechanism that any branch 'owner' can make use of easily.

cc @bgrant0607 @eparis

@david-mcmahon
Copy link
Contributor Author

An additional thought/idea on how to manage patches in batches. #23345 (comment)

@bgrant0607
Copy link
Member

I think the existing script can handle multiple PRs, but there are some technical issues with how it cherrypicks.

@bgrant0607 bgrant0607 added the area/release-eng Issues or PRs related to the Release Engineering subproject label Mar 23, 2016
@eparis
Copy link
Contributor

eparis commented Mar 23, 2016

The script works ok. It gets the .patch files from github and applies them. This means that the actual merge commit on 'master' is not reflected in the 'release' branch.

The shoddy tools I hacked together around cherry-picking looked for the merge commit on the 'release' branch because git cherry-pick -m 1 was a lot easier for me. The shoddy tools 'could' be updated to look for 'every' commit instead of just for the 'merge' commit. (they are all checked into contrib)

I'm not sure if I should comment on @roberthbailey / @zmerlynn thoughts here or in #23345, but I'm lazy so I'll do it here. Even during the 'slush' I probably had 25% of PRs which would not apply. EVERY case was a conflict in some generated file or a conflict involving docs. While the batch picker can certainly be smart enough to handle those cases, I think they should be solved elsewhere. I believe that automated tooling with have << 75% success rate with our current setup.

Let me pontificate a moment on what the RHEL kernel team does. We have similar issues and needs. The team constantly 'backports' patches that landed in the 'devel' branch to some 'older' branch. Our internal process is that developers need only commit to the 'devel' branch and verify that the appropriate hoops have been jumped through to indicate the patch should also be applied to the 'older version'. In actuality those hoops aren't the developers problem, but that's not important. We then have a team of people who try to do the backport. If the backport is not easy and obvious or if the result fails testing they pretty quickly give up. They inform the original developer that THEY must do the backport themselves.

This has the result that most of the time key people are able to completely ignore the backport process. But if things are hard they must do it themselves.

So while I'm being selfish and not watch to do the batch updates myself, I think having a junior person attempt to do batch updates will be beneficial for the project. I think that if the backport is difficult or beyond the skills of the backporter only then should the original developer be required to do it themselves.

I think 1.1 had some good things (the people who knew the patch got it working both places when needed) and 1.2 has some good things (most of you were able to just keep working and not worry about it). But, I'd suggest a hybrid approach...

@david-mcmahon
Copy link
Contributor Author

The process I described in #23345 (comment) I implemented here at Google for our kernel team many years ago and it was a huge success. Developer/contributors were very happy with the notification mechanism and the workflow that allowed them to target changes to multiple branches and have the resolver deal with figuring out when order or refactoring was an issue and only asked them to intervene when necessary. I don't have specific stats, but it was certainly less than 20% changes targeted notified a developer to resolve by order or refactor.
Once that tooling and process was in place there was no other human intervention needed except for the occasional contributor to resolve their own change to a particular branch using a completely guided workflow. Everything was automated down to the point where a contributor actually had to edit a file. It scaled nicely and supported 3-4 active branches and 20 or so developers.
I'd like to see something like that for our process at some point.

@zmerlynn
Copy link
Member

Why do you think batching is a reasonable process after the release has shipped? I think it made sense prior to 1.2.0 but makes less sense now and as the branch ages and conflicts are inevitable. Are you going to push conflict resolution onto the batcher? That seems wrong.

@zmerlynn zmerlynn reopened this Mar 23, 2016
@zmerlynn
Copy link
Member

Ah, I see this thread is split all over the place. Maybe we need a face to face.

@david-mcmahon david-mcmahon added sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience. area/ecosystem and removed area/release-eng Issues or PRs related to the Release Engineering subproject labels Apr 18, 2016
@david-mcmahon david-mcmahon added this to the v1.3 milestone Apr 18, 2016
@david-mcmahon david-mcmahon added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Apr 18, 2016
@mml mml added the area/release-eng Issues or PRs related to the Release Engineering subproject label May 2, 2016
@roberthbailey
Copy link
Contributor

@david-mcmahon The batched cherry picks were great during the time between the 1.2 branch cut and the 1.2 release. I expect we will do the same for the upcoming 1.3 release. Do we want/need to implement automation in the next 2 weeks to make it easier?

@david-mcmahon
Copy link
Contributor Author

@roberthbailey @eparis if we can capture the basic skeleton procedures here, I can product-ize a script that could be run more easily by more people and get @eparis out of the critical path for this.

@erictune erictune removed this from the v1.3 milestone Jun 15, 2016
@jdumars
Copy link
Member

jdumars commented Oct 12, 2017

Is there still work planned here, or should this be closed?

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 10, 2018
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Feb 10, 2018
@nikhita
Copy link
Member

nikhita commented Mar 3, 2018

Is there still work planned here, or should this be closed?

bump

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/release-eng Issues or PRs related to the Release Engineering subproject lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience.
Projects
None yet
Development

Successfully merging a pull request may close this issue.