check tags while fetching storage pool for importing vm #9764

vishesh92 · 2024-10-03T13:42:13Z

Description

This PR fixes #9670

Details

This pull request includes several changes to the UnmanagedVMsManagerImpl class to enhance the handling of disk offerings during the import and conversion of unmanaged instances. The most important changes include modifications to the getStoragePool method, updates to the conversion and import logic, and adjustments to the test cases.

Enhancements to disk offering handling:

server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java: Modified the getStoragePool method to accept an additional diskOfferingTags parameter and updated the logic to ensure the storage pool supports the disk offering. [1] [2] [3]

Updates to conversion and import logic:

server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java: Updated various methods to include disk offering tags and ensure compatibility with storage pools, such as checkUnmanagedDiskAndOfferingForImport, importDisk, importUnmanagedInstanceFromVmwareToKvm, sanitizeConvertedInstance, checkConversionSupportOnHost, convertVmwareInstanceToKVMWithOVFOnConvertLocation, and convertVmwareInstanceToKVMAfterExportingOVFToConvertLocation. [1] [2] [3] [4] [5] [6] [7]

Adjustments to test cases:

server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java: Updated the test methods importUnmanagedInstanceTest and baseTestImportVmFromVmwareToKvm to mock the doesTargetStorageSupportDiskOffering method and ensure the tests reflect the new disk offering logic. [1] [2]

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)
build/CI
test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

vishesh92 · 2024-10-03T13:42:52Z

@blueorangutan package

blueorangutan · 2024-10-03T13:44:03Z

@vishesh92 a [SL] 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 · 2024-10-03T13:50:26Z

Codecov Report

Attention: Patch coverage is 52.23881% with 32 lines in your changes missing coverage. Please review.

Project coverage is 15.13%. Comparing base (ed1b145) to head (5e3c0f5).
Report is 3 commits behind head on 4.19.

Files with missing lines	Patch %	Lines
.../apache/cloudstack/vm/UnmanagedVMsManagerImpl.java	52.23%	17 Missing and 15 partials ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               4.19    #9764   +/-   ##
=========================================
  Coverage     15.13%   15.13%           
- Complexity    11265    11272    +7     
=========================================
  Files          5408     5408           
  Lines        473900   473946   +46     
  Branches      57791    57809   +18     
=========================================
+ Hits          71705    71725   +20     
- Misses       394192   394203   +11     
- Partials       8003     8018   +15

Flag	Coverage Δ
uitests	`4.30% <ø> (+<0.01%)`	⬆️
unittests	`15.85% <52.23%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

DaanHoogland

clgtm

blueorangutan · 2024-10-03T14:11:08Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 11268

sureshanaparti

clgtm

sureshanaparti · 2024-10-03T17:29:51Z

@vishesh92 check/fix unit tests.

vishesh92 · 2024-10-03T19:59:50Z

@blueorangutan package

blueorangutan · 2024-10-03T20:02:04Z

@vishesh92 a [SL] 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 · 2024-10-03T20:53:37Z

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

vishesh92 · 2024-10-04T09:58:01Z

@blueorangutan test

blueorangutan · 2024-10-04T10:00:04Z

@vishesh92 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

JoaoJandre

CLGTM. Could you describe your tests @vishesh92 ?

blueorangutan · 2024-10-05T01:02:08Z

[SF] Trillian test result (tid-11613)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 51907 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9764-t11613-kvm-ol8.zip
Smoke tests completed. 130 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_secure_vm_migration	`Error`	134.42	test_vm_life_cycle.py
test_01_secure_vm_migration	`Error`	134.42	test_vm_life_cycle.py
test_05_vmschedule_test_e2e	`Failure`	362.27	test_vm_schedule.py
test_05_rvpc_multi_tiers	`Failure`	482.80	test_vpc_redundant.py
test_05_rvpc_multi_tiers	`Error`	482.81	test_vpc_redundant.py

vishesh92 · 2024-10-05T06:26:08Z

CLGTM. Could you describe your tests @vishesh92 ?

I am still testing it. Let me make this PR as a draft for now.

vishesh92 · 2024-10-15T11:49:06Z

There are some issues. I will continue on this after #9787.

vishesh92 · 2024-12-26T05:47:51Z

@blueorangutan package

blueorangutan · 2024-12-26T05:50:03Z

@vishesh92 a [SL] 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 · 2024-12-26T06:48:57Z

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

…ools

vishesh92 · 2024-12-31T07:51:24Z

@blueorangutan package

blueorangutan · 2024-12-31T07:52:04Z

@vishesh92 a [SL] 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 · 2024-12-31T08:58:14Z

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

DaanHoogland · 2024-12-31T09:01:16Z

@blueorangutan test keepEnv

blueorangutan · 2024-12-31T09:02:04Z

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

blueorangutan · 2024-12-31T09:31:14Z

[SF] Trillian Build Failed (tid-12013)

DaanHoogland

clgtm still

blueorangutan · 2024-12-31T22:57:09Z

[SF] Trillian test result (tid-12016)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 44529 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9764-t12016-kvm-ol8.zip
Smoke tests completed. 132 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_secure_vm_migration	`Error`	134.38	test_vm_life_cycle.py
test_01_secure_vm_migration	`Error`	134.39	test_vm_life_cycle.py

kiranchavala

LGTM

Steps to reproduce the issue

Launch a Cloudstack environment with 2 clusters vmware and kvm
Tag the Primay storages associated with vmware cluster (t1)
Tag the Primay storages associated with kvm cluster (t2)
Create a compute offering with storage tag (t1)
Create a 2 disk offerings vmware disk offering with tag (t1) and kvm disk offering with tag (t2)
Launch a vm on vmware cluster with compute offering (mentioned in step 4) and a data disk offering (t1 )
Import a VM from VMWare cluster to kvmcluster. Specify the disk offering for the disk (t2)

Before fix

Exception observed and the vm import fails

2025-01-02 10:59:58,629 DEBUG [c.c.s.VolumeApiServiceImpl] (API-Job-Executor-75:[ctx-6f2e67f9, job-173, ctx-ca492698]) (logid:bf43ecb6) Destination storage pool [551184a2-ff07-338a-b271-55975f6be3c7] accepts tags [tag4]? false
2025-01-02 10:59:58,629 ERROR [o.a.c.v.UnmanagedVMsManagerImpl] (API-Job-Executor-75:[ctx-6f2e67f9, job-173, ctx-ca492698]) (logid:bf43ecb6) Error importing VM: Disk offering: ff026253-1000-4a32-be90-92eaa3aa163d is not compatible with storage pool: 551184a2-ff07-338a-b271-55975f6be3c7 of unmanaged disk: 25-2000 com.cloud.exception.InvalidParameterValueException: Disk offering: ff026253-1000-4a32-be90-92eaa3aa163d is not compatible with storage pool: 551184a2-ff07-338a-b271-55975f6be3c7 of unmanaged disk: 25-2000

After fix , no exception observed and able to import the vm successful

2025-01-02 11:29:43,701 DEBUG [c.c.s.VolumeApiServiceImpl] (API-Job-Executor-52:ctx-aa2e87b2 job-113 ctx-697a3a12) (logid:5e8257b0) Destination storage pool has tags [%s], and the disk offering has no tags. Therefore, they are compatible.
2025-01-02 11:29:43,703 DEBUG [c.c.s.VolumeApiServiceImpl] (API-Job-Executor-52:ctx-aa2e87b2 job-113 ctx-697a3a12) (logid:5e8257b0) Destination storage pool [10242ad6-ae25-3fd8-8507-3e78e254b28c] accepts tags [tag3]? true

* 4.19: check tags while fetching storage pool for importing vm (#9764)

* 4.20: log name change after merge forward check tags while fetching storage pool for importing vm (#9764) UI: Add cluster arch type to the zone creation wizard (#10080)

DaanHoogland linked an issue Oct 3, 2024 that may be closed by this pull request

Incorrectly tagged storage pool selected while importing VM leading to failure #9670

Closed

DaanHoogland approved these changes Oct 3, 2024

View reviewed changes

DaanHoogland added the status:needs-testing label Oct 3, 2024

DaanHoogland added this to the 4.19.2.0 milestone Oct 3, 2024

sureshanaparti approved these changes Oct 3, 2024

View reviewed changes

JoaoJandre approved these changes Oct 4, 2024

View reviewed changes

vishesh92 marked this pull request as draft October 5, 2024 06:26

vishesh92 added 2 commits December 26, 2024 10:10

check tags while fetching storage pool for importing vm

6556c98

Fix unit tests

1779a33

vishesh92 force-pushed the fix-get-storage-pools branch from 61ebded to 2479d6a Compare December 26, 2024 04:41

Check disk offering tags before importing

8e473f3

vishesh92 force-pushed the fix-get-storage-pools branch from 2479d6a to 8e473f3 Compare December 26, 2024 04:53

apache deleted a comment from blueorangutan Dec 26, 2024

vishesh92 force-pushed the fix-get-storage-pools branch from 5128522 to df3083c Compare December 31, 2024 06:03

vishesh92 requested review from DaanHoogland and sureshanaparti December 31, 2024 06:07

add checks for storage tags and fix ordering of destination storage p…

5e3c0f5

…ools

vishesh92 force-pushed the fix-get-storage-pools branch from df3083c to 5e3c0f5 Compare December 31, 2024 07:51

apache deleted a comment from blueorangutan Dec 31, 2024

DaanHoogland approved these changes Dec 31, 2024

View reviewed changes

kiranchavala approved these changes Jan 2, 2025

View reviewed changes

kiranchavala removed the status:needs-testing label Jan 2, 2025

vishesh92 marked this pull request as ready for review January 3, 2025 08:05

DaanHoogland merged commit d8c321d into apache:4.19 Jan 3, 2025
26 checks passed

DaanHoogland deleted the fix-get-storage-pools branch January 3, 2025 08:05

DaanHoogland mentioned this pull request Jan 3, 2025

Incorrectly tagged storage pool selected while importing VM leading to failure #9670

Closed

DaanHoogland added a commit that referenced this pull request Jan 3, 2025

Merge release branch 4.19 to 4.20

7931645

* 4.19: check tags while fetching storage pool for importing vm (#9764)

DaanHoogland added a commit that referenced this pull request Jan 3, 2025

Merge release branch 4.20 to main

30b2588

* 4.20: log name change after merge forward check tags while fetching storage pool for importing vm (#9764) UI: Add cluster arch type to the zone creation wizard (#10080)

DaanHoogland pushed a commit to shapeblue/cloudstack that referenced this pull request Jan 7, 2025

check tags while fetching storage pool for importing vm (apache#9764)

e8c409b

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jan 10, 2025

check tags while fetching storage pool for importing vm (apache#9764)

68d20f6

kiranchavala mentioned this pull request Feb 17, 2025

Vmware to kvm import is failing on 4.19.2 rc3 #10404

Closed

sureshanaparti mentioned this pull request Jun 12, 2025

[VMware to KVM Migration] Fix for converted instance NPE issue when source VMware instance OVF is exported from management server #11003

Merged

14 tasks

check tags while fetching storage pool for importing vm #9764

check tags while fetching storage pool for importing vm #9764

Uh oh!

Conversation

vishesh92 commented Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

vishesh92 commented Oct 3, 2024

Uh oh!

blueorangutan commented Oct 3, 2024

Uh oh!

codecov bot commented Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Oct 3, 2024

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

sureshanaparti commented Oct 3, 2024

Uh oh!

vishesh92 commented Oct 3, 2024

Uh oh!

blueorangutan commented Oct 3, 2024

Uh oh!

blueorangutan commented Oct 3, 2024

Uh oh!

vishesh92 commented Oct 4, 2024

Uh oh!

blueorangutan commented Oct 4, 2024

Uh oh!

JoaoJandre left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Oct 5, 2024

Uh oh!

vishesh92 commented Oct 5, 2024

Uh oh!

vishesh92 commented Oct 15, 2024

Uh oh!

vishesh92 commented Dec 26, 2024

Uh oh!

blueorangutan commented Dec 26, 2024

Uh oh!

blueorangutan commented Dec 26, 2024

Uh oh!

vishesh92 commented Dec 31, 2024

Uh oh!

blueorangutan commented Dec 31, 2024

Uh oh!

blueorangutan commented Dec 31, 2024

Uh oh!

DaanHoogland commented Dec 31, 2024

Uh oh!

blueorangutan commented Dec 31, 2024

Uh oh!

blueorangutan commented Dec 31, 2024

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Dec 31, 2024

Uh oh!

kiranchavala left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vishesh92 commented Oct 3, 2024 •

edited

Loading

codecov bot commented Oct 3, 2024 •

edited

Loading