FR563 - Capacity recalculation on migration - considering overcommit ratio (#21)

Pearl1594 · Pearl Dsilva · shwstppr · commit 8dfd125fe685 · 2021-08-09T15:26:13.000+05:30
https://shapeblue.atlassian.net/browse/FRO-916 Co-authored-by: Pearl Dsilva <pearl.dsilva@shapeblue.com>
diff --git a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java
@@ -2391,6 +2391,7 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy
         }
 
         final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, null, _offeringDao.findById(vm.getId(), vm.getServiceOfferingId()), null, null);
+
         _networkMgr.prepareNicForMigration(profile, dest);
         volumeMgr.prepareForMigration(profile, dest);
         profile.setConfigDriveLabel(VmConfigDriveLabel.value());
diff --git a/server/src/com/cloud/api/query/dao/HostJoinDaoImpl.java b/server/src/com/cloud/api/query/dao/HostJoinDaoImpl.java
@@ -39,13 +39,15 @@
 
 import com.cloud.api.ApiDBUtils;
 import com.cloud.api.query.vo.HostJoinVO;
+import com.cloud.dc.ClusterDetailsDao;
 import com.cloud.gpu.HostGpuGroupsVO;
 import com.cloud.gpu.VGPUTypesVO;
 import com.cloud.host.Host;
 import com.cloud.host.HostStats;
 import com.cloud.host.dao.HostDetailsDao;
 import com.cloud.hypervisor.Hypervisor;
 import com.cloud.storage.StorageStats;
+import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.db.GenericDaoBase;
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
@@ -64,6 +66,8 @@ public class HostJoinDaoImpl extends GenericDaoBase<HostJoinVO, Long> implements
     @Inject
     private HAConfigDao haConfigDao;
     @Inject
+    protected ClusterDetailsDao _clusterDetailsDao;
+    @Inject
     private OutOfBandManagementDao outOfBandManagementDao;
 
     private final SearchBuilder<HostJoinVO> hostSearch;
@@ -183,7 +187,9 @@ public HostResponse newHostResponse(HostJoinVO host, EnumSet<HostDetails> detail
                 Float cpuWithOverprovisioning = host.getCpus() * host.getSpeed() * ApiDBUtils.getCpuOverprovisioningFactor(host.getClusterId());
                 String cpuAlloc = decimalFormat.format(((float)cpu / cpuWithOverprovisioning * 100f)) + "%";
                 hostResponse.setCpuAllocated(cpuAlloc);
-                hostResponse.setCpuWithOverprovisioning(cpuWithOverprovisioning.toString());
+                final float clusterCpuOvercommitRatio = NumbersUtil.parseFloat(_clusterDetailsDao.findDetail(host.getClusterId(), "cpuOvercommitRatio").getValue(), ApiDBUtils.getCpuOverprovisioningFactor(host.getClusterId()));
+                String cpuWithOverprovisioningStr = Float.toString(host.getCpus() * host.getSpeed() * clusterCpuOvercommitRatio);
+                hostResponse.setCpuWithOverprovisioning(cpuWithOverprovisioningStr);
             }
 
             if (details.contains(HostDetails.all) || details.contains(HostDetails.stats)) {
@@ -332,7 +338,9 @@ public HostForMigrationResponse newHostForMigrationResponse(HostJoinVO host, Enu
                 Float cpuWithOverprovisioning = new Float(host.getCpus() * host.getSpeed() * ApiDBUtils.getCpuOverprovisioningFactor(host.getClusterId()));
                 String cpuAlloc = decimalFormat.format(((float)cpu / cpuWithOverprovisioning * 100f)).toString() + "%";
                 hostResponse.setCpuAllocated(cpuAlloc);
-                hostResponse.setCpuWithOverprovisioning(cpuWithOverprovisioning.toString());
+                final float clusterCpuOvercommitRatio = NumbersUtil.parseFloat(_clusterDetailsDao.findDetail(host.getClusterId(), "cpuOvercommitRatio").getValue(), ApiDBUtils.getCpuOverprovisioningFactor(host.getClusterId()));
+                String cpuWithOverprovisioningStr = Float.toString(host.getCpus() * host.getSpeed() * clusterCpuOvercommitRatio);
+                hostResponse.setCpuWithOverprovisioning(cpuWithOverprovisioningStr);
             }
 
             if (details.contains(HostDetails.all) || details.contains(HostDetails.stats)) {
diff --git a/server/src/com/cloud/capacity/CapacityManagerImpl.java b/server/src/com/cloud/capacity/CapacityManagerImpl.java
@@ -48,7 +48,6 @@
 import com.cloud.configuration.Config;
 import com.cloud.dc.ClusterDetailsDao;
 import com.cloud.dc.ClusterDetailsVO;
-import com.cloud.dc.ClusterVO;
 import com.cloud.dc.dao.ClusterDao;
 import com.cloud.deploy.DeploymentClusterPlanner;
 import com.cloud.event.UsageEventVO;
@@ -264,6 +263,9 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
     @Override
     public void allocateVmCapacity(VirtualMachine vm, final boolean fromLastHost) {
 
+        if (vm == null) {
+            return;
+        }
         final long hostId = vm.getHostId();
         HostVO host = _hostDao.findById(hostId);
         final long clusterId = host.getClusterId();
@@ -274,18 +276,22 @@ public void allocateVmCapacity(VirtualMachine vm, final boolean fromLastHost) {
 
         CapacityVO capacityCpu = _capacityDao.findByHostIdType(hostId, Capacity.CAPACITY_TYPE_CPU);
         CapacityVO capacityMem = _capacityDao.findByHostIdType(hostId, Capacity.CAPACITY_TYPE_MEMORY);
-
         if (capacityCpu == null || capacityMem == null || svo == null) {
             return;
         }
 
-        final int cpu = svo.getCpu() * svo.getSpeed();
-        final long ram = svo.getRamSize() * 1024L * 1024L;
+        final int cpu = (int) (svo.getCpu() * svo.getSpeed());
+        final long ram = (long) (svo.getRamSize() * 1024L * 1024L);
 
         try {
             final long capacityCpuId = capacityCpu.getId();
             final long capacityMemId = capacityMem.getId();
 
+            // Update the over commit ratio of the VM to reflect the same value as that of the cluster to which it has been migrated to / deployed on.
+            VMInstanceVO vmInstanceVO = _vmDao.findById(vm.getId());
+            _userVmDetailsDao.addDetail(vmInstanceVO.getId(), "cpuOvercommitRatio", String.valueOf(cpuOvercommitRatio), true);
+            _userVmDetailsDao.addDetail(vmInstanceVO.getId(), "memoryOvercommitRatio", String.valueOf(memoryOvercommitRatio), true);
+
             Transaction.execute(new TransactionCallbackNoReturn() {
                 @Override
                 public void doInTransactionWithoutResult(TransactionStatus status) {
@@ -591,35 +597,19 @@ public void updateCapacityForHost(final Host host) {
             s_logger.debug("Found " + vms.size() + " VMs on host " + host.getId());
         }
 
-        ClusterVO cluster = _clusterDao.findById(host.getClusterId());
-        ClusterDetailsVO clusterDetailCpu = _clusterDetailsDao.findDetail(cluster.getId(), "cpuOvercommitRatio");
-        ClusterDetailsVO clusterDetailRam = _clusterDetailsDao.findDetail(cluster.getId(), "memoryOvercommitRatio");
-        Float clusterCpuOvercommitRatio = Float.parseFloat(clusterDetailCpu.getValue());
-        Float clusterRamOvercommitRatio = Float.parseFloat(clusterDetailRam.getValue());
         for (VMInstanceVO vm : vms) {
             Float cpuOvercommitRatio = 1.0f;
             Float ramOvercommitRatio = 1.0f;
             Map<String, String> vmDetails = _userVmDetailsDao.listDetailsKeyPairs(vm.getId());
-            String vmDetailCpu = vmDetails.get("cpuOvercommitRatio");
-            String vmDetailRam = vmDetails.get("memoryOvercommitRatio");
-            if (vmDetailCpu != null) {
-                //if vmDetail_cpu is not null it means it is running in a overcommited cluster.
-                cpuOvercommitRatio = Float.parseFloat(vmDetailCpu);
-                ramOvercommitRatio = Float.parseFloat(vmDetailRam);
-            }
             ServiceOffering so = offeringsMap.get(vm.getServiceOfferingId());
             if (so.isDynamic()) {
                 usedMemory +=
-                    ((Integer.parseInt(vmDetails.get(UsageEventVO.DynamicParameters.memory.name())) * 1024L * 1024L) / ramOvercommitRatio) *
-                        clusterRamOvercommitRatio;
+                    (Integer.parseInt(vmDetails.get(UsageEventVO.DynamicParameters.memory.name())) * 1024L * 1024L);
                 usedCpu +=
-                    ((Integer.parseInt(vmDetails.get(UsageEventVO.DynamicParameters.cpuNumber.name())) * Integer.parseInt(vmDetails.get(UsageEventVO.DynamicParameters.cpuSpeed.name()))) / cpuOvercommitRatio) *
-                        clusterCpuOvercommitRatio;
-                usedCpuCore += Integer.parseInt(vmDetails.get(UsageEventVO.DynamicParameters.cpuNumber.name()));
+                    (Integer.parseInt(vmDetails.get(UsageEventVO.DynamicParameters.cpuNumber.name())) * Integer.parseInt(vmDetails.get(UsageEventVO.DynamicParameters.cpuSpeed.name())));
             } else {
-                usedMemory += ((so.getRamSize() * 1024L * 1024L) / ramOvercommitRatio) * clusterRamOvercommitRatio;
-                usedCpu += ((so.getCpu() * so.getSpeed()) / cpuOvercommitRatio) * clusterCpuOvercommitRatio;
-                usedCpuCore += so.getCpu();
+                usedMemory += (so.getRamSize() * 1024L * 1024L);
+                usedCpu += (so.getCpu() * so.getSpeed());
             }
         }
 
@@ -632,27 +622,16 @@ public void updateCapacityForHost(final Host host) {
             Float ramOvercommitRatio = 1.0f;
             long secondsSinceLastUpdate = (DateUtil.currentGMTTime().getTime() - vm.getUpdateTime().getTime()) / 1000;
             if (secondsSinceLastUpdate < _vmCapacityReleaseInterval) {
-                UserVmDetailVO vmDetailCpu = _userVmDetailsDao.findDetail(vm.getId(), "cpuOvercommitRatio");
-                UserVmDetailVO vmDetailRam = _userVmDetailsDao.findDetail(vm.getId(), "memoryOvercommitRatio");
-                if (vmDetailCpu != null) {
-                    //if vmDetail_cpu is not null it means it is running in a overcommited cluster.
-                    cpuOvercommitRatio = Float.parseFloat(vmDetailCpu.getValue());
-                    ramOvercommitRatio = Float.parseFloat(vmDetailRam.getValue());
-                }
                 ServiceOffering so = offeringsMap.get(vm.getServiceOfferingId());
                 Map<String, String> vmDetails = _userVmDetailsDao.listDetailsKeyPairs(vm.getId());
                 if (so.isDynamic()) {
-                    reservedMemory +=
-                        ((Integer.parseInt(vmDetails.get(UsageEventVO.DynamicParameters.memory.name())) * 1024L * 1024L) / ramOvercommitRatio) *
-                            clusterRamOvercommitRatio;
-                    reservedCpu +=
-                        ((Integer.parseInt(vmDetails.get(UsageEventVO.DynamicParameters.cpuNumber.name())) * Integer.parseInt(vmDetails.get(UsageEventVO.DynamicParameters.cpuSpeed.name()))) / cpuOvercommitRatio) *
-                            clusterCpuOvercommitRatio;
-                    reservedCpuCore += Integer.parseInt(vmDetails.get(UsageEventVO.DynamicParameters.cpuNumber.name()));
+                        reservedMemory +=
+                                ((Integer.parseInt(vmDetails.get(UsageEventVO.DynamicParameters.memory.name())) * 1024L * 1024L));
+                        reservedCpu +=
+                                (Integer.parseInt(vmDetails.get(UsageEventVO.DynamicParameters.cpuNumber.name())) * Integer.parseInt(vmDetails.get(UsageEventVO.DynamicParameters.cpuSpeed.name())));
                 } else {
-                    reservedMemory += ((so.getRamSize() * 1024L * 1024L) / ramOvercommitRatio) * clusterRamOvercommitRatio;
-                    reservedCpu += (so.getCpu() * so.getSpeed() / cpuOvercommitRatio) * clusterCpuOvercommitRatio;
-                    reservedCpuCore += so.getCpu();
+                        reservedMemory += (so.getRamSize() * 1024L * 1024L);
+                        reservedCpu += (so.getCpu() * so.getSpeed());
                 }
             } else {
                 // signal if not done already, that the VM has been stopped for skip.counting.hours,
@@ -885,6 +864,15 @@ public boolean postStateTransitionEvent(StateMachine2.Transition<State, Event> t
         allocateVmCapacity(vm, fromLastHost);
       }
 
+      if (oldState == State.Migrating && newState == State.Running) {
+          boolean fromLastHost = false;
+          if (vm.getHostId().equals(vm.getLastHostId())) {
+              s_logger.debug("VM starting again on the last host it was stopped on");
+              fromLastHost = true;
+          }
+          allocateVmCapacity(vm, fromLastHost);
+      }
+
       if (newState == State.Stopped) {
         if (vm.getType() == VirtualMachine.Type.User) {
 
diff --git a/test/integration/smoke/test_vm_life_cycle.py b/test/integration/smoke/test_vm_life_cycle.py
@@ -17,7 +17,7 @@
 """ BVT tests for Virtual Machine Life Cycle
 """
 # Import Local Modules
-from marvin.cloudstackTestCase import cloudstackTestCase
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
 from marvin.cloudstackAPI import (recoverVirtualMachine,
                                   destroyVirtualMachine,
                                   attachIso,
@@ -40,8 +40,10 @@
                              DiskOffering)
 from marvin.lib.common import (get_domain,
                                get_zone,
-                               get_template,
-                               list_hosts)
+                               list_clusters,
+                               list_hosts,
+                               list_storage_pools,
+                               get_template)
 from marvin.codes import FAILED, PASS
 from nose.plugins.attrib import attr
 # Import System modules
@@ -786,6 +788,112 @@ def test_10_attachAndDetach_iso(self):
         )
         return
 
+    @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg", "security"], required_hardware="false")
+    def test_11_livemigrate_VM_across_cluster_vmware(self):
+        count_host = 0
+        count_pool = 0
+        try:
+            self.list_vmware_clusters = list_clusters(self.apiclient, hypervisor="vmware")
+        except Exception as e:
+            raise unittest.SkipTest(e)
+        if len(self.list_vmware_clusters) < 2:
+            raise self.skipTest("The setup doesn't have more than one cluster, cannot execute live migration across cluster")
+        if len(self.list_vmware_clusters) >= 2:
+            for cluster in self.list_vmware_clusters:
+                if len(list_hosts(self.apiclient, clusterid=cluster.id)) >= 1:
+                    count_host += 1
+                pools = list_storage_pools(self.apiclient, clusterid=cluster.id)
+                if len(pools) >= 1:
+                    count_pool += 1
+        if count_host < 2 or count_pool < 2:
+            raise self.skipTest("The setup doesn't have enough pools or enough hosts. To run these tests the setup must have atleast 2 clusters, \
+            each having min 1 host and 1 storage pools")
+
+        # get each cluster's cpu and memory over commit ratios
+        cluster_1_cpu_oc = self.list_vmware_clusters[0].cpuovercommitratio
+        cluster_1_ram_oc = self.list_vmware_clusters[0].memoryovercommitratio
+        cluster_2_cpu_oc = self.list_vmware_clusters[1].cpuovercommitratio
+        cluster_2_ram_oc = self.list_vmware_clusters[1].memoryovercommitratio
+
+        if cluster_1_cpu_oc == cluster_2_cpu_oc and cluster_1_ram_oc == cluster_2_ram_oc:
+            raise self.skipTest("The 2 clusters have same memory and cpu over commit ratios, skipping test")
+
+        hosts_c1 = Host.list(
+            self.apiclient,
+            zoneid=self.zone.id,
+            type='Routing',
+            clusterid=self.list_vmware_clusters[0]
+        )
+
+        hosts_c2 = Host.list(
+            self.apiclient,
+            zoneid=self.zone.id,
+            type='Routing',
+            clusterid=self.list_vmware_clusters[1]
+        )
+
+        target_host = hosts_c1[0]
+        migrate_host = hosts_c2[0]
+
+        self.vm_to_migrate = VirtualMachine.create(
+            self.apiclient,
+            self.services["small"],
+            accountid=self.account.name,
+            domainid=self.account.domainid,
+            serviceofferingid=self.small_offering.id,
+            mode=self.services["mode"],
+            hostid=target_host.id
+        )
+        self.debug("Migrating VM-ID: %s to Host: %s" % (
+            self.vm_to_migrate.id,
+            migrate_host.id
+        ))
+
+        self.cleanup.append(self.vm_to_migrate)
+
+        try:
+            self.vm_to_migrate.migrate(self.apiclient, migrate_host.id)
+        except Exception as e:
+            self.fail("Failed to migrate instance: %s" % e)
+
+        # verify if the overcommit ratios are modified
+        qresultset = self.dbclient.execute(
+            "select value from user_vm_details where vm_id = '%s and name = 'cpuOvercommitRatio';" \
+            % self.vm_to_migrate.id
+        )
+        self.assertNotEqual(
+            len(qresultset),
+            0,
+            "Check DB Query result set"
+        )
+        self.assertEqual(
+            isinstance(qresultset, list),
+            True,
+            "Check DB query result set for valid data"
+        )
+        qresult = str(qresultset)
+        self.debug("Query result: %s" % qresult)
+
+        self.assertEqual(qresultset[0][0], cluster_2_cpu_oc, "VM's cpu over-commit ratio not updated on migration")
+
+        qresultset = self.dbclient.execute(
+            "select value from user_vm_details where vm_id = '%s and name = 'memoryOvercommitRatio';" \
+            % self.vm_to_migrate.id
+        )
+        self.assertNotEqual(
+            len(qresultset),
+            0,
+            "Check DB Query result set"
+        )
+        self.assertEqual(
+            isinstance(qresultset, list),
+            True,
+            "Check DB query result set for valid data"
+        )
+        qresult = str(qresultset)
+        self.debug("Query result: %s" % qresult)
+
+        self.assertEqual(qresultset[0][0], cluster_2_ram_oc, "VM's memory over-commit ratio not updated on migration")
 
 class TestSecuredVmMigration(cloudstackTestCase):
 

Original file line number	Diff line number	Diff line change
`@@ -2391,6 +2391,7 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy`
`2391`	`2391`	`}`
`2392`	`2392`
`2393`	`2393`	`final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, null, _offeringDao.findById(vm.getId(), vm.getServiceOfferingId()), null, null);`
	`2394`	`+`
`2394`	`2395`	`_networkMgr.prepareNicForMigration(profile, dest);`
`2395`	`2396`	`volumeMgr.prepareForMigration(profile, dest);`
`2396`	`2397`	`profile.setConfigDriveLabel(VmConfigDriveLabel.value());`