diff --git a/glance/db/__init__.py b/glance/db/__init__.py index c1a8340631..3cf537fa04 100644 --- a/glance/db/__init__.py +++ b/glance/db/__init__.py @@ -57,6 +57,7 @@ IMAGE_ATTRS = BASE_MODEL_ATTRS | set(['name', 'status', 'size', 'virtual_size', 'min_disk', 'min_ram', 'is_public', 'locations', 'checksum', 'owner', 'protected']) +IMAGE_ATOMIC_PROPS = set(['os_glance_import_task']) class ImageRepo(object): @@ -186,7 +187,9 @@ class ImageRepo(object): image.image_id, image_values, purge_props=True, - from_state=from_state) + from_state=from_state, + atomic_props=( + IMAGE_ATOMIC_PROPS)) except (exception.ImageNotFound, exception.Forbidden): msg = _("No image found with ID %s") % image.image_id raise exception.ImageNotFound(msg) diff --git a/glance/db/simple/api.py b/glance/db/simple/api.py index 3f4acf7385..21c1defbee 100644 --- a/glance/db/simple/api.py +++ b/glance/db/simple/api.py @@ -784,7 +784,7 @@ def image_create(context, image_values, v1_mode=False): @log_call def image_update(context, image_id, image_values, purge_props=False, - from_state=None, v1_mode=False): + from_state=None, v1_mode=False, atomic_props=None): global DATA try: image = DATA['images'][image_id] @@ -795,17 +795,24 @@ def image_update(context, image_id, image_values, purge_props=False, if location_data is not None: _image_locations_set(context, image_id, location_data) + if atomic_props is None: + atomic_props = [] + # replace values for properties that already exist new_properties = image_values.pop('properties', {}) for prop in image['properties']: - if prop['name'] in new_properties: + if prop['name'] in atomic_props: + continue + elif prop['name'] in new_properties: prop['value'] = new_properties.pop(prop['name']) elif purge_props: # this matches weirdness in the sqlalchemy api prop['deleted'] = True image['updated_at'] = timeutils.utcnow() - _image_update(image, image_values, new_properties) + _image_update(image, image_values, + {k: v for k, v in new_properties.items() + if k not in atomic_props}) DATA['images'][image_id] = image image = _normalize_locations(context, copy.deepcopy(image)) diff --git a/glance/db/sqlalchemy/api.py b/glance/db/sqlalchemy/api.py index 13dd21914a..3c84c27d28 100644 --- a/glance/db/sqlalchemy/api.py +++ b/glance/db/sqlalchemy/api.py @@ -151,14 +151,14 @@ def image_create(context, values, v1_mode=False): def image_update(context, image_id, values, purge_props=False, - from_state=None, v1_mode=False): + from_state=None, v1_mode=False, atomic_props=None): """ Set the given properties on an image and update it. :raises: ImageNotFound if image does not exist. """ image = _image_update(context, values, image_id, purge_props, - from_state=from_state) + from_state=from_state, atomic_props=atomic_props) if v1_mode: image = db_utils.mutate_image_dict_to_v1(image) return image @@ -866,13 +866,19 @@ def image_delete_property_atomic(image_id, name, value): stop_max_attempt_number=50) @utils.no_4byte_params def _image_update(context, values, image_id, purge_props=False, - from_state=None): + from_state=None, atomic_props=None): """ Used internally by image_create and image_update :param context: Request context :param values: A dict of attributes to set :param image_id: If None, create the image, otherwise, find and update it + :param from_state: If not None, reequire the image be in this state to do + the update + :param purge_props: If True, delete properties found in the database but + not present in values + :param atomic_props: If non-None, refuse to create or update properties + in this list """ # NOTE(jbresnah) values is altered in this so a copy is needed @@ -963,7 +969,7 @@ def _image_update(context, values, image_id, purge_props=False, % values['id']) _set_properties_for_image(context, image_ref, properties, purge_props, - session) + atomic_props, session) if location_data: _image_locations_set(context, image_ref.id, location_data, @@ -1078,15 +1084,24 @@ def _image_locations_delete_all(context, image_id, @utils.no_4byte_params def _set_properties_for_image(context, image_ref, properties, - purge_props=False, session=None): + purge_props=False, atomic_props=None, + session=None): """ Create or update a set of image_properties for a given image :param context: Request context :param image_ref: An Image object :param properties: A dict of properties to set + :param purge_props: If True, delete properties in the database + that are not in properties + :param atomic_props: If non-None, skip update/create/delete of properties + named in this list :param session: A SQLAlchemy session to use (if present) """ + + if atomic_props is None: + atomic_props = [] + orig_properties = {} for prop_ref in image_ref.properties: orig_properties[prop_ref.name] = prop_ref @@ -1095,7 +1110,11 @@ def _set_properties_for_image(context, image_ref, properties, prop_values = {'image_id': image_ref.id, 'name': name, 'value': value} - if name in orig_properties: + if name in atomic_props: + # NOTE(danms): Never update or create properties in the list + # of atomics + continue + elif name in orig_properties: prop_ref = orig_properties[name] _image_property_update(context, prop_ref, prop_values, session=session) @@ -1104,7 +1123,9 @@ def _set_properties_for_image(context, image_ref, properties, if purge_props: for key in orig_properties.keys(): - if key not in properties: + if key in atomic_props: + continue + elif key not in properties: prop_ref = orig_properties[key] image_property_delete(context, prop_ref.name, image_ref.id, session=session) diff --git a/glance/tests/functional/db/test_sqlalchemy.py b/glance/tests/functional/db/test_sqlalchemy.py index 0093a44f49..b6ddadacc1 100644 --- a/glance/tests/functional/db/test_sqlalchemy.py +++ b/glance/tests/functional/db/test_sqlalchemy.py @@ -337,3 +337,26 @@ class TestImageAtomicOps(base.TestDriver, # Only the new value should result in proper deletion self.db_api.image_delete_property_atomic(self.image['id'], 'speed', '89mph') + + def test_image_update_ignores_atomics(self): + image = self.db_api.image_get_all(self.adm_context)[0] + + # Set two atomic properties atomically + self.db_api.image_set_property_atomic(image['id'], + 'test1', 'foo') + self.db_api.image_set_property_atomic(image['id'], + 'test2', 'bar') + + # Try to change test1, delete test2, add test3 and test4 via + # normal image_update() where the first three are passed as + # atomic + self.db_api.image_update( + self.adm_context, image['id'], + {'properties': {'test1': 'baz', 'test3': 'bat', 'test4': 'yep'}}, + purge_props=True, atomic_props=['test1', 'test2', 'test3']) + + # Expect that none of the updates to the atomics are applied, but + # the regular property is added. + image = self.db_api.image_get(self.adm_context, image['id']) + self.assertEqual({'test1': 'foo', 'test2': 'bar', 'test4': 'yep'}, + self._propdict(image['properties'])) diff --git a/glance/tests/unit/test_db.py b/glance/tests/unit/test_db.py index 88effebfa3..f2c2414e20 100644 --- a/glance/tests/unit/test_db.py +++ b/glance/tests/unit/test_db.py @@ -368,6 +368,44 @@ class TestImageRepo(test_utils.BaseTestCase): image) self.assertIn(fake_uuid, encodeutils.exception_to_unicode(exc)) + def test_save_excludes_atomic_props(self): + fake_uuid = str(uuid.uuid4()) + image = self.image_repo.get(UUID1) + + # Try to set the property normally + image.extra_properties['os_glance_import_task'] = fake_uuid + self.image_repo.save(image) + + # Expect it was ignored + image = self.image_repo.get(UUID1) + self.assertNotIn('os_glance_import_task', image.extra_properties) + + # Set the property atomically + self.image_repo.set_property_atomic(image, + 'os_glance_import_task', fake_uuid) + # Expect it is set + image = self.image_repo.get(UUID1) + self.assertEqual(fake_uuid, + image.extra_properties['os_glance_import_task']) + + # Try to clobber it + image.extra_properties['os_glance_import_task'] = 'foo' + self.image_repo.save(image) + + # Expect it is unchanged + image = self.image_repo.get(UUID1) + self.assertEqual(fake_uuid, + image.extra_properties['os_glance_import_task']) + + # Try to delete it + del image.extra_properties['os_glance_import_task'] + self.image_repo.save(image) + + # Expect it is still present and set accordingly + image = self.image_repo.get(UUID1) + self.assertEqual(fake_uuid, + image.extra_properties['os_glance_import_task']) + def test_remove_image(self): image = self.image_repo.get(UUID1) previous_update_time = image.updated_at