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

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}
 
 

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)