api: Add response body schemas for host aggregate actions API

Change-Id: Id6644a943c7ac735e3abf072bdc78674645945a4
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

nova/api/openstack/compute/aggregates.py

@@ -23,7 +23,7 @@ from webob import exc
 from nova.api.openstack import api_version_request
 from nova.api.openstack import common
 from nova.api.openstack.compute.schemas import aggregate_images
-from nova.api.openstack.compute.schemas import aggregates
+from nova.api.openstack.compute.schemas import aggregates as schema
 from nova.api.openstack import wsgi
 from nova.api import validation
 from nova.compute import api as compute
@@ -49,7 +49,7 @@ class AggregateController(wsgi.Controller):
         self.conductor_tasks = conductor.ComputeTaskAPI()
 
     @wsgi.expected_errors(())
-    @validation.query_schema(aggregates.index_query)
+    @validation.query_schema(schema.index_query)
     def index(self, req):
         """Returns a list a host aggregate's id, name, availability_zone."""
         context = _get_context(req)
@@ -61,8 +61,8 @@ class AggregateController(wsgi.Controller):
     # NOTE(gmann): Returns 200 for backwards compatibility but should be 201
     # as this operation complete the creation of aggregates resource.
     @wsgi.expected_errors((400, 409))
-    @validation.schema(aggregates.create_v20, '2.0', '2.0')
-    @validation.schema(aggregates.create, '2.1')
+    @validation.schema(schema.create_v20, '2.0', '2.0')
+    @validation.schema(schema.create, '2.1')
     def create(self, req, body):
         """Creates an aggregate, given its name and
         optional availability zone.
@@ -95,7 +95,7 @@ class AggregateController(wsgi.Controller):
         return agg
 
     @wsgi.expected_errors((400, 404))
-    @validation.query_schema(aggregates.show_query)
+    @validation.query_schema(schema.show_query)
     def show(self, req, id):
         """Shows the details of an aggregate, hosts and metadata included."""
         context = _get_context(req)
@@ -113,8 +113,8 @@ class AggregateController(wsgi.Controller):
         return self._marshall_aggregate(req, aggregate)
 
     @wsgi.expected_errors((400, 404, 409))
-    @validation.schema(aggregates.update_v20, '2.0', '2.0')
-    @validation.schema(aggregates.update, '2.1')
+    @validation.schema(schema.update_v20, '2.0', '2.0')
+    @validation.schema(schema.update, '2.1')
     def update(self, req, id, body):
         """Updates the name and/or availability_zone of given aggregate."""
         context = _get_context(req)
@@ -165,7 +165,9 @@ class AggregateController(wsgi.Controller):
     # request hypervisor driver to complete the same in async mode.
     @wsgi.expected_errors((400, 404, 409))
     @wsgi.action('add_host')
-    @validation.schema(aggregates.add_host)
+    @validation.schema(schema.add_host)
+    @validation.response_body_schema(schema.add_host_response, '2.1', '2.40')  # noqa: E501
+    @validation.response_body_schema(schema.add_host_response_v241, '2.41')
     def _add_host(self, req, id, body):
         """Adds a host to the specified aggregate."""
         host = body['add_host']['host']
@@ -194,7 +196,9 @@ class AggregateController(wsgi.Controller):
     # request hypervisor driver to complete the same in async mode.
     @wsgi.expected_errors((400, 404, 409))
     @wsgi.action('remove_host')
-    @validation.schema(aggregates.remove_host)
+    @validation.schema(schema.remove_host)
+    @validation.response_body_schema(schema.remove_host_response, '2.1', '2.40')  # noqa: E501
+    @validation.response_body_schema(schema.remove_host_response_v241, '2.41')
     def _remove_host(self, req, id, body):
         """Removes a host from the specified aggregate."""
         host = body['remove_host']['host']
@@ -226,7 +230,9 @@ class AggregateController(wsgi.Controller):
 
     @wsgi.expected_errors((400, 404))
     @wsgi.action('set_metadata')
-    @validation.schema(aggregates.set_metadata)
+    @validation.schema(schema.set_metadata)
+    @validation.response_body_schema(schema.set_metadata_response, '2.1', '2.40')  # noqa: E501
+    @validation.response_body_schema(schema.set_metadata_response_v241, '2.41')
     def _set_metadata(self, req, id, body):
         """Replaces the aggregate's existing metadata with new metadata."""
         context = _get_context(req)

nova/api/openstack/compute/schemas/aggregates.py

@@ -16,13 +16,12 @@ import copy
 
 from nova.api.validation import parameter_types
 
-availability_zone = {'oneOf': [parameter_types.az_name, {'type': 'null'}]}
-availability_zone_with_leading_trailing_spaces = {
+_availability_zone = {'oneOf': [parameter_types.az_name, {'type': 'null'}]}
+_availability_zone_with_leading_trailing_spaces = {
     'oneOf': [parameter_types.az_name_with_leading_trailing_spaces,
               {'type': 'null'}]
 }
 
-
 create = {
     'type': 'object',
     'properties': {
@@ -30,7 +29,7 @@ create = {
             'type': 'object',
             'properties': {
                 'name': parameter_types.name,
-                'availability_zone': availability_zone,
+                'availability_zone': _availability_zone,
             },
             'required': ['name'],
             'additionalProperties': False,
@@ -42,10 +41,10 @@ create = {
 
 
 create_v20 = copy.deepcopy(create)
-create_v20['properties']['aggregate']['properties']['name'] = (parameter_types.
-    name_with_leading_trailing_spaces)
+create_v20['properties']['aggregate']['properties']['name'] = (
+    parameter_types.name_with_leading_trailing_spaces)
 create_v20['properties']['aggregate']['properties']['availability_zone'] = (
-    availability_zone_with_leading_trailing_spaces)
+    _availability_zone_with_leading_trailing_spaces)
 
 
 update = {
@@ -55,7 +54,7 @@ update = {
             'type': 'object',
             'properties': {
                 'name': parameter_types.name_with_leading_trailing_spaces,
-                'availability_zone': availability_zone
+                'availability_zone': _availability_zone
             },
             'additionalProperties': False,
             'anyOf': [
@@ -70,10 +69,10 @@ update = {
 
 
 update_v20 = copy.deepcopy(update)
-update_v20['properties']['aggregate']['properties']['name'] = (parameter_types.
-    name_with_leading_trailing_spaces)
+update_v20['properties']['aggregate']['properties']['name'] = (
+    parameter_types.name_with_leading_trailing_spaces)
 update_v20['properties']['aggregate']['properties']['availability_zone'] = (
-    availability_zone_with_leading_trailing_spaces)
+    _availability_zone_with_leading_trailing_spaces)
 
 
 add_host = {
@@ -108,7 +107,6 @@ remove_host = {
     'additionalProperties': False,
 }
 
-
 set_metadata = {
     'type': 'object',
     'properties': {
@@ -138,3 +136,72 @@ show_query = {
     'properties': {},
     'additionalProperties': True,
 }
+
+_aggregate_response = {
+    'type': 'object',
+    'properties': {
+        'aggregate': {
+            'type': 'object',
+            'properties': {
+                'availability_zone': {'type': ['null', 'string']},
+                'created_at': {'type': 'string', 'format': 'date-time'},
+                'deleted': {'type': 'boolean'},
+                'deleted_at': {
+                    'oneOf': [
+                        {'type': 'null'},
+                        {'type': 'string', 'format': 'date-time'},
+                    ],
+                },
+                'hosts': {
+                    'type': ['array', 'null'],
+                    'items': {
+                        'type': 'string',
+                    },
+                },
+                'id': {'type': 'integer'},
+                # TODO(stephenfin): This should be stricter
+                'metadata': {
+                    'type': ['null', 'object'],
+                    'properties': {},
+                    'additionalProperties': True,
+                },
+                'name': {'type': 'string'},
+                'updated_at': {
+                    'oneOf': [
+                        {'type': 'null'},
+                        {'type': 'string', 'format': 'date-time'},
+                    ],
+                },
+            },
+            'required': [
+                'availability_zone',
+                'created_at',
+                'deleted',
+                'deleted_at',
+                'hosts',
+                'id',
+                'metadata',
+                'name',
+                'updated_at',
+            ],
+            'additionalProperties': False,
+        },
+    },
+    'required': ['aggregate'],
+    'additionalProperties': False,
+}
+
+_aggregate_response_v241 = copy.deepcopy(_aggregate_response)
+_aggregate_response_v241['properties']['aggregate']['properties'].update(
+    {'uuid': {'type': 'string', 'format': 'uuid'}},
+)
+_aggregate_response_v241['properties']['aggregate']['required'].append('uuid')
+
+add_host_response = copy.deepcopy(_aggregate_response)
+add_host_response_v241 = copy.deepcopy(_aggregate_response_v241)
+
+remove_host_response = copy.deepcopy(_aggregate_response)
+remove_host_response_v241 = copy.deepcopy(_aggregate_response_v241)
+
+set_metadata_response = copy.deepcopy(_aggregate_response)
+set_metadata_response_v241 = copy.deepcopy(_aggregate_response_v241)

nova/tests/unit/api/openstack/compute/test_aggregates.py

@@ -15,6 +15,7 @@
 
 """Tests for the aggregates admin api."""
 
+import datetime
 from unittest import mock
 
 from oslo_utils.fixture import uuidsentinel
@@ -32,7 +33,19 @@ from nova.tests.unit.api.openstack import fakes
 
 
 def _make_agg_obj(agg_dict):
-    return objects.Aggregate(**agg_dict)
+    _aggregate = {
+        "name": "aggregate1",
+        "id": "1",
+        "uuid": uuidsentinel.aggregate,
+        "metadata": {"availability_zone": "nova1"},
+        "hosts": ["host1", "host2"],
+        "created_at": datetime.datetime(2012, 11, 14, 9, 53, 25, 0),
+        "deleted": False,
+        "deleted_at": None,
+        "updated_at": None,
+    }
+    _aggregate.update(**agg_dict)
+    return objects.Aggregate(**_aggregate)
 
 
 def _make_agg_list(agg_list):
@@ -68,11 +81,12 @@ AGGREGATE_LIST = [
             "metadata": {"availability_zone": "nova1"}}]
 AGGREGATE_LIST = _make_agg_list(AGGREGATE_LIST)
 
-AGGREGATE = {"name": "aggregate1",
-                  "id": "1",
-                  "metadata": {"foo": "bar",
-                               "availability_zone": "nova1"},
-                  "hosts": ["host1", "host2"]}
+AGGREGATE = {
+    "name": "aggregate1",
+    "id": "1",
+    "metadata": {"foo": "bar", "availability_zone": "nova1"},
+    "hosts": ["host1", "host2"],
+}
 AGGREGATE = _make_agg_obj(AGGREGATE)
 
 FORMATTED_AGGREGATE = {"name": "aggregate1",
@@ -706,7 +720,7 @@ class AggregateTestCaseV21(test.NoDBTestCase):
                'id': 1, 'uuid': uuidsentinel.aggregate,
                'metadata': {'foo': 'bar', 'availability_zone': 'nova'},
                'hosts': ['host1', 'host2']}
-        agg_obj = _make_agg_obj(agg)
+        agg_obj = objects.Aggregate(**agg)
 
         # _marshall_aggregate() puts all fields and obj_extra_fields in the
         # top-level dict, so we need to put availability_zone at the top also

nova/tests/unit/policies/test_aggregates.py

@@ -13,6 +13,7 @@
 from unittest import mock
 
 from oslo_utils.fixture import uuidsentinel as uuids
+from oslo_utils import timeutils
 
 from nova.api.openstack.compute import aggregates
 from nova import objects
@@ -39,6 +40,20 @@ class AggregatesPolicyTest(base.BasePolicyTest):
             self.legacy_admin_context, self.system_admin_context,
             self.project_admin_context]
 
+        now = timeutils.utcnow()
+        self.fake_aggregate = objects.Aggregate(
+            **{
+                "created_at": now,
+                "name": "aggregate1",
+                "id": "1",
+                "metadata": {'availability_zone': 'nova1'},
+                "hosts": ["host1", "host2"],
+                "deleted": False,
+                "deleted_at": None,
+                "updated_at": now,
+            }
+        )
+
     @mock.patch('nova.compute.api.AggregateAPI.get_aggregate_list')
     def test_list_aggregate_policy(self, mock_list):
         rule_name = "os_compute_api:os-aggregates:index"
@@ -49,10 +64,7 @@ class AggregatesPolicyTest(base.BasePolicyTest):
     @mock.patch('nova.compute.api.AggregateAPI.create_aggregate')
     def test_create_aggregate_policy(self, mock_create):
         rule_name = "os_compute_api:os-aggregates:create"
-        mock_create.return_value = objects.Aggregate(**{"name": "aggregate1",
-                                   "id": "1",
-                                   "metadata": {'availability_zone': 'nova1'},
-                                   "hosts": ["host1", "host2"]})
+        mock_create.return_value = self.fake_aggregate
         body = {"aggregate": {"name": "test",
                               "availability_zone": "nova1"}}
         self.common_policy_auth(self.project_admin_authorized_contexts,
@@ -63,6 +75,7 @@ class AggregatesPolicyTest(base.BasePolicyTest):
     @mock.patch('nova.compute.api.AggregateAPI.update_aggregate')
     def test_update_aggregate_policy(self, mock_update):
         rule_name = "os_compute_api:os-aggregates:update"
+        mock_update.return_value = self.fake_aggregate
         self.common_policy_auth(self.project_admin_authorized_contexts,
                                 rule_name, self.controller.update,
                                 self.req, 1,
@@ -71,6 +84,7 @@ class AggregatesPolicyTest(base.BasePolicyTest):
     @mock.patch('nova.compute.api.AggregateAPI.delete_aggregate')
     def test_delete_aggregate_policy(self, mock_delete):
         rule_name = "os_compute_api:os-aggregates:delete"
+        mock_delete.return_value = None
         self.common_policy_auth(self.project_admin_authorized_contexts,
                                 rule_name,
                                 self.controller.delete,
@@ -79,6 +93,7 @@ class AggregatesPolicyTest(base.BasePolicyTest):
     @mock.patch('nova.compute.api.AggregateAPI.get_aggregate')
     def test_show_aggregate_policy(self, mock_show):
         rule_name = "os_compute_api:os-aggregates:show"
+        mock_show.return_value = self.fake_aggregate
         self.common_policy_auth(self.project_admin_authorized_contexts,
                                 rule_name, self.controller.show,
                                 self.req, 1)
@@ -86,6 +101,7 @@ class AggregatesPolicyTest(base.BasePolicyTest):
     @mock.patch('nova.compute.api.AggregateAPI.update_aggregate_metadata')
     def test_set_metadata_aggregate_policy(self, mock_metadata):
         rule_name = "os_compute_api:os-aggregates:set_metadata"
+        mock_metadata.return_value = self.fake_aggregate
         body = {"set_metadata": {"metadata": {"foo": "bar"}}}
         self.common_policy_auth(self.project_admin_authorized_contexts,
                                 rule_name,
@@ -95,6 +111,7 @@ class AggregatesPolicyTest(base.BasePolicyTest):
     @mock.patch('nova.compute.api.AggregateAPI.add_host_to_aggregate')
     def test_add_host_aggregate_policy(self, mock_add):
         rule_name = "os_compute_api:os-aggregates:add_host"
+        mock_add.return_value = self.fake_aggregate
         self.common_policy_auth(self.project_admin_authorized_contexts,
                                 rule_name, self.controller._add_host,
                                 self.req, 1,
@@ -103,6 +120,7 @@ class AggregatesPolicyTest(base.BasePolicyTest):
     @mock.patch('nova.compute.api.AggregateAPI.remove_host_from_aggregate')
     def test_remove_host_aggregate_policy(self, mock_remove):
         rule_name = "os_compute_api:os-aggregates:remove_host"
+        mock_remove.return_value = self.fake_aggregate
         self.common_policy_auth(self.project_admin_authorized_contexts,
                                 rule_name,
                                 self.controller._remove_host,