implement TortoiseHPATargetUtilizationMaxIncrease (#175)

* implement TortoiseHPATargetUtilizationMaxIncrease

* fix: modify HPA service initialization in main

* fix: initialize HPA service correctly in test

* modify test case

api/autoscaling/v2/webhook_suite_test.go

@@ -149,7 +149,7 @@ var _ = BeforeSuite(func() {
 	eventRecorder := mgr.GetEventRecorderFor("tortoise-controller")
 	tortoiseService, err := tortoise.New(mgr.GetClient(), eventRecorder, config.RangeOfMinMaxReplicasRecommendationHours, config.TimeZone, config.TortoiseUpdateInterval, config.MinMaxReplicasRecommendationType)
 	Expect(err).NotTo(HaveOccurred())
-	hpaService := hpa.New(mgr.GetClient(), eventRecorder, config.ReplicaReductionFactor, config.UpperTargetResourceUtilization)
+	hpaService := hpa.New(mgr.GetClient(), eventRecorder, config.ReplicaReductionFactor, config.UpperTargetResourceUtilization, 100)
 	Expect(err).NotTo(HaveOccurred())
 
 	hpaWebhook := New(tortoiseService, hpaService)

controllers/tortoise_controller_test.go

@@ -157,7 +157,7 @@ func startController(ctx context.Context) func() {
 	Expect(err).ShouldNot(HaveOccurred())
 	reconciler := &TortoiseReconciler{
 		Scheme:             scheme,
-		HpaService:         hpa.New(mgr.GetClient(), record.NewFakeRecorder(10), 0.95, 90),
+		HpaService:         hpa.New(mgr.GetClient(), record.NewFakeRecorder(10), 0.95, 90, 25),
 		EventRecorder:      record.NewFakeRecorder(10),
 		VpaService:         cli,
 		DeploymentService:  deployment.New(mgr.GetClient()),

main.go

@@ -134,7 +134,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	hpaService := hpa.New(mgr.GetClient(), eventRecorder, config.ReplicaReductionFactor, config.UpperTargetResourceUtilization)
+	hpaService := hpa.New(mgr.GetClient(), eventRecorder, config.ReplicaReductionFactor, config.UpperTargetResourceUtilization, config.TortoiseHPATargetUtilizationMaxIncrease)
 
 	if err = (&controllers.TortoiseReconciler{
 		Scheme:             mgr.GetScheme(),

pkg/config/config.go

@@ -35,6 +35,8 @@ type Config struct {
 	TimeZone string `yaml:"TimeZone"`
 	// TortoiseUpdateInterval is the interval of updating each tortoise (default: 15s)
 	TortoiseUpdateInterval time.Duration `yaml:"TortoiseUpdateInterval"`
+	// TortoiseHPATargetUtilizationMaxIncrease is the max increase of target utilization that tortoise can give to the HPA (default: 5)
+	TortoiseHPATargetUtilizationMaxIncrease int `yaml:"TortoiseHPATargetUtilizationMaxIncrease"`
 }
 
 // ParseConfig parses the config file (yaml) and returns Config.
@@ -53,6 +55,7 @@ func ParseConfig(path string) (*Config, error) {
 		MaximumMemoryBytes:                       "10Gi",
 		TimeZone:                                 "Asia/Tokyo",
 		TortoiseUpdateInterval:                   15 * time.Second,
+		TortoiseHPATargetUtilizationMaxIncrease:  5,
 	}
 	if path == "" {
 		return config, nil

pkg/config/config_test.go

@@ -35,6 +35,7 @@ func TestParseConfig(t *testing.T) {
 				MaximumMemoryBytes:                       "10Gi",
 				TimeZone:                                 "Asia/Tokyo",
 				TortoiseUpdateInterval:                   1 * time.Hour,
+				TortoiseHPATargetUtilizationMaxIncrease:  5,
 			},
 		},
 		{
@@ -56,6 +57,7 @@ func TestParseConfig(t *testing.T) {
 				MaximumMemoryBytes:                       "10Gi",
 				TimeZone:                                 "Asia/Tokyo",
 				TortoiseUpdateInterval:                   15 * time.Second,
+				TortoiseHPATargetUtilizationMaxIncrease:  5,
 			},
 		},
 		{
@@ -84,6 +86,7 @@ func TestParseConfig(t *testing.T) {
 				MaximumMemoryBytes:                       "10Gi",
 				TimeZone:                                 "Asia/Tokyo",
 				TortoiseUpdateInterval:                   15 * time.Second,
+				TortoiseHPATargetUtilizationMaxIncrease:  5,
 			},
 		},
 	}

pkg/hpa/service.go

@@ -29,17 +29,19 @@ import (
 type Service struct {
 	c client.Client
 
-	replicaReductionFactor         float64
-	upperTargetResourceUtilization int32
-	recorder                       record.EventRecorder
+	replicaReductionFactor                  float64
+	upperTargetResourceUtilization          int32
+	tortoiseHPATargetUtilizationMaxIncrease int
+	recorder                                record.EventRecorder
 }
 
-func New(c client.Client, recorder record.EventRecorder, replicaReductionFactor float64, upperTargetResourceUtilization int) *Service {
+func New(c client.Client, recorder record.EventRecorder, replicaReductionFactor float64, upperTargetResourceUtilization, tortoiseHPATargetUtilizationMaxIncrease int) *Service {
 	return &Service{
-		c:                              c,
-		replicaReductionFactor:         replicaReductionFactor,
-		upperTargetResourceUtilization: int32(upperTargetResourceUtilization),
-		recorder:                       recorder,
+		c:                                       c,
+		replicaReductionFactor:                  replicaReductionFactor,
+		upperTargetResourceUtilization:          int32(upperTargetResourceUtilization),
+		tortoiseHPATargetUtilizationMaxIncrease: tortoiseHPATargetUtilizationMaxIncrease,
+		recorder:                                recorder,
 	}
 }
 
@@ -316,13 +318,14 @@ func (c *Service) ChangeHPAFromTortoiseRecommendation(tortoise *autoscalingv1bet
 	}
 
 	for _, t := range tortoise.Status.Recommendations.Horizontal.TargetUtilizations {
-		for resourcename, targetutil := range t.TargetUtilization {
+		for resourcename, proposedTarget := range t.TargetUtilization {
 			if !readyHorizontalResourceAndContainer.Has(resourceNameAndContainerName{resourcename, t.ContainerName}) {
 				// this recommendation is not ready. We don't want to apply it.
 				continue
 			}
-			metrics.ProposedHPATargetUtilization.WithLabelValues(tortoise.Name, tortoise.Namespace, t.ContainerName, resourcename.String(), hpa.Name).Set(float64(targetutil))
-			if err := updateHPATargetValue(hpa, t.ContainerName, resourcename, targetutil); err != nil {
+
+			metrics.ProposedHPATargetUtilization.WithLabelValues(tortoise.Name, tortoise.Namespace, t.ContainerName, resourcename.String(), hpa.Name).Set(float64(proposedTarget))
+			if err := c.updateHPATargetValue(hpa, t.ContainerName, resourcename, proposedTarget); err != nil {
 				return nil, tortoise, fmt.Errorf("update HPA from the recommendation from tortoise")
 			}
 		}
@@ -496,7 +499,7 @@ func GetReplicasRecommendation(recommendations []autoscalingv1beta2.ReplicasReco
 
 // updateHPATargetValue updates the target value of the HPA.
 // It looks for the corresponding metric (ContainerResource) and updates the target value.
-func updateHPATargetValue(hpa *v2.HorizontalPodAutoscaler, containerName string, k corev1.ResourceName, targetValue int32) error {
+func (c *Service) updateHPATargetValue(hpa *v2.HorizontalPodAutoscaler, containerName string, k corev1.ResourceName, targetValue int32) error {
 	for _, m := range hpa.Spec.Metrics {
 		if m.Type != v2.ContainerResourceMetricSourceType {
 			continue
@@ -511,7 +514,18 @@ func updateHPATargetValue(hpa *v2.HorizontalPodAutoscaler, containerName string,
 			continue
 		}
 
+		if targetValue-*m.ContainerResource.Target.AverageUtilization > int32(c.tortoiseHPATargetUtilizationMaxIncrease) {
+			// We don't want to increase the target utilization that much because it might be dangerous.
+			// (Reduce is OK)
+
+			// We only allow to increase the target utilization by c.tortoiseHPATargetUtilizationMaxIncrease.
+			maxIncrease := *m.ContainerResource.Target.AverageUtilization + int32(c.tortoiseHPATargetUtilizationMaxIncrease)
+			m.ContainerResource.Target.AverageUtilization = &maxIncrease
+			return nil
+		}
+
 		m.ContainerResource.Target.AverageUtilization = &targetValue
+
 		return nil
 	}
 

pkg/hpa/service_test.go

@@ -193,6 +193,161 @@ func TestClient_UpdateHPAFromTortoiseRecommendation(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "the change is limited by tortoiseHPATargetUtilizationMaxIncrease",
+			args: args{
+				ctx: context.Background(),
+				tortoise: &autoscalingv1beta2.Tortoise{
+					Spec: autoscalingv1beta2.TortoiseSpec{
+						ResourcePolicy: []autoscalingv1beta2.ContainerResourcePolicy{
+							{
+								ContainerName: "app",
+								AutoscalingPolicy: map[v1.ResourceName]v1beta2.AutoscalingType{
+									v1.ResourceMemory: v1beta2.AutoscalingTypeHorizontal,
+								},
+							},
+							{
+								ContainerName: "istio-proxy",
+								AutoscalingPolicy: map[v1.ResourceName]v1beta2.AutoscalingType{
+									v1.ResourceCPU: v1beta2.AutoscalingTypeHorizontal,
+								},
+							},
+						},
+					},
+					Status: autoscalingv1beta2.TortoiseStatus{
+						ContainerResourcePhases: []autoscalingv1beta2.ContainerResourcePhases{
+							{
+								ContainerName: "app",
+								ResourcePhases: map[v1.ResourceName]autoscalingv1beta2.ResourcePhase{
+									v1.ResourceMemory: {
+										Phase: autoscalingv1beta2.ContainerResourcePhaseWorking,
+									},
+								},
+							},
+							{
+								ContainerName: "istio-proxy",
+								ResourcePhases: map[v1.ResourceName]autoscalingv1beta2.ResourcePhase{
+									v1.ResourceCPU: {
+										Phase: autoscalingv1beta2.ContainerResourcePhaseWorking,
+									},
+								},
+							},
+						},
+						Targets: autoscalingv1beta2.TargetsStatus{
+							HorizontalPodAutoscaler: "hpa",
+						},
+						Recommendations: autoscalingv1beta2.Recommendations{
+							Horizontal: autoscalingv1beta2.HorizontalRecommendations{
+								TargetUtilizations: []autoscalingv1beta2.HPATargetUtilizationRecommendationPerContainer{
+									{
+										ContainerName: "app",
+										TargetUtilization: map[v1.ResourceName]int32{
+											v1.ResourceMemory: 90,
+										},
+									},
+									{
+										ContainerName: "istio-proxy",
+										TargetUtilization: map[v1.ResourceName]int32{
+											v1.ResourceCPU: 80,
+										},
+									},
+								},
+								MaxReplicas: []autoscalingv1beta2.ReplicasRecommendation{
+									{
+										From:      0,
+										To:        2,
+										Value:     6,
+										UpdatedAt: now,
+										WeekDay:   pointer.String(now.Weekday().String()),
+									},
+								},
+								MinReplicas: []autoscalingv1beta2.ReplicasRecommendation{
+									{
+										From:      0,
+										To:        2,
+										Value:     3,
+										UpdatedAt: now,
+										WeekDay:   pointer.String(now.Weekday().String()),
+									},
+								},
+							},
+						},
+					},
+				},
+				now: now.Time,
+			},
+			initialHPA: &v2.HorizontalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "hpa",
+				},
+				Spec: v2.HorizontalPodAutoscalerSpec{
+					MinReplicas: ptrInt32(1),
+					MaxReplicas: 2,
+					Metrics: []v2.MetricSpec{
+						{
+							Type: v2.ObjectMetricSourceType,
+							// should be ignored
+						},
+						{
+							Type: v2.ContainerResourceMetricSourceType,
+							ContainerResource: &v2.ContainerResourceMetricSource{
+								Name: v1.ResourceMemory,
+								Target: v2.MetricTarget{
+									AverageUtilization: pointer.Int32(60),
+								},
+								Container: "app",
+							},
+						},
+						{
+							Type: v2.ContainerResourceMetricSourceType,
+							ContainerResource: &v2.ContainerResourceMetricSource{
+								Name: v1.ResourceCPU,
+								Target: v2.MetricTarget{
+									AverageUtilization: pointer.Int32(10),
+								},
+								Container: "istio-proxy",
+							},
+						},
+					},
+				},
+			},
+			want: &v2.HorizontalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "hpa",
+				},
+				Spec: v2.HorizontalPodAutoscalerSpec{
+					MinReplicas: ptrInt32(3),
+					MaxReplicas: 6,
+					Metrics: []v2.MetricSpec{
+						{
+							Type: v2.ObjectMetricSourceType,
+							// should be ignored
+						},
+						{
+							Type: v2.ContainerResourceMetricSourceType,
+							ContainerResource: &v2.ContainerResourceMetricSource{
+								Name: v1.ResourceMemory,
+								Target: v2.MetricTarget{
+									AverageUtilization: pointer.Int32(90),
+								},
+								Container: "app",
+							},
+						},
+						{
+							Type: v2.ContainerResourceMetricSourceType,
+							ContainerResource: &v2.ContainerResourceMetricSource{
+								Name: v1.ResourceCPU,
+								Target: v2.MetricTarget{
+									AverageUtilization: pointer.Int32(60), // 80 is recommended but only +50 is allowed.
+								},
+								Container: "istio-proxy",
+							},
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
 		{
 			name: "no update preformed when updateMode is Off",
 			args: args{
@@ -983,7 +1138,7 @@ func TestClient_UpdateHPAFromTortoiseRecommendation(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			c := New(fake.NewClientBuilder().WithRuntimeObjects(tt.initialHPA).Build(), record.NewFakeRecorder(10), 0.95, 90)
+			c := New(fake.NewClientBuilder().WithRuntimeObjects(tt.initialHPA).Build(), record.NewFakeRecorder(10), 0.95, 90, 50)
 			got, tortoise, err := c.UpdateHPAFromTortoiseRecommendation(tt.args.ctx, tt.args.tortoise, tt.args.now)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("UpdateHPAFromTortoiseRecommendation() error = %v, wantErr %v", err, tt.wantErr)
@@ -1250,9 +1405,9 @@ func TestService_InitializeHPA(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			c := New(fake.NewClientBuilder().Build(), record.NewFakeRecorder(10), 0.95, 90)
+			c := New(fake.NewClientBuilder().Build(), record.NewFakeRecorder(10), 0.95, 90, 100)
 			if tt.initialHPA != nil {
-				c = New(fake.NewClientBuilder().WithRuntimeObjects(tt.initialHPA).Build(), record.NewFakeRecorder(10), 0.95, 90)
+				c = New(fake.NewClientBuilder().WithRuntimeObjects(tt.initialHPA).Build(), record.NewFakeRecorder(10), 0.95, 90, 100)
 			}
 			_, err := c.InitializeHPA(context.Background(), tt.args.tortoise, tt.args.dm, time.Now())
 			if (err != nil) != tt.wantErr {
@@ -1972,9 +2127,9 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			c := New(fake.NewClientBuilder().Build(), record.NewFakeRecorder(10), 0.95, 90)
+			c := New(fake.NewClientBuilder().Build(), record.NewFakeRecorder(10), 0.95, 90, 100)
 			if tt.initialHPA != nil {
-				c = New(fake.NewClientBuilder().WithRuntimeObjects(tt.initialHPA).Build(), record.NewFakeRecorder(10), 0.95, 90)
+				c = New(fake.NewClientBuilder().WithRuntimeObjects(tt.initialHPA).Build(), record.NewFakeRecorder(10), 0.95, 90, 100)
 			}
 			tortoise, err := c.UpdateHPASpecFromTortoiseAutoscalingPolicy(context.Background(), tt.args.tortoise, tt.args.dm, time.Now())
 			if (err != nil) != tt.wantErr {