Merge pull request #1867 from anshul1886/CLOUDSTACK-9706

CLOUDSTACK-9706: Added snapshots cleanup in start and storage GC thre…

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

@@ -63,4 +63,5 @@ public interface SnapshotDataStoreDao extends GenericDao<SnapshotDataStoreVO, Lo
 
     SnapshotDataStoreVO findByVolume(long volumeId, DataStoreRole role);
 
+    List<SnapshotDataStoreVO> listByState(ObjectInDataStoreStateMachine.State... states);
 }

engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java

@@ -191,7 +191,9 @@ public class SnapshotObject implements SnapshotInfo {
             s_logger.debug("Failed to update state:" + e.toString());
             throw new CloudRuntimeException("Failed to update state: " + e.toString());
         } finally {
-            if (event == ObjectInDataStoreStateMachine.Event.OperationFailed) {
+            DataObjectInStore obj = objectInStoreMgr.findObject(this, this.getDataStore());
+            if (event == ObjectInDataStoreStateMachine.Event.OperationFailed && !obj.getState().equals(ObjectInDataStoreStateMachine.State.Destroying)) {
+                // Don't delete db entry if snapshot is successfully removed.
                 objectInStoreMgr.deleteIfNotReady(this);
             }
         }

engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotStateMachineManagerImpl.java

@@ -52,6 +52,7 @@ public class SnapshotStateMachineManagerImpl implements SnapshotStateMachineMana
         stateMachine.addTransition(Snapshot.State.Copying, Event.OperationFailed, Snapshot.State.BackedUp);
         stateMachine.addTransition(Snapshot.State.Destroying, Event.OperationSucceeded, Snapshot.State.Destroyed);
         stateMachine.addTransition(Snapshot.State.Destroying, Event.OperationFailed, State.BackedUp);
+        stateMachine.addTransition(Snapshot.State.Destroying, Event.DestroyRequested, Snapshot.State.Destroying);
 
         stateMachine.registerListener(new SnapshotStateListener());
     }

engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java

@@ -194,6 +194,7 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
                     }
                 }
                 if (!deleted) {
+                    try {
                         boolean r = snapshotSvr.deleteSnapshot(snapshot);
                         if (r) {
                             // delete snapshot in cache if there is
@@ -207,6 +208,10 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
                             result = r;
                             resultIsSet = true;
                         }
+                    } catch (Exception e) {
+                        // Snapshots which are not successfully deleted will be retried again.
+                        s_logger.debug("Failed to delete snapshot on storage. ", e);
+                    }
                 }
                 snapshot = parent;
             }
@@ -245,7 +250,8 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
             return true;
         }
 
-        if (!Snapshot.State.BackedUp.equals(snapshotVO.getState()) && !Snapshot.State.Error.equals(snapshotVO.getState())) {
+        if (!Snapshot.State.BackedUp.equals(snapshotVO.getState()) && !Snapshot.State.Error.equals(snapshotVO.getState()) &&
+                !Snapshot.State.Destroying.equals(snapshotVO.getState())) {
             throw new InvalidParameterValueException("Can't delete snapshotshot " + snapshotId + " due to it is in " + snapshotVO.getState() + " Status");
         }
 

engine/storage/src/org/apache/cloudstack/storage/image/db/SnapshotDataStoreDaoImpl.java

@@ -53,6 +53,7 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
     private SearchBuilder<SnapshotDataStoreVO> snapshotIdSearch;
     private SearchBuilder<SnapshotDataStoreVO> volumeIdSearch;
     private SearchBuilder<SnapshotDataStoreVO> volumeSearch;
+    private SearchBuilder<SnapshotDataStoreVO> stateSearch;
 
     private final String parentSearch = "select store_id, store_role, snapshot_id from cloud.snapshot_store_ref where store_id = ? "
         + " and store_role = ? and volume_id = ? and state = 'Ready'" + " order by created DESC " + " limit 1";
@@ -123,6 +124,10 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
         volumeSearch.and("store_role", volumeSearch.entity().getRole(), SearchCriteria.Op.EQ);
         volumeSearch.done();
 
+        stateSearch = createSearchBuilder();
+        stateSearch.and("state", stateSearch.entity().getState(), SearchCriteria.Op.IN);
+        stateSearch.done();
+
         return true;
     }
 
@@ -407,4 +412,11 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
         UpdateBuilder ub = getUpdateBuilder(snapshot);
         update(ub, sc, null);
     }
+
+    @Override
+    public List<SnapshotDataStoreVO> listByState(ObjectInDataStoreStateMachine.State... states) {
+        SearchCriteria<SnapshotDataStoreVO> sc = stateSearch.create();
+        sc.setParameters("state", (Object[])states);
+        return listBy(sc, null);
+    }
 }

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

@@ -41,6 +41,8 @@ import javax.naming.ConfigurationException;
 
 import com.cloud.hypervisor.Hypervisor;
 
+import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 import org.apache.cloudstack.api.command.admin.storage.CancelPrimaryStorageMaintenanceCmd;
@@ -291,6 +293,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
     @Inject
     EntityManager _entityMgr;
     @Inject
+    SnapshotService _snapshotService;
+    @Inject
     StoragePoolTagsDao _storagePoolTagsDao;
 
     protected List<StoragePoolDiscoverer> _discoverers;
@@ -1078,6 +1082,16 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C
                         }
                     }
 
+                    //destroy snapshots in destroying state in snapshot_store_ref
+                    List<SnapshotDataStoreVO>  ssSnapshots = _snapshotStoreDao.listByState(ObjectInDataStoreStateMachine.State.Destroying);
+                    for(SnapshotDataStoreVO ssSnapshotVO : ssSnapshots){
+                        try {
+                            _snapshotService.deleteSnapshot(snapshotFactory.getSnapshot(ssSnapshotVO.getSnapshotId(), DataStoreRole.Image));
+                        } catch (Exception e) {
+                            s_logger.debug("Failed to delete snapshot: " + ssSnapshotVO.getId() + " from storage");
+                        }
+                    }
+
                     cleanupSecondaryStorage(recurring);
 
                     List<VolumeVO> vols = _volsDao.listVolumesToBeDestroyed(new Date(System.currentTimeMillis() - ((long) StorageCleanupDelay.value() << 10)));

server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java

@@ -1192,6 +1192,17 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
 
     @Override
     public boolean start() {
+        //destroy snapshots in destroying state
+        List<SnapshotVO> snapshots = _snapshotDao.listAllByStatus(Snapshot.State.Destroying);
+        for (SnapshotVO snapshotVO : snapshots) {
+            try {
+                if (!deleteSnapshot(snapshotVO.getId())) {
+                    s_logger.debug("Failed to delete snapshot in destroying state with id " + snapshotVO.getUuid());
+                }
+            } catch (Exception e) {
+                s_logger.debug("Failed to delete snapshot in destroying state with id " + snapshotVO.getUuid());
+            }
+        }
         return true;
     }