diff --git a/CHANGELOG.md b/CHANGELOG.md index 08f3ca53462..179a0e27267 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,7 @@ Here is an overview of all new **experimental** features: ### Fixes +- **General**: Fix regression in fallback mechanism ([#4249](https://github.com/kedacore/keda/issues/4249)) - **General**: Prevent a panic that might occur while refreshing a scaler cache ([#4092](https://github.com/kedacore/keda/issues/4092)) - **Azure Service Bus Scaler:** Use correct auth flows with pod identity ([#4026](https://github.com/kedacore/keda/issues/4026)|[#4123](https://github.com/kedacore/keda/issues/4123)) - **Cassandra Scaler**: Checking whether the port information is entered in the ClusterIPAddres is done correctly. ([#4110](https://github.com/kedacore/keda/issues/4110)) diff --git a/pkg/fallback/fallback.go b/pkg/fallback/fallback.go index b14cf018669..286144692f0 100644 --- a/pkg/fallback/fallback.go +++ b/pkg/fallback/fallback.go @@ -100,10 +100,10 @@ func validateFallback(scaledObject *kedav1alpha1.ScaledObject) bool { func doFallback(scaledObject *kedav1alpha1.ScaledObject, metricSpec v2.MetricSpec, metricName string, suppressedError error) []external_metrics.ExternalMetricValue { replicas := int64(scaledObject.Spec.Fallback.Replicas) - normalisationValue, _ := metricSpec.External.Target.AverageValue.AsInt64() + normalisationValue := metricSpec.External.Target.AverageValue.AsApproximateFloat64() metric := external_metrics.ExternalMetricValue{ MetricName: metricName, - Value: *resource.NewQuantity(normalisationValue*replicas, resource.DecimalSI), + Value: *resource.NewMilliQuantity(int64(normalisationValue*1000)*replicas, resource.DecimalSI), Timestamp: metav1.Now(), } fallbackMetrics := []external_metrics.ExternalMetricValue{metric} diff --git a/pkg/fallback/fallback_test.go b/pkg/fallback/fallback_test.go index 726e196f38f..9f10ff8aad2 100644 --- a/pkg/fallback/fallback_test.go +++ b/pkg/fallback/fallback_test.go @@ -63,7 +63,7 @@ var _ = Describe("fallback", func() { It("should return the expected metric when fallback is disabled", func() { - expectedMetricValue := int64(5) + expectedMetricValue := float64(5) primeGetMetrics(scaler, expectedMetricValue) so := buildScaledObject(nil, nil) metricSpec := createMetricSpec(3) @@ -73,12 +73,12 @@ var _ = Describe("fallback", func() { metrics, err = GetMetricsWithFallback(context.Background(), client, metrics, err, metricName, so, metricSpec) Expect(err).ToNot(HaveOccurred()) - value, _ := metrics[0].Value.AsInt64() + value := metrics[0].Value.AsApproximateFloat64() Expect(value).Should(Equal(expectedMetricValue)) }) It("should reset the health status when scaler metrics are available", func() { - expectedMetricValue := int64(6) + expectedMetricValue := float64(6) startingNumberOfFailures := int32(5) primeGetMetrics(scaler, expectedMetricValue) @@ -104,7 +104,7 @@ var _ = Describe("fallback", func() { metrics, err = GetMetricsWithFallback(context.Background(), client, metrics, err, metricName, so, metricSpec) Expect(err).ToNot(HaveOccurred()) - value, _ := metrics[0].Value.AsInt64() + value := metrics[0].Value.AsApproximateFloat64() Expect(value).Should(Equal(expectedMetricValue)) Expect(so.Status.Health[metricName]).To(haveFailureAndStatus(0, kedav1alpha1.HealthStatusHappy)) }) @@ -156,7 +156,7 @@ var _ = Describe("fallback", func() { It("should return a normalised metric when number of failures are beyond threshold", func() { scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Eq(metricName)).Return(nil, false, errors.New("Some error")) startingNumberOfFailures := int32(3) - expectedMetricValue := int64(100) + expectedMetricValue := float64(100) so := buildScaledObject( &kedav1alpha1.Fallback{ @@ -179,7 +179,7 @@ var _ = Describe("fallback", func() { metrics, err = GetMetricsWithFallback(context.Background(), client, metrics, err, metricName, so, metricSpec) Expect(err).ToNot(HaveOccurred()) - value, _ := metrics[0].Value.AsInt64() + value := metrics[0].Value.AsApproximateFloat64() Expect(value).Should(Equal(expectedMetricValue)) Expect(so.Status.Health[metricName]).To(haveFailureAndStatus(4, kedav1alpha1.HealthStatusFailing)) }) @@ -209,7 +209,7 @@ var _ = Describe("fallback", func() { It("should ignore error if we fail to update kubernetes status", func() { scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Eq(metricName)).Return(nil, false, errors.New("Some error")) startingNumberOfFailures := int32(3) - expectedMetricValue := int64(100) + expectedMetricValue := float64(100) so := buildScaledObject( &kedav1alpha1.Fallback{ @@ -235,7 +235,7 @@ var _ = Describe("fallback", func() { metrics, err = GetMetricsWithFallback(context.Background(), client, metrics, err, metricName, so, metricSpec) Expect(err).ToNot(HaveOccurred()) - value, _ := metrics[0].Value.AsInt64() + value := metrics[0].Value.AsApproximateFloat64() Expect(value).Should(Equal(expectedMetricValue)) Expect(so.Status.Health[metricName]).To(haveFailureAndStatus(4, kedav1alpha1.HealthStatusFailing)) }) @@ -411,10 +411,10 @@ func buildScaledObject(fallbackConfig *kedav1alpha1.Fallback, status *kedav1alph return scaledObject } -func primeGetMetrics(scaler *mock_scalers.MockScaler, value int64) { +func primeGetMetrics(scaler *mock_scalers.MockScaler, value float64) { expectedMetric := external_metrics.ExternalMetricValue{ MetricName: metricName, - Value: *resource.NewQuantity(value, resource.DecimalSI), + Value: *resource.NewQuantity(int64(value), resource.DecimalSI), Timestamp: metav1.Now(), } diff --git a/tests/internals/fallback/fallback_test.go b/tests/internals/fallback/fallback_test.go index e75f6e89eba..7073a594be1 100644 --- a/tests/internals/fallback/fallback_test.go +++ b/tests/internals/fallback/fallback_test.go @@ -175,7 +175,13 @@ spec: fallback: failureThreshold: 3 replicas: {{.DefaultFallback}} + advanced: + horizontalPodAutoscalerConfig: + behavior: + scaleDown: + stabilizationWindowSeconds: 1 cooldownPeriod: 1 + pollingInterval: 5 triggers: - type: metrics-api metadata: @@ -194,6 +200,7 @@ metadata: name: update-ms-value namespace: {{.Namespace}} spec: + ttlSecondsAfterFinished: 30 template: spec: containers: @@ -252,12 +259,18 @@ func testFallback(t *testing.T, kc *kubernetes.Clientset, data templateData) { KubectlApplyWithTemplate(t, data, "fallbackMSDeploymentTemplate", fallbackMSDeploymentTemplate) assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, namespace, defaultFallback, 60, 3), "replica count should be %d after 3 minutes", defaultFallback) + // We need to ensure that the fallback value is stable to cover this regression + // https://github.com/kedacore/keda/issues/4249 + AssertReplicaCountNotChangeDuringTimePeriod(t, kc, deploymentName, namespace, defaultFallback, 180) } // restore MS to scale back from fallback replicas func testRestoreAfterFallback(t *testing.T, kc *kubernetes.Clientset, data templateData) { t.Log("--- testing after fallback ---") KubectlApplyWithTemplate(t, data, "metricsServerDeploymentTemplate", metricsServerDeploymentTemplate) + data.MetricValue = 50 + KubectlApplyWithTemplate(t, data, "updateMetricsTemplate", updateMetricsTemplate) + assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, namespace, maxReplicas, 60, 3), "replica count should be %d after 3 minutes", maxReplicas) }