kubernetes-sigs / prow

Prow is a Kubernetes based CI/CD system developed to serve the Kubernetes community. This repository contains Prow source code and Hugo sources for Prow documentation site.
https://docs.prow.k8s.io
Apache License 2.0
129 stars 99 forks source link

`blunderbuss`: allow `request_count` to be set to '0' effectively disabling functionality #273

Closed smg247 closed 2 months ago

smg247 commented 2 months ago

There is a situation where a repository doesn't want blunderbuss, but it is configured at the org level. Since there is no way to exclude the plugin from functioning on the repo, we must have some way to disable the functionality assigning reviewers. We can do this by allowing request_count to be 0

For: https://issues.redhat.com/browse/DPTP-4164

k8s-ci-robot commented 2 months ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: smg247 Once this PR has been reviewed and has the lgtm label, please assign cjwagner for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[pkg/plugins/OWNERS](https://github.com/kubernetes-sigs/prow/blob/main/pkg/plugins/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
netlify[bot] commented 2 months ago

Deploy Preview for k8s-prow ready!

Name Link
Latest commit 96e88c5c121cfd7722cbbf01944e5d17b85a9913
Latest deploy log https://app.netlify.com/sites/k8s-prow/deploys/66e1d69d5255c70008ed766d
Deploy Preview https://deploy-preview-273--k8s-prow.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

cjwagner commented 2 months ago

Since there is no way to exclude the plugin from functioning on the repo

This isn't true since February 2021. Check out the excluded_repos field in the plugin configuration. https://github.com/kubernetes/test-infra/issues/20631, https://github.com/kubernetes/test-infra/pull/20707

smg247 commented 2 months ago

Since there is no way to exclude the plugin from functioning on the repo

This isn't true since February 2021. Check out the excluded_repos field in the plugin configuration. kubernetes/test-infra#20631, kubernetes/test-infra#20707

Ah, thanks for pointing that out. I even went searching for something like that and couldn't find anything.

We won't need this after all then

/close

k8s-ci-robot commented 2 months ago

@smg247: Closed this PR.

In response to [this](https://github.com/kubernetes-sigs/prow/pull/273#issuecomment-2345151762): >> > Since there is no way to exclude the plugin from functioning on the repo >> >> This isn't true since February 2021. Check out the [`excluded_repos`](https://github.com/search?q=repo%3Akubernetes-sigs%2Fprow+excluded_repos&type=code) field in the plugin configuration. [kubernetes/test-infra#20631](https://github.com/kubernetes/test-infra/issues/20631), [kubernetes/test-infra#20707](https://github.com/kubernetes/test-infra/pull/20707) > >Ah, thanks for pointing that out. I even went searching for something like that and couldn't find anything. > >We won't need this after all then > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
smg247 commented 2 months ago

@cjwagner, I am attempting to use the excluded_repos config, but I am getting a validation error. Maybe I am missing how this is supposed to be used, but I don't see how this logic allows us to exclude a single plugin from an org's configuration for a repo, while including the rest of them.

cjwagner commented 2 months ago

From what I can tell the validation is incomplete and needs to be improved to account for excluded_repos. Maybe we just haven't encountered this case before.

smg247 commented 2 months ago

That is my conclusion as well. I do think that fixing that validation is a better solution than what I have done with this PR, so I will take that approach instead.