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

Reply via email to