Linstor backup snaphots #8067

rp- · 2023-10-10T13:16:52Z

Description

This PR adds an config option for the Linstor primary storage driver, that allows you to automatically backup
volume snapshots to the secondary storage.
Additionally it will not mangle the need java-linstor dependency into the client.jar, but instead just copy
the java-linstor.jar into lib.

Config option is called: lin.backup.snapshots and is default false

The scope of this change should be limited, as it only touches the Linstor driver and a part of copyAsync
was implemented with 2 new Linstor specific commands.

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

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

How Has This Been Tested?

Tested on 2 different Linstor clusters, one running LVM the other ZFS.
Additionally send this PR to a PoC tester, who requested this feature.

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

codecov · 2023-10-10T13:53:56Z

Codecov Report

Merging #8067 (ec5ab15) into main (e9b24b6) will decrease coverage by 0.14%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               main    #8067      +/-   ##
============================================
- Coverage     28.33%   28.20%   -0.14%     
+ Complexity    29966    29614     -352     
============================================
  Files          5181     5186       +5     
  Lines        365434   365752     +318     
  Branches      53449    53491      +42     
============================================
- Hits         103550   103143     -407     
- Misses       247552   248420     +868     
+ Partials      14332    14189     -143

Flag	Coverage Δ
simulator-marvin-tests	`23.86% <0.00%> (-0.17%)`	⬇️
uitests	`4.50% <ø> (ø)`
unit-tests	`14.77% <0.00%> (-0.03%)`	⬇️

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

Files	Coverage Δ
.../provider/LinstorPrimaryDatastoreProviderImpl.java	`0.00% <0.00%> (ø)`
...loud/api/storage/LinstorBackupSnapshotCommand.java	`0.00% <0.00%> (ø)`
...pi/storage/LinstorRevertBackupSnapshotCommand.java	`0.00% <0.00%> (ø)`
.../hypervisor/kvm/storage/LinstorStorageAdaptor.java	`0.00% <0.00%> (ø)`
...ge/datastore/util/LinstorConfigurationManager.java	`0.00% <0.00%> (ø)`
...per/LinstorRevertBackupSnapshotCommandWrapper.java	`0.00% <0.00%> (ø)`
...cloudstack/storage/datastore/util/LinstorUtil.java	`0.00% <0.00%> (ø)`
...e/wrapper/LinstorBackupSnapshotCommandWrapper.java	`0.00% <0.00%> (ø)`
...tore/driver/LinstorPrimaryDataStoreDriverImpl.java	`0.00% <0.00%> (ø)`

... and 412 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

DaanHoogland

looks mostly good. licenses are needed!.

...storage/volume/linstor/src/main/java/com/cloud/api/storage/LinstorBackupSnapshotCommand.java

...e/volume/linstor/src/main/java/com/cloud/api/storage/LinstorRevertBackupSnapshotCommand.java

...main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java

...ava/com/cloud/hypervisor/kvm/resource/wrapper/LinstorRevertBackupSnapshotCommandWrapper.java

...n/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java

.../src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorConfigurationManager.java

DaanHoogland

clgtm

DaanHoogland · 2023-10-12T13:40:54Z

@blueorangutan package

blueorangutan · 2023-10-12T13: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.

blueorangutan · 2023-10-12T14:54:42Z

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

rp- · 2023-10-13T07:24:42Z

@DaanHoogland I have another small feature, that depends on this merged.
Should I add that to this PR? or wait and create another PR?

DaanHoogland · 2023-10-13T09:49:43Z

@DaanHoogland I have another small feature, that depends on this merged. Should I add that to this PR? or wait and create another PR?

your choice @rp- , if it is reasonably risk free, I'd add it. else you could wait.

rp- · 2023-10-13T10:15:49Z

I have added the commit, thanks

...n/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java

rp- · 2023-10-13T13:40:12Z

@DaanHoogland done, thanks for the review

DaanHoogland · 2023-10-13T13:47:14Z

@blueorangutan package

blueorangutan · 2023-10-13T13:48:04Z

@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-13T14:43:10Z

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

DaanHoogland · 2023-10-16T06:51:15Z

@blueorangutan test

blueorangutan · 2023-10-16T06:52:03Z

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

blueorangutan · 2023-10-16T07:43:39Z

[SF] Trillian Build Failed (tid-7976)

rp- · 2023-11-04T19:42:51Z

usually I test on ubuntu.
But I would be happy for any packages, to check whats wrong.
because on my end it works of course...

github-actions · 2023-11-06T07:56:48Z

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

rp- · 2023-11-06T14:24:23Z

if you can't provide packages, could you check if java-linstor-0.3.0.jar is included in the cloudstack-agent/lib ?
or maybe it is possible to get more debug output, why it gets stuck on loading the LinstorStorageAdaptor?

shwstppr · 2023-11-07T08:11:53Z

if you can't provide packages, could you check if java-linstor-0.3.0.jar is included in the cloudstack-agent/lib ? or maybe it is possible to get more debug output, why it gets stuck on loading the LinstorStorageAdaptor?

@rp- sorry I could get time to upload packages publicly.
I checked in the env and lib was missing in cloudstack-agent/lib. Is it because the dependency is removed from plugins/hypervisors/kvm/pom.xml? Would it be better to add the dependency in another module that is present in cloudstack-agent?

rp- · 2023-11-07T08:32:57Z

@shwstppr where are the dependencies for cloudstack-agent declared?
Seems not in agent/pom.xml
Sorry I'm not really familiar with Maven, we use gradle on our end.

agent should than probably have a dependency to the cloud-plugin-storage-volume-linstor because that is where the LinstorStorageAdapter is located, or maybe the kvm-hyporvisor-plugin should have a dep to it?

shwstppr · 2023-11-07T09:01:18Z

@rp- Adding dependency in cloud-plugin-storage-volume-linstor makes sense as it is present in cloudstack-agent. cc @DaanHoogland @rohityadavcloud

DaanHoogland · 2023-11-07T09:01:45Z

@shwstppr where are the dependencies for cloudstack-agent declared? Seems not in agent/pom.xml Sorry I'm not really familiar with Maven, we use gradle on our end.

agent should than probably have a dependency to the cloud-plugin-storage-volume-linstor because that is where the LinstorStorageAdapter is located, or maybe the kvm-hyporvisor-plugin should have a dep to it?

@rp- , sounds like you need to add it in the cloud.spec files. but I already see a line

cp plugins/storage/volume/linstor/target/*.jar  ${RPM_BUILD_ROOT}%{_datadir}/%{name}-agent/lib

there. Maybe something else is needed? for the non acs parts of linstore?

yadvr · 2023-11-07T09:55:08Z

@rp- like Daan has advised, you could configure mvn/pom.xml rule to exclude the specific dependency you don't it to shade but ensure it copies the dependency in the lib directory (in the debian and rpm/spec build config file).

This makes it easier to apply patches, if the linstor library needs to be updated outside of cloudstack releases.

Create the template image resource before the host gets the copy command. This allowes to tag(set property) in linstor to which template this resource belongs to and we can maybe in future reduce the linstor managing code in the storagepoolAdaptor class.

rp- · 2023-11-07T10:07:26Z

@shwstppr I think I got it right now

shwstppr · 2023-11-07T10:19:34Z

@blueorangutan package

blueorangutan · 2023-11-07T10:20:03Z

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

shwstppr · 2023-11-08T09:04:25Z

@blueorangutan package

blueorangutan · 2023-11-08T09:06:03Z

@shwstppr 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 · 2023-11-08T10:05:34Z

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

shwstppr · 2023-11-08T10:08:15Z

@blueorangutan test

blueorangutan · 2023-11-08T10:10:03Z

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

blueorangutan · 2023-11-08T23:59:52Z

[SF] Trillian test result (tid-8271)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 48291 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8067-t8271-kvm-centos7.zip
Smoke tests completed. 116 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_migrate_vm	`Error`	44.85	test_vm_life_cycle.py

shwstppr · 2023-11-09T04:07:47Z

Tests look good now. One failure is unrelated. GH failures are codecov related. Thanks @rp- for fixing. Merging this.

This PR adds an config option for the Linstor primary storage driver, that allows you to automatically backup volume snapshots to the secondary storage. Additionally it will not mangle the need java-linstor dependency into the client.jar, but instead just copy the java-linstor.jar into lib. Config option is called: lin.backup.snapshots and is default false The scope of this change should be limited, as it only touches the Linstor driver and a part of copyAsync was implemented with 2 new Linstor specific commands.

boring-cyborg bot added component:build component:kvm component:primary-storage component:storage labels Oct 10, 2023

DaanHoogland reviewed Oct 10, 2023

View reviewed changes

yadvr added this to the 4.19.0.0 milestone Oct 10, 2023

rp- force-pushed the linstor-backup-snaphots branch from 8c28e64 to a99293d Compare October 12, 2023 07:13

rp- requested a review from DaanHoogland October 12, 2023 07:16

DaanHoogland approved these changes Oct 12, 2023

View reviewed changes

DaanHoogland added the has-3rdparty-components label Oct 12, 2023

rp- requested a review from DaanHoogland October 13, 2023 10:15

DaanHoogland reviewed Oct 13, 2023

View reviewed changes

...n/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java Outdated Show resolved Hide resolved

DaanHoogland reviewed Oct 13, 2023

View reviewed changes

...n/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java Outdated Show resolved Hide resolved

DaanHoogland reviewed Oct 13, 2023

View reviewed changes

...n/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java Show resolved Hide resolved

rp- force-pushed the linstor-backup-snaphots branch from 3429caa to 2b809cb Compare October 13, 2023 13:38

boring-cyborg bot added the component:UI label Oct 13, 2023

github-actions bot added the status:has-conflicts label Nov 6, 2023

rp- added 2 commits November 6, 2023 15:18

linstor: use prefix constant

f6aeca5

linstor: Add configurable snapshot to secondary storage backup feature

c0afcf5

rp- force-pushed the linstor-backup-snaphots branch from 51d914e to 6f21ce7 Compare November 6, 2023 14:19

github-actions bot removed the status:has-conflicts label Nov 6, 2023

rp- added 3 commits November 7, 2023 11:06

linstor: don't merge java-linstor into client jar

a5dbbd7

This makes it easier to apply patches, if the linstor library needs to be updated outside of cloudstack releases.

Linstor/StorageAdaptor: Improve debug/error logging

ec5ab15

rp- force-pushed the linstor-backup-snaphots branch from 6f21ce7 to ec5ab15 Compare November 7, 2023 10:06

shwstppr merged commit 68e504a into apache:main Nov 9, 2023

rp- deleted the linstor-backup-snaphots branch February 5, 2024 08:39

Linstor backup snaphots #8067

Linstor backup snaphots #8067

Uh oh!

Conversation

rp- commented Oct 10, 2023

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

How Has This Been Tested?

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

Uh oh!

codecov bot commented Oct 10, 2023 • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Oct 12, 2023

Uh oh!

blueorangutan commented Oct 12, 2023

Uh oh!

blueorangutan commented Oct 12, 2023

Uh oh!

rp- commented Oct 13, 2023

Uh oh!

DaanHoogland commented Oct 13, 2023

Uh oh!

rp- commented Oct 13, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rp- commented Oct 13, 2023

Uh oh!

DaanHoogland commented Oct 13, 2023

Uh oh!

blueorangutan commented Oct 13, 2023

Uh oh!

blueorangutan commented Oct 13, 2023

Uh oh!

DaanHoogland commented Oct 16, 2023

Uh oh!

blueorangutan commented Oct 16, 2023

Uh oh!

blueorangutan commented Oct 16, 2023

Uh oh!

rp- commented Nov 4, 2023

Uh oh!

github-actions bot commented Nov 6, 2023

Uh oh!

rp- commented Nov 6, 2023

Uh oh!

shwstppr commented Nov 7, 2023

Uh oh!

rp- commented Nov 7, 2023

Uh oh!

shwstppr commented Nov 7, 2023

Uh oh!

DaanHoogland commented Nov 7, 2023

Uh oh!

yadvr commented Nov 7, 2023

Uh oh!

rp- commented Nov 7, 2023

Uh oh!

shwstppr commented Nov 7, 2023

Uh oh!

codecov bot commented Oct 10, 2023 •

edited

Loading