vmware: encode disk path for URL based access #6989

shwstppr · 2022-12-14T07:09:31Z

Description

During disk operations, the VMware plugin tries to connect to the VMware datacenter datastore using a URL based on the disk path and datastore name. This PR encodes the disk path to prevent any failure from the server due to a bad URL.

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?

Without change,

2022-12-14 12:04:34,963 ERROR [c.c.s.r.VmwareStorageProcessor] (DirectAgent-482:ctx-38e8658a 10.0.34.64, job-169/job-170, cmd: DettachCommand) (logid:e6c5c585) Failed to detach volume!
java.io.IOException: Server returned HTTP response code: 400 for URL: https://10.0.33.178/folder/space test/space test_30.vmdk?dcPath=Trillian&dsName=fd54f3559fe43f60b8694fe4ccac777f
	at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1924)
	at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1520)
	at java.base/sun.net.www.protocol.https.HttpsURLConnectionImpl.getInputStream(HttpsURLConnectionImpl.java:250)
	at com.cloud.hypervisor.vmware.util.VmwareContext.getResourceContent(VmwareContext.java:514)
	at com.cloud.hypervisor.vmware.mo.VirtualMachineMO.getDiskDatastorePathChain(VirtualMachineMO.java:2853)
	at com.cloud.hypervisor.vmware.mo.VirtualMachineMO.detachDisk(VirtualMachineMO.java:1507)
	at com.cloud.storage.resource.VmwareStorageProcessor.attachVolume(VmwareStorageProcessor.java:2143)
	at com.cloud.storage.resource.VmwareStorageProcessor.dettachVolume(VmwareStorageProcessor.java:2407)
	at com.cloud.storage.resource.StorageSubsystemCommandHandlerBase.execute(StorageSubsystemCommandHandlerBase.java:172)
	at com.cloud.storage.resource.StorageSubsystemCommandHandlerBase.handleStorageCommands(StorageSubsystemCommandHandlerBase.java:69)
	at com.cloud.hypervisor.vmware.resource.VmwareResource.executeRequest(VmwareResource.java:583)

With change,

2022-12-14 12:42:27,497 INFO  [c.c.h.v.m.DatastoreMO] (DirectAgent-18:ctx-43ed63e2 10.0.34.64, job-184/job-185, cmd: DettachCommand) (logid:0f0b1695) File [fd54f3559fe43f60b8694fe4ccac777f] space test/space test_32-flat.vmdk exists on datastore
2022-12-14 12:42:27,592 INFO  [c.c.h.v.m.DatastoreMO] (DirectAgent-18:ctx-43ed63e2 10.0.34.64, job-184/job-185, cmd: DettachCommand) (logid:0f0b1695) Folder fcd exists on datastore
2022-12-14 12:42:27,593 INFO  [c.c.s.r.VmwareStorageLayoutHelper] (DirectAgent-18:ctx-43ed63e2 10.0.34.64, job-184/job-185, cmd: DettachCommand) (logid:0f0b1695) Fixup folder-synchronization. move [fd54f3559fe43f60b8694fe4ccac777f] space test/space test_32.vmdk -> [fd54f3559fe43f60b8694fe4ccac777f] fcd/space test_32.vmdk
2022-12-14 12:42:27,602 INFO  [c.c.h.v.m.DatastoreMO] (DirectAgent-18:ctx-43ed63e2 10.0.34.64, job-184/job-185, cmd: DettachCommand) (logid:0f0b1695) Search file space test_32.vmdk on [fd54f3559fe43f60b8694fe4ccac777f] space test
2022-12-14 12:42:27,617 INFO  [c.c.h.v.m.DatastoreMO] (DirectAgent-18:ctx-43ed63e2 10.0.34.64, job-184/job-185, cmd: DettachCommand) (logid:0f0b1695) File [fd54f3559fe43f60b8694fe4ccac777f] space test/space test_32.vmdk exists on datastore
2022-12-14 12:42:27,662 INFO  [c.c.h.v.u.VmwareContext] (DirectAgent-18:ctx-43ed63e2 10.0.34.64, job-184/job-185, cmd: DettachCommand) (logid:0f0b1695) Connected, conn: sun.net.www.protocol.https.DelegateHttpsURLConnection:https://10.0.33.178/folder/space+test?dcPath=Trillian&dsName=fd54f3559fe43f60b8694fe4ccac777f, retry: 0
2022-12-14 12:42:27,684 DEBUG [c.c.a.m.DirectAgentAttache] (DirectAgent-18:ctx-43ed63e2) (logid:0f0b1695) Seq 1-6176968363915345936: Response Received:

Tested following:

Import a VM with spaces in name/path
Attach-detach data volume
Migrate VM with storage

6989.mp4

During disk operations VMware plugin tries to connect to VMware datacenter datastore using URL based on disk path and datastore name. This PR encodes disk path to prevent any failure from server due to bad URL. Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

rohityadavcloud

LGTM, the general idea that we should URL encode all params that can have issue character such as space, + etc.

sonarcloud · 2022-12-14T08:05:57Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

codecov · 2022-12-14T10:07:21Z

Codecov Report

Merging #6989 (b271066) into 4.17 (0075717) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##               4.17    #6989   +/-   ##
=========================================
  Coverage     10.36%   10.36%           
- Complexity     6631     6633    +2     
=========================================
  Files          2453     2453           
  Lines        242385   242385           
  Branches      37927    37927           
=========================================
+ Hits          25126    25130    +4     
+ Misses       214156   214151    -5     
- Partials       3103     3104    +1

Impacted Files	Coverage Δ
...om/cloud/hypervisor/vmware/util/VmwareContext.java	`14.94% <0.00%> (ø)`
...dstack/network/contrail/model/ModelObjectBase.java	`28.84% <0.00%> (+7.69%)`	⬆️

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

rohityadavcloud · 2022-12-14T14:33:56Z

@shwstppr would this cover the case for:

VM with names having space(s)
Names of folder or vmdk in storage having space(s)
vMotion/migration of such VMs (ref an old discussion: https://communities.vmware.com/t5/VI-VMware-ESX-3-5-Discussions/Storage-VMotion-How-to-cope-with-spaces-in-VM-name/td-p/725955)

shwstppr · 2022-12-28T08:55:02Z

@blueorangutan package

blueorangutan · 2022-12-28T08:56:05Z

@shwstppr 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 · 2022-12-28T09:46:28Z

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

DaanHoogland · 2022-12-30T12:05:16Z

@blueorangutan test rocky8 vmware-7u3

blueorangutan · 2022-12-30T12:06:03Z

@DaanHoogland unsupported parameters provided. Supported mgmt server os are: centos7, centos6, suse15, alma8, ubuntu18, ubuntu22, ubuntu20, rocky8. Supported hypervisors are: kvm-centos6, kvm-centos7, kvm-rocky8, kvm-alma8, kvm-ubuntu18, kvm-ubuntu20, kvm-ubuntu22, kvm-suse15, vmware-55u3, vmware-60u2, vmware-65u2, vmware-67u3, vmware-70u1, vmware-70u2, vmware-70u3, xenserver-65sp1, xenserver-71, xenserver-74, xcpng74, xcpng76, xcpng80, xcpng81, xcpng82

DaanHoogland · 2022-12-30T12:08:50Z

@blueorangutan test rocky8 vmware-70u3 keepEnv

blueorangutan · 2022-12-30T12:09:03Z

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

DaanHoogland

clgtm

blueorangutan · 2022-12-31T00:28:07Z

Trillian test result (tid-5718)
Environment: vmware-70u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 42726 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6989-t5718-vmware-70u3.zip
Smoke tests completed. 99 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_create_pvlan_network	`Error`	0.08	test_pvlan.py
test_08_upgrade_kubernetes_ha_cluster	`Failure`	609.36	test_kubernetes_clusters.py

shwstppr · 2023-01-10T07:56:49Z

@blueorangutan package

blueorangutan · 2023-01-10T07:57:03Z

@shwstppr 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-10T08:43:32Z

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

shwstppr · 2023-01-11T12:10:15Z

Tested, ready for review
@DaanHoogland

DaanHoogland · 2023-01-12T11:43:03Z

@blueorangutan package

blueorangutan · 2023-01-12T11:44:03Z

@DaanHoogland 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-12T12:36:03Z

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

DaanHoogland · 2023-01-12T13:05:36Z

@blueorangutan test rocky8 vmware-70u3

blueorangutan · 2023-01-12T13:06:03Z

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

DaanHoogland · 2023-01-12T14:20:36Z

tested; successfully attached, migrated and detached a volume called "has space".

blueorangutan · 2023-01-13T01:26:28Z

Trillian test result (tid-5813)
Environment: vmware-70u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 42808 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6989-t5813-vmware-70u3.zip
Smoke tests completed. 100 look OK, 1 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`	731.10	test_kubernetes_clusters.py

boring-cyborg bot added the component:vmware label Dec 14, 2022

rohityadavcloud requested review from harikrishna-patnala and rohityadavcloud December 14, 2022 07:17

rohityadavcloud added this to the 4.18.0.0 milestone Dec 14, 2022

rohityadavcloud approved these changes Dec 14, 2022

View reviewed changes

rohityadavcloud requested a review from nvazquez December 14, 2022 07:18

DaanHoogland self-assigned this Dec 30, 2022

DaanHoogland approved these changes Dec 30, 2022

View reviewed changes

shwstppr linked an issue Jan 11, 2023 that may be closed by this pull request

Volumes fail to detach on VMware VM imported in CloudStack which has space in its name #6992

Closed

shwstppr marked this pull request as ready for review January 11, 2023 12:10

DaanHoogland merged commit e5158b2 into apache:4.17 Jan 12, 2023

DaanHoogland mentioned this pull request Jan 13, 2023

Volumes fail to detach on VMware VM imported in CloudStack which has space in its name #6992

Closed

DaanHoogland deleted the fix-vmw-detach-disk-url branch January 13, 2023 04:34

DaanHoogland removed their assignment Jun 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vmware: encode disk path for URL based access #6989

vmware: encode disk path for URL based access #6989

shwstppr commented Dec 14, 2022 •

edited

rohityadavcloud left a comment

sonarcloud bot commented Dec 14, 2022

codecov bot commented Dec 14, 2022

rohityadavcloud commented Dec 14, 2022

shwstppr commented Dec 28, 2022

blueorangutan commented Dec 28, 2022

blueorangutan commented Dec 28, 2022

DaanHoogland commented Dec 30, 2022

blueorangutan commented Dec 30, 2022

DaanHoogland commented Dec 30, 2022

blueorangutan commented Dec 30, 2022

DaanHoogland left a comment

blueorangutan commented Dec 31, 2022

shwstppr commented Jan 10, 2023

blueorangutan commented Jan 10, 2023

blueorangutan commented Jan 10, 2023

shwstppr commented Jan 11, 2023

DaanHoogland commented Jan 12, 2023

blueorangutan commented Jan 12, 2023

blueorangutan commented Jan 12, 2023

DaanHoogland commented Jan 12, 2023

blueorangutan commented Jan 12, 2023

DaanHoogland commented Jan 12, 2023

blueorangutan commented Jan 13, 2023

vmware: encode disk path for URL based access #6989

vmware: encode disk path for URL based access #6989

Conversation

shwstppr commented Dec 14, 2022 • edited

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

rohityadavcloud left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Dec 14, 2022

codecov bot commented Dec 14, 2022

Codecov Report

rohityadavcloud commented Dec 14, 2022

shwstppr commented Dec 28, 2022

blueorangutan commented Dec 28, 2022

blueorangutan commented Dec 28, 2022

DaanHoogland commented Dec 30, 2022

blueorangutan commented Dec 30, 2022

DaanHoogland commented Dec 30, 2022

blueorangutan commented Dec 30, 2022

DaanHoogland left a comment

Choose a reason for hiding this comment

blueorangutan commented Dec 31, 2022

shwstppr commented Jan 10, 2023

blueorangutan commented Jan 10, 2023

blueorangutan commented Jan 10, 2023

shwstppr commented Jan 11, 2023

DaanHoogland commented Jan 12, 2023

blueorangutan commented Jan 12, 2023

blueorangutan commented Jan 12, 2023

DaanHoogland commented Jan 12, 2023

blueorangutan commented Jan 12, 2023

DaanHoogland commented Jan 12, 2023

blueorangutan commented Jan 13, 2023

shwstppr commented Dec 14, 2022 •

edited