From 6dba83ba3a7c047df282f2edb7217548f43d6251 Mon Sep 17 00:00:00 2001 From: Abhishek Kekane Date: Wed, 17 Oct 2018 07:28:26 +0000 Subject: [PATCH] Rethinking filesystem access In Rocky multiple backend support is added as experimental feature. In order to take advantage of this feature it is decided to deprecate work_dir and node_staging_uri configuration options and reserve two filesystem stores 'os_glance_tasks_store' and 'os_glance_staging_store', which can be used to get rid of initializing store via internal functions. These internal stores are considered "reserved stores" by Glance. For the time being, these are hard-coded as filesystem stores. The store prefix 'os_glance_' is reserved for internal Glance use and the glance-api service will refuse to start if a store with this prefix is included in the enabled_backends config option in glance-api.conf. NOTE: Because there are no sensible default values for the location of the datadir for each of these stores, the operator must define 'os_glance_tasks_store' and 'os_glance_staging_store' in glance-api.conf configuration file as shown below. [os_glance_tasks_store] filesystem_store_datadir = /var/lib/glance/tasks_work_dir/ [os_glance_staging_store] filesystem_store_datadir = /var/lib/glance/staging/ Each filesystem store must have a unique datadir. Depends-On: https://review.openstack.org/#/c/639765/ Implements: blueprint rethinking-filesystem-access Change-Id: I86ec513c5fc653dbb97b79d953d8430f014e684f --- doc/source/admin/multistores.rst | 116 +++++++++++++++--- glance/api/v2/discovery.py | 3 + glance/api/v2/image_data.py | 44 ++++--- glance/api/v2/images.py | 49 +++++--- .../flows/_internal_plugins/web_download.py | 26 ++-- glance/async_/flows/api_image_import.py | 70 +++++++---- glance/async_/flows/base_import.py | 43 +++++-- glance/async_/flows/convert.py | 8 +- glance/async_/flows/ovf_process.py | 8 +- glance/async_/taskflow_executor.py | 4 +- .../common/scripts/api_image_import/main.py | 4 +- glance/common/store_utils.py | 10 +- glance/common/wsgi.py | 16 ++- glance/common/wsgi_app.py | 16 ++- glance/tests/functional/__init__.py | 5 +- glance/tests/functional/v2/test_images.py | 12 ++ glance/tests/unit/common/test_wsgi.py | 10 ++ glance/tests/unit/v2/test_discovery_stores.py | 13 ++ ...ng-filesystem-access-120bc46064b3d40a.yaml | 57 +++++++++ 19 files changed, 407 insertions(+), 107 deletions(-) create mode 100644 releasenotes/notes/rethinking-filesystem-access-120bc46064b3d40a.yaml diff --git a/doc/source/admin/multistores.rst b/doc/source/admin/multistores.rst index 9956571193..67260de538 100644 --- a/doc/source/admin/multistores.rst +++ b/doc/source/admin/multistores.rst @@ -16,11 +16,8 @@ Multi Store Support =================== -.. note:: The Multi Store feature is introduced as EXPERIMENTAL in Rocky and - its use in production systems is currently **not supported**. - However we encourage people to use this feature for testing - purposes and report the issues so that we can make it stable and - fully supported in Stein release. +.. note:: The Multi Store feature was introduced as EXPERIMENTAL in Rocky + and is now fully supported in the Train release. Scope of this document ---------------------- @@ -53,10 +50,10 @@ operators to enable multiple stores support. multiple stores operator can specify multiple key:value separated by comma. - Due to the special read only nature and characteristics of the - http store type, we do not encourage nor support configuring - multiple instances of the http type store even though it's - possible. + .. warning:: + The store identifier prefix ``os_glance_`` is reserved. If you + define a store identifier with this prefix, the glance service will + refuse to start. The http store type is always treated by Glance as a read-only store. This is indicated in the response to the ``/v2/stores/info`` @@ -112,15 +109,98 @@ operators to enable multiple stores support. .. note :: ``store_description`` is a new config option added to each store where - operator can add meaningful description about that store. This description - is displayed in the GET /v2/info/stores response. + operator can add meaningful description about that store. This + description is displayed in the GET /v2/info/stores response. -* For new image import workflow glance will reserve a ``os_staging`` file - store identifier for staging the images data during staging operation. This - should be added by default in ``glance-api.conf`` as shown below: +Store Configuration Issues +~~~~~~~~~~~~~~~~~~~~~~~~~~ - .. code-block:: ini +Please keep the following points in mind. - [os_staging] - filesystem_store_datadir = /opt/stack/data/glance/os_staging/ - store_description = "Filesystem store for staging purpose" +* Due to the special read only nature and characteristics of the + http store type, configuring multiple instances of the http type + store **is not supported**. (This constraint is not currently + enforced in the code.) + +* Each instance of the filesystem store **must** have a different value + for the ``filesystem_store_datadir``. (This constraint is not currently + enforced in the code.) + + +Reserved Stores +--------------- + +With the Train release, Glance is beginning a transition from its former +reliance upon local directories for temporary data storage to the ability +to use backend stores accessed via the glance_store library. + +In the Train release, the use of backend stores for this purpose is optional +**unless you are using the multi store support feature**. Since you are +reading this document, this situation most likely applies to you. + +.. note:: + Currently, only the filesystem store type is supported as a Glance + reserved store. + +The reserved stores are not intended to be exposed to end users. Thus +they will not appear in the response to the store discovery call, GET +/v2/info/stores, or as values in the ``OpenStack-image-store-ids`` +response header of the image-create call. + +You do not get to select the name of a reserved store; these are defined +by Glance and begin with the prefix ``os_glance_``. In the Train release, +you do not get to select the store type: all reserved stores must be of +type filesystem. + +Currently, there are two reserved stores: + +``os_glance_tasks_store`` + This store is used for the tasks engine. It replaces the use of the + DEPRECATED configuration option ``[task]/work_dir``. + +``os_glance_staging_store`` + This store is used for the staging area for the interoperable image + import process. It replaces the use of the DEPRECATED configuration + option ``[DEFAULT]/node_staging_uri``. + +Configuration +~~~~~~~~~~~~~ + +As mentioned above, you do not get to select the name or the type of +a reserved store (though we anticipate that you will be able configure +the store type in a future release). + +The reserved stores *must* be of type filesystem. Hence, you must +provide configuration for them in your ``glance-api.conf`` file. You +do this by introducing a section in ``glance-api.conf`` for each reserved +store as follows: + +.. code-block:: ini + + [os_glance_tasks_store] + filesystem_store_datadir = /var/lib/glance/tasks_work_dir + + [os_glance_staging_store] + filesystem_store_datadir = /var/lib/glance/staging + +Since these are both filesystem stores (remember, you do not get a choice) +the only option you must configure for each is the +``filesystem_store_datadir``. Please keep the following points in mind: + +* The path for ``filesystem_store_datadir`` used for the reserved + stores must be **different** from the path you are using for + any filesystem store you have listed in ``enabled_backends``. + Using the same data directory for multiple filesystem stores is + **unsupported** and may lead to data loss. + +* The identifiers for reserved stores, that is, ``os_glance_tasks_store`` + and ``os_glance_staging_store``, must **not** be included in the + ``enabled_backends`` list. + +* The reserved stores will **not** appear in the store discovery response + or as values in the ``OpenStack-image-store-ids`` response header of + the image-create call. + +* The reserved stores will **not** be accepted as the value of the + ``X-Image-Meta-Store`` header on the image-data-upload call or + the image-import call. diff --git a/glance/api/v2/discovery.py b/glance/api/v2/discovery.py index 4e8f993f3f..1002c8a5db 100644 --- a/glance/api/v2/discovery.py +++ b/glance/api/v2/discovery.py @@ -46,6 +46,9 @@ class InfoController(object): backends = [] for backend in enabled_backends: + if backend.startswith("os_glance_"): + continue + stores = {} stores['id'] = backend description = getattr(CONF, backend).store_description diff --git a/glance/api/v2/image_data.py b/glance/api/v2/image_data.py index bdf9798cc4..b15e4eb58f 100644 --- a/glance/api/v2/image_data.py +++ b/glance/api/v2/image_data.py @@ -17,6 +17,7 @@ import os from cursive import exception as cursive_exception import glance_store from glance_store import backend +from glance_store import location from oslo_config import cfg from oslo_log import log as logging from oslo_utils import encodeutils @@ -75,21 +76,29 @@ class ImageDataController(object): :param image: The image will be restored :param staging_store: The store used for staging """ - # NOTE(abhishek): staging_store not being used in this function - # because of bug #1803498 - # TODO(abhishek): refactor to use the staging_store when the - # "Rethinking Filesystem Access" spec is implemented in Train - file_path = str(CONF.node_staging_uri + '/' + image.image_id)[7:] - if os.path.exists(file_path): + if CONF.enabled_backends: + file_path = "%s/%s" % (getattr( + CONF, 'os_glance_staging_store').filesystem_store_datadir, + image.image_id) try: - os.unlink(file_path) - except OSError as e: - LOG.error(_("Cannot delete staged image data %(fn)s " - "[Errno %(en)d]"), {'fn': file_path, - 'en': e.errno}) + loc = location.get_location_from_uri_and_backend( + file_path, 'os_glance_staging_store') + staging_store.delete(loc) + except (glance_store.exceptions.NotFound, + glance_store.exceptions.UnknownScheme): + pass else: - LOG.warning(_("Staged image data not found " - "at %(fn)s"), {'fn': file_path}) + file_path = str(CONF.node_staging_uri + '/' + image.image_id)[7:] + if os.path.exists(file_path): + try: + os.unlink(file_path) + except OSError as e: + LOG.error(_("Cannot delete staged image data %(fn)s " + "[Errno %(en)d]"), {'fn': file_path, + 'en': e.errno}) + else: + LOG.warning(_("Staged image data not found " + "at %(fn)s"), {'fn': file_path}) self._restore(image_repo, image) @@ -320,7 +329,14 @@ class ImageDataController(object): raise exception.BadStoreUri(message=msg) return staging_store - staging_store = _build_staging_store() + # NOTE(abhishekk): Use reserved 'os_glance_staging_store' for staging + # the data, the else part will be removed once multiple backend feature + # is declared as stable. + if CONF.enabled_backends: + staging_store = glance_store.get_store_from_store_identifier( + 'os_glance_staging_store') + else: + staging_store = _build_staging_store() try: image = image_repo.get(image_id) diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py index 258bc4bc0d..41903bd439 100644 --- a/glance/api/v2/images.py +++ b/glance/api/v2/images.py @@ -20,6 +20,7 @@ import re from castellan.common import exception as castellan_exception from castellan import key_manager import glance_store +from glance_store import location from oslo_config import cfg from oslo_log import log as logging from oslo_serialization import jsonutils as json @@ -368,25 +369,39 @@ class ImagesController(object): image = image_repo.get(image_id) if image.status == 'uploading': - file_path = str( - CONF.node_staging_uri + '/' + image.image_id)[7:] - if os.path.exists(file_path): + if CONF.enabled_backends: + file_path = "%s/%s" % (getattr( + CONF, 'os_glance_staging_store' + ).filesystem_store_datadir, image_id) try: - LOG.debug( - "After upload to the backend, deleting staged " - "image data from %(fn)s", {'fn': file_path}) - os.unlink(file_path) - except OSError as e: - LOG.error( - "After upload to backend, deletion of staged " - "image data from %(fn)s has failed because " - "[Errno %(en)d]", {'fn': file_path, - 'en': e.errno}) + fn_call = glance_store.get_store_from_store_identifier + staging_store = fn_call('os_glance_staging_store') + loc = location.get_location_from_uri_and_backend( + file_path, 'os_glance_staging_store') + staging_store.delete(loc) + except (glance_store.exceptions.NotFound, + glance_store.exceptions.UnknownScheme): + pass else: - LOG.warning(_( - "After upload to backend, deletion of staged " - "image data has failed because " - "it cannot be found at %(fn)s"), {'fn': file_path}) + file_path = str( + CONF.node_staging_uri + '/' + image_id)[7:] + if os.path.exists(file_path): + try: + LOG.debug( + "After upload to the backend, deleting staged " + "image data from %(fn)s", {'fn': file_path}) + os.unlink(file_path) + except OSError as e: + LOG.error( + "After upload to backend, deletion of staged " + "image data from %(fn)s has failed because " + "[Errno %(en)d]", {'fn': file_path, + 'en': e.errno}) + else: + LOG.warning(_( + "After upload to backend, deletion of staged " + "image data has failed because " + "it cannot be found at %(fn)s"), {'fn': file_path}) image.delete() self._delete_encryption_key(req.context, image) diff --git a/glance/async_/flows/_internal_plugins/web_download.py b/glance/async_/flows/_internal_plugins/web_download.py index 678b4c1a76..ad1f58de9d 100644 --- a/glance/async_/flows/_internal_plugins/web_download.py +++ b/glance/async_/flows/_internal_plugins/web_download.py @@ -12,7 +12,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. - +import glance_store as store_api from glance_store import backend from oslo_config import cfg from oslo_log import log as logging @@ -44,15 +44,22 @@ class _WebDownload(task.Task): super(_WebDownload, self).__init__( name='%s-WebDownload-%s' % (task_type, task_id)) - if CONF.node_staging_uri is None: - msg = (_("%(task_id)s of %(task_type)s not configured " - "properly. Missing node_staging_uri: %(work_dir)s") % - {'task_id': self.task_id, - 'task_type': self.task_type, - 'work_dir': CONF.node_staging_uri}) - raise exception.BadTaskConfiguration(msg) + # NOTE(abhishekk): Use reserved 'os_glance_staging_store' for + # staging the data, the else part will be removed once old way + # of configuring store is deprecated. + if CONF.enabled_backends: + self.store = store_api.get_store_from_store_identifier( + 'os_glance_staging_store') + else: + if CONF.node_staging_uri is None: + msg = (_("%(task_id)s of %(task_type)s not configured " + "properly. Missing node_staging_uri: %(work_dir)s") % + {'task_id': self.task_id, + 'task_type': self.task_type, + 'work_dir': CONF.node_staging_uri}) + raise exception.BadTaskConfiguration(msg) - self.store = self._build_store() + self.store = self._build_store() def _build_store(self): # NOTE(flaper87): Due to the nice glance_store api (#sarcasm), we're @@ -111,7 +118,6 @@ class _WebDownload(task.Task): "task_id": self.task_id}) path = self.store.add(self.image_id, data, 0)[0] - return path def revert(self, result, **kwargs): diff --git a/glance/async_/flows/api_image_import.py b/glance/async_/flows/api_image_import.py index b789db37b4..147da0f889 100644 --- a/glance/async_/flows/api_image_import.py +++ b/glance/async_/flows/api_image_import.py @@ -14,6 +14,7 @@ # under the License. import os +import glance_store as store_api from glance_store import backend from oslo_config import cfg from oslo_log import log as logging @@ -86,23 +87,28 @@ class _DeleteFromFS(task.Task): :param file_path: path to the file being deleted """ - # TODO(abhishekk): After removal of backend module from glance_store - # need to change this to use multi_backend module. - file_path = file_path[7:] - if os.path.exists(file_path): - try: - LOG.debug(_("After upload to the backend, deleting staged " - "image data from %(fn)s"), {'fn': file_path}) - os.unlink(file_path) - except OSError as e: - LOG.error(_("After upload to backend, deletion of staged " - "image data from %(fn)s has failed because " - "[Errno %(en)d]"), {'fn': file_path, - 'en': e.errno}) + if CONF.enabled_backends: + store_api.delete(file_path, 'os_glance_staging_store') else: - LOG.warning(_("After upload to backend, deletion of staged " - "image data has failed because " - "it cannot be found at %(fn)s"), {'fn': file_path}) + # TODO(abhishekk): After removal of backend module from + # glance_store need to change this to use multi_backend + # module. + file_path = file_path[7:] + if os.path.exists(file_path): + try: + LOG.debug(_("After upload to the backend, deleting staged " + "image data from %(fn)s"), {'fn': file_path}) + os.unlink(file_path) + except OSError as e: + LOG.error(_("After upload to backend, deletion of staged " + "image data from %(fn)s has failed because " + "[Errno %(en)d]"), {'fn': file_path, + 'en': e.errno}) + else: + LOG.warning(_("After upload to backend, deletion of staged " + "image data has failed because " + "it cannot be found at %(fn)s"), { + 'fn': file_path}) class _VerifyStaging(task.Task): @@ -132,10 +138,11 @@ class _VerifyStaging(task.Task): 'task_type': self.task_type}) raise exception.BadTaskConfiguration(msg) - # NOTE(jokke): We really don't need the store for anything but - # verifying that we actually can build the store will allow us to - # fail the flow early with clear message why that happens. - self._build_store() + if not CONF.enabled_backends: + # NOTE(jokke): We really don't need the store for anything but + # verifying that we actually can build the store will allow us to + # fail the flow early with clear message why that happens. + self._build_store() def _build_store(self): # TODO(abhishekk): After removal of backend module from glance_store @@ -332,18 +339,26 @@ def get_flow(**kwargs): backend = kwargs.get('backend') separator = '' - if not CONF.node_staging_uri.endswith('/'): + if not CONF.enabled_backends and not CONF.node_staging_uri.endswith('/'): separator = '/' if not uri and import_method == 'glance-direct': - uri = separator.join((CONF.node_staging_uri, str(image_id))) + if CONF.enabled_backends: + separator, staging_dir = _get_dir_separator() + uri = separator.join((staging_dir, str(image_id))) + else: + uri = separator.join((CONF.node_staging_uri, str(image_id))) flow = lf.Flow(task_type, retry=retry.AlwaysRevert()) if import_method == 'web-download': downloadToStaging = internal_plugins.get_import_plugin(**kwargs) flow.add(downloadToStaging) - file_uri = separator.join((CONF.node_staging_uri, str(image_id))) + if CONF.enabled_backends: + separator, staging_dir = _get_dir_separator() + file_uri = separator.join((staging_dir, str(image_id))) + else: + file_uri = separator.join((CONF.node_staging_uri, str(image_id))) else: file_uri = uri @@ -381,3 +396,12 @@ def get_flow(**kwargs): image_repo.save(image, from_state=from_state) return flow + + +def _get_dir_separator(): + separator = '' + staging_dir = "file://%s" % getattr( + CONF, 'os_glance_staging_store').filesystem_store_datadir + if not staging_dir.endswith('/'): + separator = '/' + return separator, staging_dir diff --git a/glance/async_/flows/base_import.py b/glance/async_/flows/base_import.py index 66a0be1e2e..11071d14c0 100644 --- a/glance/async_/flows/base_import.py +++ b/glance/async_/flows/base_import.py @@ -95,15 +95,22 @@ class _ImportToFS(task.Task): super(_ImportToFS, self).__init__( name='%s-ImportToFS-%s' % (task_type, task_id)) - if CONF.task.work_dir is None: - msg = (_("%(task_id)s of %(task_type)s not configured " - "properly. Missing work dir: %(work_dir)s") % - {'task_id': self.task_id, - 'task_type': self.task_type, - 'work_dir': CONF.task.work_dir}) - raise exception.BadTaskConfiguration(msg) + # NOTE(abhishekk): Use reserved 'os_glance_tasks_store' for tasks, + # the else part will be removed once old way of configuring store + # is deprecated. + if CONF.enabled_backends: + self.store = store_api.get_store_from_store_identifier( + 'os_glance_tasks_store') + else: + if CONF.task.work_dir is None: + msg = (_("%(task_id)s of %(task_type)s not configured " + "properly. Missing work dir: %(work_dir)s") % + {'task_id': self.task_id, + 'task_type': self.task_type, + 'work_dir': CONF.task.work_dir}) + raise exception.BadTaskConfiguration(msg) - self.store = self._build_store() + self.store = self._build_store() def _build_store(self): # NOTE(flaper87): Due to the nice glance_store api (#sarcasm), we're @@ -185,7 +192,10 @@ class _ImportToFS(task.Task): return if os.path.exists(result.split("file://")[-1]): - store_api.delete_from_backend(result) + if CONF.enabled_backends: + store_api.delete(result, 'os_glance_tasks_store') + else: + store_api.delete_from_backend(result) class _DeleteFromFS(task.Task): @@ -201,16 +211,20 @@ class _DeleteFromFS(task.Task): :param file_path: path to the file being deleted """ - store_api.delete_from_backend(file_path) + if CONF.enabled_backends: + store_api.delete(file_path, 'os_glance_tasks_store') + else: + store_api.delete_from_backend(file_path) class _ImportToStore(task.Task): - def __init__(self, task_id, task_type, image_repo, uri): + def __init__(self, task_id, task_type, image_repo, uri, backend): self.task_id = task_id self.task_type = task_type self.image_repo = image_repo self.uri = uri + self.backend = backend super(_ImportToStore, self).__init__( name='%s-ImportToStore-%s' % (task_type, task_id)) @@ -311,7 +325,8 @@ class _ImportToStore(task.Task): # image_import.set_image_data(image, image_path, None) try: image_import.set_image_data(image, - file_path or self.uri, self.task_id) + file_path or self.uri, self.task_id, + backend=self.backend) except IOError as e: msg = (_('Uploading the image failed due to: %(exc)s') % {'exc': encodeutils.exception_to_unicode(e)}) @@ -423,11 +438,13 @@ def get_flow(**kwargs): image_repo = kwargs.get('image_repo') image_factory = kwargs.get('image_factory') uri = kwargs.get('uri') + backend = kwargs.get('backend') flow = lf.Flow(task_type, retry=retry.AlwaysRevert()).add( _CreateImage(task_id, task_type, task_repo, image_repo, image_factory)) - import_to_store = _ImportToStore(task_id, task_type, image_repo, uri) + import_to_store = _ImportToStore(task_id, task_type, image_repo, uri, + backend) try: # NOTE(flaper87): ImportToLocal and DeleteFromLocal shouldn't be here. diff --git a/glance/async_/flows/convert.py b/glance/async_/flows/convert.py index 48732004e4..5dcf5a3bf4 100644 --- a/glance/async_/flows/convert.py +++ b/glance/async_/flows/convert.py @@ -114,7 +114,13 @@ class _Convert(task.Task): # shields us from being vulnerable to an attack vector described here # https://bugs.launchpad.net/glance/+bug/1449062 - dest_path = os.path.join(CONF.task.work_dir, "%s.converted" % image_id) + data_dir = CONF.task.work_dir + # NOTE(abhishekk): Use reserved 'os_glance_tasks_store' for tasks. + if CONF.enabled_backends: + data_dir = getattr( + CONF, 'os_glance_tasks_store').filesystem_store_datadir + + dest_path = os.path.join(data_dir, "%s.converted" % image_id) stdout, stderr = putils.trycmd('qemu-img', 'convert', '-f', src_format, '-O', conversion_format, diff --git a/glance/async_/flows/ovf_process.py b/glance/async_/flows/ovf_process.py index eea581fc9b..5e2b2e9d24 100644 --- a/glance/async_/flows/ovf_process.py +++ b/glance/async_/flows/ovf_process.py @@ -59,7 +59,13 @@ class _OVF_Process(task.Task): name='%s-OVF_Process-%s' % (task_type, task_id)) def _get_extracted_file_path(self, image_id): - return os.path.join(CONF.task.work_dir, + file_path = CONF.task.work_dir + # NOTE(abhishekk): Use reserved 'os_glance_tasks_store' for tasks. + if CONF.enabled_backends: + file_path = getattr( + CONF, 'os_glance_tasks_store').filesystem_store_datadir + + return os.path.join(file_path, "%s.extracted" % image_id) def _get_ova_iter_objects(self, uri): diff --git a/glance/async_/taskflow_executor.py b/glance/async_/taskflow_executor.py index 943c33cfc3..81cd2c2a61 100644 --- a/glance/async_/taskflow_executor.py +++ b/glance/async_/taskflow_executor.py @@ -119,7 +119,8 @@ class TaskExecutor(glance.async_.TaskExecutor): 'context': self.context, 'task_repo': self.task_repo, 'image_repo': self.image_repo, - 'image_factory': self.image_factory + 'image_factory': self.image_factory, + 'backend': task_input.get('backend') } if task.type == "import": @@ -129,7 +130,6 @@ class TaskExecutor(glance.async_.TaskExecutor): if task.type == 'api_image_import': kwds['image_id'] = task_input['image_id'] kwds['import_req'] = task_input['import_req'] - kwds['backend'] = task_input['backend'] return driver.DriverManager('glance.flows', task.type, invoke_on_load=True, invoke_kwds=kwds).driver diff --git a/glance/common/scripts/api_image_import/main.py b/glance/common/scripts/api_image_import/main.py index 4c5bee1278..38d3be055e 100644 --- a/glance/common/scripts/api_image_import/main.py +++ b/glance/common/scripts/api_image_import/main.py @@ -116,13 +116,13 @@ def import_image(image_repo, image_factory, task_input, task_id, uri): location) -def set_image_data(image, uri, task_id): +def set_image_data(image, uri, task_id, backend=None): data_iter = None try: LOG.info("Task %(task_id)s: Got image data uri %(data_uri)s to be " "imported", {"data_uri": uri, "task_id": task_id}) data_iter = script_utils.get_image_data_iter(uri) - image.set_data(data_iter) + image.set_data(data_iter, backend=backend) except Exception as e: with excutils.save_and_reraise_exception(): LOG.warn("Task %(task_id)s failed with exception %(error)s" % diff --git a/glance/common/store_utils.py b/glance/common/store_utils.py index 8a9379c6df..610dc8e744 100644 --- a/glance/common/store_utils.py +++ b/glance/common/store_utils.py @@ -32,6 +32,13 @@ CONF.import_opt('use_user_token', 'glance.registry.client') RESTRICTED_URI_SCHEMAS = frozenset(['file', 'filesystem', 'swift+config']) +def check_reserved_stores(enabled_stores): + for store in enabled_stores: + if store.startswith("os_glance_"): + return True + return False + + def safe_delete_from_backend(context, image_id, location): """ Given a location, delete an image from the store and @@ -159,7 +166,8 @@ def _get_store_id_from_uri(uri): return for store in location_map[scheme]: store_instance = location_map[scheme][store]['store'] - if uri.startswith(store_instance.url_prefix): + url_prefix = store_instance.url_prefix + if url_prefix and uri.startswith(url_prefix): url_matched = True break diff --git a/glance/common/wsgi.py b/glance/common/wsgi.py index 811cca8cba..e63fa4a517 100644 --- a/glance/common/wsgi.py +++ b/glance/common/wsgi.py @@ -54,6 +54,7 @@ from webob import multidict from glance.common import config from glance.common import exception +from glance.common import store_utils from glance.common import utils from glance import i18n from glance.i18n import _, _LE, _LI, _LW @@ -370,6 +371,12 @@ except ImportError: LOG.debug('Detected not running under uwsgi') uwsgi = None +# Reserved file stores for staging and tasks operations +RESERVED_STORES = { + 'os_glance_staging_store': 'file', + 'os_glance_tasks_store': 'file' +} + def register_cli_opts(): CONF.register_cli_opts(cli_opts) @@ -492,8 +499,8 @@ def initialize_glance_store(): def initialize_multi_store(): """Initialize glance multi store backends.""" - glance_store.register_store_opts(CONF) - glance_store.create_multi_stores(CONF) + glance_store.register_store_opts(CONF, reserved_stores=RESERVED_STORES) + glance_store.create_multi_stores(CONF, reserved_stores=RESERVED_STORES) glance_store.verify_store() @@ -619,6 +626,11 @@ class BaseServer(object): self.configure_socket(old_conf, has_changed) if self.initialize_glance_store: if CONF.enabled_backends: + if store_utils.check_reserved_stores(CONF.enabled_backends): + msg = _("'os_glance_' prefix should not be used in " + "enabled_backends config option. It is reserved " + "for internal use only.") + raise RuntimeError(msg) initialize_multi_store() else: initialize_glance_store() diff --git a/glance/common/wsgi_app.py b/glance/common/wsgi_app.py index f4675746c4..e505e14c75 100644 --- a/glance/common/wsgi_app.py +++ b/glance/common/wsgi_app.py @@ -18,6 +18,7 @@ from oslo_log import log as logging import osprofiler.initializer from glance.common import config +from glance.common import store_utils from glance import notifier CONF = cfg.CONF @@ -29,6 +30,12 @@ CONFIG_FILES = ['glance-api-paste.ini', 'glance-image-import.conf', 'glance-api.conf'] +# Reserved file stores for staging and tasks operations +RESERVED_STORES = { + 'os_glance_staging_store': 'file', + 'os_glance_tasks_store': 'file' +} + def _get_config_files(env=None): if env is None: @@ -63,8 +70,13 @@ def init_app(): logging.setup(CONF, "glance") if CONF.enabled_backends: - glance_store.register_store_opts(CONF) - glance_store.create_multi_stores(CONF) + if store_utils.check_reserved_stores(CONF.enabled_backends): + msg = _("'os_glance_' prefix should not be used in " + "enabled_backends config option. It is reserved " + "for internal use only.") + raise RuntimeError(msg) + glance_store.register_store_opts(CONF, reserved_stores=RESERVED_STORES) + glance_store.create_multi_stores(CONF, reserved_stores=RESERVED_STORES) glance_store.verify_store() else: glance_store.register_opts(CONF) diff --git a/glance/tests/functional/__init__.py b/glance/tests/functional/__init__.py index 1ed043c02c..938313254f 100644 --- a/glance/tests/functional/__init__.py +++ b/glance/tests/functional/__init__.py @@ -565,6 +565,7 @@ class ApiServerForMultipleBackend(Server): self.metadata_encryption_key = "012345678901234567890123456789ab" self.image_dir_backend_1 = os.path.join(self.test_dir, "images_1") self.image_dir_backend_2 = os.path.join(self.test_dir, "images_2") + self.staging_dir = os.path.join(self.test_dir, "staging") self.pid_file = pid_file or os.path.join(self.test_dir, "multiple_backend_api.pid") self.log_file = os.path.join(self.test_dir, "multiple_backend_api.log") @@ -635,7 +636,7 @@ image_tag_quota=%(image_tag_quota)s image_location_quota=%(image_location_quota)s location_strategy=%(location_strategy)s allow_additional_image_properties = True -enabled_backends=file1:file, file2:file +enabled_backends=file1:file,file2:file [oslo_policy] policy_file = %(policy_file)s policy_default_rule = %(policy_default_rule)s @@ -651,6 +652,8 @@ filesystem_store_datadir=%(image_dir_backend_1)s filesystem_store_datadir=%(image_dir_backend_2)s [import_filtering_opts] allowed_ports = [] +[os_glance_staging_store] +filesystem_store_datadir=%(staging_dir)s """ self.paste_conf_base = """[pipeline:glance-api] pipeline = diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py index d167e8fe39..c3d7d34157 100644 --- a/glance/tests/functional/v2/test_images.py +++ b/glance/tests/functional/v2/test_images.py @@ -4506,9 +4506,11 @@ class TestImagesMultipleBackend(functional.MultipleBackendFunctionalTest): self.assertEqual(http.OK, response.status_code) discovery_calls = jsonutils.loads( response.text)['stores'] + # os_glance_staging_store should not be available in discovery response for stores in discovery_calls: self.assertIn('id', stores) self.assertIn(stores['id'], available_stores) + self.assertFalse(stores["id"].startswith("os_glance_")) # Create an image path = self._url('/v2/images') @@ -4667,9 +4669,11 @@ class TestImagesMultipleBackend(functional.MultipleBackendFunctionalTest): self.assertEqual(http.OK, response.status_code) discovery_calls = jsonutils.loads( response.text)['stores'] + # os_glance_staging_store should not be available in discovery response for stores in discovery_calls: self.assertIn('id', stores) self.assertIn(stores['id'], available_stores) + self.assertFalse(stores["id"].startswith("os_glance_")) # Create an image path = self._url('/v2/images') @@ -4829,9 +4833,11 @@ class TestImagesMultipleBackend(functional.MultipleBackendFunctionalTest): self.assertEqual(http.OK, response.status_code) discovery_calls = jsonutils.loads( response.text)['stores'] + # os_glance_staging_store should not be available in discovery response for stores in discovery_calls: self.assertIn('id', stores) self.assertIn(stores['id'], available_stores) + self.assertFalse(stores["id"].startswith("os_glance_")) # Create an image path = self._url('/v2/images') @@ -4990,9 +4996,11 @@ class TestImagesMultipleBackend(functional.MultipleBackendFunctionalTest): self.assertEqual(http.OK, response.status_code) discovery_calls = jsonutils.loads( response.text)['stores'] + # os_glance_staging_store should not be available in discovery response for stores in discovery_calls: self.assertIn('id', stores) self.assertIn(stores['id'], available_stores) + self.assertFalse(stores["id"].startswith("os_glance_")) # Create an image path = self._url('/v2/images') @@ -5142,9 +5150,11 @@ class TestImagesMultipleBackend(functional.MultipleBackendFunctionalTest): self.assertEqual(http.OK, response.status_code) discovery_calls = jsonutils.loads( response.text)['stores'] + # os_glance_staging_store should not be available in discovery response for stores in discovery_calls: self.assertIn('id', stores) self.assertIn(stores['id'], available_stores) + self.assertFalse(stores["id"].startswith("os_glance_")) # Create an image (with two deployer-defined properties) path = self._url('/v2/images') @@ -5309,9 +5319,11 @@ class TestImagesMultipleBackend(functional.MultipleBackendFunctionalTest): self.assertEqual(http.OK, response.status_code) discovery_calls = jsonutils.loads( response.text)['stores'] + # os_glance_staging_store should not be available in discovery response for stores in discovery_calls: self.assertIn('id', stores) self.assertIn(stores['id'], available_stores) + self.assertFalse(stores["id"].startswith("os_glance_")) # Create an image (with two deployer-defined properties) path = self._url('/v2/images') diff --git a/glance/tests/unit/common/test_wsgi.py b/glance/tests/unit/common/test_wsgi.py index c66f92fdcc..0a205b4330 100644 --- a/glance/tests/unit/common/test_wsgi.py +++ b/glance/tests/unit/common/test_wsgi.py @@ -568,6 +568,16 @@ class ServerTest(test_utils.BaseTestCase): actual = wsgi.Server(threads=1).create_pool() self.assertIsInstance(actual, eventlet.greenpool.GreenPool) + @mock.patch.object(prefetcher, 'Prefetcher') + @mock.patch.object(wsgi.Server, 'configure_socket') + def test_reserved_stores_not_allowed(self, mock_configure_socket, + mock_prefetcher): + """Ensure the reserved stores are not allowed""" + enabled_backends = {'os_glance_file_store': 'file'} + self.config(enabled_backends=enabled_backends) + server = wsgi.Server(threads=1, initialize_glance_store=True) + self.assertRaises(RuntimeError, server.configure) + @mock.patch.object(prefetcher, 'Prefetcher') @mock.patch.object(wsgi.Server, 'configure_socket') def test_http_keepalive(self, mock_configure_socket, mock_prefetcher): diff --git a/glance/tests/unit/v2/test_discovery_stores.py b/glance/tests/unit/v2/test_discovery_stores.py index 4de7976051..51974bf3af 100644 --- a/glance/tests/unit/v2/test_discovery_stores.py +++ b/glance/tests/unit/v2/test_discovery_stores.py @@ -59,3 +59,16 @@ class TestInfoControllers(base.MultiStoreClearingUnitTest): self.assertTrue(stores['read-only']) else: self.assertIsNone(stores.get('read-only')) + + def test_get_stores_reserved_stores_excluded(self): + enabled_backends = { + 'fast': 'file', + 'cheap': 'file' + } + self.config(enabled_backends=enabled_backends) + req = unit_test_utils.get_fake_request() + output = self.controller.get_stores(req) + self.assertIn('stores', output) + self.assertEqual(2, len(output['stores'])) + for stores in output["stores"]: + self.assertFalse(stores["id"].startswith("os_glance_")) diff --git a/releasenotes/notes/rethinking-filesystem-access-120bc46064b3d40a.yaml b/releasenotes/notes/rethinking-filesystem-access-120bc46064b3d40a.yaml new file mode 100644 index 0000000000..0944e46308 --- /dev/null +++ b/releasenotes/notes/rethinking-filesystem-access-120bc46064b3d40a.yaml @@ -0,0 +1,57 @@ +--- +features: + - | + With the introduction of the Glance multiple stores feature, introduced + on an experimental basis in Rocky and now established as a full feature + in the Train release, it is now possible for Glance to use backends + accessed via the glance_store library for the temporary storage of + data that previously required access to the local filesystem. Please + note the following: + + * In this release, the use of stores (instead of local directories) is + optional, but it will become mandatory for the 'U' release. + + * In this release, the stores used *must* be the filesystem store type. + Our goal is that in a future release, operators will be able to + configure a store of their choice for these functions. In Train, + however, each of these *must* be a filesystem store. + + Please see the Upgrades section of this document and the "Multi Store + Support" chapter of the Glance Administration Guide for more information. + +upgrade: + - | + The configuration options ``work_dir`` and ``node_staging_uri`` are + deprecated and will be removed early in the 'U' development cycle. + + These local directories are used by Glance for the temporary storage + of data during the interoperable image import process and by the + tasks engine. This release introduces the ability to instead use a + backend filesystem store accessed via the glance_store library for this + temporary storage. Please note the following: + + * If you wish to use the backend store feature now, please see the + "Reserved Stores" section of the "Multi Store Support" chapter of + the Glance Administration Guide for configuration information. + + * If you use the Glance multiple stores feature, introduced on an + experimental basis in Rocky and now fully supported in the Train + release, then you *must* use backing stores instead of ``work_dir`` + and ``node_staging_uri`` for Glance's temporary storage **beginning + right now with the current release**. See the "Reserved Stores" + section of the "Multi Store Support" chapter of the Glance + Administration Guide for more information. + + - | + The store name prefix ``os_glance_*`` is reserved by Glance for internal + stores. Glance will refuse to start if a store with this prefix is + included in the ``enabled_backends`` option. + + The internal store identifiers introduced in this release are + ``os_glance_tasks_store`` and ``os_glance_staging_store``. + +issues: + - | + When using the multiple stores feature, each filesystem store **must** + be configured with a different value for the ``filesystem_store_datadir`` + option. This is not currently enforced in the code.