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.

Sign up for GitHub

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

Jump to bottom

Remove nodeInfo endpoint from kubelet #6073

Closed
ddysher opened this issue Mar 27, 2015 · 10 comments
Closed

Remove nodeInfo endpoint from kubelet #6073

ddysher opened this issue Mar 27, 2015 · 10 comments
Assignees
@ddysher
Labels
area/kubelet-api area/nodecontroller priority/backlog Higher priority than priority/awaiting-more-evidence. sig/node Categorizes an issue or PR as relevant to SIG Node.

Comments

@ddysher
Copy link
Contributor

ddysher commented Mar 27, 2015

/nodeInfo endpoint is added for nodecontroller to pull node info from kubelet. We need to evaluate if it's still necessary to keep it given this #6005

NodeInfo is also confusing in our api object

// NodeSystemInfo is a set of ids/uuids to uniquely identify the node.
type NodeSystemInfo struct {
...
}

// NodeStatus is information about the current status of a node.
type NodeStatus struct {
    NodeInfo NodeSystemInfo `json:"nodeInfo,omitempty"`
}

// NodeInfo is the information collected on the node.
type NodeInfo struct {
    NodeSystemInfo `json:",inline,omitempty"`
}

@davidopp @bgrant0607 @gmarek
@ddysher ddysher added priority/backlog Higher priority than priority/awaiting-more-evidence. area/nodecontroller labels Mar 27, 2015
@ddysher ddysher mentioned this issue Mar 27, 2015
Closed
@bgrant0607
Copy link
Member

The plan was to get rid of /nodeinfo.

I was also planning to rename type NodeSystemInfo to type NodeInfo, but maybe we should rename the field to NodeSystemInfo instead, since that seems slightly more informative to me.

As part of #2098, I think it would be useful for Kubelet to export its current value of Node - either a copy of the last state sent to apiserver or the most recently updated state that will be next sent to apiserver (or both).

@bgrant0607 bgrant0607 added area/kubelet-api sig/node Categorizes an issue or PR as relevant to SIG Node. labels Mar 28, 2015
@davidopp
Copy link
Member

Kubelet is also using the /nodeInfo endpoint:
https://github.com/GoogleCloudPlatform/kubernetes/blob/984bc8d5f6fd235229e8d2b0af029a5d2bf585c0/pkg/client/kubelet.go#L135

@gmarek
Copy link
Contributor

gmarek commented Apr 2, 2015

IIUC this means we can't get rid of it easily. Do you think it's still worth doing?

@ddysher
Copy link
Contributor Author

ddysher commented Apr 2, 2015

@davidopp If I recall correctly, /nodeinfo endpoint is only used by nodecontroller now, the link is where kubelet expose the endpoint

@gmarek I don't expect much difficulty to remove it. It's added here #5030 for node controller to fetch node info. Once you remove the sync_status=true, it should be easy.

I'm not following the thread #2098, but @bgrant0607 are you suggesting we keep exposing node info even if node controller stops probing? I'm fine with that, but if so, I want to clean up the types I mentioned above, it's really confusing.

@bgrant0607
Copy link
Member

No, I was suggesting that the Kubelet expose the same Node API as the apiserver, just for that Kubelet.

@bgrant0607
Copy link
Member

@davidopp @gmarek pkg/client/kubelet.go is the Kubelet client. Kubelet isn't using that, it's exporting that for the master.

@ddysher
Copy link
Contributor Author

ddysher commented Apr 3, 2015

ok. @gmarek I'm assigning this to myself for load balance the work.

@ddysher ddysher assigned ddysher and unassigned gmarek Apr 3, 2015
@davidopp
Copy link
Member

davidopp commented Apr 5, 2015

Yeah, sorry, what I saw was the definition, not another use. Looks like we can remove this.

@bgrant0607
Copy link
Member

cc @nikhiljindal re #5475

@ddysher ddysher mentioned this issue Apr 10, 2015
Merged
@ddysher
Copy link
Contributor Author

ddysher commented Apr 22, 2015

Closed by #6702

@ddysher ddysher closed this as completed Apr 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ddysher ddysher

Labels
area/kubelet-api area/nodecontroller priority/backlog Higher priority than priority/awaiting-more-evidence. sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@gmarek @ddysher @bgrant0607 @davidopp