From 67af7c736ca6666ae9756e084d60b15f6f0f311e Mon Sep 17 00:00:00 2001 From: Dan Wendlandt Date: Mon, 1 Aug 2011 10:11:16 -0700 Subject: [PATCH] fix incorrect handling of duplicate network name, add exception for duplicate network name, and add unit test to confirm detection. --- quantum/api/faults.py | 16 +++++++++++ quantum/api/networks.py | 13 ++++++--- quantum/common/exceptions.py | 5 ++++ quantum/db/api.py | 47 ++++++++++++++++----------------- quantum/plugins/SamplePlugin.py | 5 +--- tests/unit/test_api.py | 19 +++++++++++++ 6 files changed, 73 insertions(+), 32 deletions(-) diff --git a/quantum/api/faults.py b/quantum/api/faults.py index ef4d50bc6..52b36109d 100644 --- a/quantum/api/faults.py +++ b/quantum/api/faults.py @@ -31,6 +31,7 @@ class Fault(webob.exc.HTTPException): 401: "unauthorized", 420: "networkNotFound", 421: "networkInUse", + 422: "networkNameExists", 430: "portNotFound", 431: "requestedStateInvalid", 432: "portInUse", @@ -90,6 +91,21 @@ class NetworkInUse(webob.exc.HTTPClientError): title = 'Network in Use' explanation = ('Unable to remove the network: attachments still plugged.') +class NetworkNameExists(webob.exc.HTTPClientError): + """ + subclass of :class:`~HTTPClientError` + + This indicates that the server could not set the network name to the + specified value because another network for the same tenant already has + that name. + + code: 422, title: Network Name Exists + """ + code = 422 + title = 'Network Name Exists' + explanation = ('Unable to set network name: tenant already has network' \ + ' with same name.') + class PortNotFound(webob.exc.HTTPClientError): """ diff --git a/quantum/api/networks.py b/quantum/api/networks.py index 9afc09c24..27709eb59 100644 --- a/quantum/api/networks.py +++ b/quantum/api/networks.py @@ -107,12 +107,15 @@ class Controller(common.QuantumController): self._network_ops_param_list) except exc.HTTPError as e: return faults.Fault(e) - network = self._plugin.\ + try: + network = self._plugin.\ create_network(tenant_id, request_params['net-name']) - builder = networks_view.get_view_builder(request) - result = builder.build(network) - return dict(networks=result) + builder = networks_view.get_view_builder(request) + result = builder.build(network) + return dict(networks=result) + except exception.NetworkNameExists as e: + return faults.Fault(faults.NetworkNameExists(e)) def update(self, request, tenant_id, id): """ Updates the name for the network with the given id """ @@ -128,6 +131,8 @@ class Controller(common.QuantumController): return exc.HTTPAccepted() except exception.NetworkNotFound as e: return faults.Fault(faults.NetworkNotFound(e)) + except exception.NetworkNameExists as e: + return faults.Fault(faults.NetworkNameExists(e)) def delete(self, request, tenant_id, id): """ Destroys the network with the given id """ diff --git a/quantum/common/exceptions.py b/quantum/common/exceptions.py index 4daf31762..b9705fb75 100644 --- a/quantum/common/exceptions.py +++ b/quantum/common/exceptions.py @@ -107,6 +107,11 @@ class AlreadyAttached(QuantumException): "%(port_id)s for network %(net_id)s. The attachment is " \ "already plugged into port %(att_port_id)s") +class NetworkNameExists(QuantumException): + message = _("Unable to set network name to %(net_name). " \ + "Network with id %(net_id) already has this name for " \ + "tenant %(tenant_id)") + class Duplicate(Error): pass diff --git a/quantum/db/api.py b/quantum/db/api.py index 0c42bda3d..5711a47dd 100644 --- a/quantum/db/api.py +++ b/quantum/db/api.py @@ -76,21 +76,28 @@ def unregister_models(): BASE.metadata.drop_all(_ENGINE) -def network_create(tenant_id, name): +def _check_duplicate_net_name(tenant_id, net_name): session = get_session() - net = None try: net = session.query(models.Network).\ - filter_by(tenant_id=tenant_id, name=name).\ + filter_by(tenant_id=tenant_id, name=net_name).\ one() - raise Exception("Network with name %(name)s already " \ - "exists for tenant %(tenant_id)s" % locals()) + raise q_exc.NetworkNameExists(tenant_id=tenant_id, + net_name=net_name, net_id=net.uuid) except exc.NoResultFound: - with session.begin(): - net = models.Network(tenant_id, name) - session.add(net) - session.flush() - return net + # this is the "normal" path, as API spec specifies + # that net-names are unique within a tenant + pass + +def network_create(tenant_id, name): + session = get_session() + + _check_duplicate_net_name(tenant_id, name) + with session.begin(): + net = models.Network(tenant_id, name) + session.add(net) + session.flush() + return net def network_list(tenant_id): @@ -112,20 +119,12 @@ def network_get(net_id): def network_rename(net_id, tenant_id, new_name): session = get_session() - try: - res = session.query(models.Network).\ - filter_by(tenant_id=tenant_id, name=new_name).\ - one() - if not res: - raise exc.NetworkNotFound(net_id=net_id) - except exc.NoResultFound: - net = network_get(net_id) - net.name = new_name - session.merge(net) - session.flush() - return net - raise Exception("A network with name \"%s\" already exists" % new_name) - + net = network_get(net_id) + _check_duplicate_net_name(tenant_id, new_name) + net.name = new_name + session.merge(net) + session.flush() + return net def network_destroy(net_id): session = get_session() diff --git a/quantum/plugins/SamplePlugin.py b/quantum/plugins/SamplePlugin.py index f4aefac0a..9b155a2c1 100644 --- a/quantum/plugins/SamplePlugin.py +++ b/quantum/plugins/SamplePlugin.py @@ -322,10 +322,7 @@ class FakePlugin(object): Virtual Network. """ LOG.debug("FakePlugin.rename_network() called") - try: - db.network_rename(net_id, tenant_id, new_name) - except: - raise exc.NetworkNotFound(net_id=net_id) + db.network_rename(net_id, tenant_id, new_name) net = self._get_network(tenant_id, net_id) return net diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index d17e6ed87..c52f6cefa 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -152,6 +152,19 @@ class APITest(unittest.TestCase): network_data['network']) LOG.debug("_test_rename_network - format:%s - END", format) + def _test_rename_network_duplicate(self, format): + LOG.debug("_test_rename_network_duplicate - format:%s - START", format) + content_type = "application/%s" % format + network_id1 = self._create_network(format, name="net1") + network_id2 = self._create_network(format, name="net2") + update_network_req = testlib.update_network_request(self.tenant_id, + network_id2, + "net1", + format) + update_network_res = update_network_req.get_response(self.api) + self.assertEqual(update_network_res.status_int, 422) + LOG.debug("_test_rename_network_duplicate - format:%s - END", format) + def _test_rename_network_badrequest(self, format): LOG.debug("_test_rename_network_badrequest - format:%s - START", format) @@ -693,6 +706,12 @@ class APITest(unittest.TestCase): def test_rename_network_xml(self): self._test_rename_network('xml') + def test_rename_network_duplicate_json(self): + self._test_rename_network_duplicate('json') + + def test_rename_network_duplicate_xml(self): + self._test_rename_network_duplicate('xml') + def test_rename_network_badrequest_json(self): self._test_rename_network_badrequest('json') -- 2.45.2