Add /version to server and check it in client. #627
Conversation
| func Get() (major, minor, gitCommit string) { | ||
| return "v1beta", "1", commitFromGit | ||
| type Info struct { | ||
| Major string |
There was a problem hiding this comment.
Done, though I'm not sure it matters. I didn't make this an API object... I can do that, though, if you think it's better.
There was a problem hiding this comment.
Probably worth doing that eventually (doesn't have to be this PR) since I think we will eventually split API + Client away from the "core" librararies, since many people will only want to build the client libs.
|
I moved the implementation from kubecfg to client. Components besides kubecfg might also want to protect against version skew. |
|
LGTM |
|
@smarterclayton I was thinking of this more like a build number; this is the binary version, not the API version. Open to suggestions to make that clearer. (Maybe make it not sound similar?) |
|
Isn't the whole point of API versioning and enforced backwards compatible changes to allow the kubecfg and apiserver binary versions to drift? Making it check as default seems very restricting - what if we run two apiservers from different code versions? A caching apiserver compiled by team X and the original by team Y? Will kubecfg reject my requests randomly if the server is in the middle of a rolling update when I get balanced to a particular server? If I'm using kubecfg to update a docker image running the apiserver on top of a kubelet, should kubecfg fail? Seems like this is something that our test cases and test environments would leverage, but other than that would be uncommonly used outside of that. That's why I picked up on the similarity to the api version because that's what I assumed it was doing. |
|
All good points. You're right, checking by default is much too restrictive. I'll reverse the flag. |
|
Comments addressed, PTAL. |
| templateFile = flag.String("template_file", "", "If present, load this file as a golang template and use it for output printing") | ||
| templateStr = flag.String("template", "", "If present, parse this string as a golang template and use it for output printing") | ||
| versionFlag = flag.Bool("V", false, "Print the version number.") | ||
| serverVersion = flag.Bool("SV", false, "Print the server's version number.") |
There was a problem hiding this comment.
SV leaves a bad taste in my mouth - posix long arguments are always lowercase. How about 'server_version' (how often will someone call this)? Kind of a nit though.
|
LGTM besides nits - |
|
Nits fixed. |
|
One more - I think we should add -expect_version_match to hack/test-cmd.sh |
Will help detect client/version skew and prevent e2e test from passing while running a version other than the one you think it's running.
|
OK, done. |
|
LGTM |
|
I'm gonna self-merge this later if no one beats me to it. |
Add /version to server and check it in client.
|
Saved you from taking the blame on a self merge break |
Add a v2 client library, starting with machine information.
Bug 1949661: UPSTREAM: <carry>: management pinning annotations
Add medik8s.io to Remdy Systems section
Will help detect client/version skew and prevent e2e test from passing
while running a version other than the one you think it's running.