cleanup HPAs and VPAs created by tortoise

api/v1alpha1/tortoise_types.go

@@ -57,6 +57,14 @@ type TortoiseSpec struct {
 	// FeatureGates allows to list the alpha feature names.
 	// +optional
 	FeatureGates []string `json:"featureGates,omitempty" protobuf:"bytes,4,opt,name=featureGates"`
+	// DeletionPolicy is the policy how the controller deletes associated HPA and VPAs when tortoise is removed.
+	// If "DeleteAll", tortoise deletes all associated HPA and VPAs, created by tortoise. If the associated HPA is not created by tortoise,
+	// which is associated by spec.targetRefs.horizontalPodAutoscalerName, tortoise never delete the HPA.
+	// If "NoDelete", tortoise doesn't delete any associated HPA and VPAs.
+	//
+	// "DeleteAll" is the default value.
+	// +optional
+	DeletionPolicy DeletionPolicy `json:"deletionPolicy,omitempty" protobuf:"bytes,5,opt,name=deletionPolicy"`
 }
 
 type ContainerResourcePolicy struct {
@@ -82,6 +90,14 @@ type ContainerResourcePolicy struct {
 	AutoscalingPolicy map[v1.ResourceName]AutoscalingType `json:"autoscalingPolicy,omitempty" protobuf:"bytes,3,opt,name=autoscalingPolicy"`
 }
 
+// +kubebuilder:validation:Enum=DeleteAll;NoDelete
+type DeletionPolicy string
+
+const (
+	DeletionPolicyDeleteAll DeletionPolicy = "DeleteAll"
+	DeletionPolicyNoDelete  DeletionPolicy = "NoDelete"
+)
+
 // +kubebuilder:validation:Enum=Off;Auto;Emergency
 type UpdateMode string
 

config/crd/bases/autoscaling.mercari.com_tortoises.yaml

@@ -35,6 +35,18 @@ spec:
           spec:
             description: TortoiseSpec defines the desired state of Tortoise
             properties:
+              deletionPolicy:
+                description: "DeletionPolicy is the policy how the controller deletes
+                  associated HPA and VPAs when tortoise is removed. If \"DeleteAll\",
+                  tortoise deletes all associated HPA and VPAs, created by tortoise.
+                  If the associated HPA is not created by tortoise, which is associated
+                  by spec.targetRefs.horizontalPodAutoscalerName, tortoise never delete
+                  the HPA. If \"NoDelete\", tortoise doesn't delete any associated
+                  HPA and VPAs. \n \"DeleteAll\" is the default value."
+                enum:
+                - DeleteAll
+                - NoDelete
+                type: string
               featureGates:
                 description: FeatureGates allows to list the alpha feature names.
                 items:

controllers/tortoise_controller.go

@@ -72,16 +72,32 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	tortoise, err := r.TortoiseService.GetTortoise(ctx, req.NamespacedName)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
-			// Probably deleted.
+			// Probably deleted already and finalizer is already removed.
 			logger.V(4).Info("tortoise is not found", "tortoise", req.NamespacedName)
-			// TODO: delete VPA and HPA created by the Tortoise?
 			return ctrl.Result{}, nil
 		}
 
 		logger.Error(err, "failed to get tortoise", "tortoise", req.NamespacedName)
 		return ctrl.Result{}, err
 	}
 
+	if !tortoise.ObjectMeta.DeletionTimestamp.IsZero() {
+		// Tortoise is deleted by user and waiting for finalizer.
+		logger.Info("tortoise is deleted", "tortoise", req.NamespacedName)
+		if err := r.deleteVPAAndHPA(ctx, tortoise, now); err != nil {
+			return ctrl.Result{}, fmt.Errorf("delete VPAs and HPA: %w", err)
+		}
+		if err := r.TortoiseService.RemoveFinalizer(ctx, tortoise); err != nil {
+			return ctrl.Result{}, fmt.Errorf("remove finalizer: %w", err)
+		}
+		return ctrl.Result{}, nil
+	}
+
+	// tortoise is not deleted. Make sure finalizer is added to tortoise.
+	if err := r.TortoiseService.AddFinalizer(ctx, tortoise); err != nil {
+		return ctrl.Result{}, fmt.Errorf("add finalizer: %w", err)
+	}
+
 	reconcileNow, requeueAfter := r.TortoiseService.ShouldReconcileTortoiseNow(tortoise, now)
 	if !reconcileNow {
 		return ctrl.Result{RequeueAfter: requeueAfter}, nil
@@ -169,6 +185,32 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	return ctrl.Result{}, nil
 }
 
+func (r *TortoiseReconciler) deleteVPAAndHPA(ctx context.Context, tortoise *autoscalingv1alpha1.Tortoise, now time.Time) error {
+	if tortoise.Spec.DeletionPolicy == autoscalingv1alpha1.DeletionPolicyNoDelete {
+		// don't delete anything.
+		return nil
+	}
+
+	var err error
+	if tortoise.Spec.TargetRefs.HorizontalPodAutoscalerName == nil {
+		// delete HPA created by tortoise
+		err = r.HpaService.DeleteHPACreatedByTortoise(ctx, tortoise)
+		if err != nil {
+			return fmt.Errorf("delete HPA created by tortoise: %w", err)
+		}
+	}
+
+	err = r.VpaService.DeleteTortoiseMonitorVPA(ctx, tortoise)
+	if err != nil {
+		return fmt.Errorf("delete monitor VPA created by tortoise: %w", err)
+	}
+	err = r.VpaService.DeleteTortoiseUpdaterVPA(ctx, tortoise)
+	if err != nil {
+		return fmt.Errorf("delete updater VPA created by tortoise: %w", err)
+	}
+	return nil
+}
+
 func (r *TortoiseReconciler) initializeVPAAndHPA(ctx context.Context, tortoise *autoscalingv1alpha1.Tortoise, dm *v1.Deployment, now time.Time) error {
 	// need to initialize HPA and VPA.
 	tortoise, err := r.HpaService.InitializeHPA(ctx, tortoise, dm)
@@ -178,15 +220,15 @@ func (r *TortoiseReconciler) initializeVPAAndHPA(ctx context.Context, tortoise *
 
 	_, tortoise, err = r.VpaService.CreateTortoiseMonitorVPA(ctx, tortoise)
 	if err != nil {
-		return err
+		return fmt.Errorf("create tortoise monitor VPA: %w", err)
 	}
 	_, tortoise, err = r.VpaService.CreateTortoiseUpdaterVPA(ctx, tortoise)
 	if err != nil {
-		return err
+		return fmt.Errorf("create tortoise updater VPA: %w", err)
 	}
 	_, err = r.TortoiseService.UpdateTortoiseStatus(ctx, tortoise, now)
 	if err != nil {
-		return err
+		return fmt.Errorf("update Tortoise status: %w", err)
 	}
 	return nil
 }

controllers/tortoise_controller_test.go

@@ -10,11 +10,13 @@ import (
 	v1 "k8s.io/api/apps/v1"
 	v2 "k8s.io/api/autoscaling/v2"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	autoscalingv1 "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned"
 	"k8s.io/client-go/rest"
+	"k8s.io/client-go/util/retry"
 	"k8s.io/utils/pointer"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -35,20 +37,11 @@ var _ = Describe("Test TortoiseController", func() {
 	ctx := context.Background()
 	var stopFunc func()
 	cleanUp := func() {
-		err := deleteObj(ctx, &v1alpha1.Tortoise{}, "mercari")
-		Expect(err).ShouldNot(HaveOccurred())
-
-		err = deleteObj(ctx, &v1.Deployment{}, "mercari-app")
-		Expect(err).ShouldNot(HaveOccurred())
-
-		err = deleteObj(ctx, &autoscalingv1.VerticalPodAutoscaler{}, "tortoise-updater-mercari")
-		Expect(err).ShouldNot(HaveOccurred())
-
-		err = deleteObj(ctx, &autoscalingv1.VerticalPodAutoscaler{}, "tortoise-monitor-mercari")
-		Expect(err).ShouldNot(HaveOccurred())
-
-		err = deleteObj(ctx, &v2.HorizontalPodAutoscaler{}, "tortoise-hpa-mercari")
-		Expect(err).ShouldNot(HaveOccurred())
+		deleteObj(ctx, &v1alpha1.Tortoise{}, "mercari")
+		deleteObj(ctx, &v1.Deployment{}, "mercari-app")
+		deleteObj(ctx, &autoscalingv1.VerticalPodAutoscaler{}, "tortoise-updater-mercari")
+		deleteObj(ctx, &autoscalingv1.VerticalPodAutoscaler{}, "tortoise-monitor-mercari")
+		deleteObj(ctx, &v2.HorizontalPodAutoscaler{}, "tortoise-hpa-mercari")
 	}
 
 	BeforeEach(func() {
@@ -91,6 +84,14 @@ var _ = Describe("Test TortoiseController", func() {
 			suiteFailed = true
 		} else {
 			cleanUp()
+			for {
+				// make sure all resources are deleted
+				t := &v1alpha1.Tortoise{}
+				err := k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "mercari"}, t)
+				if apierrors.IsNotFound(err) {
+					break
+				}
+			}
 		}
 
 		stopFunc()
@@ -1343,6 +1344,121 @@ var _ = Describe("Test TortoiseController", func() {
 			}).Should(Succeed())
 		})
 	})
+	Context("DeletionPolicy is handled correctly", func() {
+		It("[DeletionPolicy = DeleteAll] delete HPA and VPAs when Tortoise is deleted", func() {
+			now := time.Now()
+			tc := testCase{
+				before: resources{
+					tortoise: utils.NewTortoiseBuilder().
+						SetName("mercari").
+						SetNamespace("default").
+						SetDeletionPolicy(v1alpha1.DeletionPolicyDeleteAll).
+						SetTargetRefs(v1alpha1.TargetRefs{
+							DeploymentName:              "mercari-app",
+							HorizontalPodAutoscalerName: pointer.String("hpa"),
+						}).
+						AddResourcePolicy(v1alpha1.ContainerResourcePolicy{
+							ContainerName: "app",
+							AutoscalingPolicy: map[corev1.ResourceName]v1alpha1.AutoscalingType{
+								corev1.ResourceCPU:    v1alpha1.AutoscalingTypeHorizontal,
+								corev1.ResourceMemory: v1alpha1.AutoscalingTypeVertical,
+							},
+						}).
+						AddResourcePolicy(v1alpha1.ContainerResourcePolicy{
+							ContainerName: "istio-proxy",
+							AutoscalingPolicy: map[corev1.ResourceName]v1alpha1.AutoscalingType{
+								corev1.ResourceCPU:    v1alpha1.AutoscalingTypeHorizontal,
+								corev1.ResourceMemory: v1alpha1.AutoscalingTypeVertical,
+							},
+						}).
+						SetTortoisePhase(v1alpha1.TortoisePhaseWorking).
+						SetRecommendations(v1alpha1.Recommendations{
+							Horizontal: &v1alpha1.HorizontalRecommendations{
+								TargetUtilizations: []v1alpha1.HPATargetUtilizationRecommendationPerContainer{
+									{
+										ContainerName: "app",
+										TargetUtilization: map[corev1.ResourceName]int32{
+											corev1.ResourceCPU: 50, // will be updated.
+										},
+									},
+									{
+										ContainerName: "istio-proxy",
+										TargetUtilization: map[corev1.ResourceName]int32{
+											corev1.ResourceCPU: 50, // will be updated.
+										},
+									},
+								},
+								MaxReplicas: []v1alpha1.ReplicasRecommendation{
+									{
+										From:      0,
+										To:        24,
+										WeekDay:   now.Weekday().String(),
+										TimeZone:  now.Location().String(),
+										Value:     15, // will be updated
+										UpdatedAt: metav1.NewTime(now),
+									},
+								},
+								MinReplicas: []v1alpha1.ReplicasRecommendation{
+									{
+										From:      0,
+										To:        24,
+										WeekDay:   now.Weekday().String(),
+										TimeZone:  now.Location().String(),
+										Value:     3, // will be updated
+										UpdatedAt: metav1.NewTime(now),
+									},
+								},
+							},
+						}).
+						AddCondition(v1alpha1.ContainerRecommendationFromVPA{
+							ContainerName: "app",
+							Recommendation: map[corev1.ResourceName]v1alpha1.ResourceQuantity{
+								corev1.ResourceCPU:    {},
+								corev1.ResourceMemory: {},
+							},
+							MaxRecommendation: map[corev1.ResourceName]v1alpha1.ResourceQuantity{
+								corev1.ResourceCPU:    {},
+								corev1.ResourceMemory: {},
+							},
+						}).
+						AddCondition(v1alpha1.ContainerRecommendationFromVPA{
+							ContainerName: "istio-proxy",
+							Recommendation: map[corev1.ResourceName]v1alpha1.ResourceQuantity{
+								corev1.ResourceCPU:    {},
+								corev1.ResourceMemory: {},
+							},
+							MaxRecommendation: map[corev1.ResourceName]v1alpha1.ResourceQuantity{
+								corev1.ResourceCPU:    {},
+								corev1.ResourceMemory: {},
+							},
+						}).
+						Build(),
+					deployment: multiContainerDeploymentWithReplicaNum(10),
+				},
+			}
+
+			err := tc.initializeResources(ctx, k8sClient, cfg)
+			Expect(err).ShouldNot(HaveOccurred())
+			time.Sleep(1 * time.Second)
+
+			// delete Tortoise
+			err = k8sClient.Delete(ctx, tc.before.tortoise)
+			Expect(err).ShouldNot(HaveOccurred())
+
+			Eventually(func(g Gomega) {
+				// make sure all resources are deleted
+				t := &v1alpha1.Tortoise{}
+				err = k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "mercari"}, t)
+				g.Expect(apierrors.IsNotFound(err)).To(Equal(true))
+				err = k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "hpa"}, &v2.HorizontalPodAutoscaler{})
+				g.Expect(apierrors.IsNotFound(err)).To(Equal(true))
+				err = k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "tortoise-updater-mercari"}, &autoscalingv1.VerticalPodAutoscaler{})
+				g.Expect(apierrors.IsNotFound(err)).To(Equal(true))
+				err = k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "tortoise-monitor-mercari"}, &autoscalingv1.VerticalPodAutoscaler{})
+				g.Expect(apierrors.IsNotFound(err)).To(Equal(true))
+			}).Should(Succeed())
+		})
+	})
 })
 
 type testCase struct {
@@ -1452,19 +1568,22 @@ func (t *testCase) initializeResources(ctx context.Context, k8sClient client.Cli
 
 	err = k8sClient.Create(ctx, t.before.tortoise.DeepCopy())
 	if err != nil {
-		return err
-	}
-	tortoise := &v1alpha1.Tortoise{}
-	err = k8sClient.Get(ctx, client.ObjectKey{Namespace: t.before.tortoise.Namespace, Name: t.before.tortoise.Name}, tortoise)
-	if err != nil {
-		panic(err)
-	}
-	tortoise.Status = t.before.tortoise.DeepCopy().Status
-	err = k8sClient.Status().Update(ctx, tortoise)
-	if err != nil {
-		return err
+		return fmt.Errorf("failed to create tortoise: %w", err)
 	}
-	return nil
+
+	return retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		tortoise := &v1alpha1.Tortoise{}
+		err = k8sClient.Get(ctx, client.ObjectKey{Namespace: t.before.tortoise.Namespace, Name: t.before.tortoise.Name}, tortoise)
+		if err != nil {
+			return fmt.Errorf("failed to get tortoise: %w", err)
+		}
+		tortoise.Status = t.before.tortoise.DeepCopy().Status
+		err = k8sClient.Status().Update(ctx, tortoise)
+		if err != nil {
+			return fmt.Errorf("failed to update tortoise status: %w", err)
+		}
+		return nil
+	})
 }
 
 func deploymentWithReplicaNum(replica int32) *v1.Deployment {

pkg/annotation/annotation.go

@@ -6,4 +6,8 @@ const (
 
 	// TortoiseNameAnnotation - VPA and HPA managed by tortoise have this label.
 	TortoiseNameAnnotation = "tortoises.autoscaling.mercari.com/tortoise-name"
+
+	// If this annotation is set to "true", it means that tortoise manages that resource,
+	// and will be removed when the tortoise is deleted.
+	ManagedByTortoiseAnnotation = "tortoise.autoscaling.mercari.com/managed-by-tortoise"
 )