Martin Mucha has uploaded a new change for review. Change subject: core: tests for HostSetupNetworksValidator#validModifiedBonds ......................................................................
core: tests for HostSetupNetworksValidator#validModifiedBonds • added tests • extracted creation of HostInterfaceValidator for mocking purposes • renamed two methods allowing to use them in different context as well. Change-Id: I75fcab4ff2e5a7e5576d54f639343116ac06c037 Signed-off-by: Martin Mucha <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidatorTest.java 2 files changed, 273 insertions(+), 15 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/96/36996/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidator.java index 821e41e..9578243 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidator.java @@ -149,7 +149,7 @@ } VdsNetworkInterface existingBond = existingInterfaces.get(bondName); - if (!validateAndAddViolation(new HostInterfaceValidator(existingBond).interfaceIsBond(), bondName)) { + if (!validateAndAddViolation(createHostInterfaceValidator(existingBond).interfaceIsBond(), bondName)) { passed = false; continue; } @@ -203,19 +203,19 @@ return candidateAttachments; } - private boolean validModifiedBonds() { + boolean validModifiedBonds() { boolean passed = true; for (Bond modifiedOrNewBond : params.getBonds()) { - String bondName = modifiedOrNewBond.getName(); - if (!validateAndAddViolation(new HostInterfaceValidator(modifiedOrNewBond).interfaceByNameExists(), null)) { + if (!validateAndAddViolation(createHostInterfaceValidator(modifiedOrNewBond).interfaceByNameExists(), null)) { passed = false; continue; } + String bondName = modifiedOrNewBond.getName(); //either it's newly create bond, thus non existing, or given name must reference existing bond. if (!validateAndAddViolation( - new HostInterfaceValidator(existingInterfaces.get(bondName)).interfaceIsBond(), bondName)) { + createHostInterfaceValidator(existingInterfaces.get(bondName)).interfaceIsBond(), bondName)) { passed = false; continue; } @@ -227,17 +227,17 @@ continue; } - passed &= validateModifiedBondSlaves(modifiedOrNewBond, bondName); + passed &= validateModifiedBondSlaves(modifiedOrNewBond); } return passed; } - private boolean validateModifiedBondSlaves(Bond modifiedOrNewBond, String bondName) { + boolean validateModifiedBondSlaves(Bond modifiedOrNewBond) { boolean passed = true; for (String slaveName : modifiedOrNewBond.getSlaves()) { VdsNetworkInterface slave = existingInterfaces.get(slaveName); - HostInterfaceValidator slaveHostInterfaceValidator = new HostInterfaceValidator(slave); + HostInterfaceValidator slaveHostInterfaceValidator = createHostInterfaceValidator(slave); if (!validateAndAddViolation(slaveHostInterfaceValidator.interfaceExists(), slaveName)) { passed = false; @@ -255,9 +255,9 @@ if (slave.isBondSlave() && /* we're creating new bond, and it's definition contains reference to slave already assigned to a different bond. */ - (!slaveBondName.equals(bondName) + !slaveBondName.equals(modifiedOrNewBond.getName()) && //…but this bond is also removed in this request, so it's ok. - || !slaveUsedByRemovedBond(slaveBondName))) { + !bondIsBeingRemoved(slaveBondName)) { addViolation(VdcBllMessages.NETWORK_INTERFACE_ALREADY_IN_BOND, slaveName); passed = false; continue; @@ -266,7 +266,7 @@ /* slave has network assigned and there isn't request for unassigning it; so this probably check, that nic is part of newly crated bond, and any previously attached network has to be unattached. */ - if (slave.getNetworkName() != null && !nicUsedByRemovedNetwork(slave)) { + if (slave.getNetworkName() != null && !networkAttachmentIsBeingRemoved(slave.getName())) { addViolation(VdcBllMessages.NETWORK_INTERFACE_ATTACHED_TO_NETWORK_CANNOT_BE_SLAVE, slave.getName()); passed = false; continue; @@ -275,9 +275,19 @@ return passed; } - private boolean nicUsedByRemovedNetwork(VdsNetworkInterface slave) { + HostInterfaceValidator createHostInterfaceValidator(VdsNetworkInterface vdsNetworkInterface) { + return new HostInterfaceValidator(vdsNetworkInterface); + } + + /** + * + * @param interfaceName name of interface + * @return true if there's request to remove network attachment (disassociate network with interface) related to + * given interface name. + */ + private boolean networkAttachmentIsBeingRemoved(String interfaceName) { for (NetworkAttachment attachment : params.getRemovedNetworkAttachments()) { - if (Objects.equals(slave.getName(), attachment.getNicName())) { + if (Objects.equals(interfaceName, attachment.getNicName())) { return true; } } @@ -285,9 +295,13 @@ return false; } - private boolean slaveUsedByRemovedBond(String slaveBondName) { + /** + * @param bondName name of bonded interface. + * @return true if there's request to remove bond of given name. + */ + private boolean bondIsBeingRemoved(String bondName) { for (Bond removedBond : params.getRemovedBonds()) { - if (slaveBondName.equals(removedBond.getName())) { + if (bondName.equals(removedBond.getName())) { return true; } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidatorTest.java index 59c7111..4fbbef7 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidatorTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksValidatorTest.java @@ -21,7 +21,9 @@ import org.junit.ClassRule; import org.junit.Test; import org.mockito.ArgumentCaptor; +import org.ovirt.engine.core.bll.ValidationResult; import org.ovirt.engine.core.bll.network.VmInterfaceManager; +import org.ovirt.engine.core.bll.validator.HostInterfaceValidator; import org.ovirt.engine.core.common.action.HostSetupNetworksParameters; import org.ovirt.engine.core.common.businessentities.BusinessEntityMap; import org.ovirt.engine.core.common.businessentities.Entities; @@ -403,6 +405,248 @@ assertThat(validator.containsViolation(VdcBllMessages.NETWORKS_ALREADY_ATTACHED_TO_IFACES), is(true)); } + @Test + public void testValidModifiedBonds_failsWhenBondIsNullOrUnnamed() throws Exception { + doTestValidModifiedBonds(null, + new ValidationResult(VdcBllMessages.HOST_NETWORK_INTERFACE_NOT_EXIST), + ValidationResult.VALID, + VdcBllMessages.HOST_NETWORK_INTERFACE_NOT_EXIST, + false, + false); + } + + @Test + public void testValidModifiedBonds_failsWhenReferencingExistingNonBondInterface() throws Exception { + Bond bond = new Bond(); + doTestValidModifiedBonds(bond, + ValidationResult.VALID, + new ValidationResult(VdcBllMessages.NETWORK_INTERFACE_IS_NOT_BOND), + VdcBllMessages.NETWORK_INTERFACE_IS_NOT_BOND, + false, + false); + } + + @Test + public void testValidModifiedBonds_failsWhenInsufficientNumberOfSlaves() throws Exception { + Bond bond = new Bond(); + doTestValidModifiedBonds(bond, + ValidationResult.VALID, + ValidationResult.VALID, + VdcBllMessages.NETWORK_BONDS_INVALID_SLAVE_COUNT, + false, + false); + } + + @Test + public void testValidModifiedBonds_failsWhenSlavesValidationFails() throws Exception { + Bond bond = new Bond(); + bond.setSlaves(Arrays.asList("slaveA", "slaveB")); + doTestValidModifiedBonds(bond, ValidationResult.VALID, ValidationResult.VALID, null, false, false); + } + + @Test + public void testValidModifiedBonds_whenAllOk() throws Exception { + Bond bond = new Bond(); + bond.setSlaves(Arrays.asList("slaveA", "slaveB")); + doTestValidModifiedBonds(bond, ValidationResult.VALID, ValidationResult.VALID, null, true, true); + } + + private void doTestValidModifiedBonds(Bond bond, + ValidationResult interfaceByNameExistValidationResult, + ValidationResult interfaceIsBondValidationResult, + VdcBllMessages expectedViolation, + boolean expectedValidationResult, + boolean slavesValidationOk) { + HostSetupNetworksParameters params = new HostSetupNetworksParameters(host.getId()); + params.setBonds(Collections.singletonList(bond)); + + HostSetupNetworksValidator validator = spy(new HostSetupNetworksValidator(host, params, null, null, null)); + + HostInterfaceValidator hostInterfaceValidatorMock = mock(HostInterfaceValidator.class); + when(hostInterfaceValidatorMock.interfaceByNameExists()).thenReturn(interfaceByNameExistValidationResult); + when(hostInterfaceValidatorMock.interfaceIsBond()).thenReturn(interfaceIsBondValidationResult); + + doReturn(hostInterfaceValidatorMock).when(validator).createHostInterfaceValidator(any(VdsNetworkInterface.class)); + doReturn(slavesValidationOk).when(validator).validateModifiedBondSlaves(any(Bond.class)); + + assertThat(validator.validModifiedBonds(), is(expectedValidationResult)); + if (expectedViolation != null) { + assertThat(validator.containsViolation(expectedViolation), is(true)); + } + + verify(hostInterfaceValidatorMock).interfaceByNameExists(); + + //assert only if previous call was successful, otherwise this method was not called. + if (interfaceByNameExistValidationResult.isValid()) { + verify(hostInterfaceValidatorMock).interfaceIsBond(); + } + } + + @Test + public void testValidateModifiedBondSlaves_whenSlaveInterfaceDoesNotExist() throws Exception { + Bond bond = new Bond(); + bond.setSlaves(Arrays.asList("slaveA", "slaveB")); + doTestValidateModifiedBondSlaves(bond, + null, + new ValidationResult(VdcBllMessages.HOST_NETWORK_INTERFACE_NOT_EXIST), + ValidationResult.VALID, + false, + VdcBllMessages.HOST_NETWORK_INTERFACE_NOT_EXIST); + } + + @Test + public void testValidateModifiedBondSlaves_whenSlaveIsNotValid() throws Exception { + Bond bond = new Bond(); + bond.setSlaves(Arrays.asList("slaveA", "slaveB")); + doTestValidateModifiedBondSlaves(bond, + null, + ValidationResult.VALID, + new ValidationResult(VdcBllMessages.NETWORK_INTERFACE_BOND_OR_VLAN_CANNOT_BE_SLAVE), + false, + VdcBllMessages.NETWORK_INTERFACE_BOND_OR_VLAN_CANNOT_BE_SLAVE); + } + + @Test + public void testValidateModifiedBondSlaves_whenSlaveAlreadySlavesForDifferentBond() throws Exception { + Bond bond = new Bond(); + bond.setName("bondName"); + + Bond differentBond = new Bond(); + differentBond.setName("differentBond"); + + VdsNetworkInterface slaveA = createBondSlave(bond, "slaveA"); + VdsNetworkInterface slaveB = createBondSlave(differentBond, "slaveB"); + + bond.setSlaves(Arrays.asList(slaveA.getName(), slaveB.getName())); + doTestValidateModifiedBondSlaves(bond, + Arrays.asList(bond, differentBond, slaveA, slaveB), + ValidationResult.VALID, + ValidationResult.VALID, + false, + VdcBllMessages.NETWORK_INTERFACE_ALREADY_IN_BOND); + } + + @Test + public void testValidateModifiedBondSlaves_whenSlaveAlreadySlavesForDifferentBondWhichGetsRemoved() throws Exception { + Bond bond = new Bond(); + bond.setName("bondName"); + + Bond differentBond = new Bond(); + differentBond.setName("differentBond"); + + VdsNetworkInterface slaveA = createBondSlave(bond, "slaveA"); + VdsNetworkInterface slaveB = createBondSlave(differentBond, "slaveB"); + + HostSetupNetworksParameters params = new HostSetupNetworksParameters(host.getId()); + params.setRemovedBonds(Collections.singletonList(differentBond)); + + bond.setSlaves(Arrays.asList(slaveA.getName(), slaveB.getName())); + doTestValidateModifiedBondSlaves(bond, + params, + Arrays.asList(bond, differentBond, slaveA, slaveB), + ValidationResult.VALID, + ValidationResult.VALID, + true, + null); + } + + @Test + public void testValidateModifiedBondSlaves_whenSlaveHasNetworkAssignedWhichIsNotRemovedAsAPartOfRequest() throws Exception { + Bond bond = new Bond(); + bond.setName("bondName"); + + VdsNetworkInterface slaveA = createBondSlave(bond, "slaveA"); + slaveA.setNetworkName("assignedNetwork"); + VdsNetworkInterface slaveB = createBondSlave(bond, "slaveB"); + + bond.setSlaves(Arrays.asList(slaveA.getName(), slaveB.getName())); + doTestValidateModifiedBondSlaves(bond, + Arrays.asList(bond, slaveA, slaveB), + ValidationResult.VALID, + ValidationResult.VALID, + false, + VdcBllMessages.NETWORK_INTERFACE_ATTACHED_TO_NETWORK_CANNOT_BE_SLAVE); + } + + @Test + public void testValidateModifiedBondSlaves_whenSlaveHasNetworkAssignedWhichIsRemovedAsAPartOfRequest() throws Exception { + Bond bond = new Bond(); + bond.setName("bondName"); + + Network networkBeingRemoved = new Network(); + networkBeingRemoved.setName("assignedNetwork"); + + VdsNetworkInterface slaveA = createBondSlave(bond, "slaveA"); + slaveA.setNetworkName(networkBeingRemoved.getName()); + VdsNetworkInterface slaveB = createBondSlave(bond, "slaveB"); + + NetworkAttachment removedNetworkAttachment = new NetworkAttachment(); + removedNetworkAttachment.setNicName(slaveA.getName()); + + HostSetupNetworksParameters params = new HostSetupNetworksParameters(host.getId()); + params.setRemovedNetworkAttachments(Collections.singletonList(removedNetworkAttachment)); + + bond.setSlaves(Arrays.asList(slaveA.getName(), slaveB.getName())); + doTestValidateModifiedBondSlaves(bond, + params, + Arrays.asList(bond, slaveA, slaveB), + ValidationResult.VALID, + ValidationResult.VALID, + true, + null); + } + + private void doTestValidateModifiedBondSlaves(Bond bond, + List<VdsNetworkInterface> existingInterfaces, + ValidationResult interfaceExistValidationResult, + ValidationResult interfaceIsValidSlaveValidationResult, + boolean expectedValidationResult, + VdcBllMessages expectedViolation) { + + doTestValidateModifiedBondSlaves(bond, + new HostSetupNetworksParameters(host.getId()), + existingInterfaces, + interfaceExistValidationResult, + interfaceIsValidSlaveValidationResult, + expectedValidationResult, + expectedViolation); + + } + + private void doTestValidateModifiedBondSlaves(Bond bond, + HostSetupNetworksParameters params, + List<VdsNetworkInterface> existingInterfaces, + ValidationResult interfaceExistValidationResult, + ValidationResult interfaceIsValidSlaveValidationResult, + boolean expectedValidationResult, + VdcBllMessages expectedViolation) { + + HostSetupNetworksValidator validator = spy(new HostSetupNetworksValidator(host, + params, + existingInterfaces, + null, + null)); + + HostInterfaceValidator hostInterfaceValidatorMock = mock(HostInterfaceValidator.class); + when(hostInterfaceValidatorMock.interfaceExists()).thenReturn(interfaceExistValidationResult); + when(hostInterfaceValidatorMock.interfaceIsValidSlave()).thenReturn(interfaceIsValidSlaveValidationResult); + + doReturn(hostInterfaceValidatorMock).when(validator).createHostInterfaceValidator(any(VdsNetworkInterface.class)); + + assertThat(validator.validateModifiedBondSlaves(bond), is(expectedValidationResult)); + if (expectedViolation != null) { + assertThat(validator.containsViolation(expectedViolation), is(true)); + } + } + + private VdsNetworkInterface createBondSlave(Bond bond, String slaveName) { + VdsNetworkInterface slaveA = new VdsNetworkInterface(); + slaveA.setName(slaveName); + slaveA.setBondName(bond.getName()); + slaveA.setBonded(true); + return slaveA; + } + private HostSetupNetworksValidator createHostSetupNetworksValidator() { return createHostSetupNetworksValidator(Collections.<Network> emptyList()); } -- To view, visit http://gerrit.ovirt.org/36996 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I75fcab4ff2e5a7e5576d54f639343116ac06c037 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
