Vered Volansky has uploaded a new change for review. Change subject: core: Add space validation when creating snapshot ......................................................................
core: Add space validation when creating snapshot In CreateAllSnapshotsFromVmCommand, added to CDA missing storage space validations. For that, added allDomainsHaveSpaceForNewDisks to MultipleStorageDomainsValidator . Added relevant tests. Change-Id: I0a5b5df695d30a34fbdef31da2320b322e27466b Bug-Url: https://bugzilla.redhat.com/1053752 Signed-off-by: Vered Volansky <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidatorTest.java 4 files changed, 118 insertions(+), 8 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/56/30056/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java index f6cd6d3..a1a27ab 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java @@ -485,7 +485,8 @@ if (!(validate(diskImagesValidator.diskImagesNotLocked()) && validate(diskImagesValidator.diskImagesNotIllegal()) && validate(sdValidator.allDomainsExistAndActive()) - && validate(sdValidator.allDomainsWithinThresholds()))) { + && validate(sdValidator.allDomainsWithinThresholds()) + && validate(sdValidator.allDomainsHaveSpaceForNewDisks(disksList)))) { return false; } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidator.java index 0822b9d..f3988b5 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidator.java @@ -2,12 +2,15 @@ import java.util.Collection; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dao.StorageDomainDAO; +import org.ovirt.engine.core.utils.collections.MultiValueMapUtils; /** * A validator for multiple storage domains. @@ -39,6 +42,17 @@ } } + private Map<Guid, List<DiskImage>> getDomainsDisksMap(List<DiskImage> disksList) { + Map<Guid, List<DiskImage>> domainsDisksMap = new HashMap<>(); + for (DiskImage disk : disksList) { + List<Guid> domainIds = disk.getStorageIds(); + for (Guid domainId : domainIds) { + MultiValueMapUtils.addToMap(domainId, disk, domainsDisksMap); + } + } + return domainsDisksMap; + } + /** * Validates that all the domains exist and are active. * @return {@link ValidationResult#VALID} if all the domains are OK, or a {@link ValidationResult} with the first non-active domain encountered. @@ -46,8 +60,8 @@ public ValidationResult allDomainsExistAndActive() { return validOrFirstFailure(new ValidatorPredicate() { @Override - public ValidationResult evaluate(StorageDomainValidator validator) { - return validator.isDomainExistAndActive(); + public ValidationResult evaluate(Map.Entry<Guid, StorageDomainValidator> entry) { + return getStorageDomainValidator(entry).isDomainExistAndActive(); } }); } @@ -59,14 +73,30 @@ public ValidationResult allDomainsWithinThresholds() { return validOrFirstFailure(new ValidatorPredicate() { @Override - public ValidationResult evaluate(StorageDomainValidator validator) { - return validator.isDomainWithinThresholds(); + public ValidationResult evaluate(Map.Entry<Guid, StorageDomainValidator> entry) { + return getStorageDomainValidator(entry).isDomainWithinThresholds(); + } + }); + } + + /** + * Validates that all the domains have enough space for the request + * @return {@link ValidationResult#VALID} if all the domains have enough free space, or a {@link ValidationResult} with the first low-on-space domain encountered. + */ + public ValidationResult allDomainsHaveSpaceForNewDisks(final List<DiskImage> disksList) { + final Map<Guid, List<DiskImage>> domainsDisksMap = getDomainsDisksMap(disksList); + return validOrFirstFailure(new ValidatorPredicate() { + @Override + public ValidationResult evaluate(Map.Entry<Guid, StorageDomainValidator> entry) { + Guid sdId = entry.getKey(); + List disksForDomain = domainsDisksMap.get(sdId); + return getStorageDomainValidator(entry).hasSpaceForNewDisks(disksForDomain); } }); } /** @return The lazy-loaded validator for the given map entry */ - private StorageDomainValidator getStorageDomainValidator(Map.Entry<Guid, StorageDomainValidator> entry) { + protected StorageDomainValidator getStorageDomainValidator(Map.Entry<Guid, StorageDomainValidator> entry) { if (entry.getValue() == null) { entry.setValue(new StorageDomainValidator(getStorageDomainDAO().getForStoragePool(entry.getKey(), storagePoolId))); } @@ -87,7 +117,7 @@ */ private ValidationResult validOrFirstFailure(ValidatorPredicate predicate) { for (Map.Entry<Guid, StorageDomainValidator> entry : domainValidators.entrySet()) { - ValidationResult currResult = predicate.evaluate(getStorageDomainValidator(entry)); + ValidationResult currResult = predicate.evaluate(entry); if (!currResult.isValid()) { return currResult; } @@ -97,6 +127,6 @@ /** A predicate for evaluating storage domains */ private static interface ValidatorPredicate { - public ValidationResult evaluate(StorageDomainValidator validator); + public ValidationResult evaluate(Map.Entry<Guid, StorageDomainValidator> entry); } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java index a985acb..a32bef0 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java @@ -3,8 +3,10 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyList; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import java.util.ArrayList; import java.util.Arrays; @@ -264,11 +266,35 @@ .contains(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_STORAGE_DOMAIN.name())); } + @Test + public void testAllDomainsHaveSpaceForNewDisksFailure() { + setUpGeneralValidations(); + setUpDiskValidations(); + List<DiskImage> disksList = getNonEmptyDiskList(); + doReturn(disksList).when(cmd).getDisksList(); + doReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_STORAGE_DOMAIN)).when(multipleStorageDomainsValidator) + .allDomainsHaveSpaceForNewDisks(disksList); + CanDoActionTestUtils.runAndAssertCanDoActionFailure(cmd, VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_STORAGE_DOMAIN); + verify(multipleStorageDomainsValidator).allDomainsHaveSpaceForNewDisks(disksList); + } + + @Test + public void testAllDomainsHaveSpaceForNewDisksSuccess() { + setUpGeneralValidations(); + setUpDiskValidations(); + List<DiskImage> disksList = getNonEmptyDiskList(); + doReturn(disksList).when(cmd).getDisksList(); + doReturn(ValidationResult.VALID).when(multipleStorageDomainsValidator).allDomainsHaveSpaceForNewDisks(disksList); + CanDoActionTestUtils.runAndAssertCanDoActionSuccess(cmd); + verify(multipleStorageDomainsValidator).allDomainsHaveSpaceForNewDisks(disksList); + } + private void setUpDiskValidations() { doReturn(ValidationResult.VALID).when(diskImagesValidator).diskImagesNotLocked(); doReturn(ValidationResult.VALID).when(diskImagesValidator).diskImagesNotIllegal(); doReturn(ValidationResult.VALID).when(multipleStorageDomainsValidator).allDomainsExistAndActive(); doReturn(ValidationResult.VALID).when(multipleStorageDomainsValidator).allDomainsWithinThresholds(); + doReturn(ValidationResult.VALID).when(multipleStorageDomainsValidator).allDomainsHaveSpaceForNewDisks(anyList()); } private void setUpGeneralValidations() { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidatorTest.java index bdcbcb1..a161279 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidatorTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidatorTest.java @@ -3,12 +3,20 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyList; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; +import java.util.Map; import org.junit.Before; import org.junit.ClassRule; @@ -17,6 +25,7 @@ import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.config.ConfigValues; @@ -44,6 +53,9 @@ private StorageDomain domain2; private MultipleStorageDomainsValidator validator; + + private static final int NUM_DISKS = 3; + private static final int NUM_DOMAINS = 2; @Before public void setUp() { @@ -100,4 +112,45 @@ VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_STORAGE_DOMAIN, result.getMessage()); } + + @Test + public void testAllDomainsHaveSpaceForNewDisksSuccess(){ + List<DiskImage> disksList = generateDisksList(NUM_DISKS); + + StorageDomainValidator storageDomainValidator = mock(StorageDomainValidator.class); + doReturn(ValidationResult.VALID).when(storageDomainValidator).hasSpaceForNewDisks(anyList()); + doReturn(storageDomainValidator).when(validator).getStorageDomainValidator(any(Map.Entry.class)); + + assertTrue(validator.allDomainsHaveSpaceForNewDisks(disksList).isValid()); + verify(storageDomainValidator, times(NUM_DOMAINS)).hasSpaceForNewDisks(anyList()); + } + + @Test + public void testAllDomainsHaveSpaceForNewDisksFail(){ + List<DiskImage> disksList = generateDisksList(NUM_DISKS); + + StorageDomainValidator storageDomainValidator = mock(StorageDomainValidator.class); + doReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_STORAGE_DOMAIN)). + when(storageDomainValidator).hasSpaceForNewDisks(anyList()); + + ValidationResult result = validator.allDomainsHaveSpaceForNewDisks(disksList); + assertFalse(result.isValid()); + assertEquals("Wrong validation error", + VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_STORAGE_DOMAIN, + result.getMessage()); + } + + private List<DiskImage> generateDisksList(int size) { + List<DiskImage> disksList = new ArrayList<>(); + ArrayList<Guid> sdIds = new ArrayList<>(); + sdIds.add(sdId1); + sdIds.add(sdId2); + for (int i = 0; i < size; ++i) { + DiskImage diskImage = new DiskImage(); + diskImage.setImageId(Guid.newGuid()); + diskImage.setStorageIds(sdIds); + disksList.add(diskImage); + } + return disksList; + } } -- To view, visit http://gerrit.ovirt.org/30056 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0a5b5df695d30a34fbdef31da2320b322e27466b Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Vered Volansky <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
