Cleanup volume information from db when deleted (#4551)

* Cleanup volume information from db when deleted

* reuse search builder

* revert change

Co-authored-by: Pearl Dsilva <pearl.dsilva@shapeblue.com>

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

@@ -55,4 +55,6 @@ public interface VolumeDataStoreDao extends GenericDao<VolumeDataStoreVO, Long>,
     boolean updateVolumeId(long srcVolId, long destVolId);
 
     List<VolumeDataStoreVO> listVolumeDownloadUrlsByZoneId(long zoneId);
+
+    List<VolumeDataStoreVO> listByVolume(long volumeId, long storeId);
 }

engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java

@@ -19,6 +19,7 @@
 package org.apache.cloudstack.storage.motion;
 
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import javax.inject.Inject;
@@ -44,6 +45,8 @@ import org.apache.cloudstack.framework.async.AsyncCompletionCallback;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.storage.RemoteHostEndPoint;
 import org.apache.cloudstack.storage.command.CopyCommand;
+import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
 import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
 import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
 import org.apache.log4j.Logger;
@@ -85,6 +88,8 @@ public class AncientDataMotionStrategy implements DataMotionStrategy {
     DataStoreManager dataStoreMgr;
     @Inject
     StorageCacheManager cacheMgr;
+    @Inject
+    VolumeDataStoreDao volumeDataStoreDao;
 
     @Inject
     StorageManager storageManager;
@@ -324,6 +329,15 @@ public class AncientDataMotionStrategy implements DataMotionStrategy {
         }
     }
 
+    private void deleteVolumeOnSecondaryStore(DataObject objectInStore) {
+        ImageStoreEntity store = (ImageStoreEntity) objectInStore.getDataStore();
+        store.delete(objectInStore);
+        List<VolumeDataStoreVO> volumesOnStore =  volumeDataStoreDao.listByVolume(objectInStore.getId(), store.getId());
+        for (VolumeDataStoreVO volume : volumesOnStore) {
+            volumeDataStoreDao.remove(volume.getId());
+        }
+    }
+
     protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData) {
         String value = configDao.getValue(Config.CopyVolumeWait.key());
         int _copyvolumewait = NumbersUtil.parseInt(value, Integer.parseInt(Config.CopyVolumeWait.getDefaultValue()));
@@ -359,10 +373,9 @@ public class AncientDataMotionStrategy implements DataMotionStrategy {
             }
 
             DataObject objOnImageStore = imageStore.create(srcData);
-            objOnImageStore.processEvent(Event.CreateOnlyRequested);
-
             Answer answer = null;
             try {
+                objOnImageStore.processEvent(Event.CreateOnlyRequested);
                 answer = copyObject(srcData, objOnImageStore);
 
                 if (answer == null || !answer.getResult()) {
@@ -401,11 +414,12 @@ public class AncientDataMotionStrategy implements DataMotionStrategy {
                     objOnImageStore.processEvent(Event.OperationFailed);
                     imageStore.delete(objOnImageStore);
                 }
+                s_logger.error("Failed to perform operation: "+ e.getLocalizedMessage());
                 throw e;
             }
 
             objOnImageStore.processEvent(Event.OperationSuccessed);
-            imageStore.delete(objOnImageStore);
+            deleteVolumeOnSecondaryStore(objOnImageStore);
             return answer;
         } else {
             DataObject cacheData = cacheMgr.createCacheObject(srcData, destScope);

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

@@ -62,7 +62,6 @@ public class VolumeDataStoreDaoImpl extends GenericDaoBase<VolumeDataStoreVO, Lo
     private SearchBuilder<VolumeDataStoreVO> uploadVolumeStateSearch;
     private static final String EXPIRE_DOWNLOAD_URLS_FOR_ZONE = "update volume_store_ref set download_url_created=? where download_url_created is not null and store_id in (select id from image_store where data_center_id=?)";
 
-
     @Inject
     DataStoreManager storeMgr;
     @Inject
@@ -119,7 +118,6 @@ public class VolumeDataStoreDaoImpl extends GenericDaoBase<VolumeDataStoreVO, Lo
         uploadVolumeStateSearch.join("volumeOnlySearch", volumeOnlySearch, volumeOnlySearch.entity().getId(), uploadVolumeStateSearch.entity().getVolumeId(), JoinType.LEFT);
         uploadVolumeStateSearch.and("destroyed", uploadVolumeStateSearch.entity().getDestroyed(), SearchCriteria.Op.EQ);
         uploadVolumeStateSearch.done();
-
         return true;
     }
 
@@ -332,6 +330,15 @@ public class VolumeDataStoreDaoImpl extends GenericDaoBase<VolumeDataStoreVO, Lo
         return listBy(sc);
     }
 
+    @Override
+    public List<VolumeDataStoreVO> listByVolume(long volumeId, long storeId) {
+        SearchCriteria<VolumeDataStoreVO> sc = storeVolumeSearch.create();
+        sc.setParameters("store_id", storeId);
+        sc.setParameters("volume_id", volumeId);
+        sc.setParameters("destroyed", false);
+        return listBy(sc);
+    }
+
     @Override
     public List<VolumeDataStoreVO> listUploadedVolumesByStoreId(long id) {
         SearchCriteria<VolumeDataStoreVO> sc = uploadVolumeSearch.create();
@@ -340,7 +347,6 @@ public class VolumeDataStoreDaoImpl extends GenericDaoBase<VolumeDataStoreVO, Lo
         return listIncludingRemovedBy(sc);
     }
 
-
     @Override
     public void expireDnldUrlsForZone(Long dcId){
         TransactionLegacy txn = TransactionLegacy.currentTxn();

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

@@ -1585,6 +1585,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
             for (DataStore store : imageStores) {
                 try {
                     List<VolumeDataStoreVO> destroyedStoreVOs = _volumeStoreDao.listDestroyed(store.getId());
+                    destroyedStoreVOs.addAll(_volumeDataStoreDao.listByVolumeState(Volume.State.Expunged));
                     s_logger.debug("Secondary storage garbage collector found " + destroyedStoreVOs.size() + " volumes to cleanup on volume_store_ref for store: " + store.getName());
                     for (VolumeDataStoreVO destroyedStoreVO : destroyedStoreVOs) {
                         if (s_logger.isDebugEnabled()) {