Skip to content

Commit 3626c51

Browse files
shwstpprshwstppr
authored
server: fix listing resource limits (#12188)
1 parent d700e2d commit 3626c51

File tree

2 files changed

+95
-44
lines changed

2 files changed

+95
-44
lines changed

‎server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java‎

Lines changed: 38 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.ArrayList;
2222
import java.util.Arrays;
2323
import java.util.Date;
24+
import java.util.EnumSet;
2425
import java.util.HashMap;
2526
import java.util.HashSet;
2627
import java.util.Iterator;
@@ -36,9 +37,6 @@
3637
import javax.inject.Inject;
3738
import javax.naming.ConfigurationException;
3839

39-
import com.cloud.event.ActionEventUtils;
40-
import com.cloud.event.EventTypes;
41-
import com.cloud.utils.Ternary;
4240
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
4341
import org.apache.cloudstack.api.ApiCommandResourceType;
4442
import org.apache.cloudstack.api.response.AccountResponse;
@@ -86,6 +84,8 @@
8684
import com.cloud.domain.Domain;
8785
import com.cloud.domain.DomainVO;
8886
import com.cloud.domain.dao.DomainDao;
87+
import com.cloud.event.ActionEventUtils;
88+
import com.cloud.event.EventTypes;
8989
import com.cloud.exception.InvalidParameterValueException;
9090
import com.cloud.exception.PermissionDeniedException;
9191
import com.cloud.exception.ResourceAllocationException;
@@ -118,6 +118,7 @@
118118
import com.cloud.user.ResourceLimitService;
119119
import com.cloud.user.dao.AccountDao;
120120
import com.cloud.utils.Pair;
121+
import com.cloud.utils.Ternary;
121122
import com.cloud.utils.component.ManagerBase;
122123
import com.cloud.utils.concurrency.NamedThreadFactory;
123124
import com.cloud.utils.db.DB;
@@ -804,42 +805,43 @@ public List<ResourceLimitVO> searchForLimits(Long id, Long accountId, Long domai
804805
limits.addAll(foundLimits);
805806
}
806807
} else {
807-
limits.addAll(foundLimits);
808-
809-
// see if any limits are missing from the table, and if yes - get it from the config table and add
810-
ResourceType[] resourceTypes = ResourceCount.ResourceType.values();
811-
if (foundLimits.size() != resourceTypes.length) {
812-
List<String> accountLimitStr = new ArrayList<>();
813-
List<String> domainLimitStr = new ArrayList<>();
814-
for (ResourceLimitVO foundLimit : foundLimits) {
815-
if (foundLimit.getAccountId() != null) {
816-
accountLimitStr.add(foundLimit.getType().toString());
817-
} else {
818-
domainLimitStr.add(foundLimit.getType().toString());
819-
}
820-
}
808+
limits = getConsolidatedResourceLimitsForAllResourceTypes(accountId, domainId, foundLimits, isAccount);
809+
}
810+
addTaggedResourceLimits(limits, resourceType, isAccount ? ResourceOwnerType.Account : ResourceOwnerType.Domain, isAccount ? accountId : domainId, hostTags, storageTags);
811+
return limits;
812+
}
821813

822-
// get default from config values
823-
if (isAccount) {
824-
if (accountLimitStr.size() < resourceTypes.length) {
825-
for (ResourceType rt : resourceTypes) {
826-
if (!accountLimitStr.contains(rt.toString())) {
827-
limits.add(new ResourceLimitVO(rt, findCorrectResourceLimitForAccount(_accountMgr.getAccount(accountId), rt, null), accountId, ResourceOwnerType.Account));
828-
}
829-
}
830-
}
831-
} else {
832-
if (domainLimitStr.size() < resourceTypes.length) {
833-
for (ResourceType rt : resourceTypes) {
834-
if (!domainLimitStr.contains(rt.toString())) {
835-
limits.add(new ResourceLimitVO(rt, findCorrectResourceLimitForDomain(_domainDao.findById(domainId), rt, null), domainId, ResourceOwnerType.Domain));
836-
}
837-
}
838-
}
839-
}
814+
protected List<ResourceLimitVO> getConsolidatedResourceLimitsForAllResourceTypes(Long accountId, Long domainId,
815+
List<ResourceLimitVO> foundLimits, boolean isAccount) {
816+
List<ResourceLimitVO> limits = new ArrayList<>(foundLimits);
817+
818+
Set<ResourceType> allResourceTypes = EnumSet.allOf(ResourceType.class);
819+
Set<ResourceType> foundUntaggedTypes = foundLimits.stream()
820+
.filter(l -> StringUtils.isEmpty(l.getTag()))
821+
.map(ResourceLimitVO::getType)
822+
.collect(Collectors.toSet());
823+
824+
if (foundUntaggedTypes.containsAll(allResourceTypes)) {
825+
return limits;
826+
}
827+
828+
ResourceOwnerType ownerType = isAccount ? ResourceOwnerType.Account : ResourceOwnerType.Domain;
829+
long ownerId = isAccount ? accountId : domainId;
830+
831+
for (ResourceType rt : allResourceTypes) {
832+
if (foundUntaggedTypes.contains(rt)) {
833+
continue;
840834
}
835+
long max;
836+
if (isAccount) {
837+
Account acct = _accountMgr.getAccount(accountId);
838+
max = findCorrectResourceLimitForAccount(acct, rt, null);
839+
} else {
840+
DomainVO dom = _domainDao.findById(domainId);
841+
max = findCorrectResourceLimitForDomain(dom, rt, null);
842+
}
843+
limits.add(new ResourceLimitVO(rt, max, ownerId, ownerType));
841844
}
842-
addTaggedResourceLimits(limits, resourceType, isAccount ? ResourceOwnerType.Account : ResourceOwnerType.Domain, isAccount ? accountId : domainId, hostTags, storageTags);
843845
return limits;
844846
}
845847

‎server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java‎

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,17 @@
1616
// under the License.
1717
package com.cloud.resourcelimit;
1818

19+
import static org.mockito.Mockito.mock;
20+
import static org.mockito.Mockito.when;
21+
1922
import java.lang.reflect.Field;
2023
import java.util.ArrayList;
2124
import java.util.Arrays;
25+
import java.util.EnumSet;
2226
import java.util.HashMap;
2327
import java.util.List;
2428
import java.util.Map;
2529

26-
import com.cloud.event.ActionEventUtils;
27-
import com.cloud.event.EventTypes;
28-
import com.cloud.utils.db.EntityManager;
29-
3030
import org.apache.cloudstack.api.ApiCommandResourceType;
3131
import org.apache.cloudstack.api.response.AccountResponse;
3232
import org.apache.cloudstack.api.response.DomainResponse;
@@ -62,6 +62,8 @@
6262
import com.cloud.domain.Domain;
6363
import com.cloud.domain.DomainVO;
6464
import com.cloud.domain.dao.DomainDao;
65+
import com.cloud.event.ActionEventUtils;
66+
import com.cloud.event.EventTypes;
6567
import com.cloud.exception.ResourceAllocationException;
6668
import com.cloud.offering.DiskOffering;
6769
import com.cloud.offering.ServiceOffering;
@@ -75,21 +77,19 @@
7577
import com.cloud.storage.dao.VolumeDao;
7678
import com.cloud.template.VirtualMachineTemplate;
7779
import com.cloud.user.Account;
78-
import com.cloud.user.User;
7980
import com.cloud.user.AccountManager;
8081
import com.cloud.user.AccountVO;
8182
import com.cloud.user.ResourceLimitService;
83+
import com.cloud.user.User;
8284
import com.cloud.user.dao.AccountDao;
8385
import com.cloud.utils.Pair;
86+
import com.cloud.utils.db.EntityManager;
8487
import com.cloud.vm.VirtualMachine;
8588
import com.cloud.vm.VirtualMachineManager;
8689
import com.cloud.vm.dao.UserVmDao;
8790
import com.cloud.vm.dao.VMInstanceDao;
8891
import com.cloud.vpc.MockResourceLimitManagerImpl;
8992

90-
import static org.mockito.Mockito.mock;
91-
import static org.mockito.Mockito.when;
92-
9393
@RunWith(MockitoJUnitRunner.class)
9494
public class ResourceLimitManagerImplTest {
9595
private Logger logger = LogManager.getLogger(ResourceLimitManagerImplTest.class);
@@ -1397,4 +1397,53 @@ public void testUpdateResourceLimitForDomain() {
13971397
domainId, ApiCommandResourceType.Domain.toString()));
13981398
}
13991399
}
1400+
1401+
@Test
1402+
public void consolidatedResourceLimitsForAllResourceTypesWithAccountId() {
1403+
Long accountId = 1L;
1404+
Long domainId = null;
1405+
List<ResourceLimitVO> foundLimits = new ArrayList<>();
1406+
ResourceLimitVO limit = new ResourceLimitVO(Resource.ResourceType.cpu, 10L, accountId, Resource.ResourceOwnerType.Account);
1407+
foundLimits.add(limit);
1408+
1409+
Mockito.when(accountManager.getAccount(accountId)).thenReturn(Mockito.mock(Account.class));
1410+
Mockito.doReturn(20L).when(resourceLimitManager).findCorrectResourceLimitForAccount(Mockito.any(Account.class), Mockito.any(Resource.ResourceType.class), Mockito.isNull());
1411+
1412+
List<ResourceLimitVO> result = resourceLimitManager.getConsolidatedResourceLimitsForAllResourceTypes(accountId, domainId, foundLimits, true);
1413+
1414+
Assert.assertEquals(EnumSet.allOf(Resource.ResourceType.class).size(), result.size());
1415+
Assert.assertTrue(result.contains(limit));
1416+
}
1417+
1418+
@Test
1419+
public void consolidatedResourceLimitsForAllResourceTypesWithDomainId() {
1420+
Long accountId = null;
1421+
Long domainId = 1L;
1422+
List<ResourceLimitVO> foundLimits = new ArrayList<>();
1423+
ResourceLimitVO limit = new ResourceLimitVO(Resource.ResourceType.memory, 15L, domainId, Resource.ResourceOwnerType.Domain);
1424+
foundLimits.add(limit);
1425+
1426+
Mockito.when(domainDao.findById(domainId)).thenReturn(Mockito.mock(DomainVO.class));
1427+
Mockito.doReturn(30L).when(resourceLimitManager).findCorrectResourceLimitForDomain(Mockito.any(Domain.class), Mockito.any(Resource.ResourceType.class), Mockito.isNull());
1428+
1429+
List<ResourceLimitVO> result = resourceLimitManager.getConsolidatedResourceLimitsForAllResourceTypes(accountId, domainId, foundLimits, false);
1430+
1431+
Assert.assertEquals(EnumSet.allOf(Resource.ResourceType.class).size(), result.size());
1432+
Assert.assertTrue(result.contains(limit));
1433+
}
1434+
1435+
@Test
1436+
public void consolidatedResourceLimitsForAllResourceTypesWithEmptyFoundLimits() {
1437+
Long accountId = 1L;
1438+
Long domainId = null;
1439+
List<ResourceLimitVO> foundLimits = new ArrayList<>();
1440+
1441+
Mockito.when(accountManager.getAccount(accountId)).thenReturn(Mockito.mock(Account.class));
1442+
Mockito.doReturn(25L).when(resourceLimitManager).findCorrectResourceLimitForAccount(Mockito.any(Account.class), Mockito.any(Resource.ResourceType.class), Mockito.isNull());
1443+
1444+
List<ResourceLimitVO> result = resourceLimitManager.getConsolidatedResourceLimitsForAllResourceTypes(accountId, domainId, foundLimits, true);
1445+
1446+
Assert.assertEquals(EnumSet.allOf(Resource.ResourceType.class).size(), result.size());
1447+
Assert.assertEquals(25L, result.get(0).getMax().longValue());
1448+
}
14001449
}

0 commit comments

Comments
 (0)