-
Notifications
You must be signed in to change notification settings - Fork 11
Add support for cluster-level queries for nodes and unscheduled pods. #73
Conversation
3205067 to
7810118
Compare
| } | ||
|
|
||
| const std::string node_name( | ||
| config_.KubernetesClusterLevelMetadata() ? "" : current_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.
I'm concerned that the responses are very different here. When a node name is provided, a single node is returned. Is there any logic that distinguishes between a list / single 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.
Good catch. I forgot that in one case you get a Node and in another a NodeList. I've updated the code to be more robust.
25a628c to
7810118
Compare
igorpeshansky
left a comment
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.
Thanks, PTAL.
| } | ||
|
|
||
| const std::string node_name( | ||
| config_.KubernetesClusterLevelMetadata() ? "" : current_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.
Good catch. I forgot that in one case you get a Node and in another a NodeList. I've updated the code to be more robust.
supriyagarg
left a comment
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.
LGTM.
|
|
||
| MetadataUpdater::ResourceMetadata KubernetesReader::GetNodeMetadata( | ||
| json::value raw_node, Timestamp collected_at, bool is_deleted) const | ||
| const json::Object* node, Timestamp collected_at, bool is_deleted) const |
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.
this is a nice change!
bmoyles0117
left a comment
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.
LGTM
| LOG(INFO) << "Current node is " << current_node; | ||
| } | ||
|
|
||
| const std::string watched_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.
If we don't have it already, can we add a very prominent log line (not based on verbose) that states that we're doing a cluster level watch, vs a node level watch?
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.
fc97bfc to
61c4f0c
Compare
|
Rebased off |
No description provided.