Cleanup download urls when SSVM destroyed (#4078)

Co-authored-by: Rakesh Venkatesh <r.venkatesh@global.leaseweb.com>

engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDao.java

@@ -40,4 +40,6 @@ public interface ImageStoreDao extends GenericDao<ImageStoreVO, Long> {
     List<ImageStoreVO> listImageStores();
 
     List<ImageStoreVO> listImageCacheStores();
+
+    List<ImageStoreVO> listStoresByZoneId(long zoneId);
 }

engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java

@@ -134,4 +134,10 @@ public class ImageStoreDaoImpl extends GenericDaoBase<ImageStoreVO, Long> implem
         return listBy(sc);
     }
 
+    @Override
+    public List<ImageStoreVO> listStoresByZoneId(long zoneId) {
+        SearchCriteria<ImageStoreVO> sc = createSearchCriteria();
+        sc.addAnd("dcId", SearchCriteria.Op.EQ, zoneId);
+        return listBy(sc);
+    }
 }

engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreDao.java

@@ -91,4 +91,6 @@ public interface TemplateDataStoreDao extends GenericDao<TemplateDataStoreVO, Lo
     TemplateDataStoreVO getReadyBypassedTemplate(long templateId);
 
     boolean isTemplateMarkedForDirectDownload(long templateId);
+
+    List<TemplateDataStoreVO> listTemplateDownloadUrlsByStoreId(long storeId);
 }

engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreDao.java

@@ -53,4 +53,6 @@ public interface VolumeDataStoreDao extends GenericDao<VolumeDataStoreVO, Long>,
     List<VolumeDataStoreVO> listByVolumeState(Volume.State... states);
 
     boolean updateVolumeId(long srcVolId, long destVolId);
+
+    List<VolumeDataStoreVO> listVolumeDownloadUrlsByZoneId(long zoneId);
 }

engine/storage/src/main/java/org/apache/cloudstack/storage/image/db/TemplateDataStoreDaoImpl.java

@@ -144,6 +144,7 @@ public class TemplateDataStoreDaoImpl extends GenericDaoBase<TemplateDataStoreVO
         downloadTemplateSearch.and("download_url", downloadTemplateSearch.entity().getExtractUrl(), Op.NNULL);
         downloadTemplateSearch.and("download_url_created", downloadTemplateSearch.entity().getExtractUrlCreated(), Op.NNULL);
         downloadTemplateSearch.and("destroyed", downloadTemplateSearch.entity().getDestroyed(), SearchCriteria.Op.EQ);
+        downloadTemplateSearch.and("store_id", downloadTemplateSearch.entity().getDataStoreId(), Op.EQ);
         downloadTemplateSearch.done();
 
         directDownloadTemplateSeach = createSearchBuilder();
@@ -543,6 +544,14 @@ public class TemplateDataStoreDaoImpl extends GenericDaoBase<TemplateDataStoreVO
         return listBy(sc);
     }
 
+    @Override
+    public List<TemplateDataStoreVO> listTemplateDownloadUrlsByStoreId(long storeId) {
+        SearchCriteria<TemplateDataStoreVO> sc = downloadTemplateSearch.create();
+        sc.setParameters("destroyed", false);
+        sc.setParameters("store_id", storeId);
+        return listBy(sc);
+    }
+
     @Override
     public void expireDnldUrlsForZone(Long dcId){
         TransactionLegacy txn = TransactionLegacy.currentTxn();

engine/storage/src/main/java/org/apache/cloudstack/storage/image/db/VolumeDataStoreDaoImpl.java

@@ -25,6 +25,7 @@ import java.util.Map;
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.utils.db.Filter;
 import com.cloud.utils.exception.CloudRuntimeException;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
@@ -103,6 +104,7 @@ public class VolumeDataStoreDaoImpl extends GenericDaoBase<VolumeDataStoreVO, Lo
         downloadVolumeSearch.and("download_url", downloadVolumeSearch.entity().getExtractUrl(), Op.NNULL);
         downloadVolumeSearch.and("download_url_created", downloadVolumeSearch.entity().getExtractUrlCreated(), Op.NNULL);
         downloadVolumeSearch.and("destroyed", downloadVolumeSearch.entity().getDestroyed(), SearchCriteria.Op.EQ);
+        downloadVolumeSearch.and("zone_id", downloadVolumeSearch.entity().getZoneId(), Op.EQ);
         downloadVolumeSearch.done();
 
         uploadVolumeSearch = createSearchBuilder();
@@ -213,7 +215,13 @@ public class VolumeDataStoreDaoImpl extends GenericDaoBase<VolumeDataStoreVO, Lo
         SearchCriteria<VolumeDataStoreVO> sc = volumeSearch.create();
         sc.setParameters("volume_id", volumeId);
         sc.setParameters("destroyed", false);
-        return findOneBy(sc);
+        return findVolumeby(sc);
+    }
+
+    private VolumeDataStoreVO findVolumeby(SearchCriteria<VolumeDataStoreVO> sc) {
+        Filter filter = new Filter(VolumeDataStoreVO.class, "created", false, 0L, 1L);
+        List<VolumeDataStoreVO> results = searchIncludingRemoved(sc, filter, null, false);
+        return results.size() == 0 ? null : results.get(0);
     }
 
     @Override
@@ -316,6 +324,14 @@ public class VolumeDataStoreDaoImpl extends GenericDaoBase<VolumeDataStoreVO, Lo
         return listBy(sc);
     }
 
+    @Override
+    public List<VolumeDataStoreVO> listVolumeDownloadUrlsByZoneId(long zoneId) {
+        SearchCriteria<VolumeDataStoreVO> sc = downloadVolumeSearch.create();
+        sc.setParameters("destroyed", false);
+        sc.setParameters("zone_id", zoneId);
+        return listBy(sc);
+    }
+
     @Override
     public List<VolumeDataStoreVO> listUploadedVolumesByStoreId(long id) {
         SearchCriteria<VolumeDataStoreVO> sc = uploadVolumeSearch.create();

plugins/storage/image/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java

@@ -71,7 +71,8 @@ public class CloudStackImageStoreDriverImpl extends NfsImageStoreDriverImpl {
         // Create Symlink at ssvm
         String path = installPath;
         String uuid = UUID.randomUUID().toString() + "." + format.getFileExtension();
-        CreateEntityDownloadURLCommand cmd = new CreateEntityDownloadURLCommand(((ImageStoreEntity)store).getMountPoint(), path, uuid, dataObject.getTO());
+        CreateEntityDownloadURLCommand cmd = new CreateEntityDownloadURLCommand(((ImageStoreEntity)store).getMountPoint(),
+                                                                path, uuid, dataObject == null ? null: dataObject.getTO());
         Answer ans = null;
         if (ep == null) {
             String errMsg = "No remote endpoint to send command, check if host or ssvm is down?";

server/src/main/java/com/cloud/server/ManagementServerImpl.java

@@ -554,6 +554,10 @@ import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
+import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
 import org.apache.cloudstack.utils.identity.ManagementServerNode;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.collections.CollectionUtils;
@@ -840,6 +844,10 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
     private DpdkHelper dpdkHelper;
     @Inject
     private PrimaryDataStoreDao _primaryDataStoreDao;
+    @Inject
+    private VolumeDataStoreDao _volumeStoreDao;
+    @Inject
+    private TemplateDataStoreDao _vmTemplateStoreDao;
 
     private LockMasterListener _lockMasterListener;
     private final ScheduledExecutorService _eventExecutor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("EventChecker"));
@@ -3288,13 +3296,28 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         }
     }
 
+    private void cleanupDownloadUrlsInZone(final long zoneId) {
+        // clean download URLs when destroying ssvm
+        // clean only the volumes and templates of the zone to which ssvm belongs to
+        for (VolumeDataStoreVO volume :_volumeStoreDao.listVolumeDownloadUrlsByZoneId(zoneId)) {
+            volume.setExtractUrl(null);
+            _volumeStoreDao.update(volume.getId(), volume);
+        }
+        for (ImageStoreVO imageStore : _imgStoreDao.listStoresByZoneId(zoneId)) {
+            for (TemplateDataStoreVO template : _vmTemplateStoreDao.listTemplateDownloadUrlsByStoreId(imageStore.getId())) {
+                template.setExtractUrl(null);
+                template.setExtractUrlCreated(null);
+                _vmTemplateStoreDao.update(template.getId(), template);
+            }
+        }
+    }
+
     private SecondaryStorageVmVO startSecondaryStorageVm(final long instanceId) {
         return _secStorageVmMgr.startSecStorageVm(instanceId);
     }
 
     private SecondaryStorageVmVO stopSecondaryStorageVm(final VMInstanceVO systemVm, final boolean isForced)
             throws ResourceUnavailableException, OperationTimedoutException, ConcurrentOperationException {
-
         _itMgr.advanceStop(systemVm.getUuid(), isForced);
         return _secStorageVmDao.findById(systemVm.getId());
     }
@@ -3306,6 +3329,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
 
     protected SecondaryStorageVmVO destroySecondaryStorageVm(final long instanceId) {
         final SecondaryStorageVmVO secStorageVm = _secStorageVmDao.findById(instanceId);
+        cleanupDownloadUrlsInZone(secStorageVm.getDataCenterId());
         if (_secStorageVmMgr.destroySecStorageVm(instanceId)) {
             return secStorageVm;
         }

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

@@ -25,6 +25,7 @@ import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.UUID;
 import java.util.concurrent.ExecutionException;
 
@@ -48,6 +49,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
 import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
 import org.apache.cloudstack.engine.subsystem.api.storage.HostScope;
+import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
 import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
 import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
 import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
@@ -149,6 +151,8 @@ import com.cloud.utils.UriUtils;
 import com.cloud.utils.component.ManagerBase;
 import com.cloud.utils.db.DB;
 import com.cloud.utils.db.EntityManager;
+import com.cloud.utils.db.Filter;
+import com.cloud.utils.db.SearchCriteria;
 import com.cloud.utils.db.Transaction;
 import com.cloud.utils.db.TransactionCallback;
 import com.cloud.utils.db.TransactionCallbackWithException;
@@ -2791,9 +2795,11 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         }
 
         // Check if the url already exists
-        VolumeDataStoreVO volumeStoreRef = _volumeStoreDao.findByVolume(volumeId);
+        SearchCriteria<VolumeDataStoreVO> sc = _volumeStoreDao.createSearchCriteria();
-        if (volumeStoreRef != null && volumeStoreRef.getExtractUrl() != null) {
+
-            return volumeStoreRef.getExtractUrl();
+        Optional<String> extractUrl = setExtractVolumeSearchCriteria(sc, volume);
+        if (extractUrl.isPresent()) {
+            return extractUrl.get();
         }
 
         VMInstanceVO vm = null;
@@ -2848,6 +2854,45 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         return orchestrateExtractVolume(volume.getId(), zoneId);
     }
 
+    private Optional<String> setExtractVolumeSearchCriteria(SearchCriteria<VolumeDataStoreVO> sc, VolumeVO volume) {
+        final long volumeId = volume.getId();
+        sc.addAnd("state", SearchCriteria.Op.EQ, ObjectInDataStoreStateMachine.State.Ready.toString());
+        sc.addAnd("volumeId", SearchCriteria.Op.EQ, volumeId);
+        sc.addAnd("destroyed", SearchCriteria.Op.EQ, false);
+        // the volume should not change (attached/detached, vm not updated) after created
+        if (volume.getVolumeType() == Volume.Type.ROOT) { // for ROOT disk
+            VMInstanceVO vm = _vmInstanceDao.findById(volume.getInstanceId());
+            sc.addAnd("updated", SearchCriteria.Op.GTEQ, vm.getUpdateTime());
+        } else if (volume.getVolumeType() == Volume.Type.DATADISK && volume.getInstanceId() == null) { // for not attached DATADISK
+            sc.addAnd("updated", SearchCriteria.Op.GTEQ, volume.getUpdated());
+        } else { // for attached DATA DISK
+            VMInstanceVO vm = _vmInstanceDao.findById(volume.getInstanceId());
+            sc.addAnd("updated", SearchCriteria.Op.GTEQ, vm.getUpdateTime());
+            sc.addAnd("updated", SearchCriteria.Op.GTEQ, volume.getUpdated());
+        }
+        Filter filter = new Filter(VolumeDataStoreVO.class, "created", false, 0L, 1L);
+        List<VolumeDataStoreVO> volumeStoreRefs = _volumeStoreDao.search(sc, filter);
+        VolumeDataStoreVO volumeStoreRef = null;
+        if (volumeStoreRefs != null && !volumeStoreRefs.isEmpty()) {
+            volumeStoreRef = volumeStoreRefs.get(0);
+        }
+        if (volumeStoreRef != null && volumeStoreRef.getExtractUrl() != null) {
+            return Optional.ofNullable(volumeStoreRef.getExtractUrl());
+        } else if (volumeStoreRef != null) {
+            s_logger.debug("volume " + volumeId + " is already installed on secondary storage, install path is " +
+                    volumeStoreRef.getInstallPath());
+            ImageStoreEntity secStore = (ImageStoreEntity) dataStoreMgr.getDataStore(volumeStoreRef.getDataStoreId(), DataStoreRole.Image);
+            String extractUrl = secStore.createEntityExtractUrl(volumeStoreRef.getInstallPath(), volume.getFormat(), null);
+            volumeStoreRef = _volumeStoreDao.findByVolume(volumeId);
+            volumeStoreRef.setExtractUrl(extractUrl);
+            volumeStoreRef.setExtractUrlCreated(DateUtil.now());
+            _volumeStoreDao.update(volumeStoreRef.getId(), volumeStoreRef);
+            return Optional.ofNullable(extractUrl);
+        }
+
+        return Optional.empty();
+    }
+
     private String orchestrateExtractVolume(long volumeId, long zoneId) {
         // get latest volume state to make sure that it is not updated by other parallel operations
         VolumeVO volume = _volsDao.findById(volumeId);

server/src/main/java/com/cloud/template/TemplateManagerImpl.java

@@ -545,6 +545,9 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
                 if (tmpltStoreRef != null) {
                     if (tmpltStoreRef.getDownloadState() == com.cloud.storage.VMTemplateStorageResourceAssoc.Status.DOWNLOADED) {
                         tmpltStore = (ImageStoreEntity)store;
+                        if (tmpltStoreRef.getExtractUrl() != null) {
+                            return tmpltStoreRef.getExtractUrl();
+                        }
                         break;
                     }
                 }

services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java

@@ -106,7 +106,6 @@ import com.cloud.service.ServiceOfferingVO;
 import com.cloud.service.dao.ServiceOfferingDao;
 import com.cloud.storage.ImageStoreDetailsUtil;
 import com.cloud.storage.Storage;
-import com.cloud.storage.UploadVO;
 import com.cloud.storage.VMTemplateVO;
 import com.cloud.storage.dao.SnapshotDao;
 import com.cloud.storage.dao.StoragePoolHostDao;
@@ -747,9 +746,6 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar
                 if (_allocLock.lock(ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_SYNC)) {
                     try {
                         secStorageVm = startNew(dataCenterId, role);
-                        for (UploadVO upload : _uploadDao.listAll()) {
-                            _uploadDao.expunge(upload.getId());
-                        }
                     } finally {
                         _allocLock.unlock();
                     }

services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java

@@ -309,12 +309,15 @@ public class UploadManagerImpl extends ManagerBase implements UploadManager {
 
         //We just need to remove the UUID.vhd
         String extractUrl = cmd.getExtractUrl();
-        command.add("unlink /var/www/html/userdata/" + extractUrl.substring(extractUrl.lastIndexOf(File.separator) + 1));
+        String result;
-        String result = command.execute();
+        if (extractUrl != null) {
-        if (result != null) {
+            command.add("unlink /var/www/html/userdata/" + extractUrl.substring(extractUrl.lastIndexOf(File.separator) + 1));
-            // FIXME - Ideally should bail out if you cant delete symlink. Not doing it right now.
+            result = command.execute();
-            // This is because the ssvm might already be destroyed and the symlinks do not exist.
+            if (result != null) {
-            s_logger.warn("Error in deleting symlink :" + result);
+                // FIXME - Ideally should bail out if you cant delete symlink. Not doing it right now.
+                // This is because the ssvm might already be destroyed and the symlinks do not exist.
+                s_logger.warn("Error in deleting symlink :" + result);
+            }
         }
 
         // If its a volume also delete the Hard link since it was created only for the purpose of download.