Improve login time - #6412
Conversation
DaanHoogland
left a comment
DaanHoogland
left a comment
There was a problem hiding this comment.
clgtm
extensive testing in unit tests, I think manual testing will still be needed, though
some questions
|
Found UI changes, kicking a new UI QA build |
|
@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
|
Found UI changes, kicking a new UI QA build |
|
@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
shwstppr
left a comment
shwstppr
left a comment
There was a problem hiding this comment.
Tested locally with a simulator env against main branch.
main
pr-6412
shwstppr@shwstppr-ThinkPad:~/lab/shapeblue/cloudstack/ui|main
⇒ time cmk list apis >/dev/null 2>/dev/null
cmk list apis > /dev/null 2> /dev/null 0.34s user 0.04s system 10% cpu 3.569 total
shwstppr@shwstppr-ThinkPad:~/lab/shapeblue/cloudstack/ui|pr-6412
⇒ time cmk list apis >/dev/null 2>/dev/null
cmk list apis > /dev/null 2> /dev/null 0.40s user 0.04s system 25% cpu 1.726 total
|
@blueorangutan package |
|
@shwstppr a 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. |
|
Packaging result: ✖️ el7 ✔️ el8 ✔️ debian ✖️ suse15. SL-JID 3635 |
|
@blueorangutan package |
|
@DaanHoogland a 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. |
|
Packaging result: ✖️ el7 ✖️ el8 ✖️ debian ✖️ suse15. SL-JID 3651 |
|
@blueorangutan package |
|
Found UI changes, kicking a new UI QA build |
|
@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
@blueorangutan package |
|
UI build: ✔️ |
|
SonarCloud Quality Gate failed. |
|
@blueorangutan package |
|
@shwstppr a 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. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✖️ suse15. SL-JID 3662 |
|
@blueorangutan test |
|
@shwstppr a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-4405)
|
yadvr
left a comment
yadvr
left a comment
There was a problem hiding this comment.
LGTM, my only concern is other users of this API such as cmk aren't broken. Smoketests LGTM.
|
@harikrishna-patnala @Pearl1594 can you review the PR and test an env if possible. The changes are in dynamic and project roles checker where we can use your expertise. |
|
This is ready for merging, asking a final review from security PoV cc @harikrishna-patnala |
|
(rekick travis against latest main) |
harikrishna-patnala
left a comment
harikrishna-patnala
left a comment
There was a problem hiding this comment.
LGTM wrt the API checker code and tests.
9 participants
Description
The API
listApiscurrently has too many redundant calls to the DB, which causes slowness on the ACS login. This PR aims to improve the time of login on ACS by refactoring the classes that implement theAPICheckerinterface. In my local lab, the login usually took 13 seconds, without login cache. With this new version, the login took around 2 seconds, with no login cache as well.Fixes: #5266
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
I created multiple accounts with the roles based on the following table. I compared this PR with a version with the unmodified
APIChecker. And the number of APIs were returned as expected, as the table shows.Furthermore, I tried multiple functions through the UI with these accounts, and the APIs allowed were working correctly.