Copilot commented on code in PR #14362:
URL: https://github.com/apache/cloudstack/pull/14362#discussion_r4399194365
##########
test/integration/smoke/test_vm_lifecycle_with_snapshot_or_volume.py:
##########
@@ -40,11 +40,19 @@
from marvin.codes import FAILED, PASS
from nose.plugins.attrib import attr
-import uuid
import unittest
class TestDeployVMFromSnapshotOrVolume(cloudstackTestCase):
+ def rbd_snapshot_cannot_outlive_source_volume(self):
+ '''
+ On RBD, a snapshot lives on primary storage with its parent
+ volume, so with snapshot.backup.to.secondary=false, deleting the
+ parent volume destroys the snapshot too - it can no longer be
+ used to deploy a new volume/VM afterward.
+ '''
+ return self.storage_pool.type == 'RBD'
Review Comment:
The docstring says the RBD skip applies only when
snapshot.backup.to.secondary=false. The method, however, returns True for every
RBD pool and never reads that setting. With the setting enabled, snapshots are
copied to secondary storage and survive deletion of the source volume, so these
tests are skipped when they could run. Either read the global setting and
return False when it is true, or change the docstring to state that the skip is
unconditional for RBD.
##########
test/integration/smoke/test_vm_lifecycle_with_snapshot_or_volume.py:
##########
@@ -202,29 +220,35 @@ def
test_03_deploy_vm_with_existing_volume_deleted_template(self):
root_volume.id,
)
VirtualMachine.delete(virtual_machine, self.apiclient, expunge=True)
- self.create_volume_from_snapshot_deploy_vm(snapshot.id)
+ if not self.rbd_snapshot_cannot_outlive_source_volume():
+ self.create_volume_from_snapshot_deploy_vm(snapshot.id)
Review Comment:
On RBD, test_03 silently skips the deploy-from-snapshot step and still
reports PASS, while test_04 and test_05 raise unittest.SkipTest. This hides the
skipped step from test results. Log the skipped step explicitly, as in the
suggested fix, or move it to a dedicated skipped test.
##########
test/integration/smoke/test_vm_lifecycle_with_snapshot_or_volume.py:
##########
@@ -202,29 +220,35 @@ def
test_03_deploy_vm_with_existing_volume_deleted_template(self):
root_volume.id,
)
VirtualMachine.delete(virtual_machine, self.apiclient, expunge=True)
- self.create_volume_from_snapshot_deploy_vm(snapshot.id)
+ if not self.rbd_snapshot_cannot_outlive_source_volume():
+ self.create_volume_from_snapshot_deploy_vm(snapshot.id)
@attr(tags=["advanced"], required_hardware="false")
def test_04_deploy_vm_with_existing_snapshot_deleted_template(self):
'''
Deploy a Virtual machine with existing snapshot of a ROOT volume
created from a templated which was deleted
'''
+ if self.rbd_snapshot_cannot_outlive_source_volume():
+ raise unittest.SkipTest(
+ "Covered by test_03 on RBD: deploying from a snapshot after "
+ "its source ROOT volume was deleted is not supported here."
Review Comment:
The skip message says "Covered by test_03 on RBD", but test_03 also skips
the deploy-from-snapshot step on RBD (lines 223-224), so this case is not
covered there. Replace the message with the real reason. The same wrong message
appears in test_05 (lines 276-277) and should be updated too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
