VPA: fix logs and cleanup TODOs according to review

Signed-off-by: Max Cao <[email protected]>

Author: maxcao13

vertical-pod-autoscaler/e2e/v1/actuation.go

@@ -415,7 +415,7 @@ var _ = ActuationSuiteE2eDescribe("Actuation", func() {
 
 		InstallVPA(f, vpaCRD)
 
-		ginkgo.By(fmt.Sprintf("Waiting for in-place update, hoping it won't happen, sleep for %s", VpaEvictionTimeout.String()))
+		ginkgo.By(fmt.Sprintf("Waiting for in-place update, hoping it won't happen, sleep for %s", VpaInPlaceTimeout.String()))
 		CheckNoContainersRestarted(f)
 
 		ginkgo.By("Waiting for pods to be evicted")
@@ -457,7 +457,7 @@ var _ = ActuationSuiteE2eDescribe("Actuation", func() {
 
 		InstallVPA(f, vpaCRD)
 
-		ginkgo.By(fmt.Sprintf("Waiting for in-place update, hoping it won't happen, sleep for %s", VpaEvictionTimeout.String()))
+		ginkgo.By(fmt.Sprintf("Waiting for in-place update, hoping it won't happen, sleep for %s", VpaInPlaceTimeout.String()))
 		CheckNoContainersRestarted(f)
 
 		ginkgo.By("Waiting for pods to be evicted")
@@ -499,7 +499,7 @@ var _ = ActuationSuiteE2eDescribe("Actuation", func() {
 
 		InstallVPA(f, vpaCRD)
 
-		ginkgo.By(fmt.Sprintf("Waiting for in-place update, hoping it won't happen, sleep for %s", VpaEvictionTimeout.String()))
+		ginkgo.By(fmt.Sprintf("Waiting for in-place update, hoping it won't happen, sleep for %s", VpaInPlaceTimeout.String()))
 		CheckNoContainersRestarted(f)
 
 		ginkgo.By("Waiting for pods to be evicted")

vertical-pod-autoscaler/e2e/v1/common.go

@@ -53,6 +53,9 @@ const (
 	// VpaEvictionTimeout is a timeout for VPA to restart a pod if there are no
 	// mechanisms blocking it (for example PDB).
 	VpaEvictionTimeout = 3 * time.Minute
+	// VpaInPlaceTimeout is a timeout for the VPA to finish in-place resizing a
+	// pod (time for vpa to request inplace -> InProgress -> done)
+	VpaInPlaceTimeout = 2 * time.Minute
 
 	defaultHamsterReplicas     = int32(3)
 	defaultHamsterBackoffLimit = int32(10)
@@ -459,7 +462,7 @@ func CheckNoPodsEvicted(f *framework.Framework, initialPodSet PodSet) {
 // updating containers in-place and checks that no containers were restarted.
 func CheckNoContainersRestarted(f *framework.Framework) {
 	var foundContainerRestarts int32
-	time.Sleep(VpaEvictionTimeout)
+	time.Sleep(VpaInPlaceTimeout)
 	podList, err := GetHamsterPods(f)
 	for _, pod := range podList.Items {
 		containerRestarts := getContainerRestarts(pod.Status)

vertical-pod-autoscaler/e2e/v1/updater.go

@@ -81,6 +81,7 @@ var _ = UpdaterE2eDescribe("Updater", func() {
 		ginkgo.By("Waiting for pods to be in-place updated")
 
 		//gomega.Expect(err).NotTo(gomega.HaveOccurred())
+		// TODO(maxcao13): I don't think we need this much complexity for checking inplace, but I won't remove it for now
 		err := WaitForPodsUpdatedWithoutEviction(f, initialPods, podList)
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
 	})
@@ -120,6 +121,45 @@ var _ = UpdaterE2eDescribe("Updater", func() {
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
 	})
 
+	ginkgo.It("does not in-place update pods when there is no recommendation", func() {
+		const statusUpdateInterval = 10 * time.Second
+
+		ginkgo.By("Setting up the Admission Controller status")
+		stopCh := make(chan struct{})
+		statusUpdater := status.NewUpdater(
+			f.ClientSet,
+			status.AdmissionControllerStatusName,
+			status.AdmissionControllerStatusNamespace,
+			statusUpdateInterval,
+			"e2e test",
+		)
+		defer func() {
+			// Schedule a cleanup of the Admission Controller status.
+			// Status is created outside the test namespace.
+			ginkgo.By("Deleting the Admission Controller status")
+			close(stopCh)
+			err := f.ClientSet.CoordinationV1().Leases(status.AdmissionControllerStatusNamespace).
+				Delete(context.TODO(), status.AdmissionControllerStatusName, metav1.DeleteOptions{})
+			gomega.Expect(err).NotTo(gomega.HaveOccurred())
+		}()
+		statusUpdater.Run(stopCh)
+
+		podList := setupPodsForUpscalingWithoutRecommendation(f)
+		if len(podList.Items[0].Spec.Containers[0].ResizePolicy) <= 0 {
+			// Feature is probably not working here
+			ginkgo.Skip("Skipping test, InPlacePodVerticalScaling not available")
+		}
+
+		ginkgo.By(fmt.Sprintf("Waiting for pods to be in-place updated, hoping it won't happen, sleep for %s", VpaInPlaceTimeout.String()))
+		CheckNoContainersRestarted(f)
+
+		updatedPodList, err := GetHamsterPods(f)
+		gomega.Expect(err).NotTo(gomega.HaveOccurred())
+		for _, pod := range updatedPodList.Items {
+			gomega.Expect(getCPURequest(pod.Spec)).To(gomega.Equal(ParseQuantityOrDie("100m")))
+		}
+	})
+
 	ginkgo.It("evicts pods when Admission Controller status available", func() {
 		const statusUpdateInterval = 10 * time.Second
 
@@ -265,14 +305,18 @@ func setupPodsForEviction(f *framework.Framework, hamsterCPU, hamsterMemory stri
 }
 
 func setupPodsForUpscalingInPlace(f *framework.Framework) *apiv1.PodList {
-	return setupPodsForInPlace(f, "100m", "100Mi", nil)
+	return setupPodsForInPlace(f, "100m", "100Mi", nil, true)
 }
 
 func setupPodsForDownscalingInPlace(f *framework.Framework, er []*vpa_types.EvictionRequirement) *apiv1.PodList {
-	return setupPodsForInPlace(f, "500m", "500Mi", er)
+	return setupPodsForInPlace(f, "500m", "500Mi", er, true)
+}
+
+func setupPodsForUpscalingWithoutRecommendation(f *framework.Framework) *apiv1.PodList {
+	return setupPodsForInPlace(f, "100m", "100Mi", nil, false)
 }
 
-func setupPodsForInPlace(f *framework.Framework, hamsterCPU, hamsterMemory string, er []*vpa_types.EvictionRequirement) *apiv1.PodList {
+func setupPodsForInPlace(f *framework.Framework, hamsterCPU, hamsterMemory string, er []*vpa_types.EvictionRequirement, withRecommendation bool) *apiv1.PodList {
 	controller := &autoscaling.CrossVersionObjectReference{
 		APIVersion: "apps/v1",
 		Kind:       "Deployment",
@@ -285,22 +329,25 @@ func setupPodsForInPlace(f *framework.Framework, hamsterCPU, hamsterMemory strin
 
 	ginkgo.By("Setting up a VPA CRD")
 	containerName := GetHamsterContainerNameByIndex(0)
-	vpaCRD := test.VerticalPodAutoscaler().
+	vpaBuilder := test.VerticalPodAutoscaler().
 		WithName("hamster-vpa").
 		WithNamespace(f.Namespace.Name).
 		WithTargetRef(controller).
 		WithUpdateMode(vpa_types.UpdateModeInPlaceOrRecreate).
 		WithEvictionRequirements(er).
-		WithContainer(containerName).
-		AppendRecommendation(
+		WithContainer(containerName)
+
+	if withRecommendation {
+		vpaBuilder = vpaBuilder.AppendRecommendation(
 			test.Recommendation().
 				WithContainer(containerName).
 				WithTarget(containerName, "200m").
 				WithLowerBound(containerName, "200m").
 				WithUpperBound(containerName, "200m").
-				GetContainerResources()).
-		Get()
+				GetContainerResources())
+	}
 
+	vpaCRD := vpaBuilder.Get()
 	InstallVPA(f, vpaCRD)
 
 	return podList