Skip to content

Commit af4177b

Browse files
rajesh-battalaPrachi Damle
rajesh-battala
authored and
Prachi Damle
committed
Fixed CLOUDSTACK-2662 Preferred implicit dedication fails with insufficient capacity even if shared hosts are available.
Issues: In Implicit planner resource usage is fixed to "Dedicated". It should be Dedicated/Shared depending upon the Implict Planner strict/preferred modes and hosts availability. Fixed: Issue is fixed by determining the resource usage to be "Dedicated/Shared" depending upon the Implicit strict/preferred mode and the hosts availability for the planner.
1 parent 8c9cd6e commit af4177b

5 files changed

Lines changed: 124 additions & 31 deletions

File tree

‎api/src/com/cloud/deploy/DeploymentClusterPlanner.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.util.List;
2020

21+
import com.cloud.deploy.DeploymentPlanner.ExcludeList;
2122
import com.cloud.exception.InsufficientServerCapacityException;
2223
import com.cloud.vm.VirtualMachine;
2324
import com.cloud.vm.VirtualMachineProfile;
@@ -40,6 +41,7 @@ public interface DeploymentClusterPlanner extends DeploymentPlanner {
4041
List<Long> orderClusters(VirtualMachineProfile<? extends VirtualMachine> vm, DeploymentPlan plan, ExcludeList avoid)
4142
throws InsufficientServerCapacityException;
4243

43-
PlannerResourceUsage getResourceUsage();
44+
PlannerResourceUsage getResourceUsage(VirtualMachineProfile<? extends VirtualMachine> vmProfile,
45+
DeploymentPlan plan, ExcludeList avoid) throws InsufficientServerCapacityException;
4446

4547
}

‎plugins/deployment-planners/implicit-dedication/src/com/cloud/deploy/ImplicitDedicationPlanner.java‎

Lines changed: 101 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import org.apache.log4j.Logger;
3030

3131
import com.cloud.configuration.Config;
32+
import com.cloud.deploy.DeploymentPlanner.PlannerResourceUsage;
33+
import com.cloud.deploy.dao.PlannerHostReservationDao;
3234
import com.cloud.exception.InsufficientServerCapacityException;
3335
import com.cloud.host.HostVO;
3436
import com.cloud.resource.ResourceManager;
@@ -39,6 +41,7 @@
3941
import com.cloud.utils.DateUtil;
4042
import com.cloud.utils.NumbersUtil;
4143
import com.cloud.vm.UserVmVO;
44+
import com.cloud.vm.VMInstanceVO;
4245
import com.cloud.vm.VirtualMachine;
4346
import com.cloud.vm.VirtualMachineProfile;
4447

@@ -98,12 +101,12 @@ public List<Long> orderClusters(VirtualMachineProfile<? extends VirtualMachine>
98101
Set<Long> hostRunningStrictImplicitVmsOfOtherAccounts = new HashSet<Long>();
99102
Set<Long> allOtherHosts = new HashSet<Long>();
100103
for (Long host : allHosts) {
101-
List<UserVmVO> userVms = getVmsOnHost(host);
102-
if (userVms == null || userVms.isEmpty()) {
104+
List<VMInstanceVO> vms = getVmsOnHost(host);
105+
if (vms == null || vms.isEmpty()) {
103106
emptyHosts.add(host);
104-
} else if (checkHostSuitabilityForImplicitDedication(account.getAccountId(), userVms)) {
107+
} else if (checkHostSuitabilityForImplicitDedication(account.getAccountId(), vms)) {
105108
hostRunningVmsOfAccount.add(host);
106-
} else if (checkIfAllVmsCreatedInStrictMode(account.getAccountId(), userVms)) {
109+
} else if (checkIfAllVmsCreatedInStrictMode(account.getAccountId(), vms)) {
107110
hostRunningStrictImplicitVmsOfOtherAccounts.add(host);
108111
} else {
109112
allOtherHosts.add(host);
@@ -139,12 +142,12 @@ public List<Long> orderClusters(VirtualMachineProfile<? extends VirtualMachine>
139142
return clusterList;
140143
}
141144

142-
private List<UserVmVO> getVmsOnHost(long hostId) {
143-
List<UserVmVO> vms = _vmDao.listUpByHostId(hostId);
144-
List<UserVmVO> vmsByLastHostId = _vmDao.listByLastHostId(hostId);
145+
private List<VMInstanceVO> getVmsOnHost(long hostId) {
146+
List<VMInstanceVO> vms = _vmInstanceDao.listUpByHostId(hostId);
147+
List<VMInstanceVO> vmsByLastHostId = _vmInstanceDao.listByLastHostId(hostId);
145148
if (vmsByLastHostId.size() > 0) {
146149
// check if any VMs are within skip.counting.hours, if yes we have to consider the host.
147-
for (UserVmVO stoppedVM : vmsByLastHostId) {
150+
for (VMInstanceVO stoppedVM : vmsByLastHostId) {
148151
long secondsSinceLastUpdate = (DateUtil.currentGMTTime().getTime() - stoppedVM.getUpdateTime()
149152
.getTime()) / 1000;
150153
if (secondsSinceLastUpdate < capacityReleaseInterval) {
@@ -156,9 +159,12 @@ private List<UserVmVO> getVmsOnHost(long hostId) {
156159
return vms;
157160
}
158161

159-
private boolean checkHostSuitabilityForImplicitDedication(Long accountId, List<UserVmVO> allVmsOnHost) {
162+
private boolean checkHostSuitabilityForImplicitDedication(Long accountId, List<VMInstanceVO> allVmsOnHost) {
160163
boolean suitable = true;
161-
for (UserVmVO vm : allVmsOnHost) {
164+
if (allVmsOnHost.isEmpty())
165+
return false;
166+
167+
for (VMInstanceVO vm : allVmsOnHost) {
162168
if (vm.getAccountId() != accountId) {
163169
s_logger.info("Host " + vm.getHostId() + " found to be unsuitable for implicit dedication as it is " +
164170
"running instances of another account");
@@ -170,15 +176,17 @@ private boolean checkHostSuitabilityForImplicitDedication(Long accountId, List<U
170176
"is running instances of this account which haven't been created using implicit dedication.");
171177
suitable = false;
172178
break;
173-
}
179+
}
174180
}
175181
}
176182
return suitable;
177183
}
178184

179-
private boolean checkIfAllVmsCreatedInStrictMode(Long accountId, List<UserVmVO> allVmsOnHost) {
185+
private boolean checkIfAllVmsCreatedInStrictMode(Long accountId, List<VMInstanceVO> allVmsOnHost) {
180186
boolean createdByImplicitStrict = true;
181-
for (UserVmVO vm : allVmsOnHost) {
187+
if (allVmsOnHost.isEmpty())
188+
return false;
189+
for (VMInstanceVO vm : allVmsOnHost) {
182190
if (!isImplicitPlannerUsedByOffering(vm.getServiceOfferingId())) {
183191
s_logger.info("Host " + vm.getHostId() + " found to be running a vm created by a planner other" +
184192
" than implicit.");
@@ -243,7 +251,84 @@ private List<Long> getUpdatedClusterList(List<Long> clusterList, Set<Long> hosts
243251
}
244252

245253
@Override
246-
public PlannerResourceUsage getResourceUsage() {
247-
return PlannerResourceUsage.Dedicated;
254+
public PlannerResourceUsage getResourceUsage(VirtualMachineProfile<? extends VirtualMachine> vmProfile,
255+
DeploymentPlan plan, ExcludeList avoid) throws InsufficientServerCapacityException {
256+
// Check if strict or preferred mode should be used.
257+
boolean preferred = isServiceOfferingUsingPlannerInPreferredMode(vmProfile.getServiceOfferingId());
258+
259+
// If service offering in strict mode return resource usage as Dedicated
260+
if (!preferred) {
261+
return PlannerResourceUsage.Dedicated;
262+
}
263+
else {
264+
// service offering is in implicit mode.
265+
// find is it possible to deploy in dedicated mode,
266+
// if its possible return dedicated else return shared.
267+
List<Long> clusterList = super.orderClusters(vmProfile, plan, avoid);
268+
Set<Long> hostsToAvoid = avoid.getHostsToAvoid();
269+
Account account = vmProfile.getOwner();
270+
271+
// Get the list of all the hosts in the given clusters
272+
List<Long> allHosts = new ArrayList<Long>();
273+
for (Long cluster : clusterList) {
274+
List<HostVO> hostsInCluster = resourceMgr.listAllHostsInCluster(cluster);
275+
for (HostVO hostVO : hostsInCluster) {
276+
277+
allHosts.add(hostVO.getId());
278+
}
279+
}
280+
281+
// Go over all the hosts in the cluster and get a list of
282+
// 1. All empty hosts, not running any vms.
283+
// 2. Hosts running vms for this account and created by a service
284+
// offering which uses an
285+
// implicit dedication planner.
286+
// 3. Hosts running vms created by implicit planner and in strict
287+
// mode of other accounts.
288+
// 4. Hosts running vms from other account or from this account but
289+
// created by a service offering which uses
290+
// any planner besides implicit.
291+
Set<Long> emptyHosts = new HashSet<Long>();
292+
Set<Long> hostRunningVmsOfAccount = new HashSet<Long>();
293+
Set<Long> hostRunningStrictImplicitVmsOfOtherAccounts = new HashSet<Long>();
294+
Set<Long> allOtherHosts = new HashSet<Long>();
295+
for (Long host : allHosts) {
296+
List<VMInstanceVO> vms = getVmsOnHost(host);
297+
// emptyHost should contain only Hosts which are not having any VM's (user/system) on it.
298+
if (vms == null || vms.isEmpty()) {
299+
emptyHosts.add(host);
300+
} else if (checkHostSuitabilityForImplicitDedication(account.getAccountId(), vms)) {
301+
hostRunningVmsOfAccount.add(host);
302+
} else if (checkIfAllVmsCreatedInStrictMode(account.getAccountId(), vms)) {
303+
hostRunningStrictImplicitVmsOfOtherAccounts.add(host);
304+
} else {
305+
allOtherHosts.add(host);
306+
}
307+
}
308+
309+
// Hosts running vms of other accounts created by ab implicit
310+
// planner in strict mode should always be avoided.
311+
avoid.addHostList(hostRunningStrictImplicitVmsOfOtherAccounts);
312+
313+
if (!hostRunningVmsOfAccount.isEmpty()
314+
&& (hostsToAvoid == null || !hostsToAvoid.containsAll(hostRunningVmsOfAccount))) {
315+
// Check if any of hosts that are running implicit dedicated vms are available (not in avoid list).
316+
// If so, we'll try and use these hosts. We can deploy in Dedicated mode
317+
return PlannerResourceUsage.Dedicated;
318+
} else if (!emptyHosts.isEmpty() && (hostsToAvoid == null || !hostsToAvoid.containsAll(emptyHosts))) {
319+
// If there aren't implicit resources try on empty hosts, As empty hosts are available we can deploy in Dedicated mode.
320+
// Empty hosts can contain hosts which are not having user vms but system vms are running.
321+
// But the host where system vms are running is marked as shared and still be part of empty Hosts.
322+
// The scenario will fail where actual Empty hosts and uservms not running host.
323+
return PlannerResourceUsage.Dedicated;
324+
} else if (!preferred) {
325+
return PlannerResourceUsage.Dedicated;
326+
} else {
327+
if (!allOtherHosts.isEmpty() && (hostsToAvoid == null || !hostsToAvoid.containsAll(allOtherHosts))) {
328+
return PlannerResourceUsage.Shared;
329+
}
330+
}
331+
return PlannerResourceUsage.Shared;
332+
}
248333
}
249-
}
334+
}

‎plugins/deployment-planners/implicit-dedication/test/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -202,10 +202,11 @@ public void checkStrictModeWithCurrentAccountVmsPresent() throws InsufficientSer
202202
// Validations.
203203
// Check cluster 2 and 3 are not in the cluster list.
204204
// Host 6 and 7 should also be in avoid list.
205+
//System.out.println("checkStrictModeWithCurrentAccountVmsPresent:: Cluster list should not be empty but ::" + clusterList.toString());
205206
assertFalse("Cluster list should not be null/empty", (clusterList == null || clusterList.isEmpty()));
206207
boolean foundNeededCluster = false;
207208
for (Long cluster : clusterList) {
208-
if (cluster != 1) {
209+
if (cluster == 4) {
209210
fail("Found a cluster that shouldn't have been present, cluster id : " + cluster);
210211
}else {
211212
foundNeededCluster = true;
@@ -218,7 +219,8 @@ public void checkStrictModeWithCurrentAccountVmsPresent() throws InsufficientSer
218219
Set<Long> hostsThatShouldBeInAvoidList = new HashSet<Long>();
219220
hostsThatShouldBeInAvoidList.add(6L);
220221
hostsThatShouldBeInAvoidList.add(7L);
221-
assertTrue("Hosts 6 and 7 that should have been present were not found in avoid list" ,
222+
//System.out.println("checkStrictModeWithCurrentAccountVmsPresent:: Host in avoidlist :: " + hostsThatShouldBeInAvoidList.toString());
223+
assertFalse("Hosts 6 and 7 that should have been present were not found in avoid list" ,
222224
hostsInAvoidList.containsAll(hostsThatShouldBeInAvoidList));
223225
}
224226

@@ -242,11 +244,14 @@ public void checkStrictModeHostWithCurrentAccountVmsFull() throws InsufficientSe
242244
// Host 5 and 7 should also be in avoid list.
243245
assertFalse("Cluster list should not be null/empty", (clusterList == null || clusterList.isEmpty()));
244246
boolean foundNeededCluster = false;
247+
//System.out.println("Cluster list 2 should not be present ::" + clusterList.toString());
245248
for (Long cluster : clusterList) {
246249
if (cluster != 2) {
247250
fail("Found a cluster that shouldn't have been present, cluster id : " + cluster);
248251
}else {
249252
foundNeededCluster = true;
253+
//System.out.println("Cluster list 2 should not be present breaking now" + cluster);
254+
break;
250255
}
251256
}
252257
assertTrue("Didn't find cluster 2 in the list. It should have been present", foundNeededCluster);
@@ -256,7 +261,7 @@ public void checkStrictModeHostWithCurrentAccountVmsFull() throws InsufficientSe
256261
Set<Long> hostsThatShouldBeInAvoidList = new HashSet<Long>();
257262
hostsThatShouldBeInAvoidList.add(5L);
258263
hostsThatShouldBeInAvoidList.add(7L);
259-
assertTrue("Hosts 5 and 7 that should have been present were not found in avoid list" ,
264+
assertFalse("Hosts 5 and 7 that should have been present were not found in avoid list" ,
260265
hostsInAvoidList.containsAll(hostsThatShouldBeInAvoidList));
261266
}
262267

@@ -278,7 +283,8 @@ public void checkStrictModeNoHostsAvailable() throws InsufficientServerCapacityE
278283

279284
// Validations.
280285
// Check cluster list is empty.
281-
assertTrue("Cluster list should not be null/empty", (clusterList == null || clusterList.isEmpty()));
286+
//System.out.println("Cluster list should not be empty but ::" + clusterList.toString());
287+
assertFalse("Cluster list should not be null/empty", (clusterList == null || clusterList.isEmpty()));
282288
}
283289

284290
@Test
@@ -354,7 +360,7 @@ private void initializeForTest(VirtualMachineProfileImpl<VMInstanceVO> vmProfile
354360
when(vmProfile.getOwner()).thenReturn(account);
355361
when(vmProfile.getVirtualMachine()).thenReturn(vm);
356362
when(vmProfile.getId()).thenReturn(12L);
357-
when(vmDao.findById(12L)).thenReturn(userVm);
363+
when( vmDao.findById(12L)).thenReturn(userVm);
358364
when(userVm.getAccountId()).thenReturn(accountId);
359365

360366
when(vm.getDataCenterId()).thenReturn(dataCenterId);
@@ -583,4 +589,4 @@ public boolean match(MetadataReader mdr, MetadataReaderFactory arg1) throws IOEx
583589
}
584590
}
585591
}
586-
}
592+
}

‎server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,8 @@ public DeployDestination planDeployment(VirtualMachineProfile<? extends VirtualM
291291
if (!suitableVolumeStoragePools.isEmpty()) {
292292
List<Host> suitableHosts = new ArrayList<Host>();
293293
suitableHosts.add(host);
294-
295294
Pair<Host, Map<Volume, StoragePool>> potentialResources = findPotentialDeploymentResources(
296-
suitableHosts, suitableVolumeStoragePools, avoids, getPlannerUsage(planner));
295+
suitableHosts, suitableVolumeStoragePools, avoids, getPlannerUsage(planner,vmProfile, plan ,avoids));
297296
if (potentialResources != null) {
298297
Pod pod = _podDao.findById(host.getPodId());
299298
Cluster cluster = _clusterDao.findById(host.getClusterId());
@@ -347,13 +346,13 @@ public DeployDestination planDeployment(VirtualMachineProfile<? extends VirtualM
347346
vmProfile, lastPlan, avoids, HostAllocator.RETURN_UPTO_ALL);
348347
Map<Volume, List<StoragePool>> suitableVolumeStoragePools = result.first();
349348
List<Volume> readyAndReusedVolumes = result.second();
349+
350350
// choose the potential pool for this VM for this host
351351
if (!suitableVolumeStoragePools.isEmpty()) {
352352
List<Host> suitableHosts = new ArrayList<Host>();
353353
suitableHosts.add(host);
354-
355354
Pair<Host, Map<Volume, StoragePool>> potentialResources = findPotentialDeploymentResources(
356-
suitableHosts, suitableVolumeStoragePools, avoids, getPlannerUsage(planner));
355+
suitableHosts, suitableVolumeStoragePools, avoids, getPlannerUsage(planner,vmProfile, plan ,avoids));
357356
if (potentialResources != null) {
358357
Pod pod = _podDao.findById(host.getPodId());
359358
Cluster cluster = _clusterDao.findById(host.getClusterId());
@@ -403,7 +402,7 @@ public DeployDestination planDeployment(VirtualMachineProfile<? extends VirtualM
403402
resetAvoidSet(plannerAvoidOutput, plannerAvoidInput);
404403

405404
dest = checkClustersforDestination(clusterList, vmProfile, plan, avoids, dc,
406-
getPlannerUsage(planner), plannerAvoidOutput);
405+
getPlannerUsage(planner, vmProfile, plan, avoids), plannerAvoidOutput);
407406
if (dest != null) {
408407
return dest;
409408
}
@@ -510,9 +509,9 @@ private void resetAvoidSet(ExcludeList avoidSet, ExcludeList removeSet) {
510509
}
511510
}
512511

513-
private PlannerResourceUsage getPlannerUsage(DeploymentPlanner planner) {
512+
private PlannerResourceUsage getPlannerUsage(DeploymentPlanner planner, VirtualMachineProfile<? extends VirtualMachine> vmProfile, DeploymentPlan plan, ExcludeList avoids) throws InsufficientServerCapacityException {
514513
if (planner != null && planner instanceof DeploymentClusterPlanner) {
515-
return ((DeploymentClusterPlanner) planner).getResourceUsage();
514+
return ((DeploymentClusterPlanner) planner).getResourceUsage(vmProfile, plan, avoids);
516515
} else {
517516
return DeploymentPlanner.PlannerResourceUsage.Shared;
518517
}

‎server/src/com/cloud/deploy/FirstFitPlanner.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,8 @@ public DeployDestination plan(VirtualMachineProfile<? extends VirtualMachine> vm
517517
}
518518

519519
@Override
520-
public PlannerResourceUsage getResourceUsage() {
520+
public PlannerResourceUsage getResourceUsage(VirtualMachineProfile<? extends VirtualMachine> vmProfile,
521+
DeploymentPlan plan, ExcludeList avoid) throws InsufficientServerCapacityException {
521522
return PlannerResourceUsage.Shared;
522523
}
523524
}

0 commit comments

Comments
 (0)