openstack/glance_store
Code Issues Proposed changes

Address multi store nits

Browse Source 
This addresses some nits from I1f2e8fa61d6dfecd8395a1f894f74ec5bcb5573c
that were not worth holding up that review to address.

Change-Id: Iab60d5622dddf29871fbbc9f51deb93de26235e8
This commit is contained in:
Sean McGinnis 2018-07-16 14:24:47 -05:00
parent 87114c8ec7
commit d4c5fa95e5
3 changed files with 57 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
glance_store/location.py
View File

@ -79,7 +79,8 @@ def get_location_from_uri(uri, conf=CONF):
def get_location_from_uri_and_backend(uri, backend, conf=CONF): def get_location_from_uri_and_backend(uri, backend, conf=CONF):
""" """Extract backend location from a URI.
Given a URI, return a Location object that has had an appropriate Given a URI, return a Location object that has had an appropriate
store parse the URI. store parse the URI.
@ -112,7 +113,8 @@ def get_location_from_uri_and_backend(uri, backend, conf=CONF):
def register_scheme_backend_map(scheme_map): def register_scheme_backend_map(scheme_map):
""" """Registers a mapping between a scheme and a backend.
Given a mapping of 'scheme' to store_name, adds the mapping to the Given a mapping of 'scheme' to store_name, adds the mapping to the
known list of schemes. known list of schemes.

38
glance_store/multi_backend.py
View File

@ -23,7 +23,7 @@ from stevedore import extension
from glance_store import capabilities from glance_store import capabilities
from glance_store import exceptions from glance_store import exceptions
from glance_store.i18n import _, _LW from glance_store.i18n import _
from glance_store import location from glance_store import location
@ -123,7 +123,7 @@ def _list_driver_opts():
def register_store_opts(conf): def register_store_opts(conf):
LOG.debug("Registering options for group %s" % _STORE_CFG_GROUP) LOG.debug("Registering options for group %s", _STORE_CFG_GROUP)
conf.register_opts(_STORE_OPTS, group=_STORE_CFG_GROUP) conf.register_opts(_STORE_OPTS, group=_STORE_CFG_GROUP)
driver_opts = _list_driver_opts() driver_opts = _list_driver_opts()
@ -133,7 +133,7 @@ def register_store_opts(conf):
if enabled_backends[backend] not in opt_list: if enabled_backends[backend] not in opt_list:
continue continue
LOG.debug("Registering options for group %s" % backend) LOG.debug("Registering options for group %s", backend)
conf.register_opts(driver_opts[opt_list], group=backend) conf.register_opts(driver_opts[opt_list], group=backend)
@ -153,7 +153,7 @@ def _load_multi_store(conf, store_entry,
return mgr.driver return mgr.driver
except RuntimeError as e: except RuntimeError as e:
LOG.warning("Failed to load driver %(driver)s. The " LOG.warning("Failed to load driver %(driver)s. The "
"driver will be disabled" % dict(driver=str([driver, e]))) "driver will be disabled", dict(driver=str([driver, e])))
def _load_multi_stores(conf): def _load_multi_stores(conf):
@ -176,10 +176,7 @@ def _load_multi_stores(conf):
def create_multi_stores(conf=CONF): def create_multi_stores(conf=CONF):
""" """Registers all store modules and all schemes from the given config."""
Registers all store modules and all schemes
from the given config.
"""
store_count = 0 store_count = 0
scheme_map = {} scheme_map = {}
for (store_entry, store_instance, for (store_entry, store_instance,
@ -191,9 +188,9 @@ def create_multi_stores(conf=CONF):
continue continue
if not schemes: if not schemes:
raise exceptions.BackendException('Unable to register store %s. ' raise exceptions.BackendException(
'No schemes associated with it.' _('Unable to register store %s. No schemes associated '
% store_entry) 'with it.') % store_entry)
else: else:
LOG.debug("Registering store %s with schemes %s", LOG.debug("Registering store %s with schemes %s",
store_entry, schemes) store_entry, schemes)
@ -227,7 +224,8 @@ def verify_store():
def get_store_from_store_identifier(store_identifier): def get_store_from_store_identifier(store_identifier):
""" """Determine backing store from identifier.
Given a store identifier, return the appropriate store object Given a store identifier, return the appropriate store object
for handling that scheme. for handling that scheme.
""" """
@ -282,9 +280,9 @@ def add(conf, image_id, data, size, backend, context=None,
def store_add_to_backend(image_id, data, size, store, context=None, def store_add_to_backend(image_id, data, size, store, context=None,
verifier=None): verifier=None):
""" """A wrapper around a call to each stores add() method.
A wrapper around a call to each stores add() method. This gives glance
a common place to check the output This gives glance a common place to check the output.
:param image_id: The image add to which data is added :param image_id: The image add to which data is added
:param data: The data to be stored :param data: The data to be stored
@ -349,9 +347,8 @@ def delete(uri, backend, context=None):
store = get_store_from_store_identifier(backend) store = get_store_from_store_identifier(backend)
return store.delete(loc, context=context) return store.delete(loc, context=context)
msg = _LW('Backend is not set to image, searching ' LOG.warning('Backend is not set to image, searching all backends based on '
'all backends based on location URI.') 'location URI.')
LOG.warn(msg)
backends = CONF.enabled_backends backends = CONF.enabled_backends
for backend in backends: for backend in backends:
@ -400,9 +397,8 @@ def get(uri, backend, offset=0, chunk_size=None, context=None):
chunk_size=chunk_size, chunk_size=chunk_size,
context=context) context=context)
msg = _LW('Backend is not set to image, searching ' LOG.warning('Backend is not set to image, searching all backends based on '
'all backends based on location URI.') 'location URI.')
LOG.warn(msg)
backends = CONF.enabled_backends backends = CONF.enabled_backends
for backend in backends: for backend in backends:

83
glance_store/tests/unit/test_multistore_filesystem.py
View File

@ -171,10 +171,7 @@ class TestMultiStore(base.MultiStoreBaseTest,
self.assertEqual(chunk_size, image_size) self.assertEqual(chunk_size, image_size)
def test_get_non_existing(self): def test_get_non_existing(self):
""" """Test trying to retrieve a file that doesn't exist raises error."""
Test that trying to retrieve a file that doesn't exist
raises an error
"""
loc = location.get_location_from_uri_and_backend( loc = location.get_location_from_uri_and_backend(
"file:///%s/non-existing" % self.test_dir, 'file1', conf=self.conf) "file:///%s/non-existing" % self.test_dir, 'file1', conf=self.conf)
self.assertRaises(exceptions.NotFound, self.assertRaises(exceptions.NotFound,
@ -182,10 +179,7 @@ class TestMultiStore(base.MultiStoreBaseTest,
loc) loc)
def test_get_non_existing_identifier(self): def test_get_non_existing_identifier(self):
""" """Test trying to retrieve a store that doesn't exist raises error."""
Test that trying to retrieve a store that doesn't exist
raises an error
"""
self.assertRaises(exceptions.UnknownScheme, self.assertRaises(exceptions.UnknownScheme,
location.get_location_from_uri_and_backend, location.get_location_from_uri_and_backend,
"file:///%s/non-existing" % self.test_dir, "file:///%s/non-existing" % self.test_dir,
@ -341,39 +335,36 @@ class TestMultiStore(base.MultiStoreBaseTest,
self.assertFalse(os.path.exists(path)) self.assertFalse(os.path.exists(path))
def test_add_storage_full(self): def test_add_storage_full(self):
""" """Tests adding an image without enough space.
Tests that adding an image without enough space on disk Tests that adding an image without enough space on disk
raises an appropriate exception raises an appropriate exception.
""" """
self._do_test_add_write_failure(errno.ENOSPC, exceptions.StorageFull) self._do_test_add_write_failure(errno.ENOSPC, exceptions.StorageFull)
def test_add_file_too_big(self): def test_add_file_too_big(self):
""" """Tests adding a very large image.
Tests that adding an excessively large image file Tests that adding an excessively large image file
raises an appropriate exception raises an appropriate exception.
""" """
self._do_test_add_write_failure(errno.EFBIG, exceptions.StorageFull) self._do_test_add_write_failure(errno.EFBIG, exceptions.StorageFull)
def test_add_storage_write_denied(self): def test_add_storage_write_denied(self):
""" """Tests adding an image without store permissions.
Tests that adding an image with insufficient filestore permissions Tests that adding an image with insufficient filestore permissions
raises an appropriate exception raises an appropriate exception.
""" """
self._do_test_add_write_failure(errno.EACCES, self._do_test_add_write_failure(errno.EACCES,
exceptions.StorageWriteDenied) exceptions.StorageWriteDenied)
def test_add_other_failure(self): def test_add_other_failure(self):
""" """Tests other IOErrors do not raise a StorageFull exception."""
Tests that a non-space-related IOError does not raise a
StorageFull exceptions.
"""
self._do_test_add_write_failure(errno.ENOTDIR, IOError) self._do_test_add_write_failure(errno.ENOTDIR, IOError)
def test_add_cleanup_on_read_failure(self): def test_add_cleanup_on_read_failure(self):
""" """Tests partial image is cleaned up after a read failure."""
Tests the partial image file is cleaned up after a read
failure.
"""
filesystem.ChunkedFile.CHUNKSIZE = units.Ki filesystem.ChunkedFile.CHUNKSIZE = units.Ki
image_id = str(uuid.uuid4()) image_id = str(uuid.uuid4())
file_size = 5 * units.Ki # 5K file_size = 5 * units.Ki # 5K
@ -393,9 +384,7 @@ class TestMultiStore(base.MultiStoreBaseTest,
self.assertFalse(os.path.exists(path)) self.assertFalse(os.path.exists(path))
def test_delete(self): def test_delete(self):
""" """Test we can delete an existing image in the filesystem store."""
Test we can delete an existing image in the filesystem store
"""
# First add an image # First add an image
image_id = str(uuid.uuid4()) image_id = str(uuid.uuid4())
file_size = 5 * units.Ki # 5K file_size = 5 * units.Ki # 5K
@ -416,10 +405,7 @@ class TestMultiStore(base.MultiStoreBaseTest,
self.assertRaises(exceptions.NotFound, self.store.get, loc) self.assertRaises(exceptions.NotFound, self.store.get, loc)
def test_delete_non_existing(self): def test_delete_non_existing(self):
""" """Test deleting file that doesn't exist raises an error."""
Test that trying to delete a file that doesn't exist
raises an error
"""
loc = location.get_location_from_uri_and_backend( loc = location.get_location_from_uri_and_backend(
"file:///tmp/glance-tests/non-existing", "file1", conf=self.conf) "file:///tmp/glance-tests/non-existing", "file1", conf=self.conf)
self.assertRaises(exceptions.NotFound, self.assertRaises(exceptions.NotFound,
@ -427,10 +413,7 @@ class TestMultiStore(base.MultiStoreBaseTest,
loc) loc)
def test_delete_forbidden(self): def test_delete_forbidden(self):
""" """Tests deleting file without permissions raises the correct error."""
Tests that trying to delete a file without permissions
raises the correct error
"""
# First add an image # First add an image
image_id = str(uuid.uuid4()) image_id = str(uuid.uuid4())
file_size = 5 * units.Ki # 5K file_size = 5 * units.Ki # 5K
@ -463,10 +446,7 @@ class TestMultiStore(base.MultiStoreBaseTest,
self.store.get(loc) self.store.get(loc)
def test_configure_add_with_multi_datadirs(self): def test_configure_add_with_multi_datadirs(self):
""" """Test multiple filesystems are parsed correctly."""
Tests multiple filesystem specified by filesystem_store_datadirs
are parsed correctly.
"""
store_map = [self.useFixture(fixtures.TempDir()).path, store_map = [self.useFixture(fixtures.TempDir()).path,
self.useFixture(fixtures.TempDir()).path] self.useFixture(fixtures.TempDir()).path]
self.conf.set_override('filesystem_store_datadir', self.conf.set_override('filesystem_store_datadir',
@ -560,9 +540,11 @@ class TestMultiStore(base.MultiStoreBaseTest,
self.store.configure_add) self.store.configure_add)
def test_configure_add_same_dir_multiple_times(self): def test_configure_add_same_dir_multiple_times(self):
""" """Tests handling of same dir in config multiple times.
Tests BadStoreConfiguration exception is raised if same directory Tests BadStoreConfiguration exception is raised if same directory
is specified multiple times in filesystem_store_datadirs. is specified multiple times in filesystem_store_datadirs with different
priorities.
""" """
store_map = [self.useFixture(fixtures.TempDir()).path, store_map = [self.useFixture(fixtures.TempDir()).path,
self.useFixture(fixtures.TempDir()).path] self.useFixture(fixtures.TempDir()).path]
@ -577,9 +559,11 @@ class TestMultiStore(base.MultiStoreBaseTest,
self.store.configure_add) self.store.configure_add)
def test_configure_add_same_dir_multiple_times_same_priority(self): def test_configure_add_same_dir_multiple_times_same_priority(self):
""" """Tests handling of same dir in config multiple times.
Tests BadStoreConfiguration exception is raised if same directory Tests BadStoreConfiguration exception is raised if same directory
is specified multiple times in filesystem_store_datadirs. is specified multiple times in filesystem_store_datadirs with the same
priority.
""" """
store_map = [self.useFixture(fixtures.TempDir()).path, store_map = [self.useFixture(fixtures.TempDir()).path,
self.useFixture(fixtures.TempDir()).path] self.useFixture(fixtures.TempDir()).path]
@ -676,7 +660,8 @@ class TestMultiStore(base.MultiStoreBaseTest,
self.assertEqual(expected_file_size, new_image_file_size) self.assertEqual(expected_file_size, new_image_file_size)
def test_add_with_multiple_dirs_storage_full(self): def test_add_with_multiple_dirs_storage_full(self):
""" """Tests adding dirs with storage full.
Test StorageFull exception is raised if no filesystem directory Test StorageFull exception is raised if no filesystem directory
is found that can store an image. is found that can store an image.
""" """
@ -709,7 +694,8 @@ class TestMultiStore(base.MultiStoreBaseTest,
expected_file_size) expected_file_size)
def test_configure_add_with_file_perm(self): def test_configure_add_with_file_perm(self):
""" """Tests adding with permissions.
Tests filesystem specified by filesystem_store_file_perm Tests filesystem specified by filesystem_store_file_perm
are parsed correctly. are parsed correctly.
""" """
@ -721,8 +707,9 @@ class TestMultiStore(base.MultiStoreBaseTest,
self.store.configure_add() self.store.configure_add()
self.assertEqual(self.store.datadir, store) self.assertEqual(self.store.datadir, store)
def test_configure_add_with_unaccessible_file_perm(self): def test_configure_add_with_inaccessible_file_perm(self):
""" """Tests adding with inaccessible file permissions.
Tests BadStoreConfiguration exception is raised if an invalid Tests BadStoreConfiguration exception is raised if an invalid
file permission specified in filesystem_store_file_perm. file permission specified in filesystem_store_file_perm.
""" """
@ -735,7 +722,8 @@ class TestMultiStore(base.MultiStoreBaseTest,
self.store.configure_add) self.store.configure_add)
def test_add_with_file_perm_for_group_other_users_access(self): def test_add_with_file_perm_for_group_other_users_access(self):
""" """Tests adding image with file permissions.
Test that we can add an image via the filesystem backend with a Test that we can add an image via the filesystem backend with a
required image file permission. required image file permission.
""" """
@ -778,7 +766,8 @@ class TestMultiStore(base.MultiStoreBaseTest,
self.assertEqual(perm, stat.S_IMODE(mode)) self.assertEqual(perm, stat.S_IMODE(mode))
def test_add_with_file_perm_for_owner_users_access(self): def test_add_with_file_perm_for_owner_users_access(self):
""" """Tests adding image with file permissions.
Test that we can add an image via the filesystem backend with a Test that we can add an image via the filesystem backend with a
required image file permission. required image file permission.
""" """