Vered Volansky has uploaded a new change for review. Change subject: core: Add space validation for AddVmFromSnapshot ......................................................................
core: Add space validation for AddVmFromSnapshot AddVmFromSnapshotCommand didn't have storage space validation in its CDA. This patch adds validateSpaceRequirements() to the CDA in the parent class and a test for the above method. Change-Id: Ic0251766df9fdfad7d8dc77bcadc7f2b8b686772 Bug-Url: https://bugzilla.redhat.com/917682 Signed-off-by: Vered Volansky <vvola...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.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 3 files changed, 197 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/73/25773/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 182648e..c30daf2 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 @@ -129,12 +129,44 @@ return false; } } - return true; + return validateSpaceRequirements(); } return false; } protected abstract boolean checkImageConfiguration(DiskImage diskImage); + + /** + * Check if destination storage has enough space + * @return + */ + 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 (!validate(storageDomainValidator.isDomainWithinThresholds())) { + System.out.println("storageDomainValidator.isDomainWithinThresholds()" + storageDomainValidator.isDomainWithinThresholds()); + return false; + } + + for (DiskImage diskImage : disksList) { + List<DiskImage> snapshots = getAllImageSnapshots(diskImage); + diskImage.getSnapshots().addAll(snapshots); + + } + if (!validate(storageDomainValidator.hasSpaceForClonedDisks(disksList))) { + System.out.println("storageDomainValidator.hasSpaceForClonedDisks(disksList)" + storageDomainValidator.hasSpaceForClonedDisks(disksList) + disksList.toString()); + return false; + } + } + return true; + } + + protected List<DiskImage> getAllImageSnapshots(DiskImage diskImage) { + System.out.println("in actual getAllImageSnapshots"); + return ImagesHandler.getAllImageSnapshots(diskImage.getImageId(), diskImage.getImageTemplateId()); + } protected DiskImage cloneDiskImage(Guid newVmId, Guid storageDomainId, @@ -199,7 +231,7 @@ .getAllForStoragePool(getStoragePoolId()); Map<Guid, StorageDomain> storageDomainsMap = new HashMap<Guid, StorageDomain>(); for (StorageDomain storageDomain : domains) { - StorageDomainValidator validator = new StorageDomainValidator(storageDomain); + StorageDomainValidator validator = createStorageDomainValidator(storageDomain); if (validate(validator.isDomainExistAndActive()) && validate(validator.domainIsValidDestination())) { storageDomainsMap.put(storageDomain.getId(), storageDomain); } @@ -259,4 +291,8 @@ getVmStaticDao().update(getVm().getStaticData()); } + protected StorageDomainValidator createStorageDomainValidator(StorageDomain storageDomain) { + return new StorageDomainValidator(storageDomain); + } + } 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 9b95202..3134168 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 @@ -351,7 +351,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); @@ -374,7 +374,7 @@ return cmd; } - private static <T extends VmManagementParametersBase> void initCommandMethods(AddVmCommand<T> cmd) { + protected static <T extends VmManagementParametersBase> void initCommandMethods(AddVmCommand<T> cmd) { doReturn(Guid.newGuid()).when(cmd).getStoragePoolId(); doReturn(true).when(cmd).canAddVm(anyListOf(String.class), anyString(), any(Guid.class), anyInt()); doReturn(STORAGE_POOL_ID).when(cmd).getStoragePoolId(); 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..96e1553 --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommandTest.java @@ -0,0 +1,157 @@ +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 org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +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; +import org.ovirt.engine.core.dao.DiskImageDAO; +import org.ovirt.engine.core.dao.StorageDomainDAO; +import org.ovirt.engine.core.dao.VmDAO; +import org.ovirt.engine.core.dao.VmDeviceDAO; +import org.ovirt.engine.core.dao.VmTemplateDAO; + +@RunWith(MockitoJUnitRunner.class) +public class AddVmFromSnapshotCommandTest extends AddVmCommandTest{ + + 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; + + @Mock + private DiskImageDAO diskImageDao; + + @Mock + private StorageDomainDAO storageDomainDao; + + @Mock + private VmDAO vmDao; + + @Mock + private VmDeviceDAO vmDeviceDao; + + @Mock + private VmTemplateDAO vmTemplateDAO; + + private StorageDomainValidator storageDomainValidator; + + /** + * 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()); + } + + @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()); + } + + @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(ValidationResult.VALID).when(storageDomainValidator).hasSpaceForClonedDisks(any(List.class)); + doReturn(storageDomainValidator).when(command).createStorageDomainValidator(any(StorageDomain.class)); + mockGetAllSnapshots(); + assertFalse(command.validateSpaceRequirements()); + } + + private void generateStorageToDisksMap() { + 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)); + } + + private List<DiskImage> generateDisksList(int size) { + List<DiskImage> disksList = new ArrayList<>(); + for (int i = 0; i < size; ++i) { + DiskImage diskImage = new DiskImage(); + diskImage.setImageId(Guid.newGuid()); + disksList.add(diskImage); + } + return disksList; + } + + private void initDestSDs() { + StorageDomain sd1 = new StorageDomain(); + StorageDomain sd2 = new StorageDomain(); + sd1.setId(STORAGE_DOMAIN_ID_1); + sd2.setId(STORAGE_DOMAIN_ID_2); + command.destStorages.put(STORAGE_DOMAIN_ID_1, sd1); + command.destStorages.put(STORAGE_DOMAIN_ID_2, sd2); + } + + private List<DiskImage> createDiskSnapshot(Guid diskId, int size) { + List<DiskImage> disksList = new ArrayList<>(); + for (int i = 0; i < size; ++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 int domainSizeGB = 15; + final int sizeRequired = 4; + final Guid sourceSnapshotId = Guid.newGuid(); + command = setupCanAddVmFromSnapshotTests(domainSizeGB, sizeRequired, sourceSnapshotId); + generateStorageToDisksMap(); + initDestSDs(); + storageDomainValidator = mock(StorageDomainValidator.class); + } +} -- To view, visit http://gerrit.ovirt.org/25773 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic0251766df9fdfad7d8dc77bcadc7f2b8b686772 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vered Volansky <vvola...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
