Add console session cleanup task by nvazquez · Pull Request #7132 · apache/cloudstack

nvazquez · 2023-01-25T12:51:48Z

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

Screenshots (if appropriate):

How Has This Been Tested?

nvazquez · 2023-01-25T12:55:15Z

@blueorangutan package

blueorangutan · 2023-01-25T12:56:03Z

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

DaanHoogland

looks mostly good,
one debatable remark/implementation style discusion to be had.

api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java

server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java

api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java

engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java

server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java

blueorangutan · 2023-01-25T14:01:00Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5401

nvazquez · 2023-01-25T15:49:40Z

Thanks @DaanHoogland @weizhouapache @GutoVeronezi - I've addressed your review comments

nvazquez · 2023-01-25T15:49:50Z

@blueorangutan package

blueorangutan · 2023-01-25T15:50:04Z

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

server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java

blueorangutan · 2023-01-25T16:44:56Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5406

nvazquez · 2023-01-25T17:01:23Z

@blueorangutan package

blueorangutan · 2023-01-25T17:02:03Z

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

blueorangutan · 2023-01-25T17:54:24Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5407

weizhouapache

Code LGTM

Not tested yet

nvazquez · 2023-01-25T18:35:30Z

@blueorangutan package

blueorangutan · 2023-01-25T18:36:04Z

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

codecov · 2023-01-25T18:59:07Z

Codecov Report

Merging #7132 (6a41242) into main (4d41b6b) will increase coverage by 5.29%.
The diff coverage is n/a.

❗ Current head 6a41242 differs from pull request most recent head 30da7a9. Consider uploading reports for the commit 30da7a9 to get more accurate results

@@              Coverage Diff              @@
##               main    #7132       +/-   ##
=============================================
+ Coverage      6.56%   11.86%    +5.29%     
+ Complexity     8441     7734      -707     
=============================================
  Files          4361     2512     -1849     
  Lines        370925   246872   -124053     
  Branches      47614    38502     -9112     
=============================================
+ Hits          24360    29289     +4929     
+ Misses       343630   213729   -129901     
- Partials       2935     3854      +919

Impacted Files	Coverage Δ
...va/com/cloud/upgrade/dao/DatabaseAccessObject.java	`0.00% <0.00%> (-97.92%)`	⬇️
...ain/java/com/cloud/upgrade/dao/DbUpgradeUtils.java	`0.00% <0.00%> (-93.75%)`	⬇️
...java/com/cloud/upgrade/DatabaseUpgradeChecker.java	`0.00% <0.00%> (-41.25%)`	⬇️
.../com/cloud/storage/dao/StoragePoolTagsDaoImpl.java	`12.90% <0.00%> (-31.70%)`	⬇️
...va/com/cloud/upgrade/DatabaseVersionHierarchy.java	`56.81% <0.00%> (-29.12%)`	⬇️
...rce/wrapper/LibvirtResizeVolumeCommandWrapper.java	`49.50% <0.00%> (-28.63%)`	⬇️
...n/java/com/cloud/upgrade/dao/Upgrade490to4910.java	`0.00% <0.00%> (-26.93%)`	⬇️
...in/java/com/cloud/upgrade/dao/LegacyDbUpgrade.java	`0.00% <0.00%> (-25.00%)`	⬇️
...in/java/com/cloud/upgrade/dao/Upgrade444to450.java	`0.00% <0.00%> (-25.00%)`	⬇️
...in/java/com/cloud/upgrade/dao/Upgrade453to460.java	`0.00% <0.00%> (-25.00%)`	⬇️
... and 3746 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

blueorangutan · 2023-01-25T19:28:27Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5409

DaanHoogland

clgtm

nvazquez · 2023-01-31T10:35:49Z

Thanks for reviewing @GutoVeronezi @DaanHoogland, conflicts fixed
@blueorangutan package

blueorangutan · 2023-01-31T10:36:03Z

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

blueorangutan · 2023-01-31T11:34:23Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5456

nvazquez · 2023-01-31T12:06:47Z

@blueorangutan test

blueorangutan · 2023-01-31T12:07:03Z

@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

nvazquez · 2023-01-31T15:43:19Z

@blueorangutan package

blueorangutan · 2023-01-31T15:44:03Z

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

blueorangutan · 2023-01-31T16:46:50Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5461

sonarqubecloud · 2023-01-31T16:59:04Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.4% Coverage
6.7% Duplication

nvazquez · 2023-01-31T22:03:44Z

@blueorangutan test

blueorangutan · 2023-01-31T22:04:03Z

@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2023-02-01T00:19:17Z

Trillian test result (tid-6041)
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

nvazquez · 2023-02-01T01:52:05Z

@blueorangutan test

blueorangutan · 2023-02-01T01:53:04Z

@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2023-02-01T09:51:21Z

Trillian test result (tid-6057)
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

DaanHoogland · 2023-02-01T13:37:12Z

@GutoVeronezi is this lgty now?

blueorangutan · 2023-02-01T14:14:36Z

Trillian test result (tid-6058)
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

yadvr · 2023-02-01T14:36:59Z

Tests LGTM.

GutoVeronezi · 2023-02-01T14:44:48Z

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

cc: @nvazquez

DaanHoogland · 2023-02-01T14:50:03Z

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

cc: @nvazquez

right, I knew there was a reason I didn´t merge this yet ;)

nvazquez · 2023-02-01T15:01:29Z

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

weizhouapache · 2023-02-01T15:19:39Z

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 ?

GutoVeronezi · 2023-02-01T15:35:37Z

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

Add console session cleanup task

2875edc

nvazquez requested review from DaanHoogland and GutoVeronezi January 25, 2023 12:51

boring-cyborg bot added the component:api label Jan 25, 2023

nvazquez mentioned this pull request Jan 25, 2023

Handle console session in multiple management servers #7094

Merged

10 tasks

DaanHoogland reviewed Jan 25, 2023

View reviewed changes

weizhouapache reviewed Jan 25, 2023

View reviewed changes

GutoVeronezi reviewed Jan 25, 2023

View reviewed changes

server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java Outdated Show resolved Hide resolved

server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java Outdated Show resolved Hide resolved

Address review

2bb9d55

weizhouapache reviewed Jan 25, 2023

View reviewed changes

server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java Show resolved Hide resolved

Add missing config keys

9e103c9

weizhouapache approved these changes Jan 25, 2023

View reviewed changes

Fix sonar cloud reports

3e24f81

In progress fix console load report

1b50ed7

boring-cyborg bot added the component:console-proxy label Jan 26, 2023

DaanHoogland approved these changes Jan 26, 2023

View reviewed changes

Improve logs

6a41242

github-actions bot removed the status:has-conflicts label Jan 31, 2023

Fix code smells from sonarcloud

30da7a9

nvazquez mentioned this pull request Feb 1, 2023

Improve the console session acquired field #7159

Closed

GutoVeronezi approved these changes Feb 1, 2023

View reviewed changes

DaanHoogland merged commit 89bf475 into apache:main Feb 1, 2023

nvazquez deleted the addconsolesessioncleanuptask branch February 15, 2023 00:56

Conversation

nvazquez commented Jan 25, 2023

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

nvazquez commented Jan 25, 2023

Uh oh!

blueorangutan commented Jan 25, 2023

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

blueorangutan commented Jan 25, 2023

Uh oh!

nvazquez commented Jan 25, 2023

Uh oh!

nvazquez commented Jan 25, 2023

Uh oh!

blueorangutan commented Jan 25, 2023

Uh oh!

Uh oh!

blueorangutan commented Jan 25, 2023

Uh oh!

nvazquez commented Jan 25, 2023

Uh oh!

blueorangutan commented Jan 25, 2023

Uh oh!

blueorangutan commented Jan 25, 2023

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

nvazquez commented Jan 25, 2023

Uh oh!

blueorangutan commented Jan 25, 2023

Uh oh!

codecov bot commented Jan 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

blueorangutan commented Jan 25, 2023

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

nvazquez commented Jan 31, 2023

Uh oh!

blueorangutan commented Jan 31, 2023

Uh oh!

blueorangutan commented Jan 31, 2023

Uh oh!

nvazquez commented Jan 31, 2023

Uh oh!

blueorangutan commented Jan 31, 2023

Uh oh!

nvazquez commented Jan 31, 2023

Uh oh!

blueorangutan commented Jan 31, 2023

Uh oh!

blueorangutan commented Jan 31, 2023

Uh oh!

sonarqubecloud bot commented Jan 31, 2023

Uh oh!

nvazquez commented Jan 31, 2023

Uh oh!

blueorangutan commented Jan 31, 2023

codecov bot commented Jan 25, 2023 •

edited

Loading