Optimize GetCurrentPodCount() by listing pods only once #36
Conversation
Signed-off-by: Raul Sevilla <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #36 +/- ##
=======================================
Coverage 83.33% 83.33%
=======================================
Files 6 6
Lines 312 312
=======================================
Hits 260 260
Misses 38 38
Partials 14 14 ☔ View full report in Codecov by Sentry. |
| podList, err := meta.clientSet.CoreV1().Pods(metav1.NamespaceAll).List(context.TODO(), metav1.ListOptions{FieldSelector: "status.phase=Running,spec.nodeName=" + node.Name}) | ||
| if err != nil { | ||
| return podCount, err | ||
| podList, err := meta.clientSet.CoreV1().Pods(metav1.NamespaceAll).List(context.TODO(), metav1.ListOptions{FieldSelector: "status.phase=Running"}) |
There was a problem hiding this comment.
I am not sure on how this logic is any different from previous code. Its seems like previously code is also collecting podsCount per each node. Are we seeing any duplicate counts in the previous logic?
There was a problem hiding this comment.
From what I can see, with the previous code if you have 100 nodes, this will create 100 API calls... Where the code change that @rsevilla87 has will be a single API call no matter the node count then using the pod description he filters per-node. Effectively doing the same thing, but with less API calls.
There was a problem hiding this comment.
Yes perfect. Make sense to me now.
Type of change
Description
This simple code change boosts this function
Related Tickets & Documents
Checklist before requesting a review
Testing