assertion improvement on cluster DRS (#8508)

* PR template enhanced

* assertion improvement on cluster DRS

* Update test/integration/smoke/test_cluster_drs.py

* Fix host id check for balanced drs e2e test

* Update test/integration/smoke/test_cluster_drs.py

Co-authored-by: Vishesh <[email protected]>

Author: DaanHoogland

PULL_REQUEST_TEMPLATE.md

@@ -23,6 +23,7 @@ This PR...
 - [ ] Enhancement (improves an existing feature and functionality)
 - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
 - [ ] build/CI
+- [ ] test (unit or integration test code)
 
 ### Feature/Enhancement Scale or Bug Severity
 

test/integration/smoke/test_cluster_drs.py

@@ -21,8 +21,10 @@
 
 import logging
 import time
+from collections.abc import Iterable
 
 from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.cloudstackAPI import (migrateSystemVm, listRouters, listSystemVms)
 from marvin.lib.base import (Cluster, Configurations, Host, Network, NetworkOffering, ServiceOffering, VirtualMachine,
                              Zone)
 from marvin.lib.common import (get_domain, get_zone, get_template)
@@ -98,6 +100,41 @@ def setUpClass(cls):
         )
         cls._cleanup.append(cls.network)
 
+        cls.migrateSvms(cls.cluster)
+
+    @classmethod
+    def migrateSvms(cls, cluster):
+        """
+            for testing the balanced algorithm we must make sure there is at least as more free memory on host[1] than on
+            host[0]. As a grude measure we migrate any and all system vms to host[0] before the testing commences
+
+        :param cluster: the cluser to check
+        :return: None
+        """
+
+        systemVmIds = []
+        cmds = listSystemVms.listSystemVmsCmd()
+        responseS = cls.apiclient.listSystemVms(cmds)
+        if isinstance(responseS, Iterable):
+            for svm in responseS:
+                if svm.hostid != cls.hosts[0].id:
+                    systemVmIds.append(svm.id)
+        cmdv = listRouters.listRoutersCmd()
+        responseR = cls.apiclient.listRouters(cmdv)
+        if isinstance(responseR, Iterable):
+            for svm in responseR:
+                if svm.hostid != cls.hosts[0].id:
+                    systemVmIds.append(svm.id)
+        numToMigrate = len(systemVmIds)
+        cls.logger.debug(f'system vms and routers to migrate -- {numToMigrate}')
+        cmdM = migrateSystemVm.migrateSystemVmCmd()
+        cmdM.hostId=cls.hosts[0].id
+        for id in systemVmIds:
+            cmdM.virtualmachineid=id
+            responseM = cls.apiclient.migrateSystemVm(cmdM)
+            cls.logger.debug(f'migrated {responseM}')
+
+
     @classmethod
     def tearDownClass(cls):
         super(TestClusterDRS, cls).tearDownClass()
@@ -111,7 +148,6 @@ def setUp(self):
     def tearDown(self):
         super(TestClusterDRS, self).tearDown()
 
-    @classmethod
     def get_vm_host_id(cls, vm_id):
         list_vms = VirtualMachine.list(cls.apiclient, id=vm_id)
         vm = list_vms[0]
@@ -188,8 +224,8 @@ def test_01_condensed_drs_algorithm(self):
                                                        serviceofferingid=self.service_offering.id,
                                                        templateid=self.template.id, zoneid=self.zone.id,
                                                        networkids=self.network.id, hostid=self.hosts[1].id)
-        vm_2_host_id = self.get_vm_host_id(self.virtual_machine_2.id)
         self.cleanup.append(self.virtual_machine_2)
+        vm_2_host_id = self.get_vm_host_id(self.virtual_machine_2.id)
 
         self.assertNotEqual(vm_1_host_id, vm_2_host_id, msg="Both VMs should be on different hosts")
         self.wait_for_vm_start(self.virtual_machine_1)
@@ -216,13 +252,15 @@ def test_01_condensed_drs_algorithm(self):
 
     @attr(tags=["advanced"], required_hardware="false")
     def test_02_balanced_drs_algorithm(self):
-        """ Verify DRS algorithm - balanced"""
-
-        # 1. Deploy vm-1 on host 1
-        # 2. Deploy vm-2 on host 2
-        # 3. Execute DRS to move all VMs on different hosts
+        """
+            Verify DRS algorithm - balanced
 
+            # 1. Deploy vm-1 on host 1
+            # 2. Deploy vm-2 on host 2
+            # 3. Execute DRS to move all VMs on different hosts
+        """
         self.logger.debug("=== Running test_02_balanced_drs_algorithm ===")
+
         # 1. Deploy vm-1 on host 1
         self.services["virtual_machine"]["name"] = "virtual-machine-1"
         self.services["virtual_machine"]["displayname"] = "virtual-machine-1"
@@ -240,8 +278,8 @@ def test_02_balanced_drs_algorithm(self):
                                                        serviceofferingid=self.service_offering.id,
                                                        templateid=self.template.id, zoneid=self.zone.id,
                                                        networkids=self.network.id, hostid=self.hosts[0].id)
-        vm_2_host_id = self.get_vm_host_id(self.virtual_machine_2.id)
         self.cleanup.append(self.virtual_machine_2)
+        vm_2_host_id = self.get_vm_host_id(self.virtual_machine_2.id)
 
         self.assertEqual(vm_1_host_id, vm_2_host_id, msg="Both VMs should be on same hosts")
         self.wait_for_vm_start(self.virtual_machine_1)
@@ -256,12 +294,15 @@ def test_02_balanced_drs_algorithm(self):
             migration["virtualmachineid"]: migration["destinationhostid"] for migration in migrations
         }
 
-        self.assertEqual(len(vm_to_dest_host_map), 1, msg="DRS plan should have 1 migrations")
+        # this is one if no svm is considered to be migrated, it might be higher
+        self.assertTrue(len(vm_to_dest_host_map) >= 1, msg="DRS plan should have at least 1 migrations")
 
         executed_plan = self.cluster.executeDrsPlan(self.apiclient, vm_to_dest_host_map)
         self.wait_for_plan_completion(executed_plan)
 
         vm_1_host_id = self.get_vm_host_id(self.virtual_machine_1.id)
         vm_2_host_id = self.get_vm_host_id(self.virtual_machine_2.id)
 
-        self.assertNotEqual(vm_1_host_id, vm_2_host_id, msg="Both VMs should be on different hosts")
+        self.assertTrue(
+            vm_1_host_id != self.virtual_machine_1.hostid or vm_2_host_id != self.virtual_machine_2.hostid,
+                        msg="At least one VM should have been migrated to a different host")