-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Maintenance mode: Add host to deployment planner avoid list to fix local storage migration #9892
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
Conversation
…cal storage vm migration
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #9892 +/- ##
============================================
- Coverage 15.81% 15.80% -0.01%
Complexity 12580 12580
============================================
Files 5627 5627
Lines 492260 492262 +2
Branches 63955 60734 -3221
============================================
- Hits 77832 77825 -7
- Misses 405905 405914 +9
Partials 8523 8523
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@blueorangutan package |
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 11525 |
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.
clgtm
this seems to have to do with github api limits. trying a build in the background. |
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.
Code looks good
@blueorangutan test keepEnv |
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
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.
code lgtm
make sense
[SF] Trillian test result (tid-11796)
|
@BartJM , I finally got arounf testing this. There is no error but |
ping @BartJM , is this alright/as you expect as well? |
@DaanHoogland There are some scenarios for vms that are expected to still fail:
The ErrorInMaintenance is expected when a vm failed to migrate. |
Awesome work, congrats on your first merged pull request! |
* 4.20: Maintenance mode: Add host to deployment planner avoid list to fix local storage vm migration (#9892) Add project-user association normalization script to 4.20.1 upgrade (#10116) fix slider component for global settings of the range type (#10187) Clean up network permissions on account deletion (#10176)
Description
This PR adds the host preparing for maintenance to the avoid list for the deployment planner.
Without adding the host to the avoid list the deployment planner will return the host preparing for maintenance when a vm has a local storage root disk and has the host preparing for maintenance as the last host.
Steps to reproduce
Actual behaviour
Expected
Local storage vm live migrated to another host.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Centos8 mbx env with
host.maintenance.local.storage.strategy
set to Migration.Tested maintenance mode on a host with
5 and 6 result in an ErrorInMaintenance due to dest being null. Similar as described in #9887. Without this patch these would still fail with the same error as described in actual behavior.
After creation of a ha vm with ha data disk maintenance was not possible. After manual migration to another host and back, migration did complete. Migration without selecting host also gave a no destination found error and also occurred without this patch so is unrelated.
How did you try to break this feature and the system with this change?