From aff272eff9042698fe674e7ef80670627d26f50e Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Tue, 16 Jul 2013 14:50:59 +0800 Subject: [PATCH] Fix extensions agent follow API v3 rules * Add decorator expected_errors * Use AgentBuildExists instead of generic exception catching in create * Use HTTPBadRequest instead of HTTPUnprocessableEntity * Rename the request parameter 'para' to 'agent' for update Partially implements bp v3-api-extension-versioning DocImpact Change-Id: I45203203b8396517ef94e604366183ce93810205 --- .../openstack/compute/plugins/v3/agents.py | 26 +++- .../compute/plugins/v3/test_agents.py | 124 +++++++++++++++++- 2 files changed, 141 insertions(+), 9 deletions(-) diff --git a/nova/api/openstack/compute/plugins/v3/agents.py b/nova/api/openstack/compute/plugins/v3/agents.py index da6297c36dfd..6848dfa6dfd7 100644 --- a/nova/api/openstack/compute/plugins/v3/agents.py +++ b/nova/api/openstack/compute/plugins/v3/agents.py @@ -22,6 +22,7 @@ from nova.api.openstack import wsgi from nova.api.openstack import xmlutil from nova import db from nova import exception +from nova.openstack.common.gettextutils import _ ALIAS = "os-agents" @@ -66,6 +67,7 @@ class AgentController(object): http://wiki.openstack.org/GuestAgent http://wiki.openstack.org/GuestAgentXenStoreCommunication """ + @extensions.expected_errors(()) @wsgi.serializers(xml=AgentsIndexTemplate) def index(self, req): """ @@ -89,18 +91,22 @@ class AgentController(object): return {'agents': agents} + @extensions.expected_errors((400, 404)) def update(self, req, id, body): """Update an existing agent build.""" context = req.environ['nova.context'] authorize(context) try: - para = body['para'] + para = body['agent'] url = para['url'] md5hash = para['md5hash'] version = para['version'] - except (TypeError, KeyError): - raise webob.exc.HTTPUnprocessableEntity() + except TypeError as e: + raise webob.exc.HTTPBadRequest() + except KeyError as e: + raise webob.exc.HTTPBadRequest(explanation=_( + "Could not find %s parameter in the request") % e.args[0]) try: db.agent_build_update(context, id, @@ -113,6 +119,8 @@ class AgentController(object): return {"agent": {'agent_id': id, 'version': version, 'url': url, 'md5hash': md5hash}} + @extensions.expected_errors(404) + @wsgi.response(204) def delete(self, req, id): """Deletes an existing agent build.""" context = req.environ['nova.context'] @@ -123,6 +131,7 @@ class AgentController(object): except exception.AgentBuildNotFound as ex: raise webob.exc.HTTPNotFound(explanation=ex.format_message()) + @extensions.expected_errors((400, 409)) def create(self, req, body): """Creates a new agent build.""" context = req.environ['nova.context'] @@ -136,8 +145,11 @@ class AgentController(object): version = agent['version'] url = agent['url'] md5hash = agent['md5hash'] - except (TypeError, KeyError): - raise webob.exc.HTTPUnprocessableEntity() + except TypeError as e: + raise webob.exc.HTTPBadRequest() + except KeyError as e: + raise webob.exc.HTTPBadRequest(explanation=_( + "Could not find %s parameter in the request") % e.args[0]) try: agent_build_ref = db.agent_build_create(context, @@ -148,8 +160,8 @@ class AgentController(object): 'url': url, 'md5hash': md5hash}) agent['agent_id'] = agent_build_ref.id - except Exception as ex: - raise webob.exc.HTTPServerError(str(ex)) + except exception.AgentBuildExists as ex: + raise webob.exc.HTTPConflict(explanation=ex.format_message()) return {'agent': agent} diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_agents.py b/nova/tests/api/openstack/compute/plugins/v3/test_agents.py index d8b75744a9ac..840f55933332 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_agents.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_agents.py @@ -11,12 +11,13 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. - +from webob import exc from nova.api.openstack.compute.plugins.v3 import agents from nova import context from nova import db from nova.db.sqlalchemy import models +from nova import exception from nova import test fake_agents_list = [{'hypervisor': 'kvm', 'os': 'win', @@ -82,6 +83,10 @@ class FakeRequestWithHypervisor(object): GET = {'hypervisor': 'kvm'} +def fake_agent_build_create_with_exited_agent(context, values): + raise exception.AgentBuildExists(**values) + + class AgentsTest(test.TestCase): def setUp(self): @@ -116,6 +121,90 @@ class AgentsTest(test.TestCase): res_dict = self.controller.create(req, body) self.assertEqual(res_dict, response) + def test_agents_create_with_existed_agent(self): + self.stubs.Set(db, 'agent_build_create', + fake_agent_build_create_with_exited_agent) + req = FakeRequest() + body = {'agent': {'hypervisor': 'kvm', + 'os': 'win', + 'architecture': 'x86', + 'version': '7.0', + 'url': 'xxx://xxxx/xxx/xxx', + 'md5hash': 'add6bb58e139be103324d04d82d8f545'}} + self.assertRaises(exc.HTTPConflict, self.controller.create, req, body) + + def test_agents_create_without_md5hash(self): + req = FakeRequest() + body = {'agent': {'hypervisor': 'kvm', + 'os': 'win', + 'architecture': 'x86', + 'version': '7.0', + 'url': 'xxx://xxxx/xxx/xxx'}} + self.assertRaises(exc.HTTPBadRequest, self.controller.create, + req, body) + + def test_agents_create_without_url(self): + req = FakeRequest() + body = {'agent': {'hypervisor': 'kvm', + 'os': 'win', + 'architecture': 'x86', + 'version': '7.0', + 'md5hash': 'add6bb58e139be103324d04d82d8f545'}} + self.assertRaises(exc.HTTPBadRequest, self.controller.create, + req, body) + + def test_agents_create_without_version(self): + req = FakeRequest() + body = {'agent': {'hypervisor': 'kvm', + 'os': 'win', + 'architecture': 'x86', + 'url': 'xxx://xxxx/xxx/xxx', + 'md5hash': 'add6bb58e139be103324d04d82d8f545'}} + self.assertRaises(exc.HTTPBadRequest, self.controller.create, + req, body) + + def test_agents_create_without_architecture(self): + req = FakeRequest() + body = {'agent': {'hypervisor': 'kvm', + 'os': 'win', + 'version': '7.0', + 'url': 'xxx://xxxx/xxx/xxx', + 'md5hash': 'add6bb58e139be103324d04d82d8f545'}} + self.assertRaises(exc.HTTPBadRequest, self.controller.create, + req, body) + + def test_agents_create_without_os(self): + req = FakeRequest() + body = {'agent': {'hypervisor': 'kvm', + 'architecture': 'x86', + 'version': '7.0', + 'url': 'xxx://xxxx/xxx/xxx', + 'md5hash': 'add6bb58e139be103324d04d82d8f545'}} + self.assertRaises(exc.HTTPBadRequest, self.controller.create, + req, body) + + def test_agents_create_without_hypervisor(self): + req = FakeRequest() + body = {'agent': {'os': 'win', + 'architecture': 'x86', + 'version': '7.0', + 'url': 'xxx://xxxx/xxx/xxx', + 'md5hash': 'add6bb58e139be103324d04d82d8f545'}} + self.assertRaises(exc.HTTPBadRequest, self.controller.create, + req, body) + + def test_agents_create_with_wrong_type(self): + req = FakeRequest() + body = {'agent': None} + self.assertRaises(exc.HTTPBadRequest, self.controller.create, + req, body) + + def test_agents_create_with_empty_type(self): + req = FakeRequest() + body = {} + self.assertRaises(exc.HTTPBadRequest, self.controller.create, + req, body) + def test_agents_delete(self): req = FakeRequest() self.controller.delete(req, 1) @@ -170,7 +259,7 @@ class AgentsTest(test.TestCase): def test_agents_update(self): req = FakeRequest() - body = {'para': {'version': '7.0', + body = {'agent': {'version': '7.0', 'url': 'xxx://xxxx/xxx/xxx', 'md5hash': 'add6bb58e139be103324d04d82d8f545'}} response = {'agent': {'agent_id': 1, @@ -179,3 +268,34 @@ class AgentsTest(test.TestCase): 'md5hash': 'add6bb58e139be103324d04d82d8f545'}} res_dict = self.controller.update(req, 1, body) self.assertEqual(res_dict, response) + + def test_agents_update_without_md5hash(self): + req = FakeRequest() + body = {'agent': {'version': '7.0', + 'url': 'xxx://xxxx/xxx/xxx'}} + self.assertRaises(exc.HTTPBadRequest, self.controller.update, + req, 1, body) + + def test_agents_update_without_url(self): + req = FakeRequest() + body = {'agent': {'version': '7.0'}} + self.assertRaises(exc.HTTPBadRequest, self.controller.update, + req, 1, body) + + def test_agents_update_without_version(self): + req = FakeRequest() + body = {'agent': {}} + self.assertRaises(exc.HTTPBadRequest, self.controller.update, + req, 1, body) + + def test_agents_update_with_wrong_type(self): + req = FakeRequest() + body = {'agent': None} + self.assertRaises(exc.HTTPBadRequest, self.controller.update, + req, 1, body) + + def test_agents_update_with_empty(self): + req = FakeRequest() + body = {} + self.assertRaises(exc.HTTPBadRequest, self.controller.update, + req, 1, body)