Skip to content

Add console session cleanup task - #7132

Merged
DaanHoogland merged 10 commits into
apache:mainfrom
shapeblue:addconsolesessioncleanuptask
Feb 1, 2023
Merged

Add console session cleanup task#7132
DaanHoogland merged 10 commits into
apache:mainfrom
shapeblue:addconsolesessioncleanuptask

Conversation

@nvazquez

Copy link
Copy Markdown
Contributor

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

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

@nvazquez

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@DaanHooglandDaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment threadengine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java Outdated
@blueorangutan

Copy link
Copy Markdown

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

@nvazquez

Copy link
Copy Markdown
ContributorAuthor

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

@nvazquez

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@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

Copy link
Copy Markdown

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

@nvazquez

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@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

Copy link
Copy Markdown

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

@weizhouapacheweizhouapache left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM

Not tested yet

@nvazquez

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@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

codecovBot commented Jan 25, 2023

Copy link
Copy Markdown

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 FilesCoverage Δ
...va/com/cloud/upgrade/dao/DatabaseAccessObject.java0.00% <0.00%> (-97.92%)⬇️
...ain/java/com/cloud/upgrade/dao/DbUpgradeUtils.java0.00% <0.00%> (-93.75%)⬇️
...java/com/cloud/upgrade/DatabaseUpgradeChecker.java0.00% <0.00%> (-41.25%)⬇️
.../com/cloud/storage/dao/StoragePoolTagsDaoImpl.java12.90% <0.00%> (-31.70%)⬇️
...va/com/cloud/upgrade/DatabaseVersionHierarchy.java56.81% <0.00%> (-29.12%)⬇️
...rce/wrapper/LibvirtResizeVolumeCommandWrapper.java49.50% <0.00%> (-28.63%)⬇️
...n/java/com/cloud/upgrade/dao/Upgrade490to4910.java0.00% <0.00%> (-26.93%)⬇️
...in/java/com/cloud/upgrade/dao/LegacyDbUpgrade.java0.00% <0.00%> (-25.00%)⬇️
...in/java/com/cloud/upgrade/dao/Upgrade444to450.java0.00% <0.00%> (-25.00%)⬇️
...in/java/com/cloud/upgrade/dao/Upgrade453to460.java0.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

Copy link
Copy Markdown

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

@DaanHooglandDaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

@nvazquez

Copy link
Copy Markdown
ContributorAuthor

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

@blueorangutan

Copy link
Copy Markdown

@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

Copy link
Copy Markdown

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

@nvazquez

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@nvazquez

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@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

Copy link
Copy Markdown

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

@sonarqubecloud

Copy link
Copy Markdown

@nvazquez

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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:

TestResultTime (s)Test File
test_01_add_primary_storage_disabled_hostError0.64test_primary_storage.py
test_01_primary_storage_nfsError0.12test_primary_storage.py
ContextSuite context=TestStorageTags>:setupError0.21test_primary_storage.py
test_01_non_strict_host_anti_affinityFailure106.63test_nonstrict_affinity_group.py
test_02_non_strict_host_affinityError80.04test_nonstrict_affinity_group.py
test_03_deploy_and_scale_kubernetes_clusterFailure35.99test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_clusterFailure61.47test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_clusterFailure42.05test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_clusterFailure38.95test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardownError126.71test_kubernetes_clusters.py
test_01_secure_vm_migrationError156.21test_vm_life_cycle.py
test_02_unsecure_vm_migrationError268.85test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migrationError140.72test_vm_life_cycle.py
test_08_migrate_vmError43.77test_vm_life_cycle.py
test_hostha_enable_ha_when_host_in_maintenanceError304.90test_hostha_kvm.py

@nvazquez

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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:

TestResultTime (s)Test File

@DaanHoogland

Copy link
Copy Markdown
Contributor

@GutoVeronezi is this lgty now?

@blueorangutan

Copy link
Copy Markdown

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:

TestResultTime (s)Test File

@yadvr

yadvr commented Feb 1, 2023

Copy link
Copy Markdown
Member

Tests LGTM.

@GutoVeronezi

Copy link
Copy Markdown
Contributor

@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

Copy link
Copy Markdown
Contributor

@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

Copy link
Copy Markdown
ContributorAuthor

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

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor

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

@DaanHoogland
DaanHoogland merged commit 89bf475 into apache:mainFeb 1, 2023
@nvazquez
nvazquez deleted the addconsolesessioncleanuptask branch February 15, 2023 00:56
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@nvazquez@blueorangutan@DaanHoogland@yadvr@GutoVeronezi@weizhouapache