Fix for create template from snapshot (for snapshots on primary storage and storage doesn't support create snapshot to template directly)

[sureshanaparti]

Description

This PR fixes create template from snapshot, for snapshots on primary storage and storage doesn't support create snapshot to template directly.

seems to be regression from #9478

Fixes #11451

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

• BLOCKER
• Critical
• Major
• Minor
• Trivial

Screenshots (if appropriate):

How Has This Been Tested?

Tested create volume snapshot (for volumes of running instance) with NFS & PowerFlex primary pools and NFS secondary, and created template & volume from snapshot on NFS primary, created template from snapshot on PowerFlex primary.

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

[sureshanaparti]

@blueorangutan package

[blueorangutan]

@sureshanaparti 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]

Codecov Report

❌ Patch coverage is 3.70370% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.36%. Comparing base (9fd2b90) to head (1aef01f).
⚠️ Report is 11 commits behind head on main.

Files with missing lines	Patch %	Lines
...n/java/com/cloud/template/TemplateManagerImpl.java	0.00%	12 Missing ⚠️
...stack/engine/orchestration/VolumeOrchestrator.java	0.00%	9 Missing ⚠️
...org/apache/cloudstack/snapshot/SnapshotHelper.java	16.66%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               main   #11452   +/-   ##
=========================================
  Coverage     17.36%   17.36%           
+ Complexity    15236    15235    -1     
=========================================
  Files          5886     5886           
  Lines        525645   525663   +18     
  Branches      64156    64158    +2     
=========================================
+ Hits          91257    91261    +4     
- Misses       424093   424107   +14     
  Partials      10295    10295           

Flag	Coverage Δ
uitests	3.63% <ø> (ø)
unittests	18.40% <3.70%> (+<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.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[blueorangutan]

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

[rp-]

Sorry no, still the same error.

[sureshanaparti]

Sorry no, still the same error.

ok

[rp-]

I'm trying a commit before #9478 to see if it really is the cause of it.

[sureshanaparti]

I'm trying a commit before #9478 to see if it really is the cause of it.

The if cond. added there matches for only StorPool. I'm assuming Linstor should take the else path, as snapshots will be in primary storage, not Image and earlier condition kvmSnapshotOnlyInPrimaryStorage is missing.

[rp-]

I'm trying a commit before #9478 to see if it really is the cause of it.

The if cond. added there matches for only StorPool. I'm assuming Linstor should take the else path, as snapshots will be in primary storage, not Image and earlier condition kvmSnapshotOnlyInPrimaryStorage is missing.

Volume snapshots per default go to secondary storage, we have an option to keep all snapshots on primary storage.
So it depends...

[rp-]

I can confirm that it worked before: #9478

[slavkap]

@rp-, Linstor is keeping only on primary or only on secondary storage? Does it have an option to keep the snapshot on both?

[rp-]

@rp-, Linstor is keeping only on primary or only on secondary storage? Does it have an option to keep the snapshot on both?

Yes, only on either primary or secondary. There is NO option for both.

[sureshanaparti]

@blueorangutan package

[blueorangutan]

@sureshanaparti 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]

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

[DaanHoogland]

clgtm, I think storage providers must test this in addition to smoke tests

[sureshanaparti]

@blueorangutan package

[blueorangutan]

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

[sureshanaparti]

Hi @slavkap @rp- Please verify template & volume creation from snapshot with this PR and confirm. Thanks.

[blueorangutan]

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

[slavkap]

code LGTM
created volume from a snapshot with NFS, Ceph and StorPool as primary storage
created template from a snapshot with NFS/Ceph and StorPool
with enabled/disabled snapshot.backup.to.secondary option

[DaanHoogland]

@rp- ,can you test as well (and maybe others?)

[rp-]

tested create template and create volume from snapshot and works now

[sureshanaparti]

thanks @slavkap @rp- for testing this.

[weizhouapache]

code lgtm

[nvazquez]

@blueorangutan test