add additional log & UT for HPA

kevinteng525 · kevinteng525 · commit f9ffa7f4553c · 2025-03-09T21:44:37.000+08:00
add additional log &amp; UT for HPA
diff --git a/pkg/controller/podautoscaler/horizontal.go b/pkg/controller/podautoscaler/horizontal.go
@@ -58,6 +58,8 @@ import (
 var (
 	scaleUpLimitFactor  = 2.0
 	scaleUpLimitMinimum = 4.0
+	// recommendationOutputLimit is the maximum number of recommendations that will be logged.
+	recommendationOutputLimit = 100
 )
 
 var (
@@ -845,9 +847,11 @@ func (a *HorizontalController) reconcileAutoscaler(ctx context.Context, hpaShare
 
 		logger.V(4).Info("Proposing desired replicas",
 			"desiredReplicas", metricDesiredReplicas,
+			"currentReplicas", currentReplicas,
 			"metric", metricName,
 			"timestamp", metricTimestamp,
-			"scaleTarget", reference)
+			"scaleTarget", reference,
+			"metrics", metricStatuses)
 
 		rescaleMetric := ""
 		if metricDesiredReplicas > desiredReplicas {
@@ -861,9 +865,9 @@ func (a *HorizontalController) reconcileAutoscaler(ctx context.Context, hpaShare
 			rescaleReason = "All metrics below target"
 		}
 		if hpa.Spec.Behavior == nil {
-			desiredReplicas = a.normalizeDesiredReplicas(hpa, key, currentReplicas, desiredReplicas, minReplicas)
+			desiredReplicas = a.normalizeDesiredReplicas(ctx, hpa, key, currentReplicas, desiredReplicas, minReplicas)
 		} else {
-			desiredReplicas = a.normalizeDesiredReplicasWithBehaviors(hpa, key, currentReplicas, desiredReplicas, minReplicas)
+			desiredReplicas = a.normalizeDesiredReplicasWithBehaviors(ctx, hpa, key, currentReplicas, desiredReplicas, minReplicas)
 		}
 		rescale = desiredReplicas != currentReplicas
 	}
@@ -885,6 +889,7 @@ func (a *HorizontalController) reconcileAutoscaler(ctx context.Context, hpaShare
 		a.storeScaleEvent(hpa.Spec.Behavior, key, currentReplicas, desiredReplicas)
 		logger.Info("Successfully rescaled",
 			"HPA", klog.KObj(hpa),
+			"scaleTarget", reference,
 			"currentReplicas", currentReplicas,
 			"desiredReplicas", desiredReplicas,
 			"reason", rescaleReason)
@@ -942,7 +947,7 @@ func (a *HorizontalController) stabilizeRecommendation(key string, prenormalized
 
 // normalizeDesiredReplicas takes the metrics desired replicas value and normalizes it based on the appropriate conditions (i.e. < maxReplicas, >
 // minReplicas, etc...)
-func (a *HorizontalController) normalizeDesiredReplicas(hpa *autoscalingv2.HorizontalPodAutoscaler, key string, currentReplicas int32, prenormalizedDesiredReplicas int32, minReplicas int32) int32 {
+func (a *HorizontalController) normalizeDesiredReplicas(ctx context.Context, hpa *autoscalingv2.HorizontalPodAutoscaler, key string, currentReplicas int32, prenormalizedDesiredReplicas int32, minReplicas int32) int32 {
 	stabilizedRecommendation := a.stabilizeRecommendation(key, prenormalizedDesiredReplicas)
 	if stabilizedRecommendation != prenormalizedDesiredReplicas {
 		setCondition(hpa, autoscalingv2.AbleToScale, v1.ConditionTrue, "ScaleDownStabilized", "recent recommendations were higher than current one, applying the highest recent recommendation")
@@ -957,6 +962,16 @@ func (a *HorizontalController) normalizeDesiredReplicas(hpa *autoscalingv2.Horiz
 	} else {
 		setCondition(hpa, autoscalingv2.ScalingLimited, v1.ConditionTrue, reason, "%s", message)
 	}
+	logger := klog.FromContext(ctx)
+	reference := fmt.Sprintf("%s/%s/%s", hpa.Spec.ScaleTargetRef.Kind, hpa.Namespace, hpa.Spec.ScaleTargetRef.Name)
+	logger.V(4).Info("Normalized desired replicas",
+		"scaleTarget", reference,
+		"currentReplicas", currentReplicas,
+		"desiredReplicas", desiredReplicas,
+		"minReplicas", minReplicas,
+		"stabilizedRecommendation", stabilizedRecommendation,
+		"prenormalizedDesiredReplicas", prenormalizedDesiredReplicas,
+		"reason", reason)
 
 	return desiredReplicas
 }
@@ -977,7 +992,7 @@ type NormalizationArg struct {
 // 2. Apply the scale up/down limits from the hpaSpec.Behaviors (i.e. add no more than 4 pods)
 // 3. Apply the constraints period (i.e. add no more than 4 pods per minute)
 // 4. Apply the stabilization (i.e. add no more than 4 pods per minute, and pick the smallest recommendation during last 5 minutes)
-func (a *HorizontalController) normalizeDesiredReplicasWithBehaviors(hpa *autoscalingv2.HorizontalPodAutoscaler, key string, currentReplicas, prenormalizedDesiredReplicas, minReplicas int32) int32 {
+func (a *HorizontalController) normalizeDesiredReplicasWithBehaviors(ctx context.Context, hpa *autoscalingv2.HorizontalPodAutoscaler, key string, currentReplicas, prenormalizedDesiredReplicas, minReplicas int32) int32 {
 	a.maybeInitScaleDownStabilizationWindow(hpa)
 	normalizationArg := NormalizationArg{
 		Key:               key,
@@ -987,21 +1002,42 @@ func (a *HorizontalController) normalizeDesiredReplicasWithBehaviors(hpa *autosc
 		MaxReplicas:       hpa.Spec.MaxReplicas,
 		CurrentReplicas:   currentReplicas,
 		DesiredReplicas:   prenormalizedDesiredReplicas}
-	stabilizedRecommendation, reason, message := a.stabilizeRecommendationWithBehaviors(normalizationArg)
+	stabilizedRecommendation, reason, message := a.stabilizeRecommendationWithBehaviors(ctx, normalizationArg)
 	normalizationArg.DesiredReplicas = stabilizedRecommendation
 	if stabilizedRecommendation != prenormalizedDesiredReplicas {
 		// "ScaleUpStabilized" || "ScaleDownStabilized"
 		setCondition(hpa, autoscalingv2.AbleToScale, v1.ConditionTrue, reason, "%s", message)
 	} else {
 		setCondition(hpa, autoscalingv2.AbleToScale, v1.ConditionTrue, "ReadyForNewScale", "recommended size matches current size")
 	}
-	desiredReplicas, reason, message := a.convertDesiredReplicasWithBehaviorRate(normalizationArg)
+
+	logger := klog.FromContext(ctx)
+	reference := fmt.Sprintf("%s/%s/%s", hpa.Spec.ScaleTargetRef.Kind, hpa.Namespace, hpa.Spec.ScaleTargetRef.Name)
+	logger.V(4).Info("Normalized desired replicas with behaviors - after stabilized recommendation",
+		"scaleTarget", reference,
+		"currentReplicas", currentReplicas,
+		"minReplicas", minReplicas,
+		"stabilizedRecommendation", stabilizedRecommendation,
+		"prenormalizedDesiredReplicas", prenormalizedDesiredReplicas,
+		"reason", reason,
+		"message", message)
+
+	desiredReplicas, reason, message := a.convertDesiredReplicasWithBehaviorRate(ctx, normalizationArg)
 	if desiredReplicas == stabilizedRecommendation {
 		setCondition(hpa, autoscalingv2.ScalingLimited, v1.ConditionFalse, reason, "%s", message)
 	} else {
 		setCondition(hpa, autoscalingv2.ScalingLimited, v1.ConditionTrue, reason, "%s", message)
 	}
 
+	logger.V(4).Info("Normalized desired replicas with behaviors - after rated recommendation",
+		"scaleTarget", reference,
+		"currentReplicas", currentReplicas,
+		"minReplicas", minReplicas,
+		"stabilizedRecommendation", stabilizedRecommendation,
+		"desiredReplicas", desiredReplicas,
+		"reason", reason,
+		"message", message)
+
 	return desiredReplicas
 }
 
@@ -1090,7 +1126,7 @@ func (a *HorizontalController) storeScaleEvent(behavior *autoscalingv2.Horizonta
 // stabilizeRecommendationWithBehaviors:
 // - replaces old recommendation with the newest recommendation,
 // - returns {max,min} of recommendations that are not older than constraints.Scale{Up,Down}.DelaySeconds
-func (a *HorizontalController) stabilizeRecommendationWithBehaviors(args NormalizationArg) (int32, string, string) {
+func (a *HorizontalController) stabilizeRecommendationWithBehaviors(ctx context.Context, args NormalizationArg) (int32, string, string) {
 	now := time.Now()
 
 	foundOldSample := false
@@ -1128,6 +1164,24 @@ func (a *HorizontalController) stabilizeRecommendationWithBehaviors(args Normali
 	if recommendation > downRecommendation {
 		recommendation = downRecommendation
 	}
+	// Only keep the recommendations and ignore timestamp for logging, keep last recommendationOutputLimit recommendations
+	var recommendations = make([]int32, 0, recommendationOutputLimit)
+	start := len(a.recommendations[args.Key]) - recommendationOutputLimit
+	if start < 0 {
+		start = 0
+	}
+	for i := start; i < len(a.recommendations[args.Key]); i++ {
+		recommendations = append(recommendations, a.recommendations[args.Key][i].recommendation)
+	}
+
+	logger := klog.FromContext(ctx)
+	logger.V(4).Info("Stabilizing recommendation",
+		"key", args.Key,
+		"currentReplicas", args.CurrentReplicas,
+		"desiredReplicas", args.DesiredReplicas,
+		"upRecommendation", upRecommendation,
+		"downRecommendation", downRecommendation,
+		"recommendations", recommendations)
 
 	// Record the unstabilized recommendation.
 	if foundOldSample {
@@ -1150,14 +1204,22 @@ func (a *HorizontalController) stabilizeRecommendationWithBehaviors(args Normali
 
 // convertDesiredReplicasWithBehaviorRate performs the actual normalization, given the constraint rate
 // It doesn't consider the stabilizationWindow, it is done separately
-func (a *HorizontalController) convertDesiredReplicasWithBehaviorRate(args NormalizationArg) (int32, string, string) {
+func (a *HorizontalController) convertDesiredReplicasWithBehaviorRate(ctx context.Context, args NormalizationArg) (int32, string, string) {
 	var possibleLimitingReason, possibleLimitingMessage string
+	logger := klog.FromContext(ctx)
 
 	if args.DesiredReplicas > args.CurrentReplicas {
 		a.scaleUpEventsLock.RLock()
 		defer a.scaleUpEventsLock.RUnlock()
 		a.scaleDownEventsLock.RLock()
 		defer a.scaleDownEventsLock.RUnlock()
+		logger.V(4).Info("Converting desired replicas with behavior rate - scale up",
+			"key", args.Key,
+			"currentReplicas", args.CurrentReplicas,
+			"desiredReplicas", args.DesiredReplicas,
+			"scaleUpEvents", a.scaleUpEvents[args.Key],
+			"scaleDownEvents", a.scaleDownEvents[args.Key])
+
 		scaleUpLimit := calculateScaleUpLimitWithScalingRules(args.CurrentReplicas, a.scaleUpEvents[args.Key], a.scaleDownEvents[args.Key], args.ScaleUpBehavior)
 
 		if scaleUpLimit < args.CurrentReplicas {
@@ -1181,6 +1243,13 @@ func (a *HorizontalController) convertDesiredReplicasWithBehaviorRate(args Norma
 		defer a.scaleUpEventsLock.RUnlock()
 		a.scaleDownEventsLock.RLock()
 		defer a.scaleDownEventsLock.RUnlock()
+		logger.V(4).Info("Converting desired replicas with behavior rate - scale down",
+			"key", args.Key,
+			"currentReplicas", args.CurrentReplicas,
+			"desiredReplicas", args.DesiredReplicas,
+			"scaleUpEvents", a.scaleUpEvents[args.Key],
+			"scaleDownEvents", a.scaleDownEvents[args.Key])
+
 		scaleDownLimit := calculateScaleDownLimitWithBehaviors(args.CurrentReplicas, a.scaleUpEvents[args.Key], a.scaleDownEvents[args.Key], args.ScaleDownBehavior)
 
 		if scaleDownLimit > args.CurrentReplicas {
diff --git a/pkg/controller/podautoscaler/horizontal_test.go b/pkg/controller/podautoscaler/horizontal_test.go
@@ -4315,6 +4315,42 @@ func TestScalingWithRules(t *testing.T) {
 			expectedReplicas:             16,
 			expectedCondition:            "ScaleUpLimit",
 		},
+		{
+			name:                         "scaleUp with percent policy and no previous events, thus scaled up within range",
+			scaleUpEvents:                generateEventsUniformDistribution([]int{0}, 120),
+			scaleDownEvents:              generateEventsUniformDistribution([]int{0}, 120),
+			specMinReplicas:              3,
+			specMaxReplicas:              6,
+			scaleUpRules:                 generateScalingRules(0, 0, 20, 600, 300),
+			currentReplicas:              3,
+			prenormalizedDesiredReplicas: 4,
+			expectedReplicas:             4,
+			expectedCondition:            "DesiredWithinRange",
+		},
+		{
+			name:                         "scaleUp with both pod and percent policy and previous scale up events, thus no scale up",
+			scaleUpEvents:                generateEventsUniformDistribution([]int{2}, 120),
+			scaleDownEvents:              generateEventsUniformDistribution([]int{0}, 120),
+			specMinReplicas:              3,
+			specMaxReplicas:              6,
+			scaleUpRules:                 generateScalingRules(1, 600, 20, 600, 300),
+			currentReplicas:              3,
+			prenormalizedDesiredReplicas: 4,
+			expectedReplicas:             3,
+			expectedCondition:            "ScaleUpLimit",
+		},
+		{
+			name:                         "scaleUp with percent policy and previous events but still be able to scale up with limited rate",
+			scaleUpEvents:                generateEventsUniformDistribution([]int{1}, 600),
+			scaleDownEvents:              generateEventsUniformDistribution([]int{0}, 600),
+			specMinReplicas:              3,
+			specMaxReplicas:              20,
+			scaleUpRules:                 generateScalingRules(0, 0, 20, 600, 300),
+			currentReplicas:              10,
+			prenormalizedDesiredReplicas: 12,
+			expectedReplicas:             11,
+			expectedCondition:            "ScaleUpLimit",
+		},
 		// ScaleDown with PeriodSeconds usage
 		{
 			name:                         "scaleDown with default policy and previous events",
@@ -4486,6 +4522,7 @@ func TestScalingWithRules(t *testing.T) {
 		},
 	}
 
+	tCtx := ktesting.Init(t)
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 
@@ -4510,7 +4547,7 @@ func TestScalingWithRules(t *testing.T) {
 				CurrentReplicas:   tc.currentReplicas,
 			}
 
-			replicas, condition, _ := hc.convertDesiredReplicasWithBehaviorRate(arg)
+			replicas, condition, _ := hc.convertDesiredReplicasWithBehaviorRate(tCtx, arg)
 			assert.Equal(t, tc.expectedReplicas, replicas, "expected replicas do not match with converted replicas")
 			assert.Equal(t, tc.expectedCondition, condition, "HPA condition does not match with expected condition")
 		})
@@ -4865,6 +4902,7 @@ func TestNormalizeDesiredReplicasWithBehavior(t *testing.T) {
 			scaleDownStabilizationWindowSeconds: 300,
 		},
 	}
+	tCtx := ktesting.Init(t)
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			hc := HorizontalController{
@@ -4883,7 +4921,7 @@ func TestNormalizeDesiredReplicasWithBehavior(t *testing.T) {
 					StabilizationWindowSeconds: &tc.scaleDownStabilizationWindowSeconds,
 				},
 			}
-			r, _, _ := hc.stabilizeRecommendationWithBehaviors(arg)
+			r, _, _ := hc.stabilizeRecommendationWithBehaviors(tCtx, arg)
 			assert.Equal(t, tc.expectedStabilizedReplicas, r, "expected replicas do not match")
 			if tc.expectedRecommendations != nil {
 				if !assert.Len(t, hc.recommendations[tc.key], len(tc.expectedRecommendations), "stored recommendations differ in length") {
