kvm: Fix double-escape issue while creating rbd disk options by yadvr · Pull Request #4568 · apache/cloudstack

yadvr · 2021-01-04T14:26:02Z

This fixes issue introduced in c3554ec
which enable block of code that will double escape rados host/monitor
port.

Seen in logs:

2021-01-04 14:21:35,412 INFO  [kvm.storage.LibvirtStorageAdaptor] (agentRequest-Handler-4:null) (logid:c5187751) Creating volume 125a9ebb-de4c-49cf-b9a7-9249c4af53b9 from template d9eb3660-23e5-4b37-85ec-b6ad4978bf22 in pool 509bd564-b7e3-3d5f-b84b-b799055b11ca (RBD) with size (5.00 GB) 5368709120

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)

This fixes issue introduced in c3554ec which enable block of code that will double escape rados host/monitor port. Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

yadvr · 2021-01-04T14:26:24Z

@blueorangutan package

blueorangutan · 2021-01-04T14:27:12Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

wido · 2021-01-04T14:29:07Z

@GabrielBrascher had a discussion about this. As the port usually is 6789 the escaping wouldn't happen. Now the if-statement changed and this causes it to escape most of the time.

Gabriel is testing this to see if this fixes it.

GabrielBrascher · 2021-01-04T15:00:21Z

@rhtyd just adding a small change that is necessary in order to have all tests passing.

At line:

cloudstack/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDiskTest.java

Line 28 in 261750c

    
           "rbd:volume1:mon_host=ceph-monitor\\\\:8000:auth_supported=cephx:id=admin:key=supersecret:rbd_default_format=2:client_mount_timeout=30");

It needs to be updated from

... monitor\\\\:8000:auth_supported=...

to
... monitor\\:8000:auth_supported=...

GabrielBrascher

Update KVMPhysicalDiskTest class to match mon_host=ceph-monitor\\:8000.

GabrielBrascher · 2021-01-04T16:16:25Z

@DaanHoogland @rhtyd @wido tests are looking good 👍

Additionally, from my tests, this also fixes #4498.

Once the test class is updated this is good to go.

DaanHoogland · 2021-01-04T20:54:01Z

@GabrielBrascher can you send a PR to this one to include the test fix?

weizhouapache · 2021-01-04T21:01:29Z

code lgtm
thanks @GabrielBrascher for testing.
@div8cn could you please test it as well ?

div8cn · 2021-01-05T01:11:25Z

thanks @rhtyd @GabrielBrascher @weizhouapache

This patch has fixed the double-escape .
Now copy template to primary storage is running

2021-01-05 01:07:56,411 DEBUG [kvm.storage.LibvirtStorageAdaptor] (agentRequest-Handler-2:null) (logid:a8028c86) copyPhysicalDisk: disk size:(356.96 MB) 374304768, virtualsize:(2.44 GB) 2621440000 format:qcow2 2021-01-05 01:07:56,412 DEBUG [kvm.storage.LibvirtStorageAdaptor] (agentRequest-Handler-2:null) (logid:a8028c86) The source image is not RBD, but the destination is. We will convert into RBD format 2 2021-01-05 01:07:56,413 DEBUG [kvm.storage.LibvirtStorageAdaptor] (agentRequest-Handler-2:null) (logid:a8028c86) Starting copy from source image /mnt/466f03d4-9dfe-3af4-a042-33a00dae0e97/40165b83-896c-4693-abe7-9fd96b40ce9a.qcow2 to RBD image rbd/40165b83-896c-4693-abe7-9fd96b40ce9a 2021-01-05 01:07:56,413 DEBUG [utils.script.Script] (agentRequest-Handler-2:null) (logid:a8028c86) Executing: qemu-img convert -O raw /mnt/466f03d4-9dfe-3af4-a042-33a00dae0e97/40165b83-896c-4693-abe7-9fd96b40ce9a.qcow2 rbd:rbd/40165b83-896c-4693-abe7-9fd96b40ce9a:mon_host=10.100.250.14\:6789:auth_supported=cephx:id=hyperx:key=AQAywfFf8jCiIxAAbnDBjX1QQAO9Sj22kUBh7g==:rbd_default_format=2:client_mount_timeout=30 2021-01-05 01:07:56,414 DEBUG [utils.script.Script] (agentRequest-Handler-2:null) (logid:a8028c86) Executing while with timeout : 10800000 2021-01-05 01:08:16,919 DEBUG [cloud.agent.Agent] (agentRequest-Handler-4:null) (logid:248c7e0b) Processing command: com.cloud.agent.api.GetStorageStatsCommand

GabrielBrascher · 2021-01-05T01:33:40Z

@DaanHoogland the only change necessary to pass all tests was the following commit: CLDIN@cc6f7f7

for reference, I added it under ceph-double-quote-fix-test branch.

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

yadvr · 2021-01-05T06:41:34Z

@blueorangutan package

blueorangutan · 2021-01-05T06:42:09Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-01-05T07:20:04Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2523

DaanHoogland · 2021-01-05T08:06:47Z

ok, looks good, I'll await travis and assume that will do. Given that there is no ceph automation and we have manual confirmation, i'll create a new rc today.

yadvr · 2021-01-05T09:53:29Z

Travis is green and we've manual tests from @div8cn and me, as well as lgtm from Daan and Wei. Merging this, let's cut RC4 after this.

* commit '280c13a4bb103dd748ec304bfe0714a148c24602': Updating pom.xml version numbers for release 4.15.0.0 kvm: Fix double-escape issue while creating rbd disk options (apache#4568) networkorchestrator: Fix typo in exception message (apache#4559) checkstyle: fix pom version Updating pom.xml version numbers for release 4.15.1.0-SNAPSHOT Updating pom.xml version numbers for release 4.15.0.0 vmware: Fix for mapping guest OS type read from OVF to existing guest OS in C… (apache#4553) vmware: Fix template upload from local (apache#4555)

kvm: Fix double-escape issue while creating rbd disk options

24da5c1

This fixes issue introduced in c3554ec which enable block of code that will double escape rados host/monitor port. Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

yadvr added this to the 4.15.0.0 milestone Jan 4, 2021

yadvr requested review from DaanHoogland, GabrielBrascher, weizhouapache and wido January 4, 2021 14:26

yadvr closed this Jan 4, 2021

yadvr reopened this Jan 4, 2021

DaanHoogland approved these changes Jan 4, 2021

View reviewed changes

GabrielBrascher requested changes Jan 4, 2021

View reviewed changes

fix unit test

c2faeeb

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

yadvr mentioned this pull request Jan 5, 2021

Fix RBD primary storage host port null error #4565

Merged

12 tasks

DaanHoogland requested a review from GabrielBrascher January 5, 2021 08:10

yadvr added component:ceph Severity:BLOCKER labels Jan 5, 2021

yadvr merged commit c82688a into apache:4.15 Jan 5, 2021

GabrielBrascher mentioned this pull request Jan 5, 2021

RBD Snapshot fail when snapshot.backup.to.secondary: true #4498

Closed

Uh oh!

Conversation

yadvr commented Jan 4, 2021

Types of changes

Uh oh!

yadvr commented Jan 4, 2021

Uh oh!

blueorangutan commented Jan 4, 2021

Uh oh!

wido commented Jan 4, 2021

Uh oh!

GabrielBrascher commented Jan 4, 2021

Uh oh!

GabrielBrascher 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!

GabrielBrascher commented Jan 4, 2021

Uh oh!

DaanHoogland commented Jan 4, 2021

Uh oh!

weizhouapache commented Jan 4, 2021

Uh oh!

div8cn commented Jan 5, 2021

Uh oh!

GabrielBrascher commented Jan 5, 2021

Uh oh!

yadvr commented Jan 5, 2021

Uh oh!

blueorangutan commented Jan 5, 2021

Uh oh!

blueorangutan commented Jan 5, 2021

Uh oh!

DaanHoogland commented Jan 5, 2021

Uh oh!

yadvr commented Jan 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

GabrielBrascher left a comment •

edited

Loading