diff --git a/pkg/controller/podautoscaler/horizontal.go b/pkg/controller/podautoscaler/horizontal.go index 4517b0d7299..553764b911b 100644 --- a/pkg/controller/podautoscaler/horizontal.go +++ b/pkg/controller/podautoscaler/horizontal.go @@ -320,26 +320,52 @@ func (a *HorizontalController) reconcileKey(key string) (deleted bool, err error // computeStatusForObjectMetric computes the desired number of replicas for the specified metric of type ObjectMetricSourceType. func (a *HorizontalController) computeStatusForObjectMetric(currentReplicas int32, metricSpec autoscalingv2.MetricSpec, hpa *autoscalingv2.HorizontalPodAutoscaler, selector labels.Selector, status *autoscalingv2.MetricStatus, metricSelector labels.Selector) (int32, time.Time, string, error) { - replicaCountProposal, utilizationProposal, timestampProposal, err := a.replicaCalc.GetObjectMetricReplicas(currentReplicas, metricSpec.Object.Target.Value.MilliValue(), metricSpec.Object.Metric.Name, hpa.Namespace, &metricSpec.Object.DescribedObject, selector, metricSelector) - if err != nil { - a.eventRecorder.Event(hpa, v1.EventTypeWarning, "FailedGetObjectMetric", err.Error()) - setCondition(hpa, autoscalingv2.ScalingActive, v1.ConditionFalse, "FailedGetObjectMetric", "the HPA was unable to compute the replica count: %v", err) - return 0, timestampProposal, "", err - } - *status = autoscalingv2.MetricStatus{ - Type: autoscalingv2.ObjectMetricSourceType, - Object: &autoscalingv2.ObjectMetricStatus{ - DescribedObject: metricSpec.Object.DescribedObject, - Metric: autoscalingv2.MetricIdentifier{ - Name: metricSpec.Object.Metric.Name, - Selector: metricSpec.Object.Metric.Selector, + if metricSpec.Object.Target.Type == autoscalingv2.ValueMetricType { + replicaCountProposal, utilizationProposal, timestampProposal, err := a.replicaCalc.GetObjectMetricReplicas(currentReplicas, metricSpec.Object.Target.Value.MilliValue(), metricSpec.Object.Metric.Name, hpa.Namespace, &metricSpec.Object.DescribedObject, selector, metricSelector) + if err != nil { + a.eventRecorder.Event(hpa, v1.EventTypeWarning, "FailedGetObjectMetric", err.Error()) + setCondition(hpa, autoscalingv2.ScalingActive, v1.ConditionFalse, "FailedGetObjectMetric", "the HPA was unable to compute the replica count: %v", err) + return 0, timestampProposal, "", err + } + *status = autoscalingv2.MetricStatus{ + Type: autoscalingv2.ObjectMetricSourceType, + Object: &autoscalingv2.ObjectMetricStatus{ + DescribedObject: metricSpec.Object.DescribedObject, + Metric: autoscalingv2.MetricIdentifier{ + Name: metricSpec.Object.Metric.Name, + Selector: metricSpec.Object.Metric.Selector, + }, + Current: autoscalingv2.MetricValueStatus{ + Value: resource.NewMilliQuantity(utilizationProposal, resource.DecimalSI), + }, }, - Current: autoscalingv2.MetricValueStatus{ - Value: resource.NewMilliQuantity(utilizationProposal, resource.DecimalSI), + } + return replicaCountProposal, timestampProposal, fmt.Sprintf("%s metric %s", metricSpec.Object.DescribedObject.Kind, metricSpec.Object.Metric.Name), nil + } else if metricSpec.Object.Target.Type == autoscalingv2.AverageValueMetricType { + replicaCountProposal, utilizationProposal, timestampProposal, err := a.replicaCalc.GetObjectPerPodMetricReplicas(currentReplicas, metricSpec.Object.Target.AverageValue.MilliValue(), metricSpec.Object.Metric.Name, hpa.Namespace, &metricSpec.Object.DescribedObject, metricSelector) + if err != nil { + a.eventRecorder.Event(hpa, v1.EventTypeWarning, "FailedGetObjectMetric", err.Error()) + setCondition(hpa, autoscalingv2.ScalingActive, v1.ConditionFalse, "FailedGetObjectMetric", "the HPA was unable to compute the replica count: %v", err) + return 0, time.Time{}, "", fmt.Errorf("failed to get %s object metric: %v", metricSpec.Object.Metric.Name, err) + } + *status = autoscalingv2.MetricStatus{ + Type: autoscalingv2.ObjectMetricSourceType, + Object: &autoscalingv2.ObjectMetricStatus{ + Metric: autoscalingv2.MetricIdentifier{ + Name: metricSpec.Object.Metric.Name, + Selector: metricSpec.Object.Metric.Selector, + }, + Current: autoscalingv2.MetricValueStatus{ + AverageValue: resource.NewMilliQuantity(utilizationProposal, resource.DecimalSI), + }, }, - }, + } + return replicaCountProposal, timestampProposal, fmt.Sprintf("external metric %s(%+v)", metricSpec.Object.Metric.Name, metricSpec.Object.Metric.Selector), nil } - return replicaCountProposal, timestampProposal, fmt.Sprintf("%s metric %s", metricSpec.Object.DescribedObject.Kind, metricSpec.Object.Metric.Name), nil + errMsg := "invalid object metric source: neither a value target nor an average value target was set" + a.eventRecorder.Event(hpa, v1.EventTypeWarning, "FailedGetObjectMetric", errMsg) + setCondition(hpa, autoscalingv2.ScalingActive, v1.ConditionFalse, "FailedGetObjectMetric", "the HPA was unable to compute the replica count: %s", errMsg) + return 0, time.Time{}, "", fmt.Errorf(errMsg) } // computeStatusForPodsMetric computes the desired number of replicas for the specified metric of type PodsMetricSourceType. diff --git a/pkg/controller/podautoscaler/horizontal_test.go b/pkg/controller/podautoscaler/horizontal_test.go index e9f57836f4f..406c77ffff9 100644 --- a/pkg/controller/podautoscaler/horizontal_test.go +++ b/pkg/controller/podautoscaler/horizontal_test.go @@ -1051,6 +1051,37 @@ func TestScaleUpCMObject(t *testing.T) { tc.runTest(t) } +func TestScaleUpPerPodCMObject(t *testing.T) { + targetAverageValue := resource.MustParse("10.0") + tc := testCase{ + minReplicas: 2, + maxReplicas: 6, + initialReplicas: 3, + expectedDesiredReplicas: 4, + CPUTarget: 0, + metricsTarget: []autoscalingv2.MetricSpec{ + { + Type: autoscalingv2.ObjectMetricSourceType, + Object: &autoscalingv2.ObjectMetricSource{ + DescribedObject: autoscalingv2.CrossVersionObjectReference{ + APIVersion: "apps/v1", + Kind: "Deployment", + Name: "some-deployment", + }, + Metric: autoscalingv2.MetricIdentifier{ + Name: "qps", + }, + Target: autoscalingv2.MetricTarget{ + AverageValue: &targetAverageValue, + }, + }, + }, + }, + reportedLevels: []uint64{40000}, + } + tc.runTest(t) +} + func TestScaleUpCMExternal(t *testing.T) { tc := testCase{ minReplicas: 2, @@ -1203,6 +1234,39 @@ func TestScaleDownCMObject(t *testing.T) { tc.runTest(t) } +func TestScaleDownPerPodCMObject(t *testing.T) { + targetAverageValue := resource.MustParse("20.0") + tc := testCase{ + minReplicas: 2, + maxReplicas: 6, + initialReplicas: 5, + expectedDesiredReplicas: 3, + CPUTarget: 0, + metricsTarget: []autoscalingv2.MetricSpec{ + { + Type: autoscalingv2.ObjectMetricSourceType, + Object: &autoscalingv2.ObjectMetricSource{ + DescribedObject: autoscalingv2.CrossVersionObjectReference{ + APIVersion: "apps/v1", + Kind: "Deployment", + Name: "some-deployment", + }, + Metric: autoscalingv2.MetricIdentifier{ + Name: "qps", + }, + Target: autoscalingv2.MetricTarget{ + AverageValue: &targetAverageValue, + }, + }, + }, + }, + reportedLevels: []uint64{60000}, + reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, + recommendations: []timestampedRecommendation{}, + } + tc.runTest(t) +} + func TestScaleDownCMExternal(t *testing.T) { tc := testCase{ minReplicas: 2, @@ -1446,6 +1510,41 @@ func TestToleranceCMExternal(t *testing.T) { tc.runTest(t) } +func TestTolerancePerPodCMObject(t *testing.T) { + tc := testCase{ + minReplicas: 2, + maxReplicas: 6, + initialReplicas: 4, + expectedDesiredReplicas: 4, + metricsTarget: []autoscalingv2.MetricSpec{ + { + Type: autoscalingv2.ObjectMetricSourceType, + Object: &autoscalingv2.ObjectMetricSource{ + DescribedObject: autoscalingv2.CrossVersionObjectReference{ + APIVersion: "apps/v1", + Kind: "Deployment", + Name: "some-deployment", + }, + Metric: autoscalingv2.MetricIdentifier{ + Name: "qps", + Selector: &metav1.LabelSelector{}, + }, + Target: autoscalingv2.MetricTarget{ + AverageValue: resource.NewMilliQuantity(2200, resource.DecimalSI), + }, + }, + }, + }, + reportedLevels: []uint64{8600}, + expectedConditions: statusOkWithOverrides(autoscalingv2.HorizontalPodAutoscalerCondition{ + Type: autoscalingv2.AbleToScale, + Status: v1.ConditionTrue, + Reason: "ReadyForNewScale", + }), + } + tc.runTest(t) +} + func TestTolerancePerPodCMExternal(t *testing.T) { tc := testCase{ minReplicas: 2, diff --git a/pkg/controller/podautoscaler/replica_calculator.go b/pkg/controller/podautoscaler/replica_calculator.go index a1eb82794f8..ace9803bdf3 100644 --- a/pkg/controller/podautoscaler/replica_calculator.go +++ b/pkg/controller/podautoscaler/replica_calculator.go @@ -257,6 +257,23 @@ func (c *ReplicaCalculator) GetObjectMetricReplicas(currentReplicas int32, targe return replicaCount, utilization, timestamp, nil } +// GetObjectPerPodMetricReplicas calculates the desired replica count based on a target metric utilization (as a milli-value) +// for the given object in the given namespace, and the current replica count. +func (c *ReplicaCalculator) GetObjectPerPodMetricReplicas(currentReplicas int32, targetAverageUtilization int64, metricName string, namespace string, objectRef *autoscaling.CrossVersionObjectReference, metricSelector labels.Selector) (replicaCount int32, utilization int64, timestamp time.Time, err error) { + utilization, timestamp, err = c.metricsClient.GetObjectMetric(metricName, namespace, objectRef, metricSelector) + if err != nil { + return 0, 0, time.Time{}, fmt.Errorf("unable to get metric %s: %v on %s %s/%s", metricName, objectRef.Kind, namespace, objectRef.Name, err) + } + + usageRatio := float64(utilization) / (float64(targetAverageUtilization) * float64(replicaCount)) + if math.Abs(1.0-usageRatio) > c.tolerance { + // update number of replicas if change is large enough + replicaCount = int32(math.Ceil(float64(utilization) / float64(targetAverageUtilization))) + } + utilization = int64(math.Ceil(float64(utilization) / float64(currentReplicas))) + return replicaCount, utilization, timestamp, nil +} + // @TODO(mattjmcnaughton) Many different functions in this module use variations // of this function. Make this function generic, so we don't repeat the same // logic in multiple places. diff --git a/pkg/controller/podautoscaler/replica_calculator_test.go b/pkg/controller/podautoscaler/replica_calculator_test.go index c3342d62605..0638f89341f 100644 --- a/pkg/controller/podautoscaler/replica_calculator_test.go +++ b/pkg/controller/podautoscaler/replica_calculator_test.go @@ -64,6 +64,7 @@ type metricType int const ( objectMetric metricType = iota + objectPerPodMetric externalMetric externalPerPodMetric podMetric @@ -384,6 +385,11 @@ func (tc *replicaCalcTestCase) runTest(t *testing.T) { t.Fatal("Metric specified as objectMetric but metric.singleObject is nil.") } outReplicas, outUtilization, outTimestamp, err = replicaCalc.GetObjectMetricReplicas(tc.currentReplicas, tc.metric.targetUtilization, tc.metric.name, testNamespace, tc.metric.singleObject, selector, nil) + case objectPerPodMetric: + if tc.metric.singleObject == nil { + t.Fatal("Metric specified as objectMetric but metric.singleObject is nil.") + } + outReplicas, outUtilization, outTimestamp, err = replicaCalc.GetObjectPerPodMetricReplicas(tc.currentReplicas, tc.metric.perPodTargetUtilization, tc.metric.name, testNamespace, tc.metric.singleObject, nil) case externalMetric: if tc.metric.selector == nil { t.Fatal("Metric specified as externalMetric but metric.selector is nil.") @@ -631,6 +637,26 @@ func TestReplicaCalcScaleUpCMObject(t *testing.T) { tc.runTest(t) } +func TestReplicaCalcScaleUpCMPerPodObject(t *testing.T) { + tc := replicaCalcTestCase{ + currentReplicas: 3, + expectedReplicas: 4, + metric: &metricInfo{ + metricType: objectPerPodMetric, + name: "qps", + levels: []int64{20000}, + perPodTargetUtilization: 5000, + expectedUtilization: 6667, + singleObject: &autoscalingv2.CrossVersionObjectReference{ + Kind: "Deployment", + APIVersion: "apps/v1", + Name: "some-deployment", + }, + }, + } + tc.runTest(t) +} + func TestReplicaCalcScaleUpCMObjectIgnoresUnreadyPods(t *testing.T) { tc := replicaCalcTestCase{ currentReplicas: 3, @@ -747,6 +773,26 @@ func TestReplicaCalcScaleDownCM(t *testing.T) { tc.runTest(t) } +func TestReplicaCalcScaleDownPerPodCMObject(t *testing.T) { + tc := replicaCalcTestCase{ + currentReplicas: 5, + expectedReplicas: 3, + metric: &metricInfo{ + name: "qps", + levels: []int64{6000}, + perPodTargetUtilization: 2000, + expectedUtilization: 1200, + singleObject: &autoscalingv2.CrossVersionObjectReference{ + Kind: "Deployment", + APIVersion: "apps/v1", + Name: "some-deployment", + }, + metricType: objectPerPodMetric, + }, + } + tc.runTest(t) +} + func TestReplicaCalcScaleDownCMObject(t *testing.T) { tc := replicaCalcTestCase{ currentReplicas: 5, diff --git a/pkg/kubectl/describe/versioned/describe.go b/pkg/kubectl/describe/versioned/describe.go index 207fd88d8cc..e648ca486cb 100644 --- a/pkg/kubectl/describe/versioned/describe.go +++ b/pkg/kubectl/describe/versioned/describe.go @@ -3111,11 +3111,20 @@ func describeHorizontalPodAutoscaler(hpa *autoscalingv2beta2.HorizontalPodAutosc } w.Write(LEVEL_1, "%q on pods:\t%s / %s\n", metric.Pods.Metric.Name, current, metric.Pods.Target.AverageValue.String()) case autoscalingv2beta2.ObjectMetricSourceType: - current := "" - if len(hpa.Status.CurrentMetrics) > i && hpa.Status.CurrentMetrics[i].Object != nil { - current = hpa.Status.CurrentMetrics[i].Object.Current.Value.String() + w.Write(LEVEL_1, "\"%s\" on %s/%s ", metric.Object.Metric.Name, metric.Object.DescribedObject.Kind, metric.Object.DescribedObject.Name) + if metric.Object.Target.Type == autoscalingv2beta2.AverageValueMetricType { + current := "" + if len(hpa.Status.CurrentMetrics) > i && hpa.Status.CurrentMetrics[i].Object != nil { + current = hpa.Status.CurrentMetrics[i].Object.Current.AverageValue.String() + } + w.Write(LEVEL_0, "(target average value):\t%s / %s\n", current, metric.Object.Target.AverageValue.String()) + } else { + current := "" + if len(hpa.Status.CurrentMetrics) > i && hpa.Status.CurrentMetrics[i].Object != nil { + current = hpa.Status.CurrentMetrics[i].Object.Current.Value.String() + } + w.Write(LEVEL_0, "(target value):\t%s / %s\n", current, metric.Object.Target.Value.String()) } - w.Write(LEVEL_1, "%q on %s/%s:\t%s / %s\n", metric.Object.Metric.Name, metric.Object.DescribedObject.Kind, metric.Object.DescribedObject.Name, current, metric.Object.Target.Value.String()) case autoscalingv2beta2.ResourceMetricSourceType: w.Write(LEVEL_1, "resource %s on pods", string(metric.Resource.Name)) if metric.Resource.Target.AverageValue != nil { diff --git a/pkg/kubectl/describe/versioned/describe_test.go b/pkg/kubectl/describe/versioned/describe_test.go index 1f7689df14c..40e1116241b 100644 --- a/pkg/kubectl/describe/versioned/describe_test.go +++ b/pkg/kubectl/describe/versioned/describe_test.go @@ -1880,7 +1880,94 @@ func TestDescribeHorizontalPodAutoscaler(t *testing.T) { }, }, { - "object source type (no current)", + "object source type target average value (no current)", + autoscalingv2beta2.HorizontalPodAutoscaler{ + Spec: autoscalingv2beta2.HorizontalPodAutoscalerSpec{ + ScaleTargetRef: autoscalingv2beta2.CrossVersionObjectReference{ + Name: "some-rc", + Kind: "ReplicationController", + }, + MinReplicas: &minReplicasVal, + MaxReplicas: 10, + Metrics: []autoscalingv2beta2.MetricSpec{ + { + Type: autoscalingv2beta2.ObjectMetricSourceType, + Object: &autoscalingv2beta2.ObjectMetricSource{ + DescribedObject: autoscalingv2beta2.CrossVersionObjectReference{ + Name: "some-service", + Kind: "Service", + }, + Metric: autoscalingv2beta2.MetricIdentifier{ + Name: "some-service-metric", + }, + Target: autoscalingv2beta2.MetricTarget{ + Type: autoscalingv2beta2.AverageValueMetricType, + AverageValue: resource.NewMilliQuantity(100, resource.DecimalSI), + }, + }, + }, + }, + }, + Status: autoscalingv2beta2.HorizontalPodAutoscalerStatus{ + CurrentReplicas: 4, + DesiredReplicas: 5, + }, + }, + }, + { + "object source type target average value (with current)", + autoscalingv2beta2.HorizontalPodAutoscaler{ + Spec: autoscalingv2beta2.HorizontalPodAutoscalerSpec{ + ScaleTargetRef: autoscalingv2beta2.CrossVersionObjectReference{ + Name: "some-rc", + Kind: "ReplicationController", + }, + MinReplicas: &minReplicasVal, + MaxReplicas: 10, + Metrics: []autoscalingv2beta2.MetricSpec{ + { + Type: autoscalingv2beta2.ObjectMetricSourceType, + Object: &autoscalingv2beta2.ObjectMetricSource{ + DescribedObject: autoscalingv2beta2.CrossVersionObjectReference{ + Name: "some-service", + Kind: "Service", + }, + Metric: autoscalingv2beta2.MetricIdentifier{ + Name: "some-service-metric", + }, + Target: autoscalingv2beta2.MetricTarget{ + Type: autoscalingv2beta2.AverageValueMetricType, + AverageValue: resource.NewMilliQuantity(100, resource.DecimalSI), + }, + }, + }, + }, + }, + Status: autoscalingv2beta2.HorizontalPodAutoscalerStatus{ + CurrentReplicas: 4, + DesiredReplicas: 5, + CurrentMetrics: []autoscalingv2beta2.MetricStatus{ + { + Type: autoscalingv2beta2.ObjectMetricSourceType, + Object: &autoscalingv2beta2.ObjectMetricStatus{ + DescribedObject: autoscalingv2beta2.CrossVersionObjectReference{ + Name: "some-service", + Kind: "Service", + }, + Metric: autoscalingv2beta2.MetricIdentifier{ + Name: "some-service-metric", + }, + Current: autoscalingv2beta2.MetricValueStatus{ + AverageValue: resource.NewMilliQuantity(50, resource.DecimalSI), + }, + }, + }, + }, + }, + }, + }, + { + "object source type target value (no current)", autoscalingv2beta2.HorizontalPodAutoscaler{ Spec: autoscalingv2beta2.HorizontalPodAutoscalerSpec{ ScaleTargetRef: autoscalingv2beta2.CrossVersionObjectReference{ @@ -1915,7 +2002,7 @@ func TestDescribeHorizontalPodAutoscaler(t *testing.T) { }, }, { - "object source type (with current)", + "object source type target value (with current)", autoscalingv2beta2.HorizontalPodAutoscaler{ Spec: autoscalingv2beta2.HorizontalPodAutoscalerSpec{ ScaleTargetRef: autoscalingv2beta2.CrossVersionObjectReference{