Merge lp:~vishvananda/nova/automatic-metadata into lp:~hudson-openstack/nova/trunk

[Vish Ishaya]

Automatically add the metadata address to the network host. This allows guests to ARP for the address properly.

I also uncovered an issue with moving the gateway. Apparently specifying route add 0.0.0.0 doesn't actually work, you have use route add 'default'. I also added a line to specifically delete the old gateway since it doesn't always automatically get deleted.

[Soren Hansen (soren)]

2011/4/7 Vish Ishaya <email address hidden>:
> === modified file 'nova/network/linux_net.py'
> --- nova/network/linux_net.py   2011-03-25 02:04:13 +0000
> +++ nova/network/linux_net.py   2011-04-06 23:14:24 +0000
> @@ -391,6 +391,13 @@
>              'dev', FLAGS.public_interface)
>
>
> +def ensure_metadata_ip(interface):
> +    """Sets up local metadata ip"""
> +    _execute('sudo', 'ip', 'addr', 'add', '169.254.169.254/32',
> +             'scope', 'link', 'dev', interface,
> +             check_exit_code=False)
> +
> +

Does the interface really matter? What if it were "lo"?

> @@ -504,7 +514,7 @@
>                 _execute(*_ip_bridge_cmd('del', params, fields[-1]))
>                 _execute(*_ip_bridge_cmd('add', params, bridge))
>         if gateway:
> -            _execute('sudo', 'route', 'add', '0.0.0.0', 'gw', gateway)
> +            _execute('sudo', 'route', 'add', 'default', 'gw', gateway)

I never really noticed this bit of code before. We really ought to
apply policy routing here. It's perfectly reasonable to have the VM's
use one routing table and the host use another. I've filed bug #753280
about this.

"route" is a dreadful interface for modifying routes, but if we're
using it, you should be passing "-net" as you're not adding a host
route. It must be special casing "default" and behind the scenes
pretending like you passed "-net" as well, since even 0.0.0.0/0 does
not work.

--
Soren Hansen        | http://linux2go.dk/
Ubuntu Developer    | http://www.ubuntu.com/
OpenStack Developer | http://www.openstack.org/

[Chuck Short (zulcss)]

looks good to me

[Vish Ishaya (vishvananda)]

On Apr 7, 2011, at 1:21 AM, Soren Hansen wrote:

>
> Does the interface really matter? What if it were "lo"?

I haven't tested adding the ip to the loopback interface. Do you want me to give it a shot?

>
> I never really noticed this bit of code before. We really ought to
> apply policy routing here. It's perfectly reasonable to have the VM's
> use one routing table and the host use another. I've filed bug #753280
> about this.
>
> "route" is a dreadful interface for modifying routes, but if we're
> using it, you should be passing "-net" as you're not adding a host
> route. It must be special casing "default" and behind the scenes
> pretending like you passed "-net" as well, since even 0.0.0.0/0 does
> not work.

Ah good point, i was wondering why 0.0.0.0/0 wasn't working.

>
> --
> Soren Hansen | http://linux2go.dk/
> Ubuntu Developer | http://www.ubuntu.com/
> OpenStack Developer | http://www.openstack.org/
>
> https://code.launchpad.net/~vishvananda/nova/automatic-metadata/+merge/56672
> You are the owner of lp:~vishvananda/nova/automatic-metadata.

[lp:~vishvananda/nova/automatic-metadata]

951. By Vish Ishaya on 2011-04-07

Simplify by always adding to loopback

[Vish Ishaya (vishvananda)]

Tested loopback, it does work. The flag seems extraneous now, so I removed it. It automatically adds the address to the loopback device now.

[Soren Hansen (soren)]

> > "route" is a dreadful interface for modifying routes, but if we're
> > using it, you should be passing "-net" as you're not adding a host
> > route. It must be special casing "default" and behind the scenes
> > pretending like you passed "-net" as well, since even 0.0.0.0/0 does
> > not work.
> Ah good point, i was wondering why 0.0.0.0/0 wasn't working.

There's probably a (not necessarily good) reason why, but it seems that a route to "0.0.0.0/0" really ends up being a route to "0.0.0.0/32" which really isn't very useful. meh.

[Soren Hansen (soren)]

> Tested loopback, it does work. The flag seems extraneous now, so I removed
> it. It automatically adds the address to the loopback device now.

Excellent. lgtm now.

review: Approve

[Devin Carlen (devcamcar)]

great patch, lgtm

review: Approve

=== modified file 'nova/network/linux_net.py'
--- nova/network/linux_net.py	2011-03-25 02:04:13 +0000
+++ nova/network/linux_net.py	2011-04-07 17:39:32 +0000
@@ -391,6 +391,12 @@
              'dev', FLAGS.public_interface)
 
 
+def ensure_metadata_ip():
+    """Sets up local metadata ip"""
+    _execute('sudo', 'ip', 'addr', 'add', '169.254.169.254/32',
+             'scope', 'link', 'dev', 'lo', check_exit_code=False)
+
+
 def ensure_vlan_forward(public_ip, port, private_ip):
     """Sets up forwarding rules for vlan"""
     iptables_manager.ipv4['filter'].add_rule("FORWARD",
@@ -442,6 +448,7 @@
     return interface
 
 
[e-mail]('ensure_bridge', external=True)
 def ensure_bridge(bridge, interface, net_attrs=None):
     """Create a bridge unless it already exists.
 
@@ -495,6 +502,8 @@
             fields = line.split()
             if fields and fields[0] == "0.0.0.0" and fields[-1] == interface:
                 gateway = fields[1]
+                _execute('sudo', 'route', 'del', 'default', 'gw', gateway,
+                         'dev', interface)
         out, err = _execute('sudo', 'ip', 'addr', 'show', 'dev', interface,
                             'scope', 'global')
         for line in out.split("\n"):
@@ -504,7 +513,7 @@
                 _execute(*_ip_bridge_cmd('del', params, fields[-1]))
                 _execute(*_ip_bridge_cmd('add', params, bridge))
         if gateway:
-            _execute('sudo', 'route', 'add', '0.0.0.0', 'gw', gateway)
+            _execute('sudo', 'route', 'add', 'default', 'gw', gateway)
         out, err = _execute('sudo', 'brctl', 'addif', bridge, interface,
                             check_exit_code=False)
 

=== modified file 'nova/network/manager.py'
--- nova/network/manager.py	2011-03-25 02:04:13 +0000
+++ nova/network/manager.py	2011-04-07 17:39:32 +0000
@@ -126,6 +126,7 @@
         standalone service.
         """
         self.driver.init_host()
+        self.driver.ensure_metadata_ip()
         # Set up networking for the projects for which we're already
         # the designated network host.
         ctxt = context.get_admin_context()