Test-infra: turn down misc-mungers, cherrypick-queue instances of mungegithub

Created on 11 Sep 2018  路  5Comments  路  Source: kubernetes/test-infra

Pulled from https://github.com/kubernetes/test-infra/issues/3866#issuecomment-420284883

These should have been turned down as part of turning down the submit-queue instance, but in the interest of being careful I went and looked at how the remaining mungers are being used in the face of cherrypicks:

  • kubernetes-misc-mungers is running with mungers: stale-green-ci,comment-deleter
  • kubernetes-cherrypick is running with mungers: cherrypick-must-have-milestone,cherrypick-clear-after-merge,cherrypick-queue (cherrypick.k8s.io is up)

I reviewed the last ~30d worth of cherry picks to release-1.11, release-1.10, release-1.9: https://gist.github.com/spiffxp/dfe52c0881645052ff037e317d9ad18b

Observations:

  • biggest difference: tide doesn't require a milestone to merge cherry picks; mungegithub did, but it seemingly never commented that fact - we can drop cherrypick-must-have-milestone and/or re-implement it as a prow plugin if necessary (the release team likes milestones)
  • almost nobody has used the cherrypick-candidate label on source PRs in the last 30d, the two occurrences I found didn't have the label removed after merge, so cherrypick-clear-after-merge is meaningless
  • cherrypick-queue basically doesn't track PR's unless they have the cherrypick-candidate label applied, so cherrypick-queue can be dropped

/assign @foxish @MaciekPytel @mbohlool
do my observations above agree with your experience?

The stale-green-ci munger is obviated by tide, and comment-deleter is only there to support any messages the cherrypick-queue instances might post.

Most helpful comment

@MaciekPytel summarized what I think too. I just look for PRs against 1.9 branch and use cherry-pick-approved to approve them. Also I am not sure do-not-merge/cherry-pick-not-approved is useful. Automation should check for one of these not both:

  • Merge if cherry-pick-approval label exists
    OR
  • do not merge until do-not-merge/cherry-pick-not-approved

This will be of course in addition to any other checks required such as lgtm and approval.

All 5 comments

I don't find cherrypick-candidate useful for anything, never ask people to apply it and never apply it myself. I find cherry-picks by looking for PR to release-1.10 branch, not based on the label. I haven't noticed any difference in how automation handles PRs with and without cherrypick-candidate.

As to milestone: it's only needed to merge during master code freeze (they also need priority/critical-urgent during freeze on master). Personally I don't find it very useful for patch releases, because I already have different tools providing all I need:

  • I don't use milestone to keep track of pending cherry-picks, looking for PRs open against 1.10 branch is more reliable.
  • I don't need milestone as a way to approve PR, I already have cherry-pick-approved label for that.

To sum up: I don't think I'll miss any of those special mungers.

@MaciekPytel summarized what I think too. I just look for PRs against 1.9 branch and use cherry-pick-approved to approve them. Also I am not sure do-not-merge/cherry-pick-not-approved is useful. Automation should check for one of these not both:

  • Merge if cherry-pick-approval label exists
    OR
  • do not merge until do-not-merge/cherry-pick-not-approved

This will be of course in addition to any other checks required such as lgtm and approval.

/assign @cjwagner
/close
We've turned the instances down, ref: https://github.com/kubernetes/test-infra/pull/9362

@spiffxp: Closing this issue.

In response to this:

/assign @cjwagner
/close
We've turned the instances down, ref: https://github.com/kubernetes/test-infra/pull/9362

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.

Was this page helpful?
0 / 5 - 0 ratings