Refactor scan_ports() and update_ancillary_ports() in OVS Neutron Agent

Used a helper method which contains the common code from scan_ports()
and update_ancillary_ports(). And also renamed the name of
update_ancillary_ports() method to scan_ancillary_ports() to have parity
between normal ports and ancillary ports.

Added unit tests for scan_ancillary_ports.

Co-Authored-By: Romil Gupta <romilg@hp.com>
Partial-Bug: #1329223

Change-Id: I8b3e00a9371d5a03cc8b4be24bf20eec10bef5df

diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
index 8e5a5c18cedb6ecd3272008aa4d5156b2d176c2d..0e2b7eb24aedaf3d7bcbe1c77e7d092351a4f332 100644 (file)
--- a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
+++ b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
@@ -1050,10 +1050,22 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
                 port_moves.append(name)
         return port_moves
 
+    def _get_port_info(self, registered_ports, cur_ports):
+        port_info = {'current': cur_ports}
+        # FIXME(salv-orlando): It's not really necessary to return early
+        # if nothing has changed.
+        if cur_ports == registered_ports:
+            # No added or removed ports to set, just return here
+            return port_info
+        port_info['added'] = cur_ports - registered_ports
+        # Remove all the known ports not found on the integration bridge
+        port_info['removed'] = registered_ports - cur_ports
+        return port_info
+
     def scan_ports(self, registered_ports, updated_ports=None):
         cur_ports = self.int_br.get_vif_port_set()
         self.int_br_device_count = len(cur_ports)
-        port_info = {'current': cur_ports}
+        port_info = self._get_port_info(registered_ports, cur_ports)
         if updated_ports is None:
             updated_ports = set()
         updated_ports.update(self.check_changed_vlans(registered_ports))
@@ -1065,18 +1077,14 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
             updated_ports &= cur_ports
             if updated_ports:
                 port_info['updated'] = updated_ports
-
-        # FIXME(salv-orlando): It's not really necessary to return early
-        # if nothing has changed.
-        if cur_ports == registered_ports:
-            # No added or removed ports to set, just return here
-            return port_info
-
-        port_info['added'] = cur_ports - registered_ports
-        # Remove all the known ports not found on the integration bridge
-        port_info['removed'] = registered_ports - cur_ports
         return port_info
 
+    def scan_ancillary_ports(self, registered_ports):
+        cur_ports = set()
+        for bridge in self.ancillary_brs:
+            cur_ports |= bridge.get_vif_port_set()
+        return self._get_port_info(registered_ports, cur_ports)
+
     def check_changed_vlans(self, registered_ports):
         """Return ports which have lost their vlan tag.
 
@@ -1101,19 +1109,6 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
                     changed_ports.add(port)
         return changed_ports
 
-    def update_ancillary_ports(self, registered_ports):
-        ports = set()
-        for bridge in self.ancillary_brs:
-            ports |= bridge.get_vif_port_set()
-
-        if ports == registered_ports:
-            return
-        added = ports - registered_ports
-        removed = registered_ports - ports
-        return {'current': ports,
-                'added': added,
-                'removed': removed}
-
     def treat_vif_port(self, vif_port, port_id, network_id, network_type,
                        physical_network, segmentation_id, admin_state_up,
                        fixed_ips, device_owner, ovs_restarted):
@@ -1559,7 +1554,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
                     ports = port_info['current']
                     # Treat ancillary devices if they exist
                     if self.ancillary_brs:
-                        port_info = self.update_ancillary_ports(
+                        port_info = self.scan_ancillary_ports(
                             ancillary_ports)
                         LOG.debug("Agent rpc_loop - iteration:%(iter_num)d - "
                                   "ancillary port info retrieved. "
@@ -1567,20 +1562,19 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
                                   {'iter_num': self.iter_num,
                                    'elapsed': time.time() - start})
 
-                        if port_info:
-                            rc = self.process_ancillary_network_ports(
-                                port_info)
-                            LOG.debug("Agent rpc_loop - iteration: "
-                                      "%(iter_num)d - ancillary ports "
-                                      "processed. Elapsed:%(elapsed).3f",
-                                      {'iter_num': self.iter_num,
-                                       'elapsed': time.time() - start})
-                            ancillary_ports = port_info['current']
-                            port_stats['ancillary']['added'] = (
-                                len(port_info.get('added', [])))
-                            port_stats['ancillary']['removed'] = (
-                                len(port_info.get('removed', [])))
-                            sync = sync | rc
+                        rc = self.process_ancillary_network_ports(
+                            port_info)
+                        LOG.debug("Agent rpc_loop - iteration: "
+                                  "%(iter_num)d - ancillary ports "
+                                  "processed. Elapsed:%(elapsed).3f",
+                                  {'iter_num': self.iter_num,
+                                   'elapsed': time.time() - start})
+                        ancillary_ports = port_info['current']
+                        port_stats['ancillary']['added'] = (
+                            len(port_info.get('added', [])))
+                        port_stats['ancillary']['removed'] = (
+                            len(port_info.get('removed', [])))
+                        sync = sync | rc
 
                     polling_manager.polling_completed()
                     # Keep this flag in the last line of "try" block,

diff --git a/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py
index 10c9f54409009bde921178e8af2909c259c1e1bf..76c4887180624abeb343a1ecf3c9cd98f87b7805 100644 (file)
--- a/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py
+++ b/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py
@@ -1211,6 +1211,41 @@ class AncillaryBridgesTest(object):
         bridges = ['br-int', 'br-ex1', 'br-ex2']
         self._test_ancillary_bridges(bridges, ['br-ex1', 'br-ex2'])
 
+    def mock_scan_ancillary_ports(self, vif_port_set=None,
+                                  registered_ports=None):
+        bridges = ['br-int', 'br-ex']
+        ancillary = ['br-ex']
+
+        with mock.patch.object(self.mod_agent.OVSNeutronAgent,
+                               'setup_integration_br'), \
+                mock.patch.object(self.mod_agent.OVSNeutronAgent,
+                                  '_restore_local_vlan_map'), \
+                mock.patch('neutron.agent.common.ovs_lib.BaseOVS.get_bridges',
+                           return_value=bridges), \
+                mock.patch('neutron.agent.common.ovs_lib.BaseOVS.'
+                           'get_bridge_external_bridge_id',
+                           side_effect=ancillary), \
+                mock.patch('neutron.agent.common.ovs_lib.OVSBridge.'
+                           'get_vif_port_set',
+                           return_value=vif_port_set):
+            self.agent = self.mod_agent.OVSNeutronAgent(self._bridge_classes(),
+                                                        **self.kwargs)
+            return self.agent.scan_ancillary_ports(registered_ports)
+
+    def test_scan_ancillary_ports_returns_cur_only_for_unchanged_ports(self):
+        vif_port_set = set([1, 2])
+        registered_ports = set([1, 2])
+        expected = dict(current=vif_port_set)
+        actual = self.mock_scan_ancillary_ports(vif_port_set, registered_ports)
+        self.assertEqual(expected, actual)
+
+    def test_scan_ancillary_ports_returns_port_changes(self):
+        vif_port_set = set([1, 3])
+        registered_ports = set([1, 2])
+        expected = dict(current=vif_port_set, added=set([3]), removed=set([2]))
+        actual = self.mock_scan_ancillary_ports(vif_port_set, registered_ports)
+        self.assertEqual(expected, actual)
+
 
 class AncillaryBridgesTestOFCtl(AncillaryBridgesTest,
                                 ovs_test_base.OVSOFCtlTestBase):