-
Notifications
You must be signed in to change notification settings - Fork 4.7k
back project cache with local authorizer #5760
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
Conversation
@liggitt This eliminates half of all server requests in @ironcladlou's runs |
LGTM, and confirmed as being enough to resolve #5737 |
[test] |
adding to the milestone while we consider it |
Do we know what types don't have conversions? |
This is LGTM, and looks as simple as possible. @liggitt agree this is low risk high payoff? |
[test] |
LGTM. This backs the project cache with the same authorizer that is used to satisfy the localresourceaccessreview request submitted previously. The only difference between the localresourceaccessreview call and this impl is that an empty namespace was explicitly disallowed when processing a localresourceaccessreview, but I verified that
|
[merge], although this may result in test flakes due to improved performance, so I'll be watching for them today (this does not result in goroutine scheduling like previous flow did) |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/3965/) (Image: devenv-rhel7_2657) |
Evaluated for origin merge up to 16d6c23 |
continuous-integration/openshift-jenkins/test Running (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/6993/) (Extended Tests: core) |
I have trouble seeing how this is related - an ACL race wouldn't look like this. |
[test] |
Worried that we're seeing a lot of flakes here, but none of them look attributable so far. Testing more |
Evaluated for origin test up to 16d6c23 |
Fixes #5737