Avoid re-adding iptables rules for instances that have disappeared

The remove_filters_for_instance() method fails silently if the
instance's chain is gone (i.e. it's been deleted). If this
happens while we're refreshing security group rules, we will
not notice this case and re-add stale rules for an old instance,
breaking our firewall for new instances.

This adds a quick check after we've captured the lock to see if
the associated chain exists, and bails if it doesn't.

Conflicts:
	nova/virt/firewall.py

Required changes:
- unit test was updated because in Havana, IptablesFirewallDriver has
  .instances instead of .instance_infos, and a separate .network_infos
  attribute.

Change-Id: Ic75988939f82de49735d85fe99a9eecd4baf45c9
Related-bug: #1182131
(cherry picked from commit 6aa368b99249d01f8fd7183c15d11986ad6a6fb7)
(cherry picked from commit 132d5a2064cfdc579a2e51f07e10e8eafbdf6327)

3 files changed, 37 insertions, 0 deletions

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 5183ed50d4..7552b1a217 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -185,6 +185,12 @@ class IptablesTable(object):
         self.remove_chains = set()
         self.dirty = True
 
+    def has_chain(self, name, wrap=True):
+        if wrap:
+            return name in self.chains
+        else:
+            return name in self.unwrapped_chains
+
     def add_chain(self, name, wrap=True):
         """Adds a named chain to the table.
 
diff --git a/nova/tests/virt/libvirt/test_libvirt.py b/nova/tests/virt/libvirt/test_libvirt.py
index b88bcac053..6f57b1a2ef 100644
--- a/nova/tests/virt/libvirt/test_libvirt.py
+++ b/nova/tests/virt/libvirt/test_libvirt.py
@@ -6163,6 +6163,8 @@ class IptablesFirewallTestCase(test.TestCase):
         self.mox.StubOutWithMock(self.fw,
                                  'add_filters_for_instance',
                                  use_mock_anything=True)
+        self.mox.StubOutWithMock(self.fw.iptables.ipv4['filter'],
+                                 'has_chain')
 
         self.fw.instance_rules(instance_ref,
                                mox.IgnoreArg()).AndReturn((None, None))
@@ -6170,6 +6172,8 @@ class IptablesFirewallTestCase(test.TestCase):
                                          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())
         self.mox.ReplayAll()
@@ -6178,6 +6182,26 @@ class IptablesFirewallTestCase(test.TestCase):
         self.fw.instances[instance_ref['id']] = instance_ref
         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)
+        mock_filter = mock.MagicMock()
+        with mock.patch.dict(self.fw.iptables.ipv4, {'filter': mock_filter}):
+            mock_filter.has_chain.return_value = False
+            with mock.patch.object(self.fw, 'instance_rules') as mock_ir:
+                mock_ir.return_value = (None, None)
+                self.fw.do_refresh_security_group_rules('secgroup')
+                self.assertEqual(2, mock_ir.call_count)
+            # NOTE(danms): Make sure that it is checking has_chain each time,
+            # continuing to process all the instances, and never adding the
+            # new chains back if has_chain() is False
+            mock_filter.has_chain.assert_has_calls([mock.call('inst-0'),
+                                                    mock.call('inst-1')],
+                                                   any_order=True)
+            self.assertEqual(0, mock_filter.add_chain.call_count)
+
     def test_unfilter_instance_undefines_nwfilter(self):
         admin_ctxt = context.get_admin_context()
 
diff --git a/nova/virt/firewall.py b/nova/virt/firewall.py
index 1c1cd0aab1..f4929c3d50 100644
--- a/nova/virt/firewall.py
+++ b/nova/virt/firewall.py
@@ -441,6 +441,13 @@ class IptablesFirewallDriver(FirewallDriver):
     @utils.synchronized('iptables', external=True)
     def _inner_do_refresh_rules(self, instance, ipv4_rules,
                                                ipv6_rules):
+        chain_name = self._instance_chain_name(instance)
+        if not self.iptables.ipv4['filter'].has_chain(chain_name):
+            LOG.info(
+                _('instance chain %s disappeared during refresh, '
+                    'skipping') % chain_name,
+                instance=instance)
+            return
         self.remove_filters_for_instance(instance)
         self.add_filters_for_instance(instance, ipv4_rules, ipv6_rules)