From f24677df878c2363b1cde3519b6089d2916b831c Mon Sep 17 00:00:00 2001
From: Kensei Nakada <handbomusic@gmail.com>
Date: Wed, 11 Oct 2023 18:38:05 +0900
Subject: [PATCH] record events when the HPA is modified (#166)

* record events when the HPA is modified

* give an annotation to HPAs in test
---
 controllers/tortoise_controller.go |  5 +++++
 pkg/hpa/service.go                 | 13 ++++++++++++-
 pkg/hpa/service_test.go            | 22 ++++++++++++++++------
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/controllers/tortoise_controller.go b/controllers/tortoise_controller.go
index 2ad3af0a..f82a9cb2 100644
--- a/controllers/tortoise_controller.go
+++ b/controllers/tortoise_controller.go
@@ -99,6 +99,11 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
 	}
 
 	defer func() {
+		if tortoise == nil {
+			logger.Error(reterr, "get error during the reconciliation, but cannot record the event because tortoise object is nil", "tortoise", req.NamespacedName)
+			return
+		}
+
 		tortoise = r.TortoiseService.RecordReconciliationFailure(tortoise, reterr, now)
 		_, err = r.TortoiseService.UpdateTortoiseStatus(ctx, tortoise, now, false)
 		if err != nil {
diff --git a/pkg/hpa/service.go b/pkg/hpa/service.go
index a875e9bb..299026c7 100644
--- a/pkg/hpa/service.go
+++ b/pkg/hpa/service.go
@@ -72,7 +72,7 @@ func (c *Service) InitializeHPA(ctx context.Context, tortoise *autoscalingv1beta
 		return tortoise, fmt.Errorf("create hpa: %w", err)
 	}
 
-	c.recorder.Event(tortoise, corev1.EventTypeNormal, "HPACreated", fmt.Sprintf("Initialized a HPA %s/%s", tortoise.Namespace, tortoise.Status.Targets.HorizontalPodAutoscaler))
+	c.recorder.Event(tortoise, corev1.EventTypeNormal, "HPACreated", fmt.Sprintf("Initialized a HPA %s/%s for a created tortoise", tortoise.Namespace, tortoise.Status.Targets.HorizontalPodAutoscaler))
 
 	return tortoise, nil
 }
@@ -347,6 +347,8 @@ func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx context.Context
 		}
 		// No need to edit container resource phase.
 
+		c.recorder.Event(tortoise, corev1.EventTypeNormal, "HPADeleted", fmt.Sprintf("Deleted a HPA %s/%s because tortoise has no resource to scale horizontally", tortoise.Namespace, tortoise.Status.Targets.HorizontalPodAutoscaler))
+
 		return tortoise, nil
 	}
 
@@ -361,11 +363,18 @@ func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx context.Context
 			if err != nil {
 				return tortoise, fmt.Errorf("initialize hpa: %w", err)
 			}
+
+			c.recorder.Event(tortoise, corev1.EventTypeNormal, "HPACreated", fmt.Sprintf("Initialized a HPA %s/%s because tortoise has resource to scale horizontally", tortoise.Namespace, tortoise.Status.Targets.HorizontalPodAutoscaler))
 			return tortoise, nil
 		}
 		return tortoise, fmt.Errorf("failed to get hpa on tortoise: %w", err)
 	}
 
+	// make sure it's managed by tortoise
+	if v, ok := hpa.Annotations[annotation.ManagedByTortoiseAnnotation]; !ok || v != "true" {
+		return tortoise, fmt.Errorf("the HPA %s/%s is specified in tortoise, but not managed by tortoise", hpa.Namespace, hpa.Name)
+	}
+
 	var newhpa *v2.HorizontalPodAutoscaler
 	var isHpaEdited bool
 	newhpa, tortoise, isHpaEdited = c.addHPAMetricsFromTortoiseAutoscalingPolicy(ctx, tortoise, hpa)
@@ -389,6 +398,8 @@ func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx context.Context
 		return tortoise, err
 	}
 
+	c.recorder.Event(tortoise, corev1.EventTypeNormal, "HPAUpdated", fmt.Sprintf("Updated a HPA %s/%s because the autoscaling policy is changed in the tortoise", tortoise.Namespace, tortoise.Status.Targets.HorizontalPodAutoscaler))
+
 	return tortoise, nil
 }
 
diff --git a/pkg/hpa/service_test.go b/pkg/hpa/service_test.go
index 50a40103..4ed975f3 100644
--- a/pkg/hpa/service_test.go
+++ b/pkg/hpa/service_test.go
@@ -1171,9 +1171,11 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) {
 			},
 			initialHPA: &v2.HorizontalPodAutoscaler{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:        "hpa",
-					Namespace:   "default",
-					Annotations: map[string]string{},
+					Name:      "hpa",
+					Namespace: "default",
+					Annotations: map[string]string{
+						annotation.ManagedByTortoiseAnnotation: "true",
+					},
 				},
 				Spec: v2.HorizontalPodAutoscalerSpec{
 					MinReplicas: ptrInt32(1),
@@ -1265,6 +1267,9 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) {
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "hpa",
 					Namespace: "default",
+					Annotations: map[string]string{
+						annotation.ManagedByTortoiseAnnotation: "true",
+					},
 				},
 				Spec: v2.HorizontalPodAutoscalerSpec{
 					MinReplicas: ptrInt32(1),
@@ -1391,9 +1396,11 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) {
 			},
 			initialHPA: &v2.HorizontalPodAutoscaler{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:        "hpa",
-					Namespace:   "default",
-					Annotations: map[string]string{},
+					Name:      "hpa",
+					Namespace: "default",
+					Annotations: map[string]string{
+						annotation.ManagedByTortoiseAnnotation: "true",
+					},
 				},
 				Spec: v2.HorizontalPodAutoscalerSpec{
 					MinReplicas: ptrInt32(1),
@@ -1496,6 +1503,9 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) {
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "hpa",
 					Namespace: "default",
+					Annotations: map[string]string{
+						annotation.ManagedByTortoiseAnnotation: "true",
+					},
 				},
 				Spec: v2.HorizontalPodAutoscalerSpec{
 					MinReplicas: ptrInt32(1),