diff --git a/vertical-pod-autoscaler/pkg/updater/priority/scaling_direction_pod_eviction_admission.go b/vertical-pod-autoscaler/pkg/updater/priority/scaling_direction_pod_eviction_admission.go index af3b4b6e0a9f..5ae6c7ea035a 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/scaling_direction_pod_eviction_admission.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/scaling_direction_pod_eviction_admission.go @@ -35,7 +35,6 @@ type scalingDirectionPodEvictionAdmission struct { // Admit admits a Pod for eviction in one of three cases // * no EvictionRequirement exists for this Pod -// * no Recommendation exists for at least one Container in this Pod // * no Resource requests are set for at least one Container in this Pod // * all EvictionRequirements are evaluated to 'true' for at least one Container in this Pod func (s *scalingDirectionPodEvictionAdmission) Admit(pod *apiv1.Pod, resources *vpa_types.RecommendedPodResources) bool { @@ -45,8 +44,10 @@ func (s *scalingDirectionPodEvictionAdmission) Admit(pod *apiv1.Pod, resources * } for _, container := range pod.Spec.Containers { recommendedResources := vpa_utils.GetRecommendationForContainer(container.Name, resources) + // if a container doesn't have a recommendation, the VPA has set `.containerPolicy.mode: off` for this container, + // so we can skip this container if recommendedResources == nil { - return true + continue } if s.admitContainer(container, recommendedResources, podEvictionRequirements) { return true diff --git a/vertical-pod-autoscaler/pkg/updater/priority/scaling_direction_pod_eviction_admission_test.go b/vertical-pod-autoscaler/pkg/updater/priority/scaling_direction_pod_eviction_admission_test.go index 22780b658fa2..8cdfd22a8158 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/scaling_direction_pod_eviction_admission_test.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/scaling_direction_pod_eviction_admission_test.go @@ -402,4 +402,24 @@ func TestAdmitForMultipleContainer(t *testing.T) { assert.Equal(tt, false, sdpea.Admit(pod, recommendation)) }) + + t.Run("it should not admit the Pod even if there is a container that doesn't have a Recommendation and the other one doesn't fulfill the EvictionRequirements", func(tt *testing.T) { + evictionRequirements := map[*corev1.Pod][]*v1.EvictionRequirement{ + pod: { + { + Resources: []corev1.ResourceName{corev1.ResourceCPU}, + ChangeRequirement: v1.TargetHigherThanRequests, + }, + }, + } + sdpea := NewScalingDirectionPodEvictionAdmission() + sdpea.(*scalingDirectionPodEvictionAdmission).EvictionRequirements = evictionRequirements + recommendation := &v1.RecommendedPodResources{ + ContainerRecommendations: []v1.RecommendedContainerResources{ + test.Recommendation().WithContainer(container2Name).WithTarget("300m", "10Gi").GetContainerResources(), + }, + } + + assert.Equal(tt, false, sdpea.Admit(pod, recommendation)) + }) }