Amit Aviram has uploaded a new change for review. Change subject: core: Storage domain attaching validator addition. ......................................................................
core: Storage domain attaching validator addition. Storage domain attaching validations was extracted to a new validator. Storage domain validations was extracted as well to its proper validator. Change-Id: I079a51b5ddc36805a945ffc59965101418187441 Signed-off-by: Amit Aviram <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStoragePoolWithStoragesCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/AttachDomainValidator.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/StorageDomainValidator.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/StoragePoolValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommonTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBaseTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommandTest.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/storage/AttachDomainValidatorTest.java 15 files changed, 572 insertions(+), 438 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/87/36287/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommand.java index 1260dfa..45de95b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommand.java @@ -6,10 +6,11 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Set; import org.ovirt.engine.core.bll.profiles.DiskProfileHelper; import org.ovirt.engine.core.bll.utils.PermissionSubject; +import org.ovirt.engine.core.bll.validator.storage.AttachDomainValidator; +import org.ovirt.engine.core.bll.validator.storage.StorageDomainValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.StorageDomainManagementParameter; @@ -162,10 +163,9 @@ } ensureStorageFormatInitialized(); - if (!isStorageFormatSupportedByStoragePool() || !isStorageFormatCompatibleWithDomain()) { - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL_HOST); - getReturnValue().getCanDoActionMessages().add( - String.format("$storageFormat %1$s", getStorageDomain().getStorageFormat())); + AttachDomainValidator attachDomainValidator = getAttachDomainValidator(); + StorageDomainValidator sdValidator = getStorageDomainValidator(); + if ( !validate(attachDomainValidator.isStorageDomainFormatCorrectForDC()) || !validate(sdValidator.isStorageFormatCompatibleWithDomain()) ) { return false; } return canAddDomain(); @@ -187,22 +187,6 @@ } } - private boolean isStorageFormatSupportedByStoragePool() { - StorageFormatType storageFormat = getStorageDomain().getStorageFormat(); - StoragePool targetStoragePool = getTargetStoragePool(); - - // No reason to check supported storage format if we don't have a pool, the storage format will be validated - // upon the future attachment of the the created domain to a pool - if (targetStoragePool == null) { - return true; - } - - Set<StorageFormatType> supportedStorageFormats = - getSupportedStorageFormatSet(targetStoragePool.getcompatibility_version()); - return supportedStorageFormats.contains(storageFormat); - - } - private StoragePool getTargetStoragePool() { StoragePool targetStoragePool = getStoragePool(); @@ -210,24 +194,6 @@ targetStoragePool = getStoragePoolDAO().get(getVds().getStoragePoolId()); } return targetStoragePool; - } - - private boolean isStorageFormatCompatibleWithDomain() { - StorageFormatType storageFormat = getStorageDomain().getStorageFormat(); - StorageType storageType = getStorageDomain().getStorageType(); - StorageDomainType storageDomainFunction = getStorageDomain().getStorageDomainType(); - - // V2 is applicable only for block data storage domains - if (storageFormat == StorageFormatType.V2) { - return storageDomainFunction.isDataDomain() && storageType.isBlockDomain(); - } - - // V3 is applicable only for data storage domains - if (storageFormat == StorageFormatType.V3) { - return storageDomainFunction.isDataDomain(); - } - - return true; } protected String getStorageArgs() { @@ -253,4 +219,12 @@ super.setActionMessageParameters(); addCanDoActionMessage(VdcBllMessages.VAR__ACTION__ADD); } + + public AttachDomainValidator getAttachDomainValidator() { + return new AttachDomainValidator(getStorageDomain(), getTargetStoragePool()); + } + + public StorageDomainValidator getStorageDomainValidator() { + return new StorageDomainValidator(getStorageDomain()); + } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStoragePoolWithStoragesCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStoragePoolWithStoragesCommand.java index a7e7855..e0732ed 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStoragePoolWithStoragesCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStoragePoolWithStoragesCommand.java @@ -7,6 +7,7 @@ import org.ovirt.engine.core.bll.LockMessagesMatchUtil; import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute; import org.ovirt.engine.core.bll.context.CommandContext; +import org.ovirt.engine.core.bll.validator.storage.AttachDomainValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.LockProperties; import org.ovirt.engine.core.common.action.LockProperties.Scope; @@ -265,7 +266,8 @@ StorageFormatType storageFormat = null; for (Guid storageDomainId : getParameters().getStorages()) { StorageDomain domain = DbFacade.getInstance().getStorageDomainDao().get(storageDomainId); - if (isStorageDomainNotNull(domain) && checkDomainCanBeAttached(domain)) { + AttachDomainValidator attachDomainValidator = new AttachDomainValidator(domain, getStoragePool()); + if (isStorageDomainNotNull(domain) && validate(attachDomainValidator.validateDomainCanBeAttachedToPool())) { if (domain.getStorageDomainType() == StorageDomainType.Data) { _hasData = true; if (storageFormat == null) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java index d12a4fe..8147a45 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java @@ -14,6 +14,7 @@ import org.ovirt.engine.core.bll.RetrieveImageDataParameters; import org.ovirt.engine.core.bll.context.CommandContext; import org.ovirt.engine.core.bll.profiles.DiskProfileHelper; +import org.ovirt.engine.core.bll.validator.storage.AttachDomainValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.action.AttachStorageDomainToPoolParameters; @@ -435,9 +436,10 @@ protected boolean canDoAction() { // We can share only ISO or Export domain , or a data domain // which is not attached. + AttachDomainValidator attachDomainValidator = new AttachDomainValidator(getStorageDomain(), getStoragePool()); boolean returnValue = checkStoragePool() - && initializeVds() && checkStorageDomain() && checkDomainCanBeAttached(getStorageDomain()); + && initializeVds() && checkStorageDomain() && validate(attachDomainValidator.validateDomainCanBeAttachedToPool()); if (returnValue && getStoragePool().getStatus() == StoragePoolStatus.Uninitialized && getStorageDomain().getStorageDomainType() != StorageDomainType.Data) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommand.java index a2c058f..34b2d70 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommand.java @@ -6,6 +6,7 @@ import org.ovirt.engine.core.bll.LockMessagesMatchUtil; import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute; import org.ovirt.engine.core.bll.context.CommandContext; +import org.ovirt.engine.core.bll.validator.storage.StorageDomainValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.DetachStorageDomainFromPoolParameters; import org.ovirt.engine.core.common.action.LockProperties; @@ -109,12 +110,13 @@ VDS vds = getVds(); boolean localFs = isLocalFs(dom); + StorageDomainValidator domainValidator = createDomainValidator(dom); - if (!checkStorageDomain() || !checkStorageDomainSharedStatusNotLocked(dom)) { + if (!checkStorageDomain() || !validate(domainValidator.checkStorageDomainSharedStatusNotLocked())) { return false; } - if (!localFs && !checkStorageDomainNotInPool()) { + if (!localFs && !validate(domainValidator.isStorageDomainNotInAnyPool())) { return false; } @@ -141,6 +143,10 @@ return true; } + protected StorageDomainValidator createDomainValidator(StorageDomain dom) { + return new StorageDomainValidator(dom); + } + private Pair<Boolean, VdcFault> connectStorage() { return getStorageHelper(getStorageDomain()).connectStorageToDomainByVdsIdDetails(getStorageDomain(), getVds().getId()); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java index 37392f9..ef85618 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java @@ -182,11 +182,6 @@ return returnValue; } - protected boolean checkStorageDomainNotInPool() { - return isStorageDomainNotInPool(getStorageDomain()); - } - - protected boolean checkMasterDomainIsUp() { boolean returnValue = true; List<StorageDomain> storageDomains = getStorageDomainDAO().getAllForStoragePool(getStoragePool().getId()); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java index 9f18511..6fdd662 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java @@ -3,34 +3,27 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Random; -import java.util.Set; import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.CommandBase; -import org.ovirt.engine.core.bll.ValidationResult; import org.ovirt.engine.core.bll.context.CommandContext; import org.ovirt.engine.core.bll.interfaces.BackendInternal; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.utils.PermissionSubject; -import org.ovirt.engine.core.bll.validator.storage.StoragePoolValidator; -import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.StoragePoolParametersBase; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.OvfEntityData; import org.ovirt.engine.core.common.businessentities.StorageDomain; -import org.ovirt.engine.core.common.businessentities.StorageDomainSharedStatus; import org.ovirt.engine.core.common.businessentities.StorageDomainStatic; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.StorageDomainType; import org.ovirt.engine.core.common.businessentities.StorageFormatType; import org.ovirt.engine.core.common.businessentities.StoragePool; import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; -import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.VDS; import org.ovirt.engine.core.common.businessentities.VDSStatus; import org.ovirt.engine.core.common.businessentities.VM; @@ -48,7 +41,6 @@ import org.ovirt.engine.core.compat.Version; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dao.DiskImageDAO; -import org.ovirt.engine.core.dao.StorageDomainDynamicDAO; import org.ovirt.engine.core.dao.StoragePoolIsoMapDAO; import org.ovirt.engine.core.dao.UnregisteredOVFDataDAO; import org.ovirt.engine.core.utils.SyncronizeNumberOfAsyncOperations; @@ -58,7 +50,6 @@ import org.ovirt.engine.core.utils.transaction.TransactionSupport; public abstract class StorageHandlingCommandBase<T extends StoragePoolParametersBase> extends CommandBase<T> { - private List<DiskImage> diskImagesForStorageDomain; protected StorageHandlingCommandBase(T parameters, CommandContext commandContext) { super(parameters, commandContext); @@ -69,7 +60,6 @@ protected StorageHandlingCommandBase(T parameters) { this(parameters, null); } - protected void init(T parameters) { setVdsId(parameters.getVdsId()); @@ -90,10 +80,6 @@ public static List<VDS> getAllRunningVdssInPool(StoragePool pool) { return DbFacade.getInstance().getVdsDao().getAllForStoragePoolAndStatus(pool.getId(), VDSStatus.Up); - } - - protected int getAmountOfVdssInPool() { - return getVdsDAO().getAllForStoragePool(getStoragePool().getId()).size(); } protected List<VDS> getAllRunningVdssInPool() { @@ -122,12 +108,6 @@ return ret; } - protected List<DiskImage> getDiskImagesForStorageDomain(Guid storageDomainId) { - if (diskImagesForStorageDomain == null) { - diskImagesForStorageDomain = getDiskImageDao().getAllForStorageDomain(storageDomainId); - } - return diskImagesForStorageDomain; - } @SuppressWarnings({ "unchecked", "rawtypes" }) protected boolean initializeVds() { @@ -311,143 +291,6 @@ return returnValue; } - protected boolean isStorageDomainTypeCorrect(StorageDomain storageDomain) { - if (!isStorageDomainOfTypeIsoOrExport(storageDomain) && storageDomain.isLocal() != getStoragePool().isLocal()) { - addCanDoActionMessage(VdcBllMessages.ERROR_CANNOT_ATTACH_STORAGE_DOMAIN_STORAGE_TYPE_NOT_MATCH); - return false; - } - return true; - } - - protected boolean isStorageDomainNotInPool(StorageDomain storageDomain) { - boolean returnValue = false; - if (storageDomain != null) { - // check if there is no pool-domain map - returnValue = getStoragePoolIsoMapDAO().getAllForStorage(storageDomain.getId()).isEmpty(); - if (!returnValue) { - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL); - } - } - return returnValue; - } - - protected boolean isStorageDomainCompatibleWithDC(StorageDomain storageDomain) { - StoragePoolValidator spv = new StoragePoolValidator(getStoragePool()); - if (storageDomain.getStorageType() == StorageType.GLUSTERFS) { - ValidationResult result = spv.isGlusterSupportedInDC(); - return validate(result); - } - - if (storageDomain.getStorageType() == StorageType.POSIXFS) { - ValidationResult result = spv.isPosixSupportedInDC(); - return validate(result); - } - - return true; - } - - protected boolean checkDomainCanBeAttached(StorageDomain storageDomain) { - if (!validateAmountOfIsoAndExportDomainsInDC(storageDomain)) { - return false; - } - if (!isStorageDomainFormatCorrectForDC(storageDomain, getStoragePool())) { - return false; - } - if (!checkStorageDomainSharedStatusNotLocked(storageDomain)) { - return false; - } - if (!(isStorageDomainOfTypeIsoOrExport(storageDomain) || isStorageDomainNotInPool(storageDomain))) { - return false; - } - if (!isStorageDomainTypeCorrect(storageDomain)) { - return false; - } - if (!isStorageDomainCompatibleWithDC(storageDomain)) { - return false; - } - if (!isStorageDomainOfTypeIsoOrExport(storageDomain ) && !isMixedTypesAllowedInDC(getStoragePool().getcompatibility_version()) - && isMixedTypeDC(storageDomain)) { - return false; - } - - return true; - } - - - // TODO: Should be removed when 3.0 compatibility will not be supported, for now we are blocking the possibility - // to mix NFS domains with block domains on 3.0 pools since block domains on 3.0 pools can be in V2 format while NFS - // domains on 3.0 can only be in V1 format - protected boolean isMixedTypesAllowedInDC(Version version) { - return FeatureSupported.mixedDomainTypesOnDataCenter(version); - } - - public boolean isMixedTypeDC(StorageDomain storageDomain) { - boolean isBlockDomain = storageDomain.getStorageType().isBlockDomain(); - - List<StorageType> storageTypesOnPool = getStoragePoolDAO().getStorageTypesInPool(getStoragePoolId()); - for (StorageType storageType : storageTypesOnPool) { - if (storageType.isBlockDomain() != isBlockDomain) { - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_MIXED_STORAGE_TYPES_NOT_ALLOWED); - return true; - } - } - return false; - } - - - private boolean isStorageDomainOfTypeIsoOrExport(StorageDomain storageDomain) { - return storageDomain.getStorageDomainType().isIsoOrImportExportDomain(); - } - - /** - * Check that we are not trying to attach more than one ISO or export - * domain to the same data center. - */ - protected boolean validateAmountOfIsoAndExportDomainsInDC(StorageDomain storageDomain) { - // Nothing to check if the storage domain is not an ISO or export: - if (!isStorageDomainOfTypeIsoOrExport(storageDomain)) { - return true; - } - - final StorageDomainType type = storageDomain.getStorageDomainType(); - - // Get the number of storage domains of the given type currently attached - // to the pool: - int count = LinqUtils.filter( - getStorageDomainDAO().getAllForStoragePool(getStoragePool().getId()), - new Predicate<StorageDomain>() { - @Override - public boolean eval(StorageDomain a) { - return a.getStorageDomainType() == type; - } - } - ).size(); - - // If the count is zero we are okay, we can add a new one: - if (count == 0) { - return true; - } - - // If we are here then we already have at least one storage type of the given type - // so when have to prepare a friendly message for the user (see #713160) and fail: - if (type == StorageDomainType.ISO) { - addCanDoActionMessage(VdcBllMessages.ERROR_CANNOT_ATTACH_MORE_THAN_ONE_ISO_DOMAIN); - } - else if (type == StorageDomainType.ImportExport) { - addCanDoActionMessage(VdcBllMessages.ERROR_CANNOT_ATTACH_MORE_THAN_ONE_EXPORT_DOMAIN); - } - return false; - } - - protected boolean checkStorageDomainSharedStatusNotLocked(StorageDomain storageDomain) { - boolean returnValue = storageDomain != null - && storageDomain.getStorageDomainSharedStatus() != StorageDomainSharedStatus.Locked; - if (!returnValue) { - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL); - } - return returnValue; - } - protected boolean isStorageDomainNotNull(StorageDomain domain) { if (domain == null) { addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST); @@ -523,40 +366,6 @@ return result; } - /** - * The following method should check if the format of the storage domain allows to it to be attached to the storage - * pool. At case of failure the false value will be return and appropriate error message will be added to - * canDoActionMessages - * @param storageDomain - * -the domain object - * @param storagePool - * - the pool object - * @return - */ - protected boolean isStorageDomainFormatCorrectForDC(StorageDomain storageDomain, StoragePool storagePool) { - if (storageDomain.getStorageDomainType().isIsoOrImportExportDomain()) { - return true; - } - Set<StorageFormatType> supportedFormatsSet = - getSupportedStorageFormatSet(storagePool.getcompatibility_version()); - if (supportedFormatsSet.contains(storageDomain.getStorageFormat())) { - return true; - } - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL); - getReturnValue().getCanDoActionMessages().add( - String.format("$storageFormat %1$s", storageDomain.getStorageFormat().toString())); - return false; - } - - protected Set<StorageFormatType> getSupportedStorageFormatSet(Version version) { - String[] supportedFormats = getSupportedStorageFormats(version).split("[,]"); - Set<StorageFormatType> supportedFormatsSet = new HashSet<StorageFormatType>(); - for (String supportedFormat : supportedFormats) { - supportedFormatsSet.add(StorageFormatType.forValue(supportedFormat)); - } - return supportedFormatsSet; - } - protected void runSynchronizeOperation(ActivateDeactivateSingleAsyncOperationFactory factory, Object... addionalParams) { List<VDS> allRunningVdsInPool = getAllRunningVdssInPool(); @@ -615,10 +424,6 @@ protected StoragePoolIsoMapDAO getStoragePoolIsoMapDAO() { return getDbFacade().getStoragePoolIsoMapDao(); - } - - protected StorageDomainDynamicDAO getStorageDomainDynamicDao() { - return getDbFacade().getStorageDomainDynamicDao(); } protected DiskImageDAO getDiskImageDao() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java index c96d497..f44d5ed 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java @@ -164,28 +164,25 @@ @Override protected boolean canDoAction() { - boolean returnValue = checkStoragePool(); - if (returnValue && !StringUtils.equals(getOldStoragePool().getName(), getStoragePool().getName()) + if (!checkStoragePool()) { + return false; + } + if (!StringUtils.equals(getOldStoragePool().getName(), getStoragePool().getName()) && !isStoragePoolUnique(getStoragePool().getName())) { - returnValue = false; - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_NAME_ALREADY_EXIST); + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_NAME_ALREADY_EXIST); } - if (returnValue - && getOldStoragePool().isLocal() != getStoragePool().isLocal() - && getStorageDomainStaticDAO().getAllForStoragePool(getStoragePool().getId()).size() > 0) { - returnValue = false; - getReturnValue() - .getCanDoActionMessages() - .add(VdcBllMessages.ERROR_CANNOT_CHANGE_STORAGE_POOL_TYPE_WITH_DOMAINS - .toString()); + + List<StorageDomainStatic> poolDomains = getStorageDomainStaticDAO().getAllForStoragePool(getStoragePool().getId()); + if ( getOldStoragePool().isLocal() != getStoragePool().isLocal() && !poolDomains.isEmpty() ) { + return failCanDoAction(VdcBllMessages.ERROR_CANNOT_CHANGE_STORAGE_POOL_TYPE_WITH_DOMAINS); } - returnValue = returnValue && checkStoragePoolNameLengthValid(); - if (returnValue - && !getOldStoragePool().getcompatibility_version().equals(getStoragePool() - .getcompatibility_version())) { + if (!checkStoragePoolNameLengthValid()) { + return false; + } + if ( !getOldStoragePool().getcompatibility_version().equals(getStoragePool() + .getcompatibility_version())) { if (!isStoragePoolVersionSupported()) { - addCanDoActionMessage(VersionSupport.getUnsupportedVersionMessage()); - returnValue = false; + return failCanDoAction(VersionSupport.getUnsupportedVersionMessage()); } // decreasing of compatibility version is allowed under conditions else if (getStoragePool().getcompatibility_version().compareTo(getOldStoragePool().getcompatibility_version()) < 0) { @@ -196,24 +193,21 @@ validator.setDataCenter(getStoragePool()); if (!NetworkUtils.isManagementNetwork(network) || !validator.canNetworkCompatabilityBeDecreased()) { - returnValue = false; - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION); + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION); } } else if (networks.size() > 1) { - returnValue = false; - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION); + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION); } } else { // Check all clusters has at least the same compatibility version. - returnValue = checkAllClustersLevel(); + if (!checkAllClustersLevel()) { + return false; + } } } StoragePoolValidator validator = createStoragePoolValidator(); - if (returnValue) { - returnValue = validate(validator.isNotLocalfsWithDefaultCluster()); - } - return returnValue; + return validate(validator.isNotLocalfsWithDefaultCluster()); } protected boolean checkAllClustersLevel() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/AttachDomainValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/AttachDomainValidator.java new file mode 100644 index 0000000..2b580db --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/AttachDomainValidator.java @@ -0,0 +1,169 @@ +package org.ovirt.engine.core.bll.validator.storage; + +import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.common.businessentities.StorageDomain; +import org.ovirt.engine.core.common.businessentities.StorageDomainType; +import org.ovirt.engine.core.common.businessentities.StoragePool; +import org.ovirt.engine.core.common.businessentities.StorageType; +import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.common.utils.VersionStorageFormatUtil; +import org.ovirt.engine.core.dal.dbbroker.DbFacade; +import org.ovirt.engine.core.dao.StorageDomainDAO; +import org.ovirt.engine.core.dao.StoragePoolDAO; +import org.ovirt.engine.core.utils.linq.LinqUtils; +import org.ovirt.engine.core.utils.linq.Predicate; + +import java.util.List; + +/** + * CanDoAction validation methods for attaching a storage domain to a DC (pool). + */ +public class AttachDomainValidator { + private final StorageDomain storageDomain; + private final StoragePool storagePool; + private final boolean isStorageDomainOfTypeIsoOrExport; + private final StorageDomainValidator domainValidator; + private final StoragePoolValidator poolValidator; + + public AttachDomainValidator(StorageDomain domain, StoragePool pool) { + storageDomain = domain; + storagePool = pool; + isStorageDomainOfTypeIsoOrExport = storageDomain.getStorageDomainType().isIsoOrImportExportDomain(); + domainValidator = new StorageDomainValidator(storageDomain); + poolValidator = new StoragePoolValidator(storagePool); + } + + protected StorageDomainDAO getStorageDomainDao() { + return DbFacade.getInstance().getStorageDomainDao(); + } + protected StoragePoolDAO getStoragePoolDao() { + return DbFacade.getInstance().getStoragePoolDao(); + } + protected StorageDomainValidator getDomainValidator() { + return domainValidator; + } + protected StoragePoolValidator getPoolValidator() { + return poolValidator; + } + + public ValidationResult isStorageDomainCompatibleWithDC() { + if (storageDomain.getStorageType() == StorageType.GLUSTERFS) { + return getPoolValidator().isGlusterSupportedInDC(); + } + + if (storageDomain.getStorageType() == StorageType.POSIXFS) { + return getPoolValidator().isPosixSupportedInDC(); + } + + return ValidationResult.VALID; + } + + /** + * Check that we are not trying to attach more than one ISO or export + * domain to the same data center. + */ + public ValidationResult validateAmountOfIsoAndExportDomainsInDC() { + // Nothing to check if the storage domain is not an ISO or export: + if (!isStorageDomainOfTypeIsoOrExport) { + return ValidationResult.VALID; + } + + final StorageDomainType type = storageDomain.getStorageDomainType(); + + // Get the number of storage domains of the given type currently attached + // to the pool: + int count = LinqUtils.filter( + getStorageDomainDao().getAllForStoragePool(storagePool.getId()), + new Predicate<StorageDomain>() { + @Override + public boolean eval(StorageDomain a) { + return a.getStorageDomainType() == type; + } + } + ).size(); + + // If the count is zero we are okay, we can add a new one: + if (count == 0) { + return ValidationResult.VALID; + } + + // If we are here then we already have at least one storage type of the given type + // so when have to prepare a friendly message for the user (see #713160) and fail: + if (type == StorageDomainType.ISO) { + return new ValidationResult(VdcBllMessages.ERROR_CANNOT_ATTACH_MORE_THAN_ONE_ISO_DOMAIN); + } else { + return new ValidationResult(VdcBllMessages.ERROR_CANNOT_ATTACH_MORE_THAN_ONE_EXPORT_DOMAIN); + } + } + + /** + * The following method should check if the format of the storage domain allows to it to be attached to the storage + * pool. At case of failure the false value will be return and appropriate error message will be added to + * canDoActionMessages + */ + public ValidationResult isStorageDomainFormatCorrectForDC() { + if (isStorageDomainOfTypeIsoOrExport) { + return ValidationResult.VALID; + } + + if (storagePool != null) { + if (VersionStorageFormatUtil.getPreferredForVersion(storagePool.getcompatibility_version(), + storageDomain.getStorageType()).compareTo(storageDomain.getStorageFormat()) < 0) { + + return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL, String.format("$storageFormat %1$s", storageDomain.getStorageFormat().toString())); + } + } + return ValidationResult.VALID; + } + + public ValidationResult isStorageDomainLocalityFitsDC() { + if (!isStorageDomainOfTypeIsoOrExport && storageDomain.isLocal() != storagePool.isLocal()) { + return new ValidationResult(VdcBllMessages.ERROR_CANNOT_ATTACH_STORAGE_DOMAIN_STORAGE_TYPE_NOT_MATCH); + } + return ValidationResult.VALID; + } + + public ValidationResult isStorageDomainTypeFitsDC() { + boolean isBlockDomain = storageDomain.getStorageType().isBlockDomain(); + + if (!poolValidator.isMixedTypesAllowedInDC()) { + List<StorageType> storageTypesOnPool = + getStoragePoolDao().getStorageTypesInPool(storagePool.getId()); + for (StorageType storageType : storageTypesOnPool) { + if (storageType.isBlockDomain() != isBlockDomain) { + return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_MIXED_STORAGE_TYPES_NOT_ALLOWED); + } + } + } + return ValidationResult.VALID; + } + + public ValidationResult validateDomainCanBeAttachedToPool() { + ValidationResult valResult; + if (!isStorageDomainOfTypeIsoOrExport) { + if (!(valResult = isStorageDomainFormatCorrectForDC()).isValid()) { + return valResult; + } + if (!(valResult = getDomainValidator().isStorageDomainNotInAnyPool()).isValid()) { + return valResult; + } + if (!(valResult = isStorageDomainLocalityFitsDC()).isValid()) { + return valResult; + } + if (!(valResult = isStorageDomainTypeFitsDC()).isValid()) { + return valResult; + } + } else { + if (!(valResult = validateAmountOfIsoAndExportDomainsInDC()).isValid()) { + return valResult; + } + } + if (!(valResult = getDomainValidator().checkStorageDomainSharedStatusNotLocked()).isValid()) { + return valResult; + } + if (!(valResult = isStorageDomainCompatibleWithDC()).isValid()) { + return valResult; + } + return ValidationResult.VALID; + } +} diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/StorageDomainValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/StorageDomainValidator.java index 4a2b691..864ff4b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/StorageDomainValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/StorageDomainValidator.java @@ -7,12 +7,18 @@ import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainDynamic; +import org.ovirt.engine.core.common.businessentities.StorageDomainSharedStatus; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; +import org.ovirt.engine.core.common.businessentities.StorageDomainType; +import org.ovirt.engine.core.common.businessentities.StorageFormatType; +import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.VolumeFormat; import org.ovirt.engine.core.common.businessentities.VolumeType; import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.dal.dbbroker.DbFacade; +import org.ovirt.engine.core.dao.StoragePoolIsoMapDAO; public class StorageDomainValidator { @@ -24,6 +30,10 @@ public StorageDomainValidator(StorageDomain domain) { storageDomain = domain; + } + + protected StoragePoolIsoMapDAO getStoragePoolIsoMapDao() { + return DbFacade.getInstance().getStoragePoolIsoMapDao(); } public ValidationResult isDomainExist() { @@ -217,6 +227,25 @@ storageName()); } + public ValidationResult checkStorageDomainSharedStatusNotLocked() { + if (storageDomain != null) { + if (storageDomain.getStorageDomainSharedStatus() == StorageDomainSharedStatus.Locked) { + return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL); + } + } + return ValidationResult.VALID; + } + + public ValidationResult isStorageDomainNotInAnyPool() { + if (storageDomain != null) { + // check if there is no pool-domain map + if (!getStoragePoolIsoMapDao().getAllForStorage(storageDomain.getId()).isEmpty()) { + return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL); + } + } + return ValidationResult.VALID; + } + /** * Validates all the storage domains by a given predicate. * @@ -237,4 +266,28 @@ private static interface SizeAssessment { public double getSizeForDisk(DiskImage diskImage); } + + public ValidationResult isStorageFormatCompatibleWithDomain() { + StorageFormatType storageFormat = storageDomain.getStorageFormat(); + StorageType storageType = storageDomain.getStorageType(); + StorageDomainType storageDomainFunction = storageDomain.getStorageDomainType(); + boolean validationSucceeded = true; + + // V2 is applicable only for block data storage domains + if (storageFormat == StorageFormatType.V2) { + if ( !(storageDomainFunction.isDataDomain() && storageType.isBlockDomain()) ) { + validationSucceeded = false; + } + } + + // V3 is applicable only for data storage domains + if (storageFormat == StorageFormatType.V3) { + if (!storageDomainFunction.isDataDomain()) { + validationSucceeded = false; + } + } + + return validationSucceeded? ValidationResult.VALID : new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL_HOST, + String.format("$storageFormat %1$s", storageDomain.getStorageFormat())); + } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/StoragePoolValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/StoragePoolValidator.java index 0d28d7fd..0d6851b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/StoragePoolValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/StoragePoolValidator.java @@ -3,6 +3,7 @@ import java.util.List; import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.businessentities.StoragePool; import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; import org.ovirt.engine.core.common.businessentities.VDSGroup; @@ -81,4 +82,10 @@ return ValidationResult.VALID; } + // TODO: Should be removed when 3.0 compatibility will not be supported, for now we are blocking the possibility + // to mix NFS domains with block domains on 3.0 pools since block domains on 3.0 pools can be in V2 format while NFS + // domains on 3.0 can only be in V1 format + public boolean isMixedTypesAllowedInDC() { + return FeatureSupported.mixedDomainTypesOnDataCenter(storagePool.getcompatibility_version()); + } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommonTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommonTest.java index 2e7d8f8..06e8290 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommonTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommonTest.java @@ -201,7 +201,7 @@ public void canDoActionFailsUnsupportedFormat() { sp.setcompatibility_version(Version.v3_0); CanDoActionTestUtils.runAndAssertCanDoActionFailure - (cmd, VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL_HOST); + (cmd, VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL); } @Test diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommandTest.java index 7697512..46025b7 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommandTest.java @@ -17,6 +17,7 @@ import org.mockito.runners.MockitoJUnitRunner; import org.ovirt.engine.core.bll.CanDoActionTestUtils; import org.ovirt.engine.core.bll.CommandAssertUtils; +import org.ovirt.engine.core.bll.validator.storage.StorageDomainValidator; import org.ovirt.engine.core.common.action.RemoveStorageDomainParameters; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; @@ -30,6 +31,7 @@ import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; 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.dao.StoragePoolDAO; import org.ovirt.engine.core.dao.StoragePoolIsoMapDAO; @@ -86,6 +88,9 @@ doReturn(vdsDAOMock).when(command).getVdsDAO(); doReturn(vds).when(vdsDAOMock).get(vdsID); + StorageDomainValidatorForTesting domainValidator = spy(new StorageDomainValidatorForTesting(storageDomain)); + doReturn(storagePoolIsoMapDAOMock).when(domainValidator).getStoragePoolIsoMapDao(); + doReturn(domainValidator).when(command).createDomainValidator(storageDomain); } @Test @@ -155,4 +160,14 @@ doReturn(ret).when(command).runVdsCommand (eq(VDSCommandType.FormatStorageDomain), any(FormatStorageDomainVDSCommandParameters.class)); } + + protected class StorageDomainValidatorForTesting extends StorageDomainValidator { + public StorageDomainValidatorForTesting(StorageDomain domain) { + super(domain); + } + + public StoragePoolIsoMapDAO getStoragePoolIsoMapDao() { + return DbFacade.getInstance().getStoragePoolIsoMapDao(); + } + } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBaseTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBaseTest.java index dacfd23..91430f8 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBaseTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBaseTest.java @@ -7,8 +7,6 @@ import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig; import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.List; import org.junit.Before; @@ -17,16 +15,10 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; -import org.ovirt.engine.core.bll.CanDoActionTestUtils; -import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.action.StoragePoolManagementParameter; import org.ovirt.engine.core.common.businessentities.StorageDomain; -import org.ovirt.engine.core.common.businessentities.StorageDomainSharedStatus; -import org.ovirt.engine.core.common.businessentities.StorageDomainType; -import org.ovirt.engine.core.common.businessentities.StorageFormatType; import org.ovirt.engine.core.common.businessentities.StoragePool; import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMap; -import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.compat.Guid; @@ -128,165 +120,6 @@ public void nameAcceptableLength() { setAcceptableNameLength(255); checkStoragePoolNameLengthFails(); - } - - - - private StorageDomain createValidStorageDomain() { - StorageDomain storageDomain = new StorageDomain(); - storageDomain.setId(Guid.newGuid()); - storageDomain.setStorageFormat(StorageFormatType.V3); - return storageDomain; - } - - @Test - public void testAttachOnValidDomain() { - StorageDomain storageDomain = createValidStorageDomain(); - assertTrue("Attaching a valid domain to attach was failed", cmd.checkDomainCanBeAttached(storageDomain)); - } - - - /** - * Mixed types are not allowed on version lower than V3.4, test that attempting to attach a domain of different type - * than what already exists in the data center will fail for versions 3.0 to 3.3 inclusive - */ - @Test - public void testMixedTypesOnAllVersions() { - for (Version version : Version.ALL) { - if (version.compareTo(Version.v3_0) >= 0) { // No reason to test unsupported versions - testAddingMixedTypes(version, FeatureSupported.mixedDomainTypesOnDataCenter(version)); - } - } - } - - private void testAddingMixedTypes(Version version, boolean addingMixedTypesShouldSucceed) { - storagePool.setcompatibility_version(version); - - // This will make the storage pool show as if he already has an NFS domain attached - when(storagePoolDAO.getStorageTypesInPool(storagePool.getId())).thenReturn(Collections.singletonList(StorageType.NFS)); - - StorageDomain domainToAttach = createValidStorageDomain(); - domainToAttach.setStorageFormat(cmd.getSupportedStorageFormatSet(version).iterator().next()); - initCommand(); - assertTrue("Attaching an NFS domain to a pool with NFS domain with no mixed type allowed failed, version: " + version, cmd.checkDomainCanBeAttached(domainToAttach)); - - domainToAttach.setStorageType(StorageType.ISCSI); - initCommand(); - if (addingMixedTypesShouldSucceed) { - assertTrue("Attaching an ISCSI domain to a pool with NFS domain with with mixed type allowed failed, version: " + version, cmd.checkDomainCanBeAttached(domainToAttach)); - } - else { - assertFalse("Attaching an ISCSI domain to a pool with NFS domain with no mixed type allowed succeeded, version: " + version, cmd.checkDomainCanBeAttached(domainToAttach)); - CanDoActionTestUtils.assertCanDoActionMessages("Attaching an ISCSI domain to a pool with NFS domain with no mixed type failed with the wrong message", cmd, - VdcBllMessages.ACTION_TYPE_FAILED_MIXED_STORAGE_TYPES_NOT_ALLOWED); - } - - } - - @Test - public void testAttachPosixCompatibility() { - StorageDomain storageDomain = createValidStorageDomain(); - - storageDomain.setStorageType(StorageType.POSIXFS); - assertTrue("Attaching a POSIX domain failed while it should have succeeded", cmd.checkDomainCanBeAttached(storageDomain)); - - storagePool.setcompatibility_version(Version.v3_0); - storageDomain.setStorageFormat(StorageFormatType.V1); - - initCommand(); - storageDomain.setStorageType(StorageType.POSIXFS); - assertFalse("Attaching a POSIX domain succeeded while it should have failed", cmd.checkDomainCanBeAttached(storageDomain)); - CanDoActionTestUtils.assertCanDoActionMessages("Attaching a POSIX domain failed with the wrong message", cmd, - VdcBllMessages.DATA_CENTER_POSIX_STORAGE_NOT_SUPPORTED_IN_CURRENT_VERSION); - - - } - - @Test - public void testAttachGlusterCompatibility() { - StorageDomain storageDomain = createValidStorageDomain(); - storageDomain.setStorageType(StorageType.GLUSTERFS); - assertTrue("Attaching a GLUSTER domain failed while it should have succeeded", cmd.checkDomainCanBeAttached(storageDomain)); - - storagePool.setcompatibility_version(Version.v3_0); - storageDomain.setStorageFormat(StorageFormatType.V1); - - initCommand(); - storageDomain.setStorageType(StorageType.GLUSTERFS); - assertFalse("Attaching a GLUSTER domain succeeded while it should have failed", cmd.checkDomainCanBeAttached(storageDomain)); - CanDoActionTestUtils.assertCanDoActionMessages("Attaching a GLUSTER domain failed failed with the wrong message", cmd, - VdcBllMessages.DATA_CENTER_GLUSTER_STORAGE_NOT_SUPPORTED_IN_CURRENT_VERSION); - } - - @Test - public void testAttachFailDomainTypeIncorrect() { - StorageDomain storageDomain = createValidStorageDomain(); - storageDomain.setStorageType(StorageType.LOCALFS); - assertFalse("Attaching domain with an incorrect type succeeded while it should have failed", cmd.checkDomainCanBeAttached(storageDomain)); - CanDoActionTestUtils.assertCanDoActionMessages("Attaching domain with an incorrect type failed with the wrong message", cmd, - VdcBllMessages.ERROR_CANNOT_ATTACH_STORAGE_DOMAIN_STORAGE_TYPE_NOT_MATCH); - } - - @Test - public void testAttachFailDomainAlreadyInPool() { - StorageDomain storageDomain = createValidStorageDomain(); - isoMap.add(new StoragePoolIsoMap()); - assertFalse("Attaching domain that is already in a pool succeeded while it should have failed", cmd.checkDomainCanBeAttached(storageDomain)); - CanDoActionTestUtils.assertCanDoActionMessages("Attaching domain that is already in a pool failed with the wrong message", cmd, - VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL); - } - - - @Test - public void testAttachFailLockDomain() { - StorageDomain storageDomain = createValidStorageDomain(); - storageDomain.setStorageDomainSharedStatus(StorageDomainSharedStatus.Locked); - assertFalse("Attaching domain in locked status succeeded while it should have failed", cmd.checkDomainCanBeAttached(storageDomain)); - CanDoActionTestUtils.assertCanDoActionMessages("Attaching domain with in locked status failed with the wrong message", cmd, - VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL); - } - - @Test - public void testAttachFailFormatType() { - StorageDomain domainToAttach = createValidStorageDomain(); - domainToAttach.setStorageFormat(StorageFormatType.V2); - assertFalse("Attaching domain with unsupported version succeeded while it should have failed", cmd.checkDomainCanBeAttached(domainToAttach)); - CanDoActionTestUtils.assertCanDoActionMessages("Attaching domain with unsupported version failed with the wrong message", cmd, - VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL); - } - - @Test - public void testCanAttachISOOrExport() { - for (StorageDomainType type : Arrays.<StorageDomainType> asList(StorageDomainType.ISO, StorageDomainType.ImportExport)) { - StorageDomain storageDomainToAttach = createValidStorageDomain(); - storageDomainToAttach.setStorageDomainType(type); - testCanAttachWithISOOrExport(storageDomainToAttach); - } - } - - /** - * Tests attaching an ISO/Export domain to a pool first to a pool without an ISO/Export domain attached (should succeed) - * then to a pool with an ISO/Export domain attached (should fail) - */ - private void testCanAttachWithISOOrExport(StorageDomain domainToAttach) { - assertTrue("Attaching domain of type " + domainToAttach.getStorageDomainType() + " failed while it should have succeed", - cmd.checkDomainCanBeAttached(domainToAttach)); - - StorageDomain existingDomain = createValidStorageDomain(); - existingDomain.setStorageDomainType(domainToAttach.getStorageDomainType()); - addDomainToPool(existingDomain); - assertFalse("Attaching domain of type " + domainToAttach.getStorageDomainType() + " succeeded while it should have failed", - cmd.checkDomainCanBeAttached(domainToAttach)); - CanDoActionTestUtils.assertCanDoActionMessages("Attaching domain of type " + domainToAttach.getStorageDomainType() - + " succeeded though another domain" + - "of the same type already exists in the pool", cmd, - domainToAttach.getStorageDomainType() == StorageDomainType.ISO ? VdcBllMessages.ERROR_CANNOT_ATTACH_MORE_THAN_ONE_ISO_DOMAIN : - VdcBllMessages.ERROR_CANNOT_ATTACH_MORE_THAN_ONE_EXPORT_DOMAIN); - - } - - private void addDomainToPool(StorageDomain storageDomain) { - attachedDomains.add(storageDomain); } private void checkStoragePoolSucceeds() { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommandTest.java index f4d8c69..154a253 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommandTest.java @@ -100,6 +100,7 @@ doReturn(vdsDao).when(cmd).getVdsDAO(); doReturn(networkDao).when(cmd).getNetworkDAO(); doReturn(validator).when(cmd).createStoragePoolValidator(); + doReturn(true).when(sdList).isEmpty(); mcr.mockConfigValue(ConfigValues.AutoRegistrationDefaultVdsGroupID, DEFAULT_VDS_GROUP_ID); mcr.mockConfigValue(ConfigValues.ManagementNetwork, "test_mgmt"); @@ -233,6 +234,7 @@ } private void domainListNotEmpty() { + when(sdList.isEmpty()).thenReturn(false); when(sdList.size()).thenReturn(1); } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/storage/AttachDomainValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/storage/AttachDomainValidatorTest.java new file mode 100644 index 0000000..cd9fb9e --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/storage/AttachDomainValidatorTest.java @@ -0,0 +1,277 @@ +package org.ovirt.engine.core.bll.validator.storage; + +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.common.FeatureSupported; +import org.ovirt.engine.core.common.businessentities.StorageDomain; +import org.ovirt.engine.core.common.businessentities.StorageDomainSharedStatus; +import org.ovirt.engine.core.common.businessentities.StorageDomainType; +import org.ovirt.engine.core.common.businessentities.StorageFormatType; +import org.ovirt.engine.core.common.businessentities.StoragePool; +import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMap; +import org.ovirt.engine.core.common.businessentities.StorageType; +import org.ovirt.engine.core.common.config.ConfigValues; +import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.compat.Version; +import org.ovirt.engine.core.dao.StorageDomainDAO; +import org.ovirt.engine.core.dao.StoragePoolDAO; +import org.ovirt.engine.core.dao.StoragePoolIsoMapDAO; +import org.ovirt.engine.core.utils.MockConfigRule; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; +import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig; + +@RunWith(MockitoJUnitRunner.class) +public class AttachDomainValidatorTest { + StorageDomain storageDomain; + StoragePool storagePool; + AttachDomainValidator validator; + StorageDomainValidator domainValidator; + StoragePoolValidator poolValidator; + + @Mock + StoragePoolDAO storagePoolDAO; + @Mock + StorageDomainDAO storageDomainDAO; + @Mock + StoragePoolIsoMapDAO storagePoolIsoMapDAO; + + @Before + public void setUp() throws Exception { + // Create the storage domain. + storageDomain = new StorageDomain(); + storageDomain.setId(Guid.newGuid()); + storageDomain.setStorageFormat(StorageFormatType.V3); + storageDomain.setStorageType(StorageType.NFS); + + // Create the storage pool. + storagePool = new StoragePool(); + storagePool.setId(Guid.newGuid()); + storagePool.setcompatibility_version(Version.v3_5); + + when(storagePoolIsoMapDAO.getAllForStorage(any(Guid.class))).thenReturn(new ArrayList<StoragePoolIsoMap>()); + spyValidator(); + } + + private void spyValidator() { + // Create the spied validators. + validator = spy(new AttachDomainValidator(storageDomain, storagePool)); + domainValidator = spy(new StorageDomainValidator(storageDomain)); + poolValidator = spy(new StoragePoolValidator(storagePool)); + + doReturn(storagePoolIsoMapDAO).when(domainValidator).getStoragePoolIsoMapDao(); + + doReturn(poolValidator).when(validator).getPoolValidator(); + doReturn(domainValidator).when(validator).getDomainValidator(); + + doReturn(storagePoolDAO).when(validator).getStoragePoolDao(); + doReturn(storageDomainDAO).when(validator).getStorageDomainDao(); + } + + @ClassRule + public static MockConfigRule mcr = new MockConfigRule( + mockConfig(ConfigValues.GlusterFsStorageEnabled, Version.v3_0.toString(), false), + mockConfig(ConfigValues.GlusterFsStorageEnabled, Version.v3_4.toString(), true), + mockConfig(ConfigValues.GlusterFsStorageEnabled, Version.v3_5.toString(), true), + mockConfig(ConfigValues.PosixStorageEnabled, Version.v3_0.toString(), false), + mockConfig(ConfigValues.PosixStorageEnabled, Version.v3_4.toString(), true), + mockConfig(ConfigValues.PosixStorageEnabled, Version.v3_5.toString(), true), + mockConfig(ConfigValues.MixedDomainTypesInDataCenter, Version.v3_0.toString(), false), + mockConfig(ConfigValues.MixedDomainTypesInDataCenter, Version.v3_1.toString(), false), + mockConfig(ConfigValues.MixedDomainTypesInDataCenter, Version.v3_2.toString(), false), + mockConfig(ConfigValues.MixedDomainTypesInDataCenter, Version.v3_3.toString(), false), + mockConfig(ConfigValues.MixedDomainTypesInDataCenter, Version.v3_4.toString(), true), + mockConfig(ConfigValues.MixedDomainTypesInDataCenter, Version.v3_5.toString(), true) + ); + + @Test + public void testAttachOnValidDomain() { + assertTrue("Attaching a valid domain to attach was failed", + validator.validateDomainCanBeAttachedToPool().isValid()); + } + + /** + * Mixed types are not allowed on version lower than V3.4, test that attempting to attach a domain of different type + * than what already exists in the data center will fail for versions 3.0 to 3.3 inclusive + */ + @Test + public void testMixedTypesOnAllVersions() { + // Use an old format so the domain will be able to attach to each DC. + storageDomain.setStorageFormat(StorageFormatType.V1); + + // Mock the pool to have a NFS type domain + when(storagePoolDAO.getStorageTypesInPool(storagePool.getId())).thenReturn(Collections.singletonList(StorageType.NFS)); + + storageDomain.setStorageType(StorageType.ISCSI); + for (Version version : Version.ALL) { + if (version.compareTo(Version.v3_0) >= 0) { // No reason to test unsupported versions + testAddingMixedTypes(version, FeatureSupported.mixedDomainTypesOnDataCenter(version)); + } + } + } + + private void testAddingMixedTypes(Version version, boolean addingMixedTypesShouldSucceed) { + storagePool.setcompatibility_version(version); + + ValidationResult attachDomainResult = validator.validateDomainCanBeAttachedToPool(); + if (addingMixedTypesShouldSucceed) { + assertTrue("Attaching an ISCSI domain to a pool with NFS domain with with mixed type allowed failed, version: " + version, attachDomainResult.isValid()); + } + else { + + assertFalse("Attaching an ISCSI domain to a pool with NFS domain with no mixed type allowed succeeded, version: " + version, attachDomainResult.isValid()); + assertFailingMessage( + "Attaching an ISCSI domain to a pool with NFS domain with no mixed type failed with the wrong message", + attachDomainResult, + VdcBllMessages.ACTION_TYPE_FAILED_MIXED_STORAGE_TYPES_NOT_ALLOWED); + } + } + + @Test + public void testPosixCompatibility() { + storageDomain.setStorageType(StorageType.POSIXFS); + assertTrue("Attaching a POSIX domain failed while it should have succeeded", + validator.validateDomainCanBeAttachedToPool().isValid()); + } + + @Test + public void testAttachPosixCompatibilityOnLowVersion() { + storagePool.setcompatibility_version(Version.v3_0); + + storageDomain.setStorageType(StorageType.POSIXFS); + storageDomain.setStorageFormat(StorageFormatType.V1); + + ValidationResult attachPosixToLowVersionResult = validator.validateDomainCanBeAttachedToPool(); + assertFalse("Attaching a POSIX domain succeeded while it should have failed", + attachPosixToLowVersionResult.isValid()); + assertFailingMessage("Attaching a POSIX domain failed with the wrong message", + attachPosixToLowVersionResult, + VdcBllMessages.DATA_CENTER_POSIX_STORAGE_NOT_SUPPORTED_IN_CURRENT_VERSION); + } + + @Test + public void testGlusterCompatibility() { + storageDomain.setStorageType(StorageType.GLUSTERFS); + assertTrue("Attaching a GLUSTER domain failed while it should have succeeded", validator.validateDomainCanBeAttachedToPool().isValid()); + } + + @Test + public void testGlusterCompatibilityOnLowVersion() { + storagePool.setcompatibility_version(Version.v3_0); + + storageDomain.setStorageFormat(StorageFormatType.V1); + storageDomain.setStorageType(StorageType.GLUSTERFS); + + ValidationResult attachGlusterToLowVersionResult = validator.validateDomainCanBeAttachedToPool(); + assertFalse("Attaching a GLUSTER domain succeeded while it should have failed", attachGlusterToLowVersionResult.isValid()); + assertFailingMessage("Attaching a GLUSTER domain failed failed with the wrong message", + attachGlusterToLowVersionResult, + VdcBllMessages.DATA_CENTER_GLUSTER_STORAGE_NOT_SUPPORTED_IN_CURRENT_VERSION); + } + + @Test + public void testAttachFailDomainTypeIncorrect() { + storageDomain.setStorageType(StorageType.LOCALFS); + ValidationResult attachIncorrectTypeResult = validator.validateDomainCanBeAttachedToPool(); + assertFalse("Attaching domain with an incorrect type succeeded while it should have failed", attachIncorrectTypeResult.isValid()); + assertFailingMessage("Attaching domain with an incorrect type failed with the wrong message", + attachIncorrectTypeResult, + VdcBllMessages.ERROR_CANNOT_ATTACH_STORAGE_DOMAIN_STORAGE_TYPE_NOT_MATCH); + } + + @Test + public void testAttachFailDomainAlreadyInPool() { + List<StoragePoolIsoMap> isoMap = new ArrayList<>(); + isoMap.add(new StoragePoolIsoMap()); + when(storagePoolIsoMapDAO.getAllForStorage(any(Guid.class))).thenReturn(isoMap); + + ValidationResult attachedDomainInsertionResult = validator.validateDomainCanBeAttachedToPool(); + assertFalse("Attaching domain that is already in a pool succeeded while it should have failed", + attachedDomainInsertionResult.isValid()); + assertFailingMessage("Attaching domain that is already in a pool failed with the wrong message", + attachedDomainInsertionResult, + VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL); + } + + @Test + public void testAttachFailLockDomain() { + storageDomain.setStorageDomainSharedStatus(StorageDomainSharedStatus.Locked); + ValidationResult shareLockedDomainInsertionResult = validator.validateDomainCanBeAttachedToPool(); + assertFalse("Attaching domain in locked status succeeded while it should have failed", + shareLockedDomainInsertionResult.isValid()); + assertFailingMessage("Attaching domain with in locked status failed with the wrong message", + shareLockedDomainInsertionResult, + VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL); + } + + @Test + public void testAttachFailFormatType() { + storageDomain.setStorageFormat(StorageFormatType.V3); + storagePool.setcompatibility_version(Version.v3_0); + + ValidationResult invalidFormatAttachingResult = validator.validateDomainCanBeAttachedToPool(); + assertFalse("Attaching domain with unsupported version succeeded while it should have failed", invalidFormatAttachingResult.isValid()); + assertFailingMessage("Attaching domain with unsupported version failed with the wrong message", + invalidFormatAttachingResult, + VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL); + } + + /** + * Tests attaching an ISO/Export domain to a pool first to a pool without an ISO/Export domain attached (should succeed) + * then to a pool with an ISO/Export domain attached (should fail) + */ + + @Test + public void testCanAttachSingleISOOrExport() { + for (StorageDomainType type : Arrays.<StorageDomainType> asList(StorageDomainType.ISO, StorageDomainType.ImportExport)) { + storageDomain.setStorageDomainType(type); + spyValidator(); + assertTrue("Attaching domain of type " + type + " failed while it should have succeed", validator.validateDomainCanBeAttachedToPool().isValid()); + } + } + + @Test + public void testCanAttachMultipleISOOrExport() { + for (StorageDomainType type : Arrays.<StorageDomainType> asList(StorageDomainType.ISO, StorageDomainType.ImportExport)) { + storageDomain.setStorageDomainType(type); + spyValidator(); + + // Make the pool to have already a domain with the same type of the domain we want to attach. + ArrayList<StorageDomain> domainList = new ArrayList<StorageDomain>(); + StorageDomain domainWithSameType = new StorageDomain(); + domainWithSameType.setStorageDomainType(type); + domainList.add(domainWithSameType); + when(storageDomainDAO.getAllForStoragePool(any(Guid.class))).thenReturn(domainList); + + ValidationResult attachMultipleISOOrExportResult = validator.validateDomainCanBeAttachedToPool(); + assertFalse("Attaching domain of type " + type + " succeeded while it should have failed", + attachMultipleISOOrExportResult.isValid()); + + assertFailingMessage("Attaching domain of type " + type + " succeeded though another domain of the same type already exists in the pool", + attachMultipleISOOrExportResult, + (type == StorageDomainType.ISO ? VdcBllMessages.ERROR_CANNOT_ATTACH_MORE_THAN_ONE_ISO_DOMAIN : + VdcBllMessages.ERROR_CANNOT_ATTACH_MORE_THAN_ONE_EXPORT_DOMAIN)); + } + } + + private void assertFailingMessage(String failMessage, ValidationResult validationResult, VdcBllMessages vdcBllMessage) { + assertTrue(failMessage, validationResult.getMessage().equals(vdcBllMessage)); + } +} -- To view, visit http://gerrit.ovirt.org/36287 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I079a51b5ddc36805a945ffc59965101418187441 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Amit Aviram <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
