Autoscaler fails to update spec.replicas when scaling boundaries change

operator/elasticsearch.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"math"
 	"net/url"
 	"sync"
 	"time"
@@ -816,14 +815,18 @@ type ESResource struct {
 	Pods                 []v1.Pod
 }
 
-// Replicas returns the desired node replicas of an ElasticsearchDataSet
-// In case it was not specified, it will return '1'.
+// Replicas returns the desired node replicas of an ElasticsearchDataSet.
+// For implementation details, see edsReplicas.
 func (es *ESResource) Replicas() int32 {
 	return edsReplicas(es.ElasticsearchDataSet)
 }
 
 // edsReplicas returns the desired node replicas of an ElasticsearchDataSet
-// In case it was not specified, it will return '1'.
+// as determined through spec.Replicas and autoscaling settings.
+// If unset, and autoscaling is disabled, it will return 1 as the default value.
+// In case autoscaling is enabled and spec.Replicas is nil, it will return 0,
+// leaving the actual scaling target to be calculated by scaleEDS, which will
+// then set spec.Replicas accordingly.
 func edsReplicas(eds *zv1.ElasticsearchDataSet) int32 {
 	scaling := eds.Spec.Scaling
 	if scaling == nil || !scaling.Enabled {
@@ -832,13 +835,11 @@ func edsReplicas(eds *zv1.ElasticsearchDataSet) int32 {
 		}
 		return *eds.Spec.Replicas
 	}
-	// initialize with minReplicas
-	minReplicas := eds.Spec.Scaling.MinReplicas
+	// initialize with 0
 	if eds.Spec.Replicas == nil {
-		return minReplicas
+		return 0
 	}
-	currentReplicas := *eds.Spec.Replicas
-	return int32(math.Max(float64(currentReplicas), float64(scaling.MinReplicas)))
+	return *eds.Spec.Replicas
 }
 
 // collectResources collects all the ElasticsearchDataSet resources and there

operator/elasticsearch_test.go

@@ -245,3 +245,68 @@ func TestValidateScalingSettings(tt *testing.T) {
 		})
 	}
 }
+
+func TestEDSReplicas(t *testing.T) {
+	one := int32(1)
+	three := int32(3)
+	four := int32(4)
+
+	for _, tc := range []struct {
+		name     string
+		eds      *zv1.ElasticsearchDataSet
+		expected int32
+	}{
+		{
+			name: "scaling disabled, replicas nil -> default 1",
+			eds: &zv1.ElasticsearchDataSet{
+				Spec: zv1.ElasticsearchDataSetSpec{},
+			},
+			expected: 1,
+		},
+		{
+			name: "scaling disabled, replicas set -> value",
+			eds: &zv1.ElasticsearchDataSet{
+				Spec: zv1.ElasticsearchDataSetSpec{
+					Replicas: &three,
+				},
+			},
+			expected: 3,
+		},
+		{
+			name: "scaling enabled, replicas nil -> 0",
+			eds: &zv1.ElasticsearchDataSet{
+				Spec: zv1.ElasticsearchDataSetSpec{
+					Scaling: &zv1.ElasticsearchDataSetScaling{Enabled: true},
+				},
+			},
+			expected: 0,
+		},
+		{
+			name: "scaling enabled, replicas set -> value",
+			eds: &zv1.ElasticsearchDataSet{
+				Spec: zv1.ElasticsearchDataSetSpec{
+					Scaling:  &zv1.ElasticsearchDataSetScaling{Enabled: true},
+					Replicas: &one,
+				},
+			},
+			expected: 1,
+		},
+		{
+			name: "scaling enabled, min and max replicas > spec.replicas",
+			eds: &zv1.ElasticsearchDataSet{
+				Spec: zv1.ElasticsearchDataSetSpec{
+					Scaling:  &zv1.ElasticsearchDataSetScaling{Enabled: true, MinReplicas: four, MaxReplicas: four},
+					Replicas: &three,
+				},
+			},
+			// edsReplicas should reflect the current scaling target
+			// ensuring bounds is responsibility of the autoscaler
+			expected: 3,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			actual := edsReplicas(tc.eds)
+			assert.Equal(t, tc.expected, actual)
+		})
+	}
+}