Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions operator/elasticsearch.go
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,22 @@ func (o *ElasticsearchOperator) scaleEDS(ctx context.Context, eds *zv1.Elasticse
namespace := eds.Namespace

currentReplicas := edsReplicas(eds)

// Prevent writing 0 to spec.replicas when it would violate minReplicas
// This handles the case where:
// - spec.replicas is nil (e.g., after kubectl patch)
// - edsReplicas returns 0 for autoscaling initialization
// - autoscaler returns no-op (e.g., excludeSystemIndices filters all indices)
if currentReplicas == 0 && scaling != nil && scaling.MinReplicas > 0 {
// Prefer status.replicas (reflects actual StatefulSet state)
if eds.Status.Replicas > 0 {
currentReplicas = eds.Status.Replicas
} else {
// Fallback to minReplicas for new/uninitialized EDS
currentReplicas = scaling.MinReplicas
}
}

eds.Spec.Replicas = &currentReplicas
as := NewAutoScaler(es, o.metricsInterval, client)

Expand Down
105 changes: 105 additions & 0 deletions operator/elasticsearch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func TestHasOwnership(t *testing.T) {
operator := &ElasticsearchOperator{
operatorID: "my-operator",
}

assert.True(t, operator.hasOwnership(eds))

eds.Annotations[esOperatorAnnotationKey] = "not-my-operator"
Expand Down Expand Up @@ -310,3 +311,107 @@ func TestEDSReplicas(t *testing.T) {
})
}
}

// TestScaleToZeroPrevention validates that the defensive logic in scaleEDS prevents
// writing 0 to spec.replicas when it would violate minReplicas.
// This test documents the fix for the regression introduced in PR #511 where:
// - kubectl patch operations could leave spec.replicas as nil
// - edsReplicas would return 0 for autoscaling-enabled EDS
// - When autoscaler returned no-op (e.g., excludeSystemIndices filters all indices),
// spec.replicas would be written as 0, violating minReplicas
func TestScaleToZeroPrevention(t *testing.T) {
minReplicas := int32(3)
maxReplicas := int32(10)
statusReplicas := int32(5)

for _, tc := range []struct {
name string
eds *zv1.ElasticsearchDataSet
expectedReplicas int32
description string
}{
{
name: "nil replicas + status replicas set -> use status",
eds: &zv1.ElasticsearchDataSet{
ObjectMeta: metav1.ObjectMeta{
Name: "test-eds",
Namespace: "default",
},
Spec: zv1.ElasticsearchDataSetSpec{
Replicas: nil, // Cleared by kubectl patch
Scaling: &zv1.ElasticsearchDataSetScaling{
Enabled: true,
MinReplicas: minReplicas,
MaxReplicas: maxReplicas,
},
},
Status: zv1.ElasticsearchDataSetStatus{
Replicas: statusReplicas, // Actually running 5 replicas
},
},
expectedReplicas: statusReplicas,
description: "Should use status.replicas (5) to preserve current state",
},
{
name: "nil replicas + status replicas zero -> use minReplicas",
eds: &zv1.ElasticsearchDataSet{
ObjectMeta: metav1.ObjectMeta{
Name: "test-eds-new",
Namespace: "default",
},
Spec: zv1.ElasticsearchDataSetSpec{
Replicas: nil, // Not yet initialized
Scaling: &zv1.ElasticsearchDataSetScaling{
Enabled: true,
MinReplicas: minReplicas,
MaxReplicas: maxReplicas,
},
},
Status: zv1.ElasticsearchDataSetStatus{
Replicas: 0, // New EDS not yet running
},
},
expectedReplicas: minReplicas,
description: "Should use minReplicas (3) for new/uninitialized EDS",
},
{
name: "nil replicas + no minReplicas -> use status",
eds: &zv1.ElasticsearchDataSet{
ObjectMeta: metav1.ObjectMeta{
Name: "test-eds-no-min",
Namespace: "default",
},
Spec: zv1.ElasticsearchDataSetSpec{
Replicas: nil,
Scaling: &zv1.ElasticsearchDataSetScaling{
Enabled: true,
MinReplicas: 0, // No minimum set
MaxReplicas: maxReplicas,
},
},
Status: zv1.ElasticsearchDataSetStatus{
Replicas: statusReplicas,
},
},
expectedReplicas: 0, // No defensive logic applied when minReplicas is 0
description: "Should return 0 when minReplicas is not set",
},
} {
t.Run(tc.name, func(t *testing.T) {
// Simulate what happens in scaleEDS
currentReplicas := edsReplicas(tc.eds)
scaling := tc.eds.Spec.Scaling

// Apply the defensive logic from scaleEDS
if currentReplicas == 0 && scaling != nil && scaling.MinReplicas > 0 {
if tc.eds.Status.Replicas > 0 {
currentReplicas = tc.eds.Status.Replicas
} else {
currentReplicas = scaling.MinReplicas
}
}

assert.Equal(t, tc.expectedReplicas, currentReplicas, tc.description)
})
}
}
Loading