From 735b6de296e07eb5def98f8e1c3a9b73960c506f Mon Sep 17 00:00:00 2001 From: Rakesh Date: Wed, 18 Nov 2020 14:01:31 +0100 Subject: [PATCH] Cleanup download urls when SSVM destroyed (#4078) Co-authored-by: Rakesh Venkatesh --- .../storage/datastore/db/ImageStoreDao.java | 2 + .../datastore/db/ImageStoreDaoImpl.java | 6 +++ .../datastore/db/TemplateDataStoreDao.java | 2 + .../datastore/db/VolumeDataStoreDao.java | 2 + .../image/db/TemplateDataStoreDaoImpl.java | 9 ++++ .../image/db/VolumeDataStoreDaoImpl.java | 18 ++++++- .../CloudStackImageStoreDriverImpl.java | 3 +- .../cloud/server/ManagementServerImpl.java | 26 +++++++++- .../cloud/storage/VolumeApiServiceImpl.java | 51 +++++++++++++++++-- .../cloud/template/TemplateManagerImpl.java | 3 ++ .../SecondaryStorageManagerImpl.java | 4 -- .../storage/template/UploadManagerImpl.java | 15 +++--- 12 files changed, 125 insertions(+), 16 deletions(-) diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDao.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDao.java index 84cba70e861..71609a982ca 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDao.java @@ -40,4 +40,6 @@ public interface ImageStoreDao extends GenericDao { List listImageStores(); List listImageCacheStores(); + + List listStoresByZoneId(long zoneId); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java index 96a41af4107..44ae9618077 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java @@ -134,4 +134,10 @@ public class ImageStoreDaoImpl extends GenericDaoBase implem return listBy(sc); } + @Override + public List listStoresByZoneId(long zoneId) { + SearchCriteria sc = createSearchCriteria(); + sc.addAnd("dcId", SearchCriteria.Op.EQ, zoneId); + return listBy(sc); + } } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreDao.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreDao.java index fc695f47677..77e88a9466e 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/TemplateDataStoreDao.java @@ -91,4 +91,6 @@ public interface TemplateDataStoreDao extends GenericDao listTemplateDownloadUrlsByStoreId(long storeId); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreDao.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreDao.java index fb9844116c1..0e4e380c753 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreDao.java @@ -53,4 +53,6 @@ public interface VolumeDataStoreDao extends GenericDao, List listByVolumeState(Volume.State... states); boolean updateVolumeId(long srcVolId, long destVolId); + + List listVolumeDownloadUrlsByZoneId(long zoneId); } diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/db/TemplateDataStoreDaoImpl.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/db/TemplateDataStoreDaoImpl.java index 5a0e4eeceed..ea53825eab0 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/db/TemplateDataStoreDaoImpl.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/db/TemplateDataStoreDaoImpl.java @@ -144,6 +144,7 @@ public class TemplateDataStoreDaoImpl extends GenericDaoBase listTemplateDownloadUrlsByStoreId(long storeId) { + SearchCriteria 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(); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/db/VolumeDataStoreDaoImpl.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/db/VolumeDataStoreDaoImpl.java index 8258042984d..34854d5d598 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/db/VolumeDataStoreDaoImpl.java +++ b/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 sc = volumeSearch.create(); sc.setParameters("volume_id", volumeId); sc.setParameters("destroyed", false); - return findOneBy(sc); + return findVolumeby(sc); + } + + private VolumeDataStoreVO findVolumeby(SearchCriteria sc) { + Filter filter = new Filter(VolumeDataStoreVO.class, "created", false, 0L, 1L); + List results = searchIncludingRemoved(sc, filter, null, false); + return results.size() == 0 ? null : results.get(0); } @Override @@ -316,6 +324,14 @@ public class VolumeDataStoreDaoImpl extends GenericDaoBase listVolumeDownloadUrlsByZoneId(long zoneId) { + SearchCriteria sc = downloadVolumeSearch.create(); + sc.setParameters("destroyed", false); + sc.setParameters("zone_id", zoneId); + return listBy(sc); + } + @Override public List listUploadedVolumesByStoreId(long id) { SearchCriteria sc = uploadVolumeSearch.create(); diff --git a/plugins/storage/image/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java b/plugins/storage/image/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java index b8b52262617..8abf802d9de 100644 --- a/plugins/storage/image/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java +++ b/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?"; diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 042ade5c808..8df118dcbc7 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/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; } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index be2d52e3c44..8430670ac0c 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/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); - if (volumeStoreRef != null && volumeStoreRef.getExtractUrl() != null) { - return volumeStoreRef.getExtractUrl(); + SearchCriteria sc = _volumeStoreDao.createSearchCriteria(); + + Optional 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 setExtractVolumeSearchCriteria(SearchCriteria 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 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); diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 07c7013a3f2..4261269d9f9 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/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; } } 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 5ee60eb7e03..2fc1eeda53c 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 @@ -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(); } diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java index 95692ddaeb0..cdda8161ba6 100644 --- a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java +++ b/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 = command.execute(); - if (result != null) { - // 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); + String result; + if (extractUrl != null) { + command.add("unlink /var/www/html/userdata/" + extractUrl.substring(extractUrl.lastIndexOf(File.separator) + 1)); + result = command.execute(); + if (result != null) { + // 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.