openstack/nova
Code Issues Proposed changes

Fix errors in os-networks extension

Browse Source 
* Makes sure the uuid is returned as id if it exists
 * Simplifies db get for manager.get_networks
 * Removes direct db access from manager which was breaking test
 * Updates tests to verify the new logic
 * Makes sure Remote NotFounds are turned into 404s
   (The RemoteError blocks can be removed once
    https://review.openstack.org/5749 lands)
 * Fixes bug 977712
 * Fixes bug 977723

Change-Id: I6aa815960782c7ae5165aeebd83bdaaa62c19b04
This commit is contained in:
Vishvananda Ishaya 2012-04-09 22:11:10 -07:00
parent 7a1957dfe9
commit a0e37c6d29
5 changed files with 65 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
nova/api/openstack/compute/contrib/networks.py
View File

@ -24,6 +24,7 @@ from nova import exception
from nova import flags from nova import flags
from nova import log as logging from nova import log as logging
import nova.network.api import nova.network.api
from nova.rpc import common
FLAGS = flags.FLAGS FLAGS = flags.FLAGS
@ -40,7 +41,10 @@ def network_dict(network):
'netmask', 'injected', 'cidr', 'vpn_public_address', 'netmask', 'injected', 'cidr', 'vpn_public_address',
'multi_host', 'dns1', 'host', 'gateway_v6', 'netmask_v6', 'multi_host', 'dns1', 'host', 'gateway_v6', 'netmask_v6',
'created_at') 'created_at')
return dict((field, network[field]) for field in fields) result = dict((field, network[field]) for field in fields)
if 'uuid' in network:
result['id'] = network['uuid']
return result
else: else:
return {} return {}
@ -72,6 +76,11 @@ class NetworkController(object):
self.network_api.disassociate(context, network_id) self.network_api.disassociate(context, network_id)
except exception.NetworkNotFound: except exception.NetworkNotFound:
raise exc.HTTPNotFound(_("Network not found")) raise exc.HTTPNotFound(_("Network not found"))
except common.RemoteError as ex:
if ex.exc_type in ["NetworkNotFound", "NetworkNotFoundForUUID"]:
raise exc.HTTPNotFound(_("Network not found"))
else:
raise
return exc.HTTPAccepted() return exc.HTTPAccepted()
def index(self, req): def index(self, req):
@ -89,6 +98,11 @@ class NetworkController(object):
network = self.network_api.get(context, id) network = self.network_api.get(context, id)
except exception.NetworkNotFound: except exception.NetworkNotFound:
raise exc.HTTPNotFound(_("Network not found")) raise exc.HTTPNotFound(_("Network not found"))
except common.RemoteError as ex:
if ex.exc_type in ["NetworkNotFound", "NetworkNotFoundForUUID"]:
raise exc.HTTPNotFound(_("Network not found"))
else:
raise
return {'network': network_dict(network)} return {'network': network_dict(network)}
def delete(self, req, id): def delete(self, req, id):
@ -99,6 +113,11 @@ class NetworkController(object):
self.network_api.delete(context, id) self.network_api.delete(context, id)
except exception.NetworkNotFound: except exception.NetworkNotFound:
raise exc.HTTPNotFound(_("Network not found")) raise exc.HTTPNotFound(_("Network not found"))
except common.RemoteError as ex:
if ex.exc_type in ["NetworkNotFound", "NetworkNotFoundForUUID"]:
raise exc.HTTPNotFound(_("Network not found"))
else:
raise
return exc.HTTPAccepted() return exc.HTTPAccepted()
def create(self, req, id, body=None): def create(self, req, id, body=None):

17
nova/network/manager.py
View File

@ -55,7 +55,6 @@ import netaddr
from nova.compute import api as compute_api from nova.compute import api as compute_api
from nova import context from nova import context
from nova import db
from nova import exception from nova import exception
from nova import flags from nova import flags
from nova import ipv6 from nova import ipv6
@ -1412,15 +1411,16 @@ class NetworkManager(manager.SchedulerDependentManager):
require_disassociated=True): require_disassociated=True):
# Prefer uuid but we'll also take cidr for backwards compatibility # Prefer uuid but we'll also take cidr for backwards compatibility
elevated = context.elevated()
if uuid: if uuid:
network = db.network_get_by_uuid(context.elevated(), uuid) network = self.db.network_get_by_uuid(elevated, uuid)
elif fixed_range: elif fixed_range:
network = db.network_get_by_cidr(context.elevated(), fixed_range) network = self.db.network_get_by_cidr(elevated, fixed_range)
if require_disassociated and network.project_id is not None: if require_disassociated and network.project_id is not None:
raise ValueError(_('Network must be disassociated from project %s' raise ValueError(_('Network must be disassociated from project %s'
' before delete') % network.project_id) ' before delete') % network.project_id)
db.network_delete_safe(context, network.id) self.db.network_delete_safe(context, network.id)
@property @property
def _bottom_reserved_ips(self): # pylint: disable=R0201 def _bottom_reserved_ips(self): # pylint: disable=R0201
@ -1557,12 +1557,7 @@ class NetworkManager(manager.SchedulerDependentManager):
@wrap_check_policy @wrap_check_policy
def get_network(self, context, network_uuid): def get_network(self, context, network_uuid):
networks = self._get_networks_by_uuids(context, [network_uuid]) network = self.db.network_get_by_uuid(context.elevated(), network_uuid)
try:
network = networks[0]
except (IndexError, TypeError):
raise exception.NetworkNotFound(network_id=network_uuid)
return dict(network.iteritems()) return dict(network.iteritems())
@wrap_check_policy @wrap_check_policy
@ -1846,7 +1841,7 @@ class VlanManager(RPCAllocateFixedIP, FloatingIP, NetworkManager):
net = {} net = {}
address = FLAGS.vpn_ip address = FLAGS.vpn_ip
net['vpn_public_address'] = address net['vpn_public_address'] = address
network = db.network_update(context, network['id'], net) network = self.db.network_update(context, network['id'], net)
else: else:
address = network['vpn_public_address'] address = network['vpn_public_address']
network['dhcp_server'] = self._get_dhcp_ip(context, network) network['dhcp_server'] = self._get_dhcp_ip(context, network)

34
nova/tests/api/openstack/compute/contrib/test_networks.py
View File

@ -20,6 +20,7 @@ import webob
from nova.api.openstack.compute.contrib import networks from nova.api.openstack.compute.contrib import networks
from nova import exception from nova import exception
from nova.rpc import common
from nova import test from nova import test
from nova.tests.api.openstack import fakes from nova.tests.api.openstack import fakes
@ -28,7 +29,8 @@ FAKE_NETWORKS = [
{ {
'bridge': 'br100', 'vpn_public_port': 1000, 'bridge': 'br100', 'vpn_public_port': 1000,
'dhcp_start': '10.0.0.3', 'bridge_interface': 'eth0', 'dhcp_start': '10.0.0.3', 'bridge_interface': 'eth0',
'updated_at': '2011-08-16 09:26:13.048257', 'id': 1, 'updated_at': '2011-08-16 09:26:13.048257',
'id': 1, 'uuid': '20c8acc0-f747-4d71-a389-46d078ebf047',
'cidr_v6': None, 'deleted_at': None, 'cidr_v6': None, 'deleted_at': None,
'gateway': '10.0.0.1', 'label': 'mynet_0', 'gateway': '10.0.0.1', 'label': 'mynet_0',
'project_id': '1234', 'project_id': '1234',
@ -68,21 +70,21 @@ class FakeNetworkAPI(object):
if network['id'] == network_id: if network['id'] == network_id:
del self.networks[0] del self.networks[0]
return True return True
raise exception.NetworkNotFound() raise exception.NetworkNotFoundForUUID()
#NOTE(bcwaldon): this does nothing other than check for existance #NOTE(bcwaldon): this does nothing other than check for existance
def disassociate(self, context, network_id): def disassociate(self, context, network_id):
for i, network in enumerate(self.networks): for i, network in enumerate(self.networks):
if network['id'] == network_id: if network.get('uuid') == network_id:
return True return True
raise exception.NetworkNotFound() raise common.RemoteError(type(exception.NetworkNotFound()).__name__)
def get_all(self, context): def get_all(self, context):
return self.networks return self.networks
def get(self, context, network_id): def get(self, context, network_id):
for network in self.networks: for network in self.networks:
if network['id'] == network_id: if network.get('uuid') == network_id:
return network return network
raise exception.NetworkNotFound() raise exception.NetworkNotFound()
@ -99,11 +101,15 @@ class NetworksTest(test.TestCase):
def test_network_list_all(self): def test_network_list_all(self):
req = fakes.HTTPRequest.blank('/v2/1234/os-networks') req = fakes.HTTPRequest.blank('/v2/1234/os-networks')
res_dict = self.controller.index(req) res_dict = self.controller.index(req)
self.assertEquals(res_dict, {'networks': FAKE_NETWORKS}) expected = copy.deepcopy(FAKE_NETWORKS)
expected[0]['id'] = expected[0]['uuid']
del expected[0]['uuid']
self.assertEquals(res_dict, {'networks': expected})
def test_network_disassociate(self): def test_network_disassociate(self):
req = fakes.HTTPRequest.blank('/v2/1234/os-networks/1/action') uuid = FAKE_NETWORKS[0]['uuid']
res = self.controller.action(req, 1, {'disassociate': None}) req = fakes.HTTPRequest.blank('/v2/1234/os-networks/%s/action' % uuid)
res = self.controller.action(req, uuid, {'disassociate': None})
self.assertEqual(res.status_int, 202) self.assertEqual(res.status_int, 202)
def test_network_disassociate_not_found(self): def test_network_disassociate_not_found(self):
@ -113,9 +119,12 @@ class NetworksTest(test.TestCase):
req, 100, {'disassociate': None}) req, 100, {'disassociate': None})
def test_network_get(self): def test_network_get(self):
req = fakes.HTTPRequest.blank('/v2/1234/os-networks/1') uuid = FAKE_NETWORKS[0]['uuid']
res_dict = self.controller.show(req, 1) req = fakes.HTTPRequest.blank('/v2/1234/os-networks/%s' % uuid)
expected = {'network': FAKE_NETWORKS[0]} res_dict = self.controller.show(req, uuid)
expected = {'network': copy.deepcopy(FAKE_NETWORKS[0])}
expected['network']['id'] = expected['network']['uuid']
del expected['network']['uuid']
self.assertEqual(res_dict, expected) self.assertEqual(res_dict, expected)
def test_network_get_not_found(self): def test_network_get_not_found(self):
@ -124,7 +133,8 @@ class NetworksTest(test.TestCase):
self.controller.show, req, 100) self.controller.show, req, 100)
def test_network_delete(self): def test_network_delete(self):
req = fakes.HTTPRequest.blank('/v2/1234/os-networks/1') uuid = FAKE_NETWORKS[0]['uuid']
req = fakes.HTTPRequest.blank('/v2/1234/os-networks/%s' % uuid)
res = self.controller.delete(req, 1) res = self.controller.delete(req, 1)
self.assertEqual(res.status_int, 202) self.assertEqual(res.status_int, 202)

3
nova/tests/fake_network.py
View File

@ -120,6 +120,9 @@ class FakeNetworkManager(network_manager.NetworkManager):
def network_get(self, context, network_id): def network_get(self, context, network_id):
return {'cidr_v6': '2001:db8:69:%x::/64' % network_id} return {'cidr_v6': '2001:db8:69:%x::/64' % network_id}
def network_get_by_uuid(self, context, network_uuid):
raise exception.NetworkNotFoundForUUID()
def network_get_all(self, context): def network_get_all(self, context):
raise exception.NoNetworksFound() raise exception.NoNetworksFound()

28
nova/tests/network/test_manager.py
View File

@ -1266,10 +1266,9 @@ class CommonNetworkTestCase(test.TestCase):
def test_get_network(self): def test_get_network(self):
manager = fake_network.FakeNetworkManager() manager = fake_network.FakeNetworkManager()
fake_context = context.RequestContext('user', 'project') fake_context = context.RequestContext('user', 'project')
self.mox.StubOutWithMock(manager.db, 'network_get_all_by_uuids') self.mox.StubOutWithMock(manager.db, 'network_get_by_uuid')
manager.db.network_get_all_by_uuids( manager.db.network_get_by_uuid(mox.IgnoreArg(),
mox.IgnoreArg(), mox.IgnoreArg()).AndReturn(networks[0])
mox.IgnoreArg()).AndReturn(networks)
self.mox.ReplayAll() self.mox.ReplayAll()
uuid = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' uuid = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'
network = manager.get_network(fake_context, uuid) network = manager.get_network(fake_context, uuid)
@ -1278,9 +1277,10 @@ class CommonNetworkTestCase(test.TestCase):
def test_get_network_not_found(self): def test_get_network_not_found(self):
manager = fake_network.FakeNetworkManager() manager = fake_network.FakeNetworkManager()
fake_context = context.RequestContext('user', 'project') fake_context = context.RequestContext('user', 'project')
self.mox.StubOutWithMock(manager.db, 'network_get_all_by_uuids') self.mox.StubOutWithMock(manager.db, 'network_get_by_uuid')
manager.db.network_get_all_by_uuids(mox.IgnoreArg(), manager.db.network_get_by_uuid(
mox.IgnoreArg()).AndReturn([]) mox.IgnoreArg(),
mox.IgnoreArg()).AndRaise(exception.NetworkNotFoundForUUID)
self.mox.ReplayAll() self.mox.ReplayAll()
uuid = 'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee' uuid = 'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee'
self.assertRaises(exception.NetworkNotFound, self.assertRaises(exception.NetworkNotFound,
@ -1302,10 +1302,9 @@ class CommonNetworkTestCase(test.TestCase):
def test_disassociate_network(self): def test_disassociate_network(self):
manager = fake_network.FakeNetworkManager() manager = fake_network.FakeNetworkManager()
fake_context = context.RequestContext('user', 'project') fake_context = context.RequestContext('user', 'project')
self.mox.StubOutWithMock(manager.db, 'network_get_all_by_uuids') self.mox.StubOutWithMock(manager.db, 'network_get_by_uuid')
manager.db.network_get_all_by_uuids( manager.db.network_get_by_uuid(mox.IgnoreArg(),
mox.IgnoreArg(), mox.IgnoreArg()).AndReturn(networks[0])
mox.IgnoreArg()).AndReturn(networks)
self.mox.ReplayAll() self.mox.ReplayAll()
uuid = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' uuid = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'
manager.disassociate_network(fake_context, uuid) manager.disassociate_network(fake_context, uuid)
@ -1313,9 +1312,10 @@ class CommonNetworkTestCase(test.TestCase):
def test_disassociate_network_not_found(self): def test_disassociate_network_not_found(self):
manager = fake_network.FakeNetworkManager() manager = fake_network.FakeNetworkManager()
fake_context = context.RequestContext('user', 'project') fake_context = context.RequestContext('user', 'project')
self.mox.StubOutWithMock(manager.db, 'network_get_all_by_uuids') self.mox.StubOutWithMock(manager.db, 'network_get_by_uuid')
manager.db.network_get_all_by_uuids(mox.IgnoreArg(), manager.db.network_get_by_uuid(
mox.IgnoreArg()).AndReturn([]) mox.IgnoreArg(),
mox.IgnoreArg()).AndRaise(exception.NetworkNotFoundForUUID)
self.mox.ReplayAll() self.mox.ReplayAll()
uuid = 'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee' uuid = 'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee'
self.assertRaises(exception.NetworkNotFound, self.assertRaises(exception.NetworkNotFound,