Lior Vernia has uploaded a new change for review. Change subject: engine: Modified network static IP validation ......................................................................
engine: Modified network static IP validation It used to not allow changing the management network's IP address whenever the hostname was given as an IP address, but actually problems only arise if the old management network's IP address was equal to the hostname. However, this validation should be applied to ANY network, not just the management network. Also modified the tests accordingly, as well as fixed some logic that was supposed to give existing NICs different IP addresses but I don't think did anything. Change-Id: I85f873fe4e87d16ec60da5bd8f9d6c90ee1e0030 Bug-Url: https://bugzilla.redhat.com/989360 Signed-off-by: Lior Vernia <lver...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.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, 87 insertions(+), 30 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/45/20345/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java index a366792..9908608 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java @@ -19,7 +19,6 @@ import org.ovirt.engine.core.common.businessentities.network.NetworkBootProtocol; import org.ovirt.engine.core.common.businessentities.network.VdsNetworkInterface; import org.ovirt.engine.core.common.errors.VdcBllMessages; -import org.ovirt.engine.core.common.utils.ValidationUtils; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.utils.NetworkUtils; @@ -304,9 +303,8 @@ addViolation(VdcBllMessages.NETWORKS_NOT_IN_SYNC, networkName); } } else if (networkWasModified(iface)) { - if (NetworkUtils.isManagementNetwork(iface.getNetworkName()) - && !managementNetworkModifiedCorrectly(iface)) { - addViolation(VdcBllMessages.ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_ADDRESS_CANNOT_BE_CHANGED, networkName); + if (networkIpAddressWasSameAsHostnameAndChanged(iface)) { + addViolation(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED, networkName); } modifiedNetworks.add(network); } @@ -324,23 +322,27 @@ } /** - * Checks that the management network is configured correctly: + * Checks if a network is configured incorrectly: * <ul> * <li>If the host was added to the system using its IP address as the computer name for the certification creation, * it is forbidden to modify the IP address without reinstalling the host.</li> * </ul> * * @param iface - * The network interface which carries the management network - * @return <code>true</code> if the management network was reconfigured properly + * The network interface which carries the network + * @return <code>true</code> if the network was reconfigured improperly */ - private boolean managementNetworkModifiedCorrectly(VdsNetworkInterface iface) { - if (iface.getBootProtocol() == NetworkBootProtocol.STATIC_IP - && vds.getHostName().matches(ValidationUtils.IP_PATTERN)) { - return StringUtils.equals(vds.getHostName(), iface.getAddress()); + private boolean networkIpAddressWasSameAsHostnameAndChanged(VdsNetworkInterface iface) { + if (iface.getBootProtocol() == NetworkBootProtocol.STATIC_IP) { + VdsNetworkInterface existingIface = getExistingIfaceByNetwork(iface.getNetworkName()); + if (existingIface != null) { + String oldAddress = existingIface.getAddress(); + String hostName = vds.getHostName(); + return StringUtils.equals(oldAddress, hostName) && !StringUtils.equals(oldAddress, iface.getAddress()); + } } - return true; + return false; } private NetworkType determineNetworkType(Integer vlanId, boolean vmNetwork) { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.java index 6c9295f..c7ca256 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.java @@ -179,6 +179,7 @@ VDS vds = mock(VDS.class); when(vds.getId()).thenReturn(Guid.Empty); when(vds.getVdsGroupCompatibilityVersion()).thenReturn(Version.v3_3); + when(vds.getHostName()).thenReturn(RandomUtils.instance().nextString(10)); SetupNetworksHelper helper = createHelper(createParametersForNics(nic), vds); @@ -214,7 +215,10 @@ mockExistingIfaces(nic); nic.setSubnet(RandomUtils.instance().nextString(10)); - SetupNetworksHelper helper = createHelper(createParametersForNics(nic)); + VDS vds = mock(VDS.class); + when(vds.getHostName()).thenReturn(RandomUtils.instance().nextString(10)); + + SetupNetworksHelper helper = createHelper(createParametersForNics(nic), vds); validateAndAssertNetworkModified(helper, net); } @@ -229,9 +233,33 @@ mockExistingIfaces(nic); nic.setAddress(RandomUtils.instance().nextString(10)); - SetupNetworksHelper helper = createHelper(createParametersForNics(nic)); + VDS vds = mock(VDS.class); + when(vds.getHostName()).thenReturn(RandomUtils.instance().nextString(10)); + + SetupNetworksHelper helper = createHelper(createParametersForNics(nic), vds); validateAndAssertNetworkModified(helper, net); + } + + @Test + public void ipChangedWhenEqualToHostname() { + String hostName = "1.1.1.1"; + + Network net = createNetwork("net"); + mockExistingNetworks(net); + VdsNetworkInterface nic = createNicSyncedWithNetwork("nic0", net); + nic.setBootProtocol(NetworkBootProtocol.STATIC_IP); + nic.setAddress(hostName); + mockExistingIfaces(nic); + nic.setAddress(RandomUtils.instance().nextString(10)); + + VDS vds = mock(VDS.class); + when(vds.getHostName()).thenReturn(hostName); + + SetupNetworksHelper helper = createHelper(createParametersForNics(nic), vds); + + validateAndExpectViolation(helper, + VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED); } @Test @@ -270,8 +298,7 @@ validateAndAssertNetworkModified(helper, net); } - @Test - public void managementNetworkChangedIncorrectly() { + public void managementNetworkChangedCorrectlyWithIpHostname() { Network net = createNetwork(MANAGEMENT_NETWORK_NAME); mockExistingNetworks(net); VdsNetworkInterface nic = createNicSyncedWithNetwork("nic0", net); @@ -285,8 +312,28 @@ when(vds.getHostName()).thenReturn("1.1.1.1"); SetupNetworksHelper helper = createHelper(createParametersForNics(nic), vds); + validateAndAssertNetworkModified(helper, net); + } + + @Test + public void managementNetworkChangedIncorrectly() { + String hostName = "1.1.1.1"; + + Network net = createNetwork(MANAGEMENT_NETWORK_NAME); + mockExistingNetworks(net); + VdsNetworkInterface nic = createNicSyncedWithNetwork("nic0", net); + nic.setBootProtocol(NetworkBootProtocol.STATIC_IP); + nic.setAddress(hostName); + mockExistingIfaces(nic); + nic.setAddress(RandomUtils.instance().nextString(10)); + + VDS vds = mock(VDS.class); + when(vds.getId()).thenReturn(Guid.Empty); + when(vds.getHostName()).thenReturn(hostName); + SetupNetworksHelper helper = createHelper(createParametersForNics(nic), vds); + validateAndExpectViolation(helper, - VdcBllMessages.ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_ADDRESS_CANNOT_BE_CHANGED); + VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED); } /* --- Tests for external networks --- */ @@ -1312,13 +1359,17 @@ * @param vlanId * @param networkName * @param bridged + * @param address * @return A network interface. */ private VdsNetworkInterface createVdsInterface(Guid id, String name, Boolean bonded, String bondName, - Integer vlanId, String networkName, boolean bridged) { + Integer vlanId, + String networkName, + boolean bridged, + String address) { VdsNetworkInterface iface = new VdsNetworkInterface(); iface.setId(id); iface.setName(name); @@ -1327,6 +1378,7 @@ iface.setVlanId(vlanId); iface.setNetworkName(networkName); iface.setBridged(bridged); + iface.setAddress(address); return iface; } @@ -1338,7 +1390,7 @@ * @return {@link VdsNetworkInterface} representing a regular NIC with the given parameters. */ private VdsNetworkInterface createNic(String nicName, String networkName) { - return createVdsInterface(Guid.newGuid(), nicName, false, null, null, networkName, true); + return createVdsInterface(Guid.newGuid(), nicName, false, null, null, networkName, true, null); } /** @@ -1355,7 +1407,8 @@ null, network.getVlanId(), network.getName(), - network.isVmNetwork()); + network.isVmNetwork(), + network.getAddr()); return nic; } @@ -1381,7 +1434,7 @@ * @return Bond with the given parameters. */ private VdsNetworkInterface createBond(String name, String networkName) { - return createVdsInterface(Guid.newGuid(), name, true, null, null, networkName, true); + return createVdsInterface(Guid.newGuid(), name, true, null, null, networkName, true, null); } /** @@ -1400,7 +1453,8 @@ null, vlanId, networkName, - true); + true, + null); } /** @@ -1413,7 +1467,7 @@ * @return NIC from given NIC which is either enslaved or freed. */ private VdsNetworkInterface enslaveOrReleaseNIC(VdsNetworkInterface iface, String bondName) { - return createVdsInterface(iface.getId(), iface.getName(), false, bondName, null, null, true); + return createVdsInterface(iface.getId(), iface.getName(), false, bondName, null, null, true, null); } /** @@ -1529,7 +1583,8 @@ nics[i].getBondName(), nics[i].getVlanId(), nics[i].getNetworkName(), - nics[i].isBridged())); + nics[i].isBridged(), + nics[i].getAddress())); } when(interfaceDAO.getAllInterfacesForVds(any(Guid.class))).thenReturn(existingIfaces); } 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 ec700a2..5f828fd 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 @@ -461,7 +461,7 @@ ACTION_TYPE_FAILED_MIGRATION_NETWORK_IS_NOT_SUPPORTED(ErrorType.NOT_SUPPORTED), ACTION_TYPE_FAILED_PROVIDER_DOESNT_EXIST(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_REQUIRED(ErrorType.BAD_PARAMETERS), - ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_ADDRESS_CANNOT_BE_CHANGED(ErrorType.BAD_PARAMETERS), + ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_EXTERNAL_NETWORK_ALREADY_EXISTS(ErrorType.CONFLICT), ACTION_TYPE_FAILED_EXTERNAL_NETWORK_DETAILS_CANNOT_BE_EDITED(ErrorType.NOT_SUPPORTED), ACTION_TYPE_FAILED_EXTERNAL_NETWORK_MUST_BE_VM_NETWORK(ErrorType.BAD_PARAMETERS), 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 136179a..378b9f6 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -538,7 +538,7 @@ ACTION_TYPE_FAILED_EXTERNAL_NETWORK_CANNOT_BE_REWIRED=Cannot ${action} ${type}. External network cannot be changed while the virtual machine is running. ACTION_TYPE_FAILED_EXTERNAL_NETWORK_CANNOT_HAVE_MTU=Cannot ${action} ${type}. External network cannot have MTU set. ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_REQUIRED=Cannot ${action} ${type}. The management network '${NetworkName}' must be required, please change the network to be required and try again. -ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_ADDRESS_CANNOT_BE_CHANGED=Cannot ${action} ${type}. The management network address cannot be modified without reinstalling the host, since this address was used to create the host's certification. +ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED=Cannot ${action} ${type}. The address of the network '${ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED_LIST}' cannot be modified without reinstalling the host, since this address was used to create the host's certification. CANNOT_PREIEW_CURRENT_IMAGE=The currently used VM Snapshot Image cannot be used in Preview command. CONFIG_UNKNOWN_KEY=Illegal configuration entry.\n\ -Please check configuration entry name. 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 1cb765d..4cbcb4c 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 @@ -1462,8 +1462,8 @@ @DefaultStringValue("Cannot ${action} ${type}. The management network '${NetworkName}' must be required, please change the network to be required and try again.") String ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_REQUIRED(); - @DefaultStringValue("Cannot ${action} ${type}. The management network address cannot be modified without reinstalling the host, since this address was used to create the host's certification.") - String ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_ADDRESS_CANNOT_BE_CHANGED(); + @DefaultStringValue("Cannot ${action} ${type}. The address of the network '${ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED_LIST}' cannot be modified without reinstalling the host, since this address was used to create the host's certification.") + String ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED(); @DefaultStringValue("The currently used VM Snapshot Image cannot be used in Preview command.") String CANNOT_PREIEW_CURRENT_IMAGE(); 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 ed7926a..dbcaea5 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 @@ -523,7 +523,7 @@ ACTION_TYPE_FAILED_EXTERNAL_NETWORK_CANNOT_BE_REWIRED=Cannot ${action} ${type}. External network cannot be changed while the virtual machine is running. ACTION_TYPE_FAILED_EXTERNAL_NETWORK_CANNOT_HAVE_MTU=Cannot ${action} ${type}. External network cannot have MTU set. ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_REQUIRED=Cannot ${action} ${type}. The management network '${NetworkName}' must be required, please change the network to be required and try again. -ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_ADDRESS_CANNOT_BE_CHANGED=Cannot ${action} ${type}. The management network address cannot be modified without reinstalling the host, since this address was used to create the host's certification. +ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED=Cannot ${action} ${type}. The address of the network '${ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED_LIST}' cannot be modified without reinstalling the host, since this address was used to create the host's certification. CANNOT_PREIEW_CURRENT_IMAGE=The currently used VM Snapshot Image cannot be used in Preview command. CONFIG_UNKNOWN_KEY=Illegal configuration entry.\n\ -Please check configuration entry name. 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 4f5cb03..58d91b0 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 @@ -542,7 +542,7 @@ ACTION_TYPE_FAILED_EXTERNAL_NETWORK_CANNOT_BE_REWIRED=Cannot ${action} ${type}. External network cannot be changed while the virtual machine is running. ACTION_TYPE_FAILED_EXTERNAL_NETWORK_CANNOT_HAVE_MTU=Cannot ${action} ${type}. External network cannot have MTU set. ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_REQUIRED=Cannot ${action} ${type}. The management network '${NetworkName}' must be required, please change the network to be required and try again. -ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_ADDRESS_CANNOT_BE_CHANGED=Cannot ${action} ${type}. The management network address cannot be modified without reinstalling the host, since this address was used to create the host's certification. +ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED=Cannot ${action} ${type}. The address of the network '${ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED_LIST}' cannot be modified without reinstalling the host, since this address was used to create the host's certification. CANNOT_PREIEW_CURRENT_IMAGE=The currently used VM Snapshot Image cannot be used in Preview command. CONFIG_UNKNOWN_KEY=Illegal configuration entry.\n\ -Please check configuration entry name. -- To view, visit http://gerrit.ovirt.org/20345 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I85f873fe4e87d16ec60da5bd8f9d6c90ee1e0030 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.3 Gerrit-Owner: Lior Vernia <lver...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
