From 8f2014ea556404fb99c78add1e46b80c718cf491 Mon Sep 17 00:00:00 2001 From: Aman Kumar Date: Tue, 16 Jun 2015 05:32:44 -0700 Subject: [PATCH] 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 Partial-Bug: #1329223 Change-Id: I8b3e00a9371d5a03cc8b4be24bf20eec10bef5df --- .../openvswitch/agent/ovs_neutron_agent.py | 72 +++++++++---------- .../agent/test_ovs_neutron_agent.py | 35 +++++++++ 2 files changed, 68 insertions(+), 39 deletions(-) diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py index 8e5a5c18c..0e2b7eb24 100644 --- 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 10c9f5440..76c488718 100644 --- 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): -- 2.45.2