You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "nvazquez (via GitHub)" <gi...@apache.org> on 2023/01/25 12:51:51 UTC
[GitHub] [cloudstack] nvazquez opened a new pull request, #7132: Add console session cleanup task
nvazquez opened a new pull request, #7132:
URL: https://github.com/apache/cloudstack/pull/7132
### Description
This PR adds a console session cleanup task after changes introduced on PR #7094
### 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)
- [x] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1405817381
@nvazquez 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on pull request #7132: Add console session cleanup task
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1412164710
Tests LGTM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1410198680
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5456
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1411129233
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7132: Add console session cleanup task
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1410046982
@nvazquez can you resolve the conflict?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1410122679
@nvazquez 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1403829819
@nvazquez 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1405857254
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5416
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #7132: Add console session cleanup task
Posted by sonarcloud.
sonarcloud[bot] commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1410216375
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=7132)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list) [0.6% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list) [7.7% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1411256901
<b>Trillian test result (tid-6041)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42557 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7132-t6041-kvm-centos7.zip
Smoke tests completed. 102 look OK, 5 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_add_primary_storage_disabled_host | `Error` | 0.64 | test_primary_storage.py
test_01_primary_storage_nfs | `Error` | 0.12 | test_primary_storage.py
ContextSuite context=TestStorageTags>:setup | `Error` | 0.21 | test_primary_storage.py
test_01_non_strict_host_anti_affinity | `Failure` | 106.63 | test_nonstrict_affinity_group.py
test_02_non_strict_host_affinity | `Error` | 80.04 | test_nonstrict_affinity_group.py
test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 35.99 | test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster | `Failure` | 61.47 | test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster | `Failure` | 42.05 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 38.95 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 126.71 | test_kubernetes_clusters.py
test_01_secure_vm_migration | `Error` | 156.21 | test_vm_life_cycle.py
test_02_unsecure_vm_migration | `Error` | 268.85 | test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration | `Error` | 140.72 | test_vm_life_cycle.py
test_08_migrate_vm | `Error` | 43.77 | test_vm_life_cycle.py
test_hostha_enable_ha_when_host_in_maintenance | `Error` | 304.90 | test_hostha_kvm.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1411331891
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1403829280
Thanks @DaanHoogland @weizhouapache @GutoVeronezi - I've addressed your review comments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on a diff in pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1091738987
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -100,9 +110,63 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
ConsoleAccessManagerImpl.secretKeysManager = keysManager;
+ executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("ConsoleSession-Scavenger"));
return super.configure(name, params);
}
+ @Override
+ public boolean start() {
+ int consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value();
+ if (consoleCleanupInterval > 0) {
+ s_logger.info(String.format("The ConsoleSessionCleanupTask will run every %s hours", consoleCleanupInterval));
+ executorService.scheduleWithFixedDelay(new ConsoleSessionCleanupTask(), consoleCleanupInterval, consoleCleanupInterval, TimeUnit.HOURS);
+ }
+ return true;
+ }
+
+ @Override
+ public String getConfigComponentName() {
+ return ConsoleAccessManager.class.getName();
+ }
+
+ @Override
+ public ConfigKey<?>[] getConfigKeys() {
+ return new ConfigKey[] {
+ ConsoleAccessManager.ConsoleSessionCleanupInterval,
+ ConsoleAccessManager.ConsoleSessionCleanupRetentionHours
+ };
+ }
+
+ public class ConsoleSessionCleanupTask extends ManagedContextRunnable {
+ @Override
+ protected void runInContext() {
+ final GlobalLock gcLock = GlobalLock.getInternLock("ConsoleSession.Cleanup.Lock");
+ try {
+ if (gcLock.lock(3)) {
+ try {
+ reallyRun();
+ } finally {
+ gcLock.unlock();
+ }
+ }
+ } finally {
+ gcLock.releaseRef();
+ }
+ }
+
+ private void reallyRun() {
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("Starting ConsoleSessionCleanupTask...");
+ }
+ Integer retentionHours = ConsoleAccessManager.ConsoleSessionCleanupRetentionHours.value();
+ Date dateBefore = DateTime.now().minusHours(retentionHours).toDate();
Review Comment:
Added, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1404123949
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5409
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1405817217
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1403937325
@nvazquez 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1404056661
@nvazquez 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #7132: Add console session cleanup task
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1086886126
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -66,6 +70,9 @@
import java.util.List;
import java.util.Map;
import java.util.UUID;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAccessManager {
Review Comment:
@nvazquez
this should implement `Configurable` class and its methods as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on a diff in pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1086750935
##########
api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java:
##########
@@ -18,9 +18,31 @@
import com.cloud.utils.component.Manager;
import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint;
+import org.apache.cloudstack.framework.config.ConfigKey;
public interface ConsoleAccessManager extends Manager {
+ ConfigKey<Boolean> ConsoleSessionCleanupEnabled = new ConfigKey<>("Advanced", Boolean.class,
+ "console.session.cleanup.enabled",
+ "true",
+ "Enables/disables the console session cleanup thread.",
+ true,
Review Comment:
Yes, my mistake, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1403936424
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] codecov[bot] commented on pull request #7132: Add console session cleanup task
Posted by codecov.
codecov[bot] commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1404092720
# [Codecov](https://codecov.io/gh/apache/cloudstack/pull/7132?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7132](https://codecov.io/gh/apache/cloudstack/pull/7132?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9e103c9) into [main](https://codecov.io/gh/apache/cloudstack/commit/d288bb0c78e0f3964982926070bd14785c88a441?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d288bb0) will **decrease** coverage by `0.01%`.
> The diff coverage is `3.03%`.
> :exclamation: Current head 9e103c9 differs from pull request most recent head 3e24f81. Consider uploading reports for the commit 3e24f81 to get more accurate results
```diff
@@ Coverage Diff @@
## main #7132 +/- ##
============================================
- Coverage 11.77% 11.77% -0.01%
Complexity 7660 7660
============================================
Files 2505 2505
Lines 246242 246274 +32
Branches 38415 38420 +5
============================================
+ Hits 28993 28994 +1
- Misses 213470 213501 +31
Partials 3779 3779
```
| [Impacted Files](https://codecov.io/gh/apache/cloudstack/pull/7132?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java](https://codecov.io/gh/apache/cloudstack/pull/7132?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC92bS9kYW8vQ29uc29sZVNlc3Npb25EYW9JbXBsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...udstack/consoleproxy/ConsoleAccessManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/7132?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL2NvbnNvbGVwcm94eS9Db25zb2xlQWNjZXNzTWFuYWdlckltcGwuamF2YQ==) | `5.17% <4.00%> (-0.14%)` | :arrow_down: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1410619479
@nvazquez 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1410617969
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a diff in pull request #7132: Add console session cleanup task
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1090861795
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -100,9 +110,63 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
ConsoleAccessManagerImpl.secretKeysManager = keysManager;
+ executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("ConsoleSession-Scavenger"));
return super.configure(name, params);
}
+ @Override
+ public boolean start() {
+ int consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value();
+ if (consoleCleanupInterval > 0) {
+ s_logger.info(String.format("The ConsoleSessionCleanupTask will run every %s hours", consoleCleanupInterval));
+ executorService.scheduleWithFixedDelay(new ConsoleSessionCleanupTask(), consoleCleanupInterval, consoleCleanupInterval, TimeUnit.HOURS);
+ }
+ return true;
+ }
+
+ @Override
+ public String getConfigComponentName() {
+ return ConsoleAccessManager.class.getName();
+ }
+
+ @Override
+ public ConfigKey<?>[] getConfigKeys() {
+ return new ConfigKey[] {
+ ConsoleAccessManager.ConsoleSessionCleanupInterval,
+ ConsoleAccessManager.ConsoleSessionCleanupRetentionHours
+ };
+ }
+
+ public class ConsoleSessionCleanupTask extends ManagedContextRunnable {
+ @Override
+ protected void runInContext() {
+ final GlobalLock gcLock = GlobalLock.getInternLock("ConsoleSession.Cleanup.Lock");
+ try {
+ if (gcLock.lock(3)) {
+ try {
+ reallyRun();
+ } finally {
+ gcLock.unlock();
+ }
+ }
+ } finally {
+ gcLock.releaseRef();
+ }
+ }
+
+ private void reallyRun() {
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("Starting ConsoleSessionCleanupTask...");
+ }
+ Integer retentionHours = ConsoleAccessManager.ConsoleSessionCleanupRetentionHours.value();
+ Date dateBefore = DateTime.now().minusHours(retentionHours).toDate();
+ int sessionsExpunged = consoleSessionDao.expungeSessionsOlderThanDate(dateBefore);
+ if (sessionsExpunged > 0 && s_logger.isDebugEnabled()) {
+ s_logger.info(String.format("Expunged %s removed console session records", sessionsExpunged));
Review Comment:
It is validating if debug is enabled (`s_logger.isDebugEnabled()`); however, the log is in info.
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -100,9 +110,63 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
ConsoleAccessManagerImpl.secretKeysManager = keysManager;
+ executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("ConsoleSession-Scavenger"));
return super.configure(name, params);
}
+ @Override
+ public boolean start() {
+ int consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value();
+ if (consoleCleanupInterval > 0) {
+ s_logger.info(String.format("The ConsoleSessionCleanupTask will run every %s hours", consoleCleanupInterval));
+ executorService.scheduleWithFixedDelay(new ConsoleSessionCleanupTask(), consoleCleanupInterval, consoleCleanupInterval, TimeUnit.HOURS);
+ }
+ return true;
+ }
+
+ @Override
+ public String getConfigComponentName() {
+ return ConsoleAccessManager.class.getName();
+ }
+
+ @Override
+ public ConfigKey<?>[] getConfigKeys() {
+ return new ConfigKey[] {
+ ConsoleAccessManager.ConsoleSessionCleanupInterval,
+ ConsoleAccessManager.ConsoleSessionCleanupRetentionHours
+ };
+ }
+
+ public class ConsoleSessionCleanupTask extends ManagedContextRunnable {
+ @Override
+ protected void runInContext() {
+ final GlobalLock gcLock = GlobalLock.getInternLock("ConsoleSession.Cleanup.Lock");
+ try {
+ if (gcLock.lock(3)) {
+ try {
+ reallyRun();
+ } finally {
+ gcLock.unlock();
+ }
+ }
+ } finally {
+ gcLock.releaseRef();
+ }
+ }
+
+ private void reallyRun() {
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("Starting ConsoleSessionCleanupTask...");
+ }
+ Integer retentionHours = ConsoleAccessManager.ConsoleSessionCleanupRetentionHours.value();
+ Date dateBefore = DateTime.now().minusHours(retentionHours).toDate();
+ int sessionsExpunged = consoleSessionDao.expungeSessionsOlderThanDate(dateBefore);
+ if (sessionsExpunged > 0 && s_logger.isDebugEnabled()) {
+ s_logger.info(String.format("Expunged %s removed console session records", sessionsExpunged));
Review Comment:
We could also add a log when there is no console sessions to expunge.
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -100,9 +110,63 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
ConsoleAccessManagerImpl.secretKeysManager = keysManager;
+ executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("ConsoleSession-Scavenger"));
return super.configure(name, params);
}
+ @Override
+ public boolean start() {
+ int consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value();
+ if (consoleCleanupInterval > 0) {
+ s_logger.info(String.format("The ConsoleSessionCleanupTask will run every %s hours", consoleCleanupInterval));
+ executorService.scheduleWithFixedDelay(new ConsoleSessionCleanupTask(), consoleCleanupInterval, consoleCleanupInterval, TimeUnit.HOURS);
+ }
+ return true;
+ }
+
+ @Override
+ public String getConfigComponentName() {
+ return ConsoleAccessManager.class.getName();
+ }
+
+ @Override
+ public ConfigKey<?>[] getConfigKeys() {
+ return new ConfigKey[] {
+ ConsoleAccessManager.ConsoleSessionCleanupInterval,
+ ConsoleAccessManager.ConsoleSessionCleanupRetentionHours
+ };
+ }
+
+ public class ConsoleSessionCleanupTask extends ManagedContextRunnable {
+ @Override
+ protected void runInContext() {
+ final GlobalLock gcLock = GlobalLock.getInternLock("ConsoleSession.Cleanup.Lock");
+ try {
+ if (gcLock.lock(3)) {
+ try {
+ reallyRun();
+ } finally {
+ gcLock.unlock();
+ }
+ }
+ } finally {
+ gcLock.releaseRef();
+ }
+ }
+
+ private void reallyRun() {
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("Starting ConsoleSessionCleanupTask...");
+ }
+ Integer retentionHours = ConsoleAccessManager.ConsoleSessionCleanupRetentionHours.value();
+ Date dateBefore = DateTime.now().minusHours(retentionHours).toDate();
Review Comment:
We could add a log in debug informing the retention and the date limit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1403829496
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on a diff in pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1086911391
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -66,6 +70,9 @@
import java.util.List;
import java.util.Map;
import java.util.UUID;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAccessManager {
Review Comment:
Done, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] github-actions[bot] commented on pull request #7132: Add console session cleanup task
Posted by github-actions.
github-actions[bot] commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1410017246
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7132: Add console session cleanup task
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1412183861
> > @GutoVeronezi is this lgty now?
>
> @DaanHoogland, in overall it looks good; I pointed out a change that could be interesting to discuss before merging this one: [#7132 (comment)](https://github.com/apache/cloudstack/pull/7132#discussion_r1090566123)
>
> cc: @nvazquez
right, I knew there was a reason I didn´t merge this yet ;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7132: Add console session cleanup task
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1086611929
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -100,9 +111,53 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
ConsoleAccessManagerImpl.secretKeysManager = keysManager;
+ executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("ConsoleSession-Scavenger"));
return super.configure(name, params);
}
+ @Override
+ public boolean start() {
+ Integer consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value();
+ Boolean consoleCleanupEnabled = ConsoleAccessManager.ConsoleSessionCleanupEnabled.value();
+ if (BooleanUtils.isTrue(consoleCleanupEnabled)) {
Review Comment:
this would be
```suggestion
if (consoleCleanupInterval > 0) {
```
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -100,9 +111,53 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
ConsoleAccessManagerImpl.secretKeysManager = keysManager;
+ executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("ConsoleSession-Scavenger"));
return super.configure(name, params);
}
+ @Override
+ public boolean start() {
+ Integer consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value();
Review Comment:
```suggestion
int consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value();
```
##########
api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java:
##########
@@ -18,9 +18,31 @@
import com.cloud.utils.component.Manager;
import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint;
+import org.apache.cloudstack.framework.config.ConfigKey;
public interface ConsoleAccessManager extends Manager {
+ ConfigKey<Boolean> ConsoleSessionCleanupEnabled = new ConfigKey<>("Advanced", Boolean.class,
+ "console.session.cleanup.enabled",
+ "true",
+ "Enables/disables the console session cleanup thread.",
+ true,
+ ConfigKey.Scope.Global);
Review Comment:
we don´t need this, retention of -1 would be effectively disable it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on a diff in pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1086741420
##########
api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java:
##########
@@ -18,9 +18,31 @@
import com.cloud.utils.component.Manager;
import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint;
+import org.apache.cloudstack.framework.config.ConfigKey;
public interface ConsoleAccessManager extends Manager {
+ ConfigKey<Boolean> ConsoleSessionCleanupEnabled = new ConfigKey<>("Advanced", Boolean.class,
+ "console.session.cleanup.enabled",
+ "true",
+ "Enables/disables the console session cleanup thread.",
+ true,
+ ConfigKey.Scope.Global);
Review Comment:
Makes sense, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #7132: Add console session cleanup task
Posted by sonarcloud.
sonarcloud[bot] commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1410742414
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=7132)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list) [0.4% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list) [6.7% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #7132: Add console session cleanup task
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1412175905
> @GutoVeronezi is this lgty now?
@DaanHoogland, in overall it looks good; I pointed out a change that could be interesting to discuss before merging this one: https://github.com/apache/cloudstack/pull/7132#discussion_r1090566123
cc: @nvazquez
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland merged pull request #7132: Add console session cleanup task
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland merged PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #7132: Add console session cleanup task
Posted by sonarcloud.
sonarcloud[bot] commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1404137907
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=7132)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list) [1.1% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list) [12.4% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #7132: Add console session cleanup task
Posted by sonarcloud.
sonarcloud[bot] commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1404579975
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=7132)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list) [0.7% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list) [9.1% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #7132: Add console session cleanup task
Posted by sonarcloud.
sonarcloud[bot] commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1403654211
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=7132)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list) [1.3% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list) [12.9% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a diff in pull request #7132: Add console session cleanup task
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1090566123
##########
engine/schema/src/main/resources/META-INF/db/schema-41720to41800.sql:
##########
@@ -1056,6 +1056,7 @@ CREATE TABLE IF NOT EXISTS `cloud`.`console_session` (
`user_id` bigint(20) unsigned NOT NULL COMMENT 'User who generated the session',
`instance_id` bigint(20) unsigned NOT NULL COMMENT 'VM for which the session was generated',
`host_id` bigint(20) unsigned NOT NULL COMMENT 'Host where the VM was when the session was generated',
+ `acquired` int(1) NOT NULL DEFAULT 0 COMMENT 'True if the session was already used',
Review Comment:
@nvazquez, could this field be a date? This way we would know the exactly time user used the session.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #7132: Add console session cleanup task
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1412236872
> Thanks @GutoVeronezi I would prefer to merge this PR as it is to not keep blocking 4.18 at this point, and have created an improvement request for the change on: #7159
@nvazquez
can you estimate the time on the coding ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1410122416
Thanks for reviewing @GutoVeronezi @DaanHoogland, conflicts fixed
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on pull request #7132: Add console session cleanup task
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1408450611
Has this been manually tested @nvazquez @borisstoyanov ?
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1403911071
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5406
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1408616856
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5442
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1403565506
@nvazquez 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1403563920
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #7132: Add console session cleanup task
Posted by sonarcloud.
sonarcloud[bot] commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1406669294
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=7132)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list) [0.6% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list) [8.1% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on a diff in pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1086779135
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -100,9 +111,53 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
ConsoleAccessManagerImpl.secretKeysManager = keysManager;
+ executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("ConsoleSession-Scavenger"));
return super.configure(name, params);
}
+ @Override
+ public boolean start() {
+ Integer consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value();
+ Boolean consoleCleanupEnabled = ConsoleAccessManager.ConsoleSessionCleanupEnabled.value();
+ if (BooleanUtils.isTrue(consoleCleanupEnabled)) {
+ s_logger.info(String.format("The ConsoleSessionCleanupTask will run every %s hours", consoleCleanupInterval));
+ executorService.scheduleWithFixedDelay(new ConsoleSessionCleanupTask(), consoleCleanupInterval, consoleCleanupInterval, TimeUnit.HOURS);
+ }
+ return true;
+ }
+
+ public class ConsoleSessionCleanupTask extends ManagedContextRunnable {
+ @Override
+ protected void runInContext() {
+ final GlobalLock gcLock = GlobalLock.getInternLock("ConsoleSession.Cleanup.Lock");
+ try {
+ if (gcLock.lock(3)) {
+ try {
+ reallyRun();
+ } finally {
+ gcLock.unlock();
+ }
+ }
+ } finally {
+ gcLock.releaseRef();
+ }
+ }
+
+ private void reallyRun() {
+ s_logger.info("Starting ConsoleSessionCleanupTask...");
+ Integer retentionDays = ConsoleAccessManager.ConsoleSessionCleanupRetentionDays.value();
+ Date date = GregorianCalendar.getInstance().getTime();
+ Date dateBefore = new Date(date.getTime() - retentionDays * 24 * 3600 * 1000);
+ List<ConsoleSessionVO> sessionsToExpunge = consoleSessionDao.listRemovedSessionsOlderThanDate(dateBefore);
+ if (CollectionUtils.isNotEmpty(sessionsToExpunge)) {
+ s_logger.info(String.format("Expunging %s removed console session records"));
Review Comment:
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #7132: Add console session cleanup task
Posted by sonarcloud.
sonarcloud[bot] commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1404015120
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=7132)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list) [1.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list) [12.1% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7132: Add console session cleanup task
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1406277897
> SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=7132)
>
> [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL)
>
> [](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list) [0.6% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list) [](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list) [8.1% Dupli
cation](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list)
@nvazquez can you look at the code smells?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #7132: Add console session cleanup task
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1408538242
> Has this been manually tested @nvazquez @borisstoyanov ? @blueorangutan package
@rohityadavcloud, I am building the code right now to test it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on pull request #7132: Add console session cleanup task
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1408620051
Thanks @GutoVeronezi I'll kick a round of smoketests
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1408451761
@rohityadavcloud 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1411129629
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1412130689
<b>Trillian test result (tid-6058)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43175 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7132-t6058-kvm-centos7.zip
Smoke tests completed. 107 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1409644021
<b>Trillian test result (tid-6029)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 45229 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7132-t6029-kvm-centos7.zip
Smoke tests completed. 107 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #7132: Add console session cleanup task
Posted by sonarcloud.
sonarcloud[bot] commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1405862083
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=7132)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list) [0.6% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list) [8.1% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1403669393
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5401
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #7132: Add console session cleanup task
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1408938773
@rohityadavcloud @nvazquez
I have tested the task with retention of 1 hour and interval of 1 hour; therefore, in the worst case, the entry would exist for almost 2 hours:
- generated some sessions:
- after 1 hour, the task executed; however, as the session were removed after `current time - 1 hour`, no sessions were expunged (as expected):
```
daniel@mgmt01:~$ sudo grep 'Starting ConsoleSessionCleanupTas' /var/log/cloudstack/management/management-server.log
2023-01-30 14:56:20,776 DEBUG [o.a.c.c.ConsoleAccessManagerImpl] (ConsoleSession-Scavenger-1:ctx-284d7f68) (logid:3662c3ad) Starting ConsoleSessionCleanupTask...
daniel@mgmt01:~$ sudo grep 'logid:3662c3ad' /var/log/cloudstack/management/management-server.log
2023-01-30 14:56:20,776 DEBUG [o.a.c.c.ConsoleAccessManagerImpl] (ConsoleSession-Scavenger-1:ctx-284d7f68) (logid:3662c3ad) Starting ConsoleSessionCleanupTask...
```
- after the first run, I generated two more sessions:
- then, in the second run, 4 sessions were expunged (as expected):
```
daniel@mgmt01:~$ sudo grep 'Starting ConsoleSessionCleanupTas' /var/log/cloudstack/management/management-server.log
2023-01-30 14:56:20,776 DEBUG [o.a.c.c.ConsoleAccessManagerImpl] (ConsoleSession-Scavenger-1:ctx-284d7f68) (logid:3662c3ad) Starting ConsoleSessionCleanupTask...
2023-01-30 15:56:20,823 DEBUG [o.a.c.c.ConsoleAccessManagerImpl] (ConsoleSession-Scavenger-1:ctx-659e1257) (logid:407a5e2e) Starting ConsoleSessionCleanupTask...
daniel@mgmt01:~$ sudo grep 'logid:407a5e2e' /var/log/cloudstack/management/management-server.log
2023-01-30 15:56:20,823 DEBUG [o.a.c.c.ConsoleAccessManagerImpl] (ConsoleSession-Scavenger-1:ctx-659e1257) (logid:407a5e2e) Starting ConsoleSessionCleanupTask...
2023-01-30 15:56:20,832 INFO [o.a.c.c.ConsoleAccessManagerImpl] (ConsoleSession-Scavenger-1:ctx-659e1257) (logid:407a5e2e) Expunged 4 removed console session records
```
The functionality is working as expected; I think we could improve the logs, though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a diff in pull request #7132: Add console session cleanup task
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1090566123
##########
engine/schema/src/main/resources/META-INF/db/schema-41720to41800.sql:
##########
@@ -1056,6 +1056,7 @@ CREATE TABLE IF NOT EXISTS `cloud`.`console_session` (
`user_id` bigint(20) unsigned NOT NULL COMMENT 'User who generated the session',
`instance_id` bigint(20) unsigned NOT NULL COMMENT 'VM for which the session was generated',
`host_id` bigint(20) unsigned NOT NULL COMMENT 'Host where the VM was when the session was generated',
+ `acquired` int(1) NOT NULL DEFAULT 0 COMMENT 'True if the session was already used',
Review Comment:
@nvazquez, could this field be a date? This way we would know the exactly time user used the session and for how many time, as removed will be set when the user closes the console.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1410722958
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5461
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1411331309
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on pull request #7132: Add console session cleanup task
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1412262500
> Thanks @GutoVeronezi I would prefer to merge this PR as it is to not keep blocking 4.18 at this point, and have created an improvement request for the change on: #7159
Right, thanks @nvazquez
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1411763355
<b>Trillian test result (tid-6057)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41061 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7132-t6057-kvm-centos7.zip
Smoke tests completed. 107 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1412203076
Thanks @GutoVeronezi I would prefer to merge this PR as it is to not keep blocking 4.18 at this point, and have created an improvement request for the change on: https://github.com/apache/cloudstack/issues/7159
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1410234944
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1410235309
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7132: Add console session cleanup task
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1412071484
@GutoVeronezi is this lgty now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1404056083
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on a diff in pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1086779485
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -100,9 +111,53 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
ConsoleAccessManagerImpl.secretKeysManager = keysManager;
+ executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("ConsoleSession-Scavenger"));
return super.configure(name, params);
}
+ @Override
+ public boolean start() {
+ Integer consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value();
+ Boolean consoleCleanupEnabled = ConsoleAccessManager.ConsoleSessionCleanupEnabled.value();
+ if (BooleanUtils.isTrue(consoleCleanupEnabled)) {
+ s_logger.info(String.format("The ConsoleSessionCleanupTask will run every %s hours", consoleCleanupInterval));
+ executorService.scheduleWithFixedDelay(new ConsoleSessionCleanupTask(), consoleCleanupInterval, consoleCleanupInterval, TimeUnit.HOURS);
+ }
+ return true;
+ }
+
+ public class ConsoleSessionCleanupTask extends ManagedContextRunnable {
+ @Override
+ protected void runInContext() {
+ final GlobalLock gcLock = GlobalLock.getInternLock("ConsoleSession.Cleanup.Lock");
+ try {
+ if (gcLock.lock(3)) {
+ try {
+ reallyRun();
+ } finally {
+ gcLock.unlock();
+ }
+ }
+ } finally {
+ gcLock.releaseRef();
+ }
+ }
+
+ private void reallyRun() {
+ s_logger.info("Starting ConsoleSessionCleanupTask...");
+ Integer retentionDays = ConsoleAccessManager.ConsoleSessionCleanupRetentionDays.value();
+ Date date = GregorianCalendar.getInstance().getTime();
Review Comment:
Replaced this for a simpler way
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -100,9 +111,53 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
ConsoleAccessManagerImpl.secretKeysManager = keysManager;
+ executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("ConsoleSession-Scavenger"));
return super.configure(name, params);
}
+ @Override
+ public boolean start() {
+ Integer consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value();
+ Boolean consoleCleanupEnabled = ConsoleAccessManager.ConsoleSessionCleanupEnabled.value();
+ if (BooleanUtils.isTrue(consoleCleanupEnabled)) {
+ s_logger.info(String.format("The ConsoleSessionCleanupTask will run every %s hours", consoleCleanupInterval));
+ executorService.scheduleWithFixedDelay(new ConsoleSessionCleanupTask(), consoleCleanupInterval, consoleCleanupInterval, TimeUnit.HOURS);
+ }
+ return true;
+ }
+
+ public class ConsoleSessionCleanupTask extends ManagedContextRunnable {
+ @Override
+ protected void runInContext() {
+ final GlobalLock gcLock = GlobalLock.getInternLock("ConsoleSession.Cleanup.Lock");
+ try {
+ if (gcLock.lock(3)) {
+ try {
+ reallyRun();
+ } finally {
+ gcLock.unlock();
+ }
+ }
+ } finally {
+ gcLock.releaseRef();
+ }
+ }
+
+ private void reallyRun() {
+ s_logger.info("Starting ConsoleSessionCleanupTask...");
+ Integer retentionDays = ConsoleAccessManager.ConsoleSessionCleanupRetentionDays.value();
+ Date date = GregorianCalendar.getInstance().getTime();
Review Comment:
Replaced this for a simpler way as the retention changed to hours
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on a diff in pull request #7132: Add console session cleanup task
Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1086769074
##########
engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java:
##########
@@ -19,11 +19,24 @@
package com.cloud.vm.dao;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
import com.cloud.vm.ConsoleSessionVO;
import com.cloud.utils.db.GenericDaoBase;
+import java.util.Date;
+import java.util.List;
+
public class ConsoleSessionDaoImpl extends GenericDaoBase<ConsoleSessionVO, Long> implements ConsoleSessionDao {
+ private final SearchBuilder<ConsoleSessionVO> SearchByRemovedDate;
+
+ public ConsoleSessionDaoImpl() {
+ SearchByRemovedDate = createSearchBuilder();
+ SearchByRemovedDate.and("removed", SearchByRemovedDate.entity().getRemoved(), SearchCriteria.Op.NNULL);
Review Comment:
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1408522582
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5439
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1404010185
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5407
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GutoVeronezi commented on a diff in pull request #7132: Add console session cleanup task
Posted by "GutoVeronezi (via GitHub)" <gi...@apache.org>.
GutoVeronezi commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1086675808
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -100,9 +111,53 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
ConsoleAccessManagerImpl.secretKeysManager = keysManager;
+ executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("ConsoleSession-Scavenger"));
return super.configure(name, params);
}
+ @Override
+ public boolean start() {
+ Integer consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value();
+ Boolean consoleCleanupEnabled = ConsoleAccessManager.ConsoleSessionCleanupEnabled.value();
+ if (BooleanUtils.isTrue(consoleCleanupEnabled)) {
+ s_logger.info(String.format("The ConsoleSessionCleanupTask will run every %s hours", consoleCleanupInterval));
+ executorService.scheduleWithFixedDelay(new ConsoleSessionCleanupTask(), consoleCleanupInterval, consoleCleanupInterval, TimeUnit.HOURS);
+ }
+ return true;
+ }
+
+ public class ConsoleSessionCleanupTask extends ManagedContextRunnable {
+ @Override
+ protected void runInContext() {
+ final GlobalLock gcLock = GlobalLock.getInternLock("ConsoleSession.Cleanup.Lock");
+ try {
+ if (gcLock.lock(3)) {
+ try {
+ reallyRun();
+ } finally {
+ gcLock.unlock();
+ }
+ }
+ } finally {
+ gcLock.releaseRef();
+ }
+ }
+
+ private void reallyRun() {
+ s_logger.info("Starting ConsoleSessionCleanupTask...");
+ Integer retentionDays = ConsoleAccessManager.ConsoleSessionCleanupRetentionDays.value();
+ Date date = GregorianCalendar.getInstance().getTime();
Review Comment:
Why use `GregorianCalendar.getInstance().getTime()` instead of a `new Date()` here?
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -100,9 +111,53 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
ConsoleAccessManagerImpl.secretKeysManager = keysManager;
+ executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("ConsoleSession-Scavenger"));
return super.configure(name, params);
}
+ @Override
+ public boolean start() {
+ Integer consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value();
+ Boolean consoleCleanupEnabled = ConsoleAccessManager.ConsoleSessionCleanupEnabled.value();
+ if (BooleanUtils.isTrue(consoleCleanupEnabled)) {
+ s_logger.info(String.format("The ConsoleSessionCleanupTask will run every %s hours", consoleCleanupInterval));
+ executorService.scheduleWithFixedDelay(new ConsoleSessionCleanupTask(), consoleCleanupInterval, consoleCleanupInterval, TimeUnit.HOURS);
+ }
+ return true;
+ }
+
+ public class ConsoleSessionCleanupTask extends ManagedContextRunnable {
+ @Override
+ protected void runInContext() {
+ final GlobalLock gcLock = GlobalLock.getInternLock("ConsoleSession.Cleanup.Lock");
+ try {
+ if (gcLock.lock(3)) {
+ try {
+ reallyRun();
+ } finally {
+ gcLock.unlock();
+ }
+ }
+ } finally {
+ gcLock.releaseRef();
+ }
+ }
+
+ private void reallyRun() {
+ s_logger.info("Starting ConsoleSessionCleanupTask...");
+ Integer retentionDays = ConsoleAccessManager.ConsoleSessionCleanupRetentionDays.value();
+ Date date = GregorianCalendar.getInstance().getTime();
+ Date dateBefore = new Date(date.getTime() - retentionDays * 24 * 3600 * 1000);
+ List<ConsoleSessionVO> sessionsToExpunge = consoleSessionDao.listRemovedSessionsOlderThanDate(dateBefore);
+ if (CollectionUtils.isNotEmpty(sessionsToExpunge)) {
+ s_logger.info(String.format("Expunging %s removed console session records"));
Review Comment:
An argument is missing for the format.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1408620503
@rohityadavcloud a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #7132: Add console session cleanup task
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1086613497
##########
api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java:
##########
@@ -18,9 +18,31 @@
import com.cloud.utils.component.Manager;
import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint;
+import org.apache.cloudstack.framework.config.ConfigKey;
public interface ConsoleAccessManager extends Manager {
+ ConfigKey<Boolean> ConsoleSessionCleanupEnabled = new ConfigKey<>("Advanced", Boolean.class,
+ "console.session.cleanup.enabled",
+ "true",
+ "Enables/disables the console session cleanup thread.",
+ true,
+ ConfigKey.Scope.Global);
+
+ ConfigKey<Integer> ConsoleSessionCleanupRetentionDays = new ConfigKey<>("Advanced", Integer.class,
+ "console.session.cleanup.retention.days",
Review Comment:
maybe both in hours ?
##########
api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java:
##########
@@ -18,9 +18,31 @@
import com.cloud.utils.component.Manager;
import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint;
+import org.apache.cloudstack.framework.config.ConfigKey;
public interface ConsoleAccessManager extends Manager {
+ ConfigKey<Boolean> ConsoleSessionCleanupEnabled = new ConfigKey<>("Advanced", Boolean.class,
+ "console.session.cleanup.enabled",
+ "true",
+ "Enables/disables the console session cleanup thread.",
+ true,
Review Comment:
If we change the values, we need to restart cloudstack management server, right ?
if yes, these configurations are not dynamic.
##########
engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java:
##########
@@ -19,11 +19,24 @@
package com.cloud.vm.dao;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
import com.cloud.vm.ConsoleSessionVO;
import com.cloud.utils.db.GenericDaoBase;
+import java.util.Date;
+import java.util.List;
+
public class ConsoleSessionDaoImpl extends GenericDaoBase<ConsoleSessionVO, Long> implements ConsoleSessionDao {
+ private final SearchBuilder<ConsoleSessionVO> SearchByRemovedDate;
+
+ public ConsoleSessionDaoImpl() {
+ SearchByRemovedDate = createSearchBuilder();
+ SearchByRemovedDate.and("removed", SearchByRemovedDate.entity().getRemoved(), SearchCriteria.Op.NNULL);
Review Comment:
is this an issue that two conditions have the same key "removed" ?
may be better to rename key in line 36 to "removedIsNotNull" (just an example)
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -100,9 +111,53 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
ConsoleAccessManagerImpl.secretKeysManager = keysManager;
+ executorService = Executors.newScheduledThreadPool(1, new NamedThreadFactory("ConsoleSession-Scavenger"));
return super.configure(name, params);
}
+ @Override
+ public boolean start() {
+ Integer consoleCleanupInterval = ConsoleAccessManager.ConsoleSessionCleanupInterval.value();
+ Boolean consoleCleanupEnabled = ConsoleAccessManager.ConsoleSessionCleanupEnabled.value();
+ if (BooleanUtils.isTrue(consoleCleanupEnabled)) {
+ s_logger.info(String.format("The ConsoleSessionCleanupTask will run every %s hours", consoleCleanupInterval));
+ executorService.scheduleWithFixedDelay(new ConsoleSessionCleanupTask(), consoleCleanupInterval, consoleCleanupInterval, TimeUnit.HOURS);
+ }
+ return true;
+ }
+
+ public class ConsoleSessionCleanupTask extends ManagedContextRunnable {
+ @Override
+ protected void runInContext() {
+ final GlobalLock gcLock = GlobalLock.getInternLock("ConsoleSession.Cleanup.Lock");
+ try {
+ if (gcLock.lock(3)) {
+ try {
+ reallyRun();
+ } finally {
+ gcLock.unlock();
+ }
+ }
+ } finally {
+ gcLock.releaseRef();
+ }
+ }
+
+ private void reallyRun() {
+ s_logger.info("Starting ConsoleSessionCleanupTask...");
+ Integer retentionDays = ConsoleAccessManager.ConsoleSessionCleanupRetentionDays.value();
+ Date date = GregorianCalendar.getInstance().getTime();
+ Date dateBefore = new Date(date.getTime() - retentionDays * 24 * 3600 * 1000);
+ List<ConsoleSessionVO> sessionsToExpunge = consoleSessionDao.listRemovedSessionsOlderThanDate(dateBefore);
+ if (CollectionUtils.isNotEmpty(sessionsToExpunge)) {
Review Comment:
I think this could be replaced by a method in ConsoleSessionDao which calls "dao.expunge(sc)".
It can reduce the number of write operations to DB.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #7132: Add console session cleanup task
Posted by sonarcloud.
sonarcloud[bot] commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1403917694
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=7132)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7132&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7132&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list) [1.1% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list) [14.3% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7132&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7132: Add console session cleanup task
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1404689763
@nvazquez can you add some testing info to the description please?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7132: Add console session cleanup task
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#issuecomment-1408538611
@GutoVeronezi a Jenkins job has been kicked to build packages. It will be bundled with
@rohityadavcloud, I am building the code right now to test it. SystemVM template(s). I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org