From ab706a74ec3f006931158dcb8ebb6bb6fcbefb02 Mon Sep 17 00:00:00 2001
From: randytqwjp <154106461+randytqwjp@users.noreply.github.com>
Date: Fri, 26 Apr 2024 14:02:57 +0900
Subject: [PATCH] fix HPA init bug (#399)

* fix HPA init bug

* Update pkg/hpa/service_test.go

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

---------

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>
---
 pkg/hpa/service.go      |  2 +-
 pkg/hpa/service_test.go | 90 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/pkg/hpa/service.go b/pkg/hpa/service.go
index 5ce80f44..e0f9fd3d 100644
--- a/pkg/hpa/service.go
+++ b/pkg/hpa/service.go
@@ -81,7 +81,7 @@ func New(
 func (c *Service) InitializeHPA(ctx context.Context, tortoise *autoscalingv1beta3.Tortoise, replicaNum int32, now time.Time) (*autoscalingv1beta3.Tortoise, error) {
 	logger := log.FromContext(ctx)
 	// if all policy is off or Vertical, we don't need HPA.
-	if !HasHorizontal(tortoise) {
+	if !HasHorizontal(tortoise) && tortoise.Spec.TargetRefs.HorizontalPodAutoscalerName == nil {
 		logger.Info("no horizontal policy, no need to create HPA")
 		return tortoise, nil
 	}
diff --git a/pkg/hpa/service_test.go b/pkg/hpa/service_test.go
index 6d046f06..4891613f 100644
--- a/pkg/hpa/service_test.go
+++ b/pkg/hpa/service_test.go
@@ -2969,6 +2969,96 @@ func TestService_InitializeHPA(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "set tortoise hpa to existing even if all autoscaling policy is vertical",
+			args: args{
+				tortoise: &v1beta3.Tortoise{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "tortoise",
+						Namespace: "default",
+					},
+					Spec: v1beta3.TortoiseSpec{
+						TargetRefs: v1beta3.TargetRefs{
+							ScaleTargetRef: v1beta3.CrossVersionObjectReference{
+								Kind:       "Deployment",
+								Name:       "deployment",
+								APIVersion: "apps/v1",
+							},
+						},
+					},
+					Status: v1beta3.TortoiseStatus{
+						AutoscalingPolicy: []v1beta3.ContainerAutoscalingPolicy{
+							{
+								ContainerName: "app",
+								Policy: map[v1.ResourceName]v1beta3.AutoscalingType{
+									v1.ResourceMemory: v1beta3.AutoscalingTypeVertical,
+								},
+							},
+							{
+								ContainerName: "istio-proxy",
+								Policy: map[v1.ResourceName]v1beta3.AutoscalingType{
+									v1.ResourceMemory: v1beta3.AutoscalingTypeVertical,
+								},
+							},
+						},
+					},
+				},
+				replicaNum: 4,
+			},
+			initialHPA: &v2.HorizontalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "existing-hpa",
+					Namespace:   "default",
+					Annotations: map[string]string{},
+				},
+				Spec: v2.HorizontalPodAutoscalerSpec{
+					MinReplicas: ptrInt32(1),
+					MaxReplicas: 2,
+					ScaleTargetRef: v2.CrossVersionObjectReference{
+						Kind:       "Deployment",
+						Name:       "deployment",
+						APIVersion: "apps/v1",
+					},
+					Behavior: &v2.HorizontalPodAutoscalerBehavior{
+						ScaleDown: &v2.HPAScalingRules{
+							Policies: []v2.HPAScalingPolicy{
+								{
+									Type:          v2.PercentScalingPolicy,
+									Value:         2,
+									PeriodSeconds: 90,
+								},
+							},
+						},
+					},
+				},
+			},
+			afterHPA: &v2.HorizontalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "existing-hpa",
+					Namespace: "default",
+				},
+				Spec: v2.HorizontalPodAutoscalerSpec{
+					MinReplicas: ptrInt32(1),
+					MaxReplicas: 2,
+					ScaleTargetRef: v2.CrossVersionObjectReference{
+						Kind:       "Deployment",
+						Name:       "deployment",
+						APIVersion: "apps/v1",
+					},
+					Behavior: &v2.HorizontalPodAutoscalerBehavior{
+						ScaleDown: &v2.HPAScalingRules{
+							Policies: []v2.HPAScalingPolicy{
+								{
+									Type:          v2.PercentScalingPolicy,
+									Value:         2,
+									PeriodSeconds: 90,
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {