This is an automated email from the ASF dual-hosted git repository. DaanHoogland pushed a commit to branch 4.22 in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 30ed411cffa57ded585c3ed81093fded409fccc5 Merge: d31b5ff94e9 e742aadfc97 Author: Daan Hoogland <[email protected]> AuthorDate: Fri Oct 23 12:45:03 2026 +0200 Merge branch '4.20' into 4.22 .../java/com/cloud/storage/dao/SnapshotDao.java | 2 + .../com/cloud/storage/dao/SnapshotDaoImpl.java | 16 +++++ .../com/cloud/storage/dao/SnapshotDaoImplTest.java | 80 ++++++++++++++++++++++ .../cloud/network/router/NetworkHelperImpl.java | 22 +----- .../storage/snapshot/SnapshotManagerImpl.java | 5 ++ .../network/router/NetworkHelperImplTest.java | 46 +++++++++++++ .../storage/snapshot/SnapshotManagerImplTest.java | 79 ++++++++++++++++++++- 7 files changed, 229 insertions(+), 21 deletions(-) diff --cc engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java index f167b573187,0f6576e996a..adcfdef8852 --- a/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java @@@ -70,9 -65,8 +70,10 @@@ public class SnapshotDaoImpl extends Ge private SearchBuilder<SnapshotVO> InstanceIdSearch; private SearchBuilder<SnapshotVO> StatusSearch; private SearchBuilder<SnapshotVO> notInStatusSearch; + private SearchBuilder<SnapshotVO> volumeIdNameNotInStatusSearch; private GenericSearchBuilder<SnapshotVO, Long> CountSnapshotsByAccount; + + private SearchBuilder<SnapshotVO> volumeIdAndTypeNotInSearch; @Inject ResourceTagDao _tagsDao; @Inject diff --cc server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java index e283fd88ed8,d99c67e2cf6..d406753a95a --- a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java @@@ -20,19 -49,17 +20,21 @@@ import com.cloud.dc.DataCenter import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.DataCenterDao; import com.cloud.event.ActionEventUtils; +import com.cloud.hypervisor.Hypervisor; import com.cloud.exception.InvalidParameterValueException; -import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.ResourceUnavailableException; ++import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.org.Grouping; import com.cloud.storage.DataStoreRole; + import com.cloud.storage.ScopeType; import com.cloud.storage.Snapshot; +import com.cloud.storage.SnapshotPolicyVO; import com.cloud.storage.SnapshotVO; import com.cloud.storage.VolumeVO; +import com.cloud.server.TaggedResourceService; import com.cloud.storage.dao.SnapshotDao; +import com.cloud.storage.dao.SnapshotPolicyDao; import com.cloud.storage.dao.SnapshotZoneDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.user.Account; @@@ -42,40 -69,8 +44,45 @@@ import com.cloud.user.ResourceLimitServ import com.cloud.user.User; import com.cloud.user.dao.AccountDao; import com.cloud.utils.Pair; - +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; + import com.cloud.vm.dao.UserVmDao; + +import org.apache.cloudstack.api.command.user.snapshot.DeleteSnapshotPoliciesCmd; +import org.apache.cloudstack.api.command.user.snapshot.ListSnapshotPoliciesCmd; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; +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.SnapshotDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotResult; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService; ++import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; ++import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.framework.async.AsyncCallFuture; ++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.StoragePoolVO; + +import org.junit.Assert; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ExecutionException; + @RunWith(MockitoJUnitRunner.class) public class SnapshotManagerImplTest { @Mock @@@ -101,28 -96,77 +108,98 @@@ @Mock VolumeDao volumeDao; @Mock + SnapshotPolicyDao snapshotPolicyDao; + @Mock + SnapshotScheduler snapshotScheduler; + @Mock + TaggedResourceService taggedResourceService; ++ @Mock + PrimaryDataStoreDao primaryDataStoreDao; + @Mock + VolumeDataFactory volFactory; + @Mock + UserVmDao userVmDao; ++ @InjectMocks SnapshotManagerImpl snapshotManager = new SnapshotManagerImpl(); + @Before + public void setUp() { + snapshotManager._snapshotPolicyDao = snapshotPolicyDao; + snapshotManager._volsDao = volumeDao; + snapshotManager._accountMgr = accountManager; + snapshotManager._snapSchedMgr = snapshotScheduler; + snapshotManager.taggedResourceService = taggedResourceService; + } + + @After + public void tearDown() { + CallContext.unregister(); + } + + @Test + public void testAllocSnapshotRejectsDuplicateNameForVolume() { + long volumeId = 1L; + CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); + try { + VolumeInfo volume = Mockito.mock(VolumeInfo.class); + Mockito.when(volFactory.getVolume(volumeId)).thenReturn(volume); + DataStore dataStore = Mockito.mock(DataStore.class); + Mockito.when(volume.getDataStore()).thenReturn(dataStore); + Mockito.when(dataStore.getId()).thenReturn(10L); + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + Mockito.when(primaryDataStoreDao.findById(10L)).thenReturn(pool); + Mockito.when(pool.getScope()).thenReturn(ScopeType.ZONE); + Mockito.when(pool.getHypervisor()).thenReturn(HypervisorType.None); + Mockito.when(volume.getInstanceId()).thenReturn(null); + Mockito.when(volume.getAccountId()).thenReturn(2L); + + Mockito.when(snapshotDao.findByVolumeIdAndNameNotInStatus(volumeId, "dup", Snapshot.State.Destroyed, Snapshot.State.Error)) + .thenReturn(Mockito.mock(SnapshotVO.class)); + + Assert.assertThrows(InvalidParameterValueException.class, () -> + snapshotManager.allocSnapshot(volumeId, Snapshot.MANUAL_POLICY_ID, "dup", null)); + } finally { + CallContext.unregister(); + } + } + + @Test + public void testAllocSnapshotDoesNotCheckDuplicateForAutoGeneratedName() { + long volumeId = 1L; + CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); + try { + VolumeInfo volume = Mockito.mock(VolumeInfo.class); + Mockito.when(volFactory.getVolume(volumeId)).thenReturn(volume); + DataStore dataStore = Mockito.mock(DataStore.class); + Mockito.when(volume.getDataStore()).thenReturn(dataStore); + Mockito.lenient().when(dataStore.getId()).thenReturn(10L); + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + Mockito.lenient().when(primaryDataStoreDao.findById(10L)).thenReturn(pool); + Mockito.lenient().when(pool.getScope()).thenReturn(ScopeType.ZONE); + Mockito.lenient().when(pool.getHypervisor()).thenReturn(HypervisorType.None); + Mockito.when(volume.getInstanceId()).thenReturn(null); + Mockito.when(volume.getAccountId()).thenReturn(2L); + + // A recurring snapshot carries no caller-supplied name, so the auto-generated name must not be checked. + Mockito.lenient().when(snapshotDao.findByVolumeIdAndNameNotInStatus(Mockito.eq(volumeId), Mockito.anyString(), + Mockito.eq(Snapshot.State.Destroyed), Mockito.eq(Snapshot.State.Error))) + .thenReturn(Mockito.mock(SnapshotVO.class)); + + try { + snapshotManager.allocSnapshot(volumeId, Snapshot.MANUAL_POLICY_ID, null, null); + } catch (Exception e) { + // The rest of allocSnapshot is not stubbed here; this test only asserts the duplicate-name guard is + // not applied to an auto-generated name. + } + + Mockito.verify(snapshotDao, Mockito.never()).findByVolumeIdAndNameNotInStatus(Mockito.eq(volumeId), + Mockito.anyString(), Mockito.eq(Snapshot.State.Destroyed), Mockito.eq(Snapshot.State.Error)); + } finally { + CallContext.unregister(); + } + } + @Test public void testGetSnapshotZoneImageStoreValid() { final long snapshotId = 1L;
