updater: fetch pods matching VPA selectors

Signed-off-by: Omer Aplatony <[email protected]>

Author: omerap12

vertical-pod-autoscaler/pkg/updater/logic/updater.go

@@ -26,6 +26,7 @@ import (
 	apiv1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/util/sets"
 	kube_client "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
 	corescheme "k8s.io/client-go/kubernetes/scheme"
@@ -166,6 +167,8 @@ func (u *updater) RunOnce(ctx context.Context) {
 
 	inPlaceFeatureEnable := features.Enabled(features.InPlaceOrRecreate)
 
+	seenPods := sets.New[*apiv1.Pod]()
+
 	for _, vpa := range vpaList {
 		if slices.Contains(u.ignoredNamespaces, vpa.Namespace) {
 			klog.V(3).InfoS("Skipping VPA object in ignored namespace", "vpa", klog.KObj(vpa), "namespace", vpa.Namespace)
@@ -185,6 +188,16 @@ func (u *updater) RunOnce(ctx context.Context) {
 			klog.V(3).InfoS("Skipping VPA object because we cannot fetch selector", "vpa", klog.KObj(vpa))
 			continue
 		}
+		podsWithSelector, err := u.podLister.List(selector)
+		if err != nil {
+			klog.ErrorS(err, "Failed to get pods", "selector", selector)
+			continue
+		}
+
+		// handle the case of overlapping VPA selectors
+		for _, pod := range podsWithSelector {
+			seenPods.Insert(pod)
+		}
 
 		vpas = append(vpas, &vpa_api_util.VpaWithSelector{
 			Vpa:      vpa,
@@ -200,13 +213,8 @@ func (u *updater) RunOnce(ctx context.Context) {
 		return
 	}
 
-	podsList, err := u.podLister.List(labels.Everything())
-	if err != nil {
-		klog.ErrorS(err, "Failed to get pods list")
-		return
-	}
 	timer.ObserveStep("ListPods")
-	allLivePods := filterDeletedPods(podsList)
+	allLivePods := filterDeletedPodsFromSet(seenPods)
 
 	controlledPods := make(map[*vpa_types.VerticalPodAutoscaler][]*apiv1.Pod)
 	for _, pod := range allLivePods {
@@ -391,10 +399,14 @@ func filterNonEvictablePods(pods []*apiv1.Pod, evictionRestriction restriction.P
 	return filterPods(pods, evictionRestriction.CanEvict)
 }
 
-func filterDeletedPods(pods []*apiv1.Pod) []*apiv1.Pod {
-	return filterPods(pods, func(pod *apiv1.Pod) bool {
-		return pod.DeletionTimestamp == nil
-	})
+func filterDeletedPodsFromSet(pods sets.Set[*apiv1.Pod]) []*apiv1.Pod {
+	result := make([]*apiv1.Pod, 0)
+	for p := range pods {
+		if p.DeletionTimestamp == nil {
+			result = append(result, p)
+		}
+	}
+	return result
 }
 
 func newPodLister(kubeClient kube_client.Interface, namespace string) v1lister.PodLister {

vertical-pod-autoscaler/pkg/updater/logic/updater_test.go

@@ -32,6 +32,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/fake"
 	featuregatetesting "k8s.io/component-base/featuregate/testing"
 
@@ -232,6 +233,7 @@ func testRunOnceBase(
 			Get()
 
 		pods[i].Labels = labels
+		pods[i].UID = types.UID(fmt.Sprintf("pod-uid-%d", i))
 
 		inplace.On("CanInPlaceUpdate", pods[i]).Return(canInPlaceUpdate)
 		if shouldInPlaceFail {
@@ -251,7 +253,7 @@ func testRunOnceBase(
 	vpaLister := &test.VerticalPodAutoscalerListerMock{}
 
 	podLister := &test.PodListerMock{}
-	podLister.On("List").Return(pods, nil)
+	podLister.On("List", selector).Return(pods, nil)
 	targetRef := &v1.CrossVersionObjectReference{
 		Kind:       rc.Kind,
 		Name:       rc.Name,
@@ -381,6 +383,7 @@ func TestRunOnceIgnoreNamespaceMatchingPods(t *testing.T) {
 			Get()
 
 		pods[i].Labels = labels
+		pods[i].UID = types.UID(fmt.Sprintf("pod-uid-%d", i))
 		eviction.On("CanEvict", pods[i]).Return(true)
 		eviction.On("Evict", pods[i], nil).Return(nil)
 	}
@@ -392,7 +395,7 @@ func TestRunOnceIgnoreNamespaceMatchingPods(t *testing.T) {
 	vpaLister := &test.VerticalPodAutoscalerListerMock{}
 
 	podLister := &test.PodListerMock{}
-	podLister.On("List").Return(pods, nil)
+	podLister.On("List", selector).Return(pods, nil)
 	targetRef := &v1.CrossVersionObjectReference{
 		Kind:       rc.Kind,
 		Name:       rc.Name,

vertical-pod-autoscaler/pkg/utils/test/test_utils.go

@@ -156,7 +156,7 @@ func (m *PodListerMock) Pods(namespace string) v1.PodNamespaceLister {
 
 // List is a mock implementation of PodLister.List
 func (m *PodListerMock) List(selector labels.Selector) (ret []*apiv1.Pod, err error) {
-	args := m.Called()
+	args := m.Called(selector)
 	var returnArg []*apiv1.Pod
 	if args.Get(0) != nil {
 		returnArg = args.Get(0).([]*apiv1.Pod)