-
Notifications
You must be signed in to change notification settings - Fork 40.4k
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
Fix bug in reflector not detecting "Too large resource version" error before 1.17.0 #115093
Fix bug in reflector not detecting "Too large resource version" error before 1.17.0 #115093
Conversation
97c4afa
to
065285a
Compare
e976938
to
f630791
Compare
f630791
to
11e5e92
Compare
I'm not very thrilled for adding code to support "unsupported" versions, is not just the skewed versions, is that last supported version is 1.22 /assign @liggitt |
Can we accept such fixes if there is no side-effect for supported versions? (This of course is out of the supported versions scope, but client-go is widely used.) If there is any side effect, we must decline the pr.
|
here is my use case:
Considering 1.17 is so old, It's fair if this is declined. But it's really nice if this could be fixed because |
I agree that it is useful for the generic aspects of client-go to work well against as many versions as possible, this doesn't seem burdensome to maintain, and the function it is in is already checking conditions which are only true for already-out-of-support servers. /assign @wojtek-t |
/triage accepted |
+1 The addition also looks correct: /lgtm |
LGTM label has been added. Git tree hash: 3e1cbb21100e706efeb070dcfcbf3270bceb689a
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wojtek-t, xuzhenglun 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 |
I stand corrected 😄 |
What type of PR is this?
/kind bug
What this PR does / why we need it:
client-go
: Automated cherry pick of #92537 upstream release 1.18 #92688 (cherry-pick of Fix bug in reflector not recovering from "Too large resource version"… #92537) fixes "Too large resource version" in reflector and not recovering.client-go
: Fix bug in reflector not detecting "Too large resource version" error #94316 fixes this problem forkube-apiserver
(1.17.0-1.18.5) with newerclient-go
(>0.18.7)kube-apiserver
is in 1.16.x andclient-go
is new, it still not work:since #72170 is reverted in 1.16.x, and the code in the
release-1.16
did not returndetails.cause
field (see ref code). soisTooLargeResourceVersionError
in reflector should fail to detect this error come fromkube-apiserver
before 1.17.0, and can not recovery as expected.I know the version of
client-go
should align withkube-apiserver
as far as we can, and the version skew policy is presented. but in some cases, for example some third party software, is hard to ensure that. so maybe being more comparable is good?Which issue(s) this PR fixes:
Fixes #94315
Fixed #91073
Fixed #94316
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: