openstack/glance
Code Issues Proposed changes

Merge "Handle atomic image properties separately"

Browse Source
This commit is contained in:
Zuul 2020-08-29 19:13:33 +00:00 committed by Gerrit Code Review
commit 2f533251dc
5 changed files with 103 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
glance/db/__init__.py
View File

@ -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)

13
glance/db/simple/api.py
View File

@ -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))

35
glance/db/sqlalchemy/api.py
View File

@ -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)

23
glance/tests/functional/db/test_sqlalchemy.py
View File

@ -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']))

38
glance/tests/unit/test_db.py
View File

@ -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