VPA: test multi VPA behaviour.

Test that in a scenario where multiple VPAs
match one Pod, one VPA has `updateMode: InPlace` and
all others have `updateMode: Off`, VPA updater always
applies the in place resize from the correct VPA.

Also adds a timestamp to a similar test in the admissions
controller to make the test stronger.

Updates #10158

Signed-off-by: Irbe Krumina <irbekrm@gmail.com>

Author: irbekrm

vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/matcher_test.go

@@ -19,6 +19,7 @@ package vpa
 import (
 	"context"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"go.uber.org/mock/gomock"
@@ -126,8 +127,8 @@ func TestGetMatchingVpa(t *testing.T) {
 			name: "two vpas, one in off mode",
 			pod:  podBuilder.Get(),
 			vpas: []*vpa_types.VerticalPodAutoscaler{
-				vpaBuilder.WithUpdateMode(vpa_types.UpdateModeOff).WithName("off-vpa").WithTargetRef(targetRef).Get(),
-				vpaBuilder.WithUpdateMode(vpa_types.UpdateModeRecreate).WithName("recreate-vpa").WithTargetRef(targetRef).Get(),
+				vpaBuilder.WithUpdateMode(vpa_types.UpdateModeOff).WithName("off-vpa").WithCreationTimestamp(time.Now().Add(-time.Hour)).WithTargetRef(targetRef).Get(),
+				vpaBuilder.WithUpdateMode(vpa_types.UpdateModeRecreate).WithName("recreate-vpa").WithCreationTimestamp(time.Now()).WithTargetRef(targetRef).Get(),
 			},
 			labelSelector:   "app = test",
 			expectedFound:   true,

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

@@ -24,8 +24,10 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"go.uber.org/mock/gomock"
 	"golang.org/x/time/rate"
+	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv1 "k8s.io/api/autoscaling/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -34,20 +36,27 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes/fake"
+	"k8s.io/client-go/tools/record"
 	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	"k8s.io/utils/set"
 
+	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/patch"
+	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation"
 	vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features"
 	controllerfetcher "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher"
 	target_mock "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/mock"
+	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/inplace"
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/priority"
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/restriction"
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/utils"
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/annotations"
+	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/limitrange"
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/status"
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/test"
+	vpa_api_util "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/vpa"
 )
 
 func parseLabelSelector(selector string) labels.Selector {
@@ -370,6 +379,152 @@ func testRunOnceBase(
 	inplace.AssertNumberOfCalls(t, "InPlaceUpdate", expectedInPlacedCount)
 }
 
+// TestRunOnce_MultipleVPAs tests that if multiple VPAs target the same Pod and only one of them is 'active' (does NOT have `spec.updateMode: Off`):
+// - the applied recommendation is always the one from the 'active' VPA
+// - the 'active' VPA is always selected as the 'controlling' VPA for the Pod
+func TestRunOnce_MultipleVPAs(t *testing.T) {
+	featuregatetesting.SetFeatureGateDuringTest(t, features.MutableFeatureGate, features.InPlace, true)
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	podLabels := map[string]string{"app": "test"}
+	selector := parseLabelSelector("app = test")
+	containerName := "test"
+	replicas := int32(1)
+	rs := appsv1.ReplicaSet{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "ReplicaSet",
+			APIVersion: "apps/v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "rs",
+			Namespace: "default",
+		},
+		Spec: appsv1.ReplicaSetSpec{
+			Replicas: &replicas,
+		},
+	}
+
+	pod := test.Pod().WithName("test-pod").
+		AddContainer(test.Container().WithName(containerName).
+			WithCPURequest(resource.MustParse("1")).
+			WithMemRequest(resource.MustParse("100M")).
+			WithCPULimit(resource.MustParse("1")).
+			WithMemLimit(resource.MustParse("100M")).Get()).
+		WithCreator(&rs.ObjectMeta, &rs.TypeMeta).
+		Get()
+	pod.Labels = podLabels
+
+	targetRef := &autoscalingv1.CrossVersionObjectReference{
+		Kind:       rs.Kind,
+		Name:       rs.Name,
+		APIVersion: rs.APIVersion,
+	}
+
+	// If multiple VPAs match a Pod, a 'controlling' VPA is selected using VPA
+	// timestamps.
+	// The selector should be given a list that does NOT include any VPAs with
+	// `updateMode: Off` and no startup boost, so the test here would catch if
+	// that logic changed.
+	now := time.Now()
+	VPAInPlace := test.VerticalPodAutoscaler().
+		WithName("vpa-inplace").
+		WithNamespace("default").
+		WithCreationTimestamp(now).
+		WithContainer(containerName).
+		WithUpdateMode(vpa_types.UpdateModeInPlace).
+		WithTarget("3", "300M").
+		WithTargetRef(targetRef).
+		Get()
+
+	VPAOff1 := test.VerticalPodAutoscaler().
+		WithName("vpa-off-1").
+		WithNamespace("default").
+		WithCreationTimestamp(now.Add(-time.Hour)). // older than the InPlace VPA
+		WithContainer(containerName).
+		WithUpdateMode(vpa_types.UpdateModeOff).
+		WithTarget("9", "900M").
+		WithTargetRef(targetRef).
+		Get()
+
+	VPAOff2 := test.VerticalPodAutoscaler().
+		WithName("vpa-off-2").
+		WithNamespace("default").
+		WithCreationTimestamp(now). // timestamp matches the InPlace VPA
+		WithContainer(containerName).
+		WithUpdateMode(vpa_types.UpdateModeOff).
+		WithTarget("5", "500M").
+		WithTargetRef(targetRef).
+		Get()
+
+	VPAOff3 := test.VerticalPodAutoscaler().
+		WithName("vpa-off-3").
+		WithNamespace("default").
+		WithCreationTimestamp(now.Add(time.Hour)). // newer than the InPlace VPA
+		WithContainer(containerName).
+		WithUpdateMode(vpa_types.UpdateModeOff).
+		WithTarget("7", "700M").
+		WithTargetRef(targetRef).
+		Get()
+
+	kubeClient := fake.NewSimpleClientset(pod)
+
+	informerFactory := informers.NewSharedInformerFactory(kubeClient, 0)
+	require.NoError(t, informerFactory.Apps().V1().ReplicaSets().Informer().GetStore().Add(&rs))
+
+	limitRangeCalculator := limitrange.NewNoopLimitsCalculator()
+	recommendationProvider := recommendation.NewProvider(limitRangeCalculator, vpa_api_util.NewCappingRecommendationProcessor(limitRangeCalculator))
+	calculators := []patch.Calculator{inplace.NewResourceInPlaceUpdatesCalculator(recommendationProvider)}
+	restrictionFactory := restriction.NewPodsRestrictionFactory(kubeClient, informerFactory, 0, 0, calculators, false)
+
+	vpaLister := &test.VerticalPodAutoscalerListerMock{}
+	vpaLister.On("List").Return([]*vpa_types.VerticalPodAutoscaler{VPAInPlace, VPAOff1, VPAOff2, VPAOff3}, nil).Once()
+
+	podLister := &test.PodListerMock{}
+	podLister.On("List").Return([]*corev1.Pod{pod}, nil)
+
+	mockSelectorFetcher := target_mock.NewMockVpaTargetSelectorFetcher(ctrl)
+	mockSelectorFetcher.EXPECT().Fetch(gomock.Eq(VPAInPlace)).Return(selector, nil)
+
+	u := &updater{
+		vpaLister:                    vpaLister,
+		podLister:                    podLister,
+		restrictionFactory:           restrictionFactory,
+		evictionRateLimiter:          rate.NewLimiter(rate.Inf, 0),
+		inPlaceRateLimiter:           rate.NewLimiter(rate.Inf, 0),
+		evictionAdmission:            priority.NewDefaultPodEvictionAdmission(),
+		recommendationProcessor:      &test.FakeRecommendationProcessor{},
+		selectorFetcher:              mockSelectorFetcher,
+		controllerFetcher:            controllerfetcher.FakeControllerFetcher{},
+		useAdmissionControllerStatus: true,
+		statusValidator:              newFakeValidator(true),
+		priorityProcessor:            priority.NewProcessor(),
+		eventRecorder:                record.NewFakeRecorder(10),
+	}
+
+	u.RunOnce(context.Background())
+
+	updatedPod, err := kubeClient.CoreV1().Pods("default").Get(context.Background(), pod.Name, metav1.GetOptions{})
+	require.NoError(t, err)
+
+	// Test that the Pod got patched with the recommendation from the VPA with `updateMode: InPlaceOrRecreate`.
+	gotCPUReq := updatedPod.Spec.Containers[0].Resources.Requests[corev1.ResourceCPU]
+	assert.True(t, resource.MustParse("3").Equal(gotCPUReq),
+		"Container's CPU request should be patched with the recommendation from %q (3 CPU), got %s", VPAInPlace.Name, gotCPU.String())
+
+	gotMemReq := updatedPod.Spec.Containers[0].Resources.Requests[corev1.ResourceMemory]
+	assert.True(t, resource.MustParse("300M").Equal(gotMemReq),
+		"Container's memory request should be patched with the recommendation from %q (300M), got %s", VPAInPlace.Name, gotMem.String())
+
+	gotCPULimit := updatedPod.Spec.Containers[0].Resources.Limits[corev1.ResourceCPU]
+	assert.True(t, resource.MustParse("3").Equal(gotCPULimit),
+		"Container's CPU limit should be patched with the recommendation from %q (3 CPU), got %s", VPAInPlace.Name, gotCPULimit.String())
+
+	gotMemLimit := updatedPod.Spec.Containers[0].Resources.Limits[corev1.ResourceMemory]
+	assert.True(t, resource.MustParse("300M").Equal(gotMemLimit),
+		"Container's memory limit should be patched with the recommendation from %q (300M), got %s", VPAInPlace.Name, gotMemLimit.String())
+}
+
 func TestRunOnceNotingToProcess(t *testing.T) {
 	eviction := &test.PodsEvictionRestrictionMock{}
 	inplace := &test.PodsInPlaceRestrictionMock{}