From eab443dc098c8b50d7540274e147058cd03275c8 Mon Sep 17 00:00:00 2001 From: Swaminathan Vasudevan Date: Tue, 1 Oct 2019 18:12:57 +0000 Subject: [PATCH] FWaaS-DVR: FWaaS rules not updated in DVR routers on compute host When a firewall is created after the routers have been deployed, we are supposed to manually do a firewall-update on specific routers where we wanted the firewall policy to be applied in the case of FWaaS-v1. But in the case of DVR routers, we have seen the firewall-update for routers that are deployed in the compute hosts are not getting propagated properly. The reason is the firewall update, firewall delete and firewall create events are not notified to all the respective router hosts. The original code only handles getting the host information from the routers that are scheduled to the l3 agent, but in the case of DVR routers, the routers are only scheduled to the network node l3 agents and the other distributed routers on compute are created based on the service port binding. This bug is applicable only for FWaaS-v1 and the patch should be applied for Rocky release and below, since FWaaS-v1 is not supported in Stein and Train release. This patch fixes the problem described above by taking care of collecting all the hosts involved with DVR routers and notifying them. Change-Id: I7ef193baba9447d0f09cd9544cce9d05a956b920 Closes-Bug: #1845557 (cherry picked from commit 20fd02611674da5365e93d06f330d1f76fd71e3e) --- .../services/firewall/fwaas_plugin.py | 11 +++++++- .../services/firewall/test_fwaas_plugin.py | 27 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/neutron_fwaas/services/firewall/fwaas_plugin.py b/neutron_fwaas/services/firewall/fwaas_plugin.py index 08632a38b..f54647fe7 100644 --- a/neutron_fwaas/services/firewall/fwaas_plugin.py +++ b/neutron_fwaas/services/firewall/fwaas_plugin.py @@ -179,9 +179,18 @@ class FirewallPlugin( l3_plugin, nl_constants.L3_AGENT_SCHEDULER_EXT_ALIAS) and getattr(l3_plugin, 'get_l3_agents_hosting_routers', False)) if no_broadcast: + # This call checks for all scheduled routers to the network node agents = l3_plugin.get_l3_agents_hosting_routers( context, router_ids, admin_state_up=True, active=True) - return [a.host for a in agents] + scheduled_rtr_hosts = set([a.host for a in agents]) + # Now check for unscheduled DVR router on distributed compute hosts + unscheduled_dvr_hosts = set() + for router_id in router_ids: + hosts = set(l3_plugin._get_dvr_hosts_for_router( + context, router_id)) + unscheduled_dvr_hosts |= hosts + total_hosts = scheduled_rtr_hosts.union(unscheduled_dvr_hosts) + return total_hosts # NOTE(blallau): default: FirewallAgentAPI performs RPC broadcast return [None] diff --git a/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin.py b/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin.py index fbaaeeff1..a992b4739 100644 --- a/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin.py +++ b/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin.py @@ -377,6 +377,33 @@ class TestFirewallPluginBase(TestFirewallRouterInsertionBase, self.assertEqual('other-tenant', fw1['firewall']['tenant_id']) self.assertEqual(self._tenant_id, fw2['firewall']['tenant_id']) + def test_update_firewall_calls_get_dvr_hosts_for_router(self): + ctx = context.get_admin_context() + name = "user_fw" + attrs = self._get_test_firewall_attrs(name) + with self.router(name='router1', admin_state_up=True, + tenant_id=self._tenant_id) as router1: + with self.firewall_policy() as fwp: + fwp_id = fwp['firewall_policy']['id'] + attrs['firewall_policy_id'] = fwp_id + with self.firewall( + firewall_policy_id=fwp_id, + admin_state_up=test_db_firewall.ADMIN_STATE_UP, + router_ids=[router1['router']['id']] + ) as firewall: + fw_id = firewall['firewall']['id'] + self.callbacks.set_firewall_status(ctx, fw_id, + nl_constants.ACTIVE) + with mock.patch.object( + self.l3_plugin, + 'get_l3_agents_hosting_routers') as s_hosts, \ + mock.patch.object( + self.l3_plugin, + '_get_dvr_hosts_for_router') as u_hosts: + self.plugin.update_firewall(ctx, fw_id, firewall) + self.assertTrue(u_hosts.called) + self.assertTrue(s_hosts.called) + def test_update_firewall(self): ctx = context.get_admin_context() name = "new_firewall1"