Skip to content

autoscaler: enforce bounds when scaling hint is NONE#516

Merged
otrosien merged 2 commits intozalando-incubator:masterfrom
adrobisch:fix-enforce-boundaries-in-noop
Jan 7, 2026
Merged

autoscaler: enforce bounds when scaling hint is NONE#516
otrosien merged 2 commits intozalando-incubator:masterfrom
adrobisch:fix-enforce-boundaries-in-noop

Conversation

@adrobisch
Copy link
Contributor

One-line summary

Follow up to c53f570 which moved the responsibility to enforce bounds to the autoscaler.

Description

This fixes the scenario when ScalingHint is NONE and no other scaling decision is triggered either. In that case we still want to enforce the replica bounds.

Types of Changes

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

Signed-off-by: Andreas Drobisch <dro@unkonstant.de>
Copy link

@hooseins hooseins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change look good.

For context, we observed that the EDS is not immediately scaling out after patching it with min replica > current replicas. But with any next scaling hint of either "UP" or "DOWN", it correctly scales to the new min replicas. This indicates that somewhere we were not executing the scaling operation when the "replica" field is missing (which is after the patch) and the scaling hint is "NONE".

Code owners to review further.

Signed-off-by: Andreas Drobisch <dro@unkonstant.de>
@otrosien otrosien added the bugfix Bug fixes and patches, e.g. fixing of a production issue that is affecting the customer experience. label Dec 29, 2025
@otrosien
Copy link
Member

otrosien commented Jan 5, 2026

@adrobisch this should finally fix the scaling-to-zero issue, and generally LGTM. But, the more I think about it, the more I believe that there's a general design flaw in making edsReplicas return int32 and not a pointer. As such, it hides all these edge cases that we uncovered (and maybe re-introduce similar issues in the future because of that).

Can you prepare a changeset with going for *int32 for comparison?

@adrobisch
Copy link
Contributor Author

@adrobisch this should finally fix the scaling-to-zero issue, and generally LGTM. But, the more I think about it, the more I believe that there's a general design flaw in making edsReplicas return int32 and not a pointer. As such, it hides all these edge cases that we uncovered (and maybe re-introduce similar issues in the future because of that).

Can you prepare a changeset with going for *int32 for comparison?

@otrosien I drafted https://github.com/zalando-incubator/es-operator/compare/master...adrobisch:es-operator:eds-replicas-pointer?expand=1 where I made it a pointer, but did not push it down to scaleUpOrDown since we are applying the bounds at that point.
Still a more conservative change, not sure if this is what you had in mind?

@otrosien
Copy link
Member

otrosien commented Jan 7, 2026

We decided to merge this first, and follow-up on an improvement.

@otrosien
Copy link
Member

otrosien commented Jan 7, 2026

👍

1 similar comment
@adrobisch
Copy link
Contributor Author

👍

@otrosien otrosien merged commit 1d040a7 into zalando-incubator:master Jan 7, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Bug fixes and patches, e.g. fixing of a production issue that is affecting the customer experience.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants