Lior Vernia has uploaded a new change for review. Change subject: engine: Block sync of network with QoS when not supported ......................................................................
engine: Block sync of network with QoS when not supported When a QoS entity is attached to network, the network is supposed to appear as out-of-sync on a host, no matter the cluster compatibility version of the host. However, when the host's cluster doesn't support Host Network QoS, it shouldn't be possible to then synchronize the network (which whould also apply QoS to it). Change-Id: Ie79c6463676b82f1743fb162fc6c660dd88756f8 Bug-Url: https://bugzilla.redhat.com/1054320 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 2 files changed, 54 insertions(+), 21 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/04/23404/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 26754da..d644bed 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 @@ -52,9 +52,12 @@ private Map<String, List<NetworkType>> ifacesWithExclusiveNetwork = new HashMap<String, List<NetworkType>>(); + private final boolean hostNetworkQosSupported; + public SetupNetworksHelper(SetupNetworksParameters parameters, VDS vds) { params = parameters; this.vds = vds; + hostNetworkQosSupported = FeatureSupported.hostNetworkQos(vds.getVdsGroupCompatibilityVersion()); } /** @@ -197,10 +200,9 @@ * with it are valid. */ private void validateNetworkQos() { - boolean featureSupported = FeatureSupported.hostNetworkQos(vds.getVdsGroupCompatibilityVersion()); for (VdsNetworkInterface iface : params.getInterfaces()) { if (iface.isQosOverridden()) { - if (!featureSupported) { + if (!hostNetworkQosSupported) { addViolation(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NETWORK_QOS_NOT_SUPPORTED, iface.getNetworkName()); } @@ -375,6 +377,9 @@ && !existingIface.getNetworkImplementationDetails().isInSync()) { if (networkShouldBeSynced(networkName)) { modifiedNetworks.add(network); + if (network.getQosId() != null && !hostNetworkQosSupported) { + addViolation(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NETWORK_QOS_NOT_SUPPORTED, networkName); + } } else if (networkWasModified(iface)) { addViolation(VdcBllMessages.NETWORKS_NOT_IN_SYNC, networkName); } 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 def2ce3..b9ca651 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 @@ -56,7 +56,7 @@ mockConfig(ConfigValues.MultipleGatewaysSupported, Version.v3_2.toString(), false), mockConfig(ConfigValues.HostNetworkQosSupported, Version.v3_2.toString(), false), mockConfig(ConfigValues.HostNetworkQosSupported, Version.v3_3.toString(), false), - mockConfig(ConfigValues.HostNetworkQosSupported, new Version(3, 4).toString(), true)); + mockConfig(ConfigValues.HostNetworkQosSupported, Version.v3_4.toString(), true)); @Mock private NetworkDao networkDAO; @@ -354,8 +354,7 @@ mockExistingIfaces(iface); iface.setQosOverridden(true); - VDS host = mock(VDS.class); - SetupNetworksHelper helper = createHelper(createParametersForNics(iface), host); + SetupNetworksHelper helper = createHelper(createParametersForNics(iface)); validateAndExpectViolation(helper, VdcBllMessages.ACTION_TYPE_FAILED_HOST_NETWORK_QOS_NOT_SUPPORTED, @@ -370,9 +369,7 @@ mockExistingIfaces(iface); iface.setQosOverridden(true); - VDS host = mock(VDS.class); - SetupNetworksHelper helper = createHelper(createParametersForNics(iface), host); - when(host.getVdsGroupCompatibilityVersion()).thenReturn(new Version(3, 4)); + SetupNetworksHelper helper = createHelper(createParametersForNics(iface), Version.v3_4); validateAndAssertQosOverridden(helper, iface); } @@ -385,15 +382,9 @@ iface.setQosOverridden(true); mockExistingIfaces(iface); - NetworkQoS qos = new NetworkQoS(); - qos.setInboundAverage(30); - qos.setInboundPeak(30); - qos.setInboundBurst(30); - iface.setQos(qos); + iface.setQos(createQos()); - VDS host = mock(VDS.class); - SetupNetworksHelper helper = createHelper(createParametersForNics(iface), host); - when(host.getVdsGroupCompatibilityVersion()).thenReturn(new Version(3, 4)); + SetupNetworksHelper helper = createHelper(createParametersForNics(iface), Version.v3_4); validateAndAssertNetworkModified(helper, network); } @@ -507,6 +498,24 @@ } @Test + public void syncNetworkQosNotSupported() { + Network network = createNetwork(MANAGEMENT_NETWORK_NAME); + mockExistingNetworks(network); + VdsNetworkInterface iface = createNicSyncedWithNetwork("eth0", network); + mockExistingIfaces(iface); + + Guid qosId = Guid.newGuid(); + when(qosDao.get(qosId)).thenReturn(createQos()); + network.setQosId(qosId); + + SetupNetworksHelper helper = createHelper(createParametersForSync(iface)); + + validateAndExpectViolation(helper, + VdcBllMessages.ACTION_TYPE_FAILED_HOST_NETWORK_QOS_NOT_SUPPORTED, + MANAGEMENT_NETWORK_NAME); + } + + @Test public void networkToSyncMovedToAnotherNic() { Network net = createNetwork("net"); VdsNetworkInterface nic1 = createNicSyncedWithNetwork("nic0", net); @@ -589,9 +598,7 @@ mockExistingIfaces(iface); iface.setQosOverridden(true); - VDS host = mock(VDS.class); - SetupNetworksHelper helper = createHelper(createParametersForSync(iface), host); - when(host.getVdsGroupCompatibilityVersion()).thenReturn(new Version(3, 4)); + SetupNetworksHelper helper = createHelper(createParametersForSync(iface), Version.v3_4); validateAndExpectNoViolations(helper); assertNoBondsRemoved(helper); @@ -1753,6 +1760,19 @@ return params; } + /** + * Create QoS with some non-empty values. + * + * @return the QoS entity. + */ + private NetworkQoS createQos() { + NetworkQoS qos = new NetworkQoS(); + qos.setInboundAverage(30); + qos.setInboundPeak(30); + qos.setInboundBurst(30); + return qos; + } + private void mockExistingNetworks(Network... networks) { when(networkDAO.getAllForCluster(any(Guid.class))).thenReturn(Arrays.asList(networks)); } @@ -1783,13 +1803,21 @@ } private SetupNetworksHelper createHelper(SetupNetworksParameters params) { + return createHelper(params, Version.v3_3); + } + + private SetupNetworksHelper createHelper(SetupNetworksParameters params, Version compatibilityVersion) { VDS vds = mock(VDS.class); when(vds.getId()).thenReturn(Guid.Empty); - return createHelper(params, vds); + return createHelper(params, vds, compatibilityVersion); } private SetupNetworksHelper createHelper(SetupNetworksParameters params, VDS vds) { - when(vds.getVdsGroupCompatibilityVersion()).thenReturn(Version.v3_3); + return createHelper(params, vds, Version.v3_3); + } + + private SetupNetworksHelper createHelper(SetupNetworksParameters params, VDS vds, Version compatibilityVersion) { + when(vds.getVdsGroupCompatibilityVersion()).thenReturn(compatibilityVersion); SetupNetworksHelper helper = spy(new SetupNetworksHelper(params, vds)); -- To view, visit http://gerrit.ovirt.org/23404 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie79c6463676b82f1743fb162fc6c660dd88756f8 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Lior Vernia <lver...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches