From d88ea97228ec8bb38f071c3969fdbb3abf6ab575 Mon Sep 17 00:00:00 2001 From: Gary Kotton Date: Thu, 20 Jun 2013 11:04:59 +0000 Subject: [PATCH] Ensure that L3 managed port have status ACTIVE Some plugins may set the port status as DOWN when it is created. This was not updated when the port was actually up. This patch ensures that gateway and router ports that are created will be set as ACTIVE when they are actually up and running. Ports that are attached to additional bridges in the OVS will only update the port status as ACTIVE. There will be no tags set on the specific ports as this will break the existing functionality. Fixes bug 1192883 Change-Id: I9993f56963ac704f0f345102d2e8b2d99dd9ad9e --- neutron/agent/linux/ovs_lib.py | 10 ++ .../openvswitch/agent/ovs_neutron_agent.py | 97 +++++++++++++++++++ .../openvswitch/test_ovs_neutron_agent.py | 57 +++++++++++ .../tests/unit/openvswitch/test_ovs_tunnel.py | 5 + 4 files changed, 169 insertions(+) diff --git a/neutron/agent/linux/ovs_lib.py b/neutron/agent/linux/ovs_lib.py index 9da39b014..9dbadb666 100644 --- a/neutron/agent/linux/ovs_lib.py +++ b/neutron/agent/linux/ovs_lib.py @@ -352,3 +352,13 @@ def get_installed_ovs_klm_version(): return ver[0] except Exception: LOG.exception(_("Unable to retrieve OVS kernel module version.")) + + +def get_bridge_external_bridge_id(root_helper, bridge): + args = ["ovs-vsctl", "--timeout=2", "br-get-external-id", + bridge, "bridge-id"] + try: + return utils.execute(args, root_helper=root_helper).strip() + except Exception: + LOG.exception(_("Bridge %s not found."), bridge) + return None diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py index 4c2d31da3..86d68cbbb 100644 --- a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py @@ -183,6 +183,8 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): self._check_ovs_version() if self.enable_tunneling: self.setup_tunnel_br(tun_br) + # Collect additional bridges to monitor + self.ancillary_brs = self.setup_ancillary_bridges(integ_br, tun_br) self.agent_state = { 'binary': 'neutron-openvswitch-agent', 'host': cfg.CONF.host, @@ -538,6 +540,32 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): int_br.add_flow(priority=1, actions="normal") return int_br + def setup_ancillary_bridges(self, integ_br, tun_br): + '''Setup ancillary bridges - for example br-ex.''' + ovs_bridges = set(ovs_lib.get_bridges(self.root_helper)) + # Remove all known bridges + ovs_bridges.remove(integ_br) + if self.enable_tunneling: + ovs_bridges.remove(tun_br) + br_names = [self.phys_brs[physical_network].br_name for + physical_network in self.phys_brs] + ovs_bridges.difference_update(br_names) + # Filter list of bridges to those that have external + # bridge-id's configured + br_names = [] + for bridge in ovs_bridges: + id = ovs_lib.get_bridge_external_bridge_id(self.root_helper, + bridge) + if id != bridge: + br_names.append(bridge) + ovs_bridges.difference_update(br_names) + ancillary_bridges = [] + for bridge in ovs_bridges: + br = ovs_lib.OVSBridge(bridge, self.root_helper) + LOG.info(_('Adding %s to list of bridges.'), bridge) + ancillary_bridges.append(br) + return ancillary_bridges + def setup_tunnel_br(self, tun_br): '''Setup the tunnel bridge. @@ -625,6 +653,19 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): 'added': added, 'removed': removed} + 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): if vif_port: @@ -667,6 +708,21 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): self.port_dead(port) return resync + def treat_ancillary_devices_added(self, devices): + resync = False + for device in devices: + LOG.info(_("Ancillary Port %s added"), device) + try: + self.plugin_rpc.get_device_details(self.context, device, + self.agent_id) + except Exception as e: + LOG.debug(_("Unable to get port details for " + "%(device)s: %(e)s"), + {'device': device, 'e': e}) + resync = True + continue + return resync + def treat_devices_removed(self, devices): resync = False self.sg_agent.remove_devices_filter(devices) @@ -689,6 +745,26 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): self.port_unbound(device) return resync + def treat_ancillary_devices_removed(self, devices): + resync = False + for device in devices: + LOG.info(_("Attachment %s removed"), device) + try: + details = self.plugin_rpc.update_device_down(self.context, + device, + self.agent_id) + except Exception as e: + LOG.debug(_("port_removed failed for %(device)s: %(e)s"), + {'device': device, 'e': e}) + resync = True + continue + if details['exists']: + LOG.info(_("Port %s updated."), device) + # Nothing to do regarding local networking + else: + LOG.debug(_("Device %s not defined on plugin"), device) + return resync + def process_network_ports(self, port_info): resync_a = False resync_b = False @@ -699,6 +775,17 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): # If one of the above opertaions fails => resync with plugin return (resync_a | resync_b) + def process_ancillary_network_ports(self, port_info): + resync_a = False + resync_b = False + if 'added' in port_info: + resync_a = self.treat_ancillary_devices_added(port_info['added']) + if 'removed' in port_info: + resync_b = self.treat_ancillary_devices_removed( + port_info['removed']) + # If one of the above opertaions fails => resync with plugin + return (resync_a | resync_b) + def tunnel_sync(self): resync = False try: @@ -725,6 +812,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): def rpc_loop(self): sync = True ports = set() + ancillary_ports = set() tunnel_sync = True while True: @@ -733,6 +821,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): if sync: LOG.info(_("Agent out of sync with plugin!")) ports.clear() + ancillary_ports.clear() sync = False # Notify the plugin of tunnel IP @@ -749,6 +838,14 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin): sync = self.process_network_ports(port_info) ports = port_info['current'] + # Treat ancillary devices if they exist + if self.ancillary_brs: + port_info = self.update_ancillary_ports(ancillary_ports) + if port_info: + rc = self.process_ancillary_network_ports(port_info) + ancillary_ports = port_info['current'] + sync = sync | rc + except Exception: LOG.exception(_("Error in agent event loop")) sync = True diff --git a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py index 33ad6b26b..650be551e 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py @@ -103,6 +103,9 @@ class TestOvsNeutronAgent(base.BaseTestCase): mock.patch('neutron.plugins.openvswitch.agent.ovs_neutron_agent.' 'OVSNeutronAgent.setup_integration_br', return_value=mock.Mock()), + mock.patch('neutron.plugins.openvswitch.agent.ovs_neutron_agent.' + 'OVSNeutronAgent.setup_ancillary_bridges', + return_value=[]), mock.patch('neutron.agent.linux.utils.get_interface_mac', return_value='00:00:00:00:00:01')): self.agent = ovs_neutron_agent.OVSNeutronAgent(**kwargs) @@ -394,3 +397,57 @@ class TestOvsNeutronAgent(base.BaseTestCase): self._check_ovs_vxlan_version('1.10', '1.9', constants.MINIMUM_OVS_VXLAN_VERSION, expecting_ok=False) + + +class AncillaryBridgesTest(base.BaseTestCase): + + def setUp(self): + super(AncillaryBridgesTest, self).setUp() + self.addCleanup(cfg.CONF.reset) + self.addCleanup(mock.patch.stopall) + notifier_p = mock.patch(NOTIFIER) + notifier_cls = notifier_p.start() + self.notifier = mock.Mock() + notifier_cls.return_value = self.notifier + # Avoid rpc initialization for unit tests + cfg.CONF.set_override('rpc_backend', + 'neutron.openstack.common.rpc.impl_fake') + cfg.CONF.set_override('report_interval', 0, 'AGENT') + self.kwargs = ovs_neutron_agent.create_agent_config_map(cfg.CONF) + + def _test_ancillary_bridges(self, bridges, ancillary): + device_ids = ancillary[:] + + def pullup_side_effect(self, *args): + result = device_ids.pop(0) + return result + + with contextlib.nested( + mock.patch('neutron.plugins.openvswitch.agent.ovs_neutron_agent.' + 'OVSNeutronAgent.setup_integration_br', + return_value=mock.Mock()), + mock.patch('neutron.agent.linux.utils.get_interface_mac', + return_value='00:00:00:00:00:01'), + mock.patch('neutron.agent.linux.ovs_lib.get_bridges', + return_value=bridges), + mock.patch( + 'neutron.agent.linux.ovs_lib.get_bridge_external_bridge_id', + side_effect=pullup_side_effect)): + self.agent = ovs_neutron_agent.OVSNeutronAgent(**self.kwargs) + self.assertEqual(len(ancillary), len(self.agent.ancillary_brs)) + if ancillary: + bridges = [br.br_name for br in self.agent.ancillary_brs] + for br in ancillary: + self.assertIn(br, bridges) + + def test_ancillary_bridges_single(self): + bridges = ['br-int', 'br-ex'] + self._test_ancillary_bridges(bridges, ['br-ex']) + + def test_ancillary_bridges_none(self): + bridges = ['br-int'] + self._test_ancillary_bridges(bridges, []) + + def test_ancillary_bridges_multiple(self): + bridges = ['br-int', 'br-ex1', 'br-ex2'] + self._test_ancillary_bridges(bridges, ['br-ex1', 'br-ex2']) diff --git a/neutron/tests/unit/openvswitch/test_ovs_tunnel.py b/neutron/tests/unit/openvswitch/test_ovs_tunnel.py index b1186e330..048ebf291 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_tunnel.py +++ b/neutron/tests/unit/openvswitch/test_ovs_tunnel.py @@ -87,6 +87,7 @@ class TunnelTest(base.BaseTestCase): self.mock_map_tun_bridge = ovs_lib.OVSBridge( self.MAP_TUN_BRIDGE, 'sudo') + self.mock_map_tun_bridge.br_name = self.MAP_TUN_BRIDGE self.mock_map_tun_bridge.remove_all_flows() self.mock_map_tun_bridge.add_flow(priority=1, actions='normal') self.mock_int_bridge.delete_port('int-tunnel_bridge_mapping') @@ -123,6 +124,10 @@ class TunnelTest(base.BaseTestCase): 'phy-tunnel_bridge_mapping').AndReturn([self.inta, self.intb]) self.mock_int_bridge.get_local_port_mac().AndReturn('000000000001') + self.mox.StubOutWithMock(ovs_lib, 'get_bridges') + ovs_lib.get_bridges('sudo').AndReturn([self.INT_BRIDGE, + self.TUN_BRIDGE, + self.MAP_TUN_BRIDGE]) def testConstruct(self): self.mox.ReplayAll()