Lior Vernia has uploaded a new change for review. Change subject: webadmin: Allow custom properties on NIC without network ......................................................................
webadmin: Allow custom properties on NIC without network Previously a validation failed the Setup Networks command when a NIC had no network but had custom properties configured, which blocked removal of networks with custom properties. Now canDoAction() doesn't block this, but the custom properties are ignored (similarly to boot protocol) in case no network is configured on the NIC. Similar changes have been made to the QoS settings, which would cause the same problems. Change-Id: Ibded6b7b7ffe9be960d522bd63cbf9db08b18d6e Bug-Url: https://bugzilla.redhat.com/1119019 Signed-off-by: Lior Vernia <[email protected]> --- 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 frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java 4 files changed, 42 insertions(+), 29 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/08/30108/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 cdd73ce..187fa66 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 @@ -158,8 +158,7 @@ Set<String> existingLabels = NetworkUtils.isLabeled(existingNic) ? existingNic.getLabels() : Collections.<String> emptySet(); if (!CollectionUtils.isEqualCollection(newLabels, existingLabels) - || nic.isQosOverridden() != existingNic.isQosOverridden() - || customPropertiesChanged(nic, existingNic)) { + || (!StringUtils.isEmpty(nic.getNetworkName()) && qosOrCustomPropertiesChanged(nic, existingNic))) { existingNic.setLabels(newLabels); existingNic.setQosOverridden(nic.isQosOverridden()); existingNic.setCustomProperties(nic.getCustomProperties()); @@ -167,6 +166,10 @@ } } } + } + + private boolean qosOrCustomPropertiesChanged(VdsNetworkInterface nic, VdsNetworkInterface existingNic) { + return nic.isQosOverridden() != existingNic.isQosOverridden() || customPropertiesChanged(nic, existingNic); } /** @@ -270,27 +273,21 @@ Map<String, String> validPropertiesNonVm = new HashMap<String, String>(validProperties); validPropertiesNonVm.remove("bridge_opts"); for (VdsNetworkInterface iface : params.getInterfaces()) { - if (iface.hasCustomProperties()) { - String networkName = iface.getNetworkName(); - if (StringUtils.isEmpty(networkName)) { - addViolation(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_NO_NETWORK, - iface.getName()); - } else { - if (!networkCustomPropertiesSupported) { - addViolation(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_NOT_SUPPORTED, - networkName); - } + String networkName = iface.getNetworkName(); + if (iface.hasCustomProperties() && !StringUtils.isEmpty(networkName)) { + if (!networkCustomPropertiesSupported) { + addViolation(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_NOT_SUPPORTED, networkName); + } - Network network = existingClusterNetworks.get(networkName); - List<ValidationError> errors = - util.validateProperties(network == null || network.isVmNetwork() ? validProperties - : validPropertiesNonVm, iface.getCustomProperties()); - if (!errors.isEmpty()) { - addViolation(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_BAD_INPUT, networkName); - List<String> messages = new ArrayList<>(); - util.handleCustomPropertiesError(errors, messages); - log.error(StringUtils.join(translateErrorMessages(messages), ',')); - } + Network network = existingClusterNetworks.get(networkName); + List<ValidationError> errors = + util.validateProperties(network == null || network.isVmNetwork() ? validProperties + : validPropertiesNonVm, iface.getCustomProperties()); + if (!errors.isEmpty()) { + addViolation(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_BAD_INPUT, networkName); + List<String> messages = new ArrayList<>(); + util.handleCustomPropertiesError(errors, messages); + log.error(StringUtils.join(translateErrorMessages(messages), ',')); } } } 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 9b2a1c8..df85bf8 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 @@ -431,9 +431,20 @@ SetupNetworksHelper helper = createHelper(createParametersForNics(iface), Version.v3_5); - validateAndExpectViolation(helper, - VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_NO_NETWORK, - iface.getName()); + validateAndAssertNoChanges(helper); + } + + @Test + public void customPropertiesNetworkRemoved() { + mockExistingNetworks(); + VdsNetworkInterface iface = createNic("eth0", MANAGEMENT_NETWORK_NAME); + iface.setCustomProperties(createCustomProperties()); + mockExistingIfaces(iface); + iface.setNetworkName(null); + + SetupNetworksHelper helper = createHelper(createParametersForNics(iface), Version.v3_5); + + validateAndAssertNetworkRemoved(helper, MANAGEMENT_NETWORK_NAME); } @Test @@ -1521,6 +1532,15 @@ assertNoInterfacesModified(helper); } + private void validateAndAssertNetworkRemoved(SetupNetworksHelper helper, String networkName) { + validateAndExpectNoViolations(helper); + assertNoBondsModified(helper); + assertNoNetworksModified(helper); + assertNetworkRemoved(helper, networkName); + assertNoBondsRemoved(helper); + assertNoInterfacesModified(helper); + } + private void validateAndAssertQosOverridden(SetupNetworksHelper helper, VdsNetworkInterface iface) { validateAndExpectNoViolations(helper); assertNoBondsModified(helper); 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 2597294..7df33c5 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 @@ -727,7 +727,6 @@ ACTION_TYPE_FAILED_CUSTOM_PROPERTIES_NOT_SUPPORTED_IN_VERSION(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_INVALID_DEVICE_TYPE_FOR_CUSTOM_PROPERTIES(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_NOT_SUPPORTED(ErrorType.NOT_SUPPORTED), - ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_NO_NETWORK(ErrorType.CONFLICT), ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_BAD_INPUT(ErrorType.BAD_PARAMETERS), NETWORK_ILEGAL_NETWORK_NAME(ErrorType.BAD_PARAMETERS), 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 ca4fd21..6376ea6 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 @@ -595,9 +595,6 @@ @DefaultStringValue("Cannot ${action} ${type}. Network custom properties are not supported in the cluster's compatibility version, but they were supplied for the following network(s): ${ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_NOT_SUPPORTED_LIST}.") String ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_NOT_SUPPORTED(); - @DefaultStringValue("Cannot ${action} ${type}. Network custom properties can only be configured on an interface to which a network is attached, but were supplied for the following interface(s): ${ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_NO_NETWORK_LIST}.") - String ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_NO_NETWORK(); - @DefaultStringValue("Cannot ${action} ${type}. Some network custom properties contained errors (bad syntax, non-existing keys or invalid values), please take a closer look at the following network(s): ${ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_BAD_INPUT_LIST}. Refer to the engine log for further details.") String ACTION_TYPE_FAILED_NETWORK_CUSTOM_PROPERTIES_BAD_INPUT(); -- To view, visit http://gerrit.ovirt.org/30108 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibded6b7b7ffe9be960d522bd63cbf9db08b18d6e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
