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

Prow Config: Do not deduplicate tide queries for different tenants #28528

Merged
merged 2 commits into from
Jan 27, 2023

Conversation

smg247
Copy link
Contributor

@smg247 smg247 commented Jan 25, 2023

This is the second half of the solution to #28459. The first half involved not filtering out exposed repos that shared tide queries with hidden repos in #28460.

This fixes the same issue when the repos that share the same query have a different default tenant configured. I opted to approach this one by simply not deduplicating those queries so that the tide queries would be separated by tenant. This solves the problem without having to tinker with the filtering logic.

An argument could be made that this bug would be better fixed by changing the filtering logic in deck (similarly to how it was done in #28460), but with the way the default tenant ids are configured that would be a very clunky and involved change. I also think that the approach I have made will be more performant as it is only run each time the config is loaded rather than every time someone hits the 'PR Status' page. The downside of this approach is that there will be additional tide queries for the different tenant ids, if configured. This won't result in any issues, but is technically unnecessary outside of fixing this issue.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 25, 2023
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. area/prow Issues or PRs related to prow sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Jan 25, 2023
@smg247 smg247 changed the title Config: Do not deduplicate tide queries for different tenants Prow Config: Do not deduplicate tide queries for different tenants Jan 25, 2023
prow/config/config.go Outdated Show resolved Hide resolved
@smg247
Copy link
Contributor Author

smg247 commented Jan 27, 2023

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jan 27, 2023
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 27, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: petr-muller, smg247

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 27, 2023
@k8s-ci-robot k8s-ci-robot merged commit 3444098 into kubernetes:master Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/prow Issues or PRs related to prow cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants