You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2025/01/20 12:20:22 UTC
(cloudstack) branch 4.19 updated: Fix NPE issues during host rolling maintenance, due to host tags and custom constrained/unconstrained service offering (#9844)
This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.19
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.19 by this push:
new c5e8f63452a Fix NPE issues during host rolling maintenance, due to host tags and custom constrained/unconstrained service offering (#9844)
c5e8f63452a is described below
commit c5e8f63452ac61431b1722400ce5c19289cbe187
Author: Suresh Kumar Anaparti <su...@gmail.com>
AuthorDate: Mon Jan 20 17:50:17 2025 +0530
Fix NPE issues during host rolling maintenance, due to host tags and custom constrained/unconstrained service offering (#9844)
---
.../resource/RollingMaintenanceManagerImpl.java | 59 ++++++++++++++++++---
.../RollingMaintenanceManagerImplTest.java | 60 ++++++++++++++++++++++
2 files changed, 113 insertions(+), 6 deletions(-)
diff --git a/server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java b/server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java
index 25b2ad53bf2..4ada43308ee 100644
--- a/server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java
+++ b/server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java
@@ -37,6 +37,7 @@ import org.apache.cloudstack.api.command.admin.resource.StartRollingMaintenanceC
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.ObjectUtils;
import org.apache.log4j.Logger;
import com.cloud.agent.AgentManager;
@@ -65,12 +66,16 @@ import com.cloud.org.Grouping;
import com.cloud.service.ServiceOfferingVO;
import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.utils.Pair;
+import com.cloud.utils.StringUtils;
import com.cloud.utils.Ternary;
import com.cloud.utils.component.ManagerBase;
import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.UserVmDetailVO;
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.VirtualMachine.State;
import com.cloud.vm.VirtualMachineProfileImpl;
+import com.cloud.vm.VmDetailConstants;
+import com.cloud.vm.dao.UserVmDetailsDao;
import com.cloud.vm.dao.VMInstanceDao;
public class RollingMaintenanceManagerImpl extends ManagerBase implements RollingMaintenanceManager {
@@ -86,6 +91,8 @@ public class RollingMaintenanceManagerImpl extends ManagerBase implements Rollin
@Inject
private VMInstanceDao vmInstanceDao;
@Inject
+ protected UserVmDetailsDao userVmDetailsDao;
+ @Inject
private ServiceOfferingDao serviceOfferingDao;
@Inject
private ClusterDetailsDao clusterDetailsDao;
@@ -621,10 +628,19 @@ public class RollingMaintenanceManagerImpl extends ManagerBase implements Rollin
int successfullyCheckedVmMigrations = 0;
for (VMInstanceVO runningVM : vmsRunning) {
boolean canMigrateVm = false;
+ Ternary<Integer, Integer, Integer> cpuSpeedAndRamSize = getComputeResourcesCpuSpeedAndRamSize(runningVM);
+ Integer cpu = cpuSpeedAndRamSize.first();
+ Integer speed = cpuSpeedAndRamSize.second();
+ Integer ramSize = cpuSpeedAndRamSize.third();
+ if (ObjectUtils.anyNull(cpu, speed, ramSize)) {
+ s_logger.warn(String.format("Cannot fetch compute resources for the VM %s, skipping it from the capacity check", runningVM));
+ continue;
+ }
+
ServiceOfferingVO serviceOffering = serviceOfferingDao.findById(runningVM.getServiceOfferingId());
for (Host hostInCluster : hostsInCluster) {
if (!checkHostTags(hostTags, hostTagsDao.getHostTags(hostInCluster.getId()), serviceOffering.getHostTag())) {
- s_logger.debug(String.format("Host tags mismatch between %s and %s Skipping it from the capacity check", host, hostInCluster));
+ s_logger.warn(String.format("Host tags mismatch between %s and %s, skipping it from the capacity check", host, hostInCluster));
continue;
}
DeployDestination deployDestination = new DeployDestination(null, null, null, host);
@@ -634,13 +650,13 @@ public class RollingMaintenanceManagerImpl extends ManagerBase implements Rollin
affinityChecks = affinityChecks && affinityProcessor.check(vmProfile, deployDestination);
}
if (!affinityChecks) {
- s_logger.debug(String.format("Affinity check failed between %s and %s Skipping it from the capacity check", host, hostInCluster));
+ s_logger.warn(String.format("Affinity check failed between %s and %s, skipping it from the capacity check", host, hostInCluster));
continue;
}
boolean maxGuestLimit = capacityManager.checkIfHostReachMaxGuestLimit(host);
- boolean hostHasCPUCapacity = capacityManager.checkIfHostHasCpuCapability(hostInCluster.getId(), serviceOffering.getCpu(), serviceOffering.getSpeed());
- int cpuRequested = serviceOffering.getCpu() * serviceOffering.getSpeed();
- long ramRequested = serviceOffering.getRamSize() * 1024L * 1024L;
+ boolean hostHasCPUCapacity = capacityManager.checkIfHostHasCpuCapability(hostInCluster.getId(), cpu, speed);
+ int cpuRequested = cpu * speed;
+ long ramRequested = ramSize * 1024L * 1024L;
ClusterDetailsVO clusterDetailsCpuOvercommit = clusterDetailsDao.findDetail(cluster.getId(), "cpuOvercommitRatio");
ClusterDetailsVO clusterDetailsRamOvercommmt = clusterDetailsDao.findDetail(cluster.getId(), "memoryOvercommitRatio");
Float cpuOvercommitRatio = Float.parseFloat(clusterDetailsCpuOvercommit.getValue());
@@ -666,11 +682,42 @@ public class RollingMaintenanceManagerImpl extends ManagerBase implements Rollin
return new Pair<>(true, "OK");
}
+ protected Ternary<Integer, Integer, Integer> getComputeResourcesCpuSpeedAndRamSize(VMInstanceVO runningVM) {
+ ServiceOfferingVO serviceOffering = serviceOfferingDao.findById(runningVM.getServiceOfferingId());
+ Integer cpu = serviceOffering.getCpu();
+ Integer speed = serviceOffering.getSpeed();
+ Integer ramSize = serviceOffering.getRamSize();
+ if (!serviceOffering.isDynamic()) {
+ return new Ternary<>(cpu, speed, ramSize);
+ }
+
+ List<UserVmDetailVO> vmDetails = userVmDetailsDao.listDetails(runningVM.getId());
+ if (CollectionUtils.isEmpty(vmDetails)) {
+ return new Ternary<>(cpu, speed, ramSize);
+ }
+
+ for (UserVmDetailVO vmDetail : vmDetails) {
+ if (StringUtils.isBlank(vmDetail.getName()) || StringUtils.isBlank(vmDetail.getValue())) {
+ continue;
+ }
+
+ if (cpu == null && VmDetailConstants.CPU_NUMBER.equals(vmDetail.getName())) {
+ cpu = Integer.valueOf(vmDetail.getValue());
+ } else if (speed == null && VmDetailConstants.CPU_SPEED.equals(vmDetail.getName())) {
+ speed = Integer.valueOf(vmDetail.getValue());
+ } else if (ramSize == null && VmDetailConstants.MEMORY.equals(vmDetail.getName())) {
+ ramSize = Integer.valueOf(vmDetail.getValue());
+ }
+ }
+
+ return new Ternary<>(cpu, speed, ramSize);
+ }
+
/**
* Check hosts tags
*/
private boolean checkHostTags(List<HostTagVO> hostTags, List<HostTagVO> hostInClusterTags, String offeringTag) {
- if (CollectionUtils.isEmpty(hostTags) && CollectionUtils.isEmpty(hostInClusterTags)) {
+ if ((CollectionUtils.isEmpty(hostTags) && CollectionUtils.isEmpty(hostInClusterTags)) || StringUtils.isBlank(offeringTag)) {
return true;
} else if ((CollectionUtils.isNotEmpty(hostTags) && CollectionUtils.isEmpty(hostInClusterTags)) ||
(CollectionUtils.isEmpty(hostTags) && CollectionUtils.isNotEmpty(hostInClusterTags))) {
diff --git a/server/src/test/java/com/cloud/resource/RollingMaintenanceManagerImplTest.java b/server/src/test/java/com/cloud/resource/RollingMaintenanceManagerImplTest.java
index ef0277fd372..d8363964f05 100644
--- a/server/src/test/java/com/cloud/resource/RollingMaintenanceManagerImplTest.java
+++ b/server/src/test/java/com/cloud/resource/RollingMaintenanceManagerImplTest.java
@@ -23,7 +23,15 @@ import com.cloud.host.Status;
import com.cloud.host.dao.HostDao;
import com.cloud.hypervisor.Hypervisor;
import com.cloud.org.Cluster;
+import com.cloud.service.ServiceOfferingVO;
+import com.cloud.service.dao.ServiceOfferingDao;
+import com.cloud.utils.Ternary;
import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.UserVmDetailVO;
+import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.VmDetailConstants;
+import com.cloud.vm.dao.UserVmDetailsDao;
+
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -53,6 +61,12 @@ public class RollingMaintenanceManagerImplTest {
HostVO host4;
@Mock
Cluster cluster;
+ @Mock
+ VMInstanceVO vm;
+ @Mock
+ ServiceOfferingDao serviceOfferingDao;
+ @Mock
+ UserVmDetailsDao userVmDetailsDao;
@Spy
@InjectMocks
@@ -164,4 +178,50 @@ public class RollingMaintenanceManagerImplTest {
Assert.assertEquals(1, hosts.size());
}
+
+ @Test
+ public void testGetComputeResourcesCpuSpeedAndRamSize_ForNormalOffering() {
+ ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class);
+ Mockito.when(serviceOffering.isDynamic()).thenReturn(false);
+ Mockito.when(serviceOffering.getCpu()).thenReturn(1);
+ Mockito.when(serviceOffering.getSpeed()).thenReturn(500);
+ Mockito.when(serviceOffering.getRamSize()).thenReturn(512);
+
+ Mockito.when(vm.getServiceOfferingId()).thenReturn(1L);
+ Mockito.when(serviceOfferingDao.findById(1L)).thenReturn(serviceOffering);
+
+ Ternary<Integer, Integer, Integer> cpuSpeedAndRamSize = manager.getComputeResourcesCpuSpeedAndRamSize(vm);
+
+ Assert.assertEquals(1, cpuSpeedAndRamSize.first().intValue());
+ Assert.assertEquals(500, cpuSpeedAndRamSize.second().intValue());
+ Assert.assertEquals(512, cpuSpeedAndRamSize.third().intValue());
+ }
+
+ @Test
+ public void testGetComputeResourcesCpuSpeedAndRamSize_ForCustomOffering() {
+ ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class);
+ Mockito.when(serviceOffering.isDynamic()).thenReturn(true);
+ Mockito.when(serviceOffering.getCpu()).thenReturn(null);
+ Mockito.when(serviceOffering.getSpeed()).thenReturn(null);
+ Mockito.when(serviceOffering.getRamSize()).thenReturn(null);
+
+ List<UserVmDetailVO> vmDetails = new ArrayList<>();
+ UserVmDetailVO cpuDetail = new UserVmDetailVO(1L, VmDetailConstants.CPU_NUMBER, "2", false);
+ vmDetails.add(cpuDetail);
+ UserVmDetailVO speedDetail = new UserVmDetailVO(1L, VmDetailConstants.CPU_SPEED, "1000", false);
+ vmDetails.add(speedDetail);
+ UserVmDetailVO ramSizeDetail = new UserVmDetailVO(1L, VmDetailConstants.MEMORY, "1024", false);
+ vmDetails.add(ramSizeDetail);
+
+ Mockito.when(vm.getId()).thenReturn(1L);
+ Mockito.when(vm.getServiceOfferingId()).thenReturn(1L);
+ Mockito.when(serviceOfferingDao.findById(1L)).thenReturn(serviceOffering);
+ Mockito.when(userVmDetailsDao.listDetails(1L)).thenReturn(vmDetails);
+
+ Ternary<Integer, Integer, Integer> cpuSpeedAndRamSize = manager.getComputeResourcesCpuSpeedAndRamSize(vm);
+
+ Assert.assertEquals(2, cpuSpeedAndRamSize.first().intValue());
+ Assert.assertEquals(1000, cpuSpeedAndRamSize.second().intValue());
+ Assert.assertEquals(1024, cpuSpeedAndRamSize.third().intValue());
+ }
}