Introducing option to avoid scaling if value is 0 by default

api/v1alpha1/watermarkpodautoscaler_types.go

@@ -105,6 +105,9 @@ type WatermarkPodAutoscalerSpec struct {
 	// Whether planned scale changes are actually applied
 	DryRun bool `json:"dryRun,omitempty"`
 
+	// Zero is a value that can lead to undesired outcomes, unless explicitely set the WPA will not take action if the value retrieved is 0.
+	TolerateZero bool `json:"tolerateZero,omitempty"`
+
 	// part of HorizontalPodAutoscalerSpec, see comments in the k8s-1.10.8 repo: staging/src/k8s.io/api/autoscaling/v1/types.go
 	// reference to scaled resource; horizontal pod autoscaler will learn the current resource consumption
 	// and will set the desired number of pods by using its Scale subresource.

config/crd/bases/v1/datadoghq.com_watermarkpodautoscalers.yaml

@@ -330,6 +330,11 @@ spec:
                   transition seamlessly, we validate that it is ]0;1[ in the code.
                 pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
                 x-kubernetes-int-or-string: true
+              tolerateZero:
+                description: Zero is a value that can lead to undesired outcomes,
+                  unless explicitely set the WPA will not take action if the value
+                  retrieved is 0.
+                type: boolean
               upscaleDelayAboveWatermarkSeconds:
                 format: int32
                 minimum: 0

config/crd/bases/v1beta1/datadoghq.com_watermarkpodautoscalers.yaml

@@ -261,6 +261,9 @@ spec:
               description: Parameter used to be a float, in order to support the transition seamlessly, we validate that it is ]0;1[ in the code.
               pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
               x-kubernetes-int-or-string: true
+            tolerateZero:
+              description: Zero is a value that can lead to undesired outcomes, unless explicitely set the WPA will not take action if the value retrieved is 0.
+              type: boolean
             upscaleDelayAboveWatermarkSeconds:
               format: int32
               minimum: 0

controllers/replica_calculator.go

@@ -138,12 +138,17 @@ func (c *ReplicaCalculator) GetExternalMetricReplicas(logger logr.Logger, target
 		value.Delete(prometheus.Labels{wpaNamePromLabel: wpa.Name, metricNamePromLabel: metricName})
 		return ReplicaCalculation{0, 0, time.Time{}, 0, metricPosition{}}, fmt.Errorf("unable to get external metric %s/%s/%+v: %s", wpa.Namespace, metricName, selector, err) //nolint:errorlint
 	}
-	logger.Info("Metrics from the External Metrics Provider", "metrics", metrics)
+	logger.V(2).Info("Metrics from the External Metrics Provider", "metrics", metrics)
 
 	var sum int64
 	for _, val := range metrics {
 		sum += val
 	}
+	if sum == 0 && !wpa.Spec.TolerateZero {
+		logger.V(1).Info("Retrieved 0 from the External Metrics Provider which is not ignored by default, use Spec.TolerateZero to enable")
+		// We artificially set the metricPos between the watermarks to force the controller not to scale the target.
+		return ReplicaCalculation{currentReadyReplicas, 0, timestamp, currentReadyReplicas, metricPosition{false, false}}, nil
+	}
 
 	// if the average algorithm is used, the metrics retrieved has to be divided by the number of available replicas.
 	adjustedUsage := float64(sum) / averaged

controllers/replica_calculator_test.go

@@ -1367,6 +1367,81 @@ func TestReplicaCalcAboveAbsoluteExternal_Upscale1(t *testing.T) {
 	tc.runTest(t)
 }
 
+func TestTolerateZeroDefault(t *testing.T) {
+	logf.SetLogger(zap.New())
+	metric1 := v1alpha1.MetricSpec{
+		Type: v1alpha1.ExternalMetricSourceType,
+		External: &v1alpha1.ExternalMetricSource{
+			MetricName:     "deadbeef",
+			MetricSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
+			HighWatermark:  resource.NewMilliQuantity(4000, resource.DecimalSI),
+			LowWatermark:   resource.NewMilliQuantity(2000, resource.DecimalSI),
+		},
+	}
+
+	tc := replicaCalcTestCase{
+		expectedReplicas: 3,
+		readyReplicas:    3,
+		pos: metricPosition{
+			isAbove: false,
+			isBelow: false,
+		},
+		scale: makeScale(testDeploymentName, 3, map[string]string{"name": "test-pod"}),
+		wpa: &v1alpha1.WatermarkPodAutoscaler{
+			Spec: v1alpha1.WatermarkPodAutoscalerSpec{
+				Algorithm:                    "absolute",
+				Tolerance:                    *resource.NewMilliQuantity(20, resource.DecimalSI),
+				Metrics:                      []v1alpha1.MetricSpec{metric1},
+				ReplicaScalingAbsoluteModulo: v1alpha1.NewInt32(1),
+			},
+		},
+		metric: &metricInfo{
+			spec:                metric1,
+			levels:              []int64{0}, // We are much lower than the LowWatermark
+			expectedUtilization: 0,
+		},
+	}
+	tc.runTest(t)
+}
+
+func TestTolerateZeroEnabled(t *testing.T) {
+	logf.SetLogger(zap.New())
+	metric1 := v1alpha1.MetricSpec{
+		Type: v1alpha1.ExternalMetricSourceType,
+		External: &v1alpha1.ExternalMetricSource{
+			MetricName:     "deadbeef",
+			MetricSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
+			HighWatermark:  resource.NewMilliQuantity(4000, resource.DecimalSI),
+			LowWatermark:   resource.NewMilliQuantity(2000, resource.DecimalSI),
+		},
+	}
+
+	tc := replicaCalcTestCase{
+		expectedReplicas: 1,
+		readyReplicas:    3,
+		pos: metricPosition{
+			isAbove: false,
+			isBelow: true,
+		},
+		scale: makeScale(testDeploymentName, 3, map[string]string{"name": "test-pod"}),
+		wpa: &v1alpha1.WatermarkPodAutoscaler{
+			Spec: v1alpha1.WatermarkPodAutoscalerSpec{
+				Algorithm:                    "absolute",
+				TolerateZero:                 true,
+				Tolerance:                    *resource.NewMilliQuantity(20, resource.DecimalSI),
+				Metrics:                      []v1alpha1.MetricSpec{metric1},
+				ReplicaScalingAbsoluteModulo: v1alpha1.NewInt32(1),
+			},
+		},
+		metric: &metricInfo{
+			spec:                metric1,
+			levels:              []int64{0}, // We are much lower than the LowWatermark
+			expectedUtilization: 0,
+		},
+	}
+	tc.runTest(t)
+}
+
 func TestReplicaCalcAboveAbsoluteExternal_Upscale2(t *testing.T) {
 	logf.SetLogger(zap.New())