chenya-zhang commented on a change in pull request #305:
URL:
https://github.com/apache/incubator-yunikorn-k8shim/pull/305#discussion_r716078393
##########
File path: pkg/cache/nodes.go
##########
@@ -100,10 +101,18 @@ func (nc *schedulerNodes) addAndReportNode(node *v1.Node,
reportNode bool) {
// add node to nodes map
if _, ok := nc.nodesMap[node.Name]; !ok {
+ nodeLabels, err := json.Marshal(node.Labels)
Review comment:
Thanks for the reminder @yangwwei! Sorry I failed to see this comment
last time.
If node.Labels is nil, JSON marshal will encode it as the "null" JSON value.
We will check string "null" when decoding it.
Our existing UT in
https://github.com/apache/incubator-yunikorn-k8shim/blob/84f52b7d72cdac1bc4904cf5a631617070225d29/pkg/cache/nodes_test.go#L76
should cover the case if a node has no labels.
On the other hand, your comment reminds me to double check what if we fail
to marshal. Unfortunately, JSON marshal will return a nil along with an err.
This nil will be problematic if passing to string() directly. In this case, I
assign a 0 byte to the JSON format of node labels, which is an empty string for
us to check in YK core.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
