Merge branch '4.20' into 4.22

Author: Daan Hoogland

api/src/main/java/org/apache/cloudstack/api/command/user/template/CreateTemplateCmd.java

@@ -211,7 +211,7 @@ public String getUrl() {
     }
 
     public String getTemplateTag() {
-        return templateTag;
+        return StringUtils.isBlank(templateTag) ? null : templateTag;
     }
 
     public Map getDetails() {

api/src/main/java/org/apache/cloudstack/api/command/user/template/GetUploadParamsForTemplateCmd.java

@@ -170,7 +170,7 @@ public Boolean isSshKeyEnabled() {
     }
 
     public String getTemplateTag() {
-        return templateTag;
+        return StringUtils.isBlank(templateTag) ? null : templateTag;
     }
 
     public boolean isDeployAsIs() {

api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java

@@ -279,7 +279,7 @@ public String getChecksum() {
     }
 
     public String getTemplateTag() {
-        return templateTag;
+        return StringUtils.isBlank(templateTag) ? null : templateTag;
     }
 
     public Map getDetails() {

debian/rules

@@ -4,7 +4,6 @@ VERSION := $(shell grep '<version>' pom.xml | head -2 | tail -1 | cut -d'>' -f2
 PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f 1)
 SYSCONFDIR = "/etc"
 DESTDIR = "debian/tmp"
-CMK_REL := $(shell wget -O - "https://api.github.com/repos/apache/cloudstack-cloudmonkey/releases" 2>/dev/null | jq -r '.[0].tag_name')
 
 %:
 	dh $@ --with systemd
@@ -90,7 +89,7 @@ override_dh_auto_install:
 	rm -rf $(DESTDIR)/usr/share/$(PACKAGE)-management/templates/systemvm/sha512sum.txt
 
 	# Bundle cmk in cloudstack-management
-	wget https://github.com/apache/cloudstack-cloudmonkey/releases/download/$(CMK_REL)/cmk.linux.x86-64 -O $(DESTDIR)/usr/bin/cmk
+	wget https://github.com/apache/cloudstack-cloudmonkey/releases/latest/download/cmk.linux.x86-64 -O $(DESTDIR)/usr/bin/cmk
 	chmod +x $(DESTDIR)/usr/bin/cmk
 
 	# nast hack for a couple of configuration files

engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java

@@ -18,12 +18,18 @@
 package org.apache.cloudstack.engine.orchestration.service;
 
 import java.util.List;
+import java.util.concurrent.Future;
 
 import org.apache.cloudstack.api.response.MigrationResponse;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult;
 import org.apache.cloudstack.storage.ImageStoreService.MigrationPolicy;
 
 public interface StorageOrchestrationService {
     MigrationResponse migrateData(Long srcDataStoreId, List<Long> destDatastores, MigrationPolicy migrationPolicy);
 
     MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreId, List<Long> templateIdList, List<Long> snapshotIdList);
+
+    Future<TemplateApiResult> orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore);
 }

engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java

@@ -78,4 +78,6 @@ public TemplateInfo getTemplate() {
     AsyncCallFuture<TemplateApiResult> createDatadiskTemplateAsync(TemplateInfo parentTemplate, TemplateInfo dataDiskTemplate, String path, String diskId, long fileSize, boolean bootable);
 
     List<DatadiskTO> getTemplateDatadisksOnImageStore(TemplateInfo templateInfo, String configurationId);
+
+    AsyncCallFuture<TemplateApiResult> copyTemplateToImageStore(DataObject source, DataStore destStore);
 }

engine/components-api/src/main/java/com/cloud/storage/StorageManager.java

@@ -161,22 +161,22 @@ public interface StorageManager extends StorageService {
             ConfigKey.Scope.StoragePool,
             null);
 
-    ConfigKey<Integer> PRIMARY_STORAGE_DOWNLOAD_WAIT = new ConfigKey<Integer>("Storage", Integer.class, "primary.storage.download.wait", "10800",
+    ConfigKey<Integer> PRIMARY_STORAGE_DOWNLOAD_WAIT = new ConfigKey<>("Storage", Integer.class, "primary.storage.download.wait", "10800",
             "In second, timeout for download template to primary storage", false);
 
-    ConfigKey<Integer>  SecStorageMaxMigrateSessions = new ConfigKey<Integer>("Advanced", Integer.class, "secstorage.max.migrate.sessions", "2",
+    ConfigKey<Integer>  SecStorageMaxMigrateSessions = new ConfigKey<>("Advanced", Integer.class, "secstorage.max.migrate.sessions", "2",
             "The max number of concurrent copy command execution sessions that an SSVM can handle", false, ConfigKey.Scope.Global);
 
-    ConfigKey<Boolean>  SecStorageVMAutoScaleDown = new ConfigKey<Boolean>("Advanced", Boolean.class, "secstorage.vm.auto.scale.down", "false",
+    ConfigKey<Boolean>  SecStorageVMAutoScaleDown = new ConfigKey<>("Advanced", Boolean.class, "secstorage.vm.auto.scale.down", "false",
             "Setting this to 'true' will auto scale down SSVMs", true, ConfigKey.Scope.Global);
 
-    ConfigKey<Integer> MaxDataMigrationWaitTime = new ConfigKey<Integer>("Advanced", Integer.class, "max.data.migration.wait.time", "15",
+    ConfigKey<Integer> MaxDataMigrationWaitTime = new ConfigKey<>("Advanced", Integer.class, "max.data.migration.wait.time", "15",
             "Maximum wait time (in minutes) for a data migration task before spawning a new SSVM", false, ConfigKey.Scope.Global);
-    ConfigKey<Boolean> DiskProvisioningStrictness = new ConfigKey<Boolean>("Storage", Boolean.class, "disk.provisioning.type.strictness", "false",
+    ConfigKey<Boolean> DiskProvisioningStrictness = new ConfigKey<>("Storage", Boolean.class, "disk.provisioning.type.strictness", "false",
             "If set to true, the disk is created only when there is a suitable storage pool that supports the disk provisioning type specified by the service/disk offering. " +
                     "If set to false, the disk is created with a disk provisioning type supported by the pool. Default value is false, and this is currently supported for VMware only.",
             true, ConfigKey.Scope.Zone);
-    ConfigKey<String> PreferredStoragePool = new ConfigKey<String>(String.class, "preferred.storage.pool", "Advanced", "",
+    ConfigKey<String> PreferredStoragePool = new ConfigKey<>(String.class, "preferred.storage.pool", "Advanced", "",
             "The UUID of preferred storage pool for allocation.", true, ConfigKey.Scope.Account, null);
 
     ConfigKey<Boolean> MountDisabledStoragePool = new ConfigKey<>(Boolean.class,
@@ -203,15 +203,15 @@ public interface StorageManager extends StorageService {
             true,
             ConfigKey.Scope.Global,
             null);
-    static final ConfigKey<Boolean> DataStoreDownloadFollowRedirects = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED,
+    ConfigKey<Boolean> DataStoreDownloadFollowRedirects = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED,
             Boolean.class, "store.download.follow.redirects", "false",
             "Whether HTTP redirect is followed during store downloads for objects such as template, volume etc.",
             true, ConfigKey.Scope.Global);
 
     ConfigKey<Long> HEURISTICS_SCRIPT_TIMEOUT = new ConfigKey<>("Advanced", Long.class, "heuristics.script.timeout", "3000",
             "The maximum runtime, in milliseconds, to execute the heuristic rule; if it is reached, a timeout will happen.", true);
 
-    ConfigKey<Boolean> AllowVolumeReSizeBeyondAllocation = new ConfigKey<Boolean>("Advanced", Boolean.class, "volume.resize.allowed.beyond.allocation", "false",
+    ConfigKey<Boolean> AllowVolumeReSizeBeyondAllocation = new ConfigKey<>("Advanced", Boolean.class, "volume.resize.allowed.beyond.allocation", "false",
             "Determines whether volume size can exceed the pool capacity allocation disable threshold (pool.storage.allocated.capacity.disablethreshold) " +
                     "when resize a volume upto resize capacity disable threshold (pool.storage.allocated.resize.capacity.disablethreshold)",
             true, List.of(ConfigKey.Scope.StoragePool, ConfigKey.Scope.Zone));
@@ -228,6 +228,10 @@ public interface StorageManager extends StorageService {
             ConfigKey.Scope.Global,
             null);
 
+    ConfigKey<Boolean> COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES = new ConfigKey<>(Boolean.class, "copy.public.templates.from.other.storages",
+            "Storage", "true", "Allow SSVMs to try copying public templates from one secondary storage to another instead of downloading them from the source.",
+            true, ConfigKey.Scope.Zone, null);
+
     /**
      * should we execute in sequence not involving any storages?
      * @return tru if commands should execute in sequence
@@ -252,14 +256,14 @@ static Boolean getFullCloneConfiguration(Long storeId) {
 
     /**
      * Returns a comma separated list of tags for the specified storage pool
-     * @param poolId
+     * @param poolId the id of the pool to get tags for
      * @return comma separated list of tags
      */
     String getStoragePoolTags(long poolId);
 
     /**
      * Returns a list of Strings with tags for the specified storage pool
-     * @param poolId
+     * @param poolId the id of the pool to get tags for
      * @return comma separated list of tags
      */
     List<String> getStoragePoolTagList(long poolId);
@@ -276,7 +280,7 @@ static Boolean getFullCloneConfiguration(Long storeId) {
 
     Pair<Long, Answer> sendToPool(StoragePool pool, long[] hostIdsToTryFirst, List<Long> hostIdsToAvoid, Command cmd) throws StorageUnavailableException;
 
-    public Answer getVolumeStats(StoragePool pool, Command cmd);
+    Answer getVolumeStats(StoragePool pool, Command cmd);
 
     boolean canPoolProvideStorageStats(StoragePool pool);
 

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java

@@ -22,10 +22,12 @@
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import javax.inject.Inject;
 
@@ -206,12 +208,22 @@ public int compare(DataObject o1, DataObject o2) {
 
     protected List<DataObject> getAllReadyTemplates(DataStore srcDataStore, Map<DataObject, Pair<List<TemplateInfo>, Long>> childTemplates, List<TemplateDataStoreVO> templates) {
         List<TemplateInfo> files = new LinkedList<>();
+        Set<Long> idsForMigration = new HashSet<>();
+
         for (TemplateDataStoreVO template : templates) {
-            VMTemplateVO templateVO = templateDao.findById(template.getTemplateId());
-            if (shouldMigrateTemplate(template, templateVO)) {
-                files.add(templateFactory.getTemplate(template.getTemplateId(), srcDataStore));
+            long templateId = template.getTemplateId();
+            if (idsForMigration.contains(templateId)) {
+                logger.warn("Template store reference [{}] is duplicated; not considering it for migration.", template);
+                continue;
+            }
+            VMTemplateVO templateVO = templateDao.findById(templateId);
+            if (!shouldMigrateTemplate(template, templateVO)) {
+                continue;
             }
+            files.add(templateFactory.getTemplate(template.getTemplateId(), srcDataStore));
+            idsForMigration.add(templateId);
         }
+
         for (TemplateInfo template: files) {
             List<VMTemplateVO> children = templateDao.listByParentTemplatetId(template.getId());
             List<TemplateInfo> temps = new ArrayList<>();
@@ -221,6 +233,7 @@ protected List<DataObject> getAllReadyTemplates(DataStore srcDataStore, Map<Data
             }
             childTemplates.put(template, new Pair<>(temps, getTotalChainSize(temps)));
         }
+
         return (List<DataObject>) (List<?>) files;
     }
 
@@ -263,16 +276,37 @@ protected boolean shouldMigrateTemplate(TemplateDataStoreVO template, VMTemplate
      */
     protected List<DataObject> getAllReadySnapshotsAndChains(DataStore srcDataStore, Map<DataObject, Pair<List<SnapshotInfo>, Long>> snapshotChains, List<SnapshotDataStoreVO> snapshots) {
         List<SnapshotInfo> files = new LinkedList<>();
+        Set<Long> idsForMigration = new HashSet<>();
+
         for (SnapshotDataStoreVO snapshot : snapshots) {
-            SnapshotVO snapshotVO = snapshotDao.findById(snapshot.getSnapshotId());
-            if (snapshot.getState() == ObjectInDataStoreStateMachine.State.Ready &&
-                    snapshotVO != null && snapshotVO.getHypervisorType() != Hypervisor.HypervisorType.Simulator
-                    && snapshot.getParentSnapshotId() == 0 ) {
-                SnapshotInfo snap = snapshotFactory.getSnapshot(snapshotVO.getSnapshotId(), snapshot.getDataStoreId(), snapshot.getRole());
-                if (snap != null) {
-                    files.add(snap);
-                }
+            long snapshotId = snapshot.getSnapshotId();
+            if (idsForMigration.contains(snapshotId)) {
+                logger.warn("Snapshot store reference [{}] is duplicated; not considering it for migration.", snapshot);
+                continue;
+            }
+            if (snapshot.getState() != ObjectInDataStoreStateMachine.State.Ready) {
+                logger.warn("Not migrating snapshot [{}] because its state is not ready.", snapshot);
+                continue;
+            }
+            SnapshotVO snapshotVO = snapshotDao.findById(snapshotId);
+            if (snapshotVO == null) {
+                logger.debug("Not migrating snapshot [{}] because we could not find its database entry.", snapshot);
+                continue;
+            }
+            if (snapshotVO.getHypervisorType() == Hypervisor.HypervisorType.Simulator) {
+                logger.debug("Not migrating snapshot [{}] because its hypervisor type is simulator.", snapshot);
+                continue;
             }
+            if (snapshot.getParentSnapshotId() != 0) {
+                continue; // The child snapshot will be migrated in the for loop below.
+            }
+            SnapshotInfo snap = snapshotFactory.getSnapshot(snapshotVO.getSnapshotId(), snapshot.getDataStoreId(), snapshot.getRole());
+            if (snap == null) {
+                logger.debug("Not migrating snapshot [{}] because we could not get its information.", snapshot);
+                continue;
+            }
+            files.add(snap);
+            idsForMigration.add(snapshotId);
         }
 
         for (SnapshotInfo parent : files) {
@@ -285,7 +319,7 @@ protected List<DataObject> getAllReadySnapshotsAndChains(DataStore srcDataStore,
                     chain.addAll(children);
                 }
             }
-            snapshotChains.put(parent, new Pair<List<SnapshotInfo>, Long>(chain, getTotalChainSize(chain)));
+            snapshotChains.put(parent, new Pair<>(chain, getTotalChainSize(chain)));
         }
 
         return (List<DataObject>) (List<?>) files;
@@ -306,14 +340,31 @@ protected Long getTotalChainSize(List<? extends DataObject> chain) {
 
     protected List<DataObject> getAllReadyVolumes(DataStore srcDataStore, List<VolumeDataStoreVO> volumes) {
         List<DataObject> files = new LinkedList<>();
+        Set<Long> idsForMigration = new HashSet<>();
+
         for (VolumeDataStoreVO volume : volumes) {
-            if (volume.getState() == ObjectInDataStoreStateMachine.State.Ready) {
-                VolumeInfo volumeInfo = volumeFactory.getVolume(volume.getVolumeId(), srcDataStore);
-                if (volumeInfo != null && volumeInfo.getHypervisorType() != Hypervisor.HypervisorType.Simulator) {
-                    files.add(volumeInfo);
-                }
+            long volumeId = volume.getVolumeId();
+            if (idsForMigration.contains(volumeId)) {
+                logger.warn("Volume store reference [{}] is duplicated; not considering it for migration.", volume);
+                continue;
             }
+            if (volume.getState() != ObjectInDataStoreStateMachine.State.Ready) {
+                logger.debug("Not migrating volume [{}] because its state is not ready.", volume);
+                continue;
+            }
+            VolumeInfo volumeInfo = volumeFactory.getVolume(volume.getVolumeId(), srcDataStore);
+            if (volumeInfo == null) {
+                logger.debug("Not migrating volume [{}] because we could not get its information.", volume);
+                continue;
+            }
+            if (volumeInfo.getHypervisorType() == Hypervisor.HypervisorType.Simulator) {
+                logger.debug("Not migrating volume [{}] because its hypervisor type is simulator.", volume);
+                continue;
+            }
+            files.add(volumeInfo);
+            idsForMigration.add(volumeId);
         }
+
         return files;
     }
 
@@ -325,10 +376,9 @@ protected List<DataObject> getAllReadyVolumes(DataStore srcDataStore) {
     /** Returns the count of active SSVMs - SSVM with agents in connected state, so as to dynamically increase the thread pool
      * size when SSVMs scale
      */
-    protected int activeSSVMCount(DataStore dataStore) {
-        long datacenterId = dataStore.getScope().getScopeId();
+    protected int activeSSVMCount(Long zoneId) {
         List<SecondaryStorageVmVO> ssvms =
-                secStorageVmDao.getSecStorageVmListInStates(null, datacenterId, VirtualMachine.State.Running, VirtualMachine.State.Migrating);
+                secStorageVmDao.getSecStorageVmListInStates(null, zoneId, VirtualMachine.State.Running, VirtualMachine.State.Migrating);
         int activeSSVMs = 0;
         for (SecondaryStorageVmVO vm : ssvms) {
             String name = "s-"+vm.getId()+"-VM";