From 007f5b842d7b0c8d200b562f48ad610b1324ca16 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 9 Sep 2015 14:48:31 +0200 Subject: [PATCH 1/3] CLOUDSTACK-5863: Add unit tests for create/delete/revert volume snapshot --- .../storage/snapshot/SnapshotManagerImpl.java | 54 ++-- .../storage/snapshot/SnapshotManagerTest.java | 282 ++++++++++++++++++ 2 files changed, 309 insertions(+), 27 deletions(-) create mode 100644 server/test/com/cloud/storage/snapshot/SnapshotManagerTest.java diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java index c622c552710..e8b9a0e0cd7 100644 --- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -134,59 +134,59 @@ import com.cloud.vm.snapshot.dao.VMSnapshotDao; public class SnapshotManagerImpl extends ManagerBase implements SnapshotManager, SnapshotApiService { private static final Logger s_logger = Logger.getLogger(SnapshotManagerImpl.class); @Inject - private VMTemplateDao _templateDao; + VMTemplateDao _templateDao; @Inject - private UserVmDao _vmDao; + UserVmDao _vmDao; @Inject - private VolumeDao _volsDao; + VolumeDao _volsDao; @Inject - private AccountDao _accountDao; + AccountDao _accountDao; @Inject - private SnapshotDao _snapshotDao; + SnapshotDao _snapshotDao; @Inject - private SnapshotDataStoreDao _snapshotStoreDao; + SnapshotDataStoreDao _snapshotStoreDao; @Inject - private PrimaryDataStoreDao _storagePoolDao; + PrimaryDataStoreDao _storagePoolDao; @Inject - private final SnapshotPolicyDao _snapshotPolicyDao = null; + SnapshotPolicyDao _snapshotPolicyDao = null; @Inject - private SnapshotScheduleDao _snapshotScheduleDao; + SnapshotScheduleDao _snapshotScheduleDao; @Inject - private DomainDao _domainDao; + DomainDao _domainDao; @Inject - private StorageManager _storageMgr; + StorageManager _storageMgr; @Inject - private SnapshotScheduler _snapSchedMgr; + SnapshotScheduler _snapSchedMgr; @Inject - private AccountManager _accountMgr; + AccountManager _accountMgr; @Inject - private AlertManager _alertMgr; + AlertManager _alertMgr; @Inject - private ClusterDao _clusterDao; + ClusterDao _clusterDao; @Inject - private ResourceLimitService _resourceLimitMgr; + ResourceLimitService _resourceLimitMgr; @Inject - private DomainManager _domainMgr; + DomainManager _domainMgr; @Inject - private ResourceTagDao _resourceTagDao; + ResourceTagDao _resourceTagDao; @Inject - private ConfigurationDao _configDao; + ConfigurationDao _configDao; @Inject - private VMSnapshotDao _vmSnapshotDao; + VMSnapshotDao _vmSnapshotDao; @Inject - private DataStoreManager dataStoreMgr; + DataStoreManager dataStoreMgr; @Inject - private SnapshotService snapshotSrv; + SnapshotService snapshotSrv; @Inject - private VolumeDataFactory volFactory; + VolumeDataFactory volFactory; @Inject - private SnapshotDataFactory snapshotFactory; + SnapshotDataFactory snapshotFactory; @Inject - private EndPointSelector _epSelector; + EndPointSelector _epSelector; @Inject - private ResourceManager _resourceMgr; + ResourceManager _resourceMgr; @Inject - private StorageStrategyFactory _storageStrategyFactory; + StorageStrategyFactory _storageStrategyFactory; private int _totalRetries; private int _pauseInterval; diff --git a/server/test/com/cloud/storage/snapshot/SnapshotManagerTest.java b/server/test/com/cloud/storage/snapshot/SnapshotManagerTest.java new file mode 100644 index 00000000000..4fb5964a76a --- /dev/null +++ b/server/test/com/cloud/storage/snapshot/SnapshotManagerTest.java @@ -0,0 +1,282 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.storage.snapshot; + +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyLong; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.UUID; + +import org.apache.cloudstack.acl.ControlledEntity; +import org.apache.cloudstack.acl.SecurityChecker.AccessType; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +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.SnapshotStrategy; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation; +import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.mockito.Spy; + +import com.cloud.configuration.Resource.ResourceType; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.ResourceAllocationException; +import com.cloud.hypervisor.Hypervisor; +import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.resource.ResourceManager; +import com.cloud.storage.DataStoreRole; +import com.cloud.storage.ScopeType; +import com.cloud.storage.Snapshot; +import com.cloud.storage.SnapshotVO; +import com.cloud.storage.Volume; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.Storage.ImageFormat; +import com.cloud.storage.dao.SnapshotDao; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.user.Account; +import com.cloud.user.AccountManager; +import com.cloud.user.AccountVO; +import com.cloud.user.ResourceLimitService; +import com.cloud.user.User; +import com.cloud.user.UserVO; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.UserVmVO; +import com.cloud.vm.VirtualMachine.State; +import com.cloud.vm.dao.UserVmDao; +import com.cloud.vm.snapshot.VMSnapshot; +import com.cloud.vm.snapshot.VMSnapshotVO; +import com.cloud.vm.snapshot.dao.VMSnapshotDao; + +public class SnapshotManagerTest { + @Spy + SnapshotManagerImpl _snapshotMgr = new SnapshotManagerImpl(); + @Mock + SnapshotDao _snapshotDao; + @Mock + VolumeDao _volumeDao; + @Mock + UserVmDao _vmDao; + @Mock + PrimaryDataStoreDao _storagePoolDao; + @Mock + VMSnapshotDao _vmSnapshotDao; + @Mock + Account account; + @Mock + UserVmVO vmMock; + @Mock + VolumeVO volumeMock; + @Mock + VolumeInfo volumeInfoMock; + @Mock + VolumeDataFactory volumeFactory; + @Mock + SnapshotVO snapshotMock; + @Mock + SnapshotInfo snapshotInfoMock; + @Mock + SnapshotDataFactory snapshotFactory; + @Mock + StorageStrategyFactory _storageStrategyFactory; + @Mock + SnapshotStrategy snapshotStrategy; + @Mock + AccountManager _accountMgr; + @Mock + ResourceLimitService _resourceLimitMgr; + @Mock + StoragePoolVO poolMock; + @Mock + ResourceManager _resourceMgr; + @Mock + DataStore storeMock; + + private static final long TEST_SNAPSHOT_ID = 3L; + private static final long TEST_VOLUME_ID = 4L; + private static final long TEST_VM_ID = 5L; + private static final long TEST_STORAGE_POOL_ID = 6L; + + @Before + public void setup() throws ResourceAllocationException { + MockitoAnnotations.initMocks(this); + _snapshotMgr._snapshotDao = _snapshotDao; + _snapshotMgr._volsDao = _volumeDao; + _snapshotMgr._vmDao = _vmDao; + _snapshotMgr.volFactory = volumeFactory; + _snapshotMgr.snapshotFactory = snapshotFactory; + _snapshotMgr._storageStrategyFactory = _storageStrategyFactory; + _snapshotMgr._accountMgr = _accountMgr; + _snapshotMgr._resourceLimitMgr = _resourceLimitMgr; + _snapshotMgr._storagePoolDao = _storagePoolDao; + _snapshotMgr._resourceMgr = _resourceMgr; + _snapshotMgr._vmSnapshotDao = _vmSnapshotDao; + + when(_snapshotDao.findById(anyLong())).thenReturn(snapshotMock); + when(snapshotMock.getVolumeId()).thenReturn(TEST_VOLUME_ID); + when(snapshotMock.isRecursive()).thenReturn(false); + + when(_volumeDao.findById(anyLong())).thenReturn(volumeMock); + when(volumeMock.getState()).thenReturn(Volume.State.Ready); + when(volumeFactory.getVolume(anyLong())).thenReturn(volumeInfoMock); + when(volumeInfoMock.getDataStore()).thenReturn(storeMock); + when(storeMock.getId()).thenReturn(TEST_STORAGE_POOL_ID); + + when(snapshotFactory.getSnapshot(anyLong(), Mockito.any(DataStoreRole.class))).thenReturn(snapshotInfoMock); + when(_storageStrategyFactory.getSnapshotStrategy(Mockito.any(SnapshotVO.class), Mockito.eq(SnapshotOperation.REVERT))).thenReturn(snapshotStrategy); + when(_storageStrategyFactory.getSnapshotStrategy(Mockito.any(SnapshotVO.class), Mockito.eq(SnapshotOperation.DELETE))).thenReturn(snapshotStrategy); + + doNothing().when(_accountMgr).checkAccess(any(Account.class), any(AccessType.class), any(Boolean.class), any(ControlledEntity.class)); + doNothing().when(_snapshotMgr._resourceLimitMgr).checkResourceLimit(any(Account.class), any(ResourceType.class)); + doNothing().when(_snapshotMgr._resourceLimitMgr).checkResourceLimit(any(Account.class), any(ResourceType.class), anyLong()); + doNothing().when(_snapshotMgr._resourceLimitMgr).decrementResourceCount(anyLong(), any(ResourceType.class), anyLong()); + doNothing().when(_snapshotMgr._resourceLimitMgr).incrementResourceCount(anyLong(), any(ResourceType.class)); + doNothing().when(_snapshotMgr._resourceLimitMgr).incrementResourceCount(anyLong(), any(ResourceType.class), anyLong()); + + Account account = new AccountVO("testaccount", 1L, "networkdomain", (short)0, "uuid"); + UserVO user = new UserVO(1, "testuser", "password", "firstname", "lastName", "email", "timezone", UUID.randomUUID().toString(), User.Source.UNKNOWN); + CallContext.register(user, account); + when(_accountMgr.getAccount(anyLong())).thenReturn(account); + + when(_storagePoolDao.findById(anyLong())).thenReturn(poolMock); + when(poolMock.getScope()).thenReturn(ScopeType.ZONE); + when(poolMock.getHypervisor()).thenReturn(HypervisorType.KVM); + when(_resourceMgr.listAllUpAndEnabledHostsInOneZoneByHypervisor(any(HypervisorType.class), anyLong())).thenReturn(null); + } + + @After + public void tearDown() { + CallContext.unregister(); + } + + // vm is destroyed + @Test(expected = CloudRuntimeException.class) + public void testAllocSnapshotF1() throws ResourceAllocationException { + when(_vmDao.findById(anyLong())).thenReturn(vmMock); + when(vmMock.getState()).thenReturn(State.Destroyed); + _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null); + } + + // active snapshots + @SuppressWarnings("unchecked") + @Test(expected = InvalidParameterValueException.class) + public void testAllocSnapshotF2() throws ResourceAllocationException { + when(_vmDao.findById(anyLong())).thenReturn(vmMock); + when(vmMock.getId()).thenReturn(TEST_VM_ID); + when(vmMock.getState()).thenReturn(State.Stopped); + when(vmMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + when(volumeInfoMock.getInstanceId()).thenReturn(TEST_VM_ID); + List mockList = mock(List.class); + when(mockList.size()).thenReturn(1); + when(_snapshotDao.listByInstanceId(TEST_VM_ID, Snapshot.State.Creating, Snapshot.State.CreatedOnPrimary, Snapshot.State.BackingUp)).thenReturn(mockList); + _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null); + } + + // active vm snapshots + @SuppressWarnings("unchecked") + @Test(expected = CloudRuntimeException.class) + public void testAllocSnapshotF3() throws ResourceAllocationException { + when(_vmDao.findById(anyLong())).thenReturn(vmMock); + when(vmMock.getId()).thenReturn(TEST_VM_ID); + when(vmMock.getState()).thenReturn(State.Stopped); + when(vmMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + when(volumeInfoMock.getInstanceId()).thenReturn(TEST_VM_ID); + List mockList = mock(List.class); + when(mockList.size()).thenReturn(0); + when(_snapshotDao.listByInstanceId(TEST_VM_ID, Snapshot.State.Creating, Snapshot.State.CreatedOnPrimary, Snapshot.State.BackingUp)).thenReturn(mockList); + List mockList2 = mock(List.class); + when(mockList2.size()).thenReturn(1); + when(_vmSnapshotDao.listByInstanceId(TEST_VM_ID, VMSnapshot.State.Creating, VMSnapshot.State.Reverting, VMSnapshot.State.Expunging)).thenReturn(mockList2); + _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null); + } + + // successful test + @SuppressWarnings("unchecked") + @Test + public void testAllocSnapshotF4() throws ResourceAllocationException { + when(_vmDao.findById(anyLong())).thenReturn(vmMock); + when(vmMock.getId()).thenReturn(TEST_VM_ID); + when(vmMock.getState()).thenReturn(State.Stopped); + when(vmMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + when(volumeInfoMock.getInstanceId()).thenReturn(TEST_VM_ID); + List mockList = mock(List.class); + when(mockList.size()).thenReturn(0); + when(_snapshotDao.listByInstanceId(TEST_VM_ID, Snapshot.State.Creating, Snapshot.State.CreatedOnPrimary, Snapshot.State.BackingUp)).thenReturn(mockList); + List mockList2 = mock(List.class); + when(mockList2.size()).thenReturn(0); + when(_vmSnapshotDao.listByInstanceId(TEST_VM_ID, VMSnapshot.State.Creating, VMSnapshot.State.Reverting, VMSnapshot.State.Expunging)).thenReturn(mockList2); + when(_snapshotDao.persist(any(SnapshotVO.class))).thenReturn(snapshotMock); + _snapshotMgr.allocSnapshot(TEST_VOLUME_ID, Snapshot.MANUAL_POLICY_ID, null); + } + + @Test + public void testDeleteSnapshotF1() { + when(snapshotStrategy.deleteSnapshot(TEST_SNAPSHOT_ID)).thenReturn(true); + when(snapshotMock.getState()).thenReturn(Snapshot.State.Destroyed); + when(snapshotMock.getAccountId()).thenReturn(2L); + when(snapshotMock.getDataCenterId()).thenReturn(2L); + boolean result =_snapshotMgr.deleteSnapshot(TEST_SNAPSHOT_ID); + Assert.assertTrue(result); + } + + // vm state not stopped + @Test(expected = InvalidParameterValueException.class) + public void testRevertSnapshotF1() { + when(volumeMock.getInstanceId()).thenReturn(TEST_VM_ID); + when(_vmDao.findById(anyLong())).thenReturn(vmMock); + when(vmMock.getState()).thenReturn(State.Running); + _snapshotMgr.revertSnapshot(TEST_SNAPSHOT_ID); + } + + // vm on Xenserver, return null + @Test + public void testRevertSnapshotF2() { + when(_vmDao.findById(anyLong())).thenReturn(vmMock); + when(vmMock.getState()).thenReturn(State.Stopped); + when(vmMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.XenServer); + when(volumeMock.getFormat()).thenReturn(ImageFormat.VHD); + Snapshot snapshot = _snapshotMgr.revertSnapshot(TEST_SNAPSHOT_ID); + Assert.assertNull(snapshot); + } + + // vm on KVM, successful + @Test + public void testRevertSnapshotF3() { + when(_vmDao.findById(anyLong())).thenReturn(vmMock); + when(vmMock.getState()).thenReturn(State.Stopped); + when(vmMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + when(volumeMock.getFormat()).thenReturn(ImageFormat.QCOW2); + when (snapshotStrategy.revertSnapshot(Mockito.any(SnapshotInfo.class))).thenReturn(true); + when(_volumeDao.update(anyLong(), any(VolumeVO.class))).thenReturn(true); + Snapshot snapshot = _snapshotMgr.revertSnapshot(TEST_SNAPSHOT_ID); + Assert.assertNotNull(snapshot); + } +} From e586d11a1b666e61cc43d75313f9b9d3c3672f8f Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 9 Sep 2015 14:49:07 +0200 Subject: [PATCH 2/3] CLOUDSTACK-5863: Add unit tests for take volume snapshot --- .../storage/VolumeApiServiceImplTest.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/server/test/com/cloud/storage/VolumeApiServiceImplTest.java b/server/test/com/cloud/storage/VolumeApiServiceImplTest.java index 789730b4407..0e46142fe2c 100644 --- a/server/test/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/test/com/cloud/storage/VolumeApiServiceImplTest.java @@ -17,6 +17,7 @@ package com.cloud.storage; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyLong; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.when; @@ -41,8 +42,10 @@ import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService; import org.apache.cloudstack.framework.jobs.AsyncJobExecutionContext; import org.apache.cloudstack.framework.jobs.AsyncJobManager; import org.apache.cloudstack.framework.jobs.dao.AsyncJobJoinMapDao; @@ -51,6 +54,7 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.ResourceAllocationException; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.storage.dao.VolumeDao; import com.cloud.user.Account; @@ -87,6 +91,12 @@ public class VolumeApiServiceImplTest { @Mock VMInstanceDao _vmInstanceDao; + @Mock + VolumeInfo volumeInfoMock; + @Mock + SnapshotInfo snapshotInfoMock; + @Mock + VolumeService volService; DetachVolumeCmd detachCmd = new DetachVolumeCmd(); Class _detachCmdClass = detachCmd.getClass(); @@ -103,6 +113,7 @@ public class VolumeApiServiceImplTest { _svc._vmInstanceDao = _vmInstanceDao; _svc._jobMgr = _jobMgr; _svc.volFactory = _volFactory; + _svc.volService = volService; // mock caller context AccountVO account = new AccountVO("admin", 1L, "networkDomain", Account.ACCOUNT_TYPE_NORMAL, "uuid"); @@ -327,6 +338,23 @@ public class VolumeApiServiceImplTest { _svc.attachVolumeToVM(2L, 6L, 0L); } + // volume not Ready + @Test(expected = InvalidParameterValueException.class) + public void testTakeSnapshotF1() throws ResourceAllocationException { + when(_volFactory.getVolume(anyLong())).thenReturn(volumeInfoMock); + when(volumeInfoMock.getState()).thenReturn(Volume.State.Allocated); + _svc.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, null, false); + } + + @Test + public void testTakeSnapshotF2() throws ResourceAllocationException { + when(_volFactory.getVolume(anyLong())).thenReturn(volumeInfoMock); + when(volumeInfoMock.getState()).thenReturn(Volume.State.Ready); + when(volumeInfoMock.getInstanceId()).thenReturn(null); + when (volService.takeSnapshot(Mockito.any(VolumeInfo.class))).thenReturn(snapshotInfoMock); + _svc.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, null, false); + } + @After public void tearDown() { CallContext.unregister(); From c15729e71afb582d300c608f6b230af15a7d1958 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 9 Sep 2015 14:57:58 +0200 Subject: [PATCH 3/3] Fix coverity scan 1323172 --- .../cloudstack/storage/snapshot/SnapshotServiceImpl.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java index a9aad0ab950..f7f044fa8f1 100644 --- a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java +++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java @@ -409,6 +409,9 @@ public class SnapshotServiceImpl implements SnapshotService { @Override public boolean revertSnapshot(SnapshotInfo snapshot) { SnapshotInfo snapshotOnPrimaryStore = _snapshotFactory.getSnapshot(snapshot.getId(), DataStoreRole.Primary); + if (snapshotOnPrimaryStore == null) { + throw new CloudRuntimeException("Cannot find an entry for snapshot " + snapshot.getId() + " on primary storage pools"); + } PrimaryDataStore store = (PrimaryDataStore)snapshotOnPrimaryStore.getDataStore(); AsyncCallFuture future = new AsyncCallFuture();