Skip to content

Commit e6e54c6

Browse files
sureshanapartidhslove
sureshanaparti
authored and
dhslove
committed
Fix NPE issues during host rolling maintenance, due to host tags and custom constrained/unconstrained service offering (apache#9844)
1 parent 481013f commit e6e54c6

File tree

2 files changed

+114
-6
lines changed

2 files changed

+114
-6
lines changed

‎server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
import org.apache.cloudstack.context.CallContext;
3838
import org.apache.cloudstack.framework.config.ConfigKey;
3939
import org.apache.commons.collections.CollectionUtils;
40+
import org.apache.commons.lang3.ObjectUtils;
41+
import org.apache.log4j.Logger;
4042

4143
import com.cloud.agent.AgentManager;
4244
import com.cloud.agent.api.Answer;
@@ -64,12 +66,16 @@
6466
import com.cloud.service.ServiceOfferingVO;
6567
import com.cloud.service.dao.ServiceOfferingDao;
6668
import com.cloud.utils.Pair;
69+
import com.cloud.utils.StringUtils;
6770
import com.cloud.utils.Ternary;
6871
import com.cloud.utils.component.ManagerBase;
6972
import com.cloud.utils.exception.CloudRuntimeException;
73+
import com.cloud.vm.UserVmDetailVO;
7074
import com.cloud.vm.VMInstanceVO;
7175
import com.cloud.vm.VirtualMachine.State;
7276
import com.cloud.vm.VirtualMachineProfileImpl;
77+
import com.cloud.vm.VmDetailConstants;
78+
import com.cloud.vm.dao.UserVmDetailsDao;
7379
import com.cloud.vm.dao.VMInstanceDao;
7480

7581
public class RollingMaintenanceManagerImpl extends ManagerBase implements RollingMaintenanceManager {
@@ -85,6 +91,8 @@ public class RollingMaintenanceManagerImpl extends ManagerBase implements Rollin
8591
@Inject
8692
private VMInstanceDao vmInstanceDao;
8793
@Inject
94+
protected UserVmDetailsDao userVmDetailsDao;
95+
@Inject
8896
private ServiceOfferingDao serviceOfferingDao;
8997
@Inject
9098
private ClusterDetailsDao clusterDetailsDao;
@@ -619,10 +627,19 @@ private Pair<Boolean, String> performCapacityChecksBeforeHostInMaintenance(Host
619627
int successfullyCheckedVmMigrations = 0;
620628
for (VMInstanceVO runningVM : vmsRunning) {
621629
boolean canMigrateVm = false;
630+
Ternary<Integer, Integer, Integer> cpuSpeedAndRamSize = getComputeResourcesCpuSpeedAndRamSize(runningVM);
631+
Integer cpu = cpuSpeedAndRamSize.first();
632+
Integer speed = cpuSpeedAndRamSize.second();
633+
Integer ramSize = cpuSpeedAndRamSize.third();
634+
if (ObjectUtils.anyNull(cpu, speed, ramSize)) {
635+
logger.warn(String.format("Cannot fetch compute resources for the VM %s, skipping it from the capacity check", runningVM));
636+
continue;
637+
}
638+
622639
ServiceOfferingVO serviceOffering = serviceOfferingDao.findById(runningVM.getServiceOfferingId());
623640
for (Host hostInCluster : hostsInCluster) {
624641
if (!checkHostTags(hostTags, hostTagsDao.getHostTags(hostInCluster.getId()), serviceOffering.getHostTag())) {
625-
logger.debug(String.format("Host tags mismatch between %s and %s Skipping it from the capacity check", host, hostInCluster));
642+
logger.warn(String.format("Host tags mismatch between %s and %s, skipping it from the capacity check", host, hostInCluster));
626643
continue;
627644
}
628645
DeployDestination deployDestination = new DeployDestination(null, null, null, host);
@@ -632,13 +649,13 @@ private Pair<Boolean, String> performCapacityChecksBeforeHostInMaintenance(Host
632649
affinityChecks = affinityChecks && affinityProcessor.check(vmProfile, deployDestination);
633650
}
634651
if (!affinityChecks) {
635-
logger.debug(String.format("Affinity check failed between %s and %s Skipping it from the capacity check", host, hostInCluster));
652+
logger.warn(String.format("Affinity check failed between %s and %s, skipping it from the capacity check", host, hostInCluster));
636653
continue;
637654
}
638655
boolean maxGuestLimit = capacityManager.checkIfHostReachMaxGuestLimit(host);
639-
boolean hostHasCPUCapacity = capacityManager.checkIfHostHasCpuCapability(hostInCluster.getId(), serviceOffering.getCpu(), serviceOffering.getSpeed());
640-
int cpuRequested = serviceOffering.getCpu() * serviceOffering.getSpeed();
641-
long ramRequested = serviceOffering.getRamSize() * 1024L * 1024L;
656+
boolean hostHasCPUCapacity = capacityManager.checkIfHostHasCpuCapability(hostInCluster.getId(), cpu, speed);
657+
int cpuRequested = cpu * speed;
658+
long ramRequested = ramSize * 1024L * 1024L;
642659
ClusterDetailsVO clusterDetailsCpuOvercommit = clusterDetailsDao.findDetail(cluster.getId(), "cpuOvercommitRatio");
643660
ClusterDetailsVO clusterDetailsRamOvercommmt = clusterDetailsDao.findDetail(cluster.getId(), "memoryOvercommitRatio");
644661
Float cpuOvercommitRatio = Float.parseFloat(clusterDetailsCpuOvercommit.getValue());
@@ -664,11 +681,42 @@ private Pair<Boolean, String> performCapacityChecksBeforeHostInMaintenance(Host
664681
return new Pair<>(true, "OK");
665682
}
666683

684+
protected Ternary<Integer, Integer, Integer> getComputeResourcesCpuSpeedAndRamSize(VMInstanceVO runningVM) {
685+
ServiceOfferingVO serviceOffering = serviceOfferingDao.findById(runningVM.getServiceOfferingId());
686+
Integer cpu = serviceOffering.getCpu();
687+
Integer speed = serviceOffering.getSpeed();
688+
Integer ramSize = serviceOffering.getRamSize();
689+
if (!serviceOffering.isDynamic()) {
690+
return new Ternary<>(cpu, speed, ramSize);
691+
}
692+
693+
List<UserVmDetailVO> vmDetails = userVmDetailsDao.listDetails(runningVM.getId());
694+
if (CollectionUtils.isEmpty(vmDetails)) {
695+
return new Ternary<>(cpu, speed, ramSize);
696+
}
697+
698+
for (UserVmDetailVO vmDetail : vmDetails) {
699+
if (StringUtils.isBlank(vmDetail.getName()) || StringUtils.isBlank(vmDetail.getValue())) {
700+
continue;
701+
}
702+
703+
if (cpu == null && VmDetailConstants.CPU_NUMBER.equals(vmDetail.getName())) {
704+
cpu = Integer.valueOf(vmDetail.getValue());
705+
} else if (speed == null && VmDetailConstants.CPU_SPEED.equals(vmDetail.getName())) {
706+
speed = Integer.valueOf(vmDetail.getValue());
707+
} else if (ramSize == null && VmDetailConstants.MEMORY.equals(vmDetail.getName())) {
708+
ramSize = Integer.valueOf(vmDetail.getValue());
709+
}
710+
}
711+
712+
return new Ternary<>(cpu, speed, ramSize);
713+
}
714+
667715
/**
668716
* Check hosts tags
669717
*/
670718
private boolean checkHostTags(List<HostTagVO> hostTags, List<HostTagVO> hostInClusterTags, String offeringTag) {
671-
if (CollectionUtils.isEmpty(hostTags) && CollectionUtils.isEmpty(hostInClusterTags)) {
719+
if ((CollectionUtils.isEmpty(hostTags) && CollectionUtils.isEmpty(hostInClusterTags)) || StringUtils.isBlank(offeringTag)) {
672720
return true;
673721
} else if ((CollectionUtils.isNotEmpty(hostTags) && CollectionUtils.isEmpty(hostInClusterTags)) ||
674722
(CollectionUtils.isEmpty(hostTags) && CollectionUtils.isNotEmpty(hostInClusterTags))) {

‎server/src/test/java/com/cloud/resource/RollingMaintenanceManagerImplTest.java‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,16 @@
2323
import com.cloud.host.dao.HostDao;
2424
import com.cloud.hypervisor.Hypervisor;
2525
import com.cloud.org.Cluster;
26+
import com.cloud.service.ServiceOfferingVO;
27+
import com.cloud.service.dao.ServiceOfferingDao;
28+
import com.cloud.utils.Ternary;
2629
import com.cloud.utils.exception.CloudRuntimeException;
2730
import org.junit.After;
31+
import com.cloud.vm.UserVmDetailVO;
32+
import com.cloud.vm.VMInstanceVO;
33+
import com.cloud.vm.VmDetailConstants;
34+
import com.cloud.vm.dao.UserVmDetailsDao;
35+
2836
import org.junit.Assert;
2937
import org.junit.Before;
3038
import org.junit.Test;
@@ -54,6 +62,12 @@ public class RollingMaintenanceManagerImplTest {
5462
HostVO host4;
5563
@Mock
5664
Cluster cluster;
65+
@Mock
66+
VMInstanceVO vm;
67+
@Mock
68+
ServiceOfferingDao serviceOfferingDao;
69+
@Mock
70+
UserVmDetailsDao userVmDetailsDao;
5771

5872
@Spy
5973
@InjectMocks
@@ -172,4 +186,50 @@ public void testPerformStateChecksForce() {
172186

173187
Assert.assertEquals(1, hosts.size());
174188
}
189+
190+
@Test
191+
public void testGetComputeResourcesCpuSpeedAndRamSize_ForNormalOffering() {
192+
ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class);
193+
Mockito.when(serviceOffering.isDynamic()).thenReturn(false);
194+
Mockito.when(serviceOffering.getCpu()).thenReturn(1);
195+
Mockito.when(serviceOffering.getSpeed()).thenReturn(500);
196+
Mockito.when(serviceOffering.getRamSize()).thenReturn(512);
197+
198+
Mockito.when(vm.getServiceOfferingId()).thenReturn(1L);
199+
Mockito.when(serviceOfferingDao.findById(1L)).thenReturn(serviceOffering);
200+
201+
Ternary<Integer, Integer, Integer> cpuSpeedAndRamSize = manager.getComputeResourcesCpuSpeedAndRamSize(vm);
202+
203+
Assert.assertEquals(1, cpuSpeedAndRamSize.first().intValue());
204+
Assert.assertEquals(500, cpuSpeedAndRamSize.second().intValue());
205+
Assert.assertEquals(512, cpuSpeedAndRamSize.third().intValue());
206+
}
207+
208+
@Test
209+
public void testGetComputeResourcesCpuSpeedAndRamSize_ForCustomOffering() {
210+
ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class);
211+
Mockito.when(serviceOffering.isDynamic()).thenReturn(true);
212+
Mockito.when(serviceOffering.getCpu()).thenReturn(null);
213+
Mockito.when(serviceOffering.getSpeed()).thenReturn(null);
214+
Mockito.when(serviceOffering.getRamSize()).thenReturn(null);
215+
216+
List<UserVmDetailVO> vmDetails = new ArrayList<>();
217+
UserVmDetailVO cpuDetail = new UserVmDetailVO(1L, VmDetailConstants.CPU_NUMBER, "2", false);
218+
vmDetails.add(cpuDetail);
219+
UserVmDetailVO speedDetail = new UserVmDetailVO(1L, VmDetailConstants.CPU_SPEED, "1000", false);
220+
vmDetails.add(speedDetail);
221+
UserVmDetailVO ramSizeDetail = new UserVmDetailVO(1L, VmDetailConstants.MEMORY, "1024", false);
222+
vmDetails.add(ramSizeDetail);
223+
224+
Mockito.when(vm.getId()).thenReturn(1L);
225+
Mockito.when(vm.getServiceOfferingId()).thenReturn(1L);
226+
Mockito.when(serviceOfferingDao.findById(1L)).thenReturn(serviceOffering);
227+
Mockito.when(userVmDetailsDao.listDetails(1L)).thenReturn(vmDetails);
228+
229+
Ternary<Integer, Integer, Integer> cpuSpeedAndRamSize = manager.getComputeResourcesCpuSpeedAndRamSize(vm);
230+
231+
Assert.assertEquals(2, cpuSpeedAndRamSize.first().intValue());
232+
Assert.assertEquals(1000, cpuSpeedAndRamSize.second().intValue());
233+
Assert.assertEquals(1024, cpuSpeedAndRamSize.third().intValue());
234+
}
175235
}

0 commit comments

Comments
 (0)