Add limit configuration for number of projects by my-code-AL · Pull Request #9172 · apache/cloudstack

my-code-AL · 2024-06-05T02:06:03Z

Description

The following PR creates a new feature of adding a limit to the number of projects of accounts within a singular domain. This helps administrators organize resources.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

The following issue has been tested with several mocked test cases that specify domain max projects and account max projects with valid and invalid inputs. Finally, the UI has been verified using a local linux environment; see above image in screenshots.

codecov · 2024-06-05T07:22:36Z

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 15.53%. Comparing base (4079906) to head (241520c).
Report is 1 commits behind head on main.

Files	Patch %	Lines
.../cloud/resourcelimit/ResourceLimitManagerImpl.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##               main    #9172     +/-   ##
===========================================
  Coverage     15.53%   15.53%             
+ Complexity    11968    11967      -1     
===========================================
  Files          5492     5492             
  Lines        480947   480953      +6     
  Branches      62483    59152   -3331     
===========================================
+ Hits          74726    74729      +3     
- Misses       397960   397963      +3     
  Partials       8261     8261

Flag	Coverage Δ
uitests	`4.16% <ø> (ø)`
unittests	`16.30% <66.66%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

DaanHoogland

clgtm needs testing. is there a issue related to this? I remember such a thing. If we find it let's link it.

DaanHoogland · 2024-06-05T07:16:30Z

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

        Assert.assertEquals(defaultAccountCpuMax, result);
    }

+    @Test


this test has several calls into the code and asserts in between. Do you think it can be split in separate test cases?

DaanHoogland · 2024-06-05T07:21:25Z

server/src/main/java/com/cloud/configuration/Config.java

            "200",
            "The default maximum primary storage space (in GiB) that can be used for an account",
            null),
+DefaultMaxAccountProjects(


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,...

DaanHoogland · 2024-06-05T07:21:35Z

server/src/main/java/com/cloud/configuration/Config.java

    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),


DaanHoogland · 2024-06-05T07:24:00Z

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

        Assert.assertEquals(defaultDomainCpuMax, result);
    }

+    @Test


again, can this test case be split?

alrighty, I was able to do the changes as you described. Please let me know if there is anything else I can do.

ok, this one looks good, not sure if the build failures are related.

…o split unit test in ResourceManagerImplTest into multiple smaller tests

borisstoyanov · 2024-06-10T07:05:59Z

@blueorangutan package

blueorangutan · 2024-06-10T07:08:03Z

@borisstoyanov a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-06-10T07:37:24Z

Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 9835

borisstoyanov

@my-code-AL hi, thanks for this PR, there seems to be a problem with the unit tests


10:19:49 [INFO] Tests run: 12, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.251 s - in org.apache.cloudstack.affinity.AffinityGroupServiceImplTest
10:19:49 [INFO] 
10:19:50 [INFO] Results:
10:19:50 [INFO] 
10:19:50 [ERROR] Errors: 
10:19:50 [ERROR]   ResourceLimitManagerImplTest.unnecessary Mockito stubbings ? UnnecessaryStubbing
10:19:50 [INFO] 
10:19:50 [ERROR] Tests run: 1885, Failures: 0, Errors: 1, Skipped: 6
10:19:50 [INFO] 
10:19:50 [INFO] ------------------------------------------------------------------------
10:19:50 [INFO] Reactor Summary for Apache CloudStack 4.20.0.0-SNAPSHOT:

Can you please look into this?

yadvr

LGTM - this needs additional review/testing, and check if it needs any Marvin tests?

DaanHoogland · 2024-07-11T11:58:47Z

@my-code-AL can you look at the unit tests @borisstoyanov pointed at?

my-code-AL · 2024-07-11T21:45:56Z

@DaanHoogland Will do! I've been busy at work but will fix soon!

my-code-AL · 2024-07-12T00:01:51Z

@blueorangutan package

blueorangutan · 2024-07-12T00:04:03Z

@my-code-AL a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-07-12T01:20:45Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10330

DaanHoogland · 2024-07-12T07:58:24Z

@blueorangutan package

blueorangutan · 2024-07-12T08:00:03Z

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-07-12T09:04:06Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10337

DaanHoogland · 2024-07-12T09:33:23Z

@blueorangutan LLtest alma9 kvm-alma9 keepEnv

blueorangutan · 2024-07-12T09:34:03Z

@DaanHoogland a [LL] Trillian-Jenkins test job (alma9 mgmt + kvm-alma9) has been kicked to run smoke tests

blueorangutan · 2024-07-13T00:18:37Z

[LL]Trillian test result (tid-6953)
Environment: kvm-alma9 (x2), Advanced Networking with Mgmt server a9
Total time taken: 50617 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9172-t6953-kvm-alma9.zip
Smoke tests completed. 134 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestClusterDRS>:setup	`Error`	0.00	test_cluster_drs.py
test_01_invalid_upgrade_kubernetes_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_02_upgrade_kubernetes_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_04_autoscale_kubernetes_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_05_basic_lifecycle_kubernetes_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_06_delete_kubernetes_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_10_vpc_tier_kubernetes_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_11_test_unmanaged_cluster_lifecycle	`Error`	0.00	test_kubernetes_clusters.py
test_06_purge_expunged_vm_background_task	`Failure`	377.98	test_purge_expunged_vms.py

DaanHoogland · 2024-07-15T06:46:57Z

setting the limit to 1 an error occurs on creating the second:

after deleting the first prj the second gets created.

setting for a domain to two

a second user can create their own prj:

good work @my-code-AL , totally out of scope but I was wondering if there is any reason this setting is not dynamic (i.e. can be changed without restart) work for another day maybe ;)

boring-cyborg · 2024-07-15T06:47:16Z

Awesome work, congrats on your first merged pull request!

my-code-AL · 2024-07-16T00:44:02Z

Will do! Thank you for your help and patience!

boring-cyborg bot added component:api component:integration-test component:orchestration component:UI component:vpc Python Warning... Python code Ahead! labels Jun 5, 2024

DaanHoogland approved these changes Jun 5, 2024

View reviewed changes

DaanHoogland added this to the 4.20.0.0 milestone Jun 5, 2024

DaanHoogland linked an issue Jun 5, 2024 that may be closed by this pull request

Add limit configuration for number of projects per domain/account and add UI function for configuring limits #9127

Closed

DaanHoogland added the status:needs-testing label Jun 5, 2024

my-code-AL added 3 commits June 7, 2024 12:51

api for resource limit configuration

86a0aef

adhere to checkstyle rulesets

da479ec

updated max project default settings to ConfigKey implementation. Als…

86746b0

…o split unit test in ResourceManagerImplTest into multiple smaller tests

my-code-AL force-pushed the add-limit-configuration-for-number-of-projects branch from 11aecf4 to 86746b0 Compare June 7, 2024 19:51

borisstoyanov requested changes Jun 10, 2024

View reviewed changes

borisstoyanov self-assigned this Jun 12, 2024

yadvr approved these changes Jun 12, 2024

View reviewed changes

my-code-AL and others added 2 commits July 11, 2024 16:53

removal of unnecessary Mockito stubbing

2afec67

Merge branch 'main' into add-limit-configuration-for-number-of-projects

874d13e

my-code-AL requested a review from borisstoyanov July 12, 2024 01:20

Merge branch 'main' into add-limit-configuration-for-number-of-projects

241520c

DaanHoogland merged commit afdf4d7 into apache:main Jul 15, 2024

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jul 23, 2024

Add limit configuration for number of projects (apache#9172)

46c8205

DaanHoogland removed the status:needs-testing label Jan 28, 2026

Conversation

my-code-AL commented Jun 5, 2024

Description

Types of changes

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

codecov bot commented Jun 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jun 5, 2024

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jun 5, 2024

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jun 5, 2024

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jun 5, 2024

Choose a reason for hiding this comment

Uh oh!

my-code-AL Jun 7, 2024

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jun 11, 2024

Choose a reason for hiding this comment

Uh oh!

borisstoyanov commented Jun 10, 2024

Uh oh!

blueorangutan commented Jun 10, 2024

Uh oh!

blueorangutan commented Jun 10, 2024

Uh oh!

borisstoyanov left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Jul 11, 2024

Uh oh!

my-code-AL commented Jul 11, 2024

Uh oh!

my-code-AL commented Jul 12, 2024

Uh oh!

blueorangutan commented Jul 12, 2024

Uh oh!

blueorangutan commented Jul 12, 2024

Uh oh!

DaanHoogland commented Jul 12, 2024

Uh oh!

blueorangutan commented Jul 12, 2024

Uh oh!

blueorangutan commented Jul 12, 2024

Uh oh!

DaanHoogland commented Jul 12, 2024

Uh oh!

blueorangutan commented Jul 12, 2024

Uh oh!

blueorangutan commented Jul 13, 2024

Uh oh!

DaanHoogland commented Jul 15, 2024

Uh oh!

boring-cyborg bot commented Jul 15, 2024

Uh oh!

my-code-AL commented Jul 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

codecov bot commented Jun 5, 2024 •

edited

Loading

my-code-AL commented Jul 16, 2024 •

edited

Loading