refactoring a few smoke tests #11748
+137
−34
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -38,13 +38,15 @@ | |||||
| StoragePool) | ||||||
| from marvin.lib.common import (get_domain, | ||||||
| get_zone, | ||||||
| get_template, | ||||||
| list_disk_offering) | ||||||
| from marvin.lib.decoratorGenerators import skipTestIf | ||||||
| from marvin.codes import FAILED, PASS | ||||||
| from nose.plugins.attrib import attr | ||||||
| import logging | ||||||
| from sp_util import (TestData, StorPoolHelper) | ||||||
| import math | ||||||
| import uuid | ||||||
|
|
||||||
| class TestSnapshotCopy(cloudstackTestCase): | ||||||
|
|
||||||
|
|
@@ -108,9 +110,11 @@ def setUpClass(cls): | |||||
| domainid=cls.domain.id) | ||||||
| cls._cleanup.append(cls.account) | ||||||
|
|
||||||
| compute_offering_service = cls.services["service_offerings"]["tiny"].copy() | ||||||
| td = TestData() | ||||||
| cls.testdata = td.testdata | ||||||
| cls.helper = StorPoolHelper() | ||||||
|
slavkap marked this conversation as resolved.
|
||||||
| cls.disk_offerings = cls.create_do_if_not_exists(cls.testdata[TestData.diskOfferingCustomAdditionalZone]) | ||||||
| cls.service_offering = ServiceOffering.create( | ||||||
| cls.apiclient, | ||||||
| compute_offering_service) | ||||||
|
|
@@ -158,10 +162,9 @@ def test_01_take_snapshot_multi_zone(self): | |||||
| """ | ||||||
|
|
||||||
| snapshot = Snapshot.create(self.userapiclient, volume_id=self.volume.id, zoneids=[str(self.additional_zone.id)], usestoragereplication=True) | ||||||
| self._cleanup.append(snapshot) | ||||||
|
Contributor
There was a problem hiding this comment.
Suggested change
Contributor
Author
There was a problem hiding this comment. @DaanHoogland, the cleanup is left on purpose for the class because there is a required time.sleep() and I don't want to delay the tests execution with invoking it in each test case
Contributor
There was a problem hiding this comment. but the creation does need to happen in the test?
Contributor
There was a problem hiding this comment. btw, if it is really needed please leave a don’t-do-this-at-home warning with it? |
||||||
| self.snapshot_id = snapshot.id | ||||||
| self.helper.verify_snapshot_copies(self.userapiclient, self.snapshot_id, [self.zone.id, self.additional_zone.id]) | ||||||
| return | ||||||
|
|
||||||
| @skipTestIf("testsNotSupported") | ||||||
|
|
@@ -171,11 +174,11 @@ def test_02_copy_snapshot_multi_pools(self): | |||||
| """ | ||||||
|
|
||||||
| snapshot = Snapshot.create(self.userapiclient, volume_id=self.volume.id) | ||||||
| self._cleanup.append(snapshot) | ||||||
|
Contributor
There was a problem hiding this comment.
Suggested change
|
||||||
| self.snapshot_id = snapshot.id | ||||||
| Snapshot.copy(self.userapiclient, self.snapshot_id, zone_ids=[str(self.additional_zone.id)], source_zone_id=self.zone.id, usestoragereplication=True) | ||||||
| self.helper.verify_snapshot_copies(self.userapiclient, self.snapshot_id, [self.zone.id, self.additional_zone.id]) | ||||||
|
|
||||||
| return | ||||||
|
|
||||||
| @skipTestIf("testsNotSupported") | ||||||
|
|
@@ -190,7 +193,7 @@ def test_03_take_snapshot_multi_pools_delete_single_zone(self): | |||||
| time.sleep(420) | ||||||
| Snapshot.delete(snapshot, self.userapiclient, self.zone.id) | ||||||
| self.helper.verify_snapshot_copies(self.userapiclient, self.snapshot_id, [self.additional_zone.id]) | ||||||
| self._cleanup.append(snapshot) | ||||||
|
Contributor
There was a problem hiding this comment. isn’t the snapshot to be removed from the cleanup list here?
Contributor
There was a problem hiding this comment. @slavkap per test items should go in the
Contributor
There was a problem hiding this comment.
Suggested change
Contributor
Author
There was a problem hiding this comment. @DaanHoogland, the cleanup is left on purpose for the class because there is a required time.sleep() and I don't want to delay the tests execution with invoking it in each test case
Contributor
There was a problem hiding this comment. sorry for the late reply @slavkap , I got it, but can you add an explicit warning/comment on why you are adding those in the "wrong" place?
slavkap marked this conversation as resolved.
|
||||||
| return | ||||||
|
|
||||||
| @skipTestIf("testsNotSupported") | ||||||
|
|
@@ -217,6 +220,7 @@ def test_05_take_snapshot_multi_zone_create_volume_additional_zone(self): | |||||
| """ | ||||||
|
|
||||||
| snapshot = Snapshot.create(self.userapiclient,volume_id=self.volume.id, zoneids=[str(self.additional_zone.id)], usestoragereplication=True) | ||||||
| self._cleanup.append(snapshot) | ||||||
|
Contributor
There was a problem hiding this comment.
Suggested change
|
||||||
| self.snapshot_id = snapshot.id | ||||||
| self.helper.verify_snapshot_copies(self.userapiclient, self.snapshot_id, [self.zone.id, self.additional_zone.id]) | ||||||
| disk_offering_id = None | ||||||
|
|
@@ -227,13 +231,11 @@ def test_05_take_snapshot_multi_zone_create_volume_additional_zone(self): | |||||
| self.apiclient, | ||||||
| service | ||||||
| ) | ||||||
| self._cleanup.append(self.disk_offering) | ||||||
|
Contributor
There was a problem hiding this comment.
Suggested change
|
||||||
| disk_offering_id = self.disk_offering.id | ||||||
|
|
||||||
| self.volume = Volume.create(self.userapiclient, {"diskname":"StorPoolDisk-1" }, snapshotid=self.snapshot_id, zoneid=self.zone.id, diskofferingid=disk_offering_id) | ||||||
| self.cleanup.append(self.volume) | ||||||
| if self.zone.id != self.volume.zoneid: | ||||||
| self.fail("Volume from snapshot not created in the additional zone") | ||||||
| return | ||||||
|
|
@@ -244,12 +246,101 @@ def test_06_take_snapshot_multi_zone_create_template_additional_zone(self): | |||||
| """Test to take volume snapshot in multiple StorPool primary storages in diff zones and create a volume in one of the additional zones | ||||||
| """ | ||||||
| snapshot = Snapshot.create(self.userapiclient, volume_id=self.volume.id, zoneids=[str(self.additional_zone.id)], usestoragereplication=True) | ||||||
| self._cleanup.append(snapshot) | ||||||
|
Contributor
There was a problem hiding this comment.
Suggested change
|
||||||
| self.snapshot_id = snapshot.id | ||||||
| self.helper.verify_snapshot_copies(self.userapiclient, self.snapshot_id, [self.zone.id, self.additional_zone.id]) | ||||||
| self.template = self.helper.create_snapshot_template(self.userapiclient, self.services, self.snapshot_id, self.additional_zone.id) | ||||||
| self.cleanup.append(self.template) | ||||||
| if self.additional_zone.id != self.template.zoneid: | ||||||
| self.fail("Template from snapshot not created in the additional zone") | ||||||
| return | ||||||
|
|
||||||
| @skipTestIf("testsNotSupported") | ||||||
| @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="false") | ||||||
| def test_07_take_snapshot_multi_zone_deply_vm_additional_zone(self): | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| """Test to take volume snapshot in multiple StorPool primary storages in diff zones and deploy a VM from snapshot in one of the additional zones | ||||||
| """ | ||||||
| snapshot = Snapshot.create(self.userapiclient, volume_id=self.volume.id, zoneids=[str(self.additional_zone.id)], usestoragereplication=True) | ||||||
| self._cleanup.append(snapshot) | ||||||
| self.snapshot_id = snapshot.id | ||||||
| self.helper.verify_snapshot_copies(self.userapiclient, self.snapshot_id, [self.zone.id, self.additional_zone.id]) | ||||||
| vm = self.deploy_vm_from_snapshot(snapshot, self.additional_zone.id) | ||||||
| if self.additional_zone.id != vm.zoneid: | ||||||
| self.fail("VM from snapshot not created in the additional zone") | ||||||
| return | ||||||
|
|
||||||
|
|
||||||
| @skipTestIf("testsNotSupported") | ||||||
| @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="false") | ||||||
| def test_08_take_snapshot_multi_zone_create_volume_additional_zone_deploy_vm(self): | ||||||
| """Test to take volume snapshot in multiple StorPool primary storages in diff zones | ||||||
| create a volume from the snapshot in the additional zone | ||||||
| and deploy a VM from the volume in one of the additional zones | ||||||
| """ | ||||||
| snapshot = Snapshot.create(self.userapiclient, volume_id=self.volume.id, zoneids=[str(self.additional_zone.id)], usestoragereplication=True) | ||||||
| self.cleanup.append(snapshot) | ||||||
| self.snapshot_id = snapshot.id | ||||||
| self.helper.verify_snapshot_copies(self.userapiclient, self.snapshot_id, [self.zone.id, self.additional_zone.id]) | ||||||
| vm = self.create_volume_from_snapshot_deploy_vm(snapshotid=self.snapshot_id, zoneid=self.additional_zone.id) | ||||||
| time.sleep(420) | ||||||
| if self.additional_zone.id != vm.zoneid: | ||||||
| self.fail("VM from snapshot not created in the additional zone") | ||||||
| return | ||||||
|
|
||||||
| def create_volume_from_snapshot_deploy_vm(self, snapshotid, zoneid=None): | ||||||
| volume = Volume.create_from_snapshot( | ||||||
| self.apiclient, | ||||||
| snapshot_id=snapshotid, | ||||||
| services=self.services, | ||||||
| disk_offering=self.disk_offerings.id, | ||||||
| size=8, | ||||||
| account=self.account.name, | ||||||
| domainid=self.account.domainid, | ||||||
| zoneid=zoneid, | ||||||
| ) | ||||||
| virtual_machine = VirtualMachine.create(self.apiclient, | ||||||
| {"name": "Test-%s" % uuid.uuid4()}, | ||||||
| accountid=self.account.name, | ||||||
| domainid=self.account.domainid, | ||||||
| zoneid=zoneid, | ||||||
| serviceofferingid=self.service_offering.id, | ||||||
| volumeid=volume.id, | ||||||
| mode="basic", | ||||||
| ) | ||||||
|
slavkap marked this conversation as resolved.
|
||||||
| self._cleanup.append(virtual_machine) | ||||||
|
DaanHoogland marked this conversation as resolved.
Outdated
|
||||||
| try: | ||||||
| ssh_client = virtual_machine.get_ssh_client() | ||||||
|
There was a problem hiding this comment. This helper appends the created VM to
slavkap marked this conversation as resolved.
Outdated
|
||||||
| except Exception as e: | ||||||
| self.fail("SSH failed for virtual machine: %s - %s" % | ||||||
| (virtual_machine.ipaddress, e)) | ||||||
| return virtual_machine | ||||||
|
|
||||||
| def deploy_vm_from_snapshot(self, snapshot, zoneid=None): | ||||||
| virtual_machine = VirtualMachine.create(self.apiclient, | ||||||
| {"name": "Test-%s" % uuid.uuid4()}, | ||||||
| accountid=self.account.name, | ||||||
| domainid=self.account.domainid, | ||||||
| zoneid=zoneid, | ||||||
| serviceofferingid=self.service_offering.id, | ||||||
| snapshotid=snapshot.id, | ||||||
| mode="basic", | ||||||
| ) | ||||||
| self._cleanup.append(virtual_machine) | ||||||
|
DaanHoogland marked this conversation as resolved.
Outdated
|
||||||
| try: | ||||||
| ssh_client = virtual_machine.get_ssh_client() | ||||||
|
There was a problem hiding this comment. This helper adds the created VM to There was a problem hiding this comment. Variable ssh_client is not used. |
||||||
| except Exception as e: | ||||||
| self.fail("SSH failed for virtual machine: %s - %s" % | ||||||
| (virtual_machine.ipaddress, e)) | ||||||
| return virtual_machine | ||||||
|
|
||||||
| @classmethod | ||||||
| def create_do_if_not_exists(cls, data): | ||||||
| disk_offerings = list_disk_offering( | ||||||
| cls.apiclient, | ||||||
| name=data["name"] | ||||||
| ) | ||||||
| if disk_offerings is None: | ||||||
| disk_offerings = DiskOffering.create(cls.apiclient, services=data, custom=True) | ||||||
| else: | ||||||
| disk_offerings = disk_offerings[0] | ||||||
| return disk_offerings | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -82,33 +82,30 @@ def setUpClass(cls): | |
|
|
||
| storage_pools_response = list_storage_pools(cls.apiclient, | ||
| zoneid=cls.zone.id, | ||
| scope="ZONE", | ||
| status="Up") | ||
|
|
||
| if storage_pools_response: | ||
| cls.zone_wide_storage = storage_pools_response[0] | ||
| storage_tag = None | ||
| if cls.zone_wide_storage.tags: | ||
| storage_tag = cls.zone_wide_storage.tags | ||
|
|
||
| cls.debug( | ||
| "zone wide storage id is %s" % | ||
| cls.zone_wide_storage.id) | ||
| cls.service_offering = ServiceOffering.create( | ||
| cls.apiclient, | ||
| cls.services["service_offerings"]["small"], | ||
| tags=storage_tag | ||
| ) | ||
| cls._cleanup.append(cls.service_offering) | ||
|
|
||
| do = { | ||
| "name": "do-tags", | ||
| "displaytext": "Disk offering with tags", | ||
| "disksize":8, | ||
| "tags": storage_tag | ||
| } | ||
| cls.disk_offering = DiskOffering.create( | ||
| cls.apiclient, | ||
|
|
@@ -277,7 +274,7 @@ def test_05_deploy_vm_with_existing_snapshot_deleted_volume(self): | |
| self.deploy_vm_from_snapshot(snapshot) | ||
|
|
||
| def deploy_vm_from_snapshot(self, snapshot): | ||
| self.virtual_machine = VirtualMachine.create(self.apiclient, | ||
| {"name": "Test-%s" % uuid.uuid4()}, | ||
| accountid=self.account.name, | ||
| domainid=self.account.domainid, | ||
|
|
@@ -286,11 +283,12 @@ def deploy_vm_from_snapshot(self, snapshot): | |
| snapshotid=snapshot.id, | ||
| mode="basic", | ||
| ) | ||
| self.cleanup.append(self.virtual_machine) | ||
| try: | ||
| ssh_client = self.virtual_machine.get_ssh_client() | ||
|
There was a problem hiding this comment. Variable ssh_client is not used. |
||
| except Exception as e: | ||
| self.fail("SSH failed for virtual machine: %s - %s" % | ||
| (self.virtual_machine.ipaddress, e)) | ||
|
|
||
| def create_volume_from_snapshot_deploy_vm(self, snapshotid): | ||
| volume = Volume.create_from_snapshot( | ||
|
|
@@ -302,7 +300,7 @@ def create_volume_from_snapshot_deploy_vm(self, snapshotid): | |
| domainid=self.account.domainid, | ||
| zoneid=self.zone.id, | ||
| ) | ||
| self.virtual_machine = VirtualMachine.create(self.apiclient, | ||
| {"name": "Test-%s" % uuid.uuid4()}, | ||
| accountid=self.account.name, | ||
| domainid=self.account.domainid, | ||
|
|
@@ -311,8 +309,9 @@ def create_volume_from_snapshot_deploy_vm(self, snapshotid): | |
| volumeid=volume.id, | ||
| mode="basic", | ||
| ) | ||
| self.cleanup.append(self.virtual_machine) | ||
| try: | ||
| ssh_client = self.virtual_machine.get_ssh_client() | ||
|
There was a problem hiding this comment. Variable ssh_client is not used. |
||
| except Exception as e: | ||
| self.fail("SSH failed for virtual machine: %s - %s" % | ||
| (self.virtual_machine.ipaddress, e)) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
StorPoolHelper.verify_snapshot_copiesnow raises exceptions for some failure cases, but it still callscls.fail(...)earlier in the method whenSnapshot.listdoesn't return a list. SinceStorPoolHelperis not aunittest.TestCase,cls.failis not defined and that path will raiseAttributeErrorinstead of a clear test failure. Replace that remainingcls.failusage with an exception/AssertionError for consistency and correct error reporting.