-
Notifications
You must be signed in to change notification settings - Fork 119
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
Remove "dying" units from planned units #936
Remove "dying" units from planned units #936
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.
Idea looks good to me, thanks! A couple of small comments, plus I think we should add a test for this.
The current planned_units()
tests only test the Harness/testing version, so perhaps we could add a couple of tests to TestModelBackend
that use fake_script(self, 'goal-state', """echo '<json here>'""")
to test a couple of cases:
- No units.
- Two units but none dying, and ensure it returns 2.
- Two units with one dying, and ensure it returns 1.
Co-authored-by: Ben Hoyt <benhoyt@gmail.com>
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.
Thanks!
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.
Just a thought about a different wording.
Thanks again @Mehdi-Bendriss! |
This PR addresses an issue that happens during a scale-down event, where
planned_units
includes units with the status marked asdying
.This becomes visible when say the
_storage_detaching
hook errors based on certain preconditions preventing a unit from immediately disappearing.This PR essentially filters out those dying units to have an accurate count of live units.