From 995744c576503b1de90c922dbecf690ad49f244f Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Mon, 27 Aug 2018 18:30:53 +0100 Subject: [PATCH] Always set ovs bridge name in vif:binding-details - This change updates _set_bridge_name to set the bridge name field in the vif binding details. - This change adds the integration_bridge name to the agent configuration report. Change-Id: I454efcb226745c585935d5bd1b3d378f69a55ca2 Closes-Bug: #1788009 --- .../openvswitch/agent/ovs_neutron_agent.py | 2 + .../mech_driver/mech_openvswitch.py | 8 +++- neutron/tests/common/helpers.py | 11 +++-- .../functional/services/trunk/test_plugin.py | 15 ++++++- .../mech_driver/test_mech_openvswitch.py | 40 +++++++++++++++++-- 5 files changed, 66 insertions(+), 10 deletions(-) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index 00b64510e65..22b5706a085 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -270,6 +270,8 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, 'host': host, 'topic': n_const.L2_AGENT_TOPIC, 'configurations': {'bridge_mappings': self.bridge_mappings, + 'integration_bridge': + ovs_conf.integration_bridge, 'tunnel_types': self.tunnel_types, 'tunneling_ip': self.local_ip, 'l2_population': self.l2_pop, diff --git a/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py b/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py index 70122a9ef96..a2ba6ee943b 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py +++ b/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py @@ -111,11 +111,11 @@ class OpenvswitchMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase): def get_vif_details(self, context, agent, segment): vif_details = self._pre_get_vif_details(agent, context) - self._set_bridge_name(context.current, vif_details) + self._set_bridge_name(context.current, vif_details, agent) return vif_details @staticmethod - def _set_bridge_name(port, vif_details): + def _set_bridge_name(port, vif_details, agent): # REVISIT(rawlin): add BridgeName as a nullable column to the Port # model and simply check here if it's set and insert it into the # vif_details. @@ -123,6 +123,10 @@ class OpenvswitchMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase): def set_bridge_name_inner(bridge_name): vif_details[portbindings.VIF_DETAILS_BRIDGE_NAME] = bridge_name + bridge_name = agent['configurations'].get('integration_bridge') + if bridge_name: + vif_details[portbindings.VIF_DETAILS_BRIDGE_NAME] = bridge_name + registry.publish(a_const.OVS_BRIDGE_NAME, events.BEFORE_READ, set_bridge_name_inner, payload=events.EventPayload( None, metadata={'port': port})) diff --git a/neutron/tests/common/helpers.py b/neutron/tests/common/helpers.py index cc1d7771966..0b424a1be02 100644 --- a/neutron/tests/common/helpers.py +++ b/neutron/tests/common/helpers.py @@ -142,7 +142,8 @@ def set_agent_admin_state(agent_id, admin_state_up=False): def _get_l2_agent_dict(host, agent_type, binary, tunnel_types=None, tunneling_ip='20.0.0.1', interface_mappings=None, bridge_mappings=None, l2pop_network_types=None, - device_mappings=None, start_flag=True): + device_mappings=None, start_flag=True, + integration_bridge=None): agent = { 'binary': binary, 'host': host, @@ -163,6 +164,8 @@ def _get_l2_agent_dict(host, agent_type, binary, tunnel_types=None, agent['configurations']['l2pop_network_types'] = l2pop_network_types if device_mappings is not None: agent['configurations']['device_mappings'] = device_mappings + if integration_bridge is not None: + agent['configurations']['integration_bridge'] = integration_bridge return agent @@ -170,11 +173,13 @@ def register_ovs_agent(host=HOST, agent_type=constants.AGENT_TYPE_OVS, binary='neutron-openvswitch-agent', tunnel_types=['vxlan'], tunneling_ip='20.0.0.1', interface_mappings=None, bridge_mappings=None, - l2pop_network_types=None, plugin=None, start_flag=True): + l2pop_network_types=None, plugin=None, start_flag=True, + integration_bridge=None): agent = _get_l2_agent_dict(host, agent_type, binary, tunnel_types, tunneling_ip, interface_mappings, bridge_mappings, l2pop_network_types, - start_flag=start_flag) + start_flag=start_flag, + integration_bridge=integration_bridge) return _register_agent(agent, plugin) diff --git a/neutron/tests/functional/services/trunk/test_plugin.py b/neutron/tests/functional/services/trunk/test_plugin.py index c124ef6c025..5c6f882fab4 100644 --- a/neutron/tests/functional/services/trunk/test_plugin.py +++ b/neutron/tests/functional/services/trunk/test_plugin.py @@ -42,7 +42,20 @@ class TestTrunkServicePlugin(ml2_test_base.ML2TestFramework): utils.gen_trunk_br_name(trunk_res['id']), bound_port[pb.VIF_DETAILS][pb.VIF_DETAILS_BRIDGE_NAME]) - def test_ovs_bridge_name_not_set_when_not_trunk(self): + def test_ovs_bridge_name_set_to_integration_bridge_when_not_trunk(self): + helpers.register_ovs_agent(host=helpers.HOST, + integration_bridge='br-fake') + with self.port() as port: + port['port'][pb.HOST_ID] = helpers.HOST + bound_port = self.core_plugin.update_port(self.context, + port['port']['id'], port) + self.assertEqual('br-fake', + bound_port[pb.VIF_DETAILS].get(pb.VIF_DETAILS_BRIDGE_NAME)) + + def test_ovs_bridge_name_not_set_if_integration_bridge_not_set(self): + """This will only happen if a stein or later ml2 driver is + binding an interface for a pre stein ml2 agent. + """ helpers.register_ovs_agent(host=helpers.HOST) with self.port() as port: port['port'][pb.HOST_ID] = helpers.HOST diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/mech_driver/test_mech_openvswitch.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/mech_driver/test_mech_openvswitch.py index 941fbdd335f..7a4e4e76ab2 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/mech_driver/test_mech_openvswitch.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/mech_driver/test_mech_openvswitch.py @@ -30,7 +30,8 @@ from neutron.tests.unit.plugins.ml2 import _test_mech_agent as base class OpenvswitchMechanismBaseTestCase(base.AgentMechanismBaseTestCase): VIF_TYPE = portbindings.VIF_TYPE_OVS - VIF_DETAILS = {portbindings.OVS_DATAPATH_TYPE: 'system', + VIF_DETAILS = {'bridge_name': 'br-int', + portbindings.OVS_DATAPATH_TYPE: 'system', portbindings.CAP_PORT_FILTER: True, portbindings.OVS_HYBRID_PLUG: True} AGENT_TYPE = constants.AGENT_TYPE_OVS @@ -38,11 +39,13 @@ class OpenvswitchMechanismBaseTestCase(base.AgentMechanismBaseTestCase): GOOD_MAPPINGS = {'fake_physical_network': 'fake_bridge'} GOOD_TUNNEL_TYPES = ['gre', 'vxlan'] GOOD_CONFIGS = {'bridge_mappings': GOOD_MAPPINGS, + 'integration_bridge': 'br-int', 'tunnel_types': GOOD_TUNNEL_TYPES} BAD_MAPPINGS = {'wrong_physical_network': 'wrong_bridge'} BAD_TUNNEL_TYPES = ['bad_tunnel_type'] BAD_CONFIGS = {'bridge_mappings': BAD_MAPPINGS, + 'integration_bridge': 'br-int', 'tunnel_types': BAD_TUNNEL_TYPES} AGENTS = [{'alive': True, @@ -70,18 +73,43 @@ class OpenvswitchMechanismBaseTestCase(base.AgentMechanismBaseTestCase): def fake_callback(resource, event, trigger, payload=None): trigger('fake-br-name') + def noop_callback(resource, event, trigger, payload=None): + pass + + # hardcode callback to override bridge name registry.subscribe(fake_callback, a_const.OVS_BRIDGE_NAME, events.BEFORE_READ) fake_vif_details = {} - self.driver._set_bridge_name('foo', fake_vif_details) + fake_agent = {'configurations': {'integration_bridge': 'fake-br'}} + old_fake_agent = {'configurations': {}} + self.driver._set_bridge_name('foo', fake_vif_details, fake_agent) + # assert that callback value is used self.assertEqual( 'fake-br-name', fake_vif_details.get(portbindings.VIF_DETAILS_BRIDGE_NAME, '')) + # replace callback with noop + registry.unsubscribe(fake_callback, a_const.OVS_BRIDGE_NAME, + events.BEFORE_READ) + registry.subscribe(noop_callback, a_const.OVS_BRIDGE_NAME, + events.BEFORE_READ) + fake_vif_details = {} + self.driver._set_bridge_name('foo', fake_vif_details, fake_agent) + # assert that agent config value is used + self.assertEqual( + 'fake-br', + fake_vif_details.get(portbindings.VIF_DETAILS_BRIDGE_NAME, '')) + fake_vif_details = {} + self.driver._set_bridge_name('foo', fake_vif_details, old_fake_agent) + # assert that if agent does not supply integration_bridge bridge_name + # is not set in vif:binding-details + self.assertIsNone( + fake_vif_details.get(portbindings.VIF_DETAILS_BRIDGE_NAME)) class OpenvswitchMechanismSGDisabledBaseTestCase( OpenvswitchMechanismBaseTestCase): - VIF_DETAILS = {portbindings.OVS_DATAPATH_TYPE: 'system', + VIF_DETAILS = {'bridge_name': 'br-int', + portbindings.OVS_DATAPATH_TYPE: 'system', portbindings.CAP_PORT_FILTER: False, portbindings.OVS_HYBRID_PLUG: False} @@ -169,7 +197,8 @@ class OpenvswitchMechanismSGDisabledLocalTestCase( class OpenvswitchMechanismFirewallUndefinedTestCase( OpenvswitchMechanismBaseTestCase, base.AgentMechanismLocalTestCase): - VIF_DETAILS = {portbindings.OVS_DATAPATH_TYPE: 'system', + VIF_DETAILS = {'bridge_name': 'br-int', + portbindings.OVS_DATAPATH_TYPE: 'system', portbindings.CAP_PORT_FILTER: True, portbindings.OVS_HYBRID_PLUG: True} @@ -190,18 +219,21 @@ class OpenvswitchMechanismDPDKTestCase(OpenvswitchMechanismBaseTestCase): GOOD_TUNNEL_TYPES = ['gre', 'vxlan'] VHOST_CONFIGS = {'bridge_mappings': GOOD_MAPPINGS, + 'integration_bridge': 'br-int', 'tunnel_types': GOOD_TUNNEL_TYPES, 'datapath_type': a_const.OVS_DATAPATH_NETDEV, 'ovs_capabilities': { 'iface_types': [a_const.OVS_DPDK_VHOST_USER]}} VHOST_SERVER_CONFIGS = {'bridge_mappings': GOOD_MAPPINGS, + 'integration_bridge': 'br-int', 'tunnel_types': GOOD_TUNNEL_TYPES, 'datapath_type': a_const.OVS_DATAPATH_NETDEV, 'ovs_capabilities': { 'iface_types': [a_const.OVS_DPDK_VHOST_USER_CLIENT]}} SYSTEM_CONFIGS = {'bridge_mappings': GOOD_MAPPINGS, + 'integration_bridge': 'br-int', 'tunnel_types': GOOD_TUNNEL_TYPES, 'datapath_type': a_const.OVS_DATAPATH_SYSTEM, 'ovs_capabilities': {'iface_types': []}}