Repository navigation
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.22 #14460 +/- ##
=========================================
Coverage 18.32% 18.32%
- Complexity 16644 16651 +7
=========================================
Files 5939 5939
Lines 537768 537797 +29
Branches 65929 65932 +3
=========================================
+ Hits 98551 98571 +20
- Misses 428008 428016 +8
- Partials 11209 11210 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| ActionEventUtils.onActionEvent(User.UID_SYSTEM, Account.ACCOUNT_ID_SYSTEM, vm.getDomainId(), | ||
| EventTypes.EVENT_VM_START, "Out of band VM power on", vm.getId(), getApiCommandResourceTypeForVm(vm).toString()); | ||
| logger.info("VM {} is sync-ed to at Running state according to power-on report from hypervisor.", vm.getInstanceName()); | ||
| reconcileNicsForRunningVm(vm); |
There was a problem hiding this comment.
why do we need it in the stopped||stopping block? if the VM never reports as running again we have a spare nic in our nic count. should it be added to the start block instead.
There was a problem hiding this comment.
when a running vm is reported as stopped, cloudstack will send a command to force-stop it (I guess it is to ensure the vm is stopped) , and release the nic when done.
as far as I know, nics_count is not used any more to determine if the network need to be shutdown. the goal of this PR is to restore the nic state, not nics_count. PR title should be updated.
There was a problem hiding this comment.
@Pearl1594 do you agree? can you update the PR title appropriately?
DaanHoogland
changed the title
99d9c27 to
9b81f09
Compare
|
ready for review, @Pearl1594 ? |
|
Yes @DaanHoogland - ready for review/test. Thanks. |
on second though I have my doubts. so I asked claude and let it generate some test code, I'll push here if you agree, @Pearl1594 . |
|
go ahead @DaanHoogland |
boring-cyborg
Bot
added
component:integration-test
Python
|
@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. |
|
| self.assertEqual(self.get_nics_count(network_id), nics_count - 1, "nics_count should drop by one when the nic is released") | ||
|
|
||
| # 4 - bring the domain back out of band, a persistent domain is still defined and only needs a start | ||
| ssh_host.execute("virsh create %s || virsh start %s" % (domain_xml, vm_response.instancename)) |
There was a problem hiding this comment.
this may fail, afaik bridge might be removed when the last vm on the bridge is stopped.
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19920 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Description
This PR fixes: #14177
Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?