prow: avoid retriggering presubmits on default remote branch rename

[spiffxp]

What would you like to be added:

I would like prow to be able to detect and avoid retriggering presubmits when PRs have their base branch changed as part of GitHub's tool to rename the default remote branch for a given repo.

The scenario I want to avoid is sort of the opposite of tide's retrigger logic:

• a presubmit is configured to trigger for PRs to 'main' and 'master'
• a PR is opened against 'master' and presubmits run at commit A
• the PR does not get reviewed, so does not meet tide's merge criteria
• 'master' moves to commit B due to other PR's merging
• the repo's default remote branch is renamed from 'master' to 'main'
• even though main is at commit B, and the last results for the PR were from commit A, prow should not retrigger the PR

I am hoping there is enough info in whatever events are fired as part of the rename (e.g. kubernetes/org#2139 (comment))...

...to detect that a PR's base branch has changed, but the underlying SHA of the base branch has not

Why is this needed:

Currently, renaming a repo's default remote branch causes prow to retrigger all presubmits for all open PRs.

The worst case scenario would be kubernetes/kubernetes, ballpark 960 open PRs * 10 presubmits = 9600 prowjobs triggered instantly. This would overwhelm our CI capacity, and incite a thundering herd of /retests.

I feel like the options are:

• make prow (hook? trigger?) detect and ignore this case
• disable triggering, rename branch, enable triggering (unsure if this prevents mass re-trigger on enable)
• throttle/queue the spike in load (doesn't necessarily stop the thundering herd)

/wg naming
/sig contributor-experience
/area github-management
/sig testing
/priority important-longterm
/area prow
/area prow/hook

/assign @cjwagner @alvaroaleman
Assigning Cole since we discussed it, and Alvaro to get input. Please /unassign if you don't want this on your plate.

[spiffxp]

/assign @BenTheElder
if you're planning on migrating kubernetes-sigs/kind at some point, work with test-infra-oncall to identify relevant events / webhook payloads

I'll see if I can dig any up for kubernetes/org or kubernetes/k8s.io

[spiffxp]

Found an event, thanks to https://github-dot-k8s-gubernator.appspot.com/timeline?repo=kubernetes/org&number=2139 (TIL, and suddenly I'm a lot less interested in deleting this app)

Note that as part of this process the master branch gets deleted before the main branch gets created (ref: kubernetes/org#2466 (comment))

2021-01-29 19:38:59.693150 | pull_request a1ae87b0-6269-11eb-9243-9aa612c4a8cf | edited | spiffxp

I trimmed down to what I think are the relevant fields. I'm not sure there is enough solely within this payload to detect that what it changed from is the same as what it changed to. Maybe skip solving the general base branch renaming case and special case on the fact that base.ref == base.repo.default_branch?

{
  "sender": {
    "login": "spiffxp", // makes sense, I'm the one that clicked the button
  }, 
  "repository": {
    "full_name": "kubernetes/org", 
    "pushed_at": "2021-01-29T19:38:57Z", // repo has new branch by this point
    "default_branch": "main"
  }, 
  "number": 2139, 
  "pull_request": {
    "merge_commit_sha": "5d0c8b8562f2f3c3a47a1bf79da5035b07fef390", 
    "number": 2139, 
    "state": "open", 
    "head": {
      "repo": {
        "full_name": "yliaog/org", 
      }, 
      "sha": "30aa59f9cf1b2eeda5d6619f20522a6680e807c6", 
      "ref": "master", 
    }, 
    "commits": 1, 
    "rebaseable": null, 
    "updated_at": "2021-01-29T19:38:58Z", 
    "base": {
      "repo": {
        "full_name": "kubernetes/org", 
        "pushed_at": "2021-01-29T19:38:57Z", 
        "default_branch": "main"
      }, 
      "sha": "e1e8ec86d24aab7998a9804c7e996c6ca99117f7", 
      "ref": "main",
      "label": "kubernetes:main"
    }, 
    "patch_url": "https://github.com/kubernetes/org/pull/2139.patch"
  }, 
  "action": "edited", // this seems relevant
  "organization": {
    "login": "kubernetes", 
  }, 
  "changes": {
    "base": {
      "sha": {
        "from": "8ef36d93bd7f042e24ee084b5ca27876cc8d35bc"
      }, 
      "ref": {
        "from": "master"
      }
    }
  }
}

[BenTheElder]

That endpoint is the best kept secret in test-infra 🙃

[spiffxp]

GitHub asked if it was possible for us to try a maintenance window approach (disabling prow just prior to a branch rename, and then enabling afterward)

This would look like disabling the trigger plugin for a given repo. Currently trigger is defined at the org level, we would need to either:

• define trigger for all repos within an org, and then remove it from the repo we want to rename
• add support for excluding org-defined plugins at the repo

It might involve disabling tide too?

We would ideally want some way to identify which jobs we should have legitimately triggered during maintenance window, and manually trigger them. Or, less ideally, we would assume contributors blocked by these missed jobs would /retest their way out.

[cblecker]

Just some numbers to think about:

• Currently there are 4 repos in @kubernetes that have over 100 open PRs: kubernetes, test-infra, website, enhancements
• Other than k/k, the other three are just over 100
• k/k has 997 at this moment

If we can handle ~100 retest PRs at once, then k/k becomes the only concern. If that's the case, maybe we just turn off the org-wide webhook in the GH config once, flip the switch, then turn it back on. No code changes to trigger needed, possibly. Just food for thought.

[spiffxp]

FWIW GitHub reports they may have figured something out on their end and will update us when they're ready for us to try another rename

[BenTheElder]

If we can handle ~100 retest PRs at once, then k/k becomes the only concern. If that's the case, maybe we just turn off the org-wide webhook in the GH config once, flip the switch, then turn it back on. No code changes to trigger needed, possibly. Just food for thought.

It really depends on what the repo is running. Temporarily disabling trigger for a repo sounds like a reasonable workaround though.

[spiffxp]

I just noticed #20707 landed, which should let us do the temporarily disable dance

[spiffxp]

FYI I just migrated kubernetes-sigs/slack-infra (kubernetes-sigs/slack-infra#50) and in the process verified that GitHub no longer blasts prow with webhooks when a repo's default branch is renamed

I think we can call this closed unless anyone feels like we need to invest further engineering into this?

[spiffxp]

/close
Please /reopen if you feel like there's anything further to discuss here

[k8s-ci-robot]

@spiffxp: Closing this issue.

In response to this:

/close
Please /reopen if you feel like there's anything further to discuss here

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.