VPA: refactor pruning grace period as vpa apiObject annotation instea…

…d of containerPolicy

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

vertical-pod-autoscaler/deploy/vpa-v1-crd-gen.yaml

@@ -372,12 +372,6 @@ spec:
                           - Auto
                           - "Off"
                           type: string
-                        pruningGracePeriod:
-                          description: |-
-                            PruningGracePeriod is the duration to wait before pruning recommendations for containers that no longer exist.
-                            By default, recommendations for non-existent containers are never pruned until its top-most controller is deleted,
-                            after which the recommendations are subject to the VPA's recommendation garbage collector.
-                          type: string
                       type: object
                     type: array
                 type: object

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

@@ -29,6 +29,7 @@ import (
 	vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
 	vpa_clientset "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned"
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/test"
+	vpa_api_util "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/vpa"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/cache"
 	klog "k8s.io/klog/v2"
@@ -434,7 +435,9 @@ var _ = RecommenderE2eDescribe("VPA CRD object", func() {
 			WithNamespace(f.Namespace.Name).
 			WithTargetRef(hamsterTargetRef).
 			WithContainer("*").
-			WithPruningGracePeriod("*", 0).
+			WithAnnotations(map[string]string{
+				vpa_api_util.VpaPruningGracePeriodAnnotation: "0",
+			}).
 			Get()
 
 		InstallVPA(f, vpaCRD)
@@ -478,7 +481,9 @@ var _ = RecommenderE2eDescribe("VPA CRD object", func() {
 			WithNamespace(f.Namespace.Name).
 			WithTargetRef(hamsterTargetRef).
 			WithContainer("*").
-			WithPruningGracePeriod("*", 0).
+			WithAnnotations(map[string]string{
+				vpa_api_util.VpaPruningGracePeriodAnnotation: "0",
+			}).
 			Get()
 
 		InstallVPA(f, vpaCRD)

vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go

@@ -215,12 +215,6 @@ type ContainerResourcePolicy struct {
 	// The default is "RequestsAndLimits".
 	// +optional
 	ControlledValues *ContainerControlledValues `json:"controlledValues,omitempty" protobuf:"bytes,6,rep,name=controlledValues"`
-
-	// PruningGracePeriod is the duration to wait before pruning recommendations for containers that no longer exist.
-	// By default, recommendations for non-existent containers are never pruned until its top-most controller is deleted,
-	// after which the recommendations are subject to the VPA's recommendation garbage collector.
-	// +optional
-	PruningGracePeriod *metav1.Duration `json:"pruningGracePeriod,omitempty" protobuf:"bytes,4,opt,name=pruningGracePeriod"`
 }
 
 const (

vertical-pod-autoscaler/pkg/recommender/model/aggregate_container_state.go

@@ -318,10 +318,10 @@ func (a *AggregateContainerState) UpdateFromPolicy(resourcePolicy *vpa_types.Con
 	}
 }
 
-// UpdatePruningGracePeriod updates the an aggregate state with a containerPruningGracePeriod or the global pruning duration if nil.
-func (a *AggregateContainerState) UpdatePruningGracePeriod(containerPruningGracePeriod *metav1.Duration) {
+// UpdatePruningGracePeriod updates an AggregateContainerState with a containerPruningGracePeriod or the global pruning-grace-period-duration if nil.
+func (a *AggregateContainerState) UpdatePruningGracePeriod(containerPruningGracePeriod *time.Duration) {
 	if containerPruningGracePeriod != nil {
-		a.PruningGracePeriod = &containerPruningGracePeriod.Duration
+		a.PruningGracePeriod = containerPruningGracePeriod
 	} else {
 		a.PruningGracePeriod = parsedPruningGracePeriodDuration
 	}

vertical-pod-autoscaler/pkg/recommender/model/aggregate_container_state_test.go

@@ -302,34 +302,36 @@ func TestParsePruningGracePeriodDuration(t *testing.T) {
 	testCases := []struct {
 		name        string
 		initialFlag string
-		policy      *vpa_types.ContainerResourcePolicy
+		gracePeriod *time.Duration
 		expected    *time.Duration
+		checkPanic  bool
 	}{
 		{
 			name:        "Explicit PruningGracePeriod",
 			initialFlag: "10m",
-			policy: &vpa_types.ContainerResourcePolicy{
-				PruningGracePeriod: &metav1.Duration{Duration: 10 * time.Minute},
-			},
-			expected: ptr.To(10 * time.Minute),
+			gracePeriod: ptr.To(10 * time.Minute),
+			expected:    ptr.To(10 * time.Minute),
+			checkPanic:  false,
 		}, {
 			name:        "No PruningGracePeriod specified - default to nil",
 			initialFlag: "",
-			policy:      &vpa_types.ContainerResourcePolicy{},
+			gracePeriod: nil,
 			expected:    nil,
+			checkPanic:  false,
 		}, {
-			name:        "Invalid policy - exit with error",
+			name:        "Invalid global PruningGracePeriod - exit with error",
 			initialFlag: "badDuration",
-			policy:      nil,
+			gracePeriod: nil,
 			expected:    nil,
+			checkPanic:  true,
 		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			flag.Set("pruning-grace-period-duration", tc.initialFlag)
 			cs := NewAggregateContainerState()
-			cs.UpdateFromPolicy(tc.policy)
-			if tc.policy != nil {
+			cs.UpdatePruningGracePeriod(tc.gracePeriod)
+			if !tc.checkPanic {
 				assert.Equal(t, tc.expected, parsePruningGracePeriodDuration())
 			} else {
 				assert.Panics(t, func() { parsePruningGracePeriodDuration() })

vertical-pod-autoscaler/pkg/recommender/model/cluster.go

@@ -287,16 +287,18 @@ func (cluster *ClusterState) AddOrUpdateVpa(apiObject *vpa_types.VerticalPodAuto
 
 			vpaExists = false
 		} else {
-			// Update the pruningGracePeriod to ensure a potential new grace period is applied.
+			// Always update the pruningGracePeriod to ensure a potential new grace period is applied.
 			// This prevents an old, excessively long grace period from persisting and
 			// potentially causing the VPA to keep stale aggregates with an outdated grace period.
-			for key, containerState := range vpa.aggregateContainerStates {
-				containerState.UpdatePruningGracePeriod(vpa_utils.GetContainerPruningGracePeriod(key.ContainerName(), apiObject.Spec.ResourcePolicy))
+			vpa.PruningGracePeriod = vpa_utils.ParsePruningGracePeriodFromAnnotations(annotationsMap)
+			for _, containerState := range vpa.aggregateContainerStates {
+				containerState.UpdatePruningGracePeriod(vpa.PruningGracePeriod)
 			}
 		}
 	}
 	if !vpaExists {
 		vpa = NewVpa(vpaID, selector, apiObject.CreationTimestamp.Time)
+		vpa.PruningGracePeriod = vpa_utils.ParsePruningGracePeriodFromAnnotations(annotationsMap)
 		cluster.Vpas[vpaID] = vpa
 		for aggregationKey, aggregation := range cluster.aggregateStateMap {
 			vpa.UseAggregationIfMatching(aggregationKey, aggregation)

vertical-pod-autoscaler/pkg/recommender/model/vpa.go

@@ -115,6 +115,10 @@ type Vpa struct {
 	// "fractionalizing" minResources erroneously during a redeploy when when a pod's
 	// container is removed or renamed
 	ContainersPerPod int
+	// PruningGracePeriod is the duration to wait before pruning recommendations for containers that no longer exist under a VPA.
+	// By default, recommendations for non-existent containers are never pruned until its top-most controller is deleted,
+	// after which the recommendations are subject to the VPA's recommendation garbage collector.
+	PruningGracePeriod *time.Duration
 }
 
 // NewVpa returns a new Vpa with a given ID and pod selector. Doesn't set the
@@ -160,7 +164,7 @@ func (vpa *Vpa) UseAggregationIfMatching(aggregationKey AggregateStateKey, aggre
 		vpa.aggregateContainerStates[aggregationKey] = aggregation
 		aggregation.IsUnderVPA = true
 		aggregation.UpdateMode = vpa.UpdateMode
-		aggregation.UpdatePruningGracePeriod(vpa_api_util.GetContainerPruningGracePeriod(aggregationKey.ContainerName(), vpa.ResourcePolicy))
+		aggregation.UpdatePruningGracePeriod(vpa.PruningGracePeriod)
 		aggregation.UpdateFromPolicy(vpa_api_util.GetContainerResourcePolicy(aggregationKey.ContainerName(), vpa.ResourcePolicy))
 	}
 }

vertical-pod-autoscaler/pkg/recommender/model/vpa_test.go

@@ -31,7 +31,7 @@ import (
 
 var (
 	anyTime = time.Unix(0, 0)
-	// TODO(maxcao13): write tests for new container policy field
+	// TODO(maxcao13): write tests for the pruningGracePeriod field
 )
 
 func TestMergeAggregateContainerState(t *testing.T) {

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

@@ -254,12 +254,11 @@ func (b *verticalPodAutoscalerBuilder) Get() *vpa_types.VerticalPodAutoscaler {
 	scalingModeAuto := vpa_types.ContainerScalingModeAuto
 	for _, containerName := range b.containerNames {
 		containerResourcePolicy := vpa_types.ContainerResourcePolicy{
-			ContainerName:      containerName,
-			Mode:               &scalingModeAuto,
-			MinAllowed:         b.minAllowed[containerName],
-			MaxAllowed:         b.maxAllowed[containerName],
-			ControlledValues:   b.controlledValues[containerName],
-			PruningGracePeriod: b.pruningGracePeriod[containerName],
+			ContainerName:    containerName,
+			Mode:             &scalingModeAuto,
+			MinAllowed:       b.minAllowed[containerName],
+			MaxAllowed:       b.maxAllowed[containerName],
+			ControlledValues: b.controlledValues[containerName],
 		}
 		if scalingMode, ok := b.scalingMode[containerName]; ok {
 			containerResourcePolicy.Mode = scalingMode

vertical-pod-autoscaler/pkg/utils/vpa/api.go

@@ -52,6 +52,8 @@ type patchRecord struct {
 	Value interface{} `json:"value"`
 }
 
+const VpaPruningGracePeriodAnnotation = "vpaPruningGracePeriod"
+
 func patchVpaStatus(vpaClient vpa_api.VerticalPodAutoscalerInterface, vpaName string, patches []patchRecord) (result *vpa_types.VerticalPodAutoscaler, err error) {
 	bytes, err := json.Marshal(patches)
 	if err != nil {
@@ -228,13 +230,20 @@ func GetContainerControlledValues(name string, vpaResourcePolicy *vpa_types.PodR
 	return *containerPolicy.ControlledValues
 }
 
-// GetContainerPruningGracePeriod returns the pruning grace period for a container.
-func GetContainerPruningGracePeriod(containerName string, vpaResourcePolicy *vpa_types.PodResourcePolicy) (gracePeriod *meta.Duration) {
-	containerPolicy := GetContainerResourcePolicy(containerName, vpaResourcePolicy)
-	if containerPolicy != nil {
-		gracePeriod = containerPolicy.PruningGracePeriod
+// ParsePruningGracePeriodFromAnnotations returns the pruning grace period from a pod's annotations.
+func ParsePruningGracePeriodFromAnnotations(annotations map[string]string) (gracePeriod *time.Duration) {
+	if annotations == nil {
+		return nil
 	}
-	return
+	if gracePeriodStr, ok := annotations[VpaPruningGracePeriodAnnotation]; ok {
+		parsed, err := time.ParseDuration(gracePeriodStr)
+		if err != nil {
+			klog.V(4).ErrorS(err, "Failed to parse pruning grace period", "gracePeriod", gracePeriodStr)
+			return nil
+		}
+		return &parsed
+	}
+	return nil
 }
 
 // CreateOrUpdateVpaCheckpoint updates the status field of the VPA Checkpoint API object.