Vered Volansky has uploaded a new change for review. Change subject: core: Fix AddVm faulty storage allocation checks ......................................................................
core: Fix AddVm faulty storage allocation checks Fix AddVmCommand and its descendants storage allocation checks. Amended and added relevant tests. Note that the tests here rely on the fact that StorageDomainValidator is well tested, and only simulate it's output, then testing the commands according to it. Change-Id: Iff4ad246934b3b94f21ae602067033347c913780 Bug-Url: https://bugzilla.redhat.com/917682 https://bugzilla.redhat.com/1053742 Signed-off-by: Vered Volansky <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommandTest.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommandTest.java 6 files changed, 314 insertions(+), 55 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/34/26734/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java index 1e0a5ae..ef4bdee 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java @@ -299,6 +299,19 @@ return super.buildAndCheckDestStorageDomains(); } + protected boolean validateFreeSpace(StorageDomainValidator storageDomainValidator, List<DiskImage> disksList) + { + for (DiskImage diskImage : disksList) { + List<DiskImage> snapshots = getAllImageSnapshots(diskImage); + diskImage.getSnapshots().addAll(snapshots); + } + return validate(storageDomainValidator.hasSpaceForClonedDisks(disksList)); + } + + protected List<DiskImage> getAllImageSnapshots(DiskImage diskImage) { + return ImagesHandler.getAllImageSnapshots(diskImage.getImageId(), diskImage.getImageTemplateId()); + } + /** * Logs error if one or more active domains are missing for disk images */ @@ -458,21 +471,6 @@ isVirtioScsiEnabled(), isBalloonEnabled(), false); - } - - @Override - protected int getNeededDiskSize(Guid storageDomainId) { - // Get the needed disk size by accumulating disk size - // of images on a given storage domain - int result = 0; - for (DiskImage img : getDiskImagesFromConfiguration()) { - if (img.getImageStatus() != ImageStatus.ILLEGAL) { - if (img.getStorageIds().get(0).equals(storageDomainId)) { - result = result + (int) Math.ceil(img.getActualSize()); - } - } - } - return result; } protected abstract VM getVmFromConfiguration(); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java index 40619b7..a46e05d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java @@ -303,10 +303,6 @@ return vmDisksSource.getStoragePoolId(); } - protected int getNeededDiskSize(Guid domainId) { - return getBlockSparseInitSizeInGb() * storageToDisksMap.get(domainId).size(); - } - protected boolean canDoAddVmCommand() { boolean returnValue = false; returnValue = areParametersLegal(getReturnValue().getCanDoActionMessages()); @@ -338,9 +334,10 @@ protected boolean validateSpaceRequirements() { for (Map.Entry<Guid, List<DiskImage>> sdImageEntry : storageToDisksMap.entrySet()) { StorageDomain destStorageDomain = destStorages.get(sdImageEntry.getKey()); + List<DiskImage> disksList = sdImageEntry.getValue(); StorageDomainValidator storageDomainValidator = createStorageDomainValidator(destStorageDomain); if (!validateDomainsThreshold(storageDomainValidator) || - !validateFreeSpace(storageDomainValidator, destStorageDomain)) { + !validateFreeSpace(storageDomainValidator, disksList)) { return false; } } @@ -355,9 +352,9 @@ return validate(storageDomainValidator.isDomainWithinThresholds()); } - protected boolean validateFreeSpace(StorageDomainValidator storageDomainValidator, StorageDomain domain) + protected boolean validateFreeSpace(StorageDomainValidator storageDomainValidator, List<DiskImage> disksList) { - return validate(storageDomainValidator.isDomainHasSpaceForRequest(getNeededDiskSize(domain.getId()))); + return validate(storageDomainValidator.hasSpaceForNewDisks(disksList)); } protected boolean checkSingleQxlDisplay() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java index 698a94d..27c9dd7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java @@ -3,6 +3,7 @@ import java.util.List; import org.ovirt.engine.core.bll.job.ExecutionHandler; +import org.ovirt.engine.core.bll.validator.StorageDomainValidator; import org.ovirt.engine.core.common.action.AddVmFromTemplateParameters; import org.ovirt.engine.core.common.action.CreateCloneOfTemplateParameters; import org.ovirt.engine.core.common.action.VdcActionType; @@ -137,14 +138,17 @@ return retValue; } - @Override - protected int getNeededDiskSize(Guid storageId) { - double actualSize = 0; - List<DiskImage> disks = storageToDisksMap.get(storageId); - for (DiskImage disk : disks) { - actualSize += disk.getActualSize(); + protected boolean validateFreeSpace(StorageDomainValidator storageDomainValidator, List<DiskImage> disksList) + { + for (DiskImage diskImage : disksList) { + List<DiskImage> snapshots = getAllImageSnapshots(diskImage); + diskImage.getSnapshots().addAll(snapshots); } - return (int) actualSize; + return validate(storageDomainValidator.hasSpaceForClonedDisks(disksList)); + } + + protected List<DiskImage> getAllImageSnapshots(DiskImage diskImage) { + return ImagesHandler.getAllImageSnapshots(diskImage.getImageId(), diskImage.getImageTemplateId()); } @Override diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java index 1c8f84f..4f9612a 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java @@ -9,7 +9,10 @@ import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.ArrayList; @@ -37,6 +40,7 @@ import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.DiskImageBase; import org.ovirt.engine.core.common.businessentities.DisplayType; +import org.ovirt.engine.core.common.businessentities.ImageStatus; import org.ovirt.engine.core.common.businessentities.Snapshot; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; @@ -68,17 +72,17 @@ private static final Guid STORAGE_DOMAIN_ID_1 = Guid.newGuid(); private static final Guid STORAGE_DOMAIN_ID_2 = Guid.newGuid(); - private static final int NUM_OF_DISKS_1 = 3; - private static final int NUM_OF_DISKS_2 = 3; + protected static final int TOTAL_NUM_DOMAINS = 2; + private static final int NUM_DISKS_STORAGE_DOMAIN_1 = 3; + private static final int NUM_DISKS_STORAGE_DOMAIN_2 = 3; private static final int REQUIRED_DISK_SIZE_GB = 10; private static final int AVAILABLE_SPACE_GB = 11; private static final int USED_SPACE_GB = 4; private static int MAX_PCI_SLOTS = 26; private static final Guid STORAGE_POOL_ID = Guid.newGuid(); - private static final Guid STORAGE_DOMAIN_ID = Guid.newGuid(); private VmTemplate vmTemplate = null; private VDSGroup vdsGroup = null; - private StorageDomainValidator storageDomainValidator; + protected StorageDomainValidator storageDomainValidator; @Rule public MockConfigRule mcr = new MockConfigRule(); @@ -127,11 +131,13 @@ mockStorageDomainDaoGetAllStoragesForPool(AVAILABLE_SPACE_GB); mockUninterestingMethods(cmd); + mockGetAllSnapshots(cmd); assertFalse("If the disk is too big, canDoAction should fail", cmd.canDoAction()); assertTrue("canDoAction failed for the wrong reason", cmd.getReturnValue() .getCanDoActionMessages() .contains(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_TARGET_STORAGE_DOMAIN.toString())); + } private void mockOsRepository() { @@ -145,6 +151,7 @@ final int sizeRequired = 5; AddVmCommand<VmManagementParametersBase> cmd = setupCanAddVmTests(domainSizeGB, sizeRequired); doReturn(Collections.emptyList()).when(cmd).validateCustomProperties(any(VmStatic.class)); + doReturn(true).when(cmd).validateSpaceRequirements(); assertTrue("vm could not be added", cmd.canAddVm(reasons, Arrays.asList(createStorageDomain(domainSizeGB)))); } @@ -208,7 +215,7 @@ when(osRepository.getArchitectureFromOS(any(Integer.class))).thenReturn(ArchitectureType.x86_64); when(osRepository.getDiskInterfaces(any(Integer.class), any(Version.class))).thenReturn( new ArrayList<>(Arrays.asList("VirtIO"))); - + mockGetAllSnapshots(cmd); CanDoActionTestUtils.runAndAssertCanDoActionFailure(cmd, VdcBllMessages.ACTION_TYPE_FAILED_ILLEGAL_OS_TYPE_DOES_NOT_SUPPORT_VIRTIO_SCSI); } @@ -224,15 +231,31 @@ } @Test - public void ValidateSpaceAndThreshold() { + public void validateSpaceAndThreshold() { AddVmCommand<VmManagementParametersBase> command = setupCanAddVmTests(0, 0); + doReturn(ValidationResult.VALID).when(storageDomainValidator).isDomainWithinThresholds(); + doReturn(ValidationResult.VALID).when(storageDomainValidator).hasSpaceForNewDisks(any(List.class)); + doReturn(storageDomainValidator).when(command).createStorageDomainValidator(any(StorageDomain.class)); assertTrue(command.validateSpaceRequirements()); + verify(storageDomainValidator, times(TOTAL_NUM_DOMAINS)).hasSpaceForNewDisks(any(List.class)); + verify(storageDomainValidator, never()).hasSpaceForClonedDisks(any(List.class)); } @Test - public void ValidateSpaceNotWithinThreshold() throws Exception { + public void validateSpaceNotEnough() throws Exception { AddVmCommand<VmManagementParametersBase> command = setupCanAddVmTests(0, 0); - storageDomainValidator = mock(StorageDomainValidator.class); + doReturn(ValidationResult.VALID).when(storageDomainValidator).isDomainWithinThresholds(); + doReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_TARGET_STORAGE_DOMAIN)). + when(storageDomainValidator).hasSpaceForNewDisks(any(List.class)); + doReturn(storageDomainValidator).when(command).createStorageDomainValidator(any(StorageDomain.class)); + assertFalse(command.validateSpaceRequirements()); + verify(storageDomainValidator).hasSpaceForNewDisks(any(List.class)); + verify(storageDomainValidator, never()).hasSpaceForClonedDisks(any(List.class)); + } + + @Test + public void validateSpaceNotWithinThreshold() throws Exception { + AddVmCommand<VmManagementParametersBase> command = setupCanAddVmTests(0, 0); doReturn((new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_TARGET_STORAGE_DOMAIN))). when(storageDomainValidator).isDomainWithinThresholds(); doReturn(storageDomainValidator).when(command).createStorageDomainValidator(any(StorageDomain.class)); @@ -258,7 +281,7 @@ doReturn(MAX_PCI_SLOTS).when(osRepository).getMaxPciDevices(anyInt(), any(Version.class)); } - private AddVmFromTemplateCommand<AddVmFromTemplateParameters> createVmFromTemplateCommand(VM vm) { + protected AddVmFromTemplateCommand<AddVmFromTemplateParameters> createVmFromTemplateCommand(VM vm) { AddVmFromTemplateParameters param = new AddVmFromTemplateParameters(); param.setVm(vm); AddVmFromTemplateCommand<AddVmFromTemplateParameters> concrete = @@ -284,6 +307,7 @@ doReturn(Collections.emptyList()).when(result).validateCustomProperties(any(VmStatic.class)); mockDAOs(result); mockBackend(result); + initCommandMethods(result); return result; } @@ -318,7 +342,7 @@ return cmd; } - private AddVmFromSnapshotCommand<AddVmFromSnapshotParameters> setupCanAddVmFromSnapshotTests(final int domainSizeGB, + protected AddVmFromSnapshotCommand<AddVmFromSnapshotParameters> setupCanAddVmFromSnapshotTests(final int domainSizeGB, final int sizeRequired, Guid sourceSnapshotId) { VM vm = initializeMock(domainSizeGB, sizeRequired); @@ -329,7 +353,7 @@ } private void initializeVmDAOMock(VM vm) { - when(vmDAO.get(Matchers.<Guid> any(Guid.class))).thenReturn(vm); + when(vmDAO.get(Matchers.<Guid>any(Guid.class))).thenReturn(vm); } private AddVmCommand<VmManagementParametersBase> setupCanAddVmTests(final int domainSizeGB, @@ -400,7 +424,7 @@ } private void mockVdsGroupDAOReturnVdsGroup() { - when(vdsGroupDao.get(Matchers.<Guid> any(Guid.class))).thenReturn(createVdsGroup()); + when(vdsGroupDao.get(Matchers.<Guid>any(Guid.class))).thenReturn(createVdsGroup()); } private VmTemplate createVmTemplate() { @@ -434,7 +458,7 @@ i.setSizeInGigabytes(USED_SPACE_GB + AVAILABLE_SPACE_GB); i.setActualSizeInBytes(REQUIRED_DISK_SIZE_GB * 1024L * 1024L * 1024L); i.setImageId(Guid.newGuid()); - i.setStorageIds(new ArrayList<Guid>(Arrays.asList(STORAGE_DOMAIN_ID))); + i.setStorageIds(new ArrayList<Guid>(Arrays.asList(STORAGE_DOMAIN_ID_1))); return i; } @@ -447,7 +471,7 @@ img.setSizeInGigabytes(size); img.setActualSize(size); img.setId(Guid.newGuid()); - img.setStorageIds(new ArrayList<Guid>(Arrays.asList(STORAGE_DOMAIN_ID))); + img.setStorageIds(new ArrayList<Guid>(Arrays.asList(STORAGE_DOMAIN_ID_1))); return img; } @@ -457,7 +481,7 @@ sd.setStatus(StorageDomainStatus.Active); sd.setAvailableDiskSize(availableSpace); sd.setUsedDiskSize(USED_SPACE_GB); - sd.setId(STORAGE_DOMAIN_ID); + sd.setId(STORAGE_DOMAIN_ID_1); return sd; } @@ -482,7 +506,7 @@ mockConfigSizeRequirements(requiredSpaceBufferInGB); } - private static VM createVm() { + protected static VM createVm() { VM vm = new VM(); VmDynamic dynamic = new VmDynamic(); VmStatic stat = new VmStatic(); @@ -509,11 +533,6 @@ } @Override - protected int getNeededDiskSize(Guid domainId) { - return getBlockSparseInitSizeInGb() * getVmTemplate().getDiskTemplateMap().size(); - } - - @Override public VmTemplate getVmTemplate() { return createVmTemplate(); } @@ -531,13 +550,13 @@ return cmd; } - private void generateStorageToDisksMap(AddVmCommand<VmManagementParametersBase> command) { + protected void generateStorageToDisksMap(AddVmCommand<? extends VmManagementParametersBase> command) { command.storageToDisksMap = new HashMap<Guid, List<DiskImage>>(); - command.storageToDisksMap.put(STORAGE_DOMAIN_ID_1, generateDisksList(NUM_OF_DISKS_1)); - command.storageToDisksMap.put(STORAGE_DOMAIN_ID_2, generateDisksList(NUM_OF_DISKS_2)); + command.storageToDisksMap.put(STORAGE_DOMAIN_ID_1, generateDisksList(NUM_DISKS_STORAGE_DOMAIN_1)); + command.storageToDisksMap.put(STORAGE_DOMAIN_ID_2, generateDisksList(NUM_DISKS_STORAGE_DOMAIN_2)); } - private List<DiskImage> generateDisksList(int size) { + static private List<DiskImage> generateDisksList(int size) { List<DiskImage> disksList = new ArrayList<>(); for (int i = 0; i < size; ++i) { DiskImage diskImage = new DiskImage(); @@ -547,7 +566,7 @@ return disksList; } - private void initDestSDs(AddVmCommand<VmManagementParametersBase> command) { + protected void initDestSDs(AddVmCommand<? extends VmManagementParametersBase> command) { StorageDomain sd1 = new StorageDomain(); StorageDomain sd2 = new StorageDomain(); sd1.setId(STORAGE_DOMAIN_ID_1); @@ -556,6 +575,33 @@ command.destStorages.put(STORAGE_DOMAIN_ID_2, sd2); } + protected List<DiskImage> createDiskSnapshot(Guid diskId, int numOfImages) { + List<DiskImage> disksList = new ArrayList<>(); + for (int i = 0; i < numOfImages; ++i) { + DiskImage diskImage = new DiskImage(); + diskImage.setActive(false); + diskImage.setId(diskId); + diskImage.setImageId(Guid.newGuid()); + diskImage.setParentId(Guid.newGuid()); + diskImage.setImageStatus(ImageStatus.OK); + disksList.add(diskImage); + } + return disksList; + } + + protected void mockGetAllSnapshots(AddVmFromTemplateCommand<AddVmFromTemplateParameters> command) { + doAnswer(new Answer<List<DiskImage>>() { + @Override + public List<DiskImage> answer(InvocationOnMock invocation) throws Throwable { + Object[] args = invocation.getArguments(); + DiskImage arg = (DiskImage) args[0]; + List<DiskImage> list = createDiskSnapshot(arg.getId(), 3); + return list; + } + }).when(command).getAllImageSnapshots(any(DiskImage.class)); + } + + private <T extends VmManagementParametersBase> void mockUninterestingMethods(AddVmCommand<T> spy) { doReturn(true).when(spy).isVmNameValidLength(Matchers.<VM> any(VM.class)); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommandTest.java new file mode 100644 index 0000000..331bb9f --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommandTest.java @@ -0,0 +1,107 @@ +package org.ovirt.engine.core.bll; + +import static junit.framework.Assert.assertTrue; +import static junit.framework.Assert.assertFalse; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.runners.MockitoJUnitRunner; +import org.ovirt.engine.core.bll.validator.StorageDomainValidator; +import org.ovirt.engine.core.common.action.AddVmFromSnapshotParameters; +import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.ImageStatus; +import org.ovirt.engine.core.common.businessentities.StorageDomain; +import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.compat.Guid; + +@RunWith(MockitoJUnitRunner.class) +public class AddVmFromSnapshotCommandTest extends AddVmCommandTest{ + + /** + * The command under test. + */ + protected AddVmFromSnapshotCommand<AddVmFromSnapshotParameters> command; + + @Test + public void validateSpaceAndThreshold() { + initCommand(); + doReturn(ValidationResult.VALID).when(storageDomainValidator).isDomainWithinThresholds(); + doReturn(ValidationResult.VALID).when(storageDomainValidator).hasSpaceForClonedDisks(any(List.class)); + doReturn(storageDomainValidator).when(command).createStorageDomainValidator(any(StorageDomain.class)); + mockGetAllSnapshots(); + assertTrue(command.validateSpaceRequirements()); + verify(storageDomainValidator, times(TOTAL_NUM_DOMAINS)).hasSpaceForClonedDisks(any(List.class)); + verify(storageDomainValidator, never()).hasSpaceForNewDisks(any(List.class)); + } + + @Test + public void validateSpaceNotEnough() throws Exception { + initCommand(); + doReturn(ValidationResult.VALID).when(storageDomainValidator).isDomainWithinThresholds(); + doReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_TARGET_STORAGE_DOMAIN)). + when(storageDomainValidator).hasSpaceForClonedDisks(any(List.class)); + doReturn(storageDomainValidator).when(command).createStorageDomainValidator(any(StorageDomain.class)); + mockGetAllSnapshots(); + assertFalse(command.validateSpaceRequirements()); + //The following is mocked to fail, should happen only once. + verify(storageDomainValidator).hasSpaceForClonedDisks(any(List.class)); + verify(storageDomainValidator, never()).hasSpaceForNewDisks(any(List.class)); + } + + @Test + public void validateSpaceNotWithinThreshold() throws Exception { + initCommand(); + doReturn((new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_TARGET_STORAGE_DOMAIN))). + when(storageDomainValidator).isDomainWithinThresholds(); + doReturn(storageDomainValidator).when(command).createStorageDomainValidator(any(StorageDomain.class)); + assertFalse(command.validateSpaceRequirements()); + } + + @Override + protected List<DiskImage> createDiskSnapshot(Guid diskId, int numOfImages) { + List<DiskImage> disksList = new ArrayList<>(); + for (int i = 0; i < numOfImages; ++i) { + DiskImage diskImage = new DiskImage(); + diskImage.setActive(false); + diskImage.setId(diskId); + diskImage.setImageId(Guid.newGuid()); + diskImage.setParentId(Guid.newGuid()); + diskImage.setImageStatus(ImageStatus.OK); + disksList.add(diskImage); + } + return disksList; + } + + private void mockGetAllSnapshots() { + doAnswer(new Answer<List<DiskImage>>() { + @Override + public List<DiskImage> answer(InvocationOnMock invocation) throws Throwable { + Object[] args = invocation.getArguments(); + DiskImage arg = (DiskImage) args[0]; + List<DiskImage> list = createDiskSnapshot(arg.getId(), 3); + return list; + } + }).when(command).getAllImageSnapshots(any(DiskImage.class)); + } + + private void initCommand() { + final Guid sourceSnapshotId = Guid.newGuid(); + command = setupCanAddVmFromSnapshotTests(0, 0, sourceSnapshotId); + generateStorageToDisksMap(command); + initDestSDs(command); + storageDomainValidator = mock(StorageDomainValidator.class); + } +} diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommandTest.java new file mode 100644 index 0000000..3041108 --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommandTest.java @@ -0,0 +1,107 @@ +package org.ovirt.engine.core.bll; + +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; +import org.ovirt.engine.core.bll.validator.StorageDomainValidator; +import org.ovirt.engine.core.common.action.AddVmFromTemplateParameters; +import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.ImageStatus; +import org.ovirt.engine.core.common.businessentities.StorageDomain; +import org.ovirt.engine.core.common.businessentities.VM; +import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.compat.Guid; + +@RunWith(MockitoJUnitRunner.class) +public class AddVmFromTemplateCommandTest extends AddVmCommandTest{ + + /** + * The command under test. + */ + protected AddVmFromTemplateCommand<AddVmFromTemplateParameters> command; + + @Test + public void validateSpaceAndThreshold() { + initCommand(); + doReturn(ValidationResult.VALID).when(storageDomainValidator).isDomainWithinThresholds(); + doReturn(ValidationResult.VALID).when(storageDomainValidator).hasSpaceForClonedDisks(any(List.class)); + doReturn(storageDomainValidator).when(command).createStorageDomainValidator(any(StorageDomain.class)); + mockGetAllSnapshots(); + assertTrue(command.validateSpaceRequirements()); + verify(storageDomainValidator, times(TOTAL_NUM_DOMAINS)).hasSpaceForClonedDisks(any(List.class)); + verify(storageDomainValidator, never()).hasSpaceForNewDisks(any(List.class)); + } + + @Test + public void validateSpaceNotEnough() throws Exception { + initCommand(); + doReturn(ValidationResult.VALID).when(storageDomainValidator).isDomainWithinThresholds(); + doReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_TARGET_STORAGE_DOMAIN)). + when(storageDomainValidator).hasSpaceForClonedDisks(any(List.class)); + doReturn(storageDomainValidator).when(command).createStorageDomainValidator(any(StorageDomain.class)); + mockGetAllSnapshots(); + assertFalse(command.validateSpaceRequirements()); + //The following is mocked to fail, should happen only once. + verify(storageDomainValidator).hasSpaceForClonedDisks(any(List.class)); + verify(storageDomainValidator, never()).hasSpaceForNewDisks(any(List.class)); + } + + @Test + public void validateSpaceNotWithinThreshold() throws Exception { + initCommand(); + doReturn((new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_TARGET_STORAGE_DOMAIN))). + when(storageDomainValidator).isDomainWithinThresholds(); + doReturn(storageDomainValidator).when(command).createStorageDomainValidator(any(StorageDomain.class)); + assertFalse(command.validateSpaceRequirements()); + } + + @Override + protected List<DiskImage> createDiskSnapshot(Guid diskId, int numOfImages) { + List<DiskImage> disksList = new ArrayList<>(); + for (int i = 0; i < numOfImages; ++i) { + DiskImage diskImage = new DiskImage(); + diskImage.setActive(false); + diskImage.setId(diskId); + diskImage.setImageId(Guid.newGuid()); + diskImage.setParentId(Guid.newGuid()); + diskImage.setImageStatus(ImageStatus.OK); + disksList.add(diskImage); + } + return disksList; + } + + private void mockGetAllSnapshots() { + doAnswer(new Answer<List<DiskImage>>() { + @Override + public List<DiskImage> answer(InvocationOnMock invocation) throws Throwable { + Object[] args = invocation.getArguments(); + DiskImage arg = (DiskImage) args[0]; + List<DiskImage> list = createDiskSnapshot(arg.getId(), 3); + return list; + } + }).when(command).getAllImageSnapshots(any(DiskImage.class)); + } + + private void initCommand() { + VM vm = createVm(); + command = createVmFromTemplateCommand(vm); + generateStorageToDisksMap(command); + initDestSDs(command); + storageDomainValidator = mock(StorageDomainValidator.class); + } +} -- To view, visit http://gerrit.ovirt.org/26734 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iff4ad246934b3b94f21ae602067033347c913780 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vered Volansky <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
