From ee47beb3be98e4ba538a96b8dc688ab924c2445e Mon Sep 17 00:00:00 2001 From: Chris Dent Date: Mon, 20 Nov 2017 18:04:19 +0000 Subject: [PATCH] [placement] Object changes to support last-modified headers A variety of changes are required to allow the various entities presented by the placement API to have last-modified times. According to the HTTP 1.1 RFC headers last-modified headers SHOULD always be sent and should have a tie to the real last modified time. If we do send them, we need Cache-Control headers to prevent inadvertent caching of resources. This patch provides necessary changes to the database and object handling that will support the API changes made in a followup patch. The main steps are: * map base.NovaTimestampObject to ovo.TimestampedObject * Add the base.NovaTimestampObject mixin to the relevant object in nova/objects/resource_provider.py * Tweak queries to retrieve updated_at and created_at fields where they are not already present Note that only those objects which are directly represented in response bodies and directly associated with a database resource that has created_at and updated_at fields are changed (e.g., resource providers). Other objects, like usage and allocaiton candidates, which are composites and represent the state of the universe _now_, will use the current time when they get last-modified headers in the next patch. Some HTTP requests, such as GET /resource_providers/{uuid}/aggregates are based on a association that happens at a time that is not recorded and is ambiguous: should we tell last-modified time of the most recently created aggregate uuid, or the time when the association between a resource provider and an aggregate was made (which we don't know). In those cases where a solution is unclear, no object or database changes are made, and the next patch will use the current time in any related last-modified headers. Change-Id: I3f6310af9c5bea682e793d27d480952aa8776d61 Partial-Bug: #1632852 Partially-Implements: bp placement-cache-headers --- nova/db/sqlalchemy/resource_class_cache.py | 37 ++++++++++++++- nova/objects/base.py | 13 +---- nova/objects/resource_provider.py | 27 +++++++---- .../db/test_resource_class_cache.py | 47 +++++++++++++++++++ .../unit/objects/test_resource_provider.py | 6 +++ 5 files changed, 108 insertions(+), 22 deletions(-) diff --git a/nova/db/sqlalchemy/resource_class_cache.py b/nova/db/sqlalchemy/resource_class_cache.py index 4c6a6cc4e16a..2c2e25f5a7b1 100644 --- a/nova/db/sqlalchemy/resource_class_cache.py +++ b/nova/db/sqlalchemy/resource_class_cache.py @@ -48,10 +48,12 @@ def _refresh_from_db(ctx, cache): :param cache: ResourceClassCache object to refresh. """ with db_api.api_context_manager.reader.connection.using(ctx) as conn: - sel = sa.select([_RC_TBL.c.id, _RC_TBL.c.name]) + sel = sa.select([_RC_TBL.c.id, _RC_TBL.c.name, _RC_TBL.c.updated_at, + _RC_TBL.c.created_at]) res = conn.execute(sel).fetchall() cache.id_cache = {r[1]: r[0] for r in res} cache.str_cache = {r[0]: r[1] for r in res} + cache.all_cache = {r[1]: r for r in res} class ResourceClassCache(object): @@ -59,7 +61,8 @@ class ResourceClassCache(object): # List of dict of all standard resource classes, where every list item # have a form {'id': , 'name': } - STANDARDS = [{'id': fields.ResourceClass.STANDARD.index(s), 'name': s} + STANDARDS = [{'id': fields.ResourceClass.STANDARD.index(s), 'name': s, + 'updated_at': None, 'created_at': None} for s in fields.ResourceClass.STANDARD] def __init__(self, ctx): @@ -71,11 +74,13 @@ class ResourceClassCache(object): self.ctx = ctx self.id_cache = {} self.str_cache = {} + self.all_cache = {} def clear(self): with lockutils.lock(_LOCKNAME): self.id_cache = {} self.str_cache = {} + self.all_cache = {} def id_from_string(self, rc_str): """Given a string representation of a resource class -- e.g. "DISK_GB" @@ -107,6 +112,34 @@ class ResourceClassCache(object): return self.id_cache[rc_str] raise exception.ResourceClassNotFound(resource_class=rc_str) + def all_from_string(self, rc_str): + """Given a string representation of a resource class -- e.g. "DISK_GB" + or "CUSTOM_IRON_SILVER" -- return all the resource class info. + + :param rc_str: The string representation of the resource class for + which to look up a resource_class. + :returns: dict representing the resource class fields, if the + resource class was found in the list of standard + resource classes or the resource_classes database table. + :raises: `exception.ResourceClassNotFound` if rc_str cannot be found in + either the standard classes or the DB. + """ + # First check the standard resource classes + if rc_str in fields.ResourceClass.STANDARD: + return {'id': fields.ResourceClass.STANDARD.index(rc_str), + 'name': rc_str, + 'updated_at': None, + 'created_at': None} + + with lockutils.lock(_LOCKNAME): + if rc_str in self.all_cache: + return self.all_cache[rc_str] + # Otherwise, check the database table + _refresh_from_db(self.ctx, self) + if rc_str in self.all_cache: + return self.all_cache[rc_str] + raise exception.ResourceClassNotFound(resource_class=rc_str) + def string_from_id(self, rc_id): """The reverse of the id_from_string() method. Given a supplied numeric identifier for a resource class, we look up the corresponding string diff --git a/nova/objects/base.py b/nova/objects/base.py index 30fbe25627f2..315ae3012646 100644 --- a/nova/objects/base.py +++ b/nova/objects/base.py @@ -79,6 +79,7 @@ remotable_classmethod = ovoo_base.remotable_classmethod remotable = ovoo_base.remotable obj_make_list = ovoo_base.obj_make_list NovaObjectDictCompat = ovoo_base.VersionedObjectDictCompat +NovaTimestampObject = ovoo_base.TimestampedObject class NovaObject(ovoo_base.VersionedObject): @@ -139,18 +140,6 @@ class NovaObject(ovoo_base.VersionedObject): self._context = original_context -class NovaTimestampObject(object): - """Mixin class for db backed objects with timestamp fields. - - Sqlalchemy models that inherit from the oslo_db TimestampMixin will include - these fields and the corresponding objects will benefit from this mixin. - """ - fields = { - 'created_at': obj_fields.DateTimeField(nullable=True), - 'updated_at': obj_fields.DateTimeField(nullable=True), - } - - class NovaPersistentObject(object): """Mixin class for Persistent objects. diff --git a/nova/objects/resource_provider.py b/nova/objects/resource_provider.py index 9d1c6bca36f2..47943f0e858c 100644 --- a/nova/objects/resource_provider.py +++ b/nova/objects/resource_provider.py @@ -44,6 +44,7 @@ _TRAIT_TBL = models.Trait.__table__ _ALLOC_TBL = models.Allocation.__table__ _INV_TBL = models.Inventory.__table__ _RP_TBL = models.ResourceProvider.__table__ +# Not used in this file but used in tests. _RC_TBL = models.ResourceClass.__table__ _AGG_TBL = models.PlacementAggregate.__table__ _RP_AGG_TBL = models.ResourceProviderAggregate.__table__ @@ -404,6 +405,8 @@ def _get_provider_by_uuid(context, uuid): rpt.c.generation, root.c.uuid.label("root_provider_uuid"), parent.c.uuid.label("parent_provider_uuid"), + rpt.c.updated_at, + rpt.c.created_at, ] sel = sa.select(cols).select_from(rp_to_parent).where(rpt.c.uuid == uuid) res = context.session.execute(sel).fetchone() @@ -490,7 +493,8 @@ def _get_traits_by_provider_id(context, rp_id): join_cond = sa.and_(t.c.id == rpt.c.trait_id, rpt.c.resource_provider_id == rp_id) join = sa.join(t, rpt, join_cond) - sel = sa.select([t.c.id, t.c.name]).select_from(join) + sel = sa.select([t.c.id, t.c.name, + t.c.created_at, t.c.updated_at]).select_from(join) return [dict(r) for r in context.session.execute(sel).fetchall()] @@ -637,7 +641,7 @@ def _provider_ids_from_uuid(context, uuid): @base.NovaObjectRegistry.register_if(False) -class ResourceProvider(base.NovaObject): +class ResourceProvider(base.NovaObject, base.NovaTimestampObject): SETTABLE_FIELDS = ('name', 'parent_provider_uuid') fields = { @@ -1438,6 +1442,8 @@ class ResourceProviderList(base.ObjectListBase, base.NovaObject): rp.c.uuid, rp.c.name, rp.c.generation, + rp.c.updated_at, + rp.c.created_at, root_rp.c.uuid.label("root_provider_uuid"), parent_rp.c.uuid.label("parent_provider_uuid"), ] @@ -1597,7 +1603,7 @@ class ResourceProviderList(base.ObjectListBase, base.NovaObject): @base.NovaObjectRegistry.register_if(False) -class Inventory(base.NovaObject): +class Inventory(base.NovaObject, base.NovaTimestampObject): fields = { 'id': fields.IntegerField(read_only=True), @@ -1628,6 +1634,8 @@ def _get_inventory_by_provider_id(ctx, rp_id): inv.c.max_unit, inv.c.step_size, inv.c.allocation_ratio, + inv.c.updated_at, + inv.c.created_at, ] sel = sa.select(cols) sel = sel.where(inv.c.resource_provider_id == rp_id) @@ -1680,7 +1688,7 @@ class InventoryList(base.ObjectListBase, base.NovaObject): @base.NovaObjectRegistry.register_if(False) -class Allocation(base.NovaObject): +class Allocation(base.NovaObject, base.NovaTimestampObject): fields = { 'id': fields.IntegerField(), @@ -1961,6 +1969,8 @@ def _get_allocations_by_provider_id(ctx, rp_id): allocs.c.resource_class_id, allocs.c.consumer_id, allocs.c.used, + allocs.c.updated_at, + allocs.c.created_at ] sel = sa.select(cols) sel = sel.where(allocs.c.resource_provider_id == rp_id) @@ -2247,7 +2257,7 @@ class UsageList(base.ObjectListBase, base.NovaObject): @base.NovaObjectRegistry.register_if(False) -class ResourceClass(base.NovaObject): +class ResourceClass(base.NovaObject, base.NovaTimestampObject): MIN_CUSTOM_RESOURCE_CLASS_ID = 10000 """Any user-defined resource classes must have an identifier greater than @@ -2283,8 +2293,9 @@ class ResourceClass(base.NovaObject): :raises: ResourceClassNotFound if no such resource class was found """ _ensure_rc_cache(context) - rc_id = _RC_CACHE.id_from_string(name) - obj = cls(context, id=rc_id, name=name) + rc = _RC_CACHE.all_from_string(name) + obj = cls(context, id=rc['id'], name=rc['name'], + updated_at=rc['updated_at'], created_at=rc['created_at']) obj.obj_reset_changes() return obj @@ -2438,7 +2449,7 @@ class ResourceClassList(base.ObjectListBase, base.NovaObject): @base.NovaObjectRegistry.register_if(False) -class Trait(base.NovaObject): +class Trait(base.NovaObject, base.NovaTimestampObject): # All the user-defined traits must begin with this prefix. CUSTOM_NAMESPACE = 'CUSTOM_' diff --git a/nova/tests/functional/db/test_resource_class_cache.py b/nova/tests/functional/db/test_resource_class_cache.py index 132f2685fc67..004e27596764 100644 --- a/nova/tests/functional/db/test_resource_class_cache.py +++ b/nova/tests/functional/db/test_resource_class_cache.py @@ -10,8 +10,11 @@ # License for the specific language governing permissions and limitations # under the License. +import datetime import mock +from oslo_utils import timeutils + from nova.db.sqlalchemy import resource_class_cache as rc_cache from nova import exception from nova.objects import fields @@ -57,6 +60,24 @@ class TestResourceClassCache(test.TestCase): standards2 = cache.STANDARDS self.assertEqual(id(standards), id(standards2)) + def test_standards_have_time_fields(self): + cache = rc_cache.ResourceClassCache(self.context) + standards = cache.STANDARDS + + first_standard = standards[0] + self.assertIn('updated_at', first_standard) + self.assertIn('created_at', first_standard) + self.assertIsNone(first_standard['updated_at']) + self.assertIsNone(first_standard['created_at']) + + def test_standard_has_time_fields(self): + cache = rc_cache.ResourceClassCache(self.context) + + vcpu_class = cache.all_from_string('VCPU') + expected = {'id': 0, 'name': 'VCPU', 'updated_at': None, + 'created_at': None} + self.assertEqual(expected, vcpu_class) + def test_rc_cache_custom(self): """Test that non-standard, custom resource classes hit the database and return appropriate results, caching the results after a single @@ -88,6 +109,32 @@ class TestResourceClassCache(test.TestCase): self.assertEqual(1001, cache.id_from_string('IRON_NFV')) self.assertFalse(sel_mock.called) + # Verify all fields available from all_from_string + iron_nfv_class = cache.all_from_string('IRON_NFV') + self.assertEqual(1001, iron_nfv_class['id']) + self.assertEqual('IRON_NFV', iron_nfv_class['name']) + # updated_at not set on insert + self.assertIsNone(iron_nfv_class['updated_at']) + self.assertIsInstance(iron_nfv_class['created_at'], datetime.datetime) + + # Update IRON_NFV (this is a no-op but will set updated_at) + with self.context.session.connection() as conn: + # NOTE(cdent): When using explict SQL that names columns, + # the automatic timestamp handling provided by the oslo_db + # TimestampMixin is not provided. created_at is a default + # but updated_at is an onupdate. + upd_stmt = rc_cache._RC_TBL.update().where( + rc_cache._RC_TBL.c.id == 1001).values( + name='IRON_NFV', updated_at=timeutils.utcnow()) + conn.execute(upd_stmt) + + # reset cache + cache = rc_cache.ResourceClassCache(self.context) + + iron_nfv_class = cache.all_from_string('IRON_NFV') + # updated_at set on update + self.assertIsInstance(iron_nfv_class['updated_at'], datetime.datetime) + def test_rc_cache_miss(self): """Test that we raise ResourceClassNotFound if an unknown resource class ID or string is searched for. diff --git a/nova/tests/unit/objects/test_resource_provider.py b/nova/tests/unit/objects/test_resource_provider.py index d7e55eaf02e4..15cf72899ee9 100644 --- a/nova/tests/unit/objects/test_resource_provider.py +++ b/nova/tests/unit/objects/test_resource_provider.py @@ -13,6 +13,8 @@ import mock import six +from oslo_utils import timeutils + import nova from nova import context from nova import exception @@ -40,6 +42,8 @@ _RESOURCE_PROVIDER_DB = { 'generation': 0, 'root_provider_uuid': _RESOURCE_PROVIDER_UUID, 'parent_provider_uuid': None, + 'updated_at': None, + 'created_at': timeutils.utcnow(with_timezone=True), } _RESOURCE_PROVIDER_ID2 = 2 @@ -66,6 +70,8 @@ _INVENTORY_DB = { 'max_unit': 8, 'step_size': 1, 'allocation_ratio': 1.0, + 'updated_at': None, + 'created_at': timeutils.utcnow(with_timezone=True), } _ALLOCATION_ID = 2 _ALLOCATION_DB = {