diff --git a/controllers/tortoise_controller.go b/controllers/tortoise_controller.go index be0836ca..30b95560 100644 --- a/controllers/tortoise_controller.go +++ b/controllers/tortoise_controller.go @@ -118,6 +118,12 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ } }() + _, err = r.HpaService.UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx, tortoise) + if err != nil { + logger.Error(err, "update HPA spec from Tortoise autoscaling policy", "tortoise", req.NamespacedName) + return ctrl.Result{}, err + } + dm, err := r.DeploymentService.GetDeploymentOnTortoise(ctx, tortoise) if err != nil { logger.Error(err, "failed to get deployment", "tortoise", req.NamespacedName) diff --git a/pkg/hpa/service.go b/pkg/hpa/service.go index 2a2133e6..341a7f0f 100644 --- a/pkg/hpa/service.go +++ b/pkg/hpa/service.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "math" + "sort" "time" v1 "k8s.io/api/apps/v1" @@ -13,6 +14,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" "k8s.io/client-go/util/retry" "k8s.io/utils/pointer" @@ -120,6 +122,72 @@ func (c *Service) DeleteHPACreatedByTortoise(ctx context.Context, tortoise *auto return nil } +type resourceNameAndContainerName struct { + rn corev1.ResourceName + containerName string +} + +// addHPAMetricsFromTortoiseAutoscalingPolicy adds metrics to the HPA based on the autoscaling policy in the tortoise. +// Note that it doesn't update the HPA in kube-apiserver, you have to do that after this function. +func (c *Service) addHPAMetricsFromTortoiseAutoscalingPolicy(ctx context.Context, tortoise *autoscalingv1beta1.Tortoise, currenthpa *v2.HorizontalPodAutoscaler) *v2.HorizontalPodAutoscaler { + policies := sets.New[string]() + horizontalResourceAndContainer := sets.New[resourceNameAndContainerName]() + for _, p := range tortoise.Spec.ResourcePolicy { + policies.Insert(p.ContainerName) + for rn, ap := range p.AutoscalingPolicy { + if ap == autoscalingv1beta1.AutoscalingTypeHorizontal { + horizontalResourceAndContainer.Insert(resourceNameAndContainerName{rn, p.ContainerName}) + } + } + } + + hpaManagedResourceAndContainer := sets.New[resourceNameAndContainerName]() + for _, m := range currenthpa.Spec.Metrics { + if m.Type != v2.ContainerResourceMetricSourceType { + continue + } + hpaManagedResourceAndContainer.Insert(resourceNameAndContainerName{m.ContainerResource.Name, m.ContainerResource.Container}) + } + + needToAddToHPA := horizontalResourceAndContainer.Difference(hpaManagedResourceAndContainer) + needToRemoveFromHPA := hpaManagedResourceAndContainer.Difference(horizontalResourceAndContainer) + + sortedList := needToAddToHPA.UnsortedList() + sort.SliceStable(sortedList, func(i, j int) bool { + return sortedList[i].containerName < sortedList[j].containerName + }) + + // add metrics + for _, d := range sortedList { + m := v2.MetricSpec{ + Type: v2.ContainerResourceMetricSourceType, + ContainerResource: &v2.ContainerResourceMetricSource{ + Name: d.rn, + Container: d.containerName, + Target: v2.MetricTarget{ + Type: v2.UtilizationMetricType, + // we always start from a conservative value. and later will be adjusted by the recommendation. + AverageUtilization: pointer.Int32(50), + }, + }, + } + currenthpa.Spec.Metrics = append(currenthpa.Spec.Metrics, m) + } + + // remove metrics + for i, m := range currenthpa.Spec.Metrics { + if m.Type != v2.ContainerResourceMetricSourceType { + continue + } + if needToRemoveFromHPA.Has(resourceNameAndContainerName{m.ContainerResource.Name, m.ContainerResource.Container}) { + currenthpa.Spec.Metrics = append(currenthpa.Spec.Metrics[:i], currenthpa.Spec.Metrics[i+1:]...) + continue + } + } + + return currenthpa +} + func (c *Service) CreateHPA(ctx context.Context, tortoise *autoscalingv1beta1.Tortoise, dm *v1.Deployment) (*v2.HorizontalPodAutoscaler, *autoscalingv1beta1.Tortoise, error) { if tortoise.Spec.TargetRefs.HorizontalPodAutoscalerName != nil { // we don't have to create HPA as the user specified the existing HPA. @@ -166,27 +234,8 @@ func (c *Service) CreateHPA(ctx context.Context, tortoise *autoscalingv1beta1.To }, } - m := make([]v2.MetricSpec, 0, len(tortoise.Spec.ResourcePolicy)) - for _, policy := range tortoise.Spec.ResourcePolicy { - for r, p := range policy.AutoscalingPolicy { - value := pointer.Int32(50) - if p == autoscalingv1beta1.AutoscalingTypeVertical { - value = pointer.Int32(c.upperTargetResourceUtilization) - } - m = append(m, v2.MetricSpec{ - Type: v2.ContainerResourceMetricSourceType, - ContainerResource: &v2.ContainerResourceMetricSource{ - Name: r, - Container: policy.ContainerName, - Target: v2.MetricTarget{ - Type: v2.UtilizationMetricType, - AverageUtilization: value, - }, - }, - }) - } - } - hpa.Spec.Metrics = m + hpa = c.addHPAMetricsFromTortoiseAutoscalingPolicy(ctx, tortoise, hpa) + tortoise.Status.Targets.HorizontalPodAutoscaler = hpa.Name err := c.c.Create(ctx, hpa) @@ -249,6 +298,31 @@ func (c *Service) ChangeHPAFromTortoiseRecommendation(tortoise *autoscalingv1bet return hpa, tortoise, nil } +func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx context.Context, tortoise *autoscalingv1beta1.Tortoise) (*v2.HorizontalPodAutoscaler, error) { + hpa := &v2.HorizontalPodAutoscaler{} + if err := c.c.Get(ctx, types.NamespacedName{Namespace: tortoise.Namespace, Name: tortoise.Status.Targets.HorizontalPodAutoscaler}, hpa); err != nil { + return nil, fmt.Errorf("failed to get hpa on tortoise: %w", err) + } + + newhpa := c.addHPAMetricsFromTortoiseAutoscalingPolicy(ctx, tortoise, hpa) + updateFn := func() error { + hpa := &v2.HorizontalPodAutoscaler{} + if err := c.c.Get(ctx, types.NamespacedName{Namespace: tortoise.Namespace, Name: tortoise.Status.Targets.HorizontalPodAutoscaler}, hpa); err != nil { + return fmt.Errorf("failed to get hpa on tortoise: %w", err) + } + + hpa.Spec.Metrics = newhpa.Spec.Metrics + + return c.c.Update(ctx, newhpa) + } + + if err := retry.RetryOnConflict(retry.DefaultRetry, updateFn); err != nil { + return nil, err + } + + return hpa, nil +} + func (c *Service) UpdateHPAFromTortoiseRecommendation(ctx context.Context, tortoise *autoscalingv1beta1.Tortoise, now time.Time) (*v2.HorizontalPodAutoscaler, *autoscalingv1beta1.Tortoise, error) { // if all policy is off or Vertical, we don't update HPA. foundHorizontal := false diff --git a/pkg/hpa/service_test.go b/pkg/hpa/service_test.go index 58e46fc5..ed4c7730 100644 --- a/pkg/hpa/service_test.go +++ b/pkg/hpa/service_test.go @@ -833,6 +833,22 @@ func TestService_InitializeHPA(t *testing.T) { Namespace: "default", }, Spec: autoscalingv1beta1.TortoiseSpec{ + ResourcePolicy: []autoscalingv1beta1.ContainerResourcePolicy{ + { + ContainerName: "app", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + { + ContainerName: "istio-proxy", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + }, TargetRefs: autoscalingv1beta1.TargetRefs{ ScaleTargetRef: autoscalingv1beta1.CrossVersionObjectReference{ Kind: "Deployment", @@ -855,6 +871,9 @@ func TestService_InitializeHPA(t *testing.T) { { Name: "app", }, + { + Name: "istio-proxy", + }, }, }, }, @@ -876,6 +895,30 @@ func TestService_InitializeHPA(t *testing.T) { Spec: v2.HorizontalPodAutoscalerSpec{ MinReplicas: ptrInt32(2), MaxReplicas: 8, + Metrics: []v2.MetricSpec{ + { + Type: v2.ContainerResourceMetricSourceType, + ContainerResource: &v2.ContainerResourceMetricSource{ + Name: v1.ResourceCPU, + Container: "app", + Target: v2.MetricTarget{ + AverageUtilization: pointer.Int32(50), + Type: v2.UtilizationMetricType, + }, + }, + }, + { + Type: v2.ContainerResourceMetricSourceType, + ContainerResource: &v2.ContainerResourceMetricSource{ + Name: v1.ResourceCPU, + Container: "istio-proxy", + Target: v2.MetricTarget{ + AverageUtilization: pointer.Int32(50), + Type: v2.UtilizationMetricType, + }, + }, + }, + }, ScaleTargetRef: v2.CrossVersionObjectReference{ Kind: "Deployment", Name: "deployment", @@ -1025,3 +1068,344 @@ func TestService_InitializeHPA(t *testing.T) { }) } } + +func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { + type args struct { + tortoise *autoscalingv1beta1.Tortoise + dm *appv1.Deployment + } + tests := []struct { + name string + initialHPA *v2.HorizontalPodAutoscaler + args args + afterHPA *v2.HorizontalPodAutoscaler + wantErr bool + }{ + { + name: "add metrics to existing hpa", + args: args{ + tortoise: &autoscalingv1beta1.Tortoise{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tortoise", + Namespace: "default", + }, + Spec: autoscalingv1beta1.TortoiseSpec{ + ResourcePolicy: []autoscalingv1beta1.ContainerResourcePolicy{ + { + ContainerName: "app", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + { + ContainerName: "istio-proxy", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + }, + TargetRefs: autoscalingv1beta1.TargetRefs{ + HorizontalPodAutoscalerName: pointer.String("existing-hpa"), + ScaleTargetRef: autoscalingv1beta1.CrossVersionObjectReference{ + Kind: "Deployment", + Name: "deployment", + }, + }, + }, + Status: autoscalingv1beta1.TortoiseStatus{ + Targets: autoscalingv1beta1.TargetsStatus{ + HorizontalPodAutoscaler: "hpa", + }, + }, + }, + dm: &appv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "deployment", + Namespace: "default", + }, + Spec: appv1.DeploymentSpec{ + Replicas: pointer.Int32(4), + Template: v1.PodTemplateSpec{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "app", + }, + }, + }, + }, + }, + Status: appv1.DeploymentStatus{ + Replicas: 4, + }, + }, + }, + initialHPA: &v2.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Name: "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", + }, + Metrics: []v2.MetricSpec{ + { + Type: v2.ContainerResourceMetricSourceType, + ContainerResource: &v2.ContainerResourceMetricSource{ + Name: v1.ResourceCPU, + Container: "app", + Target: v2.MetricTarget{ + AverageUtilization: pointer.Int32(50), + Type: v2.UtilizationMetricType, + }, + }, + }, + }, + Behavior: &v2.HorizontalPodAutoscalerBehavior{ + ScaleDown: &v2.HPAScalingRules{ + Policies: []v2.HPAScalingPolicy{ + { + Type: v2.PercentScalingPolicy, + Value: 2, + PeriodSeconds: 90, + }, + }, + }, + }, + }, + }, + afterHPA: &v2.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hpa", + Namespace: "default", + }, + Spec: v2.HorizontalPodAutoscalerSpec{ + MinReplicas: ptrInt32(1), + MaxReplicas: 2, + ScaleTargetRef: v2.CrossVersionObjectReference{ + Kind: "Deployment", + Name: "deployment", + APIVersion: "apps/v1", + }, + Metrics: []v2.MetricSpec{ + { + Type: v2.ContainerResourceMetricSourceType, + ContainerResource: &v2.ContainerResourceMetricSource{ + Name: v1.ResourceCPU, + Container: "app", + Target: v2.MetricTarget{ + AverageUtilization: pointer.Int32(50), + Type: v2.UtilizationMetricType, + }, + }, + }, + { + Type: v2.ContainerResourceMetricSourceType, + ContainerResource: &v2.ContainerResourceMetricSource{ + Name: v1.ResourceCPU, + Container: "istio-proxy", + Target: v2.MetricTarget{ + AverageUtilization: pointer.Int32(50), + Type: v2.UtilizationMetricType, + }, + }, + }, + }, + Behavior: &v2.HorizontalPodAutoscalerBehavior{ + ScaleDown: &v2.HPAScalingRules{ + Policies: []v2.HPAScalingPolicy{ + { + Type: v2.PercentScalingPolicy, + Value: 2, + PeriodSeconds: 90, + }, + }, + }, + }, + }, + }, + }, + { + name: "remove metrics from hpa", + args: args{ + tortoise: &autoscalingv1beta1.Tortoise{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tortoise", + Namespace: "default", + }, + Spec: autoscalingv1beta1.TortoiseSpec{ + ResourcePolicy: []autoscalingv1beta1.ContainerResourcePolicy{ + { + ContainerName: "app", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeVertical, + }, + }, + { + ContainerName: "istio-proxy", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + }, + TargetRefs: autoscalingv1beta1.TargetRefs{ + HorizontalPodAutoscalerName: pointer.String("existing-hpa"), + ScaleTargetRef: autoscalingv1beta1.CrossVersionObjectReference{ + Kind: "Deployment", + Name: "deployment", + }, + }, + }, + Status: autoscalingv1beta1.TortoiseStatus{ + Targets: autoscalingv1beta1.TargetsStatus{ + HorizontalPodAutoscaler: "hpa", + }, + }, + }, + dm: &appv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "deployment", + Namespace: "default", + }, + Spec: appv1.DeploymentSpec{ + Replicas: pointer.Int32(4), + Template: v1.PodTemplateSpec{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "app", + }, + }, + }, + }, + }, + Status: appv1.DeploymentStatus{ + Replicas: 4, + }, + }, + }, + initialHPA: &v2.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Name: "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", + }, + Metrics: []v2.MetricSpec{ + { + Type: v2.ContainerResourceMetricSourceType, + ContainerResource: &v2.ContainerResourceMetricSource{ + Name: v1.ResourceCPU, + Container: "app", + Target: v2.MetricTarget{ + AverageUtilization: pointer.Int32(50), + Type: v2.UtilizationMetricType, + }, + }, + }, + { + Type: v2.ContainerResourceMetricSourceType, + ContainerResource: &v2.ContainerResourceMetricSource{ + Name: v1.ResourceCPU, + Container: "istio-proxy", + Target: v2.MetricTarget{ + AverageUtilization: pointer.Int32(50), + Type: v2.UtilizationMetricType, + }, + }, + }, + }, + Behavior: &v2.HorizontalPodAutoscalerBehavior{ + ScaleDown: &v2.HPAScalingRules{ + Policies: []v2.HPAScalingPolicy{ + { + Type: v2.PercentScalingPolicy, + Value: 2, + PeriodSeconds: 90, + }, + }, + }, + }, + }, + }, + afterHPA: &v2.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hpa", + Namespace: "default", + }, + Spec: v2.HorizontalPodAutoscalerSpec{ + MinReplicas: ptrInt32(1), + MaxReplicas: 2, + ScaleTargetRef: v2.CrossVersionObjectReference{ + Kind: "Deployment", + Name: "deployment", + APIVersion: "apps/v1", + }, + Metrics: []v2.MetricSpec{ + { + Type: v2.ContainerResourceMetricSourceType, + ContainerResource: &v2.ContainerResourceMetricSource{ + Name: v1.ResourceCPU, + Container: "istio-proxy", + Target: v2.MetricTarget{ + AverageUtilization: pointer.Int32(50), + Type: v2.UtilizationMetricType, + }, + }, + }, + }, + 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) { + c := New(fake.NewClientBuilder().Build(), record.NewFakeRecorder(10), 0.95, 90) + if tt.initialHPA != nil { + c = New(fake.NewClientBuilder().WithRuntimeObjects(tt.initialHPA).Build(), record.NewFakeRecorder(10), 0.95, 90) + } + _, err := c.UpdateHPASpecFromTortoiseAutoscalingPolicy(context.Background(), tt.args.tortoise) + if (err != nil) != tt.wantErr { + t.Errorf("Service.UpdateHPASpecFromTortoiseAutoscalingPolicy() error = %v, wantErr %v", err, tt.wantErr) + return + } + hpa := &v2.HorizontalPodAutoscaler{} + err = c.c.Get(context.Background(), client.ObjectKey{Name: tt.afterHPA.Name, Namespace: tt.afterHPA.Namespace}, hpa) + if err != nil { + t.Errorf("get hpa error = %v", err) + } + + if d := cmp.Diff(tt.afterHPA, hpa, cmpopts.IgnoreFields(v2.HorizontalPodAutoscaler{}, "TypeMeta"), cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ResourceVersion")); d != "" { + t.Errorf("Service.UpdateHPASpecFromTortoiseAutoscalingPolicy() hpa diff = %v", d) + } + }) + } +}