From 740ab628f4d060f7caa33d9c3dc1e189255468c1 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Fri, 8 May 2020 13:19:05 +0200 Subject: [PATCH] Remove usage of the "firewall_driver" in the neutron server Back in Newton, patch [1] added to the agents possibility to report in the heartbeat messages if hybrid plug of the ports is required or not. Usage of "firewall_driver" option by mechanism drivers (so on the server's side) was kept just for backward compatibility. But as we are now about 4 years from the [1] I think it should be safe to do small cleaning, remove usage of this option in the neutron server and not confuse users where this config option has to be set and why. [1] https://review.opendev.org/#/c/311814/ Change-Id: I2ccc4c8784c64858acaa3c3431cf9a3d13e5e154 --- .../mech_driver/mech_openvswitch.py | 16 +++------- .../mech_driver/test_mech_openvswitch.py | 32 +++++++++---------- ...-from-neutron-server-920564ec77503631.yaml | 9 ++++++ 3 files changed, 29 insertions(+), 28 deletions(-) create mode 100644 releasenotes/notes/Remove-firewall_driver-option-from-neutron-server-920564ec77503631.yaml 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 422c5708483..e79c233b3d0 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py +++ b/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py @@ -56,13 +56,7 @@ class OpenvswitchMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase): def __init__(self): sg_enabled = securitygroups_rpc.is_firewall_enabled() - hybrid_plug_required = ( - not cfg.CONF.SECURITYGROUP.firewall_driver or - cfg.CONF.SECURITYGROUP.firewall_driver in ( - IPTABLES_FW_DRIVER_FULL, 'iptables_hybrid') - ) and sg_enabled vif_details = {portbindings.CAP_PORT_FILTER: sg_enabled, - portbindings.OVS_HYBRID_PLUG: hybrid_plug_required, portbindings.VIF_DETAILS_CONNECTIVITY: portbindings.CONNECTIVITY_L2} # NOTE(moshele): Bind DIRECT (SR-IOV) port allows @@ -185,12 +179,10 @@ class OpenvswitchMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase): a_config = agent['configurations'] vif_type = self.get_vif_type(context, agent, segment=None) if vif_type != portbindings.VIF_TYPE_VHOST_USER: - details = dict(self.vif_details) - hybrid = portbindings.OVS_HYBRID_PLUG - if hybrid in a_config: - # we only override the vif_details for hybrid plugging set - # in the constructor if the agent specifically requests it - details[hybrid] = a_config[hybrid] + details = { + **self.vif_details, + portbindings.OVS_HYBRID_PLUG: a_config.get( + portbindings.OVS_HYBRID_PLUG)} else: sock_path = self.agent_vhu_sockpath(agent, context.current['id']) caps = a_config.get('ovs_capabilities', {}) 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 2098aaa93c6..86f9576c1d4 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 @@ -44,6 +44,7 @@ class OpenvswitchMechanismBaseTestCase(base.AgentMechanismBaseTestCase): GOOD_TUNNEL_TYPES = ['gre', 'vxlan'] GOOD_CONFIGS = {'bridge_mappings': GOOD_MAPPINGS, 'integration_bridge': 'br-int', + portbindings.OVS_HYBRID_PLUG: True, 'tunnel_types': GOOD_TUNNEL_TYPES} BAD_MAPPINGS = {'wrong_physical_network': 'wrong_bridge'} @@ -119,6 +120,16 @@ class OpenvswitchMechanismSGDisabledBaseTestCase( portbindings.VIF_DETAILS_CONNECTIVITY: portbindings.CONNECTIVITY_L2} + GOOD_MAPPINGS = {'fake_physical_network': 'fake_bridge'} + GOOD_TUNNEL_TYPES = ['gre', 'vxlan'] + GOOD_CONFIGS = {'bridge_mappings': GOOD_MAPPINGS, + 'integration_bridge': 'br-int', + portbindings.OVS_HYBRID_PLUG: False, + 'tunnel_types': GOOD_TUNNEL_TYPES} + AGENTS = [{'alive': True, + 'configurations': GOOD_CONFIGS, + 'host': 'host'}] + def setUp(self): cfg.CONF.set_override('enable_security_group', False, @@ -133,17 +144,6 @@ class OpenvswitchMechanismHybridPlugTestCase(OpenvswitchMechanismBaseTestCase): return base.FakePortContext(self.AGENT_TYPE, agents, segments, vnic_type=self.VNIC_TYPE) - def test_backward_compat_with_unreporting_agent(self): - hybrid = portbindings.OVS_HYBRID_PLUG - # agent didn't report so it should be hybrid based on server config - context = self._make_port_ctx(self.AGENTS) - self.driver.bind_port(context) - self.assertTrue(context._bound_vif_details[hybrid]) - self.driver.vif_details[hybrid] = False - context = self._make_port_ctx(self.AGENTS) - self.driver.bind_port(context) - self.assertFalse(context._bound_vif_details[hybrid]) - def test_hybrid_plug_true_if_agent_requests(self): hybrid = portbindings.OVS_HYBRID_PLUG # set server side default to false and ensure that hybrid becomes @@ -240,11 +240,11 @@ 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]}} + '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', diff --git a/releasenotes/notes/Remove-firewall_driver-option-from-neutron-server-920564ec77503631.yaml b/releasenotes/notes/Remove-firewall_driver-option-from-neutron-server-920564ec77503631.yaml new file mode 100644 index 00000000000..31c8f55e0ab --- /dev/null +++ b/releasenotes/notes/Remove-firewall_driver-option-from-neutron-server-920564ec77503631.yaml @@ -0,0 +1,9 @@ +--- +upgrade: + - | + The configuration option ``firewall_driver`` is no longer used by + neutron-server, it only applies to the L2 agent. + This was required for backward-compatibility for hybrid plugging, + but since the Newton release the L2 agent has been able to report + hybrid plugging is needed in it's report message back to the + server.