Copilot commented on code in PR #13136:
URL: https://github.com/apache/cloudstack/pull/13136#discussion_r3208983179
##########
test/integration/smoke/test_deploy_vm_iso.py:
##########
@@ -92,66 +84,82 @@ def setUp(self):
self.cleanup = [self.account]
return
- def tearDown(self):
- try:
- self.debug("Cleaning up the resources")
- cleanup_resources(self.apiclient, self.cleanup)
- self.debug("Cleanup complete!")
- except Exception as e:
- self.debug("Warning! Exception in tearDown: %s" % e)
-
@attr(
tags=[
"advanced",
"eip",
"advancedns",
"basic",
- "sg"],
- required_hardware="true")
+ "sg"
+ ],
+ required_hardware="true"
+ )
def test_deploy_vm_from_iso(self):
"""Test Deploy Virtual Machine from ISO
"""
# Validate the following:
- # 1. deploy VM using ISO
- # 2. listVM command should return the deployed VM. State of this VM
- # should be "Running".
- self.hypervisor = self.testClient.getHypervisorInfo()
- if self.hypervisor.lower() in ['lxc']:
- self.skipTest(
- "vm deploy from ISO feature is not supported on %s" %
- self.hypervisor.lower())
+ # 1. Create an ISO
+ # 2. Deploy a VM from the ISO
+ # 3. VM should be in 'Running' state
self.iso = Iso.create(
self.apiclient,
- self.testdata["configurableData"]["bootableIso"],
+ self.testdata["iso"],
Review Comment:
This test now registers `self.testdata["iso"]`, which in the default Marvin
test data is not bootable (`bootable: False`). That changes the test’s intent
(“deploy VM from ISO”) and can cause the deploy to fail or the VM not to boot
from the ISO. Use the bootable ISO definition (e.g.
`testdata["configurableData"]["bootableIso"]`) or ensure the services dict sets
`bootable=True` for this test.
##########
test/integration/smoke/test_deploy_vm_iso.py:
##########
@@ -92,66 +84,82 @@ def setUp(self):
self.cleanup = [self.account]
return
- def tearDown(self):
- try:
- self.debug("Cleaning up the resources")
- cleanup_resources(self.apiclient, self.cleanup)
- self.debug("Cleanup complete!")
- except Exception as e:
- self.debug("Warning! Exception in tearDown: %s" % e)
-
@attr(
tags=[
"advanced",
"eip",
"advancedns",
"basic",
- "sg"],
- required_hardware="true")
+ "sg"
+ ],
+ required_hardware="true"
+ )
def test_deploy_vm_from_iso(self):
"""Test Deploy Virtual Machine from ISO
"""
# Validate the following:
- # 1. deploy VM using ISO
- # 2. listVM command should return the deployed VM. State of this VM
- # should be "Running".
- self.hypervisor = self.testClient.getHypervisorInfo()
- if self.hypervisor.lower() in ['lxc']:
- self.skipTest(
- "vm deploy from ISO feature is not supported on %s" %
- self.hypervisor.lower())
+ # 1. Create an ISO
+ # 2. Deploy a VM from the ISO
+ # 3. VM should be in 'Running' state
self.iso = Iso.create(
self.apiclient,
- self.testdata["configurableData"]["bootableIso"],
+ self.testdata["iso"],
account=self.account.name,
- domainid=self.account.domainid,
- zoneid=self.zone.id
+ domainid=self.account.domainid
+ )
+ self.cleanup.append(self.iso)
+
+ self.debug("ISO created with ID: %s" % self.iso.id)
+
+ list_iso_response = Iso.list(
+ self.apiclient,
+ id=self.iso.id
+ )
+ self.assertEqual(
+ isinstance(list_iso_response, list),
+ True,
+ "Check list response returns a valid list"
)
- try:
- # Download the ISO
- self.iso.download(self.apiclient)
- except Exception as e:
- raise Exception("Exception while downloading ISO %s: %s"
- % (self.iso.id, e))
-
- self.debug("Registered ISO: %s" % self.iso.name)
- self.debug("Deploying instance in the account: %s" %
- self.account.name)
+
+ self.iso.download(self.apiclient)
+
+ # Deploy Virtual Machine
self.virtual_machine = VirtualMachine.create(
self.apiclient,
self.testdata["virtual_machine"],
accountid=self.account.name,
domainid=self.account.domainid,
- templateid=self.iso.id,
serviceofferingid=self.service_offering.id,
diskofferingid=self.disk_offering.id,
- hypervisor=self.hypervisor
+ mode=self.zone.networktype
)
Review Comment:
`VirtualMachine.create(...)` no longer passes `templateid=self.iso.id`, so
the VM will be deployed from `self.testdata["virtual_machine"]["template"]`
(set to the normal test template in `setUp`), not from the ISO. This makes the
later `vm_response.isoid == self.iso.id` assertion incorrect and breaks the
“deploy from ISO” behavior. Pass the ISO id as `templateid` (or set
`services["template"]` to the ISO id for this call).
##########
test/integration/smoke/test_deploy_vm_iso.py:
##########
@@ -92,66 +84,82 @@ def setUp(self):
self.cleanup = [self.account]
return
- def tearDown(self):
- try:
- self.debug("Cleaning up the resources")
- cleanup_resources(self.apiclient, self.cleanup)
- self.debug("Cleanup complete!")
- except Exception as e:
- self.debug("Warning! Exception in tearDown: %s" % e)
-
@attr(
tags=[
"advanced",
"eip",
"advancedns",
"basic",
- "sg"],
- required_hardware="true")
+ "sg"
+ ],
+ required_hardware="true"
+ )
def test_deploy_vm_from_iso(self):
"""Test Deploy Virtual Machine from ISO
"""
# Validate the following:
- # 1. deploy VM using ISO
- # 2. listVM command should return the deployed VM. State of this VM
- # should be "Running".
- self.hypervisor = self.testClient.getHypervisorInfo()
- if self.hypervisor.lower() in ['lxc']:
- self.skipTest(
- "vm deploy from ISO feature is not supported on %s" %
- self.hypervisor.lower())
+ # 1. Create an ISO
+ # 2. Deploy a VM from the ISO
+ # 3. VM should be in 'Running' state
self.iso = Iso.create(
self.apiclient,
- self.testdata["configurableData"]["bootableIso"],
+ self.testdata["iso"],
account=self.account.name,
- domainid=self.account.domainid,
- zoneid=self.zone.id
+ domainid=self.account.domainid
+ )
+ self.cleanup.append(self.iso)
+
+ self.debug("ISO created with ID: %s" % self.iso.id)
+
+ list_iso_response = Iso.list(
+ self.apiclient,
+ id=self.iso.id
+ )
+ self.assertEqual(
+ isinstance(list_iso_response, list),
+ True,
+ "Check list response returns a valid list"
)
- try:
- # Download the ISO
- self.iso.download(self.apiclient)
- except Exception as e:
- raise Exception("Exception while downloading ISO %s: %s"
- % (self.iso.id, e))
-
- self.debug("Registered ISO: %s" % self.iso.name)
- self.debug("Deploying instance in the account: %s" %
- self.account.name)
+
+ self.iso.download(self.apiclient)
+
+ # Deploy Virtual Machine
self.virtual_machine = VirtualMachine.create(
self.apiclient,
self.testdata["virtual_machine"],
accountid=self.account.name,
domainid=self.account.domainid,
- templateid=self.iso.id,
serviceofferingid=self.service_offering.id,
diskofferingid=self.disk_offering.id,
- hypervisor=self.hypervisor
+ mode=self.zone.networktype
)
+ self.cleanup.append(self.virtual_machine)
+
+ self.debug("VM created with ID: %s" % self.virtual_machine.id)
- response = self.virtual_machine.getState(
+ list_vm_response = VirtualMachine.list(
self.apiclient,
- VirtualMachine.RUNNING)
- self.assertEqual(response[0], PASS, response[1])
+ id=self.virtual_machine.id
+ )
+
+ self.assertEqual(
+ isinstance(list_vm_response, list),
+ True,
+ "Check list response returns a valid list"
+ )
+ vm_response = list_vm_response[0]
+
+ self.assertEqual(
+ vm_response.state,
+ "Running",
Review Comment:
The previous version used `self.virtual_machine.getState(...,
VirtualMachine.RUNNING)` which polls until the VM transitions to Running. The
new code lists once and immediately asserts `state == "Running"`, which is
prone to race/flakiness right after deploy. Consider restoring `getState` (or
another wait) before asserting state.
##########
test/integration/smoke/test_deploy_vm_iso.py:
##########
@@ -92,66 +84,82 @@ def setUp(self):
self.cleanup = [self.account]
return
- def tearDown(self):
- try:
- self.debug("Cleaning up the resources")
- cleanup_resources(self.apiclient, self.cleanup)
- self.debug("Cleanup complete!")
- except Exception as e:
- self.debug("Warning! Exception in tearDown: %s" % e)
-
@attr(
tags=[
"advanced",
"eip",
"advancedns",
"basic",
- "sg"],
- required_hardware="true")
+ "sg"
+ ],
+ required_hardware="true"
+ )
def test_deploy_vm_from_iso(self):
"""Test Deploy Virtual Machine from ISO
"""
# Validate the following:
- # 1. deploy VM using ISO
- # 2. listVM command should return the deployed VM. State of this VM
- # should be "Running".
- self.hypervisor = self.testClient.getHypervisorInfo()
- if self.hypervisor.lower() in ['lxc']:
- self.skipTest(
- "vm deploy from ISO feature is not supported on %s" %
- self.hypervisor.lower())
+ # 1. Create an ISO
+ # 2. Deploy a VM from the ISO
+ # 3. VM should be in 'Running' state
Review Comment:
The LXC hypervisor skip was removed. Other ISO-deploy tests in this repo
still skip LXC (e.g. `test/integration/component/test_stopped_vm.py`), because
deploy-from-ISO isn’t supported there. Without the skip, this test will fail
when run against LXC environments.
--
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]
