Skip to content

Commit d8d1048

Browse files
sureshanapartisureshanaparti
committed
review comments & tests
1 parent 9bbb37c commit d8d1048

File tree

2 files changed

+92
-23
lines changed

2 files changed

+92
-23
lines changed

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

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
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;
4041
import org.apache.log4j.Logger;
4142

4243
import com.cloud.agent.AgentManager;
@@ -627,33 +628,16 @@ private Pair<Boolean, String> performCapacityChecksBeforeHostInMaintenance(Host
627628
int successfullyCheckedVmMigrations = 0;
628629
for (VMInstanceVO runningVM : vmsRunning) {
629630
boolean canMigrateVm = false;
630-
ServiceOfferingVO serviceOffering = serviceOfferingDao.findById(runningVM.getServiceOfferingId());
631-
Integer cpu = serviceOffering.getCpu();
632-
Integer speed = serviceOffering.getSpeed();
633-
Integer ramSize = serviceOffering.getRamSize();
634-
if (serviceOffering.isDynamic()) {
635-
List<UserVmDetailVO> vmDetails = userVmDetailsDao.listDetails(runningVM.getId());
636-
if (CollectionUtils.isNotEmpty(vmDetails)) {
637-
for (UserVmDetailVO vmDetail : vmDetails) {
638-
if (vmDetail.getName() != null &&vmDetail.getValue() != null) {
639-
if (cpu == null && VmDetailConstants.CPU_NUMBER.equals(vmDetail.getName())) {
640-
cpu = Integer.valueOf(vmDetail.getValue());
641-
}
642-
if (speed == null && VmDetailConstants.CPU_SPEED.equals(vmDetail.getName())) {
643-
speed = Integer.valueOf(vmDetail.getValue());
644-
}
645-
if (ramSize == null && VmDetailConstants.MEMORY.equals(vmDetail.getName())) {
646-
ramSize = Integer.valueOf(vmDetail.getValue());
647-
}
648-
}
649-
}
650-
}
651-
}
652-
if (cpu == null || speed == null || ramSize == null) {
631+
Ternary<Integer, Integer, Integer> cpuSpeedAndRamSize = getComputeResourcesCpuSpeedAndRamSize(runningVM);
632+
Integer cpu = cpuSpeedAndRamSize.first();
633+
Integer speed = cpuSpeedAndRamSize.second();
634+
Integer ramSize = cpuSpeedAndRamSize.third();
635+
if (ObjectUtils.anyNull(cpu, speed,ramSize)) {
653636
s_logger.warn(String.format("Cannot fetch compute resources for the VM %s, skipping it from the capacity check", runningVM));
654637
continue;
655638
}
656639

640+
ServiceOfferingVO serviceOffering = serviceOfferingDao.findById(runningVM.getServiceOfferingId());
657641
for (Host hostInCluster : hostsInCluster) {
658642
if (!checkHostTags(hostTags, hostTagsDao.getHostTags(hostInCluster.getId()), serviceOffering.getHostTag())) {
659643
s_logger.warn(String.format("Host tags mismatch between %s and %s, skipping it from the capacity check", host, hostInCluster));
@@ -698,6 +682,31 @@ private Pair<Boolean, String> performCapacityChecksBeforeHostInMaintenance(Host
698682
return new Pair<>(true, "OK");
699683
}
700684

685+
protected Ternary<Integer, Integer, Integer> getComputeResourcesCpuSpeedAndRamSize(VMInstanceVO runningVM) {
686+
ServiceOfferingVO serviceOffering = serviceOfferingDao.findById(runningVM.getServiceOfferingId());
687+
Integer cpu = serviceOffering.getCpu();
688+
Integer speed = serviceOffering.getSpeed();
689+
Integer ramSize = serviceOffering.getRamSize();
690+
if (serviceOffering.isDynamic()) {
691+
List<UserVmDetailVO> vmDetails = userVmDetailsDao.listDetails(runningVM.getId());
692+
if (CollectionUtils.isNotEmpty(vmDetails)) {
693+
for (UserVmDetailVO vmDetail : vmDetails) {
694+
if (vmDetail.getName() != null && vmDetail.getValue() != null) {
695+
if (cpu == null && VmDetailConstants.CPU_NUMBER.equals(vmDetail.getName())) {
696+
cpu = Integer.valueOf(vmDetail.getValue());
697+
} else if (speed == null && VmDetailConstants.CPU_SPEED.equals(vmDetail.getName())) {
698+
speed = Integer.valueOf(vmDetail.getValue());
699+
} else if (ramSize == null && VmDetailConstants.MEMORY.equals(vmDetail.getName())) {
700+
ramSize = Integer.valueOf(vmDetail.getValue());
701+
}
702+
}
703+
}
704+
}
705+
}
706+
707+
return new Ternary<>(cpu, speed, ramSize);
708+
}
709+
701710
/**
702711
* Check hosts tags
703712
*/

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,15 @@
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;
30+
import com.cloud.vm.UserVmDetailVO;
31+
import com.cloud.vm.VMInstanceVO;
32+
import com.cloud.vm.VmDetailConstants;
33+
import com.cloud.vm.dao.UserVmDetailsDao;
34+
2735
import org.junit.Assert;
2836
import org.junit.Before;
2937
import org.junit.Test;
@@ -53,6 +61,12 @@ public class RollingMaintenanceManagerImplTest {
5361
HostVO host4;
5462
@Mock
5563
Cluster cluster;
64+
@Mock
65+
VMInstanceVO vm;
66+
@Mock
67+
ServiceOfferingDao serviceOfferingDao;
68+
@Mock
69+
UserVmDetailsDao userVmDetailsDao;
5670

5771
@Spy
5872
@InjectMocks
@@ -164,4 +178,50 @@ public void testPerformStateChecksForce() {
164178

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

0 commit comments

Comments
 (0)