diff --git a/api/src/main/java/org/apache/cloudstack/api/response/ImageStoreResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/ImageStoreResponse.java index 79f7eb295ea2..a617906eee1c 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/ImageStoreResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/ImageStoreResponse.java @@ -71,6 +71,14 @@ public class ImageStoreResponse extends BaseResponseWithAnnotations { @Param(description = "The host's currently used disk size") private Long diskSizeUsed; + @SerializedName(ApiConstants.S3_END_POINT) + @Param(description = "The S3 endpoint URL") + private String s3Endpoint; + + @SerializedName(ApiConstants.S3_BUCKET_NAME) + @Param(description = "The S3 bucket name") + private String s3BucketName; + public ImageStoreResponse() { } @@ -156,4 +164,20 @@ public void setDiskSizeTotal(Long diskSizeTotal) { public void setDiskSizeUsed(Long diskSizeUsed) { this.diskSizeUsed = diskSizeUsed; } + + public String getS3Endpoint() { + return s3Endpoint; + } + + public void setS3Endpoint(String s3Endpoint) { + this.s3Endpoint = s3Endpoint; + } + + public String getS3BucketName() { + return s3BucketName; + } + + public void setS3BucketName(String s3BucketName) { + this.s3BucketName = s3BucketName; + } } diff --git a/core/src/main/java/com/cloud/storage/template/S3TemplateDownloader.java b/core/src/main/java/com/cloud/storage/template/S3TemplateDownloader.java index 70df906d1ce3..9dd73c2a6562 100644 --- a/core/src/main/java/com/cloud/storage/template/S3TemplateDownloader.java +++ b/core/src/main/java/com/cloud/storage/template/S3TemplateDownloader.java @@ -216,8 +216,13 @@ public void progressChanged(ProgressEvent progressEvent) { // Wait for the upload to complete. upload.waitForCompletion(); } catch (InterruptedException e) { - // Interruption while waiting for the upload to complete. - logger.warn("Interruption occurred while waiting for upload of " + downloadUrl + " to complete"); + errorString = "Interruption occurred while waiting for upload of " + downloadUrl + " to complete"; + logger.warn(errorString); + status = Status.UNRECOVERABLE_ERROR; + } catch (Exception e) { + errorString = "S3 upload failed for " + downloadUrl + ": " + e.getMessage(); + logger.warn(errorString, e); + status = Status.UNRECOVERABLE_ERROR; } downloadTime = new Date().getTime() - start.getTime(); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java index 7e9f65f43b34..1d3b781afb12 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java @@ -400,7 +400,9 @@ public EndPoint select(DataObject object) { } if (object instanceof TemplateInfo) { TemplateInfo tmplInfo = (TemplateInfo)object; - if (store.getScope().getScopeType() == ScopeType.ZONE && store.getScope().getScopeId() == null && tmplInfo.getTemplateType() == TemplateType.SYSTEM) { + if (tmplInfo.getTemplateType() == TemplateType.SYSTEM && + (store.getScope().getScopeType() == ScopeType.REGION || + (store.getScope().getScopeType() == ScopeType.ZONE && store.getScope().getScopeId() == null))) { return LocalHostEndpoint.getEndpoint(); // for bootstrap system vm template downloading to region image store } } diff --git a/server/src/main/java/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java index 9a0c271fdb48..4af590ab6265 100644 --- a/server/src/main/java/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java @@ -18,6 +18,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import javax.inject.Inject; @@ -26,7 +27,10 @@ import com.cloud.user.AccountManager; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; +import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider; +import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao; import org.springframework.stereotype.Component; import org.apache.cloudstack.api.response.ImageStoreResponse; @@ -48,6 +52,8 @@ public class ImageStoreJoinDaoImpl extends GenericDaoBase dsSearch; @@ -92,6 +98,14 @@ public ImageStoreResponse newImageStoreResponse(ImageStoreJoinVO ids) { osResponse.setHasAnnotation(annotationDao.hasAnnotations(ids.getUuid(), AnnotationService.EntityType.SECONDARY_STORAGE.name(), accountManager.isRootAdmin(CallContext.current().getCallingAccount().getId()))); + if (DataStoreProvider.S3_IMAGE.equalsIgnoreCase(ids.getProviderName())) { + Map s3Details = imageStoreDetailsDao.getDetails(ids.getId()); + if (s3Details != null) { + osResponse.setS3Endpoint(s3Details.get(ApiConstants.S3_END_POINT)); + osResponse.setS3BucketName(s3Details.get(ApiConstants.S3_BUCKET_NAME)); + } + } + osResponse.setObjectName("imagestore"); return osResponse; } diff --git a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java index 5698632249d3..66aab2a39c8f 100644 --- a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java +++ b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java @@ -1247,6 +1247,10 @@ public boolean finalizeVirtualMachineProfile(VirtualMachineProfile profile, Depl protected void addSecondaryStorageServerAddressToBuffer(StringBuilder buffer, List dataStores, String vmName) { List addresses = new ArrayList<>(); for (DataStore dataStore: dataStores) { + // S3 and other object stores may not have a URL, so better to skip them + if (dataStore == null || dataStore.getTO() == null || dataStore.getTO().getUrl() == null) { + continue; + } String url = dataStore.getTO().getUrl(); String[] urlArray = url.split("/"); diff --git a/services/secondary-storage/controller/src/test/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImplTest.java b/services/secondary-storage/controller/src/test/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImplTest.java index 83596b64ec0f..d5719aee398b 100644 --- a/services/secondary-storage/controller/src/test/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImplTest.java +++ b/services/secondary-storage/controller/src/test/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImplTest.java @@ -117,6 +117,45 @@ public void testAddSecondaryStorageServerAddressToBufferInvalidAddress() { runAddSecondaryStorageServerAddressToBufferTest(addresses, StringUtils.join(List.of(randomIp1, randomIp2), ",")); } + @Test + public void testAddSecondaryStorageServerAddressToBufferWithNullEntries() { + String randomIp1 = InetAddresses.fromInteger(secureRandom.nextInt()).getHostAddress(); + String randomIp2 = InetAddresses.fromInteger(secureRandom.nextInt()).getHostAddress(); + + List dataStores = new ArrayList<>(); + + DataStore validStore1 = Mockito.mock(DataStore.class); + DataStoreTO validStoreTO1 = Mockito.mock(DataStoreTO.class); + when(validStoreTO1.getUrl()).thenReturn(String.format("http://%s", randomIp1)); + when(validStore1.getTO()).thenReturn(validStoreTO1); + dataStores.add(validStore1); + + dataStores.add(null); + + DataStore nullToStore = Mockito.mock(DataStore.class); + when(nullToStore.getTO()).thenReturn(null); + dataStores.add(nullToStore); + + DataStore nullUrlStore = Mockito.mock(DataStore.class); + DataStoreTO nullUrlStoreTO = Mockito.mock(DataStoreTO.class); + when(nullUrlStoreTO.getUrl()).thenReturn(null); + when(nullUrlStore.getTO()).thenReturn(nullUrlStoreTO); + dataStores.add(nullUrlStore); + + DataStore validStore2 = Mockito.mock(DataStore.class); + DataStoreTO validStoreTO2 = Mockito.mock(DataStoreTO.class); + when(validStoreTO2.getUrl()).thenReturn(String.format("http://%s", randomIp2)); + when(validStore2.getTO()).thenReturn(validStoreTO2); + dataStores.add(validStore2); + + StringBuilder builder = new StringBuilder(); + secondaryStorageManager.addSecondaryStorageServerAddressToBuffer(builder, dataStores, "VM"); + String result = builder.toString(); + result = result.contains("=") ? result.split("=")[1] : null; + + assertEquals(StringUtils.join(List.of(randomIp1, randomIp2), ","), result); + } + @Test public void testCreateSecondaryStorageVm_New() { long dataCenterId = 1L; diff --git a/ui/src/config/section/infra/secondaryStorages.js b/ui/src/config/section/infra/secondaryStorages.js index 3fc64c5c9575..cedc893878a9 100644 --- a/ui/src/config/section/infra/secondaryStorages.js +++ b/ui/src/config/section/infra/secondaryStorages.js @@ -36,7 +36,7 @@ export default { return fields }, details: () => { - var fields = ['name', 'id', 'url', 'protocol', 'provider', 'scope', 'zonename'] + var fields = ['name', 'id', 'url', 'protocol', 'provider', 'scope', 'zonename', 'endpoint', 'bucket'] if (store.getters.apis.listImageStores.params.filter(x => x.name === 'readonly').length > 0) { fields.push('readonly') } diff --git a/utils/src/main/java/com/cloud/utils/storage/S3/S3Utils.java b/utils/src/main/java/com/cloud/utils/storage/S3/S3Utils.java index 6d85d2d1dadb..c04e14a9a0ba 100644 --- a/utils/src/main/java/com/cloud/utils/storage/S3/S3Utils.java +++ b/utils/src/main/java/com/cloud/utils/storage/S3/S3Utils.java @@ -114,6 +114,8 @@ public static TransferManager getTransferManager(final ClientOptions clientOptio LOGGER.debug(format("Setting the end point for S3 client with access key %1$s to %2$s.", clientOptions.getAccessKey(), clientOptions.getEndPoint())); client.setEndpoint(clientOptions.getEndPoint()); + // Enable path-style access for S3-compatible storage + client.setS3ClientOptions(com.amazonaws.services.s3.S3ClientOptions.builder().setPathStyleAccess(true).build()); } TRANSFERMANAGER_ACCESSKEY_MAP.put(clientOptions.getAccessKey(), new TransferManager(client));