Merge lp:~soren/nova/lp704351 into lp:~hudson-openstack/nova/trunk

[Soren Hansen]

Add an apply_instance_filter method to NWFilter driver.

Adjust unit tests for both firewall drivers to actually exercise these
code paths.

[Devin Carlen (devcamcar)]

lgtm

review: Approve

[Vish Ishaya (vishvananda)]

lgtm

review: Approve

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~soren/nova/lp704351 into lp:nova failed. Below is the output from the failed tests.

nova/tests/test_virt.py:356:80: E501 line too long (80 characters)
                        ' 8 -j ACCEPT' % security_group_chain in self.out_rules,
                                                                               ^
    Limit all lines to a maximum of 79 characters.

    There are still many devices around that are limited to 80 character
    lines; plus, limiting windows to 80 characters makes it possible to have
    several windows side-by-side. The default wrapping on such devices looks
    ugly. Therefore, please limit all lines to a maximum of 79 characters.
    For flowing long blocks of text (docstrings or comments), limiting the
    length to 72 characters is recommended.
nova/tests/test_virt.py:499:1: W391 blank line at end of file

^
    JCR: Trailing blank lines are superfluous.

    Okay: spam(1)
    W391: spam(1)\n

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~soren/nova/lp704351 into lp:nova failed. Below is the output from the failed tests.

nova/tests/test_virt.py:356:80: E501 line too long (80 characters)
                        ' 8 -j ACCEPT' % security_group_chain in self.out_rules,
                                                                               ^
    Limit all lines to a maximum of 79 characters.

    There are still many devices around that are limited to 80 character
    lines; plus, limiting windows to 80 characters makes it possible to have
    several windows side-by-side. The default wrapping on such devices looks
    ugly. Therefore, please limit all lines to a maximum of 79 characters.
    For flowing long blocks of text (docstrings or comments), limiting the
    length to 72 characters is recommended.
nova/tests/test_virt.py:499:1: W391 blank line at end of file

^
    JCR: Trailing blank lines are superfluous.

    Okay: spam(1)
    W391: spam(1)\n

=== modified file 'nova/tests/test_virt.py'
--- nova/tests/test_virt.py	2011-01-18 20:42:20 +0000
+++ nova/tests/test_virt.py	2011-01-19 09:28:10 +0000
@@ -228,12 +228,6 @@
         self.manager.delete_user(self.user)
         super(IptablesFirewallTestCase, self).tearDown()
 
-    def _p(self, *args, **kwargs):
-        if 'iptables-restore' in args:
-            print ' '.join(args), kwargs['stdin']
-        if 'iptables-save' in args:
-            return
-
     in_rules = [
       '# Generated by iptables-save v1.4.4 on Mon Dec  6 11:54:13 2010',
       '*filter',
@@ -255,11 +249,21 @@
       '# Completed on Mon Dec  6 11:54:13 2010',
     ]
 
+    in6_rules = [
+      '# Generated by ip6tables-save v1.4.4 on Tue Jan 18 23:47:56 2011',
+      '*filter',
+      ':INPUT ACCEPT [349155:75810423]',
+      ':FORWARD ACCEPT [0:0]',
+      ':OUTPUT ACCEPT [349256:75777230]',
+      'COMMIT',
+      '# Completed on Tue Jan 18 23:47:56 2011'
+    ]
+
     def test_static_filters(self):
-        self.fw.execute = self._p
         instance_ref = db.instance_create(self.context,
                                           {'user_id': 'fake',
-                                          'project_id': 'fake'})
+                                          'project_id': 'fake',
+                                          'mac_address': '56:12:12:12:12:12'})
         ip = '10.11.12.13'
 
         network_ref = db.project_get_network(self.context,
@@ -304,18 +308,31 @@
                                        secgroup['id'])
         instance_ref = db.instance_get(admin_ctxt, instance_ref['id'])
 
-        self.fw.add_instance(instance_ref)
+#        self.fw.add_instance(instance_ref)
+        def fake_iptables_execute(cmd, process_input=None):
+            if cmd == 'sudo ip6tables-save -t filter':
+                return '\n'.join(self.in6_rules), None
+            if cmd == 'sudo iptables-save -t filter':
+                return '\n'.join(self.in_rules), None
+            if cmd == 'sudo iptables-restore':
+                self.out_rules = process_input.split('\n')
+                return '', ''
+            if cmd == 'sudo ip6tables-restore':
+                self.out6_rules = process_input.split('\n')
+                return '', ''
+        self.fw.execute = fake_iptables_execute
 
-        out_rules = self.fw.modify_rules(self.in_rules)
+        self.fw.prepare_instance_filter(instance_ref)
+        self.fw.apply_instance_filter(instance_ref)
 
         in_rules = filter(lambda l: not l.startswith('#'), self.in_rules)
         for rule in in_rules:
             if not 'nova' in rule:
-                self.assertTrue(rule in out_rules,
+                self.assertTrue(rule in self.out_rules,
                                 'Rule went missing: %s' % rule)
 
         instance_chain = None
-        for rule in out_rules:
+        for rule in self.out_rules:
             # This is pretty crude, but it'll do for now
             if '-d 10.11.12.13 -j' in rule:
                 instance_chain = rule.split(' ')[-1]
@@ -323,7 +340,7 @@
         self.assertTrue(instance_chain, "The instance chain wasn't added")
 
         security_group_chain = None
-        for rule in out_rules:
+        for rule in self.out_rules:
             # This is pretty crude, but it'll do for now
             if '-A %s -j' % instance_chain in rule:
                 security_group_chain = rule.split(' ')[-1]
@@ -332,16 +349,16 @@
                         "The security group chain wasn't added")
 
         self.assertTrue('-A %s -p icmp -s 192.168.11.0/24 -j ACCEPT' % \
-                               security_group_chain in out_rules,
+                               security_group_chain in self.out_rules,
                         "ICMP acceptance rule wasn't added")
 
-        self.assertTrue('-A %s -p icmp -s 192.168.11.0/24 -m icmp --icmp-type'
-                        ' 8 -j ACCEPT' % security_group_chain in out_rules,
+        self.assertTrue('-A %s -p icmp -s 192.168.11.0/24 -m icmp --icmp-type '
+                        '8 -j ACCEPT' % security_group_chain in self.out_rules,
                         "ICMP Echo Request acceptance rule wasn't added")
 
         self.assertTrue('-A %s -p tcp -s 192.168.10.0/24 -m multiport '
                         '--dports 80:81 -j ACCEPT' % security_group_chain \
-                            in out_rules,
+                            in self.out_rules,
                         "TCP port 80/81 acceptance rule wasn't added")
 
 
@@ -476,5 +493,6 @@
 
         self.fw.setup_basic_filtering(instance)
         self.fw.prepare_instance_filter(instance)
+        self.fw.apply_instance_filter(instance)
         _ensure_all_called()
         self.teardown_security_group()

=== modified file 'nova/virt/libvirt_conn.py'
--- nova/virt/libvirt_conn.py	2011-01-18 22:55:03 +0000
+++ nova/virt/libvirt_conn.py	2011-01-19 09:28:10 +0000
@@ -1121,6 +1121,10 @@
 
         return
 
+    def apply_instance_filter(self, instance):
+        """No-op. Everything is done in prepare_instance_filter"""
+        pass
+
     def refresh_security_group_rules(self, security_group_id):
         return self._define_filter(
                    self.security_group_to_nwfilter_xml(security_group_id))