vm-import: fix stopped managed vms listing in unmanaged instances #7606

shwstppr · 2023-06-08T07:08:32Z

Description

When listing unmanaged instances for a cluster with multiple hosts, stopped manager VMs are getting listed as well. This PR fixes this behaviour and adds some code refactoring

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?

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

codecov · 2023-06-08T08:07:32Z

Codecov Report

Merging #7606 (3470230) into 4.18 (29c7b31) will increase coverage by 0.04%.
Report is 70 commits behind head on 4.18.
The diff coverage is 19.77%.

@@             Coverage Diff              @@
##               4.18    #7606      +/-   ##
============================================
+ Coverage     13.02%   13.06%   +0.04%     
- Complexity     9032     9109      +77     
============================================
  Files          2720     2720              
  Lines        257080   257535     +455     
  Branches      40088    40153      +65     
============================================
+ Hits          33476    33654     +178     
- Misses       219400   219654     +254     
- Partials       4204     4227      +23

Files	Coverage Δ
...hestration/service/VolumeOrchestrationService.java	`100.00% <ø> (ø)`
.../main/java/com/cloud/network/IpAddressManager.java	`100.00% <100.00%> (ø)`
...ava/com/cloud/network/as/AutoScaleVmProfileVO.java	`80.20% <100.00%> (+11.66%)`	⬆️
...java/com/cloud/upgrade/DatabaseUpgradeChecker.java	`40.89% <100.00%> (+0.64%)`	⬆️
...va/com/cloud/upgrade/DatabaseVersionHierarchy.java	`85.10% <100.00%> (+1.01%)`	⬆️
.../api/command/admin/ratelimit/ResetApiLimitCmd.java	`0.00% <ø> (ø)`
...oud/hypervisor/kvm/resource/LibvirtConnection.java	`0.00% <ø> (ø)`
.../hypervisor/kvm/storage/ScaleIOStorageAdaptor.java	`10.44% <100.00%> (ø)`
...ava/com/cloud/api/commands/StopNetScalerVMCmd.java	`0.00% <ø> (ø)`
...tungsten/api/command/ListTungstenFabricTagCmd.java	`0.00% <ø> (ø)`
... and 59 more

... and 7 files with indirect coverage changes

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

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

…stance-stopped

DaanHoogland · 2023-06-08T13:00:42Z

haven´t looked at the code thouroughly but looks good so far. I wonder about the functionality though @shwstppr ; as a user I would want to be able to import stopped VMs as well. Is this harmful/impossible and is this why you want to prevent the listing?

shwstppr · 2023-06-09T03:35:14Z

@DaanHoogland maybe the title and description wasn't that clear. This PR is trying to fix an issue where stopped managed VMs are getting listed as unmanaged instances

DaanHoogland · 2023-06-09T08:18:23Z

@DaanHoogland maybe the title and description wasn't that clear. This PR is trying to fix an issue where stopped managed VMs are getting listed as unmanaged instances

ok, tnx

shwstppr · 2023-06-25T08:41:04Z

@blueorangutan package

blueorangutan · 2023-06-25T08:42:03Z

@shwstppr 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-06-25T09:37:08Z

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

shwstppr · 2023-06-25T19:58:59Z

@blueorangutan test rocky8 vmware-67u3

DaanHoogland

clgtm

DaanHoogland · 2023-06-26T08:51:22Z

server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java

-                    UnmanagedInstanceTO unmanagedInstance = unmanagedInstances.get(name);
-                    if (unmanagedInstance == null) {
-                        throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Unable to retrieve details for unmanaged VM: %s", name));
+                if (!instanceName.equals(name)) {


we can extract the larger part of this block, from the continue here and the break just before the end of the loop.

rohityadavcloud · 2023-08-08T13:35:34Z

@shwstppr - is this ready for review?
@blueorangutan package

blueorangutan · 2023-08-08T13:36:05Z

@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-08T14:45:18Z

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

shwstppr · 2023-08-09T05:16:07Z

@rohityadavcloud yes, marked it as ready for review

DaanHoogland · 2023-08-10T06:37:24Z

@blueorangutan test

blueorangutan · 2023-08-10T06:38:03Z

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

weizhouapache

code lgtm

blueorangutan · 2023-08-10T18:48:42Z

[SF] Trillian test result (tid-7355)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42162 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7606-t7355-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`	80.83	test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks	`Error`	53.39	test_vm_life_cycle.py

DaanHoogland · 2023-08-18T13:23:55Z

tested manually @shwstppr , there was one circumstance in which a VM was visible both as managed as well as unmanaged:

New VM created in cloudstack
unmanaged
stopped in vcenter
imported
now it is visible on both lists of the dialog:

after starting it in cloudstack it is no longer visible as unmanaged:

All other combinations of stop/start seem to display correctly

shwstppr · 2023-08-18T14:22:59Z

@DaanHoogland thanks for the testing. Will have to check. Weird that an unmanaged VM is showing in the managed list

blueorangutan · 2023-08-23T12:01:43Z

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

DaanHoogland · 2023-08-23T15:39:29Z

@blueorangutan test rocky8 vmware-70u3 keepEnv

blueorangutan · 2023-08-23T15:40:03Z

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

blueorangutan · 2023-08-24T04:03:36Z

[SF] Trillian test result (tid-7525)
Environment: vmware-70u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 42816 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7606-t7525-vmware-70u3.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_02_list_cpvm_vm	`Failure`	0.04	test_ssvm.py
test_04_cpvm_internals	`Failure`	0.04	test_ssvm.py

DaanHoogland · 2023-08-24T09:11:48Z

@shwstppr still a little snag (I don't know if you already gave that attention)

note i-2-431-VM is visible on both sides, This is the case where I stopped on ACS unmanaged and imported again. At first it is not visible on the left side, but on pressing refresh it appears.

rohityadavcloud

LGTM didn't test it though

DaanHoogland · 2023-10-02T13:32:16Z

@shwstppr , is it at all possible to fix the issue I found with this? Should we close and revisit at a later time?

shwstppr · 2023-10-02T15:44:36Z

@DaanHoogland will investigate this week and update

shwstppr · 2023-10-10T06:58:02Z

@blueorangutan package

blueorangutan · 2023-10-10T07:00:04Z

@shwstppr 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-10-10T07:59:24Z

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

…stance-stopped

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr · 2023-10-10T10:21:58Z

@DaanHoogland issue should be fixed now

post-7606-fix.mp4

@blueorangutan package

blueorangutan · 2023-10-10T10:24:03Z

@shwstppr 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-10-10T11:37:46Z

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

DaanHoogland · 2023-10-10T12:10:33Z

great @shwstppr , the issue is gone indeed.

DaanHoogland · 2023-10-10T12:10:51Z

@blueorangutan test rocky8 vmware-67u3

blueorangutan · 2023-10-10T12:12:03Z

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

blueorangutan · 2023-10-11T05:58:55Z

[SF] Trillian test result (tid-7906)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 62455 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7606-t7906-vmware-67u3.zip
Smoke tests completed. 106 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_08_upgrade_kubernetes_ha_cluster	`Failure`	980.27	test_kubernetes_clusters.py
test_01_deploy_vm_on_specific_host	`Error`	3602.47	test_vm_deployment_planner.py
test_02_deploy_vm_on_specific_cluster	`Error`	17.75	test_vm_deployment_planner.py
test_03_deploy_vm_on_specific_pod	`Error`	3602.31	test_vm_deployment_planner.py
test_04_deploy_vm_on_host_override_pod_and_cluster	`Error`	18.90	test_vm_deployment_planner.py
test_05_deploy_vm_on_cluster_override_pod	`Error`	3601.95	test_vm_deployment_planner.py

DaanHoogland · 2023-10-11T08:48:26Z

@blueorangutan test alma8 vmware-70u3

blueorangutan · 2023-10-11T08:50:03Z

@DaanHoogland a [SF] Trillian-Jenkins test job (alma8 mgmt + vmware-70u3) has been kicked to run smoke tests

blueorangutan · 2023-10-11T23:01:23Z

[SF] Trillian test result (tid-7915)
Environment: vmware-70u3 (x2), Advanced Networking with Mgmt server a8
Total time taken: 49342 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7606-t7915-vmware-70u3.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

shwstppr added 2 commits June 8, 2023 12:30

vm-import: fix stopped vms listing in unmanaged instances

c3582eb

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

fix listing vms for cluster

6aff5d4

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr added 2 commits June 8, 2023 13:43

fix

672a8cb

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

Merge remote-tracking branch 'apache/4.18' into fix-list-unmanaged-in…

283c615

…stance-stopped

shwstppr changed the title ~~vm-import: fix stopped vms listing in unmanaged instances~~ vm-import: fix stopped managed vms listing in unmanaged instances Jun 9, 2023

weizhouapache added this to the 4.18.1.0 milestone Jun 20, 2023

DaanHoogland approved these changes Jun 26, 2023

View reviewed changes

weizhouapache added the Severity:Minor label Jun 26, 2023

Merge branch '4.18' into fix-list-unmanaged-instance-stopped

5c45491

shwstppr marked this pull request as ready for review August 9, 2023 05:15

weizhouapache approved these changes Aug 10, 2023

View reviewed changes

weizhouapache assigned DaanHoogland Aug 15, 2023

weizhouapache added the status:needs-testing label Aug 15, 2023

DaanHoogland removed their assignment Aug 25, 2023

weizhouapache modified the milestones: 4.18.1.0, 4.18.2.0 Aug 28, 2023

rohityadavcloud approved these changes Sep 6, 2023

View reviewed changes

shwstppr added 2 commits October 10, 2023 15:32

Merge remote-tracking branch 'apache/4.18' into fix-list-unmanaged-in…

f480707

…stance-stopped

fix issue

3470230

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

DaanHoogland self-assigned this Oct 10, 2023

DaanHoogland removed their assignment Oct 10, 2023

DaanHoogland merged commit 3e7f21a into apache:4.18 Oct 12, 2023
26 of 27 checks passed

DaanHoogland deleted the fix-list-unmanaged-instance-stopped branch October 12, 2023 07:21

vm-import: fix stopped managed vms listing in unmanaged instances #7606

vm-import: fix stopped managed vms listing in unmanaged instances #7606

Conversation

shwstppr commented Jun 8, 2023 • edited

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

codecov bot commented Jun 8, 2023 • edited

Codecov Report

DaanHoogland commented Jun 8, 2023

shwstppr commented Jun 9, 2023

DaanHoogland commented Jun 9, 2023

shwstppr commented Jun 25, 2023

blueorangutan commented Jun 25, 2023

blueorangutan commented Jun 25, 2023

shwstppr commented Jun 25, 2023

DaanHoogland left a comment

Choose a reason for hiding this comment

DaanHoogland Jun 26, 2023

Choose a reason for hiding this comment

rohityadavcloud commented Aug 8, 2023

blueorangutan commented Aug 8, 2023

blueorangutan commented Aug 8, 2023

shwstppr commented Aug 9, 2023

DaanHoogland commented Aug 10, 2023

blueorangutan commented Aug 10, 2023

weizhouapache left a comment

Choose a reason for hiding this comment

blueorangutan commented Aug 10, 2023

DaanHoogland commented Aug 18, 2023

shwstppr commented Aug 18, 2023

blueorangutan commented Aug 23, 2023

DaanHoogland commented Aug 23, 2023

blueorangutan commented Aug 23, 2023

blueorangutan commented Aug 24, 2023

DaanHoogland commented Aug 24, 2023

rohityadavcloud left a comment

Choose a reason for hiding this comment

DaanHoogland commented Oct 2, 2023

shwstppr commented Oct 2, 2023

shwstppr commented Oct 10, 2023

blueorangutan commented Oct 10, 2023

blueorangutan commented Oct 10, 2023

shwstppr commented Oct 10, 2023

blueorangutan commented Oct 10, 2023

blueorangutan commented Oct 10, 2023

DaanHoogland commented Oct 10, 2023

DaanHoogland commented Oct 10, 2023

blueorangutan commented Oct 10, 2023

blueorangutan commented Oct 11, 2023

DaanHoogland commented Oct 11, 2023

blueorangutan commented Oct 11, 2023

blueorangutan commented Oct 11, 2023

shwstppr commented Jun 8, 2023 •

edited

codecov bot commented Jun 8, 2023 •

edited