Fix data volume import / export functionality when using swift as secondary storage #4875

skattoju4 · 2021-03-25T18:53:08Z

Description

This PR is a fix for the data volume import and export functionality when using swift as secondary storage.

Background:
Secondary storage is used for templates, snapshots and iso's. A data disk is a volume and is meant to reside on primary storage. However, when importing a volume it is first uploaded to secondary storage and then copied to over to primary storage when attached to a VM. Similarly when exporting a volume, it is copied to secondary storage before it is made available to the user for download. When using swift as a secondary storage, an intermediate staging nfs store is needed when copying volumes between primary and secondary storage. This staging nfs store is leveraged for the volume import and export functionality since volumes need not have a footprint on secondary storage.

Currently this functionality does not work:

When importing a volume the following error is observed:

org.apache.cloudstack.storage.to.VolumeObjectTO cannot be cast to org.apache.cloudstack.storage.to.TemplateObjectTO

Further investigation reveals that there is no provision in the code to handle import of volumes. Currently only template import is supported.

When exporting a volume a URL to the secondary storage is generated but it clicking it does not initiate a download of the exported volume. Further investigation revealed that the url points to a broken symlink which points to an invalid path on the SSVM

Use Cases

User should be able to import a volume from a URL when using swift as secondary storage and XenServer as the hypervisor.
User should be able to export a volume to a secondary storage VM URL when using swift as secondary storage and XenServer as the hypervisor.

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

This addresses #4874

Feature/Enhancement Scale

Major
[*] Minor

Bug Severity

BLOCKER
Critical
Major
[*] Minor
Trivial

How Has This Been Tested?

Create data volume (non-managed storage)
Attach it to a VM.
Create a file system and a test file on the new data volume.
Export volume.
Upload exported volume to a bucket and generate a URL.
Import volume back into CloudStack from URL.
Attach imported disk to a VM.
Verify previously created test file.

…ondary storage

core/src/main/java/com/cloud/storage/template/SwiftVolumeDownloader.java

.../src/main/java/org/apache/cloudstack/storage/datastore/driver/SwiftImageStoreDriverImpl.java

...server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java

skattoju4 · 2021-04-21T22:27:22Z

@syed @khos2ow could you take a look at this if you have some time ?

syed · 2021-04-23T17:39:49Z

...server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java

@@ -772,7 +780,12 @@ protected Answer copyFromSwiftToNfs(CopyCommand cmd, DataTO srcData, SwiftTO swi
                }
            }

-            File destFile = SwiftUtil.getObject(swiftTO, downloadDirectory, srcData.getPath());
+            String filePath = downloadPath + File.separator + destData.getName();


Are you not using the getPath to find the path of the object anymore? does downloadPath consider the difference between snapshot and template and now volume?

Hey Syed, thanks for the review. We do call getPath() on line 766 so the downloadPath will contain the path to folder depending on whether its a volume, snapshot or template. Here i'm skipping the copy from swift if the file is already there since we don't upload volumes to swift just to the staging nfs.

server/src/main/java/com/cloud/storage/download/DownloadMonitorImpl.java

syed · 2021-04-23T17:43:28Z

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

+                } else {
+                    hostVO = _hostDao.findById(vm.getLastHostId());
+                }
+                destPrimaryStorage.setClusterId(hostVO.getClusterId());


Do we want to set the clusterID for zonewide storage too?

This was added because we were encountering a null pointer exception in some scenarios I don't recall exactly. I added the bit to get the last host id of the vm as attaching a disk to a stopped vm would fail. Have not encountered any side effects with regards to zone wide storage so far..

syed · 2021-04-23T17:45:28Z

core/src/main/java/com/cloud/storage/template/SwiftVolumeDownloader.java

@@ -0,0 +1,397 @@
+//


Where would this run? On the mgmt server or on the SSVM?

This would run on the SSVM

rohityadavcloud · 2021-05-10T17:43:10Z

@skattoju4 is this ready, how do we test it or can you provide test results?

skattoju4 · 2021-05-10T23:10:40Z

@rhtyd it should be good to go. A CloudStack setup that uses swift as secondary storage is needed to test. This was tested manually with the following high level steps:

Create data volume (only non-managed storage is supported for now)
Attach it to a VM.
Create a file system and a test file on the new data volume.
Export volume. (tests volume export feature)
Upload exported volume to a bucket and generate a URL.
Import volume back into CloudStack from URL. (tests volume import feature)
Attach imported disk to a VM.
Verify previously created test file.

Its been in production for some time with no side effects so far @pdion891 can attest.

skattoju4 · 2021-05-10T23:11:00Z

The travis ci failures are intermittent.

rohityadavcloud · 2021-06-18T01:11:21Z

@blueorangutan package

blueorangutan · 2021-06-18T01:12:02Z

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

blueorangutan · 2021-06-18T02:12:37Z

Packaging result: ✖️ centos7 ✔️ centos8 ✔️ debian. SL-JID 268

nvazquez · 2021-06-23T14:43:27Z

@blueorangutan package

blueorangutan · 2021-06-23T14:44:03Z

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

DaanHoogland · 2023-09-18T09:40:13Z

@blueorangutan package

blueorangutan · 2023-09-18T09:42:03Z

@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.

codecov · 2023-09-18T09:51:07Z

Codecov Report

Merging #4875 (9970704) into main (2e9b3d8) will increase coverage by 0.65%.
The diff coverage is 3.16%.

@@             Coverage Diff              @@
##               main    #4875      +/-   ##
============================================
+ Coverage     28.57%   29.23%   +0.65%     
- Complexity    29784    30501     +717     
============================================
  Files          5100     5101       +1     
  Lines        358565   358815     +250     
  Branches      52316    52342      +26     
============================================
+ Hits         102464   104887    +2423     
+ Misses       241968   239541    -2427     
- Partials      14133    14387     +254

Flag	Coverage Δ
simulator-marvin-tests	`25.20% <2.53%> (+0.88%)`	⬆️
uitests	`4.86% <ø> (ø)`
unit-tests	`14.50% <4.44%> (-0.01%)`	⬇️

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

Files	Coverage Δ
.../java/com/cloud/storage/template/VhdProcessor.java	`0.00% <ø> (ø)`
...tack/storage/motion/AncientDataMotionStrategy.java	`27.55% <100.00%> (+10.16%)`	⬆️
...apache/cloudstack/storage/volume/VolumeObject.java	`65.36% <100.00%> (+3.54%)`	⬆️
...oudstack/storage/volume/VolumeDataFactoryImpl.java	`64.58% <0.00%> (ø)`
...tastore/driver/CloudStackImageStoreDriverImpl.java	`0.00% <0.00%> (ø)`
...e/cloudstack/diagnostics/to/DiagnosticsDataTO.java	`0.00% <0.00%> (ø)`
...om/cloud/storage/download/DownloadMonitorImpl.java	`11.66% <0.00%> (-0.10%)`	⬇️
...dstack/storage/image/BaseImageStoreDriverImpl.java	`2.81% <0.00%> (-0.04%)`	⬇️
...n/java/com/cloud/storage/VolumeApiServiceImpl.java	`32.32% <0.00%> (+1.56%)`	⬆️
...ge/datastore/driver/SwiftImageStoreDriverImpl.java	`3.84% <0.00%> (-1.16%)`	⬇️
... and 3 more

... and 188 files with indirect coverage changes

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

blueorangutan · 2023-09-18T10:26:07Z

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

shwstppr · 2023-10-03T09:18:37Z

@Sjnas can you please check the failures? Should we consider this for 4.19.0.0?

DaanHoogland · 2023-10-03T12:37:55Z

@blueorangutan package

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

@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-10-03T12:53:33Z

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

DaanHoogland · 2023-10-04T07:19:29Z

@blueorangutan package

blueorangutan · 2023-10-04T07:20:03Z

@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-10-04T08:20:58Z

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

DaanHoogland · 2023-10-04T08:40:19Z

@blueorangutan test

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

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

blueorangutan · 2023-10-04T23:44:12Z

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

Test	Result	Time (s)	Test File
test_01_volume_usage	`Error`	127.05	test_usage.py
test_list_vms_metrics_admin	`Error`	3609.53	test_metrics_api.py
test_list_vms_metrics_history	`Error`	5.39	test_metrics_api.py
test_list_volumes_metrics_history	`Error`	4.36	test_metrics_api.py

DaanHoogland · 2023-10-05T06:49:45Z

@blueorangutan test

blueorangutan · 2023-10-05T06:50:03Z

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

blueorangutan · 2023-10-05T18:58:38Z

[SF] Trillian test result (tid-7854)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42218 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4875-t7854-kvm-centos7.zip
Smoke tests completed. 112 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_volume_usage	`Error`	130.17	test_usage.py

DaanHoogland · 2023-10-06T12:54:14Z

@skattoju4 the usage test failure seem consistent: 'Command failed due to Internal Server Error' can you check it locally?

github-actions · 2023-10-23T07:08:18Z

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

DaanHoogland · 2023-10-31T13:41:48Z

@skattoju4 are you still concerned with this?

Fix data volume import / export functionality when using swift as sec…

830b2a1

…ondary storage