Fix automatic SystemVM template download to S3 secondary storage #12426

Damans227 · 2026-01-14T19:22:51Z

Description

This PR fixes an issue where the SystemVM template is not automatically downloaded to S3 secondary storage when adding it to a CloudStack zone.

Root Cause:
S3 stores use REGION scope but DefaultEndPointSelector only returned LocalHostEndpoint for ZONE scope, so no endpoint was found to download the SystemVM template.

Fix:
Allow LocalHostEndpoint to handle SYSTEM template downloads for REGION-scoped stores, plus added null checks for S3 stores without URLs and enabled path-style access for S3-compatible storage.

Fixes: #9002

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

Screenshots (if appropriate):

Broken:

Fixed:

Screencast.from.2026-01-14.13-52-40.mp4

How Has This Been Tested?

Test Environment:

CloudStack 4.20.3.0
KVM hypervisor (Ubuntu 22.04)
MinIO S3-compatible storage

Test Steps:

Started with a fresh CloudStack zone with no secondary storage configured
Added NFS staging store (required for S3)
Added S3 image store (MinIO) via UI
Verified SystemVM template automatically downloaded to S3
Verified SSVM booted successfully using the template from S3
Verified Console Proxy VM also booted successfully

…e scopes

nvazquez · 2026-01-15T03:05:13Z

@blueorangutan package

blueorangutan · 2026-01-15T03:06:34Z

@nvazquez 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 · 2026-01-15T03:55:40Z

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

codecov · 2026-01-15T03:56:14Z

Codecov Report

❌ Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.23%. Comparing base (6a324da) to head (9a2ba84).
⚠️ Report is 2 commits behind head on 4.20.

Files with missing lines	Patch %	Lines
...tack/storage/endpoint/DefaultEndPointSelector.java	0.00%	3 Missing ⚠️
.../main/java/com/cloud/utils/storage/S3/S3Utils.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.20   #12426      +/-   ##
============================================
- Coverage     16.23%   16.23%   -0.01%     
- Complexity    13382    13385       +3     
============================================
  Files          5657     5657              
  Lines        498999   499001       +2     
  Branches      60566    60569       +3     
============================================
- Hits          81035    81033       -2     
- Misses       408928   408931       +3     
- Partials       9036     9037       +1

Flag	Coverage Δ
uitests	`4.03% <ø> (ø)`
unittests	`17.09% <33.33%> (-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.

nvazquez · 2026-01-15T04:03:11Z

@blueorangutan test

blueorangutan · 2026-01-15T04:04:34Z

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

blueorangutan · 2026-01-15T09:09:51Z

[SF] Trillian Build Failed (tid-15187)

vishesh92 · 2026-01-16T05:57:40Z

utils/src/main/java/com/cloud/utils/storage/S3/S3Utils.java


            client.setEndpoint(clientOptions.getEndPoint());
+            // Enable path-style access for S3-compatible storage
+            client.setS3ClientOptions(com.amazonaws.services.s3.S3ClientOptions.builder().setPathStyleAccess(true).build());


Why is this required?

So, when debugging the issue... I noticed that the connection to MinIO failed at the time of template upload, with an error that looked something like:

UnknownHostException: cloudstack-secondary.10.0.34.157 i.e. the SDK was trying to connect to the http://cloudstack-secondary.10.0.34.157:9000/... which is the virtual-hosted style (refer: virtual style vs path style syntax for s3).

Looking at other S3-compatible plugins in CloudStack, I found that both CephObjectStoreDriverImpl and CloudianHyperStoreUtil use enablePathStyleAccess() to get path-style URLs http://10.0.34.157:9000/cloudstack-secondary/... i.e.

AmazonS3 client = AmazonS3ClientBuilder.standard() .enablePathStyleAccess() .withCredentials(new AWSStaticCredentialsProvider(new BasicAWSCredentials(accessKey, secretKey))) .withEndpointConfiguration(new AwsClientBuilder.EndpointConfiguration(url, "auto")) .build();

Applying the same fix here worked. The AWS SDK documentation confirms that path-style access must be explicitly enabled for S3-compatible stores.

Copilot

Pull request overview

This PR fixes an issue where SystemVM templates fail to automatically download to S3 secondary storage when adding it to a CloudStack zone. The root cause was that S3 stores use REGION scope, but the endpoint selector only returned LocalHostEndpoint for ZONE-scoped stores with null scope IDs.

Changes:

Modified endpoint selection logic to support REGION-scoped stores for SYSTEM template downloads
Added null safety checks for data stores without URLs (e.g., S3 object stores)
Enabled path-style access for S3-compatible storage systems like MinIO

Reviewed changes

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

File	Description
engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java	Extended condition to allow LocalHostEndpoint for REGION-scoped stores with SYSTEM templates
services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java	Added null checks to skip data stores without URLs when building secondary storage addresses
services/secondary-storage/controller/src/test/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImplTest.java	Added comprehensive test coverage for null handling in data store processing
utils/src/main/java/com/cloud/utils/storage/S3/S3Utils.java	Enabled path-style access for S3-compatible storage systems

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

Copilot · 2026-01-16T06:03:12Z

utils/src/main/java/com/cloud/utils/storage/S3/S3Utils.java

+            // Enable path-style access for S3-compatible storage
+            client.setS3ClientOptions(com.amazonaws.services.s3.S3ClientOptions.builder().setPathStyleAccess(true).build());


Path-style access is being enabled unconditionally for all S3 endpoints, including AWS S3 which deprecated path-style access in favor of virtual-hosted-style. This could cause compatibility issues with AWS S3. Consider making path-style access configurable through ClientOptions, or only enabling it when a custom endpoint is detected (non-AWS S3).

hmm, path-style access code is already inside the if (StringUtils.isNotBlank(clientOptions.getEndPoint())) block - which means path-style access is only enabled when a custom endpoint is specified. The code is already doing what Copilot is asking for.

Copilot · 2026-01-16T06:03:12Z

...ne/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java

+            if (tmplInfo.getTemplateType() == TemplateType.SYSTEM &&
+                (store.getScope().getScopeType() == ScopeType.REGION ||
+                 (store.getScope().getScopeType() == ScopeType.ZONE && store.getScope().getScopeId() == null))) {


The modified endpoint selection logic for REGION-scoped SYSTEM templates lacks test coverage. Consider adding unit tests in the engine/storage module to verify that LocalHostEndpoint is correctly returned for REGION-scoped stores with SYSTEM templates, similar to the existing test coverage in SecondaryStorageManagerImplTest.

Daman Arora added 4 commits January 14, 2026 14:12

Allow endpoint selection logic for system templates in region and zon…

387f958

…e scopes

Skip null or missing URLs for S3

dd39393

Enable path-style access for S3-compatible storage

d6bf807

Add test for handling null entries in secondary storage

9a2ba84

boring-cyborg bot added component:secondary-storage component:ssvm component:storage labels Jan 14, 2026

apache deleted a comment from blueorangutan Jan 15, 2026

vishesh92 requested a review from Copilot January 16, 2026 05:53

Copilot started reviewing on behalf of vishesh92 January 16, 2026 05:53 View session

vishesh92 reviewed Jan 16, 2026

View reviewed changes

Copilot AI reviewed Jan 16, 2026

View reviewed changes

Damans227 requested a review from vishesh92 January 16, 2026 17:13

		// Enable path-style access for S3-compatible storage
		client.setS3ClientOptions(com.amazonaws.services.s3.S3ClientOptions.builder().setPathStyleAccess(true).build());

Fix automatic SystemVM template download to S3 secondary storage #12426

Are you sure you want to change the base?

Fix automatic SystemVM template download to S3 secondary storage #12426

Conversation

Damans227 commented Jan 14, 2026

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

nvazquez commented Jan 15, 2026

Uh oh!

blueorangutan commented Jan 15, 2026

Uh oh!

blueorangutan commented Jan 15, 2026

Uh oh!

codecov bot commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nvazquez commented Jan 15, 2026

Uh oh!

blueorangutan commented Jan 15, 2026

Uh oh!

blueorangutan commented Jan 15, 2026

Uh oh!

vishesh92 Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Damans227 Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Damans227 Jan 16, 2026 • 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!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Jan 15, 2026 •

edited

Loading

Damans227 Jan 16, 2026 •

edited

Loading