-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add limit configuration for number of projects #9172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
86a0aef
da479ec
86746b0
2afec67
874d13e
241520c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1365,6 +1365,14 @@ public enum Config { | |
| "200", | ||
| "The default maximum primary storage space (in GiB) that can be used for an account", | ||
| null), | ||
| DefaultMaxAccountProjects( | ||
| "Account Defaults", | ||
| ManagementServer.class, | ||
| Long.class, | ||
| "max.account.projects", | ||
| "10", | ||
| "The default maximum number of projects that can be created for an account", | ||
| null), | ||
|
|
||
| //disabling lb as cluster sync does not work with distributed cluster | ||
| SubDomainNetworkAccess( | ||
|
|
@@ -1414,6 +1422,7 @@ public enum Config { | |
| DefaultMaxDomainMemory("Domain Defaults", ManagementServer.class, Long.class, "max.domain.memory", "81920", "The default maximum memory (in MB) that can be used for a domain", null), | ||
| DefaultMaxDomainPrimaryStorage("Domain Defaults", ManagementServer.class, Long.class, "max.domain.primary.storage", "400", "The default maximum primary storage space (in GiB) that can be used for a domain", null), | ||
| DefaultMaxDomainSecondaryStorage("Domain Defaults", ManagementServer.class, Long.class, "max.domain.secondary.storage", "800", "The default maximum secondary storage space (in GiB) that can be used for a domain", null), | ||
| DefaultMaxDomainProjects("Domain Defaults",ManagementServer.class,Long.class,"max.domain.projects","50","The default maximum number of projects that can be created for a domain",null), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see above |
||
|
|
||
| DefaultMaxProjectUserVms( | ||
| "Project Defaults", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -190,6 +190,12 @@ protected void updateResourceLimit() { | |
|
|
||
| // update resource Limit for a domain for resource_type = 11 (Secondary storage (in GiB)) | ||
| resourceLimitServiceCall(null, (long)1, 10, (long)400); | ||
|
|
||
| // update resource Limit for an account for resource_type = 5 (Project) | ||
| resourceLimitServiceCall((long) 1, (long) 1, 5, (long) 50); | ||
|
|
||
| // update resource Limit for a domain for resource_type = 5 (Project) | ||
| resourceLimitServiceCall(null, (long) 1, 5, (long) 100); | ||
| } | ||
|
|
||
| private void resourceLimitServiceCall(Long accountId, Long domainId, Integer resourceType, Long max) { | ||
|
|
@@ -413,6 +419,36 @@ public void testFindCorrectResourceLimitForAccount() { | |
| Assert.assertEquals(defaultAccountCpuMax, result); | ||
| } | ||
|
|
||
| @Test | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this test has several calls into the code and asserts in between. Do you think it can be split in separate test cases? |
||
| public void testFindCorrectResourceLimitForAccountProjects() { | ||
| AccountVO account = Mockito.mock(AccountVO.class); | ||
| Mockito.when(account.getId()).thenReturn(1L); | ||
| Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(true); | ||
|
|
||
| long result = resourceLimitManager.findCorrectResourceLimitForAccount(account, | ||
| Resource.ResourceType.project, hostTags.get(0)); | ||
| Assert.assertEquals(Resource.RESOURCE_UNLIMITED, result); | ||
|
|
||
| Mockito.when(accountManager.isRootAdmin(1L)).thenReturn(false); | ||
| ResourceLimitVO limit = new ResourceLimitVO(); | ||
| limit.setMax(10L); | ||
| Mockito.when(resourceLimitDao.findByOwnerIdAndTypeAndTag(1L, Resource.ResourceOwnerType.Account, | ||
| Resource.ResourceType.project, hostTags.get(0))).thenReturn(limit); | ||
| result = resourceLimitManager.findCorrectResourceLimitForAccount(account, Resource.ResourceType.project, | ||
| hostTags.get(0)); | ||
| Assert.assertEquals(10L, result); | ||
|
|
||
| long defaultAccountProjectsMax = 15L; | ||
| Map<String, Long> accountResourceLimitMap = new HashMap<>(); | ||
| accountResourceLimitMap.put(Resource.ResourceType.project.name(), defaultAccountProjectsMax); | ||
| resourceLimitManager.accountResourceLimitMap = accountResourceLimitMap; | ||
| Mockito.when(resourceLimitDao.findByOwnerIdAndTypeAndTag(1L, Resource.ResourceOwnerType.Account, | ||
| Resource.ResourceType.project, hostTags.get(0))).thenReturn(null); | ||
| result = resourceLimitManager.findCorrectResourceLimitForAccount(account, Resource.ResourceType.project, | ||
| hostTags.get(0)); | ||
| Assert.assertEquals(defaultAccountProjectsMax, result); | ||
| } | ||
|
|
||
| @Test | ||
| public void testFindCorrectResourceLimitForAccountId1() { | ||
| // long accountId = 1L; | ||
|
|
@@ -472,6 +508,68 @@ public void testFindCorrectResourceLimitForDomain() { | |
| Assert.assertEquals(defaultDomainCpuMax, result); | ||
| } | ||
|
|
||
| @Test | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again, can this test case be split?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. alrighty, I was able to do the changes as you described. Please let me know if there is anything else I can do.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, this one looks good, not sure if the build failures are related. |
||
| public void testResourceUnlimitedForDomainProjects() { | ||
| DomainVO domain = Mockito.mock(DomainVO.class); | ||
| Mockito.when(domain.getId()).thenReturn(1L); | ||
|
|
||
| long result = resourceLimitManager.findCorrectResourceLimitForDomain(domain, Resource.ResourceType.project, | ||
| hostTags.get(0)); | ||
| Assert.assertEquals(Resource.RESOURCE_UNLIMITED, result); | ||
| } | ||
| @Test | ||
| public void testSpecificLimitForDomainProjects() { | ||
| DomainVO domain = Mockito.mock(DomainVO.class); | ||
| Mockito.when(domain.getId()).thenReturn(2L); | ||
|
|
||
| ResourceLimitVO limit = new ResourceLimitVO(); | ||
| limit.setMax(100L); | ||
| Mockito.when(resourceLimitDao.findByOwnerIdAndTypeAndTag(2L, Resource.ResourceOwnerType.Domain, Resource.ResourceType.project, hostTags.get(0))).thenReturn(limit); | ||
|
|
||
| long result = resourceLimitManager.findCorrectResourceLimitForDomain(domain, Resource.ResourceType.project, hostTags.get(0)); | ||
| Assert.assertEquals(100L, result); | ||
| } | ||
|
|
||
| @Test | ||
| public void testParentDomainLimitForDomainProjects() { | ||
| DomainVO domain = Mockito.mock(DomainVO.class); | ||
| Mockito.when(domain.getId()).thenReturn(3L); | ||
|
|
||
| DomainVO parentDomain = Mockito.mock(DomainVO.class); | ||
| Mockito.when(domain.getParent()).thenReturn(5L); | ||
| Mockito.when(domainDao.findById(5L)).thenReturn(parentDomain); | ||
|
|
||
| ResourceLimitVO limit = new ResourceLimitVO(); | ||
| limit.setMax(200L); | ||
| Mockito.when(resourceLimitDao.findByOwnerIdAndTypeAndTag(3L, Resource.ResourceOwnerType.Domain, | ||
| Resource.ResourceType.project, hostTags.get(0))).thenReturn(null); | ||
| Mockito.when(resourceLimitDao.findByOwnerIdAndTypeAndTag(5L, Resource.ResourceOwnerType.Domain, | ||
| Resource.ResourceType.project, hostTags.get(0))).thenReturn(limit); | ||
|
|
||
| long result = resourceLimitManager.findCorrectResourceLimitForDomain(domain, Resource.ResourceType.project, | ||
| hostTags.get(0)); | ||
| Assert.assertEquals(200L, result); | ||
| } | ||
|
|
||
| @Test | ||
| public void testDefaultDomainProjectLimit() { | ||
| DomainVO domain = Mockito.mock(DomainVO.class); | ||
| Mockito.when(domain.getId()).thenReturn(4L); | ||
| Mockito.when(domain.getParent()).thenReturn(null); | ||
|
|
||
| long defaultDomainProjectsMax = 250L; | ||
| Map<String, Long> domainResourceLimitMap = new HashMap<>(); | ||
| domainResourceLimitMap.put(Resource.ResourceType.project.name(), defaultDomainProjectsMax); | ||
| resourceLimitManager.domainResourceLimitMap = domainResourceLimitMap; | ||
|
|
||
| Mockito.when(resourceLimitDao.findByOwnerIdAndTypeAndTag(4L, Resource.ResourceOwnerType.Domain, | ||
| Resource.ResourceType.project, hostTags.get(0))).thenReturn(null); | ||
|
|
||
| long result = resourceLimitManager.findCorrectResourceLimitForDomain(domain, Resource.ResourceType.project, | ||
| hostTags.get(0)); | ||
| Assert.assertEquals(defaultDomainProjectsMax, result); | ||
| } | ||
|
|
||
| @Test | ||
| public void testCheckResourceLimitWithTag() { | ||
| AccountVO account = Mockito.mock(AccountVO.class); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding the config here is a bit of a deprecated way of doing these. Have a look at ConfigKey<> for the prefered way.
Define a ConfigKey<> in a manager implementation or a service interface and then add the configKey to the return of the
getConfirKeys()in the manager. You can then use the value more dynamically and it doesn't have to be preloaded in the config method.I understand you are just following the convention you see in the files you are changing but still,...