CLOUDSTACK-10154: fixing some smoketests failures#2335
Conversation
|
|
||
| #Giving 30 seconds to management to warm-up, | ||
| #Experienced failures when trying to deploy a VM exactly when management came up | ||
| time.sleep(30) |
There was a problem hiding this comment.
@borisstoyanov instead of a blind sleep, should we poll every few seconds using wait_until and see if mgmt server's port 8080 is reachable -- see for example, in travis we wait using this: https://github.com/apache/cloudstack/blob/master/tools/travis/before_script.sh#L25
There was a problem hiding this comment.
This is warmup, we've already used the polling approach to determine management is up, at this stage management is responding, but it need some warmup period.
|
|
||
| def waitForSystemVMAgent(self, vmname): | ||
| timeout = self.services["timeout"] | ||
| timeout = 120 |
There was a problem hiding this comment.
I've heavily refactored and fixed this test in my branch/PR #2211. In general, throughout the test codebase we should NOT be sleeping blindly but use wait_until and poll for behaviour (i.e list stuff) over some short period (1-10 seconds) and retries.
There was a problem hiding this comment.
shall we leave this as is until debian9 changes get merged?
| host = Host.list( | ||
| self.apiclient, | ||
| type='Routing', | ||
| virtualmachineid=list_vm.id |
There was a problem hiding this comment.
LGTM. While this should have worked, listHosts when passed a vm id will also return other hosts in the cluster that can host this VM (enough VM cpu/ram)
| def wait_for_attributes_and_return_root_vol(self): | ||
|
|
||
| for i in range(60): | ||
| for i in range(360): |
There was a problem hiding this comment.
@borisstoyanov can you rewrite this and use the following example/patterns in other places wherever we're sleeping blindly:
# If not import do this:
from marvin.lib.utils import wait_until
[...snipped...]
def wait_for_attributes_and_return_root_vol(self):
def checkVolumeResponse():
list_volume_response = Volume.list(
self.apiClient,
virtualmachineid=self.virtual_machine.id,
type='ROOT',
listall=True
)
if list_volume_response[0].virtualsize is not None:
return True, list_volume_response[0]
return False, None
# sleep interval is 1s, retries is 360, this will sleep atmost 360 seconds, or 6 mins
res, response = wait_until(1, 360, checkVolumeResponse)
if not res:
self.fail("Failed to return root volume response")
return response
yadvr
left a comment
There was a problem hiding this comment.
Overall LGTM, thanks @borisstoyanov. Left some comments to further improve the test code.
|
@rhtyd I've addressed your comment, also not a huge fan of implicit waits.. |
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1307 |
|
@blueorangutan test |
|
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-1703)
|
|
Lgtm, the isos failures are env related. Merging now. |
ssvm tests:
==== Marvin Init Successful ====
=== TestName: test_01_list_sec_storage_vm | Status : SUCCESS ===
=== TestName: test_02_list_cpvm_vm | Status : SUCCESS ===
=== TestName: test_03_ssvm_internals | Status : SUCCESS ===
=== TestName: test_04_cpvm_internals | Status : SUCCESS ===
=== TestName: test_05_stop_ssvm | Status : SUCCESS ===
=== TestName: test_06_stop_cpvm | Status : SUCCESS ===
=== TestName: test_07_reboot_ssvm | Status : SUCCESS ===
=== TestName: test_08_reboot_cpvm | Status : SUCCESS ===
=== TestName: test_09_destroy_ssvm | Status : SUCCESS ===
=== TestName: test_10_destroy_cpvm | Status : SUCCESS ===
deploy_vm_root_resize:
=== TestName: test_00_deploy_vm_root_resize | Status : SUCCESS ===
=== TestName: test_01_deploy_vm_root_resize | Status : SUCCESS ===
=== TestName: test_02_deploy_vm_root_resize | Status : SUCCESS ===
test volumes:
=== TestName: test_10_list_volumes | Status : SUCCESS ===
test host annotations
=== TestName: test_01_add_annotation | Status : SUCCESS ===
=== TestName: test_02_add_multiple_annotations | Status : SUCCESS ===
=== TestName: test_03_user_role_dont_see_annotations | Status : SUCCESS ===
=== TestName: test_04_remove_annotations | Status : SUCCESS ===
=== TestName: test_05_add_annotation_for_invalid_entityType | Status : SUCCESS ===
ping @rhtyd @PaulAngus @nvazquez @DaanHoogland for review