Skip to content
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

Merged
merged 2 commits into from
Mar 27, 2015
Merged

Conversation

dchen1107
Copy link
Member

cc/ @zmerlynn

related to #5948

Next PR will populate those information.

@dchen1107
Copy link
Member Author

cc/ @bgrant0607

@zmerlynn
Copy link
Member

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?

@zmerlynn
Copy link
Member

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 kubelet software version (the first part of the tuple)" part of #4855. I suspect we're going to need to be able to send parsed semantic versions, like a Major/Minor/Patch ints. I can point you to internal Go parsing code for it, though, if you don't want to reinvent the wheel.

@zmerlynn
Copy link
Member

cc @davidopp @lavalamp

Though I still wish we had a concrete example of that case. :/

@dchen1107
Copy link
Member Author

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

@zmerlynn
Copy link
Member

Agreed. I just can't tell who is supposed to carry that burden.
On Mar 25, 2015 5:41 PM, "Dawn Chen" notifications@github.com wrote:

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
https://github.com/bgrant0607


Reply to this email directly or view it on GitHub
#5951 (comment)
.

// 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"`
Copy link
Member

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@bgrant0607
Copy link
Member

As for where parsing, if any, should happen: definitely not in Kubelet.

@davidopp
Copy link
Member

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.

@zmerlynn
Copy link
Member

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.

@bgrant0607
Copy link
Member

Push changes?

@dchen1107
Copy link
Member Author

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"`
Copy link
Member

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.

@dchen1107
Copy link
Member Author

Done! PTAL?

@dchen1107
Copy link
Member Author

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"?

@bgrant0607
Copy link
Member

Discussed offline: Yes, ContainerRuntimeVersion is a good idea.

@dchen1107
Copy link
Member Author

Made the change. PTAL?

@bgrant0607
Copy link
Member

LGTM

bgrant0607 added a commit that referenced this pull request Mar 27, 2015
Add node components version to API
@bgrant0607 bgrant0607 merged commit 70e0fad into kubernetes:master Mar 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants