Merge lp:~ntt-pf-lab/nova/lp703037 into lp:~hudson-openstack/nova/trunk

[Hisaharu Ishii]

Fix Bug #703037. ra_server is None

Fix for LP Bug #703037
* update_ra() updates ra_server column every time
* When radvd is already running, not to hup, but to restart
* Fix prefixlen of RA server address in firewall rule from /64 to /128
* Fix apply_instance_filter is not implemented in NWFilterFirewall

[Vish Ishaya (vishvananda)]

I notice that you are using project_get_network to get the ra server for the project. This will not work properly in flatDHCP mode which doesn't have the project set. This needs to be changed to db.network_get_by_instance. I also noticed there are a few other references to this method that need to be changed, so I've put in an additional bug and fix here:

https://code.launchpad.net/~vishvananda/nova/lp707262

 so you might want to merge that branch as well and propose with that branch as a prerequisite

review: Needs Fixing

[Hisaharu Ishii (ishii-hisaharu)]

>Vish Ishaya

I have tried to merge your branch to my branch.
But in UnitTest, this error was occurred.
Now I'm working with this issue.

======================================================================
ERROR: test_creates_base_rule_first (nova.tests.test_virt.NWFilterTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/ishii/20110125/lp703037/nova/tests/test_virt.py", line 495, in test_creates_base_rule_first
    self.fw.prepare_instance_filter(instance)
  File "/home/ishii/20110125/lp703037/nova/virt/libvirt_conn.py", line 1103, in prepare_instance_filter
    ra_server = self._ra_server_for_instance(instance)
  File "/home/ishii/20110125/lp703037/nova/virt/libvirt_conn.py", line 886, in _ra_server_for_instance
    instance['id'])
  File "/home/ishii/20110125/lp703037/nova/db/api.py", line 517, in network_get_by_instance
    return IMPL.network_get_by_instance(context, instance_id)
  File "/home/ishii/20110125/lp703037/nova/db/sqlalchemy/api.py", line 96, in wrapper
    return f(*args, **kwargs)
  File "/home/ishii/20110125/lp703037/nova/db/sqlalchemy/api.py", line 1097, in network_get_by_instance
    raise exception.NotFound(_('No network for instance %s') % instance_id)
NotFound: No network for instance 92

[Vish Ishaya (vishvananda)]

You can use the same strategy for getting and allocating an ip that is used
by other tests test_virt if there is no other way to get the data in
prepare_instance_filter

On Mon, Jan 24, 2011 at 10:53 PM, Hisaharu Ishii <
<email address hidden>> wrote:

> >Vish Ishaya
>
> I have tried to merge your branch to my branch.
> But in UnitTest, this error was occurred.
> Now I'm working with this issue.
>
> ======================================================================
> ERROR: test_creates_base_rule_first (nova.tests.test_virt.NWFilterTestCase)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "/home/ishii/20110125/lp703037/nova/tests/test_virt.py", line 495, in
> test_creates_base_rule_first
> self.fw.prepare_instance_filter(instance)
> File "/home/ishii/20110125/lp703037/nova/virt/libvirt_conn.py", line 1103,
> in prepare_instance_filter
> ra_server = self._ra_server_for_instance(instance)
> File "/home/ishii/20110125/lp703037/nova/virt/libvirt_conn.py", line 886,
> in _ra_server_for_instance
> instance['id'])
> File "/home/ishii/20110125/lp703037/nova/db/api.py", line 517, in
> network_get_by_instance
> return IMPL.network_get_by_instance(context, instance_id)
> File "/home/ishii/20110125/lp703037/nova/db/sqlalchemy/api.py", line 96,
> in wrapper
> return f(*args, **kwargs)
> File "/home/ishii/20110125/lp703037/nova/db/sqlalchemy/api.py", line 1097,
> in network_get_by_instance
> raise exception.NotFound(_('No network for instance %s') % instance_id)
> NotFound: No network for instance 92
>
> --
> https://code.launchpad.net/~ntt-pf-lab/nova/lp703037/+merge/47028
> You are reviewing the proposed merge of lp:~ntt-pf-lab/nova/lp703037 into
> lp:nova.
>

[Vish Ishaya (vishvananda)]

like so:
https://code.launchpad.net/~vishvananda/nova/lp703037

Vish

On Jan 25, 2011, at 12:07 AM, Vish Ishaya wrote:

> You can use the same strategy for getting and allocating an ip that is used
> by other tests test_virt if there is no other way to get the data in
> prepare_instance_filter
>
> On Mon, Jan 24, 2011 at 10:53 PM, Hisaharu Ishii <
> <email address hidden>> wrote:
>
>>> Vish Ishaya
>>
>> I have tried to merge your branch to my branch.
>> But in UnitTest, this error was occurred.
>> Now I'm working with this issue.
>>
>> ======================================================================
>> ERROR: test_creates_base_rule_first (nova.tests.test_virt.NWFilterTestCase)
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>> File "/home/ishii/20110125/lp703037/nova/tests/test_virt.py", line 495, in
>> test_creates_base_rule_first
>> self.fw.prepare_instance_filter(instance)
>> File "/home/ishii/20110125/lp703037/nova/virt/libvirt_conn.py", line 1103,
>> in prepare_instance_filter
>> ra_server = self._ra_server_for_instance(instance)
>> File "/home/ishii/20110125/lp703037/nova/virt/libvirt_conn.py", line 886,
>> in _ra_server_for_instance
>> instance['id'])
>> File "/home/ishii/20110125/lp703037/nova/db/api.py", line 517, in
>> network_get_by_instance
>> return IMPL.network_get_by_instance(context, instance_id)
>> File "/home/ishii/20110125/lp703037/nova/db/sqlalchemy/api.py", line 96,
>> in wrapper
>> return f(*args, **kwargs)
>> File "/home/ishii/20110125/lp703037/nova/db/sqlalchemy/api.py", line 1097,
>> in network_get_by_instance
>> raise exception.NotFound(_('No network for instance %s') % instance_id)
>> NotFound: No network for instance 92
>>
>> --
>> https://code.launchpad.net/~ntt-pf-lab/nova/lp703037/+merge/47028
>> You are reviewing the proposed merge of lp:~ntt-pf-lab/nova/lp703037 into
>> lp:nova.
>>
>
> --
> https://code.launchpad.net/~ntt-pf-lab/nova/lp703037/+merge/47028
> You are reviewing the proposed merge of lp:~ntt-pf-lab/nova/lp703037 into lp:nova.

[lp:~ntt-pf-lab/nova/lp703037]

580. By Nachi Ueno on 2011-01-25

Merged with r606

581. By Hisaharu Ishii on 2011-01-26

Merged with vish branch

582. By Nachi Ueno on 2011-01-26

Merged with http://bazaar.launchpad.net/~vishvananda/nova/lp703037

583. By Nachi Ueno on 2011-01-26

merged r621

584. By Nachi Ueno on 2011-01-26

Changed method signature of create_network

[Nachi Ueno (nati-ueno)]

>Vish
Thank you for your helpful code.
We fixed #703037 and #705811 in this branch.
We also merge this branch with r621.
Unit test is OK now.

[lp:~ntt-pf-lab/nova/lp703037]

585. By Hisaharu Ishii on 2011-01-26

Fix merge miss

586. By Hisaharu Ishii on 2011-01-26

use 'ip addr change'

[Hisaharu Ishii (ishii-hisaharu)]

I updated our branch for
* Fix some merge miss in r583
* Fix Bug #707908

[Vish Ishaya (vishvananda)]

lgtm

review: Approve

[Soren Hansen (soren)]

lgtm

review: Approve

=== modified file 'bin/nova-manage'
--- bin/nova-manage	2011-01-25 23:29:30 +0000
+++ bin/nova-manage	2011-01-26 12:27:54 +0000
@@ -495,9 +495,9 @@
                                     cidr=fixed_range,
                                     num_networks=int(num_networks),
                                     network_size=int(network_size),
+                                    cidr_v6=fixed_range_v6,
                                     vlan_start=int(vlan_start),
-                                    vpn_start=int(vpn_start),
-                                    cidr_v6=fixed_range_v6)
+                                    vpn_start=int(vpn_start))
 
 
 class ServiceCommands(object):

=== modified file 'nova/network/linux_net.py'
--- nova/network/linux_net.py	2011-01-17 18:05:26 +0000
+++ nova/network/linux_net.py	2011-01-26 12:27:54 +0000
@@ -198,9 +198,9 @@
                  net_attrs['broadcast'],
                  net_attrs['netmask']))
         if(FLAGS.use_ipv6):
-            _execute("sudo ifconfig %s add %s up" % \
-                     (bridge,
-                      net_attrs['cidr_v6']))
+            _execute("sudo ip -f inet6 addr change %s dev %s" %
+                     (net_attrs['cidr_v6'], bridge))
+            _execute("sudo ifconfig %s up" % bridge)
     else:
         _execute("sudo ifconfig %s up" % bridge)
     if FLAGS.use_nova_chains:
@@ -298,10 +298,9 @@
                              % pid, check_exit_code=False)
         if conffile in out:
             try:
-                _execute('sudo kill -HUP %d' % pid)
-                return
+                _execute('sudo kill %d' % pid)
             except Exception as exc:  # pylint: disable-msg=W0703
-                LOG.debug(_("Hupping radvd threw %s"), exc)
+                LOG.debug(_("killing radvd threw %s"), exc)
         else:
             LOG.debug(_("Pid %d is stale, relaunching radvd"), pid)
     command = _ra_cmd(network_ref)

=== modified file 'nova/network/manager.py'
--- nova/network/manager.py	2011-01-25 22:53:09 +0000
+++ nova/network/manager.py	2011-01-26 12:27:54 +0000
@@ -428,6 +428,10 @@
         self.driver.ensure_bridge(network_ref['bridge'],
                                   FLAGS.flat_interface,
                                   network_ref)
+        if not FLAGS.fake_network:
+            self.driver.update_dhcp(context, network_id)
+            if(FLAGS.use_ipv6):
+                self.driver.update_ra(context, network_id)
 
 
 class VlanManager(NetworkManager):
@@ -497,7 +501,7 @@
                                        network_ref['bridge'])
 
     def create_networks(self, context, cidr, num_networks, network_size,
-                        vlan_start, vpn_start, cidr_v6):
+                        cidr_v6, vlan_start, vpn_start):
         """Create networks based on parameters."""
         fixed_net = IPy.IP(cidr)
         fixed_net_v6 = IPy.IP(cidr_v6)

=== modified file 'nova/test.py'
--- nova/test.py	2011-01-14 19:19:51 +0000
+++ nova/test.py	2011-01-26 12:27:54 +0000
@@ -69,9 +69,10 @@
             network_manager.VlanManager().create_networks(ctxt,
                                                           FLAGS.fixed_range,
                                                           5, 16,
+                                                          FLAGS.fixed_range_v6,
                                                           FLAGS.vlan_start,
                                                           FLAGS.vpn_start,
-                                                          FLAGS.fixed_range_v6)
+                                                          )
 
         # emulate some of the mox stuff, we can't use the metaclass
         # because it screws with our generators

=== modified file 'nova/tests/test_virt.py'
--- nova/tests/test_virt.py	2011-01-25 02:31:04 +0000
+++ nova/tests/test_virt.py	2011-01-26 12:27:54 +0000
@@ -474,6 +474,19 @@
                                           'project_id': 'fake'})
         inst_id = instance_ref['id']
 
+        ip = '10.11.12.13'
+
+        network_ref = db.project_get_network(self.context,
+                                             'fake')
+
+        fixed_ip = {'address': ip,
+                    'network_id': network_ref['id']}
+
+        admin_ctxt = context.get_admin_context()
+        db.fixed_ip_create(admin_ctxt, fixed_ip)
+        db.fixed_ip_update(admin_ctxt, ip, {'allocated': True,
+                                            'instance_id': instance_ref['id']})
+
         def _ensure_all_called():
             instance_filter = 'nova-instance-%s' % instance_ref['name']
             secgroup_filter = 'nova-secgroup-%s' % self.security_group['id']

=== modified file 'nova/utils.py'
--- nova/utils.py	2011-01-19 15:41:30 +0000
+++ nova/utils.py	2011-01-26 12:27:54 +0000
@@ -206,21 +206,17 @@
 def  get_my_linklocal(interface):
     try:
         if_str = execute("ip -f inet6 -o addr show %s" % interface)
-        condition = "\s+inet6\s+([0-9a-f:]+/\d+)\s+scope\s+link"
+        condition = "\s+inet6\s+([0-9a-f:]+)/\d+\s+scope\s+link"
         links = [re.search(condition, x) for x in if_str[0].split('\n')]
         address = [w.group(1) for w in links if w is not None]
         if address[0] is not None:
             return address[0]
         else:
-            return 'fe00::'
-    except IndexError as ex:
-        LOG.warn(_("Couldn't get Link Local IP of %(interface)s :%(ex)s")
-                % locals())
-    except ProcessExecutionError as ex:
-        LOG.warn(_("Couldn't get Link Local IP of %(interface)s :%(ex)s")
-                % locals())
-    except:
-        return 'fe00::'
+            raise exception.Error(_("Link Local address is not found.:%s")
+                                  % if_str)
+    except Exception as ex:
+        raise exception.Error(_("Couldn't get Link Local IP of %s :%s")
+                              % (interface, ex))
 
 
 def to_global_ipv6(prefix, mac):

=== modified file 'nova/virt/libvirt.xml.template'
--- nova/virt/libvirt.xml.template	2011-01-14 10:24:57 +0000
+++ nova/virt/libvirt.xml.template	2011-01-26 12:27:54 +0000
@@ -75,11 +75,13 @@
             <!--   <model type='virtio'/>  CANT RUN virtio network right now -->
             <filterref filter="nova-instance-${name}">
                 <parameter name="IP" value="${ip_address}" />
-                <parameter name="DHCPSERVER" value="${dhcp_server}" />
-                <parameter name="RASERVER" value="${ra_server}" />
+                <parameter name="DHCPSERVER" value="${dhcp_server}" />           
 #if $getVar('extra_params', False)
                 ${extra_params}
 #end if
+#if $getVar('ra_server', False)
+                <parameter name="RASERVER" value="${ra_server}" />
+#end if
             </filterref>
         </interface>
 

=== modified file 'nova/virt/libvirt_conn.py'
--- nova/virt/libvirt_conn.py	2011-01-25 23:20:55 +0000
+++ nova/virt/libvirt_conn.py	2011-01-26 12:27:54 +0000
@@ -678,8 +678,7 @@
         # Assume that the gateway also acts as the dhcp server.
         dhcp_server = network['gateway']
         ra_server = network['ra_server']
-        if not ra_server:
-            ra_server = 'fd00::'
+
         if FLAGS.allow_project_net_traffic:
             if FLAGS.use_ipv6:
                 net, mask = _get_net_and_mask(network['cidr'])
@@ -718,11 +717,13 @@
                     'mac_address': instance['mac_address'],
                     'ip_address': ip_address,
                     'dhcp_server': dhcp_server,
-                    'ra_server': ra_server,
                     'extra_params': extra_params,
                     'rescue': rescue,
                     'local': instance_type['local_gb'],
                     'driver_type': driver_type}
+
+        if ra_server:
+            xml_info['ra_server'] = ra_server + "/128"
         if not rescue:
             if instance['kernel_id']:
                 xml_info['kernel'] = xml_info['basepath'] + "/kernel"
@@ -905,6 +906,11 @@
         the security group."""
         raise NotImplementedError()
 
+    def _ra_server_for_instance(self, instance):
+        network = db.network_get_by_instance(context.get_admin_context(),
+                                             instance['id'])
+        return network['ra_server']
+
 
 class NWFilterFirewall(FirewallDriver):
     """
@@ -957,6 +963,10 @@
         self.static_filters_configured = False
         self.handle_security_groups = False
 
+    def apply_instance_filter(self, instance):
+        """No-op. Everything is done in prepare_instance_filter"""
+        pass
+
     def _get_connection(self):
         return self._libvirt_get_connection()
     _conn = property(_get_connection)
@@ -1115,7 +1125,9 @@
                                              'nova-base-ipv6',
                                              'nova-allow-dhcp-server']
         if FLAGS.use_ipv6:
-            instance_secgroup_filter_children += ['nova-allow-ra-server']
+            ra_server = self._ra_server_for_instance(instance)
+            if ra_server:
+                instance_secgroup_filter_children += ['nova-allow-ra-server']
 
         ctxt = context.get_admin_context()
 
@@ -1142,10 +1154,6 @@
 
         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))
@@ -1301,8 +1309,9 @@
             elif(ip_version == 6):
                 # Allow RA responses
                 ra_server = self._ra_server_for_instance(instance)
-                our_rules += ['-A %s -s %s -p icmpv6 '
-                                '-j ACCEPT' % (chain_name, ra_server)]
+                if ra_server:
+                    our_rules += ['-A %s -s %s -p icmpv6 -j ACCEPT' %
+                                  (chain_name, ra_server + "/128")]
                 #Allow project network traffic
                 if (FLAGS.allow_project_net_traffic):
                     cidrv6 = self._project_cidrv6_for_instance(instance)