From 02cc3ca30733c88003331af26fbd364d703dd552 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C5=82awek=20Kap=C5=82o=C5=84ski?= <slawek@kaplonski.pl>
Date: Thu, 8 Feb 2018 16:07:33 +0100
Subject: [PATCH] ovsfw: Update SG rules even if OVSFW Port is not found

In patch [1] ovs firewall driver was changed and update_port_filter()
method was not trying to initialize port flows in case when
OVSFWPortNotFound is raised.
Without that when e.g. instance is hard rebooted and of_port
number is changed firewall openflow rules were not initialized
for such port and there was no connectivity to such VM.

[1] https://review.openstack.org/#/c/531414/

Change-Id: I6d917cbac61293e9a956a2efcd9f2b720e4cac95
Closes-Bug: #1747709
---
 .../linux/openvswitch_firewall/firewall.py    | 32 +++++++++----------
 .../openvswitch_firewall/test_firewall.py     | 12 ++++++-
 2 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py
index deadf481df9..098d01e97ee 100644
--- a/neutron/agent/linux/openvswitch_firewall/firewall.py
+++ b/neutron/agent/linux/openvswitch_firewall/firewall.py
@@ -498,18 +498,7 @@ class OVSFirewallDriver(firewall.FirewallDriver):
         if not firewall.port_sec_enabled(port):
             self._initialize_egress_no_port_security(port['device'])
             return
-        old_of_port = self.get_ofport(port)
-        # Make sure delete old allowed_address_pair MACs because
-        # allowed_address_pair MACs will be updated in
-        # self.get_or_create_ofport(port)
-        if old_of_port:
-            LOG.error("Initializing port %s that was already "
-                      "initialized.",
-                      port['device'])
-            self.delete_all_port_flows(old_of_port)
-        of_port = self.get_or_create_ofport(port)
-        self.initialize_port_flows(of_port)
-        self.add_flows_from_rules(of_port)
+        self._set_port_filters(port, old_port_expected=False)
 
     def update_port_filter(self, port):
         """Update rules for given port
@@ -529,17 +518,26 @@ class OVSFirewallDriver(firewall.FirewallDriver):
                 LOG.debug(e)
             else:
                 self.prepare_port_filter(port)
-            return
-        old_of_port = self.get_ofport(port)
+                return
         try:
-            of_port = self.get_or_create_ofport(port)
+            self._set_port_filters(port, old_port_expected=True)
         except exceptions.OVSFWPortNotFound as not_found_error:
             LOG.info("port %(port_id)s does not exist in ovsdb: %(err)s.",
                      {'port_id': port['device'],
                       'err': not_found_error})
-            return
+
+    def _set_port_filters(self, port, old_port_expected):
+        old_of_port = self.get_ofport(port)
+        # Make sure delete old allowed_address_pair MACs because
+        # allowed_address_pair MACs will be updated in
+        # self.get_or_create_ofport(port)
+        if old_of_port:
+            if not old_port_expected:
+                LOG.info("Initializing port %s that was already "
+                         "initialized.", port['device'])
+            self.delete_all_port_flows(old_of_port)
         # TODO(jlibosva): Handle firewall blink
-        self.delete_all_port_flows(old_of_port)
+        of_port = self.get_or_create_ofport(port)
         self.initialize_port_flows(of_port)
         self.add_flows_from_rules(of_port)
 
diff --git a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py
index 77aa30b8ebb..81486eccf4b 100644
--- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py
+++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py
@@ -583,10 +583,20 @@ class TestOVSFirewallDriver(base.BaseTestCase):
         port_dict = {'device': 'port-id',
                      'security_groups': [1]}
         self._prepare_security_group()
+
         with mock.patch.object(
-                self.firewall, 'prepare_port_filter') as prepare_mock:
+            self.firewall, 'prepare_port_filter'
+        ) as prepare_mock, mock.patch.object(
+            self.firewall, 'initialize_port_flows'
+        ) as initialize_port_flows_mock, mock.patch.object(
+            self.firewall, 'add_flows_from_rules'
+        ) as add_flows_from_rules_mock:
             self.firewall.update_port_filter(port_dict)
+
         self.assertFalse(prepare_mock.called)
+        self.assertFalse(self.mock_bridge.br.delete_flows.called)
+        self.assertTrue(initialize_port_flows_mock.called)
+        self.assertTrue(add_flows_from_rules_mock.called)
 
     def test_update_port_filter_port_security_disabled(self):
         port_dict = {'device': 'port-id',