statusreconciler: skip draft PRs in triggerNewPresubmits#699
statusreconciler: skip draft PRs in triggerNewPresubmits#699k8s-ci-robot merged 1 commit intokubernetes-sigs:mainfrom
Conversation
GetPullRequests includes draft PRs, so status-reconciler could create newly-required presubmits for draft PRs during config reconciliation. Skip drafts in this path and document that new blocking presubmits are triggered only for trusted non-draft PRs. Signed-off-by: Jathavedhan M <jathavedhan.m@ibm.com>
✅ Deploy Preview for k8s-prow ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Welcome @carterpewpew! |
|
Hi @carterpewpew. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
|
So, with this PR, you are trying to ensure that the PR's presubmit contexts aren't created on a draft PR? |
Thank you @Prucek for taking a look, The example you linked is the trigger plugin correctly skipping CI on a draft PR push that path already handles drafts. This fix is for a different code path: when a new blocking presubmit is added to the config, the status-reconciler's triggerNewPresubmits queries all open PRs via GetPullRequests (which includes drafts) and triggers the new job for them. That reconciliation path didn't have a draft check before this. |
|
/test all |
|
@Prucek there's #190 filed for the problem. presubmits do not trigger on draft PRs, but when status-reconciler triggers new jobs it does so even on draft PRs which is inconsistent and wasteful. I'll @carterpewpew can you add /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: carterpewpew, petr-muller The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Done, thanks again |
GetPullRequests returns all open PRs, including drafts. In status-reconciler, triggerNewPresubmits iterated all open PRs and called RunRequested directly, so trusted draft PRs could receive newly-added blocking presubmits during config reconciliation.
The trigger plugin already skips drafts via buildAllButDrafts, but status-reconciler bypassed that path. This change adds the same draft guard in triggerNewPresubmits (before the existing mergeable check) so drafts are skipped early. Non-draft behavior is unchanged.
Retire and migrate operations are intentionally unchanged: they continue to apply to all PRs (including drafts) because status contexts still need cleanup/migration.
Test follows the existing unmergable-PR pattern and verifies draft PRs are not triggered.
Fixes: #190