diff --git a/vertical-pod-autoscaler/pkg/updater/logic/updater.go b/vertical-pod-autoscaler/pkg/updater/logic/updater.go index ffe266d7cbd0..a30a8ced01e2 100644 --- a/vertical-pod-autoscaler/pkg/updater/logic/updater.go +++ b/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 { diff --git a/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go b/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go index 65fc196ba8eb..0714fc4c0180 100644 --- a/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go +++ b/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, diff --git a/vertical-pod-autoscaler/pkg/utils/test/test_utils.go b/vertical-pod-autoscaler/pkg/utils/test/test_utils.go index 8ae360177f31..c24e29b2e727 100644 --- a/vertical-pod-autoscaler/pkg/utils/test/test_utils.go +++ b/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)