Skip to content

Commit c3122bb

Browse files
kevinteng525kevinteng525
committed
add additional log & UT for HPA
add additional log & UT for HPA
1 parent 8fe10dc commit c3122bb

File tree

2 files changed

+112
-11
lines changed

2 files changed

+112
-11
lines changed

‎pkg/controller/podautoscaler/horizontal.go

Lines changed: 72 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -845,9 +845,11 @@ func (a *HorizontalController) reconcileAutoscaler(ctx context.Context, hpaShare
845845

846846
logger.V(4).Info("Proposing desired replicas",
847847
"desiredReplicas", metricDesiredReplicas,
848+
"currentReplicas", currentReplicas,
848849
"metric", metricName,
849850
"timestamp", metricTimestamp,
850-
"scaleTarget", reference)
851+
"scaleTarget", reference,
852+
"metrics", metricStatuses)
851853

852854
rescaleMetric := ""
853855
if metricDesiredReplicas > desiredReplicas {
@@ -861,9 +863,9 @@ func (a *HorizontalController) reconcileAutoscaler(ctx context.Context, hpaShare
861863
rescaleReason = "All metrics below target"
862864
}
863865
if hpa.Spec.Behavior == nil {
864-
desiredReplicas = a.normalizeDesiredReplicas(hpa, key, currentReplicas, desiredReplicas, minReplicas)
866+
desiredReplicas = a.normalizeDesiredReplicas(ctx, hpa, key, currentReplicas, desiredReplicas, minReplicas)
865867
} else {
866-
desiredReplicas = a.normalizeDesiredReplicasWithBehaviors(hpa, key, currentReplicas, desiredReplicas, minReplicas)
868+
desiredReplicas = a.normalizeDesiredReplicasWithBehaviors(ctx, hpa, key, currentReplicas, desiredReplicas, minReplicas)
867869
}
868870
rescale = desiredReplicas != currentReplicas
869871
}
@@ -885,6 +887,7 @@ func (a *HorizontalController) reconcileAutoscaler(ctx context.Context, hpaShare
885887
a.storeScaleEvent(hpa.Spec.Behavior, key, currentReplicas, desiredReplicas)
886888
logger.Info("Successfully rescaled",
887889
"HPA", klog.KObj(hpa),
890+
"scaleTarget", reference,
888891
"currentReplicas", currentReplicas,
889892
"desiredReplicas", desiredReplicas,
890893
"reason", rescaleReason)
@@ -942,7 +945,7 @@ func (a *HorizontalController) stabilizeRecommendation(key string, prenormalized
942945

943946
// normalizeDesiredReplicas takes the metrics desired replicas value and normalizes it based on the appropriate conditions (i.e. < maxReplicas, >
944947
// minReplicas, etc...)
945-
func (a *HorizontalController) normalizeDesiredReplicas(hpa *autoscalingv2.HorizontalPodAutoscaler, key string, currentReplicas int32, prenormalizedDesiredReplicas int32, minReplicas int32) int32 {
948+
func (a *HorizontalController) normalizeDesiredReplicas(ctx context.Context, hpa *autoscalingv2.HorizontalPodAutoscaler, key string, currentReplicas int32, prenormalizedDesiredReplicas int32, minReplicas int32) int32 {
946949
stabilizedRecommendation := a.stabilizeRecommendation(key, prenormalizedDesiredReplicas)
947950
if stabilizedRecommendation != prenormalizedDesiredReplicas {
948951
setCondition(hpa, autoscalingv2.AbleToScale, v1.ConditionTrue, "ScaleDownStabilized", "recent recommendations were higher than current one, applying the highest recent recommendation")
@@ -957,6 +960,16 @@ func (a *HorizontalController) normalizeDesiredReplicas(hpa *autoscalingv2.Horiz
957960
} else {
958961
setCondition(hpa, autoscalingv2.ScalingLimited, v1.ConditionTrue, reason, "%s", message)
959962
}
963+
logger := klog.FromContext(ctx)
964+
reference := fmt.Sprintf("%s/%s/%s", hpa.Spec.ScaleTargetRef.Kind, hpa.Namespace, hpa.Spec.ScaleTargetRef.Name)
965+
logger.V(4).Info("Normalized desired replicas",
966+
"scaleTarget", reference,
967+
"currentReplicas", currentReplicas,
968+
"desiredReplicas", desiredReplicas,
969+
"minReplicas", minReplicas,
970+
"stabilizedRecommendation", stabilizedRecommendation,
971+
"prenormalizedDesiredReplicas", prenormalizedDesiredReplicas,
972+
"reason", reason)
960973

961974
return desiredReplicas
962975
}
@@ -977,7 +990,7 @@ type NormalizationArg struct {
977990
// 2. Apply the scale up/down limits from the hpaSpec.Behaviors (i.e. add no more than 4 pods)
978991
// 3. Apply the constraints period (i.e. add no more than 4 pods per minute)
979992
// 4. Apply the stabilization (i.e. add no more than 4 pods per minute, and pick the smallest recommendation during last 5 minutes)
980-
func (a *HorizontalController) normalizeDesiredReplicasWithBehaviors(hpa *autoscalingv2.HorizontalPodAutoscaler, key string, currentReplicas, prenormalizedDesiredReplicas, minReplicas int32) int32 {
993+
func (a *HorizontalController) normalizeDesiredReplicasWithBehaviors(ctx context.Context, hpa *autoscalingv2.HorizontalPodAutoscaler, key string, currentReplicas, prenormalizedDesiredReplicas, minReplicas int32) int32 {
981994
a.maybeInitScaleDownStabilizationWindow(hpa)
982995
normalizationArg := NormalizationArg{
983996
Key: key,
@@ -987,21 +1000,42 @@ func (a *HorizontalController) normalizeDesiredReplicasWithBehaviors(hpa *autosc
9871000
MaxReplicas: hpa.Spec.MaxReplicas,
9881001
CurrentReplicas: currentReplicas,
9891002
DesiredReplicas: prenormalizedDesiredReplicas}
990-
stabilizedRecommendation, reason, message := a.stabilizeRecommendationWithBehaviors(normalizationArg)
1003+
stabilizedRecommendation, reason, message := a.stabilizeRecommendationWithBehaviors(ctx, normalizationArg)
9911004
normalizationArg.DesiredReplicas = stabilizedRecommendation
9921005
if stabilizedRecommendation != prenormalizedDesiredReplicas {
9931006
// "ScaleUpStabilized" || "ScaleDownStabilized"
9941007
setCondition(hpa, autoscalingv2.AbleToScale, v1.ConditionTrue, reason, "%s", message)
9951008
} else {
9961009
setCondition(hpa, autoscalingv2.AbleToScale, v1.ConditionTrue, "ReadyForNewScale", "recommended size matches current size")
9971010
}
998-
desiredReplicas, reason, message := a.convertDesiredReplicasWithBehaviorRate(normalizationArg)
1011+
1012+
logger := klog.FromContext(ctx)
1013+
reference := fmt.Sprintf("%s/%s/%s", hpa.Spec.ScaleTargetRef.Kind, hpa.Namespace, hpa.Spec.ScaleTargetRef.Name)
1014+
logger.V(4).Info("Normalized desired replicas with behaviors - after stabilized recommendation",
1015+
"scaleTarget", reference,
1016+
"currentReplicas", currentReplicas,
1017+
"minReplicas", minReplicas,
1018+
"stabilizedRecommendation", stabilizedRecommendation,
1019+
"prenormalizedDesiredReplicas", prenormalizedDesiredReplicas,
1020+
"reason", reason,
1021+
"message", message)
1022+
1023+
desiredReplicas, reason, message := a.convertDesiredReplicasWithBehaviorRate(ctx, normalizationArg)
9991024
if desiredReplicas == stabilizedRecommendation {
10001025
setCondition(hpa, autoscalingv2.ScalingLimited, v1.ConditionFalse, reason, "%s", message)
10011026
} else {
10021027
setCondition(hpa, autoscalingv2.ScalingLimited, v1.ConditionTrue, reason, "%s", message)
10031028
}
10041029

1030+
logger.V(4).Info("Normalized desired replicas with behaviors - after rated recommendation",
1031+
"scaleTarget", reference,
1032+
"currentReplicas", currentReplicas,
1033+
"minReplicas", minReplicas,
1034+
"stabilizedRecommendation", stabilizedRecommendation,
1035+
"desiredReplicas", desiredReplicas,
1036+
"reason", reason,
1037+
"message", message)
1038+
10051039
return desiredReplicas
10061040
}
10071041

@@ -1090,7 +1124,7 @@ func (a *HorizontalController) storeScaleEvent(behavior *autoscalingv2.Horizonta
10901124
// stabilizeRecommendationWithBehaviors:
10911125
// - replaces old recommendation with the newest recommendation,
10921126
// - returns {max,min} of recommendations that are not older than constraints.Scale{Up,Down}.DelaySeconds
1093-
func (a *HorizontalController) stabilizeRecommendationWithBehaviors(args NormalizationArg) (int32, string, string) {
1127+
func (a *HorizontalController) stabilizeRecommendationWithBehaviors(ctx context.Context, args NormalizationArg) (int32, string, string) {
10941128
now := time.Now()
10951129

10961130
foundOldSample := false
@@ -1128,6 +1162,20 @@ func (a *HorizontalController) stabilizeRecommendationWithBehaviors(args Normali
11281162
if recommendation > downRecommendation {
11291163
recommendation = downRecommendation
11301164
}
1165+
// Only keep the recommendations and ignore timestamp for logging.
1166+
var recommendations []int32
1167+
for _, rec := range a.recommendations[args.Key] {
1168+
recommendations = append(recommendations, rec.recommendation)
1169+
}
1170+
1171+
logger := klog.FromContext(ctx)
1172+
logger.V(4).Info("Stabilizing recommendation",
1173+
"key", args.Key,
1174+
"currentReplicas", args.CurrentReplicas,
1175+
"desiredReplicas", args.DesiredReplicas,
1176+
"upRecommendation", upRecommendation,
1177+
"downRecommendation", downRecommendation,
1178+
"recommendations", recommendations)
11311179

11321180
// Record the unstabilized recommendation.
11331181
if foundOldSample {
@@ -1150,14 +1198,22 @@ func (a *HorizontalController) stabilizeRecommendationWithBehaviors(args Normali
11501198

11511199
// convertDesiredReplicasWithBehaviorRate performs the actual normalization, given the constraint rate
11521200
// It doesn't consider the stabilizationWindow, it is done separately
1153-
func (a *HorizontalController) convertDesiredReplicasWithBehaviorRate(args NormalizationArg) (int32, string, string) {
1201+
func (a *HorizontalController) convertDesiredReplicasWithBehaviorRate(ctx context.Context, args NormalizationArg) (int32, string, string) {
11541202
var possibleLimitingReason, possibleLimitingMessage string
1203+
logger := klog.FromContext(ctx)
11551204

11561205
if args.DesiredReplicas > args.CurrentReplicas {
11571206
a.scaleUpEventsLock.RLock()
11581207
defer a.scaleUpEventsLock.RUnlock()
11591208
a.scaleDownEventsLock.RLock()
11601209
defer a.scaleDownEventsLock.RUnlock()
1210+
logger.V(4).Info("Converting desired replicas with behavior rate - scale up",
1211+
"key", args.Key,
1212+
"currentReplicas", args.CurrentReplicas,
1213+
"desiredReplicas", args.DesiredReplicas,
1214+
"scaleUpEvents", a.scaleUpEvents[args.Key],
1215+
"scaleDownEvents", a.scaleDownEvents[args.Key])
1216+
11611217
scaleUpLimit := calculateScaleUpLimitWithScalingRules(args.CurrentReplicas, a.scaleUpEvents[args.Key], a.scaleDownEvents[args.Key], args.ScaleUpBehavior)
11621218

11631219
if scaleUpLimit < args.CurrentReplicas {
@@ -1181,6 +1237,13 @@ func (a *HorizontalController) convertDesiredReplicasWithBehaviorRate(args Norma
11811237
defer a.scaleUpEventsLock.RUnlock()
11821238
a.scaleDownEventsLock.RLock()
11831239
defer a.scaleDownEventsLock.RUnlock()
1240+
logger.V(4).Info("Converting desired replicas with behavior rate - scale down",
1241+
"key", args.Key,
1242+
"currentReplicas", args.CurrentReplicas,
1243+
"desiredReplicas", args.DesiredReplicas,
1244+
"scaleUpEvents", a.scaleUpEvents[args.Key],
1245+
"scaleDownEvents", a.scaleDownEvents[args.Key])
1246+
11841247
scaleDownLimit := calculateScaleDownLimitWithBehaviors(args.CurrentReplicas, a.scaleUpEvents[args.Key], a.scaleDownEvents[args.Key], args.ScaleDownBehavior)
11851248

11861249
if scaleDownLimit > args.CurrentReplicas {

‎pkg/controller/podautoscaler/horizontal_test.go

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4315,6 +4315,42 @@ func TestScalingWithRules(t *testing.T) {
43154315
expectedReplicas: 16,
43164316
expectedCondition: "ScaleUpLimit",
43174317
},
4318+
{
4319+
name: "scaleUp with percent policy and no previous events, thus scaled up within range",
4320+
scaleUpEvents: generateEventsUniformDistribution([]int{0}, 120),
4321+
scaleDownEvents: generateEventsUniformDistribution([]int{0}, 120),
4322+
specMinReplicas: 3,
4323+
specMaxReplicas: 6,
4324+
scaleUpRules: generateScalingRules(0, 0, 20, 600, 300),
4325+
currentReplicas: 3,
4326+
prenormalizedDesiredReplicas: 4,
4327+
expectedReplicas: 4,
4328+
expectedCondition: "DesiredWithinRange",
4329+
},
4330+
{
4331+
name: "scaleUp with both pod and percent policy and previous scale up events, thus no scale up",
4332+
scaleUpEvents: generateEventsUniformDistribution([]int{2}, 120),
4333+
scaleDownEvents: generateEventsUniformDistribution([]int{0}, 120),
4334+
specMinReplicas: 3,
4335+
specMaxReplicas: 6,
4336+
scaleUpRules: generateScalingRules(1, 600, 20, 600, 300),
4337+
currentReplicas: 3,
4338+
prenormalizedDesiredReplicas: 4,
4339+
expectedReplicas: 3,
4340+
expectedCondition: "ScaleUpLimit",
4341+
},
4342+
{
4343+
name: "scaleUp with percent policy and previous events but still be able to scale up with limited rate",
4344+
scaleUpEvents: generateEventsUniformDistribution([]int{1}, 600),
4345+
scaleDownEvents: generateEventsUniformDistribution([]int{0}, 600),
4346+
specMinReplicas: 3,
4347+
specMaxReplicas: 20,
4348+
scaleUpRules: generateScalingRules(0, 0, 20, 600, 300),
4349+
currentReplicas: 10,
4350+
prenormalizedDesiredReplicas: 12,
4351+
expectedReplicas: 11,
4352+
expectedCondition: "ScaleUpLimit",
4353+
},
43184354
// ScaleDown with PeriodSeconds usage
43194355
{
43204356
name: "scaleDown with default policy and previous events",
@@ -4486,6 +4522,7 @@ func TestScalingWithRules(t *testing.T) {
44864522
},
44874523
}
44884524

4525+
ctx := context.Background()
44894526
for _, tc := range tests {
44904527
t.Run(tc.name, func(t *testing.T) {
44914528

@@ -4510,7 +4547,7 @@ func TestScalingWithRules(t *testing.T) {
45104547
CurrentReplicas: tc.currentReplicas,
45114548
}
45124549

4513-
replicas, condition, _ := hc.convertDesiredReplicasWithBehaviorRate(arg)
4550+
replicas, condition, _ := hc.convertDesiredReplicasWithBehaviorRate(ctx, arg)
45144551
assert.Equal(t, tc.expectedReplicas, replicas, "expected replicas do not match with converted replicas")
45154552
assert.Equal(t, tc.expectedCondition, condition, "HPA condition does not match with expected condition")
45164553
})
@@ -4865,6 +4902,7 @@ func TestNormalizeDesiredReplicasWithBehavior(t *testing.T) {
48654902
scaleDownStabilizationWindowSeconds: 300,
48664903
},
48674904
}
4905+
ctx := context.Background()
48684906
for _, tc := range tests {
48694907
t.Run(tc.name, func(t *testing.T) {
48704908
hc := HorizontalController{
@@ -4883,7 +4921,7 @@ func TestNormalizeDesiredReplicasWithBehavior(t *testing.T) {
48834921
StabilizationWindowSeconds: &tc.scaleDownStabilizationWindowSeconds,
48844922
},
48854923
}
4886-
r, _, _ := hc.stabilizeRecommendationWithBehaviors(arg)
4924+
r, _, _ := hc.stabilizeRecommendationWithBehaviors(ctx, arg)
48874925
assert.Equal(t, tc.expectedStabilizedReplicas, r, "expected replicas do not match")
48884926
if tc.expectedRecommendations != nil {
48894927
if !assert.Len(t, hc.recommendations[tc.key], len(tc.expectedRecommendations), "stored recommendations differ in length") {

0 commit comments

Comments
 (0)