use legacy restmapper against undiscoverable servers #10058
Conversation
|
[test] |
| if err != nil { | ||
| return false | ||
| } | ||
| if serverSemVer.LT(semver.MustParse("1.3.0")) { |
There was a problem hiding this comment.
does this work for origin and ose and kubernetes?
There was a problem hiding this comment.
does this work for origin and ose and kubernetes?
Yes, it checks the kube version and we know which levels of kube we shipped with each level of origin.
|
Evaluated for origin test up to 2ee6180 |
| // enough discovery information avaiable to reliably build a RESTMapper. If not, use the | ||
| // hardcoded mapper in this client (legacy behavior) | ||
| func useDiscoveryRESTMapper(serverVersion string) bool { | ||
| serverSemVer, err := semver.Parse(serverVersion[1:]) |
There was a problem hiding this comment.
how does this handle emptystring (or whatever values ServerVersion() can return for legacy servers)?
There was a problem hiding this comment.
how does this handle emptystring (or whatever values ServerVersion() can return for legacy servers)?
We never shipped a level that gave back empty string. If there was an error getting the version, we handle that before this call.
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6998/) |
|
@bparees any further comment? |
|
Does falling back to the legacy RestMapper mean we don't dynamically filter the |
No, that's done by the shortcut expander layered on top. |
|
@deads2k nope, lgtm. |
|
[merge] |
|
Evaluated for origin merge up to 2ee6180 |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6998/) (Image: devenv-rhel7_4730) |
Simple, coarse grained solution to https://bugzilla.redhat.com/show_bug.cgi?id=1360170#c4
@bparees inelegant, but easier than trying to fine tune fallback scenarios and it's easy to inspect.