Add labelSelector argument to GetCurrentPodCount#61
Add labelSelector argument to GetCurrentPodCount#61rsevilla87 merged 3 commits intocloud-bulldozer:mainfrom
Conversation
Signed-off-by: Raul Sevilla <rsevilla@redhat.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #61 +/- ##
==========================================
+ Coverage 80.86% 82.01% +1.15%
==========================================
Files 10 10
Lines 486 406 -80
==========================================
- Hits 393 333 -60
+ Misses 71 51 -20
Partials 22 22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rsevilla87
changed the title
ocp-metadata/ocp-metadata.go
Outdated
| // GetCurrentPodCount returns the number of current running pods across all worker nodes | ||
| func (meta *Metadata) GetCurrentPodCount() (int, error) { | ||
| // GetCurrentPodCount returns the number of running pods across on nodes matching the given labelSelector | ||
| func (meta *Metadata) GetCurrentPodCount(labelSelector string) (int, error) { |
There was a problem hiding this comment.
I think you should rename the parameter to indicate that the labelSelector is for the nodes. Otherwise, since the function is called GetCurrentPodCount it's easy to assume the selector is for the pods. Something link nodesLabelSelector
ocp-metadata/ocp-metadata.go
Outdated
There was a problem hiding this comment.
If you're changing this function, consider bettering it by making a map from the list of names allowing for an O(1) search instead of the inner loop
Signed-off-by: Raul Sevilla <rsevilla@redhat.com>
ocp-metadata/ocp-metadata.go
Outdated
| func (meta *Metadata) GetCurrentPodCount(nodeLabelSelector string) (int, error) { | ||
| var podCount int | ||
| nodeList, err := meta.connector.ClientSet().CoreV1().Nodes().List(context.TODO(), metav1.ListOptions{LabelSelector: labelSelector}) | ||
| mapList := make(map[string]bool) |
There was a problem hiding this comment.
It's better to move the declaration of mapList to after you know its size (before the loop). Then you can initialize it this way:
mapList := make(map[string]bool, len(nodeList.Items))
ocp-metadata/ocp-metadata.go
Outdated
| podCount++ | ||
| break | ||
| } | ||
| if _, ok := mapList[pod.Spec.NodeName]; !ok { |
There was a problem hiding this comment.
Since you defined mapList as map[string]bool you don't need to check ok. You can just do:
if mapList[pod.Spec.NodeName] {
There was a problem hiding this comment.
you're right!, I renamed the variable to nodeMap as well, not sure why I wrote mapList
Signed-off-by: Raul Sevilla <rsevilla@redhat.com>
2 participants
Type of change
Description
Updating the GetCurrentPodCount() method to accept a labelSelector argument.
cc @ygalblum @jtaleric
Related Tickets & Documents