Mike Kolesnik has uploaded a new change for review. Change subject: engine: Added check that provider networks unused ......................................................................
engine: Added check that provider networks unused Added a limitation that provider cannot be removed if it has a network that is being used by a VM/template, in order not to incapacitate that VM/template (since without a provider, the network is unusable). Change-Id: I9e0c0c5c1cb52109b71cac29b95b25f34977a11c Signed-off-by: Mike Kolesnik <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/ProviderValidator.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/RemoveProviderCommand.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/provider/RemoveProviderValidatorTest.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/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 8 files changed, 165 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/70/13570/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/ProviderValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/ProviderValidator.java index 2eeae68..a51f041 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/ProviderValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/ProviderValidator.java @@ -8,7 +8,7 @@ public class ProviderValidator { - private Provider provider; + protected Provider provider; public ProviderValidator(Provider provider) { this.provider = provider; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/RemoveProviderCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/RemoveProviderCommand.java index 989b8f0..b746e15 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/RemoveProviderCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/RemoveProviderCommand.java @@ -1,18 +1,25 @@ package org.ovirt.engine.core.bll.provider; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import org.ovirt.engine.core.bll.CommandBase; +import org.ovirt.engine.core.bll.ValidationResult; import org.ovirt.engine.core.bll.utils.PermissionSubject; +import org.ovirt.engine.core.bll.validator.NetworkValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.ProviderParameters; import org.ovirt.engine.core.common.businessentities.ActionGroup; import org.ovirt.engine.core.common.businessentities.Provider; +import org.ovirt.engine.core.common.businessentities.network.Network; import org.ovirt.engine.core.common.validation.group.RemoveEntity; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.VdcBllMessages; +import org.ovirt.engine.core.dal.dbbroker.DbFacade; +import org.ovirt.engine.core.dao.network.NetworkDao; +import org.ovirt.engine.core.utils.ReplacementUtils; @SuppressWarnings("serial") public class RemoveProviderCommand<P extends ProviderParameters> extends CommandBase<P> { @@ -42,8 +49,8 @@ @Override protected boolean canDoAction() { - ProviderValidator validator = new ProviderValidator(getDeletedProvider()); - return validate(validator.providerIsSet()); + RemoveProviderValidator validator = new RemoveProviderValidator(getDeletedProvider()); + return validate(validator.providerIsSet()) && validate(validator.providerNetworksNotUsed()); } @Override @@ -75,4 +82,36 @@ public AuditLogType getAuditLogTypeValue() { return getSucceeded() ? AuditLogType.PROVIDER_REMOVED : AuditLogType.PROVIDER_REMOVAL_FAILED; } + + protected static class RemoveProviderValidator extends ProviderValidator { + + public RemoveProviderValidator(Provider provider) { + super(provider); + } + + public ValidationResult providerNetworksNotUsed() { + List<Network> networksInUse = new ArrayList<Network>(); + List<Network> networks = getNetworkDao().getAllForProvider(provider.getId()); + + for (Network network : networks) { + NetworkValidator networkValidator = getValidator(network); + if (!networkValidator.networkNotUsedByVms().isValid() + || !networkValidator.networkNotUsedByTemplates().isValid()) { + networksInUse.add(network); + } + } + + return networksInUse.isEmpty() ? ValidationResult.VALID + : new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_PROVIDER_NETWORKS_USED, + ReplacementUtils.replaceWithNameable("NETWORK_NAMES", networksInUse)); + } + + protected NetworkDao getNetworkDao() { + return DbFacade.getInstance().getNetworkDao(); + } + + protected NetworkValidator getValidator(Network network) { + return new NetworkValidator(network); + } + } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/provider/RemoveProviderValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/provider/RemoveProviderValidatorTest.java new file mode 100644 index 0000000..08a679d --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/provider/RemoveProviderValidatorTest.java @@ -0,0 +1,116 @@ +package org.ovirt.engine.core.bll.provider; + +import static org.junit.Assert.assertThat; +import static org.junit.matchers.JUnitMatchers.both; +import static org.junit.matchers.JUnitMatchers.containsString; +import static org.junit.matchers.JUnitMatchers.hasItem; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; +import static org.ovirt.engine.core.bll.validator.ValidationResultMatchers.failsWith; +import static org.ovirt.engine.core.bll.validator.ValidationResultMatchers.isValid; +import static org.ovirt.engine.core.bll.validator.ValidationResultMatchers.replacements; + +import java.util.ArrayList; +import java.util.List; + +import org.hamcrest.Matcher; +import org.junit.Before; +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.bll.provider.RemoveProviderCommand.RemoveProviderValidator; +import org.ovirt.engine.core.bll.validator.NetworkValidator; +import org.ovirt.engine.core.common.businessentities.Provider; +import org.ovirt.engine.core.common.businessentities.network.Network; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dal.VdcBllMessages; +import org.ovirt.engine.core.dao.network.NetworkDao; + +@RunWith(MockitoJUnitRunner.class) +public class RemoveProviderValidatorTest { + + @Mock + private Provider provider; + + private List<Network> networks = new ArrayList<Network>(); + + private RemoveProviderValidator validator; + + @Mock + private NetworkDao networkDao; + + /* --- Set up for tests --- */ + + @Before + public void setUp() throws Exception { + validator = spy(new RemoveProviderValidator(provider)); + + doReturn(networkDao).when(validator).getNetworkDao(); + when(networkDao.getAllForProvider(any(Guid.class))).thenReturn(networks); + } + + @Test + public void networksNotUsedWhenNoNetworks() throws Exception { + assertThat(validator.providerNetworksNotUsed(), isValid()); + } + + private Network mockNetwork() { + Network net = mock(Network.class); + when(net.getName()).thenReturn("net"); + networks.add(net); + return net; + } + + private void networksUsedTest(Network net, + boolean vmsNotUsingNetwork, + boolean templatesNotUsingNetwork, + Matcher<ValidationResult> matcher) { + + NetworkValidator networkValidator = mock(NetworkValidator.class); + when(validator.getValidator(net)).thenReturn(networkValidator); + + when(networkValidator.networkNotUsedByVms()).thenReturn(createValidationResult(vmsNotUsingNetwork)); + when(networkValidator.networkNotUsedByTemplates()).thenReturn(createValidationResult(templatesNotUsingNetwork)); + + assertThat(validator.providerNetworksNotUsed(), + matcher); + } + + private ValidationResult createValidationResult(boolean valid) { + return valid ? ValidationResult.VALID : new ValidationResult(VdcBllMessages.Unassigned); + } + + @Test + public void networksNotUsedByVmsNorTemplates() throws Exception { + Network net = mockNetwork(); + + networksUsedTest(net, true, true, isValid()); + } + + @Test + public void networksUsedByAVm() throws Exception { + Network net = mockNetwork(); + + networksUsedTest(net, + false, + true, + both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_PROVIDER_NETWORKS_USED)) + .and(replacements(hasItem(containsString(net.getName()))))); + } + + @Test + public void networksUsedByATemplate() throws Exception { + Network net = mockNetwork(); + + networksUsedTest(net, + false, + true, + both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_PROVIDER_NETWORKS_USED)) + .and(replacements(hasItem(containsString(net.getName()))))); + } +} diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java index cb76460..eb4c8a6 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java @@ -437,6 +437,7 @@ NETWORK_MTU_OVERRIDE_NOT_SUPPORTED, ACTION_TYPE_FAILED_MIGRATION_NETWORK_IS_NOT_SUPPORTED, ACTION_TYPE_FAILED_PROVIDER_DOESNT_EXIST, + ACTION_TYPE_FAILED_PROVIDER_NETWORKS_USED, ACTION_TYPE_FAILED_EXTERNAL_NETWORK_ALREADY_EXISTS, ACTION_TYPE_FAILED_EXTERNAL_NETWORK_MUST_BE_VM_NETWORK, ACTION_TYPE_FAILED_EXTERNAL_NETWORK_DETAILS_CANNOT_BE_EDITED, 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 c50ac60..1b78dd9 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -461,6 +461,7 @@ NETWORK_MTU_OVERRIDE_NOT_SUPPORTED=Cannot ${action} ${type}. Overriding MTU is not supported for this Data Center's compatibility version. ACTION_TYPE_FAILED_MIGRATION_NETWORK_IS_NOT_SUPPORTED=Cannot ${action} ${type}. Migration network is not supported for this cluster version. ACTION_TYPE_FAILED_PROVIDER_DOESNT_EXIST=Cannot ${action} ${type}. The provider doesn't exist in the system. +ACTION_TYPE_FAILED_PROVIDER_NETWORKS_USED=Cannot ${action} ${type}. Several external networks (${NETWORK_NAMES_COUNTER}) are being used by virtual machines and/or templates:\n${NETWORK_NAMES}\n - Please resolve the external networks usage first and try again. ACTION_TYPE_FAILED_EXTERNAL_NETWORK_ALREADY_EXISTS=Cannot ${action} ${type}. The external network already exists as '${NetworkName}' in the data center. ACTION_TYPE_FAILED_EXTERNAL_NETWORK_MUST_BE_VM_NETWORK=Cannot ${action} ${type}. An external network cannot be a non-VM network. ACTION_TYPE_FAILED_EXTERNAL_NETWORK_DETAILS_CANNOT_BE_EDITED=Cannot ${action} ${type}. External network details (except name and description) cannot be changed. 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 25ad4d0..77dbe7b 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 @@ -1240,6 +1240,9 @@ @DefaultStringValue("Cannot ${action} ${type}. The provider doesn't exist in the system.") String ACTION_TYPE_FAILED_PROVIDER_DOESNT_EXIST(); + @DefaultStringValue("Cannot ${action} ${type}. Several external networks (${NETWORK_NAMES_COUNTER}) are being used by virtual machines and/or templates:\n${NETWORK_NAMES}\n - Please resolve the external networks usage first and try again.") + String ACTION_TYPE_FAILED_PROVIDER_NETWORKS_USED(); + @DefaultStringValue("Cannot ${action} ${type}. The external network already exists as '${NetworkName}' in the data center.") String ACTION_TYPE_FAILED_EXTERNAL_NETWORK_ALREADY_EXISTS(); 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 ec3c9fc..abfe798 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 @@ -458,6 +458,7 @@ NETWORK_MTU_OVERRIDE_NOT_SUPPORTED=Cannot ${action} ${type}. Overriding MTU is not supported for this Data Center's compatibility version. ACTION_TYPE_FAILED_MIGRATION_NETWORK_IS_NOT_SUPPORTED=Cannot ${action} ${type}. Migration network is not supported for this cluster version. ACTION_TYPE_FAILED_PROVIDER_DOESNT_EXIST=Cannot ${action} ${type}. The provider doesn't exist in the system. +ACTION_TYPE_FAILED_PROVIDER_NETWORKS_USED=Cannot ${action} ${type}. Several external networks (${NETWORK_NAMES_COUNTER}) are being used by virtual machines and/or templates:\n${NETWORK_NAMES}\n - Please resolve the external networks usage first and try again. ACTION_TYPE_FAILED_EXTERNAL_NETWORK_ALREADY_EXISTS=Cannot ${action} ${type}. The external network already exists as '${NetworkName}' in the data center. ACTION_TYPE_FAILED_EXTERNAL_NETWORK_MUST_BE_VM_NETWORK=Cannot ${action} ${type}. An external network cannot be a non-VM network. ACTION_TYPE_FAILED_EXTERNAL_NETWORK_DETAILS_CANNOT_BE_EDITED=Cannot ${action} ${type}. External network details (except name and description) cannot be changed. 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 8da1b25..7632528 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 @@ -455,6 +455,7 @@ NETWORK_MTU_OVERRIDE_NOT_SUPPORTED=Cannot ${action} ${type}. Overriding MTU is not supported for this Data Center's compatibility version. ACTION_TYPE_FAILED_MIGRATION_NETWORK_IS_NOT_SUPPORTED=Cannot ${action} ${type}. Migration network is not supported for this cluster version. ACTION_TYPE_FAILED_PROVIDER_DOESNT_EXIST=Cannot ${action} ${type}. The provider doesn't exist in the system. +ACTION_TYPE_FAILED_PROVIDER_NETWORKS_USED=Cannot ${action} ${type}. Several external networks (${NETWORK_NAMES_COUNTER}) are being used by virtual machines and/or templates:\n${NETWORK_NAMES}\n - Please resolve the external networks usage first and try again. ACTION_TYPE_FAILED_EXTERNAL_NETWORK_ALREADY_EXISTS=Cannot ${action} ${type}. The external network already exists as '${NetworkName}' in the data center. ACTION_TYPE_FAILED_EXTERNAL_NETWORK_MUST_BE_VM_NETWORK=Cannot ${action} ${type}. An external network cannot be a non-VM network. ACTION_TYPE_FAILED_EXTERNAL_NETWORK_DETAILS_CANNOT_BE_EDITED=Cannot ${action} ${type}. External network details (except name and description) cannot be changed. -- To view, visit http://gerrit.ovirt.org/13570 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9e0c0c5c1cb52109b71cac29b95b25f34977a11c Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Mike Kolesnik <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
