-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: detect spread scaler requirements violation #491
fix: detect spread scaler requirements violation #491
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic looks great, and tests are 👨🏻🍳 💋. Just a comment around the wording of the status message and thoughts around reconciliation
Signed-off-by: Ahmed <[email protected]>
fix spelling error Co-authored-by: Brooks Townsend <[email protected]> Signed-off-by: Ahmed <[email protected]>
…conflicts Signed-off-by: Ahmed <[email protected]>
9804e81
to
31c2a19
Compare
…eq. conflict stuff Signed-off-by: Ahmed <[email protected]>
Signed-off-by: Ahmed <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff @ahmedtadde, feel free to merge once ready
Feature or Problem
A
ComponentSpreadScaler
config may have a set of requirements which cannot fully be satisfied so the deployment status remains as "Reconciling".Related Issues
#432
Release Information
next
Consumer Impact
This is an enhancement/correctness change. It should have no adverse effect on existing functionality. This change prevents potential
Heisenbugs
wherewadm
could potentially operate on faulty instance spread without any detection.Testing
All existing tests (unit and e2e) are functional
Unit Test(s)
Added two test cases
can_detect_spread_requirements_violation_1
andcan_detect_spread_requirements_violation_2