diff --git a/api/v1alpha1/tortoise_types.go b/api/v1alpha1/tortoise_types.go index dc50f6f1..badf55aa 100644 --- a/api/v1alpha1/tortoise_types.go +++ b/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 diff --git a/api/v1alpha1/tortoise_webhook.go b/api/v1alpha1/tortoise_webhook.go index 9baa518b..6e8ec3af 100644 --- a/api/v1alpha1/tortoise_webhook.go +++ b/api/v1alpha1/tortoise_webhook.go @@ -74,6 +74,9 @@ func (r *Tortoise) Default() { if r.Spec.UpdateMode == "" { r.Spec.UpdateMode = UpdateModeOff } + if r.Spec.DeletionPolicy == "" { + r.Spec.DeletionPolicy = DeletionPolicyDeleteAll + } d, err := ClientService.GetDeploymentOnTortoise(context.Background(), r) if err != nil { diff --git a/config/crd/bases/autoscaling.mercari.com_tortoises.yaml b/config/crd/bases/autoscaling.mercari.com_tortoises.yaml index ab5830b3..5c584120 100644 --- a/config/crd/bases/autoscaling.mercari.com_tortoises.yaml +++ b/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: diff --git a/controllers/tortoise_controller.go b/controllers/tortoise_controller.go index 29e06da2..75b5a8c1 100644 --- a/controllers/tortoise_controller.go +++ b/controllers/tortoise_controller.go @@ -72,9 +72,8 @@ 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 } @@ -82,6 +81,23 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c 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 } diff --git a/controllers/tortoise_controller_test.go b/controllers/tortoise_controller_test.go index 460a8a7a..022e5f84 100644 --- a/controllers/tortoise_controller_test.go +++ b/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" @@ -36,19 +38,25 @@ var _ = Describe("Test TortoiseController", func() { var stopFunc func() cleanUp := func() { err := deleteObj(ctx, &v1alpha1.Tortoise{}, "mercari") - Expect(err).ShouldNot(HaveOccurred()) - + if err != nil { + Expect(apierrors.IsNotFound(err)).To(Equal(true)) + } err = deleteObj(ctx, &v1.Deployment{}, "mercari-app") - Expect(err).ShouldNot(HaveOccurred()) - + if err != nil { + Expect(apierrors.IsNotFound(err)).To(Equal(true)) + } err = deleteObj(ctx, &autoscalingv1.VerticalPodAutoscaler{}, "tortoise-updater-mercari") - Expect(err).ShouldNot(HaveOccurred()) - + if err != nil { + Expect(apierrors.IsNotFound(err)).To(Equal(true)) + } err = deleteObj(ctx, &autoscalingv1.VerticalPodAutoscaler{}, "tortoise-monitor-mercari") - Expect(err).ShouldNot(HaveOccurred()) - + if err != nil { + Expect(apierrors.IsNotFound(err)).To(Equal(true)) + } err = deleteObj(ctx, &v2.HorizontalPodAutoscaler{}, "tortoise-hpa-mercari") - Expect(err).ShouldNot(HaveOccurred()) + if err != nil { + Expect(apierrors.IsNotFound(err)).To(Equal(true)) + } } BeforeEach(func() { @@ -91,6 +99,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 +1359,231 @@ 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", + }). + 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 + err = k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "mercari"}, &v1alpha1.Tortoise{}) + g.Expect(apierrors.IsNotFound(err)).To(Equal(true)) + err = k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "tortoise-hpa-mercari"}, &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()) + }) + It("[DeletionPolicy = NoDelete] do not 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.DeletionPolicyNoDelete). + SetTargetRefs(v1alpha1.TargetRefs{ + DeploymentName: "mercari-app", + }). + 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()) + + // delete Tortoise + err = k8sClient.Delete(ctx, tc.before.tortoise) + Expect(err).ShouldNot(HaveOccurred()) + + // wait for the reconciliation + time.Sleep(1 * time.Second) + + Eventually(func(g Gomega) { + err = k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "tortoise-hpa-mercari"}, &v2.HorizontalPodAutoscaler{}) + Expect(err).ShouldNot(HaveOccurred()) + err = k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "tortoise-updater-mercari"}, &autoscalingv1.VerticalPodAutoscaler{}) + Expect(err).ShouldNot(HaveOccurred()) + err = k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "tortoise-monitor-mercari"}, &autoscalingv1.VerticalPodAutoscaler{}) + Expect(err).ShouldNot(HaveOccurred()) + err = k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: "mercari"}, &v1alpha1.Tortoise{}) + g.Expect(apierrors.IsNotFound(err)).To(Equal(true)) + }).Should(Succeed()) + }) + }) }) type testCase struct { @@ -1452,19 +1693,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 { diff --git a/pkg/annotation/annotation.go b/pkg/annotation/annotation.go index c500d2cc..09df670c 100644 --- a/pkg/annotation/annotation.go +++ b/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" ) diff --git a/pkg/hpa/service.go b/pkg/hpa/service.go index 7acb60e9..f954d1c3 100644 --- a/pkg/hpa/service.go +++ b/pkg/hpa/service.go @@ -11,6 +11,7 @@ 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" "k8s.io/apimachinery/pkg/types" @@ -46,7 +47,8 @@ func (c *Service) CreateHPAForSingleContainer(ctx context.Context, tortoise *aut Name: autoscalingv1alpha1.TortoiseDefaultHPAName(tortoise.Name), Namespace: tortoise.Namespace, Annotations: map[string]string{ - annotation.TortoiseNameAnnotation: tortoise.Name, + annotation.TortoiseNameAnnotation: tortoise.Name, + annotation.ManagedByTortoiseAnnotation: "true", }, }, Spec: v2.HorizontalPodAutoscalerSpec{ @@ -133,6 +135,7 @@ func (c *Service) giveAnnotationsOnHPA(ctx context.Context, tortoise *autoscalin hpa.Annotations = map[string]string{} } hpa.Annotations[annotation.TortoiseNameAnnotation] = tortoise.Name + hpa.Annotations[annotation.ManagedByTortoiseAnnotation] = "true" tortoise.Status.Targets.HorizontalPodAutoscaler = hpa.Name return c.c.Update(ctx, hpa) } @@ -140,6 +143,37 @@ func (c *Service) giveAnnotationsOnHPA(ctx context.Context, tortoise *autoscalin return tortoise, retry.RetryOnConflict(retry.DefaultRetry, updateFn) } +func (c *Service) DeleteHPACreatedByTortoise(ctx context.Context, tortoise *autoscalingv1alpha1.Tortoise) error { + if tortoise.Spec.TargetRefs.HorizontalPodAutoscalerName != nil || tortoise.Spec.DeletionPolicy == autoscalingv1alpha1.DeletionPolicyNoDelete { + // A user specified the existing HPA and tortoise didn't create HPA by itself. + return nil + } + + hpa := &v2.HorizontalPodAutoscaler{} + if err := c.c.Get(ctx, client.ObjectKey{ + Namespace: tortoise.Namespace, + Name: tortoise.Status.Targets.HorizontalPodAutoscaler, + }, hpa); err != nil { + if apierrors.IsNotFound(err) { + // already deleted + return nil + } + return fmt.Errorf("failed to get hpa: %w", err) + } + + // make sure it's created by tortoise + if v, ok := hpa.Annotations[annotation.ManagedByTortoiseAnnotation]; !ok || v != "true" { + // shouldn't reach here unless user manually remove the annotation. + return nil + } + + if err := c.c.Delete(ctx, hpa); err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to delete hpa: %w", err) + } + + return nil +} + func (c *Service) CreateHPA(ctx context.Context, tortoise *autoscalingv1alpha1.Tortoise, dm *v1.Deployment) (*v2.HorizontalPodAutoscaler, *autoscalingv1alpha1.Tortoise, error) { if tortoise.Spec.TargetRefs.HorizontalPodAutoscalerName != nil { // we don't have to create HPA as the user specified the existing HPA. @@ -162,6 +196,7 @@ func (c *Service) CreateHPAForMultipleContainer(ctx context.Context, tortoise *a annotation.HPAContainerBasedMemoryExternalMetricNamePrefixAnnotation: fmt.Sprintf("datadogmetric@%s:%s-memory-", tortoise.Namespace, tortoise.Spec.TargetRefs.DeploymentName), annotation.HPAContainerBasedCPUExternalMetricNamePrefixAnnotation: fmt.Sprintf("datadogmetric@%s:%s-cpu-", tortoise.Namespace, tortoise.Spec.TargetRefs.DeploymentName), annotation.TortoiseNameAnnotation: tortoise.Name, + annotation.ManagedByTortoiseAnnotation: "true", }, }, Spec: v2.HorizontalPodAutoscalerSpec{ diff --git a/pkg/hpa/service_test.go b/pkg/hpa/service_test.go index a1df6659..90974eb9 100644 --- a/pkg/hpa/service_test.go +++ b/pkg/hpa/service_test.go @@ -943,7 +943,8 @@ func TestService_InitializeHPA(t *testing.T) { Name: "tortoise-hpa-tortoise", Namespace: "default", Annotations: map[string]string{ - annotation.TortoiseNameAnnotation: "tortoise", + annotation.TortoiseNameAnnotation: "tortoise", + annotation.ManagedByTortoiseAnnotation: "true", }, }, Spec: v2.HorizontalPodAutoscalerSpec{ @@ -1037,7 +1038,8 @@ func TestService_InitializeHPA(t *testing.T) { Name: "existing-hpa", Namespace: "default", Annotations: map[string]string{ - annotation.TortoiseNameAnnotation: "tortoise", + annotation.TortoiseNameAnnotation: "tortoise", + annotation.ManagedByTortoiseAnnotation: "true", }, }, Spec: v2.HorizontalPodAutoscalerSpec{ diff --git a/pkg/tortoise/tortoise.go b/pkg/tortoise/tortoise.go index a905507a..a4e1f6ee 100644 --- a/pkg/tortoise/tortoise.go +++ b/pkg/tortoise/tortoise.go @@ -15,18 +15,22 @@ import ( "k8s.io/client-go/util/retry" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" "github.com/mercari/tortoise/api/v1alpha1" ) +const tortoiseFinalizer = "tortoise.autoscaling.mercari.com/finalizer" + type Service struct { c client.Client rangeOfMinMaxReplicasRecommendationHour int timeZone *time.Location tortoiseUpdateInterval time.Duration - mu sync.RWMutex + mu sync.RWMutex + // TODO: Instead of here, we should store the last time of each tortoise in the status of the tortoise. lastTimeUpdateTortoise map[client.ObjectKey]time.Time } @@ -192,6 +196,50 @@ func (s *Service) GetTortoise(ctx context.Context, namespacedName types.Namespac return t, nil } +func (s *Service) AddFinalizer(ctx context.Context, tortoise *v1alpha1.Tortoise) error { + if controllerutil.ContainsFinalizer(tortoise, tortoiseFinalizer) { + return nil + } + + updateFn := func() error { + retTortoise := &v1alpha1.Tortoise{} + err := s.c.Get(ctx, client.ObjectKeyFromObject(tortoise), retTortoise) + if err != nil { + return err + } + controllerutil.AddFinalizer(retTortoise, tortoiseFinalizer) + return s.c.Update(ctx, retTortoise) + } + + err := retry.RetryOnConflict(retry.DefaultRetry, updateFn) + if err != nil { + return fmt.Errorf("failed to add finalizer: %w", err) + } + + return nil +} + +func (s *Service) RemoveFinalizer(ctx context.Context, tortoise *v1alpha1.Tortoise) error { + if !controllerutil.ContainsFinalizer(tortoise, tortoiseFinalizer) { + return nil + } + + updateFn := func() error { + retTortoise := &v1alpha1.Tortoise{} + err := s.c.Get(ctx, client.ObjectKeyFromObject(tortoise), retTortoise) + if err != nil { + return err + } + controllerutil.RemoveFinalizer(tortoise, tortoiseFinalizer) + return s.c.Update(ctx, tortoise) + } + err := retry.RetryOnConflict(retry.DefaultRetry, updateFn) + if err != nil { + return fmt.Errorf("failed to remove finalizer: %w", err) + } + return nil +} + func (s *Service) UpdateTortoiseStatus(ctx context.Context, originalTortoise *v1alpha1.Tortoise, now time.Time) (*v1alpha1.Tortoise, error) { logger := log.FromContext(ctx) logger.V(4).Info("update tortoise status", "tortoise", klog.KObj(originalTortoise)) diff --git a/pkg/utils/tortoise_builder.go b/pkg/utils/tortoise_builder.go index 2a2d91f6..78f3cb93 100644 --- a/pkg/utils/tortoise_builder.go +++ b/pkg/utils/tortoise_builder.go @@ -28,6 +28,10 @@ func (b *TortoiseBuilder) SetTargetRefs(targetRefs v1alpha1.TargetRefs) *Tortois b.tortoise.Spec.TargetRefs = targetRefs return b } +func (b *TortoiseBuilder) SetDeletionPolicy(policy v1alpha1.DeletionPolicy) *TortoiseBuilder { + b.tortoise.Spec.DeletionPolicy = policy + return b +} func (b *TortoiseBuilder) SetUpdateMode(updateMode v1alpha1.UpdateMode) *TortoiseBuilder { b.tortoise.Spec.UpdateMode = updateMode diff --git a/pkg/vpa/service.go b/pkg/vpa/service.go index f7988170..23cff572 100644 --- a/pkg/vpa/service.go +++ b/pkg/vpa/service.go @@ -6,6 +6,7 @@ import ( autoscaling "k8s.io/api/autoscaling/v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned" @@ -13,6 +14,7 @@ import ( "k8s.io/client-go/util/retry" autoscalingv1alpha1 "github.com/mercari/tortoise/api/v1alpha1" + "github.com/mercari/tortoise/pkg/annotation" "github.com/mercari/tortoise/pkg/metrics" ) @@ -40,12 +42,68 @@ func TortoiseUpdaterVPAName(tortoiseName string) string { return TortoiseUpdaterVPANamePrefix + tortoiseName } +func (c *Service) DeleteTortoiseMonitorVPA(ctx context.Context, tortoise *autoscalingv1alpha1.Tortoise) error { + if tortoise.Spec.DeletionPolicy == autoscalingv1alpha1.DeletionPolicyNoDelete { + return nil + } + + vpa, err := c.c.AutoscalingV1().VerticalPodAutoscalers(tortoise.Namespace).Get(ctx, TortoiseMonitorVPAName(tortoise.Name), metav1.GetOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + // already deleted + return nil + } + return fmt.Errorf("failed to get vpa: %w", err) + } + + // make sure it's created by tortoise + if v, ok := vpa.Annotations[annotation.ManagedByTortoiseAnnotation]; !ok || v != "true" { + // shouldn't reach here unless user manually remove the annotation. + return nil + } + + if err := c.c.AutoscalingV1().VerticalPodAutoscalers(tortoise.Namespace).Delete(ctx, vpa.Name, metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to delete vpa: %w", err) + } + return nil +} + +func (c *Service) DeleteTortoiseUpdaterVPA(ctx context.Context, tortoise *autoscalingv1alpha1.Tortoise) error { + if tortoise.Spec.DeletionPolicy == autoscalingv1alpha1.DeletionPolicyNoDelete { + return nil + } + + vpa, err := c.c.AutoscalingV1().VerticalPodAutoscalers(tortoise.Namespace).Get(ctx, TortoiseUpdaterVPAName(tortoise.Name), metav1.GetOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + // already deleted + return nil + } + return fmt.Errorf("failed to get vpa: %w", err) + } + + // make sure it's created by tortoise + if v, ok := vpa.Annotations[annotation.ManagedByTortoiseAnnotation]; !ok || v != "true" { + // shouldn't reach here unless user manually remove the annotation. + return nil + } + + if err := c.c.AutoscalingV1().VerticalPodAutoscalers(tortoise.Namespace).Delete(ctx, vpa.Name, metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to delete vpa: %w", err) + } + return nil +} + func (c *Service) CreateTortoiseUpdaterVPA(ctx context.Context, tortoise *autoscalingv1alpha1.Tortoise) (*v1.VerticalPodAutoscaler, *autoscalingv1alpha1.Tortoise, error) { auto := v1.UpdateModeAuto vpa := &v1.VerticalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ Namespace: tortoise.Namespace, Name: TortoiseUpdaterVPAName(tortoise.Name), + Annotations: map[string]string{ + annotation.ManagedByTortoiseAnnotation: "true", + annotation.TortoiseNameAnnotation: tortoise.Name, + }, }, Spec: v1.VerticalPodAutoscalerSpec{ Recommenders: []*v1.VerticalPodAutoscalerRecommenderSelector{ @@ -87,6 +145,10 @@ func (c *Service) CreateTortoiseMonitorVPA(ctx context.Context, tortoise *autosc ObjectMeta: metav1.ObjectMeta{ Namespace: tortoise.Namespace, Name: TortoiseMonitorVPAName(tortoise.Name), + Annotations: map[string]string{ + annotation.ManagedByTortoiseAnnotation: "true", + annotation.TortoiseNameAnnotation: tortoise.Name, + }, }, Spec: v1.VerticalPodAutoscalerSpec{ TargetRef: &autoscaling.CrossVersionObjectReference{