Merge "Avoid referencing stale instance/network_info dicts in firewall" into stable/havana

4 files changed, 32 insertions, 27 deletions

diff --git a/nova/tests/virt/libvirt/test_libvirt.py b/nova/tests/virt/libvirt/test_libvirt.py
index bc19d98785..9762540c2e 100644
--- a/nova/tests/virt/libvirt/test_libvirt.py
+++ b/nova/tests/virt/libvirt/test_libvirt.py
@@ -6194,24 +6194,25 @@ class IptablesFirewallTestCase(test.TestCase):
         self.fw.instance_rules(instance_ref,
                                mox.IgnoreArg()).AndReturn((None, None))
         self.fw.add_filters_for_instance(instance_ref, mox.IgnoreArg(),
-                                         mox.IgnoreArg())
+                                         mox.IgnoreArg(), mox.IgnoreArg())
         self.fw.instance_rules(instance_ref,
                                mox.IgnoreArg()).AndReturn((None, None))
         self.fw.iptables.ipv4['filter'].has_chain(mox.IgnoreArg()
                                                   ).AndReturn(True)
         self.fw.add_filters_for_instance(instance_ref, mox.IgnoreArg(),
-                                         mox.IgnoreArg())
+                                         mox.IgnoreArg(), mox.IgnoreArg())
         self.mox.ReplayAll()
 
         self.fw.prepare_instance_filter(instance_ref, mox.IgnoreArg())
-        self.fw.instances[instance_ref['id']] = instance_ref
+        self.fw.instance_info[instance_ref['id']] = (instance_ref, None)
         self.fw.do_refresh_security_group_rules("fake")
 
     def test_do_refresh_security_group_rules_instance_disappeared(self):
         instance1 = {'id': 0, 'uuid': 'fake-uuid1'}
         instance2 = {'id': 1, 'uuid': 'fake-uuid2'}
-        self.fw.instances = {0: instance1, 1: instance2}
-        self.fw.network_infos = _fake_network_info(self.stubs, 2)
+        network_infos = _fake_network_info(self.stubs, 2)
+        self.fw.instance_info[instance1['id']] = (instance1, network_infos[0])
+        self.fw.instance_info[instance2['id']] = (instance2, network_infos[1])
         mock_filter = mock.MagicMock()
         with mock.patch.dict(self.fw.iptables.ipv4, {'filter': mock_filter}):
             mock_filter.has_chain.return_value = False
diff --git a/nova/tests/virt/xenapi/test_xenapi.py b/nova/tests/virt/xenapi/test_xenapi.py
index 0020d7d712..631d382a5c 100644
--- a/nova/tests/virt/xenapi/test_xenapi.py
+++ b/nova/tests/virt/xenapi/test_xenapi.py
@@ -2760,7 +2760,8 @@ class XenAPIDom0IptablesFirewallTestCase(stubs.XenAPITestBase):
         db.instance_add_security_group(admin_ctxt, instance_ref['uuid'],
                                        secgroup['id'])
         self.fw.prepare_instance_filter(instance_ref, network_info)
-        self.fw.instances[instance_ref['id']] = instance_ref
+        self.fw.instance_info[instance_ref['id']] = (instance_ref,
+                                                     network_info)
         self._validate_security_group()
         # add a rule to the security group
         db.security_group_rule_create(admin_ctxt,
diff --git a/nova/virt/firewall.py b/nova/virt/firewall.py
index f4929c3d50..10d46ea9ec 100644
--- a/nova/virt/firewall.py
+++ b/nova/virt/firewall.py
@@ -141,8 +141,7 @@ class IptablesFirewallDriver(FirewallDriver):
     def __init__(self, virtapi, **kwargs):
         super(IptablesFirewallDriver, self).__init__(virtapi)
         self.iptables = linux_net.iptables_manager
-        self.instances = {}
-        self.network_infos = {}
+        self.instance_info = {}
         self.basically_filtered = False
 
         # Flags for DHCP request rule
@@ -168,9 +167,7 @@ class IptablesFirewallDriver(FirewallDriver):
         self.iptables.defer_apply_off()
 
     def unfilter_instance(self, instance, network_info):
-        if self.instances.pop(instance['id'], None):
-            # NOTE(vish): use the passed info instead of the stored info
-            self.network_infos.pop(instance['id'])
+        if self.instance_info.pop(instance['id'], None):
             self.remove_filters_for_instance(instance)
             self.iptables.apply()
         else:
@@ -178,10 +175,10 @@ class IptablesFirewallDriver(FirewallDriver):
                      'filtered'), instance=instance)
 
     def prepare_instance_filter(self, instance, network_info):
-        self.instances[instance['id']] = instance
-        self.network_infos[instance['id']] = network_info
+        self.instance_info[instance['id']] = (instance, network_info)
         ipv4_rules, ipv6_rules = self.instance_rules(instance, network_info)
-        self.add_filters_for_instance(instance, ipv4_rules, ipv6_rules)
+        self.add_filters_for_instance(instance, network_info, ipv4_rules,
+                                      ipv6_rules)
         LOG.debug(_('Filters added to instance'), instance=instance)
         self.refresh_provider_fw_rules()
         LOG.debug(_('Provider Firewall Rules refreshed'), instance=instance)
@@ -238,9 +235,8 @@ class IptablesFirewallDriver(FirewallDriver):
             for rule in ipv6_rules:
                 self.iptables.ipv6['filter'].add_rule(chain_name, rule)
 
-    def add_filters_for_instance(self, instance, inst_ipv4_rules,
+    def add_filters_for_instance(self, instance, network_info, inst_ipv4_rules,
                                  inst_ipv6_rules):
-        network_info = self.network_infos[instance['id']]
         chain_name = self._instance_chain_name(instance)
         if CONF.use_ipv6:
             self.iptables.ipv6['filter'].add_chain(chain_name)
@@ -439,8 +435,8 @@ class IptablesFirewallDriver(FirewallDriver):
         self.iptables.apply()
 
     @utils.synchronized('iptables', external=True)
-    def _inner_do_refresh_rules(self, instance, ipv4_rules,
-                                               ipv6_rules):
+    def _inner_do_refresh_rules(self, instance, network_info, ipv4_rules,
+                                ipv6_rules):
         chain_name = self._instance_chain_name(instance)
         if not self.iptables.ipv4['filter'].has_chain(chain_name):
             LOG.info(
@@ -449,19 +445,28 @@ class IptablesFirewallDriver(FirewallDriver):
                 instance=instance)
             return
         self.remove_filters_for_instance(instance)
-        self.add_filters_for_instance(instance, ipv4_rules, ipv6_rules)
+        self.add_filters_for_instance(instance, network_info, ipv4_rules,
+                                      ipv6_rules)
 
     def do_refresh_security_group_rules(self, security_group):
-        for instance in self.instances.values():
-            network_info = self.network_infos[instance['id']]
+        id_list = self.instance_info.keys()
+        for instance_id in id_list:
+            try:
+                instance, network_info = self.instance_info[instance_id]
+            except KeyError:
+                # NOTE(danms): instance cache must have been modified,
+                # ignore this deleted instance and move on
+                continue
             ipv4_rules, ipv6_rules = self.instance_rules(instance,
                                                          network_info)
-            self._inner_do_refresh_rules(instance, ipv4_rules, ipv6_rules)
+            self._inner_do_refresh_rules(instance, network_info, ipv4_rules,
+                                         ipv6_rules)
 
     def do_refresh_instance_rules(self, instance):
-        network_info = self.network_infos[instance['id']]
+        _instance, network_info = self.instance_info[instance['id']]
         ipv4_rules, ipv6_rules = self.instance_rules(instance, network_info)
-        self._inner_do_refresh_rules(instance, ipv4_rules, ipv6_rules)
+        self._inner_do_refresh_rules(instance, network_info, ipv4_rules,
+                                     ipv6_rules)
 
     def refresh_provider_fw_rules(self):
         """See :class:`FirewallDriver` docs."""
diff --git a/nova/virt/libvirt/firewall.py b/nova/virt/libvirt/firewall.py
index acfc706c38..808beb0f82 100644
--- a/nova/virt/libvirt/firewall.py
+++ b/nova/virt/libvirt/firewall.py
@@ -316,9 +316,7 @@ class IptablesFirewallDriver(base_firewall.IptablesFirewallDriver):
     def unfilter_instance(self, instance, network_info):
         # NOTE(salvatore-orlando):
         # Overriding base class method for applying nwfilter operation
-        if self.instances.pop(instance['id'], None):
-            # NOTE(vish): use the passed info instead of the stored info
-            self.network_infos.pop(instance['id'])
+        if self.instance_info.pop(instance['id'], None):
             self.remove_filters_for_instance(instance)
             self.iptables.apply()
             self.nwfilter.unfilter_instance(instance, network_info)