removal⚠️ : remove the external metric support

api/v1alpha1/tortoise_webhook.go

@@ -30,19 +30,14 @@ import (
 	"errors"
 	"fmt"
 
-	v2 "k8s.io/api/autoscaling/v2"
-	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
-
-	"github.com/mercari/tortoise/pkg/annotation"
 )
 
 // log is for logging in this package.
@@ -176,20 +171,6 @@ func (r *Tortoise) ValidateCreate() (admission.Warnings, error) {
 		return nil, fmt.Errorf("%s: tortoise should not have the policies for the container(s) which isn't defined in the deployment, but, it have the policy for the container(s) %v", fieldPath.Child("resourcePolicy"), uselessPolicies)
 	}
 
-	hpa, err := ClientService.GetHPAFromUser(ctx, r)
-	if err != nil {
-		// Check if HPA really exists or not.
-		return nil, fmt.Errorf("failed to get the horizontal pod autoscaler defined in %s: %w", fieldPath.Child("targetRefs", "horizontalPodAutoscalerName"), err)
-	}
-	if hpa != nil {
-		for _, c := range containers.List() {
-			err = validateHPAAnnotations(hpa, c)
-			if err != nil {
-				return nil, fmt.Errorf("the horizontal pod autoscaler defined in %s is invalid: %w", fieldPath.Child("targetRefs", "horizontalPodAutoscalerName"), err)
-			}
-		}
-	}
-
 	return nil, validateTortoise(r)
 }
 
@@ -230,51 +211,3 @@ func (r *Tortoise) ValidateDelete() (admission.Warnings, error) {
 	tortoiselog.Info("validate delete", "name", r.Name)
 	return nil, nil
 }
-
-func externalMetricNameFromAnnotation(hpa *v2.HorizontalPodAutoscaler, containerName string, k corev1.ResourceName) (string, bool, error) {
-	var prefix string
-	var ok bool
-	switch k {
-	case corev1.ResourceCPU:
-		prefix, ok = hpa.GetAnnotations()[annotation.HPAContainerBasedCPUExternalMetricNamePrefixAnnotation]
-	case corev1.ResourceMemory:
-		prefix, ok = hpa.GetAnnotations()[annotation.HPAContainerBasedMemoryExternalMetricNamePrefixAnnotation]
-	default:
-		return "", false, fmt.Errorf("non supported resource type: %s", k)
-	}
-	return prefix + containerName, ok, nil
-}
-
-func validateHPAAnnotations(hpa *v2.HorizontalPodAutoscaler, containerName string) error {
-	externalMetrics := sets.NewString()
-	for _, m := range hpa.Spec.Metrics {
-		if m.Type != v2.ExternalMetricSourceType {
-			continue
-		}
-
-		if m.External == nil {
-			// shouldn't reach here
-			klog.ErrorS(nil, "invalid external metric on HPA", klog.KObj(hpa))
-			continue
-		}
-
-		externalMetrics.Insert(m.External.Metric.Name)
-	}
-
-	resourceNames := []corev1.ResourceName{corev1.ResourceCPU, corev1.ResourceMemory}
-	for _, rn := range resourceNames {
-		externalMetricName, ok, err := externalMetricNameFromAnnotation(hpa, containerName, rn)
-		if err != nil {
-			return err
-		}
-		if !ok {
-			continue
-		}
-
-		if !externalMetrics.Has(externalMetricName) {
-			return fmt.Errorf("HPA doesn't have the external metrics which is defined in the annotations. (The annotation wants an external metric named %s)", externalMetricName)
-		}
-	}
-
-	return nil
-}

controllers/tortoise_controller_test.go

@@ -188,8 +188,8 @@ var _ = Describe("Test TortoiseController", func() {
 			wantHPA.Spec.MinReplicas = pointer.Int32(5)
 			wantHPA.Spec.MaxReplicas = 20
 			for i, m := range wantHPA.Spec.Metrics {
-				if m.Resource != nil && m.Resource.Name == corev1.ResourceCPU {
-					wantHPA.Spec.Metrics[i].Resource.Target.AverageUtilization = pointer.Int32(75)
+				if m.ContainerResource != nil && m.ContainerResource.Name == corev1.ResourceCPU && m.ContainerResource.Container == "app" {
+					wantHPA.Spec.Metrics[i].ContainerResource.Target.AverageUtilization = pointer.Int32(75)
 				}
 			}
 
@@ -601,8 +601,8 @@ var _ = Describe("Test TortoiseController", func() {
 			wantHPA.Spec.MinReplicas = pointer.Int32(20)
 			wantHPA.Spec.MaxReplicas = 20
 			for i, m := range wantHPA.Spec.Metrics {
-				if m.Resource != nil && m.Resource.Name == corev1.ResourceCPU {
-					wantHPA.Spec.Metrics[i].Resource.Target.AverageUtilization = pointer.Int32(75)
+				if m.ContainerResource != nil && m.ContainerResource.Name == corev1.ResourceCPU && m.ContainerResource.Container == "app" {
+					wantHPA.Spec.Metrics[i].ContainerResource.Target.AverageUtilization = pointer.Int32(75)
 				}
 			}
 
@@ -846,11 +846,11 @@ var _ = Describe("Test TortoiseController", func() {
 			wantHPA.Spec.MinReplicas = pointer.Int32(5)
 			wantHPA.Spec.MaxReplicas = 20
 			for i, m := range wantHPA.Spec.Metrics {
-				if m.External != nil && m.External.Metric.Name == "datadogmetric@default:mercari-app-cpu-app" {
-					wantHPA.Spec.Metrics[i].External.Target.Value = resourceQuantityPtr(resource.MustParse("50")) // won't get changed.
+				if m.ContainerResource != nil && m.ContainerResource.Name == corev1.ResourceCPU && m.ContainerResource.Container == "app" {
+					wantHPA.Spec.Metrics[i].ContainerResource.Target.AverageUtilization = pointer.Int32(50) // won't get changed.
 				}
-				if m.External != nil && m.External.Metric.Name == "datadogmetric@default:mercari-app-cpu-istio-proxy" {
-					wantHPA.Spec.Metrics[i].External.Target.Value = resourceQuantityPtr(resource.MustParse("75"))
+				if m.ContainerResource != nil && m.ContainerResource.Name == corev1.ResourceCPU && m.ContainerResource.Container == "istio-proxy" {
+					wantHPA.Spec.Metrics[i].ContainerResource.Target.AverageUtilization = pointer.Int32(75)
 				}
 			}
 
@@ -1151,11 +1151,11 @@ var _ = Describe("Test TortoiseController", func() {
 			wantHPA.Spec.MinReplicas = pointer.Int32(20)
 			wantHPA.Spec.MaxReplicas = 20
 			for i, m := range wantHPA.Spec.Metrics {
-				if m.External != nil && m.External.Metric.Name == "datadogmetric@default:mercari-app-cpu-app" {
-					wantHPA.Spec.Metrics[i].External.Target.Value = resourceQuantityPtr(resource.MustParse("50")) // won't get changed.
+				if m.ContainerResource != nil && m.ContainerResource.Name == corev1.ResourceCPU && m.ContainerResource.Container == "app" {
+					wantHPA.Spec.Metrics[i].ContainerResource.Target.AverageUtilization = pointer.Int32(50) // won't get changed.
 				}
-				if m.External != nil && m.External.Metric.Name == "datadogmetric@default:mercari-app-cpu-istio-proxy" {
-					wantHPA.Spec.Metrics[i].External.Target.Value = resourceQuantityPtr(resource.MustParse("75"))
+				if m.ContainerResource != nil && m.ContainerResource.Name == corev1.ResourceCPU && m.ContainerResource.Container == "istio-proxy" {
+					wantHPA.Spec.Metrics[i].ContainerResource.Target.AverageUtilization = pointer.Int32(75)
 				}
 			}
 
@@ -1793,10 +1793,6 @@ func multiContainerDeploymentWithReplicaNum(replica int32) *v1.Deployment {
 	}
 }
 
-func resourceQuantityPtr(quantity resource.Quantity) *resource.Quantity {
-	return &quantity
-}
-
 func deleteObj(ctx context.Context, deleteObj client.Object, name string) error {
 	err := k8sClient.Get(ctx, client.ObjectKey{Namespace: "default", Name: name}, deleteObj)
 	if err != nil {

docs/horizontal.md

@@ -64,59 +64,9 @@ Looking back the above formula,
 
 Currently, Tortoise supports:
 - `type: Resource` metric if Pod has only one container.
-- `type: ContainerResource` metric if Pod has only multiple containers.
-- `type: External` metric if Pod has the annotations described below.
-
-##### `type: External` support (will be removed)
-
-※ This feature will be removed soon since the container resource metrics feature graduated to beta.
-
-Regarding the `External`, given [the container resource metrics](https://kubernetes.io/docs/tasks/run-application/horizontal-pod-autoscale/#container-resource-metrics) is still alpha as of v1.26, 
-some companies are using the external metrics to fetch the container's resource utilization.
-
-So, you can let Tortoise regard some external metrics as referring to the container's resource utilization.
-You need to give the annotations to your HPA like:
-
-```yaml
-apiVersion: autoscaling/v2
-kind: HorizontalPodAutoscaler
-metadata:
-  name: example-hpa
-  namespace: example-prod
-  annotations:
-    tortoises.autoscaling.mercari.com/container-based-cpu-metric-prefix: "datadogmetric@example-prod:example-workload-cpu-"
-    tortoises.autoscaling.mercari.com/container-based-memory-metric-prefix: "datadogmetric@example-prod:example-workload-memory-"
-spec:
-  metrics:
-    - type: External
-      external:
-        metric:
-          name: datadogmetric@example-prod:example-workload-cpu-app # CPU target of the container named "app"
-        target:
-          type: Value
-          value: 60 # Tortoise regards this value as the target utilization for the CPU of the container named "app", and it will adjust this target value.
-    - type: External
-      external:
-        metric:
-          name: datadogmetric@example-prod:example-workload-cpu-istio-sidecar # CPU target of the container named "istio-sidecar"
-        target:
-          type: Value
-          value: 80 # Tortoise regards this value as the target utilization for the CPU of the container named "istio-sidecar", and it will adjust this target value. 
-    - type: External
-      external:
-        metric:
-          name: datadogmetric@example-prod:example-workload-memory-app # memory target of the container named "app"
-        target:
-          type: Value
-          value: 75 # Tortoise regards this value as the target utilization for the memory of the container named "app", and it will adjust this target value.
-    - type: External
-      external:
-        metric:
-          name: datadogmetric@example-prod:example-workload-memory-istio-sidecar # memory target of the container named "istio-sidecar"
-        target:
-          type: Value
-          value: 70 # Tortoise regards this value as the target utilization for the memory of the container named "istio-sidecar", and it will adjust this target value.
-```
+- `type: ContainerResource` metric if Pod has multiple containers.
+
+But, if a Pod has only one container but a corresponding HPA doesn't have `type: Resource`, tortoise controller looks for `type: ContainerResource` in HPA next.
 
 ### The container right sizing
 

pkg/annotation/annotation.go

@@ -1,9 +1,6 @@
 package annotation
 
 const (
-	HPAContainerBasedCPUExternalMetricNamePrefixAnnotation    = "tortoises.autoscaling.mercari.com/container-based-cpu-metric-prefix"
-	HPAContainerBasedMemoryExternalMetricNamePrefixAnnotation = "tortoises.autoscaling.mercari.com/container-based-memory-metric-prefix"
-
 	// TortoiseNameAnnotation - VPA and HPA managed by tortoise have this label.
 	TortoiseNameAnnotation = "tortoises.autoscaling.mercari.com/tortoise-name"