feat(recommender): add round memory bytes (#8298)

* feat(recommender): add round memory bytes

* bug(recommender): treat memory as BinarySi

* toc

Author: m-messiah

vertical-pod-autoscaler/docs/features.md

@@ -5,6 +5,7 @@
 - [Limits control](#limits-control)
 - [Memory Value Humanization](#memory-value-humanization)
 - [CPU Recommendation Rounding](#cpu-recommendation-rounding)
+- [Memory Recommendation Rounding](#memory-recommendation-rounding)
 - [In-Place Updates](#in-place-updates-inplaceorrecreate)
 
 ## Limits control
@@ -54,6 +55,22 @@ To enable this feature, set the --round-cpu-millicores flag when running the VPA
 --round-cpu-millicores=50
 ```
 
+## Memory Recommendation Rounding
+
+VPA can provide Memory recommendations rounded up to user-specified values, making it easier to interpret and configure resources. This feature is controlled by the `--round-memory-bytes` flag in the recommender component.
+
+When enabled, Memory recommendations will be:
+- Rounded up to the nearest multiple of the specified bytes value
+- Applied to target, lower bound, and upper bound recommendations
+
+For example, with `--round-memory-bytes=134217728`, a memory recommendation of `200Mi` would be rounded up to `256Mi`, and a recommendation of `80Mi` would be rounded up to `128Mi`.
+
+To enable this feature, set the `--round-memory-bytes` flag when running the VPA recommender:
+
+```bash
+--round-memory-bytes=134217728
+```
+
 ## In-Place Updates (`InPlaceOrRecreate`)
 
 > [!WARNING] 

vertical-pod-autoscaler/docs/flags.md

@@ -118,6 +118,7 @@ This document is auto-generated from the flag definitions in the VPA recommender
 | `recommender-interval` |  |  1m0s | duration                          How often metrics should be fetched  |
 | `recommender-name` | string |  "default" | Set the recommender name. Recommender will generate recommendations for VPAs that configure the same recommender name. If the recommender name is left as default it will also generate recommendations that don't explicitly specify recommender. You shouldn't run two recommenders with the same name in a cluster.  |
 | `round-cpu-millicores` | int |  1 | CPU recommendation rounding factor in millicores. The CPU value will always be rounded up to the nearest multiple of this factor.  |
+| `round-memory-bytes` | int |  1 | Memory recommendation rounding factor in bytes. The Memory value will always be rounded up to the nearest multiple of this factor.  |
 | `skip-headers` |  |  | If true, avoid header prefixes in the log messages |
 | `skip-log-headers` |  |  | If true, avoid headers when opening log files (no effect when -logtostderr=true) |
 | `stderrthreshold` | severity | : info | set the log level threshold for writing to standard error  |

vertical-pod-autoscaler/pkg/recommender/logic/recommender.go

@@ -39,6 +39,7 @@ var (
 	confidenceIntervalMemory   = flag.Duration("confidence-interval-memory", time.Hour*24, "The time interval used for computing the confidence multiplier for the memory lower and upper bound. Default: 24h")
 	humanizeMemory             = flag.Bool("humanize-memory", false, "Convert memory values in recommendations to the highest appropriate SI unit with up to 2 decimal places for better readability.")
 	roundCPUMillicores         = flag.Int("round-cpu-millicores", 1, `CPU recommendation rounding factor in millicores. The CPU value will always be rounded up to the nearest multiple of this factor.`)
+	roundMemoryBytes           = flag.Int("round-memory-bytes", 1, `Memory recommendation rounding factor in bytes. The Memory value will always be rounded up to the nearest multiple of this factor.`)
 )
 
 // PodResourceRecommender computes resource recommendation for a Vpa object.
@@ -193,10 +194,10 @@ func MapToListOfRecommendedContainerResources(resources RecommendedPodResources)
 	for _, name := range containerNames {
 		containerResources = append(containerResources, vpa_types.RecommendedContainerResources{
 			ContainerName:  name,
-			Target:         model.ResourcesAsResourceList(resources[name].Target, *humanizeMemory, *roundCPUMillicores),
-			LowerBound:     model.ResourcesAsResourceList(resources[name].LowerBound, *humanizeMemory, *roundCPUMillicores),
-			UpperBound:     model.ResourcesAsResourceList(resources[name].UpperBound, *humanizeMemory, *roundCPUMillicores),
-			UncappedTarget: model.ResourcesAsResourceList(resources[name].Target, *humanizeMemory, *roundCPUMillicores),
+			Target:         model.ResourcesAsResourceList(resources[name].Target, *humanizeMemory, *roundCPUMillicores, *roundMemoryBytes),
+			LowerBound:     model.ResourcesAsResourceList(resources[name].LowerBound, *humanizeMemory, *roundCPUMillicores, *roundMemoryBytes),
+			UpperBound:     model.ResourcesAsResourceList(resources[name].UpperBound, *humanizeMemory, *roundCPUMillicores, *roundMemoryBytes),
+			UncappedTarget: model.ResourcesAsResourceList(resources[name].Target, *humanizeMemory, *roundCPUMillicores, *roundMemoryBytes),
 		})
 	}
 	recommendation := &vpa_types.RecommendedPodResources{

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

@@ -73,7 +73,7 @@ func BytesFromMemoryAmount(memoryAmount ResourceAmount) float64 {
 
 // QuantityFromMemoryAmount converts memory ResourceAmount to a resource.Quantity.
 func QuantityFromMemoryAmount(memoryAmount ResourceAmount) resource.Quantity {
-	return *resource.NewScaledQuantity(int64(memoryAmount), 0)
+	return *resource.NewQuantity(int64(memoryAmount), resource.BinarySI)
 }
 
 // ScaleResource returns the resource amount multiplied by a given factor.
@@ -82,7 +82,7 @@ func ScaleResource(amount ResourceAmount, factor float64) ResourceAmount {
 }
 
 // ResourcesAsResourceList converts internal Resources representation to ResourcesList.
-func ResourcesAsResourceList(resources Resources, humanizeMemory bool, roundCPUMillicores int) apiv1.ResourceList {
+func ResourcesAsResourceList(resources Resources, humanizeMemory bool, roundCPUMillicores, roundMemoryBytes int) apiv1.ResourceList {
 	result := make(apiv1.ResourceList)
 	for key, resourceAmount := range resources {
 		var newKey apiv1.ResourceName
@@ -103,6 +103,15 @@ func ResourcesAsResourceList(resources Resources, humanizeMemory bool, roundCPUM
 		case ResourceMemory:
 			newKey = apiv1.ResourceMemory
 			quantity = QuantityFromMemoryAmount(resourceAmount)
+			if roundMemoryBytes != 1 && !quantity.IsZero() {
+				roundedValues, err := RoundUpToScale(resourceAmount, roundMemoryBytes)
+				if err != nil {
+					klog.V(4).InfoS("Error rounding memory value; leaving unchanged", "rawValue", resourceAmount, "scale", roundMemoryBytes, "error", err)
+				} else {
+					klog.V(4).InfoS("Successfully rounded memory value", "rawValue", resourceAmount, "roundedValue", roundedValues)
+				}
+				quantity = QuantityFromMemoryAmount(roundedValues)
+			}
 			if humanizeMemory && !quantity.IsZero() {
 				rawValues := quantity.Value()
 				humanizedValue := HumanizeMemoryQuantity(rawValues)

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

@@ -29,22 +29,24 @@ type ResourcesAsResourceListTestCase struct {
 	resources    Resources
 	humanize     bool
 	roundCPU     int
+	roundMemory  int
 	resourceList apiv1.ResourceList
 }
 
 func TestResourcesAsResourceList(t *testing.T) {
 	testCases := []ResourcesAsResourceListTestCase{
 		{
-			name: "basic resources without humanize and no cpu rounding",
+			name: "basic resources without humanize and no rounding",
 			resources: Resources{
 				ResourceCPU:    1000,
-				ResourceMemory: 1000,
+				ResourceMemory: 1024,
 			},
-			humanize: false,
-			roundCPU: 1,
+			humanize:    false,
+			roundCPU:    1,
+			roundMemory: 1,
 			resourceList: apiv1.ResourceList{
 				apiv1.ResourceCPU:    *resource.NewMilliQuantity(1000, resource.DecimalSI),
-				apiv1.ResourceMemory: *resource.NewQuantity(1000, resource.DecimalSI),
+				apiv1.ResourceMemory: *resource.NewQuantity(1024, resource.BinarySI),
 			},
 		},
 		{
@@ -53,21 +55,37 @@ func TestResourcesAsResourceList(t *testing.T) {
 				ResourceCPU:    1000,
 				ResourceMemory: 262144000, // 250Mi
 			},
-			humanize: true,
-			roundCPU: 1,
+			humanize:    true,
+			roundCPU:    1,
+			roundMemory: 1,
 			resourceList: apiv1.ResourceList{
 				apiv1.ResourceCPU:    *resource.NewMilliQuantity(1000, resource.DecimalSI),
 				apiv1.ResourceMemory: resource.MustParse("250.00Mi"),
 			},
 		},
+		{
+			name: "basic resources with humanize and memory rounding to 256Mi",
+			resources: Resources{
+				ResourceCPU:    1000,
+				ResourceMemory: 262144000, // 250Mi
+			},
+			humanize:    true,
+			roundCPU:    1,
+			roundMemory: 268435456,
+			resourceList: apiv1.ResourceList{
+				apiv1.ResourceCPU:    *resource.NewMilliQuantity(1000, resource.DecimalSI),
+				apiv1.ResourceMemory: resource.MustParse("256.00Mi"),
+			},
+		},
 		{
 			name: "large memory value with humanize and cpu rounding to 3",
 			resources: Resources{
 				ResourceCPU:    1000,
 				ResourceMemory: 839500000, // 800.61Mi
 			},
-			humanize: true,
-			roundCPU: 3,
+			humanize:    true,
+			roundCPU:    3,
+			roundMemory: 1,
 			resourceList: apiv1.ResourceList{
 				apiv1.ResourceCPU:    *resource.NewMilliQuantity(1002, resource.DecimalSI),
 				apiv1.ResourceMemory: resource.MustParse("800.61Mi"),
@@ -79,11 +97,12 @@ func TestResourcesAsResourceList(t *testing.T) {
 				ResourceCPU:    0,
 				ResourceMemory: 0,
 			},
-			humanize: false,
-			roundCPU: 2,
+			humanize:    false,
+			roundCPU:    2,
+			roundMemory: 1,
 			resourceList: apiv1.ResourceList{
 				apiv1.ResourceCPU:    *resource.NewMilliQuantity(0, resource.DecimalSI),
-				apiv1.ResourceMemory: *resource.NewQuantity(0, resource.DecimalSI),
+				apiv1.ResourceMemory: *resource.NewQuantity(0, resource.BinarySI),
 			},
 		},
 		{
@@ -92,17 +111,32 @@ func TestResourcesAsResourceList(t *testing.T) {
 				ResourceCPU:    1231241,
 				ResourceMemory: 839500000,
 			},
-			humanize: false,
-			roundCPU: 13,
+			humanize:    false,
+			roundCPU:    13,
+			roundMemory: 1,
+			resourceList: apiv1.ResourceList{
+				apiv1.ResourceCPU:    *resource.NewMilliQuantity(1231243, resource.DecimalSI),
+				apiv1.ResourceMemory: *resource.NewQuantity(839500000, resource.BinarySI),
+			},
+		},
+		{
+			name: "large memory value without humanize and with memory rounding to 128Mi",
+			resources: Resources{
+				ResourceCPU:    1231241,
+				ResourceMemory: 839500000,
+			},
+			humanize:    false,
+			roundCPU:    13,
+			roundMemory: 134217728,
 			resourceList: apiv1.ResourceList{
 				apiv1.ResourceCPU:    *resource.NewMilliQuantity(1231243, resource.DecimalSI),
-				apiv1.ResourceMemory: *resource.NewQuantity(839500000, resource.DecimalSI),
+				apiv1.ResourceMemory: resource.MustParse("896Mi"),
 			},
 		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			result := ResourcesAsResourceList(tc.resources, tc.humanize, tc.roundCPU)
+			result := ResourcesAsResourceList(tc.resources, tc.humanize, tc.roundCPU, tc.roundMemory)
 			if !result[apiv1.ResourceCPU].Equal(tc.resourceList[apiv1.ResourceCPU]) {
 				t.Errorf("expected %v, got %v", tc.resourceList[apiv1.ResourceCPU], result[apiv1.ResourceCPU])
 			}
@@ -503,42 +537,42 @@ func TestQuantityFromMemoryAmount(t *testing.T) {
 		{
 			name:         "should get 69",
 			memoryAmount: 69,
-			want:         *resource.NewQuantity(69, resource.DecimalSI),
+			want:         *resource.NewQuantity(69, resource.BinarySI),
 		},
 		{
 			name:         "should get 12",
 			memoryAmount: 12,
-			want:         *resource.NewQuantity(12, resource.DecimalSI),
+			want:         *resource.NewQuantity(12, resource.BinarySI),
 		},
 		{
 			name:         "should get 17",
 			memoryAmount: 17,
-			want:         *resource.NewQuantity(17, resource.DecimalSI),
+			want:         *resource.NewQuantity(17, resource.BinarySI),
 		},
 		{
 			name:         "should get 4",
 			memoryAmount: 4,
-			want:         *resource.NewQuantity(4, resource.DecimalSI),
+			want:         *resource.NewQuantity(4, resource.BinarySI),
 		},
 		{
 			name:         "should get 12",
 			memoryAmount: 12,
-			want:         *resource.NewQuantity(12, resource.DecimalSI),
+			want:         *resource.NewQuantity(12, resource.BinarySI),
 		},
 		{
 			name:         "should get 1",
 			memoryAmount: 1,
-			want:         *resource.NewQuantity(1, resource.DecimalSI),
+			want:         *resource.NewQuantity(1, resource.BinarySI),
 		},
 		{
 			name:         "should get 0",
 			memoryAmount: 0,
-			want:         *resource.NewQuantity(0, resource.DecimalSI),
+			want:         *resource.NewQuantity(0, resource.BinarySI),
 		},
 		{
 			name:         "should get 123456789",
 			memoryAmount: 123456789,
-			want:         *resource.NewQuantity(123456789, resource.DecimalSI),
+			want:         *resource.NewQuantity(123456789, resource.BinarySI),
 		},
 	}
 	for _, tc := range tc {