Skip to content

Add config keys for controlling public/private template secondary storage replica counts - #12877

Merged
weizhouapache merged 16 commits into
apache:mainfrom
Damans227:feature/FR284-template-secstorage-copy-control
Jun 29, 2026
Merged

Add config keys for controlling public/private template secondary storage replica counts#12877
weizhouapache merged 16 commits into
apache:mainfrom
Damans227:feature/FR284-template-secstorage-copy-control

Conversation

@Damans227

@Damans227Damans227 commented Mar 23, 2026

Copy link
Copy Markdown
Collaborator

Description

Adds two new operator-level configuration keys to control the number of secondary storage copies made for public and private templates, decoupling replica count from template visibility.

  • secstorage.public.template.copy.max (default: 0 = all stores, preserving existing behavior)
  • secstorage.private.template.copy.max (default: 1, preserving existing behavior)

Doc PR: apache/cloudstack-documentation#643

Types of changes

  • Breaking change
  • New feature
  • Bug fix
  • Enhancement
  • Cleanup
  • Build/CI
  • Test

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

How Has This Been Tested?

  • Unit tests in HypervisorTemplateAdapterTest covering the new replica-limit logic in isZoneAndImageStoreAvailable.
  • Manually verified on a zone with multiple secondary storage pools.

@codecov

codecovBot commented Mar 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 21.39535% with 169 lines in your changes missing coverage. Please review.
✅ Project coverage is 18.94%. Comparing base (82986f6) to head (655ce0c).

Files with missing linesPatch %Lines
.../cloudstack/storage/image/TemplateServiceImpl.java20.49%121 Missing and 7 partials ⚠️
...n/java/com/cloud/template/TemplateManagerImpl.java0.00%26 Missing ⚠️
...com/cloud/storage/ImageStoreUploadMonitorImpl.java0.00%6 Missing ⚠️
.../com/cloud/template/HypervisorTemplateAdapter.java37.50%5 Missing ⚠️
...n/java/com/cloud/template/TemplateAdapterBase.java66.66%4 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #12877 +/- ##
============================================
+ Coverage 3.51% 18.94% +15.42% - Complexity 0 18373 +18373 
============================================
Files 469 6192 +5723 Lines 40471 556550 +516079 Branches 7615 67954 +60339 ============================================
+ Hits 1424 105431 +104007 - Misses 38856 439541 +400685 - Partials 191 11578 +11387 
FlagCoverage Δ
uitests3.51% <ø> (ø)
unittests20.15% <21.39%> (?)

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

☔ View full report in Codecov by Harness.
📢 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.

@Damans227

Copy link
Copy Markdown
CollaboratorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@Damans227 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces new template-related configuration keys intended to control how many secondary storage replicas are created for public vs private templates, decoupling replication behavior from template visibility.

Changes:

  • Adds two new Zone-scoped ConfigKey<Integer> settings: public.template.secstorage.copy and private.template.secstorage.copy.
  • Reworks secondary storage allocation logic to enforce a per-zone replica limit using a zone→count map.
  • Updates unit tests to match the new method signatures/replica-limit behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
FileDescription
engine/components-api/src/main/java/com/cloud/template/TemplateManager.javaAdds new Zone-scoped config keys for public/private template replica limits.
server/src/main/java/com/cloud/template/TemplateManagerImpl.javaRegisters the new config keys via getConfigKeys().
server/src/main/java/com/cloud/template/TemplateAdapterBase.javaReplaces zone-set visibility logic with zone copy counting + replica limit enforcement.
server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.javaThreads replica-limit and zone copy count through template creation/allocation paths.
server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.javaUpdates tests for the new signatures and adds coverage for replica-limit behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadserver/src/main/java/com/cloud/template/TemplateAdapterBase.java Outdated
Comment on lines 202 to 206
/**
* If the template/ISO is marked as private, then it is allocated to a random secondary storage; otherwise, allocates to every storage pool in every zone given by the
* {@link TemplateProfile#getZoneIdList()}.
*/
protected void postUploadAllocation(List<DataStore> imageStores, VMTemplateVO template, List<TemplateOrVolumePostUploadCommand> payloads) {

CopilotAIMar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method JavaDoc no longer matches the behavior: allocation is now governed by replica-limit config keys (public/private can both be limited), not strictly by template visibility (private=random vs public=all). Please update the comment to reflect the new replica-limit semantics (and that the limits are zone-scoped).

Copilot uses AI. Check for mistakes.
Comment threadserver/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java Outdated
Comment threadserver/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java Outdated
Comment on lines +69 to +79
ConfigKey<Integer> PublicTemplateSecStorageCopy = new ConfigKey<Integer>("Advanced", Integer.class,
PublicTemplateSecStorageCopyCK, "0",
"Maximum number of secondary storage pools to which a public template is copied. " +
"0 means copy to all secondary storage pools (default behavior).",
true, ConfigKey.Scope.Zone);

ConfigKey<Integer> PrivateTemplateSecStorageCopy = new ConfigKey<Integer>("Advanced", Integer.class,
PrivateTemplateSecStorageCopyCK, "1",
"Maximum number of secondary storage pools to which a private template is copied. " +
"Default is 1 to preserve existing behavior.",
true, ConfigKey.Scope.Zone);

CopilotAIMar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The config key descriptions don’t mention that the limits are evaluated per-zone (and the keys themselves are zone-scoped). Consider clarifying the wording (e.g., “per zone”) so operators understand how replica limits are applied across multiple zones/image stores.

Copilot uses AI. Check for mistakes.
Comment threadengine/components-api/src/main/java/com/cloud/template/TemplateManager.java Outdated
Comment threadengine/components-api/src/main/java/com/cloud/template/TemplateManager.java Outdated
Comment threadserver/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java Outdated
Comment threadserver/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java Outdated

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadserver/src/main/java/com/cloud/template/TemplateAdapterBase.java Outdated
@Damans227

Copy link
Copy Markdown
CollaboratorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@Damans227 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

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

@sureshanaparti

Copy link
Copy Markdown
Contributor

@Damans227 update the description with update config names.

@sureshanapartisureshanaparti left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

@Damans227

Copy link
Copy Markdown
CollaboratorAuthor

@Damans227 update the description with update config names.

Done. Thanks!

@Damans227

Copy link
Copy Markdown
CollaboratorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@Damans227 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

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

@Damans227

Copy link
Copy Markdown
CollaboratorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@Damans227 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

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

@kiranchavala

Copy link
Copy Markdown
Member

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian Build Failed (tid-16260)

@kiranchavala

Copy link
Copy Markdown
Member

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-16286)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 49957 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12877-t16286-kvm-ol8.zip
Smoke tests completed. 151 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

TestResultTime (s)Test File

@kiranchavalakiranchavala left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Tested the feature manually.

The Public and private Templates honor the global setting value of secstorage.public.template.copy.max and secstorage.private.template.copy.max and copies the template to the respective secondary storage.

validate secstorage.public.template.copy.max at global and zone levelPass
validate secstorage.private.template.copy.max at global and zone levelPass
Validate publlic and private template creation operation from volumePass
Validate publlic and private template creation operation from a volume snapshotPass
Validate Upload Template operation from LocalPass
Validate Copy Template operation from one zone to anotherPass
Validate Update Template operation from public to private and vice versaPass

@weizhouapache

Copy link
Copy Markdown
Member

LGTM

Tested the feature manually.

The Public and private Templates honor the global setting value of secstorage.public.template.copy.max and secstorage.private.template.copy.max and copies the template to the respective secondary storage.

validate secstorage.public.template.copy.max at global and zone level Pass
validate secstorage.private.template.copy.max at global and zone level Pass
Validate publlic and private template creation operation from volume Pass
Validate publlic and private template creation operation from a volume snapshot Pass
Validate Upload Template operation from Local Pass
Validate Copy Template operation from one zone to another Pass
Validate Update Template operation from public to private and vice versa Pass

great, thanks @kiranchavala

@harikrishna-patnala
will you review the feature before merge ?

@weizhouapache
weizhouapache marked this pull request as ready for review June 24, 2026 09:16
CopilotAI review requested due to automatic review settings June 24, 2026 09:16

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown

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

@Damans227

Copy link
Copy Markdown
CollaboratorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@Damans227 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

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

@rajujithrajujith left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Tested.

@weizhouapache

Copy link
Copy Markdown
Member

LGTM Tested.

thanks @rajujith@kiranchavala@Damans227

merging

@weizhouapache
weizhouapache merged commit 5ed4894 into apache:mainJun 29, 2026
24 of 27 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@Damans227@blueorangutan@sureshanaparti@weizhouapache@kiranchavala@rajujith@winterhazel