Autoscaler fails to update spec.replicas when scaling boundaries change

[otrosien]

Description

Problem

When patching an ElasticsearchDataSet (EDS) to update scaling boundaries (spec.scaling.minReplicas or spec.scaling.maxReplicas), the autoscaler correctly scales the cluster but fails to persist the new replica count to spec.replicas. This causes a mismatch between the actual running state and the desired state in the spec.

Root Cause

The bug is in the edsReplicas() function at operator/elasticsearch.go:827-838. This function had two critical issues:

1. Premature boundary enforcement: The function applied math.Max(currentReplicas, minReplicas) which artificially inflated the current replica count before the autoscaler could evaluate it.

2. Update condition bypass: When edsReplicas() returned a value that already matched what the autoscaler wanted, the condition at line 940 (scalingOperation.NodeReplicas != currentReplicas) would evaluate to false, causing the spec update logic (lines 940-955) to be skipped entirely.

Example Scenario

Before patch:

• spec.replicas = 3
• spec.scaling.minReplicas = 2
• Cluster running with 3 nodes

Apply patch:

kubectl patch eds my-cluster --type=json -p '[
  {"op": "replace", "path": "/spec/scaling/minReplicas", "value": 4},
  {"op": "replace", "path": "/spec/scaling/maxReplicas", "value": 4}
]'

With the bug:

1. edsReplicas() returns max(3, 4) = 4 (premature enforcement)
2. Autoscaler calculates target replicas = 4
3. Check 4 != 4 → false, spec update skipped
4. Cluster scales to 4 nodes (via StatefulSet reconciliation)
5. Result: Cluster has 4 nodes but spec.replicas still shows 3

With the fix:

1. edsReplicas() returns 3 (actual current value)
2. Autoscaler calculates target replicas = 4
3. Check 4 != 3 → true, spec update triggered
4. spec.replicas set to 4 at line 954
5. Update persisted at line 968
6. Result: Both cluster and spec.replicas correctly show 4

Impact

This bug affects any operation that changes scaling boundaries:

• Scaling up by increasing minReplicas
• Scaling down by decreasing maxReplicas
• Any automation that relies on spec.replicas reflecting the actual desired state

Changes

1. Remove premature boundary enforcement (operator/elasticsearch.go:827-838)

Before:

func edsReplicas(eds *zv1.ElasticsearchDataSet) int32 {
    scaling := eds.Spec.Scaling
    if scaling == nil || !scaling.Enabled {
        if eds.Spec.Replicas == nil {
            return 1
        }
        return *eds.Spec.Replicas
    }
    // initialize with minReplicas
    minReplicas := eds.Spec.Scaling.MinReplicas
    if eds.Spec.Replicas == nil {
        return minReplicas
    }
    currentReplicas := *eds.Spec.Replicas
    return int32(math.Max(float64(currentReplicas), float64(scaling.MinReplicas)))
}

After:

func edsReplicas(eds *zv1.ElasticsearchDataSet) int32 {
    scaling := eds.Spec.Scaling
    if scaling == nil || !scaling.Enabled {
        if eds.Spec.Replicas == nil {
            return 1
        }
        return *eds.Spec.Replicas
    }
    // initialize with 0
    if eds.Spec.Replicas == nil {
        return 0
    }
    return *eds.Spec.Replicas
}

Key changes:

• Removed math.Max() enforcement: The function now returns the actual current value from spec.replicas without applying boundary constraints. This allows the autoscaler to detect when scaling is needed.

• Return 0 for nil case: When spec.replicas is nil and autoscaling is enabled, return 0 instead of minReplicas. This ensures the autoscaler always makes an explicit scaling decision rather than assuming the current state is already at minReplicas.

• Removed unused math import: Since math.Max() is no longer used, the import is removed.

Why this works:

The autoscaler (GetScalingOperation()) is responsible for enforcing scaling boundaries and making scaling decisions. The edsReplicas() function should only report the current state, not apply policy. By removing the premature boundary enforcement, we allow the autoscaler to:

1. See the actual current state (e.g., 3 replicas)
2. Compare it against the new boundaries (e.g., minReplicas = 4)
3. Detect that scaling is needed (3 < 4)
4. Trigger the update logic that persists the new spec.replicas value

The boundary enforcement still happens - but at the right place: in the autoscaler's decision logic, not in the state-reading function.

Types of Changes

• Bug fix (non-breaking change which fixes an issue)

[mikkeloscar]

This could be covered by a test case e.g. one as described in the example

[adrobisch]

see #512 for a unit test

[otrosien]

@adrobisch I cherry-picked the commit from your PR and signed off.

[mikkeloscar]

ideally we would have a test covering scaleEDS to test the bug explained in the description, but this is ofc. more complicated to test.

[mikkeloscar]

👍

[otrosien]

👍