Remove unused methods from GlanceImageService

current ironic code never neither lists images, nor
creates/updates/deletes them.

Thus only `show` and `download` methods are useful for ironic, others
can be removed as those seem to be remnants of nova-baremetal era.

Also remove verious support methods (in tests as well) that were called
only from removed methods.

Change-Id: I592683515887ea7bb4902806fad004bad526058d
This commit is contained in:
Pavlo Shchelokovskyy 2017-05-31 12:14:41 +00:00
parent 82e8a40b93
commit bf477d6d86
7 changed files with 56 additions and 586 deletions

View File

@ -46,17 +46,6 @@ def _translate_image_exception(image_id, exc_value):
return exc_value
def _translate_plain_exception(exc_value):
if isinstance(exc_value, (glance_exc.Forbidden,
glance_exc.Unauthorized)):
return exception.NotAuthorized(exc_value)
if isinstance(exc_value, glance_exc.NotFound):
return exception.NotFound(exc_value)
if isinstance(exc_value, glance_exc.BadRequest):
return exception.Invalid(exc_value)
return exc_value
def check_image_service(func):
"""Creates a glance client if doesn't exists and calls the function."""
@six.wraps(func)
@ -121,6 +110,7 @@ class BaseImageService(object):
glance_exc.BadRequest)
num_attempts = 1 + CONF.glance.glance_num_retries
# TODO(pas-ha) use retrying lib here
for attempt in range(1, num_attempts + 1):
try:
return getattr(self.client.images, method)(*args, **kwargs)
@ -142,32 +132,10 @@ class BaseImageService(object):
time.sleep(1)
except image_excs as e:
exc_type, exc_value, exc_trace = sys.exc_info()
if method == 'list':
new_exc = _translate_plain_exception(
exc_value)
else:
new_exc = _translate_image_exception(
args[0], exc_value)
new_exc = _translate_image_exception(
args[0], exc_value)
six.reraise(type(new_exc), new_exc, exc_trace)
@check_image_service
def _detail(self, method='list', **kwargs):
"""Calls out to Glance for a list of detailed image information.
:returns: A list of dicts containing image metadata.
"""
LOG.debug("Getting a full list of images metadata from glance.")
params = service_utils.extract_query_params(kwargs, self.version)
images = self.call(method, **params)
_images = []
for image in images:
if service_utils.is_image_available(self.context, image):
_images.append(service_utils.translate_from_glance(image))
return _images
@check_image_service
def _show(self, image_href, method='get'):
"""Returns a dict with image data for the given opaque image id.
@ -191,14 +159,14 @@ class BaseImageService(object):
return base_image_meta
@check_image_service
def _download(self, image_id, data=None, method='data'):
def _download(self, image_href, data=None, method='data'):
"""Calls out to Glance for data and writes data.
:param image_id: The opaque image identifier.
:param data: (Optional) File object to write data to.
"""
(image_id, self.glance_host,
self.glance_port, use_ssl) = service_utils.parse_image_ref(image_id)
self.glance_port, use_ssl) = service_utils.parse_image_ref(image_href)
if (self.version == 2 and
'file' in CONF.glance.allowed_direct_url_schemes):
@ -218,72 +186,3 @@ class BaseImageService(object):
else:
for chunk in image_chunks:
data.write(chunk)
@check_image_service
def _create(self, image_meta, data=None, method='create'):
"""Store the image data and return the new image object.
:param image_meta: A dict containing image metadata
:param data: (Optional) File object to create image from.
:returns: dict -- New created image metadata
"""
sent_service_image_meta = service_utils.translate_to_glance(image_meta)
# TODO(ghe): Allow copy-from or location headers Bug #1199532
if data:
sent_service_image_meta['data'] = data
recv_service_image_meta = self.call(method, **sent_service_image_meta)
return service_utils.translate_from_glance(recv_service_image_meta)
@check_image_service
def _update(self, image_id, image_meta, data=None, method='update',
purge_props=False):
"""Modify the given image with the new data.
:param image_id: The opaque image identifier.
:param data: (Optional) File object to update data from.
:param purge_props: (Optional=False) Purge existing properties.
:returns: dict -- New created image metadata
"""
(image_id, self.glance_host,
self.glance_port, use_ssl) = service_utils.parse_image_ref(image_id)
if image_meta:
image_meta = service_utils.translate_to_glance(image_meta)
else:
image_meta = {}
if self.version == 1:
image_meta['purge_props'] = purge_props
if data:
image_meta['data'] = data
# NOTE(bcwaldon): id is not an editable field, but it is likely to be
# passed in by calling code. Let's be nice and ignore it.
image_meta.pop('id', None)
image_meta = self.call(method, image_id, **image_meta)
if self.version == 2 and data:
self.call('upload', image_id, data)
image_meta = self._show(image_id)
return image_meta
@check_image_service
def _delete(self, image_id, method='delete'):
"""Delete the given image.
:param image_id: The opaque image identifier.
:raises: ImageNotFound if the image does not exist.
:raises: NotAuthorized if the user is not an owner.
:raises: ImageNotAuthorized if the user is not authorized.
"""
(image_id, glance_host,
glance_port, use_ssl) = service_utils.parse_image_ref(image_id)
self.call(method, image_id)

View File

@ -26,10 +26,6 @@ class ImageService(object):
def __init__(self):
"""Constructor."""
@abc.abstractmethod
def detail(self):
"""Calls out to Glance for a list of detailed image information."""
@abc.abstractmethod
def show(self, image_id):
"""Returns a dict with image data for the given opaque image id.
@ -47,35 +43,3 @@ class ImageService(object):
:param image_id: The opaque image identifier.
:param data: (Optional) File object to write data to.
"""
@abc.abstractmethod
def create(self, image_meta, data=None):
"""Store the image data and return the new image object.
:param image_meta: A dict containing image metadata
:param data: (Optional) File object to create image from.
:returns: dict -- New created image metadata
"""
@abc.abstractmethod
def update(self, image_id,
image_meta, data=None, purge_props=False):
"""Modify the given image with the new data.
:param image_id: The opaque image identifier.
:param data: (Optional) File object to update data from.
:param purge_props: (Optional=True) Purge existing properties.
:returns: dict -- New created image metadata
"""
@abc.abstractmethod
def delete(self, image_id):
"""Delete the given image.
:param image_id: The opaque image identifier.
:raises: ImageNotFound if the image does not exist.
:raises: NotAuthorized if the user is not an owner.
:raises: ImageNotAuthorized if the user is not authorized.
"""

View File

@ -89,31 +89,18 @@ def _convert_timestamps_to_datetimes(image_meta):
_CONVERT_PROPS = ('block_device_mapping', 'mappings')
def _convert(metadata, method):
def _convert(metadata):
metadata = copy.deepcopy(metadata)
properties = metadata.get('properties')
if properties:
for attr in _CONVERT_PROPS:
if attr in properties:
prop = properties[attr]
if method == 'from':
if isinstance(prop, six.string_types):
properties[attr] = jsonutils.loads(prop)
if method == 'to':
if not isinstance(prop, six.string_types):
properties[attr] = jsonutils.dumps(prop)
if isinstance(prop, six.string_types):
properties[attr] = jsonutils.loads(prop)
return metadata
def _remove_read_only(image_meta):
IMAGE_ATTRIBUTES = ['status', 'updated_at', 'created_at', 'deleted_at']
output = copy.deepcopy(image_meta)
for attr in IMAGE_ATTRIBUTES:
if attr in output:
del output[attr]
return output
def _get_api_server_iterator():
"""Return iterator over shuffled API servers.
@ -185,34 +172,10 @@ def parse_image_ref(image_href):
raise exception.InvalidImageRef(image_href=image_href)
def extract_query_params(params, version):
_params = {}
accepted_params = ('filters', 'marker', 'limit',
'sort_key', 'sort_dir')
for param in accepted_params:
if params.get(param):
_params[param] = params.get(param)
# ensure filters is a dict
_params.setdefault('filters', {})
# NOTE(vish): don't filter out private images
# NOTE(ghe): in v2, not passing any visibility doesn't filter prvate images
if version == 1:
_params['filters'].setdefault('is_public', 'none')
return _params
def translate_to_glance(image_meta):
image_meta = _convert(image_meta, 'to')
image_meta = _remove_read_only(image_meta)
return image_meta
def translate_from_glance(image):
image_meta = _extract_attributes(image)
image_meta = _convert_timestamps_to_datetimes(image_meta)
image_meta = _convert(image_meta, 'from')
image_meta = _convert(image_meta)
return image_meta

View File

@ -21,21 +21,8 @@ from ironic.common.glance_service import service
class GlanceImageService(base_image_service.BaseImageService,
service.ImageService):
def detail(self, **kwargs):
return self._detail(method='list', **kwargs)
def show(self, image_id):
return self._show(image_id, method='get')
def download(self, image_id, data=None):
return self._download(image_id, method='data', data=data)
def create(self, image_meta, data=None):
return self._create(image_meta, method='create', data=data)
def update(self, image_id, image_meta, data=None, purge_props=False):
return self._update(image_id, image_meta, data=data, method='update',
purge_props=purge_props)
def delete(self, image_id):
return self._delete(image_id, method='delete')

View File

@ -44,27 +44,12 @@ class GlanceImageService(base_image_service.BaseImageService,
# }
_cache = {}
def detail(self, **kwargs):
return self._detail(method='list', **kwargs)
def show(self, image_id):
return self._show(image_id, method='get')
def download(self, image_id, data=None):
return self._download(image_id, method='data', data=data)
def create(self, image_meta, data=None):
image_id = self._create(image_meta, method='create', data=None)['id']
return self.update(image_id, None, data)
def update(self, image_id, image_meta, data=None, purge_props=False):
# NOTE(ghe): purge_props not working until bug 1206472 solved
return self._update(image_id, image_meta, data, method='update',
purge_props=False)
def delete(self, image_id):
return self._delete(image_id, method='delete')
def _generate_temp_url(self, path, seconds, key, method, endpoint,
image_id):
"""Get Swift temporary URL.

View File

@ -21,7 +21,6 @@ from glanceclient import client as glance_client
from glanceclient import exc as glance_exc
import mock
from oslo_config import cfg
from oslo_serialization import jsonutils
from oslo_utils import uuidutils
from six.moves.urllib import parse as urlparse
import testtools
@ -53,41 +52,35 @@ class TestGlanceSerializer(testtools.TestCase):
'foo': 'bar',
'properties': {
'prop1': 'propvalue1',
'mappings': [
{'virtual': 'aaa',
'device': 'bbb'},
{'virtual': 'xxx',
'device': 'yyy'}],
'block_device_mapping': [
{'virtual_device': 'fake',
'device_name': '/dev/fake'},
{'virtual_device': 'ephemeral0',
'device_name': '/dev/fake0'}]}}
'mappings': '['
'{"virtual":"aaa","device":"bbb"},'
'{"virtual":"xxx","device":"yyy"}]',
'block_device_mapping': '['
'{"virtual_device":"fake","device_name":"/dev/fake"},'
'{"virtual_device":"ephemeral0",'
'"device_name":"/dev/fake0"}]'}}
converted_expected = {
expected = {
'name': 'image1',
'is_public': True,
'foo': 'bar',
'properties': {'prop1': 'propvalue1'}
'properties': {'prop1': 'propvalue1',
'mappings': [
{'virtual': 'aaa',
'device': 'bbb'},
{'virtual': 'xxx',
'device': 'yyy'},
],
'block_device_mapping': [
{'virtual_device': 'fake',
'device_name': '/dev/fake'},
{'virtual_device': 'ephemeral0',
'device_name': '/dev/fake0'}
]
}
}
converted = service_utils._convert(metadata, 'to')
self.assertEqual(metadata,
service_utils._convert(converted, 'from'))
# Fields that rely on dict ordering can't be compared as text
mappings = jsonutils.loads(converted['properties']
.pop('mappings'))
self.assertEqual([{"device": "bbb", "virtual": "aaa"},
{"device": "yyy", "virtual": "xxx"}],
mappings)
bd_mapping = jsonutils.loads(converted['properties']
.pop('block_device_mapping'))
self.assertEqual([{"virtual_device": "fake",
"device_name": "/dev/fake"},
{"virtual_device": "ephemeral0",
"device_name": "/dev/fake0"}],
bd_mapping)
# Compare the remaining
self.assertEqual(converted_expected, converted)
converted = service_utils._convert(metadata)
self.assertEqual(expected, converted)
class TestGlanceImageService(base.TestCase):
@ -122,7 +115,7 @@ class TestGlanceImageService(base.TestCase):
'status': None,
'is_public': None}
fixture.update(kwargs)
return fixture
return stubs.FakeImage(fixture)
@property
def endpoint(self):
@ -141,250 +134,10 @@ class TestGlanceImageService(base.TestCase):
updated_at=self.NOW_GLANCE_FORMAT,
deleted_at=self.NOW_GLANCE_FORMAT)
def test_create_with_instance_id(self):
# Ensure instance_id is persisted as an image-property.
fixture = {'name': 'test image',
'is_public': False,
'properties': {'instance_id': '42', 'user_id': 'fake'}}
image_id = self.service.create(fixture)['id']
image_meta = self.service.show(image_id)
expected = {
'id': image_id,
'name': 'test image',
'is_public': False,
'size': None,
'min_disk': None,
'min_ram': None,
'disk_format': None,
'container_format': None,
'checksum': None,
'created_at': self.NOW_DATETIME,
'updated_at': self.NOW_DATETIME,
'deleted_at': None,
'deleted': None,
'status': None,
'properties': {'instance_id': '42', 'user_id': 'fake'},
'owner': None,
}
self.assertEqual(expected, image_meta)
image_metas = self.service.detail()
self.assertEqual(expected, image_metas[0])
def test_create_without_instance_id(self):
"""Test creating an image without an instance ID.
Ensure we can create an image without having to specify an
instance_id. Public images are an example of an image not tied to an
instance.
"""
fixture = {'name': 'test image', 'is_public': False}
image_id = self.service.create(fixture)['id']
expected = {
'id': image_id,
'name': 'test image',
'is_public': False,
'size': None,
'min_disk': None,
'min_ram': None,
'disk_format': None,
'container_format': None,
'checksum': None,
'created_at': self.NOW_DATETIME,
'updated_at': self.NOW_DATETIME,
'deleted_at': None,
'deleted': None,
'status': None,
'properties': {},
'owner': None,
}
actual = self.service.show(image_id)
self.assertEqual(expected, actual)
def test_create(self):
fixture = self._make_fixture(name='test image')
num_images = len(self.service.detail())
image_id = self.service.create(fixture)['id']
self.assertIsNotNone(image_id)
self.assertEqual(
num_images + 1, len(self.service.detail()))
def test_create_and_show_non_existing_image(self):
fixture = self._make_fixture(name='test image')
image_id = self.service.create(fixture)['id']
self.assertIsNotNone(image_id)
self.assertRaises(exception.ImageNotFound,
self.service.show,
'bad image id')
def test_detail_private_image(self):
fixture = self._make_fixture(name='test image')
fixture['is_public'] = False
properties = {'owner_id': 'proj1'}
fixture['properties'] = properties
self.service.create(fixture)['id']
proj = self.context.project_id
self.context.project_id = 'proj1'
image_metas = self.service.detail()
self.context.project_id = proj
self.assertEqual(1, len(image_metas))
self.assertEqual('test image', image_metas[0]['name'])
self.assertFalse(image_metas[0]['is_public'])
def test_detail_marker(self):
fixtures = []
ids = []
for i in range(10):
fixture = self._make_fixture(name='TestImage %d' % (i))
fixtures.append(fixture)
ids.append(self.service.create(fixture)['id'])
image_metas = self.service.detail(marker=ids[1])
self.assertEqual(8, len(image_metas))
i = 2
for meta in image_metas:
expected = {
'id': ids[i],
'status': None,
'is_public': None,
'name': 'TestImage %d' % (i),
'properties': {},
'size': None,
'min_disk': None,
'min_ram': None,
'disk_format': None,
'container_format': None,
'checksum': None,
'created_at': self.NOW_DATETIME,
'updated_at': self.NOW_DATETIME,
'deleted_at': None,
'deleted': None,
'owner': None,
}
self.assertEqual(expected, meta)
i = i + 1
def test_detail_limit(self):
fixtures = []
ids = []
for i in range(10):
fixture = self._make_fixture(name='TestImage %d' % (i))
fixtures.append(fixture)
ids.append(self.service.create(fixture)['id'])
image_metas = self.service.detail(limit=5)
self.assertEqual(5, len(image_metas))
def test_detail_default_limit(self):
fixtures = []
ids = []
for i in range(10):
fixture = self._make_fixture(name='TestImage %d' % (i))
fixtures.append(fixture)
ids.append(self.service.create(fixture)['id'])
image_metas = self.service.detail()
for i, meta in enumerate(image_metas):
self.assertEqual(meta['name'], 'TestImage %d' % (i))
def test_detail_marker_and_limit(self):
fixtures = []
ids = []
for i in range(10):
fixture = self._make_fixture(name='TestImage %d' % (i))
fixtures.append(fixture)
ids.append(self.service.create(fixture)['id'])
image_metas = self.service.detail(marker=ids[3], limit=5)
self.assertEqual(5, len(image_metas))
i = 4
for meta in image_metas:
expected = {
'id': ids[i],
'status': None,
'is_public': None,
'name': 'TestImage %d' % (i),
'properties': {},
'size': None,
'min_disk': None,
'min_ram': None,
'disk_format': None,
'container_format': None,
'checksum': None,
'created_at': self.NOW_DATETIME,
'updated_at': self.NOW_DATETIME,
'deleted_at': None,
'deleted': None,
'owner': None,
}
self.assertEqual(expected, meta)
i = i + 1
def test_detail_invalid_marker(self):
fixtures = []
ids = []
for i in range(10):
fixture = self._make_fixture(name='TestImage %d' % (i))
fixtures.append(fixture)
ids.append(self.service.create(fixture)['id'])
self.assertRaises(exception.Invalid, self.service.detail,
marker='invalidmarker')
def test_update(self):
fixture = self._make_fixture(name='test image')
image = self.service.create(fixture)
image_id = image['id']
fixture['name'] = 'new image name'
self.service.update(image_id, fixture)
new_image_data = self.service.show(image_id)
self.assertEqual('new image name', new_image_data['name'])
def test_delete(self):
fixture1 = self._make_fixture(name='test image 1')
fixture2 = self._make_fixture(name='test image 2')
fixtures = [fixture1, fixture2]
num_images = len(self.service.detail())
self.assertEqual(0, num_images)
ids = []
for fixture in fixtures:
new_id = self.service.create(fixture)['id']
ids.append(new_id)
num_images = len(self.service.detail())
self.assertEqual(2, num_images)
self.service.delete(ids[0])
# When you delete an image from glance, it sets the status to DELETED
# and doesn't actually remove the image.
# Check the image is still there.
num_images = len(self.service.detail())
self.assertEqual(2, num_images)
# Check the image is marked as deleted.
num_images = len([x for x in self.service.detail()
if not x['deleted']])
self.assertEqual(1, num_images)
def test_show_passes_through_to_client(self):
fixture = self._make_fixture(name='image1', is_public=True)
image_id = self.service.create(fixture)['id']
image_meta = self.service.show(image_id)
image_id = uuidutils.generate_uuid()
image = self._make_fixture(name='image1', is_public=True,
id=image_id)
expected = {
'id': image_id,
'name': 'image1',
@ -395,65 +148,33 @@ class TestGlanceImageService(base.TestCase):
'disk_format': None,
'container_format': None,
'checksum': None,
'created_at': self.NOW_DATETIME,
'updated_at': self.NOW_DATETIME,
'created_at': None,
'updated_at': None,
'deleted_at': None,
'deleted': None,
'status': None,
'properties': {},
'owner': None,
}
with mock.patch.object(self.service, 'call', return_value=image):
image_meta = self.service.show(image_id)
self.service.call.assert_called_once_with('get', image_id)
self.assertEqual(expected, image_meta)
def test_show_makes_datetimes(self):
image_id = uuidutils.generate_uuid()
image = self._make_datetime_fixture()
with mock.patch.object(self.service, 'call', return_value=image):
image_meta = self.service.show(image_id)
self.service.call.assert_called_once_with('get', image_id)
self.assertEqual(self.NOW_DATETIME, image_meta['created_at'])
self.assertEqual(self.NOW_DATETIME, image_meta['updated_at'])
def test_show_raises_when_no_authtoken_in_the_context(self):
fixture = self._make_fixture(name='image1',
is_public=False,
properties={'one': 'two'})
image_id = self.service.create(fixture)['id']
self.context.auth_token = False
self.assertRaises(exception.ImageNotFound,
self.service.show,
image_id)
def test_detail_passes_through_to_client(self):
fixture = self._make_fixture(name='image10', is_public=True)
image_id = self.service.create(fixture)['id']
image_metas = self.service.detail()
expected = [
{
'id': image_id,
'name': 'image10',
'is_public': True,
'size': None,
'min_disk': None,
'min_ram': None,
'disk_format': None,
'container_format': None,
'checksum': None,
'created_at': self.NOW_DATETIME,
'updated_at': self.NOW_DATETIME,
'deleted_at': None,
'deleted': None,
'status': None,
'properties': {},
'owner': None,
},
]
self.assertEqual(expected, image_metas)
def test_show_makes_datetimes(self):
fixture = self._make_datetime_fixture()
image_id = self.service.create(fixture)['id']
image_meta = self.service.show(image_id)
self.assertEqual(self.NOW_DATETIME, image_meta['created_at'])
self.assertEqual(self.NOW_DATETIME, image_meta['updated_at'])
def test_detail_makes_datetimes(self):
fixture = self._make_datetime_fixture()
self.service.create(fixture)
image_meta = self.service.detail()[0]
self.assertEqual(self.NOW_DATETIME, image_meta['created_at'])
self.assertEqual(self.NOW_DATETIME, image_meta['updated_at'])
uuidutils.generate_uuid())
@mock.patch.object(time, 'sleep', autospec=True)
def test_download_with_retries(self, mock_sleep):

View File

@ -27,23 +27,9 @@ class StubGlanceClient(object):
# NOTE(bcwaldon): HACK to get client.images.* to work
self.images = lambda: None
for fn in ('list', 'get', 'data', 'create', 'update', 'delete'):
for fn in ('get', 'data'):
setattr(self.images, fn, getattr(self, fn))
# TODO(bcwaldon): implement filters
def list(self, filters=None, marker=None, limit=30):
if marker is None:
index = 0
else:
for index, image in enumerate(self._images):
if image.id == str(marker):
index += 1
break
else:
raise glance_exc.BadRequest('Marker not found')
return self._images[index:index + limit]
def get(self, image_id):
for image in self._images:
if image.id == str(image_id):
@ -54,53 +40,18 @@ class StubGlanceClient(object):
self.get(image_id)
return []
def create(self, **metadata):
metadata['created_at'] = NOW_GLANCE_FORMAT
metadata['updated_at'] = NOW_GLANCE_FORMAT
self._images.append(FakeImage(metadata))
try:
image_id = str(metadata['id'])
except KeyError:
# auto-generate an id if one wasn't provided
image_id = str(len(self._images))
self._images[-1].id = image_id
return self._images[-1]
def update(self, image_id, **metadata):
for i, image in enumerate(self._images):
if image.id == str(image_id):
for k, v in metadata.items():
setattr(self._images[i], k, v)
return self._images[i]
raise glance_exc.NotFound(image_id)
def delete(self, image_id):
for i, image in enumerate(self._images):
if image.id == image_id:
# When you delete an image from glance, it sets the status to
# DELETED. If you try to delete a DELETED image, it raises
# HTTPForbidden.
image_data = self._images[i]
if image_data.deleted:
raise glance_exc.Forbidden()
image_data.deleted = True
return
raise glance_exc.NotFound(image_id)
class FakeImage(object):
def __init__(self, metadata):
IMAGE_ATTRIBUTES = ['size', 'disk_format', 'owner',
'container_format', 'checksum', 'id',
'name', 'created_at', 'updated_at',
'name',
'deleted', 'status',
'min_disk', 'min_ram', 'is_public']
raw = dict.fromkeys(IMAGE_ATTRIBUTES)
raw.update(metadata)
# raw['created_at'] = NOW_GLANCE_FORMAT
# raw['updated_at'] = NOW_GLANCE_FORMAT
self.__dict__['raw'] = raw
def __getattr__(self, key):