-
Notifications
You must be signed in to change notification settings - Fork 39.5k
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
Add node components version to API #5951
Conversation
cc/ @bgrant0607 |
One general problem I have is whether versions should be freeform strings, or parsed semantic versions, or what. I suppose pushing it off to the consumer is a fine solution? |
I think the problem with "push it off to the consumers" comes when we hit the "apiserver must speak only the capabilities of the least capable |
Based on my internal cluster management experiment, we used to run into the issues of parsing version info failed due to arbitrary testing binaries. I believe we might run into more parse failures with kubernetes after we have bring-your-own-node-to-cluster case later. But I am fine with either approach though. @bgrant0607 |
Agreed. I just can't tell who is supposed to carry that burden.
|
// Kubelet version reported by the node | ||
KubeletVersion string `json:"kubeletVersion" description:"Kubelet version reported by the node"` | ||
// Kube-proxy version reported by the node | ||
ProxyVersion string `json:"proxyVersion" description:"Kube-proxy version reported by the node"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
KubeProxyVersion.
We have multiple "proxies" in the system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
As for where parsing, if any, should happen: definitely not in Kubelet. |
LGTM. I assume you'll address the other comments. I think freeform string is fine for now; I don't think we have enough information yet to know what to put in a structured version. |
We've already been parsing major/minor/patch of the k8s version itself for GKE, for deployment method forking. Kubelet version is going to follow that same prescriptive format. But you're right that the consumer can parse it just as easily, so, shrug, I don't particularly care. |
Push changes? |
Just pushed the changes, thanks! |
// Kernel version reported by the node | ||
KernelVersion string `json:"kernelVersion" description:"Kernel version reported by the node"` | ||
// OS image used reported by the node | ||
OsImage string `json:"osImage" description:"OS image used reported by the node"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make all the description strings the same for all API versions.
Done! PTAL? |
github is back, and travis and shippable are green. Ready to merge? But before the merge, I do have a question. I included DockerVersion, but meanwhile we are introducing Rocket as another runtime plugin. Should we replace DockerVersion with ContainerRuntimeVersion in the format like "docker: 1.5.0"? |
Discussed offline: Yes, ContainerRuntimeVersion is a good idea. |
Made the change. PTAL? |
LGTM |
Add node components version to API
cc/ @zmerlynn
related to #5948
Next PR will populate those information.