diff --git a/controllers/tortoise_controller_test.go b/controllers/tortoise_controller_test.go index 88326151..48507bf8 100644 --- a/controllers/tortoise_controller_test.go +++ b/controllers/tortoise_controller_test.go @@ -254,7 +254,7 @@ func startController(ctx context.Context) func() { VpaService: cli, DeploymentService: deployment.New(mgr.GetClient(), "100m", "100Mi", recorder), TortoiseService: tortoiseService, - RecommenderService: recommender.New(2.0, 0.5, 90, 40, 3, 30, "10m", "10Mi", map[string]string{"istio-proxy": "11m"}, map[string]string{"istio-proxy": "11Mi"}, "10", "10Gi", 10000, 0, []features.FeatureFlag{features.VerticalScalingBasedOnPreferredMaxReplicas}, recorder), + RecommenderService: recommender.New(2.0, 0.5, 90, 40, 3, 30, "10m", "10Mi", map[string]string{"istio-proxy": "11m"}, map[string]string{"istio-proxy": "11Mi"}, "10", "10Gi", 10000, 0, 0, []features.FeatureFlag{features.VerticalScalingBasedOnPreferredMaxReplicas}, recorder), } err = reconciler.SetupWithManager(mgr) Expect(err).ShouldNot(HaveOccurred()) diff --git a/main.go b/main.go index 48d852b7..fda64907 100644 --- a/main.go +++ b/main.go @@ -181,6 +181,7 @@ func main() { config.MaximumMemoryRequest, config.MaximumMaxReplicas, config.MaxAllowedScalingDownRatio, + config.BufferRatioOnVerticalResource, config.FeatureFlags, eventRecorder, ), diff --git a/pkg/config/config.go b/pkg/config/config.go index 4ab6321e..7dbf7ae8 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -190,6 +190,10 @@ type Config struct { MinimumCPURequestPerContainer map[string]string `yaml:"MinimumCPURequestPerContainer"` // MinimumMemoryRequest is the minimum memory bytes that the tortoise can give to the container resource request (default: 50Mi) MinimumMemoryRequest string `yaml:"MinimumMemoryRequest"` + // BufferRatioOnVerticalResource is the buffer ratio on vertical resource (default: 0.1) + // For example, if the recommendation from VPA is 100m, and BufferOnVerticalResource is 0.1, + // the tortoise will set the resource request to 110m. + BufferRatioOnVerticalResource float64 `yaml:"BufferRatioOnVerticalResource"` // MinimumMemoryRequestPerContainer is the minimum memory bytes per container that the tortoise can give to the container (default: nil) // If you specify both, the tortoise uses MinimumMemoryRequestPerContainer basically, but if the container name is not found in this map, the tortoise uses MinimumMemoryRequest. // @@ -290,6 +294,7 @@ func defaultConfig() *Config { IstioSidecarProxyDefaultMemory: "200Mi", MinimumCPULimit: "0", ResourceLimitMultiplier: map[string]int64{}, + BufferRatioOnVerticalResource: 0.1, } } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 005fa594..5d4f0e39 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -57,6 +57,7 @@ func TestParseConfig(t *testing.T) { "cpu": 3, "memory": 1, }, + BufferRatioOnVerticalResource: 0.2, }, }, { @@ -91,6 +92,7 @@ func TestParseConfig(t *testing.T) { MinimumCPURequestPerContainer: map[string]string{}, MinimumMemoryRequestPerContainer: map[string]string{}, ResourceLimitMultiplier: map[string]int64{}, + BufferRatioOnVerticalResource: 0.1, }, }, { @@ -132,6 +134,7 @@ func TestParseConfig(t *testing.T) { MinimumCPURequestPerContainer: map[string]string{}, MinimumMemoryRequestPerContainer: map[string]string{}, ResourceLimitMultiplier: map[string]int64{}, + BufferRatioOnVerticalResource: 0.1, }, }, } diff --git a/pkg/config/testdata/config.yaml b/pkg/config/testdata/config.yaml index 2f8139f3..198ed543 100644 --- a/pkg/config/testdata/config.yaml +++ b/pkg/config/testdata/config.yaml @@ -22,4 +22,5 @@ MaxAllowedScalingDownRatio: 0.5 ResourceLimitMultiplier: cpu: 3 memory: 1 -MinimumCPULimit: "1" \ No newline at end of file +MinimumCPULimit: "1" +BufferRatioOnVerticalResource: 0.2 \ No newline at end of file diff --git a/pkg/recommender/recommender.go b/pkg/recommender/recommender.go index 9f4fad25..138bbd1d 100644 --- a/pkg/recommender/recommender.go +++ b/pkg/recommender/recommender.go @@ -42,6 +42,8 @@ type Service struct { // For example, if the current resource request is 100m, the max allowed scaling down ratio is 0.8, // the minimum resource request that Tortoise can apply is 80m. maxAllowedScalingDownRatio float64 + + bufferRatioOnVerticalResource float64 } func New( @@ -59,6 +61,7 @@ func New( maxMemory string, maximumMaxReplica int32, maxAllowedScalingDownRatio float64, + bufferRatioOnVerticalResourceRecommendation float64, featureFlags []features.FeatureFlag, eventRecorder record.EventRecorder, ) *Service { @@ -91,9 +94,10 @@ func New( corev1.ResourceCPU: resource.MustParse(maxCPU), corev1.ResourceMemory: resource.MustParse(maxMemory), }, - maximumMaxReplica: maximumMaxReplica, - featureFlags: featureFlags, - maxAllowedScalingDownRatio: maxAllowedScalingDownRatio, + maximumMaxReplica: maximumMaxReplica, + featureFlags: featureFlags, + maxAllowedScalingDownRatio: maxAllowedScalingDownRatio, + bufferRatioOnVerticalResource: bufferRatioOnVerticalResourceRecommendation, } } @@ -216,8 +220,8 @@ func (s *Service) calculateBestNewSize( // It's the simplest case. // The user configures Vertical on this container's resource. This is just vertical scaling. // We always follow the recommendation from VPA. - newSize := recommendedResourceRequest.MilliValue() - jastified := s.justifyNewSize(resourceRequest.MilliValue(), newSize, k, minAllocatedResources, containerName) + newSize := float64(recommendedResourceRequest.MilliValue()) * (1 + s.bufferRatioOnVerticalResource) + jastified := s.justifyNewSize(resourceRequest.MilliValue(), int64(newSize), k, minAllocatedResources, containerName) return jastified, fmt.Sprintf("change %v request (%v) (%v → %v) based on VPA suggestion", k, containerName, resourceRequest.MilliValue(), jastified), tortoise, nil } diff --git a/pkg/recommender/recommender_test.go b/pkg/recommender/recommender_test.go index 2e45d285..ed5e553d 100644 --- a/pkg/recommender/recommender_test.go +++ b/pkg/recommender/recommender_test.go @@ -873,7 +873,7 @@ func TestUpdateRecommendation(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s := New(2.0, 0.5, 90, 40, 3, 30, "50m", "50Mi", map[string]string{"istio-proxy": "100m"}, map[string]string{"istio-proxy": "100m"}, "10", "10Gi", 1000, 0.5, nil, record.NewFakeRecorder(10)) + s := New(2.0, 0.5, 90, 40, 3, 30, "50m", "50Mi", map[string]string{"istio-proxy": "100m"}, map[string]string{"istio-proxy": "100m"}, "10", "10Gi", 1000, 0.5, 0, nil, record.NewFakeRecorder(10)) got, err := s.updateHPATargetUtilizationRecommendations(context.Background(), tt.args.tortoise, tt.args.hpa, tt.args.currentReplicaNum) if (err != nil) != tt.wantErr { t.Errorf("updateHPATargetUtilizationRecommendations() error = %v, wantErr %v", err, tt.wantErr) @@ -1553,7 +1553,7 @@ func Test_updateHPAMinMaxReplicasRecommendations(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s := New(2.0, 0.5, 90, 40, 3, 30, "50m", "50Mi", map[string]string{"istio-proxy": "100m"}, map[string]string{"istio-proxy": "100m"}, "10", "10Gi", 1000, 0.5, nil, record.NewFakeRecorder(10)) + s := New(2.0, 0.5, 90, 40, 3, 30, "50m", "50Mi", map[string]string{"istio-proxy": "100m"}, map[string]string{"istio-proxy": "100m"}, "10", "10Gi", 1000, 0.5, 0, nil, record.NewFakeRecorder(10)) got, err := s.updateHPAMinMaxReplicasRecommendations(tt.args.tortoise, tt.args.replicaNum, tt.args.now) if (err != nil) != tt.wantErr { t.Errorf("updateHPAMinMaxReplicasRecommendations() error = %v, wantErr %v", err, tt.wantErr) @@ -1569,12 +1569,13 @@ func Test_updateHPAMinMaxReplicasRecommendations(t *testing.T) { func TestService_UpdateVPARecommendation(t *testing.T) { now := time.Now() type fields struct { - preferredMaxReplicas int32 - minimumMinReplicas int32 - maxCPU string - maxMemory string - maxAllowedScalingDownRatio float64 - features []features.FeatureFlag + preferredMaxReplicas int32 + minimumMinReplicas int32 + maxCPU string + maxMemory string + bufferRatioOnVerticalResource float64 + maxAllowedScalingDownRatio float64 + features []features.FeatureFlag } type args struct { tortoise *v1beta3.Tortoise @@ -2443,6 +2444,84 @@ func TestService_UpdateVPARecommendation(t *testing.T) { }).Build(), wantErr: false, }, + { + name: "all vertical: reduce resources based on VPA recommendation plus bufferRatioOnVerticalResource", + fields: fields{ + preferredMaxReplicas: 6, + maxCPU: "1000m", + maxMemory: "1Gi", + bufferRatioOnVerticalResource: 0.1, + }, + args: args{ + hpa: &v2.HorizontalPodAutoscaler{ + Spec: v2.HorizontalPodAutoscalerSpec{ + MinReplicas: ptr.To[int32](1), + Metrics: []v2.MetricSpec{}, + }, + }, + tortoise: utils.NewTortoiseBuilder().AddAutoscalingPolicy(v1beta3.ContainerAutoscalingPolicy{ + ContainerName: "test-container", + Policy: map[corev1.ResourceName]v1beta3.AutoscalingType{ + corev1.ResourceCPU: v1beta3.AutoscalingTypeVertical, + corev1.ResourceMemory: v1beta3.AutoscalingTypeVertical, + }, + }).AddResourcePolicy(v1beta3.ContainerResourcePolicy{ + ContainerName: "test-container", + MinAllocatedResources: createResourceList("100m", "100Mi"), + }).AddContainerRecommendationFromVPA( + v1beta3.ContainerRecommendationFromVPA{ + ContainerName: "test-container", + Recommendation: map[corev1.ResourceName]v1beta3.ResourceQuantity{ + corev1.ResourceCPU: { + Quantity: resource.MustParse("120m"), + }, + corev1.ResourceMemory: { + Quantity: resource.MustParse("120Mi"), + }, + }, + }, + ).AddContainerResourceRequests(v1beta3.ContainerResourceRequests{ + ContainerName: "test-container", + Resource: createResourceList("130m", "130Mi"), + }).Build(), + replicaNum: 3, + }, + want: utils.NewTortoiseBuilder().AddAutoscalingPolicy(v1beta3.ContainerAutoscalingPolicy{ + ContainerName: "test-container", + Policy: map[corev1.ResourceName]v1beta3.AutoscalingType{ + corev1.ResourceCPU: v1beta3.AutoscalingTypeVertical, + corev1.ResourceMemory: v1beta3.AutoscalingTypeVertical, + }, + }).AddResourcePolicy(v1beta3.ContainerResourcePolicy{ + ContainerName: "test-container", + MinAllocatedResources: createResourceList("100m", "100Mi"), + }).AddContainerRecommendationFromVPA( + v1beta3.ContainerRecommendationFromVPA{ + ContainerName: "test-container", + Recommendation: map[corev1.ResourceName]v1beta3.ResourceQuantity{ + corev1.ResourceCPU: { + Quantity: resource.MustParse("120m"), + }, + corev1.ResourceMemory: { + Quantity: resource.MustParse("120Mi"), + }, + }, + }, + ).AddContainerResourceRequests(v1beta3.ContainerResourceRequests{ + ContainerName: "test-container", + Resource: createResourceList("130m", "130Mi"), + }).SetRecommendations(v1beta3.Recommendations{ + Vertical: v1beta3.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta3.RecommendedContainerResources{ + { + ContainerName: "test-container", + RecommendedResource: createResourceList("132m", "132Mi"), // VPA recommendation * (1 + bufferRatioOnVerticalResource) + }, + }, + }, + }).Build(), + wantErr: false, + }, { name: "all vertical: use MinAllocatedResources when VPA recommendation is smaller than MinAllocatedResources", fields: fields{ @@ -2928,7 +3007,7 @@ func TestService_UpdateVPARecommendation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - s := New(0, 0, 0, 0, int(tt.fields.minimumMinReplicas), int(tt.fields.preferredMaxReplicas), "5m", "5Mi", map[string]string{"istio-proxy": "7m"}, map[string]string{"istio-proxy": "7Mi"}, tt.fields.maxCPU, tt.fields.maxMemory, 10000, tt.fields.maxAllowedScalingDownRatio, tt.fields.features, record.NewFakeRecorder(10)) + s := New(0, 0, 0, 0, int(tt.fields.minimumMinReplicas), int(tt.fields.preferredMaxReplicas), "5m", "5Mi", map[string]string{"istio-proxy": "7m"}, map[string]string{"istio-proxy": "7Mi"}, tt.fields.maxCPU, tt.fields.maxMemory, 10000, tt.fields.maxAllowedScalingDownRatio, tt.fields.bufferRatioOnVerticalResource, tt.fields.features, record.NewFakeRecorder(10)) got, err := s.updateVPARecommendation(context.Background(), tt.args.tortoise, tt.args.hpa, tt.args.replicaNum, now) if (err != nil) != tt.wantErr { t.Errorf("updateVPARecommendation() error = %v, wantErr %v", err, tt.wantErr)