Fix smoke tests due to change in behavior of restore VM #10583
Conversation
Pearl1594
requested review from
rohityadavcloud,
shwstppr,
DaanHoogland,
vishesh92 and
weizhouapache
boring-cyborg
bot
added
component:integration-test
Python
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #10583 +/- ##
=========================================
Coverage 15.16% 15.16%
- Complexity 11327 11328 +1
=========================================
Files 5414 5414
Lines 474814 474814
Branches 57912 57912
=========================================
+ Hits 72004 72021 +17
+ Misses 394758 394738 -20
- Partials 8052 8055 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I wonder if this change in behaviour requires documentation. |
|
|
@blueorangutan package |
|
@Pearl1594 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 12822 |
|
@blueorangutan test |
|
@Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-12760)
|
|
@blueorangutan test matrix |
|
@Pearl1594 a [SL] Trillian-Jenkins matrix job (EL8 mgmt + EL8 KVM, Ubuntu22 mgmt + Ubuntu22 KVM, EL8 mgmt + VMware 7.0u3, EL9 mgmt + XCP-ng 8.2 ) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-12776)
|
|
[SF] Trillian test result (tid-12775)
|
|
[SF] Trillian test result (tid-12774)
|
|
[SF] Trillian test result (tid-12777)
|
weizhouapache
changed the title
| account_network.update(self.apiclient, name=account_network.name + ts) | ||
| account_network.delete(self.apiclient) | ||
| self.cleanup.remove(account_network) | ||
| virtual_machine.start(self.apiclient) |
There was a problem hiding this comment.
@Pearl1594 I'm not completely aware of the new behaviour but idea of the test was to do a bunch of operations related to VM, network, volume etc and then check if the events for them have resourceid an resourcetype associated for them. Can we update the test in a way that similar check can be done? With the current change it is checking only for VM action event.
There was a problem hiding this comment.
I see your point and the point of such tests, but we are not checking for specific events anyway, just listing them. If the test is error prone, not specific and sensitive to environmental issues, I'd rather we simplify it and add more specific checks. Would you agree?
| account_network.update(self.apiclient, name=account_network.name + ts) | ||
| account_network.delete(self.apiclient) | ||
| self.cleanup.remove(account_network) | ||
| virtual_machine.start(self.apiclient) |
There was a problem hiding this comment.
I see your point and the point of such tests, but we are not checking for specific events anyway, just listing them. If the test is error prone, not specific and sensitive to environmental issues, I'd rather we simplify it and add more specific checks. Would you agree?
| #22. Start VM before destroying, to recreate ROOT volume that was deleted as part of restore operation | ||
| command = """self.virtual_machine.start({apiclient})""" | ||
| self.exec_command("self.user_apiclient", command, expected=False) | ||
| self.exec_command("self.otheruser_apiclient", command, expected=True) | ||
|
|
There was a problem hiding this comment.
a second step number 22 ! would it make sense to split this test? It already has 26 steps , adn I see some doing double work as well.
Is this creating extra value to our testing?
Description
This PR fixes a regression introduced due to change in behavior with restore VM operation. Since restoreVM results in root volume deletion, if the VM is cleaned up without starting no Root disk is found.
Caused by: #8800 (#8800 (comment))
Test failures also noticed on 4.19.2 health check: #9315 (comment)
Fixes failure in:
test_events_resource.py
test_network_permissions.py
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?