Code review comment for lp:~jtran/nova/lp794651

Can we match the type of the inner exception as well? I don't know if we want to raise NoFloatingIpsDefined every time we get a remote error from network.

Vish

On Jun 8, 2011, at 5:22 PM, John Tran wrote:

> John Tran has proposed merging lp:~jtran/nova/lp794651 into lp:nova.
>
> Requested reviews:
> Nova Core (nova-core)
> Related bugs:
> Bug #794651 in OpenStack Compute (nova): "nova api allocate_address returns 'UnknownError' when no addresses available"
> https://bugs.launchpad.net/nova/+bug/794651
>
> For more details, see:
> https://code.launchpad.net/~jtran/nova/lp794651/+merge/63943
> --
> https://code.launchpad.net/~jtran/nova/lp794651/+merge/63943
> You are subscribed to branch lp:nova.
> === modified file 'nova/api/ec2/cloud.py'
> --- nova/api/ec2/cloud.py 2011-06-07 17:47:40 +0000
> +++ nova/api/ec2/cloud.py 2011-06-09 00:22:24 +0000
> @@ -39,6 +39,7 @@
> from nova import ipv6
> from nova import log as logging
> from nova import network
> +from nova import rpc
> from nova import utils
> from nova import volume
> from nova.api.ec2 import ec2utils
> @@ -872,8 +873,11 @@
>
> def allocate_address(self, context, **kwargs):
> LOG.audit(_("Allocate address"), context=context)
> - public_ip = self.network_api.allocate_floating_ip(context)
> - return {'publicIp': public_ip}
> + try:
> + public_ip = self.network_api.allocate_floating_ip(context)
> + return {'publicIp': public_ip}
> + except rpc.RemoteError:
> + raise exception.NoFloatingIpsDefined
>
> def release_address(self, context, public_ip, **kwargs):
> LOG.audit(_("Release address %s"), public_ip, context=context)
>
> === modified file 'nova/tests/test_cloud.py'
> --- nova/tests/test_cloud.py 2011-06-07 20:05:03 +0000
> +++ nova/tests/test_cloud.py 2011-06-09 00:22:24 +0000
> @@ -115,6 +115,17 @@
> public_ip=address)
> db.floating_ip_destroy(self.context, address)
>
> + def test_allocate_address(self):
> + address = "10.10.10.10"
> + allocate = self.cloud.allocate_address
> + db.floating_ip_create(self.context,
> + {'address': address,
> + 'host': self.network.host})
> + self.assertEqual(allocate(self.context)['publicIp'], address)
> + db.floating_ip_destroy(self.context, address)
> + self.assertRaises(exception.NoFloatingIpsDefined, allocate,
> + self.context)
> +
> def test_associate_disassociate_address(self):
> """Verifies associate runs cleanly without raising an exception"""
> address = "10.10.10.10"
>