feat(recommender): add OOMMinBumpUp&OOMBumpUpRatio to CRD

omerap12 · omerap12 · commit 5e23c1d073bf · 2025-04-07T15:36:09.000Z
Signed-off-by: Omer Aplatony &lt;omerap12@gmail.com&gt;
diff --git a/vertical-pod-autoscaler/deploy/vpa-v1-crd-gen.yaml b/vertical-pod-autoscaler/deploy/vpa-v1-crd-gen.yaml
@@ -4,7 +4,7 @@ kind: CustomResourceDefinition
 metadata:
   annotations:
     api-approved.kubernetes.io: https://github.com/kubernetes/kubernetes/pull/63797
-    controller-gen.kubebuilder.io/version: v0.16.5
+    controller-gen.kubebuilder.io/version: v0.17.2
   name: verticalpodautoscalercheckpoints.autoscaling.k8s.io
 spec:
   group: autoscaling.k8s.io
@@ -225,7 +225,7 @@ kind: CustomResourceDefinition
 metadata:
   annotations:
     api-approved.kubernetes.io: https://github.com/kubernetes/kubernetes/pull/63797
-    controller-gen.kubebuilder.io/version: v0.16.5
+    controller-gen.kubebuilder.io/version: v0.17.2
   name: verticalpodautoscalers.autoscaling.k8s.io
 spec:
   group: autoscaling.k8s.io
@@ -254,6 +254,9 @@ spec:
     - jsonPath: .metadata.creationTimestamp
       name: Age
       type: date
+    - jsonPath: .spec.recommenderConfig
+      name: RecommenderConfig
+      type: string
     name: v1
     schema:
       openAPIV3Schema:
@@ -284,6 +287,21 @@ spec:
               Specification of the behavior of the autoscaler.
               More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status.
             properties:
+              recommenderConfig:
+                description: RecommenderConfig specifies the configuration for the
+                  recommender.
+                properties:
+                  oomBumpUpRatio:
+                    description: OOMBumpUpRatio is the ratio to increase resources
+                      when OOM is detected.
+                    minimum: 1
+                    type: number
+                  oomMinBumpUp:
+                    description: OOMMinBumpUp is the minimum increase in resources
+                      when OOM is detected.
+                    minimum: 0
+                    type: number
+                type: object
               recommenders:
                 description: |-
                   Recommender responsible for generating recommendation for this object.
diff --git a/vertical-pod-autoscaler/docs/api.md b/vertical-pod-autoscaler/docs/api.md
@@ -201,6 +201,23 @@ _Appears in:_
 | `containerRecommendations` _[RecommendedContainerResources](#recommendedcontainerresources) array_ | Resources recommended by the autoscaler for each container. |  |  |
 
 
+#### RecommenderConfig
+
+
+
+RecommenderConfig contains configuration for the recommender.
+
+
+
+_Appears in:_
+- [VerticalPodAutoscalerSpec](#verticalpodautoscalerspec)
+
+| Field | Description | Default | Validation |
+| --- | --- | --- | --- |
+| `oomBumpUpRatio` _float_ | OOMBumpUpRatio is the ratio to increase resources when OOM is detected. |  | Minimum: 1 <br /> |
+| `oomMinBumpUp` _float_ | OOMMinBumpUp is the minimum increase in resources when OOM is detected. |  | Minimum: 0 <br /> |
+
+
 #### UpdateMode
 
 _Underlying type:_ _string_
@@ -376,6 +393,7 @@ _Appears in:_
 | `updatePolicy` _[PodUpdatePolicy](#podupdatepolicy)_ | Describes the rules on how changes are applied to the pods.<br />If not specified, all fields in the `PodUpdatePolicy` are set to their<br />default values. |  |  |
 | `resourcePolicy` _[PodResourcePolicy](#podresourcepolicy)_ | Controls how the autoscaler computes recommended resources.<br />The resource policy may be used to set constraints on the recommendations<br />for individual containers.<br />If any individual containers need to be excluded from getting the VPA recommendations, then<br />it must be disabled explicitly by setting mode to "Off" under containerPolicies.<br />If not specified, the autoscaler computes recommended resources for all containers in the pod,<br />without additional constraints. |  |  |
 | `recommenders` _[VerticalPodAutoscalerRecommenderSelector](#verticalpodautoscalerrecommenderselector) array_ | Recommender responsible for generating recommendation for this object.<br />List should be empty (then the default recommender will generate the<br />recommendation) or contain exactly one recommender. |  |  |
+| `recommenderConfig` _[RecommenderConfig](#recommenderconfig)_ | RecommenderConfig specifies the configuration for the recommender. |  |  |
 
 
 #### VerticalPodAutoscalerStatus
diff --git a/vertical-pod-autoscaler/docs/flags.md b/vertical-pod-autoscaler/docs/flags.md
@@ -93,8 +93,8 @@ This document is auto-generated from the flag definitions in the VPA recommender
 | `--metric-for-pod-labels` | "up{job=\"kubernetes-pods\"}" |                           Which metric to look for pod labels in metrics |
 | `--min-checkpoints` | 10 |                                    Minimum number of checkpoints to write per recommender's main loop |
 | `--one-output` |  |                                             If true, only write logs to their native severity level (vs also writing to each lower severity level; no effect when -logtostderr=true) |
-| `--oom-bump-up-ratio` | 1.2 |                                The memory bump up ratio when OOM occurred, default is 1.2. |
-| `--oom-min-bump-up-bytes` | 1.048576e+08 |                            The minimal increase of memory when OOM occurred in bytes, default is 100 * 1024 * 1024 |
+| `--oom-bump-up-ratio` | 1.2 |                                Default memory bump up ratio when OOM occurs. This value applies to all VPAs unless overridden in the VPA spec. Default is 1.2. |
+| `--oom-min-bump-up-bytes` | 1.048576e+08 |                            Default minimal increase of memory (in bytes) when OOM occurs. This value applies to all VPAs unless overridden in the VPA spec. Default is 100 * 1024 * 1024 (100Mi). |
 | `--password` |  |                                        The password used in the prometheus server basic auth |
 | `--pod-label-prefix` | "pod_label_" |                                Which prefix to look for pod labels in metrics |
 | `--pod-name-label` | "kubernetes_pod_name" |                                  Label name to look for pod names |
diff --git a/vertical-pod-autoscaler/e2e/v1/admission_controller.go b/vertical-pod-autoscaler/e2e/v1/admission_controller.go
@@ -831,26 +831,137 @@ var _ = AdmissionControllerE2eDescribe("Admission-controller", func() {
 		err := InstallRawVPA(f, validVPA)
 		gomega.Expect(err).NotTo(gomega.HaveOccurred(), "Valid VPA object rejected")
 
-		ginkgo.By("Setting up invalid VPA object")
-		// The invalid object differs by name and minAllowed - there is an invalid "requests" field.
-		invalidVPA := []byte(`{
-			"kind": "VerticalPodAutoscaler",
-			"apiVersion": "autoscaling.k8s.io/v1",
-			"metadata": {"name": "hamster-vpa-invalid"},
-			"spec": {
-				"targetRef": {
-					"apiVersion": "apps/v1",
-					"kind": "Deployment",
-					"name":"hamster"
-				},
-		   	"resourcePolicy": {
-		  		"containerPolicies": [{"containerName": "*", "minAllowed":{"requests":{"cpu":"50m"}}}]
-		  	}
-		  }
-		}`)
-		err2 := InstallRawVPA(f, invalidVPA)
-		gomega.Expect(err2).To(gomega.HaveOccurred(), "Invalid VPA object accepted")
-		gomega.Expect(err2.Error()).To(gomega.MatchRegexp(`.*admission webhook .*vpa.* denied the request: .*`))
+		ginkgo.By("Setting up invalid VPA objects")
+		testCases := []struct {
+			name        string
+			vpaJSON     string
+			expectedErr string
+		}{
+			{
+				name: "Invalid oomBumpUpRatio (negative value)",
+				vpaJSON: `{
+                "apiVersion": "autoscaling.k8s.io/v1",
+                "kind": "VerticalPodAutoscaler",
+                "metadata": {"name": "oom-test-vpa"},
+                "spec": {
+                    "targetRef": {
+                        "apiVersion": "apps/v1",
+                        "kind": "Deployment",
+                        "name": "oom-test"
+                    },
+                    "updatePolicy": {
+                        "updateMode": "Auto"
+                    },
+                    "recommenderConfig": {
+                        "oomBumpUpRatio": -1,
+                        "oomMinBumpUp": 104857600
+                    }
+                }
+            }`,
+				expectedErr: "spec.recommenderConfig.oomBumpUpRatio: Invalid value: -1: spec.recommenderConfig.oomBumpUpRatio in body should be greater than or equal to 1",
+			},
+			{
+				name: "Invalid oomBumpUpRatio (string value)",
+				vpaJSON: `{
+                "apiVersion": "autoscaling.k8s.io/v1",
+                "kind": "VerticalPodAutoscaler",
+                "metadata": {"name": "oom-test-vpa"},
+                "spec": {
+                    "targetRef": {
+                        "apiVersion": "apps/v1",
+                        "kind": "Deployment",
+                        "name": "oom-test"
+                    },
+                    "updatePolicy": {
+                        "updateMode": "Auto"
+                    },
+                    "recommenderConfig": {
+                        "oomBumpUpRatio": "12",
+                        "oomMinBumpUp": 104857600
+                    }
+                }
+            }`,
+				expectedErr: "json: cannot unmarshal string into Go struct field RecommenderConfig.spec.recommenderConfig.oomBumpUpRatio of type float64",
+			},
+			{
+				name: "Invalid oomBumpUpRatio (less than 1)",
+				vpaJSON: `{
+                "apiVersion": "autoscaling.k8s.io/v1",
+                "kind": "VerticalPodAutoscaler",
+                "metadata": {"name": "oom-test-vpa"},
+                "spec": {
+                    "targetRef": {
+                        "apiVersion": "apps/v1",
+                        "kind": "Deployment",
+                        "name": "oom-test"
+                    },
+                    "updatePolicy": {
+                        "updateMode": "Auto"
+                    },
+                    "recommenderConfig": {
+                        "oomBumpUpRatio": 0.5,
+                        "oomMinBumpUp": 104857600
+                    }
+                }
+            }`,
+				expectedErr: "spec.recommenderConfig.oomBumpUpRatio: Invalid value: 0.5: spec.recommenderConfig.oomBumpUpRatio in body should be greater than or equal to 1",
+			},
+			{
+				name: "Invalid oomMinBumpUp (negative value)",
+				vpaJSON: `{
+                "apiVersion": "autoscaling.k8s.io/v1",
+                "kind": "VerticalPodAutoscaler",
+                "metadata": {"name": "oom-test-vpa"},
+                "spec": {
+                    "targetRef": {
+                        "apiVersion": "apps/v1",
+                        "kind": "Deployment",
+                        "name": "oom-test"
+                    },
+                    "updatePolicy": {
+                        "updateMode": "Auto"
+                    },
+                    "recommenderConfig": {
+                        "oomBumpUpRatio": 2,
+                        "oomMinBumpUp": -1
+                    }
+                }
+            }`,
+				expectedErr: "spec.recommenderConfig.oomMinBumpUp: Invalid value: -1: spec.recommenderConfig.oomMinBumpUp in body should be greater than or equal to 0",
+			},
+			{
+				name: "Invalid minAllowed (invalid requests field)",
+				vpaJSON: `{
+                "apiVersion": "autoscaling.k8s.io/v1",
+                "kind": "VerticalPodAutoscaler",
+                "metadata": {"name": "hamster-vpa-invalid"},
+                "spec": {
+                    "targetRef": {
+                        "apiVersion": "apps/v1",
+                        "kind": "Deployment",
+                        "name": "hamster"
+                    },
+                    "resourcePolicy": {
+                        "containerPolicies": [{
+                            "containerName": "*",
+                            "minAllowed": {
+                                "requests": {
+                                    "cpu": "50m"
+                                }
+                            }
+                        }]
+                    }
+                }
+            }`,
+				expectedErr: "admission webhook .*vpa.* denied the request:",
+			},
+		}
+		for _, tc := range testCases {
+			ginkgo.By(fmt.Sprintf("Testing %s", tc.name))
+			err := InstallRawVPA(f, []byte(tc.vpaJSON))
+			gomega.Expect(err).To(gomega.HaveOccurred(), "Invalid VPA object accepted")
+			gomega.Expect(err.Error()).To(gomega.MatchRegexp(tc.expectedErr))
+		}
 	})
 
 	ginkgo.It("reloads the webhook leaf and CA certificate", func(ctx ginkgo.SpecContext) {
diff --git a/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go b/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go
@@ -45,6 +45,7 @@ type VerticalPodAutoscalerList struct {
 // +kubebuilder:printcolumn:name="Mem",type="string",JSONPath=".status.recommendation.containerRecommendations[0].target.memory"
 // +kubebuilder:printcolumn:name="Provided",type="string",JSONPath=".status.conditions[?(@.type=='RecommendationProvided')].status"
 // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
+// +kubebuilder:printcolumn:name="RecommenderConfig",type="string",JSONPath=".spec.recommenderConfig"
 // +kubebuilder:metadata:annotations="api-approved.kubernetes.io=https://github.com/kubernetes/kubernetes/pull/63797"
 
 // VerticalPodAutoscaler is the configuration for a vertical pod
@@ -107,6 +108,23 @@ type VerticalPodAutoscalerSpec struct {
 	// recommendation) or contain exactly one recommender.
 	// +optional
 	Recommenders []*VerticalPodAutoscalerRecommenderSelector `json:"recommenders,omitempty" protobuf:"bytes,4,opt,name=recommenders"`
+
+	// RecommenderConfig specifies the configuration for the recommender.
+	// +optional
+	RecommenderConfig *RecommenderConfig `json:"recommenderConfig,omitempty" protobuf:"bytes,5,opt,name=recommenderConfig"`
+}
+
+// RecommenderConfig contains configuration for the recommender.
+type RecommenderConfig struct {
+	// OOMBumpUpRatio is the ratio to increase resources when OOM is detected.
+	// +kubebuilder:validation:Minimum=1.0
+	// +optional
+	OOMBumpUpRatio *float64 `json:"oomBumpUpRatio,omitempty" protobuf:"bytes,1,opt,name=oomBumpUpRatio"`
+
+	// OOMMinBumpUp is the minimum increase in resources when OOM is detected.
+	// +kubebuilder:validation:Minimum=0
+	// +optional
+	OOMMinBumpUp *float64 `json:"oomMinBumpUp,omitempty" protobuf:"bytes,2,opt,name=oomMinBumpUp"`
 }
 
 // EvictionChangeRequirement refers to the relationship between the new target recommendation for a Pod and its current requests, what kind of change is necessary for the Pod to be evicted
diff --git a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go
@@ -524,7 +524,14 @@ Loop:
 		select {
 		case oomInfo := <-feeder.oomChan:
 			klog.V(3).InfoS("OOM detected", "oomInfo", oomInfo)
-			if err = feeder.clusterState.RecordOOM(oomInfo.ContainerID, oomInfo.Timestamp, oomInfo.Memory); err != nil {
+			pod := feeder.clusterState.Pods()[oomInfo.ContainerID.PodID]
+			controlledVpa := feeder.clusterState.GetControllingVPA(pod)
+			var oomBumpUpRatio, oomMinBumpUp *float64
+			if controlledVpa != nil && controlledVpa.RecommenderConfig != nil {
+				oomBumpUpRatio = controlledVpa.RecommenderConfig.OOMBumpUpRatio
+				oomMinBumpUp = controlledVpa.RecommenderConfig.OOMMinBumpUp
+			}
+			if err = feeder.clusterState.RecordOOM(oomInfo.ContainerID, oomInfo.Timestamp, oomInfo.Memory, oomBumpUpRatio, oomMinBumpUp); err != nil {
 				klog.V(0).InfoS("Failed to record OOM", "oomInfo", oomInfo, "error", err)
 			}
 		default:
diff --git a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go
@@ -600,11 +600,12 @@ type fakeMetricsClient struct {
 	snapshots []*metrics.ContainerMetricsSnapshot
 }
 
-func (m fakeMetricsClient) GetContainersMetrics() ([]*metrics.ContainerMetricsSnapshot, error) {
+func (m fakeMetricsClient) GetContainersMetrics(_ context.Context) ([]*metrics.ContainerMetricsSnapshot, error) {
 	return m.snapshots, nil
 }
 
 func TestClusterStateFeeder_LoadRealTimeMetrics(t *testing.T) {
+	_, tctx := ktesting.NewTestContext(t)
 	namespaceName := "test-namespace"
 	podID := model.PodID{Namespace: namespaceName, PodName: "Pod"}
 	regularContainer1 := model.ContainerID{PodID: podID, ContainerName: "Container1"}
@@ -639,7 +640,7 @@ func TestClusterStateFeeder_LoadRealTimeMetrics(t *testing.T) {
 		metricsClient:  fakeMetricsClient{snapshots: containerMetricsSnapshots},
 	}
 
-	feeder.LoadRealTimeMetrics()
+	feeder.LoadRealTimeMetrics(tctx)
 
 	assert.Equal(t, 2, len(clusterState.addedSamples))
 
diff --git a/vertical-pod-autoscaler/pkg/recommender/main.go b/vertical-pod-autoscaler/pkg/recommender/main.go
@@ -96,8 +96,8 @@ var (
 	memoryAggregationIntervalCount = flag.Int64("memory-aggregation-interval-count", model.DefaultMemoryAggregationIntervalCount, `The number of consecutive memory-aggregation-intervals which make up the MemoryAggregationWindowLength which in turn is the period for memory usage aggregation by VPA. In other words, MemoryAggregationWindowLength = memory-aggregation-interval * memory-aggregation-interval-count.`)
 	memoryHistogramDecayHalfLife   = flag.Duration("memory-histogram-decay-half-life", model.DefaultMemoryHistogramDecayHalfLife, `The amount of time it takes a historical memory usage sample to lose half of its weight. In other words, a fresh usage sample is twice as 'important' as one with age equal to the half life period.`)
 	cpuHistogramDecayHalfLife      = flag.Duration("cpu-histogram-decay-half-life", model.DefaultCPUHistogramDecayHalfLife, `The amount of time it takes a historical CPU usage sample to lose half of its weight.`)
-	oomBumpUpRatio                 = flag.Float64("oom-bump-up-ratio", model.DefaultOOMBumpUpRatio, `The memory bump up ratio when OOM occurred, default is 1.2.`)
-	oomMinBumpUp                   = flag.Float64("oom-min-bump-up-bytes", model.DefaultOOMMinBumpUp, `The minimal increase of memory when OOM occurred in bytes, default is 100 * 1024 * 1024`)
+	oomBumpUpRatio                 = flag.Float64("oom-bump-up-ratio", model.DefaultOOMBumpUpRatio, `Default memory bump up ratio when OOM occurs. This value applies to all VPAs unless overridden in the VPA spec. Default is 1.2.`)
+	oomMinBumpUp                   = flag.Float64("oom-min-bump-up-bytes", model.DefaultOOMMinBumpUp, `Default minimal increase of memory (in bytes) when OOM occurs. This value applies to all VPAs unless overridden in the VPA spec. Default is 100 * 1024 * 1024 (100Mi).`)
 )
 
 // Post processors flags
diff --git a/vertical-pod-autoscaler/pkg/recommender/model/cluster.go b/vertical-pod-autoscaler/pkg/recommender/model/cluster.go
@@ -47,7 +47,7 @@ type ClusterState interface {
 	DeletePod(podID PodID)
 	AddOrUpdateContainer(containerID ContainerID, request Resources) error
 	AddSample(sample *ContainerUsageSampleWithKey) error
-	RecordOOM(containerID ContainerID, timestamp time.Time, requestedMemory ResourceAmount) error
+	RecordOOM(containerID ContainerID, timestamp time.Time, requestedMemory ResourceAmount, OOMBumpUpRatio *float64, OOMMinBumpUp *float64) error
 	AddOrUpdateVpa(apiObject *vpa_types.VerticalPodAutoscaler, selector labels.Selector) error
 	DeleteVpa(vpaID VpaID) error
 	MakeAggregateStateKey(pod *PodState, containerName string) AggregateStateKey
@@ -250,7 +250,7 @@ func (cluster *clusterState) AddSample(sample *ContainerUsageSampleWithKey) erro
 }
 
 // RecordOOM adds info regarding OOM event in the model as an artificial memory sample.
-func (cluster *clusterState) RecordOOM(containerID ContainerID, timestamp time.Time, requestedMemory ResourceAmount) error {
+func (cluster *clusterState) RecordOOM(containerID ContainerID, timestamp time.Time, requestedMemory ResourceAmount, OOMBumpUpRatio *float64, OOMMinBumpUp *float64) error {
 	pod, podExists := cluster.pods[containerID.PodID]
 	if !podExists {
 		return NewKeyError(containerID.PodID)
@@ -259,7 +259,7 @@ func (cluster *clusterState) RecordOOM(containerID ContainerID, timestamp time.T
 	if !containerExists {
 		return NewKeyError(containerID.ContainerName)
 	}
-	err := containerState.RecordOOM(timestamp, requestedMemory)
+	err := containerState.RecordOOM(timestamp, requestedMemory, OOMBumpUpRatio, OOMMinBumpUp)
 	if err != nil {
 		return fmt.Errorf("error while recording OOM for %v, Reason: %v", containerID, err)
 	}
@@ -305,6 +305,7 @@ func (cluster *clusterState) AddOrUpdateVpa(apiObject *vpa_types.VerticalPodAuto
 	vpa.Recommendation = currentRecommendation
 	vpa.SetUpdateMode(apiObject.Spec.UpdatePolicy)
 	vpa.SetResourcePolicy(apiObject.Spec.ResourcePolicy)
+	vpa.SetRecommenderConfig(apiObject.Spec.RecommenderConfig)
 	vpa.SetAPIVersion(apiObject.GetObjectKind().GroupVersionKind().Version)
 	return nil
 }
diff --git a/vertical-pod-autoscaler/pkg/recommender/model/cluster_test.go b/vertical-pod-autoscaler/pkg/recommender/model/cluster_test.go
diff --git a/vertical-pod-autoscaler/pkg/recommender/model/container.go b/vertical-pod-autoscaler/pkg/recommender/model/container.go
diff --git a/vertical-pod-autoscaler/pkg/recommender/model/container_test.go b/vertical-pod-autoscaler/pkg/recommender/model/container_test.go
diff --git a/vertical-pod-autoscaler/pkg/recommender/model/vpa.go b/vertical-pod-autoscaler/pkg/recommender/model/vpa.go

Original file line number	Diff line number	Diff line change
`@@ -600,11 +600,12 @@ type fakeMetricsClient struct {`
`600`	`600`	`snapshots []*metrics.ContainerMetricsSnapshot`
`601`	`601`	`}`
`602`	`602`
`603`		`-func (m fakeMetricsClient) GetContainersMetrics() ([]*metrics.ContainerMetricsSnapshot, error) {`
	`603`	`+func (m fakeMetricsClient) GetContainersMetrics(_ context.Context) ([]*metrics.ContainerMetricsSnapshot, error) {`
`604`	`604`	`return m.snapshots, nil`
`605`	`605`	`}`
`606`	`606`
`607`	`607`	`func TestClusterStateFeeder_LoadRealTimeMetrics(t *testing.T) {`
	`608`	`+ _, tctx := ktesting.NewTestContext(t)`
`608`	`609`	`namespaceName := "test-namespace"`
`609`	`610`	`podID := model.PodID{Namespace: namespaceName, PodName: "Pod"}`
`610`	`611`	`regularContainer1 := model.ContainerID{PodID: podID, ContainerName: "Container1"}`
`@@ -639,7 +640,7 @@ func TestClusterStateFeeder_LoadRealTimeMetrics(t *testing.T) {`
`639`	`640`	`metricsClient: fakeMetricsClient{snapshots: containerMetricsSnapshots},`
`640`	`641`	`}`
`641`	`642`
`642`		`- feeder.LoadRealTimeMetrics()`
	`643`	`+ feeder.LoadRealTimeMetrics(tctx)`
`643`	`644`
`644`	`645`	`assert.Equal(t, 2, len(clusterState.addedSamples))`
`645`	`646`