Clean up primary-only snapshot DB records on volume expunge

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

@@ -2145,6 +2145,7 @@ public void cleanupStorage(boolean recurring) {
                         }
 
                         try {
+                            cleanupSnapshotRecordsInPrimaryStorageOnly(vol);
                             VolumeInfo volumeInfo = volFactory.getVolume(vol.getId());
                             if (volumeInfo != null) {
                                 volService.ensureVolumeIsExpungeReady(vol.getId());
@@ -2292,6 +2293,34 @@ private void deleteSnapshot(SnapshotVO snapshot, SnapshotDataStoreVO snapshotDat
         }
     }
 
+    /**
+     * Cleans up snapshot DB records for snapshots that exist only on primary storage (no secondary copy).
+     * This handles the case where snapshot.backup.to.secondary=false and the volume
+     * is being deleted during VM expunge — the RBD snapshots will be destroyed along with the image,
+     * so the DB records need to be cleaned up to avoid orphaned entries.
+     */
+    protected void cleanupSnapshotRecordsInPrimaryStorageOnly(VolumeVO volume) {
+        List<SnapshotVO> snapshots = _snapshotDao.listByVolumeId(volume.getId());
+        if (CollectionUtils.isEmpty(snapshots)) {
+            return;
+        }
+        for (SnapshotVO snapshot : snapshots) {
+            if (Snapshot.State.Destroyed.equals(snapshot.getState())) {
+                continue;
+            }
+            List<SnapshotDataStoreVO> snapshotsOnPrimaryStorage = _snapshotStoreDao.listBySnapshotAndDataStoreRole(snapshot.getId(), DataStoreRole.Primary);
+            List<SnapshotDataStoreVO> snapshotsOnSecondaryStorage = _snapshotStoreDao.listBySnapshotAndDataStoreRole(snapshot.getId(), DataStoreRole.Image);
+            if (CollectionUtils.isNotEmpty(snapshotsOnPrimaryStorage) && CollectionUtils.isEmpty(snapshotsOnSecondaryStorage)) {
+                logger.info("Cleaning up snapshot {} (primary-only, no secondary copy) as volume {} is being deleted", snapshot, volume);
+                for (SnapshotDataStoreVO snapshotOnPrimaryStorage : snapshotsOnPrimaryStorage) {
+                    _snapshotStoreDao.expunge(snapshotOnPrimaryStorage.getId());
+                }
+                snapshot.setState(Snapshot.State.Destroyed);
+                _snapshotDao.update(snapshot.getId(), snapshot);
+            }
+        }
+    }
+
     private void removeSnapshotsInErrorStatus() {
         List<SnapshotVO> snapshotsInErrorStatus = _snapshotDao.listAllByStatusIncludingRemoved(Snapshot.State.Error);
         for (SnapshotVO snapshotVO : snapshotsInErrorStatus) {

server/src/test/java/com/cloud/storage/StorageManagerImplTest.java

@@ -18,6 +18,7 @@
 
 import java.lang.reflect.Field;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
@@ -47,6 +48,8 @@
 import org.apache.cloudstack.storage.datastore.db.ObjectStoreDao;
 import org.apache.cloudstack.storage.datastore.db.ObjectStoreVO;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
@@ -85,6 +88,7 @@
 import com.cloud.host.Host;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.hypervisor.HypervisorGuruManager;
+import com.cloud.storage.dao.SnapshotDao;
 import com.cloud.storage.dao.VolumeDao;
 import com.cloud.user.AccountManagerImpl;
 import com.cloud.utils.Pair;
@@ -129,6 +133,10 @@ public class StorageManagerImplTest {
     AccountManagerImpl accountMgr;
     @Mock
     StoragePoolDetailsDao storagePoolDetailsDao;
+    @Mock
+    SnapshotDao snapshotDao;
+    @Mock
+    SnapshotDataStoreDao snapshotStoreDao;
 
     @Mock
     ClusterDao clusterDao;
@@ -1716,4 +1724,62 @@ public void testDiscoverObjectStoreInitializationFailure() {
 
         storageManagerImpl.discoverObjectStore(name, url, size, providerName, details);
     }
+
+    @Test
+    public void testCleanupSnapshotRecordsInPrimaryStorageOnly() {
+        VolumeVO volume = Mockito.mock(VolumeVO.class);
+        Mockito.when(volume.getId()).thenReturn(1L);
+
+        SnapshotVO snapshot = Mockito.mock(SnapshotVO.class);
+        Mockito.when(snapshot.getId()).thenReturn(10L);
+        Mockito.when(snapshot.getState()).thenReturn(Snapshot.State.BackedUp);
+        Mockito.when(snapshotDao.listByVolumeId(1L)).thenReturn(List.of(snapshot));
+
+        SnapshotDataStoreVO snapshotOnPrimaryStorage = Mockito.mock(SnapshotDataStoreVO.class);
+        Mockito.when(snapshotOnPrimaryStorage.getId()).thenReturn(100L);
+        Mockito.when(snapshotStoreDao.listBySnapshotAndDataStoreRole(10L, DataStoreRole.Primary)).thenReturn(List.of(snapshotOnPrimaryStorage));
+        Mockito.when(snapshotStoreDao.listBySnapshotAndDataStoreRole(10L, DataStoreRole.Image)).thenReturn(Collections.emptyList());
+
+        storageManagerImpl.cleanupSnapshotRecordsInPrimaryStorageOnly(volume);
+
+        Mockito.verify(snapshotStoreDao).expunge(100L);
+        Mockito.verify(snapshot).setState(Snapshot.State.Destroyed);
+        Mockito.verify(snapshotDao).update(10L, snapshot);
+    }
+
+    @Test
+    public void testCleanupSnapshotRecordsInPrimaryStorageOnlySkipsWhenSecondaryExists() {
+        VolumeVO volume = Mockito.mock(VolumeVO.class);
+        Mockito.when(volume.getId()).thenReturn(1L);
+
+        SnapshotVO snapshot = Mockito.mock(SnapshotVO.class);
+        Mockito.when(snapshot.getId()).thenReturn(10L);
+        Mockito.when(snapshot.getState()).thenReturn(Snapshot.State.BackedUp);
+        Mockito.when(snapshotDao.listByVolumeId(1L)).thenReturn(List.of(snapshot));
+
+        SnapshotDataStoreVO snapshotOnPrimaryStorage = Mockito.mock(SnapshotDataStoreVO.class);
+        SnapshotDataStoreVO snapshotOnSecondaryStorage = Mockito.mock(SnapshotDataStoreVO.class);
+        Mockito.when(snapshotStoreDao.listBySnapshotAndDataStoreRole(10L, DataStoreRole.Primary)).thenReturn(List.of(snapshotOnPrimaryStorage));
+        Mockito.when(snapshotStoreDao.listBySnapshotAndDataStoreRole(10L, DataStoreRole.Image)).thenReturn(List.of(snapshotOnSecondaryStorage));
+
+        storageManagerImpl.cleanupSnapshotRecordsInPrimaryStorageOnly(volume);
+
+        Mockito.verify(snapshotStoreDao, Mockito.never()).expunge(Mockito.anyLong());
+        Mockito.verify(snapshotDao, Mockito.never()).update(Mockito.anyLong(), Mockito.any(SnapshotVO.class));
+    }
+
+    @Test
+    public void testCleanupSnapshotRecordsInPrimaryStorageOnlySkipsDestroyedSnapshots() {
+        VolumeVO volume = Mockito.mock(VolumeVO.class);
+        Mockito.when(volume.getId()).thenReturn(1L);
+
+        SnapshotVO snapshot = Mockito.mock(SnapshotVO.class);
+        Mockito.when(snapshot.getState()).thenReturn(Snapshot.State.Destroyed);
+        Mockito.when(snapshotDao.listByVolumeId(1L)).thenReturn(List.of(snapshot));
+
+        storageManagerImpl.cleanupSnapshotRecordsInPrimaryStorageOnly(volume);
+
+        Mockito.verify(snapshotStoreDao, Mockito.never()).listBySnapshotAndDataStoreRole(Mockito.anyLong(), Mockito.any());
+        Mockito.verify(snapshotStoreDao, Mockito.never()).expunge(Mockito.anyLong());
+    }
 }