Maor Lipchuk has uploaded a new change for review. Change subject: core: Add a validation when deactivate ISO domain. ......................................................................
core: Add a validation when deactivate ISO domain. Adding a CDA when deactivating an ISO storage domain if there are VMs with an ISO file attached to them and are not down. Signed-off-by: Maor Lipchuk <mlipc...@redhat.com> Bug-Url: https://bugzilla.redhat.com/969641 Change-Id: I47c1a8155762ecd0b04bb17676151946982bb919 --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java M frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 7 files changed, 105 insertions(+), 11 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/62/24462/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java index fc399cd..58f756d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java @@ -2,9 +2,11 @@ import java.util.Collections; import java.util.HashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; +import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.LockIdNameAttribute; import org.ovirt.engine.core.bll.LockMessagesMatchUtil; import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute; @@ -18,6 +20,9 @@ import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMapId; import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; import org.ovirt.engine.core.common.businessentities.VDS; +import org.ovirt.engine.core.common.businessentities.VMStatus; +import org.ovirt.engine.core.common.businessentities.VmDynamic; +import org.ovirt.engine.core.common.businessentities.VmStatic; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.utils.Pair; @@ -112,6 +117,9 @@ return failCanDoAction(VdcBllMessages.ERROR_CANNOT_DEACTIVATE_MASTER_WITH_LOCKED_DOMAINS); } } + if (!isRunningVmsWithIsoAttached()) { + return false; + } if (!getParameters().getIsInternal() && !getVmDAO() .getAllActiveForStorageDomain(getStorageDomain().getId()) @@ -132,6 +140,31 @@ return true; } + protected boolean isRunningVmsWithIsoAttached() { + if (!getParameters().getIsInternal() && getStorageDomain().getStorageDomainType() == StorageDomainType.ISO) { + List<String> vmNames = getVmsWithAttachedISO(); + if (!vmNames.isEmpty()) { + return failCanDoAction(VdcBllMessages.ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED, + String.format("$VmNames %1$s", StringUtils.join(vmNames, ","))); + } + } + return true; + } + + protected List<String> getVmsWithAttachedISO() { + List<VmStatic> vms = getVmStaticDAO().getAllByStoragePoolId(getStorageDomain().getStoragePoolId()); + List<String> vmNames = new LinkedList<>(); + for (VmStatic vmStatic : vms) { + VmDynamic vmDynamic = getVmDynamicDAO().get(vmStatic.getId()); + if (getVmDynamicDAO().get(vmStatic.getId()).getStatus() != VMStatus.Down + && !StringUtils.isEmpty(vmDynamic.getCurrentCd() != null ? vmDynamic.getCurrentCd() + : vmStatic.getIsoPath())) { + vmNames.add(vmStatic.getName()); + } + } + return vmNames; + } + /** * Filter out the domains with the requested status from the given domains list, excluding the domain which the * command is run for. diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java index 2160dca..374b1fe 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java @@ -1,5 +1,6 @@ package org.ovirt.engine.core.bll.storage; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Mockito.doReturn; @@ -8,7 +9,9 @@ import static org.mockito.Mockito.when; import java.util.ArrayList; +import java.util.List; +import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; @@ -17,13 +20,16 @@ import org.ovirt.engine.core.bll.context.CompensationContext; import org.ovirt.engine.core.bll.interfaces.BackendInternal; import org.ovirt.engine.core.common.action.StorageDomainPoolParametersBase; +import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; +import org.ovirt.engine.core.common.businessentities.StorageDomainType; +import org.ovirt.engine.core.common.businessentities.StoragePool; +import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMap; import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMapId; import org.ovirt.engine.core.common.businessentities.VDS; import org.ovirt.engine.core.common.businessentities.VDSStatus; -import org.ovirt.engine.core.common.businessentities.StorageDomain; -import org.ovirt.engine.core.common.businessentities.StoragePool; -import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMap; +import org.ovirt.engine.core.common.businessentities.VmStatic; +import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.common.vdscommands.VDSParametersBase; @@ -35,6 +41,8 @@ import org.ovirt.engine.core.dao.StoragePoolDAO; import org.ovirt.engine.core.dao.StoragePoolIsoMapDAO; import org.ovirt.engine.core.dao.VdsDAO; +import org.ovirt.engine.core.dao.VmDynamicDAO; +import org.ovirt.engine.core.dao.VmStaticDAO; import org.ovirt.engine.core.utils.MockEJBStrategyRule; @RunWith(MockitoJUnitRunner.class) @@ -58,24 +66,34 @@ private AsyncTaskDAO asyncTaskDAO; @Mock private VDS vds; + @Mock + private VmStaticDAO vmStaticDAO; + @Mock + private VmDynamicDAO vmDynamicDAO; + StoragePoolIsoMap map = new StoragePoolIsoMap(); + + private StorageDomain domain = null; @ClassRule public static MockEJBStrategyRule mockEjbRule = new MockEJBStrategyRule(); + StorageDomainPoolParametersBase params = new StorageDomainPoolParametersBase(Guid.newGuid(), Guid.newGuid()); + DeactivateStorageDomainCommand<StorageDomainPoolParametersBase> cmd = + spy(new DeactivateStorageDomainCommand<StorageDomainPoolParametersBase>(params)); + + @Before + public void setup() { + doReturn(dbFacade).when(cmd).getDbFacade(); + when(dbFacade.getStoragePoolDao()).thenReturn(storagePoolDAO); + when(dbFacade.getStorageDomainDao()).thenReturn(storageDomainDAO); + } + @Test public void statusSetInMap() { - StorageDomainPoolParametersBase params = new StorageDomainPoolParametersBase(Guid.newGuid(), Guid.newGuid()); - DeactivateStorageDomainCommand<StorageDomainPoolParametersBase> cmd = - spy(new DeactivateStorageDomainCommand<StorageDomainPoolParametersBase>(params)); - doReturn(mock(IStorageHelper.class)).when(cmd).getStorageHelper(any(StorageDomain.class)); - doReturn(dbFacade).when(cmd).getDbFacade(); - when(dbFacade.getStoragePoolIsoMapDao()).thenReturn(isoMapDAO); - when(dbFacade.getStoragePoolDao()).thenReturn(storagePoolDAO); when(dbFacade.getVdsDao()).thenReturn(vdsDAO); - when(dbFacade.getStorageDomainDao()).thenReturn(storageDomainDAO); when(dbFacade.getAsyncTaskDao()).thenReturn(asyncTaskDAO); when(storagePoolDAO.get(any(Guid.class))).thenReturn(new StoragePool()); when(isoMapDAO.get(any(StoragePoolIsoMapId.class))).thenReturn(map); @@ -95,4 +113,40 @@ cmd.executeCommand(); assertTrue(map.getStatus() == StorageDomainStatus.Maintenance); } + + @Test + public void testVmsWithNoIsoAttached() { + mockDomain(); + doReturn(domain).when(cmd).getStorageDomain(); + when(dbFacade.getVmStaticDao()).thenReturn(vmStaticDAO); + List<VmStatic> listVMs = new ArrayList<>(); + when(vmStaticDAO.getAllByStoragePoolId(any(Guid.class))).thenReturn(listVMs); + assertTrue(cmd.isRunningVmsWithIsoAttached()); + assertTrue(cmd.getReturnValue().getCanDoActionMessages().isEmpty()); + } + + @Test + public void testVmsWithIsoAttached() { + setup(); + mockDomain(); + doReturn(domain).when(cmd).getStorageDomain(); + when(dbFacade.getVmStaticDao()).thenReturn(vmStaticDAO); + when(dbFacade.getVmDynamicDao()).thenReturn(vmDynamicDAO); + + List<VmStatic> listVMs = new ArrayList<>(); + VmStatic vmStatic = new VmStatic(); + vmStatic.setName("TestVM"); + vmStatic.setId(Guid.newGuid()); + listVMs.add(vmStatic); + doReturn(listVMs).when(cmd).getVmsWithAttachedISO(); + assertFalse(cmd.isRunningVmsWithIsoAttached()); + assertTrue(cmd.getReturnValue() + .getCanDoActionMessages() + .contains(VdcBllMessages.ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED.toString())); + } + + private void mockDomain() { + domain = new StorageDomain(); + domain.setStorageDomainType(StorageDomainType.ISO); + } } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java index 397bff5..2d89f60 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java @@ -563,6 +563,7 @@ ERROR_CANNOT_DEACTIVATE_MASTER_WITH_NON_DATA_DOMAINS(ErrorType.CONFLICT), ERROR_CANNOT_DEACTIVATE_MASTER_WITH_LOCKED_DOMAINS(ErrorType.CONFLICT), ERROR_CANNOT_DEACTIVATE_DOMAIN_WITH_TASKS(ErrorType.CONFLICT), + ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED(ErrorType.CONFLICT), ERROR_CANNOT_DEACTIVATE_MASTER_DOMAIN_WITH_TASKS_ON_POOL(ErrorType.CONFLICT), ERROR_CANNOT_ADD_STORAGE_POOL_WITHOUT_DATA_AND_ISO_DOMAINS(ErrorType.BAD_PARAMETERS), ERROR_CANNOT_ADD_STORAGE_POOL_WITHOUT_DATA_DOMAIN(ErrorType.CONFLICT), diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index f3e1147..f3688da 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -818,6 +818,7 @@ VAR__TYPE__VM__CLUSTER=$type VM Cluster VDS_FENCE_OPERATION_FAILED=Cannot ${action} ${type}. Fence operation failed. VM_CANNOT_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO=Cannot ${action} without active ISO domain. +ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED=Cannot ${action} ${type}. The ISO Storage Domain is being used by the following VMs: ${VmNames}. MAC_ADDRESS_IS_IN_USE=MAC Address is in use. CAN_DO_ACTION_GENERAL_FAILURE=General command validation failure. ERROR_CANNOT_REMOVE_ACTIVE_STORAGE_POOL=Cannot remove an active Data Center. diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java index 0367d8e..7dbe433 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java @@ -2212,6 +2212,9 @@ @DefaultStringValue("Cannot ${action} without active ISO domain.") String VM_CANNOT_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO(); + @DefaultStringValue("Cannot ${action} ${type}. The ISO Storage Domain is being used by the following VMs: ${VmNames}.") + String ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED(); + @DefaultStringValue("MAC Address is in use.") String MAC_ADDRESS_IS_IN_USE(); diff --git a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 85b5cab..31b5648 100644 --- a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -792,6 +792,7 @@ VAR__TYPE__VM__CLUSTER=$type VM Cluster VDS_FENCE_OPERATION_FAILED=Cannot ${action} ${type}. Fence operation failed. VM_CANNOT_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO=Cannot ${action} without active ISO domain. +ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED=Cannot ${action} ${type}. The ISO Storage Domain is being used by the following VMs: ${VmNames}. MAC_ADDRESS_IS_IN_USE=Mac Address is in use. CAN_DO_ACTION_GENERAL_FAILURE=General command validation failure. ERROR_CANNOT_REMOVE_ACTIVE_STORAGE_POOL=Cannot remove an active Data Center. diff --git a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 7757a9d..dcd5759 100644 --- a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -815,6 +815,7 @@ VAR__TYPE__VM__CLUSTER=$type VM Cluster VDS_FENCE_OPERATION_FAILED=Cannot ${action} ${type}. Fence operation failed. VM_CANNOT_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO=Cannot ${action} without active ISO domain. +ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED=Cannot ${action} ${type}. The ISO Storage Domain is being used by the following VMs: ${VmNames}. MAC_ADDRESS_IS_IN_USE=Mac Address is in use. CAN_DO_ACTION_GENERAL_FAILURE=General command validation failure. ERROR_CANNOT_REMOVE_ACTIVE_STORAGE_POOL=Cannot remove an active Data Center. -- To view, visit http://gerrit.ovirt.org/24462 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I47c1a8155762ecd0b04bb17676151946982bb919 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches