cpvm: Fix CPVM not releasing CPU after closing console by JoaoJandre · Pull Request #7826 · apache/cloudstack

JoaoJandre · 2023-08-07T18:59:46Z

Description

When using VNC over Nio socket, even after closing the console, the CPVM stays in a while true and will not stop using resources until a restart. This PR introduces a new verification to check if the session is still alive.

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?

JoaoJandre · 2023-08-07T19:00:34Z

I'm still validating what happens when using vnc over websocket

codecov · 2023-08-07T20:34:44Z

Codecov Report

Merging #7826 (b7820b3) into 4.18 (a47a4f4) will increase coverage by 0.01%.
Report is 6 commits behind head on 4.18.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               4.18    #7826      +/-   ##
============================================
+ Coverage     13.02%   13.04%   +0.01%     
- Complexity     9040     9067      +27     
============================================
  Files          2720     2720              
  Lines        257094   257234     +140     
  Branches      40092    40102      +10     
============================================
+ Hits          33491    33552      +61     
- Misses       219398   219472      +74     
- Partials       4205     4210       +5

see 9 files with indirect coverage changes

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

yadvr · 2023-08-08T09:21:49Z

@JoaoJandre can you raise this for 4.18 branch? I hit this issue with VMware recently with 4.18 but didn't know what caused it.

yadvr · 2023-08-08T13:33:42Z

@JoaoJandre could you also check #6970 cc @alexandru-bagu - if this is a better fix?

cc @DaanHoogland @weizhouapache

JoaoJandre · 2023-08-08T17:42:03Z

@JoaoJandre could you also check #6970 cc @alexandru-bagu - if this is a better fix?

@rohityadavcloud, actually there are two adjacent problems happening in the CPVM: the first one is the high CPU consumption on the CPVM; and the second one is processes not being released after closing the console session. This PR aims to fix the second situation; and PR #6970 aims to fix the first situation; therefore, I think that both issues should be treated in separated PRs (as it is currently).

yadvr · 2023-08-08T18:31:49Z

Thanks for replying that makes sense. In that case we should aspire to get both merged.

@blueorangutan package

blueorangutan · 2023-08-08T18:32:03Z

@rohityadavcloud a [SF] 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-08-08T19:36:43Z

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

yadvr · 2023-08-09T15:35:55Z

Manual test on all supported hypervisors types may be needed cc @weizhouapache
@blueorangutan test

blueorangutan · 2023-08-09T15:38:06Z

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

nvazquez

Code LGTM - thanks @JoaoJandre

blueorangutan · 2023-08-10T05:49:31Z

[SF] Trillian test result (tid-7342)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 49470 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7826-t7342-kvm-centos7.zip
Smoke tests completed. 107 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_migrate_VM_and_root_volume	`Error`	79.74	test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks	`Error`	55.42	test_vm_life_cycle.py

DaanHoogland · 2023-08-10T07:32:33Z

code looks good @JoaoJandre , this is still work in progress?

JoaoJandre · 2023-08-10T18:51:09Z

code looks good @JoaoJandre , this is still work in progress?

I was just doing some testing, but the PR is ready now

github-actions · 2023-08-14T14:13:31Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

…-closing-connection

JoaoJandre · 2023-08-14T19:32:55Z

@JoaoJandre can you fix the conflicts ? thanks

Done

DaanHoogland · 2023-08-15T06:49:51Z

@blueorangutan package

blueorangutan · 2023-08-15T06:50:06Z

@DaanHoogland a [SF] 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-08-15T07:55:50Z

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

DaanHoogland · 2023-08-15T08:09:56Z

@blueorangutan test rocky8 kvm-rocky8 keepEnv

yadvr · 2023-08-15T14:52:02Z

I'm able to reproduce the issue with 4.18, restarting the cloud service on cpvm fixes this as a workaround.
PR LGTM, left some comments - but otherwise we should merge this.

DaanHoogland · 2023-08-17T07:35:59Z

@blueorangutan test rocky8 kvm-rocky8 keepEnv

blueorangutan · 2023-08-17T07:38:04Z

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

weizhouapache · 2023-08-17T20:26:18Z

@blueorangutan package

weizhouapache

thanks @JoaoJandre
code lgtm

blueorangutan · 2023-08-17T20:52:48Z

[SF] Trillian test result (tid-7457)
Environment: kvm-rocky8 (x2), Advanced Networking with Mgmt server r8
Total time taken: 45588 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7826-t7457-kvm-rocky8.zip
Smoke tests completed. 107 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_invalid_upgrade_kubernetes_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_02_upgrade_kubernetes_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_04_autoscale_kubernetes_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_05_basic_lifecycle_kubernetes_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_06_delete_kubernetes_cluster	`Failure`	0.00	test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_10_vpc_tier_kubernetes_cluster	`Failure`	0.01	test_kubernetes_clusters.py

weizhouapache · 2023-08-17T22:11:48Z

@blueorangutan package

blueorangutan · 2023-08-17T22:12:03Z

@weizhouapache a [SF] 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-08-17T23:05:18Z

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

DaanHoogland

tested looks good, load is immediately decreasing after closing the console:

boring-cyborg Bot added the component:console-proxy label Aug 7, 2023

GutoVeronezi requested a review from nvazquez August 7, 2023 19:38

yadvr added this to the 4.18.1.0 milestone Aug 8, 2023

yadvr added the type:bug label Aug 8, 2023

yadvr changed the base branch from main to 4.18 August 8, 2023 10:28

boring-cyborg Bot added component:agent component:api component:build component:integration labels Aug 8, 2023

yadvr changed the base branch from 4.18 to main August 8, 2023 10:28

yadvr added the Severity:Critical Critical bug label Aug 8, 2023

JoaoJandre changed the base branch from main to 4.18 August 8, 2023 17:34

Fix websocket

128852e

JoaoJandre force-pushed the fix-console-proxy-not-closing-connection branch from 140f033 to 128852e Compare August 8, 2023 17:39

nvazquez approved these changes Aug 9, 2023

View reviewed changes

JoaoJandre marked this pull request as ready for review August 10, 2023 18:49

add some logs

f80deb7

github-actions Bot added the status:has-conflicts label Aug 14, 2023

JoaoJandre added 2 commits August 14, 2023 14:49

Merge remote-tracking branch 'origin/4.18' into fix-console-proxy-not…

813d094

…-closing-connection

fix pokemon catch

595bdfb

github-actions Bot removed the status:has-conflicts label Aug 14, 2023

weizhouapache reviewed Aug 15, 2023

View reviewed changes

Comment thread services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java Outdated

yadvr reviewed Aug 15, 2023

View reviewed changes

Comment thread services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java

yadvr reviewed Aug 15, 2023

View reviewed changes

Comment thread services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java

yadvr approved these changes Aug 15, 2023

View reviewed changes

JoaoJandre added 2 commits August 15, 2023 14:26

address Wei's review

a0bb491

remove unnecessary import

8b5cd4d

weizhouapache requested a review from DaanHoogland August 17, 2023 11:26

address wei's reviews

b7820b3

weizhouapache approved these changes Aug 17, 2023

View reviewed changes

DaanHoogland approved these changes Aug 18, 2023

View reviewed changes

DaanHoogland merged commit f7345e8 into apache:4.18 Aug 18, 2023

JoaoJandre mentioned this pull request Aug 23, 2023

engine/schema: fix CPU cap limitation for upgraded environment #7808

Merged

12 tasks

Conversation

JoaoJandre commented Aug 7, 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!

JoaoJandre commented Aug 7, 2023

Uh oh!

codecov Bot commented Aug 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yadvr commented Aug 8, 2023

Uh oh!

yadvr commented Aug 8, 2023

Uh oh!

JoaoJandre commented Aug 8, 2023

Uh oh!

yadvr commented Aug 8, 2023

Uh oh!

blueorangutan commented Aug 8, 2023

Uh oh!

blueorangutan commented Aug 8, 2023

Uh oh!

yadvr commented Aug 9, 2023

Uh oh!

blueorangutan commented Aug 9, 2023

Uh oh!

nvazquez left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Aug 10, 2023

Uh oh!

DaanHoogland commented Aug 10, 2023

Uh oh!

JoaoJandre commented Aug 10, 2023

Uh oh!

github-actions Bot commented Aug 14, 2023

Uh oh!

JoaoJandre commented Aug 14, 2023

Uh oh!

DaanHoogland commented Aug 15, 2023

Uh oh!

blueorangutan commented Aug 15, 2023

Uh oh!

blueorangutan commented Aug 15, 2023

Uh oh!

DaanHoogland commented Aug 15, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yadvr commented Aug 15, 2023

Uh oh!

DaanHoogland commented Aug 17, 2023

Uh oh!

blueorangutan commented Aug 17, 2023

Uh oh!

weizhouapache commented Aug 17, 2023

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Aug 17, 2023

Uh oh!

weizhouapache commented Aug 17, 2023

Uh oh!

blueorangutan commented Aug 17, 2023

Uh oh!

blueorangutan commented Aug 17, 2023

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

codecov Bot commented Aug 7, 2023 •

edited

Loading

nvazquez left a comment •

edited

Loading