From d886d6d7e73dc5484e7171b7af318b1bcb2598c8 Mon Sep 17 00:00:00 2001 From: wangxiyuan Date: Mon, 9 Oct 2017 16:56:57 +0800 Subject: [PATCH] Scrubber refactor The glance-scrubber utility is used by administrators for the offline deletion of images when the Glance option `delayed_delete` is enabled. The refactoring in this patch eliminates a dependency on the Glance Registry v1 client. Further, after this change, the glance-scrubber does not use the deprecated Glance Registry at all. Instead, like the glance-manage tool, it now visits the Glance database directly. bp: scrubber-refactor Change-Id: I26f570b85617200880543b7114730a1ac76d3fb1 --- glance/opts.py | 5 +- glance/scrubber.py | 142 +++--------------- glance/tests/unit/test_scrubber.py | 30 ++-- .../scrubber-refactor-73ddbd61ebbf1e86.yaml | 8 + 4 files changed, 40 insertions(+), 145 deletions(-) create mode 100644 releasenotes/notes/scrubber-refactor-73ddbd61ebbf1e86.yaml diff --git a/glance/opts.py b/glance/opts.py index 033b26868d..df67f06a27 100644 --- a/glance/opts.py +++ b/glance/opts.py @@ -90,10 +90,7 @@ _scrubber_opts = [ glance.common.config.common_opts, glance.scrubber.scrubber_opts, glance.scrubber.scrubber_cmd_opts, - glance.scrubber.scrubber_cmd_cli_opts, - glance.registry.client.registry_client_opts, - glance.registry.client.registry_client_ctx_opts, - glance.registry.registry_addr_opts))), + glance.scrubber.scrubber_cmd_cli_opts))), ] _cache_opts = [ (None, list(itertools.chain( diff --git a/glance/scrubber.py b/glance/scrubber.py index 1894c7f2d0..dfc3bef499 100644 --- a/glance/scrubber.py +++ b/glance/scrubber.py @@ -21,14 +21,13 @@ from glance_store import exceptions as store_exceptions from oslo_config import cfg from oslo_log import log as logging from oslo_utils import encodeutils -import six from glance.common import crypt from glance.common import exception +from glance.common import timeutils from glance import context import glance.db as db_api from glance.i18n import _, _LC, _LE, _LI, _LW -import glance.registry.client.v1.api as registry LOG = logging.getLogger(__name__) @@ -100,63 +99,6 @@ Related options: * ``wakeup_time`` * ``scrub_pool_size`` -""")), - - # Note: Though the conf option admin_role is used by other Glance - # service and their usage differs requiring us to have a differing - # help text here, oslo.config generator treats them as the same - # config option and would throw a DuplicateError exception in case - # of differing help texts. Hence we have the same help text for - # admin_role here and in context.py. - - cfg.StrOpt('admin_role', default='admin', - help=_(""" -Role used to identify an authenticated user as administrator. - -Provide a string value representing a Keystone role to identify an -administrative user. Users with this role will be granted -administrative privileges. The default value for this option is -'admin'. - -Possible values: - * A string value which is a valid Keystone role - -Related options: - * None - -""")), - cfg.BoolOpt('send_identity_headers', - default=False, - help=_(""" -Send headers received from identity when making requests to -registry. - -Typically, Glance registry can be deployed in multiple flavors, -which may or may not include authentication. For example, -``trusted-auth`` is a flavor that does not require the registry -service to authenticate the requests it receives. However, the -registry service may still need a user context to be populated to -serve the requests. This can be achieved by the caller -(the Glance API usually) passing through the headers it received -from authenticating with identity for the same request. The typical -headers sent are ``X-User-Id``, ``X-Tenant-Id``, ``X-Roles``, -``X-Identity-Status`` and ``X-Service-Catalog``. - -Provide a boolean value to determine whether to send the identity -headers to provide tenant and user information along with the -requests to registry service. By default, this option is set to -``False``, which means that user and tenant information is not -available readily. It must be obtained by authenticating. Hence, if -this is set to ``False``, ``flavor`` must be set to value that -either includes authentication or authenticated user context. - -Possible values: - * True - * False - -Related options: - * flavor - """)), ] @@ -214,6 +156,7 @@ Related options: CONF = cfg.CONF CONF.register_opts(scrubber_opts) CONF.import_opt('metadata_encryption_key', 'glance.common.config') +REASONABLE_DB_PAGE_SIZE = 1000 class ScrubDBQueue(object): @@ -221,26 +164,7 @@ class ScrubDBQueue(object): def __init__(self): self.scrub_time = CONF.scrub_time self.metadata_encryption_key = CONF.metadata_encryption_key - registry.configure_registry_client() - registry.configure_registry_admin_creds() - admin_user = CONF.admin_user - admin_tenant = CONF.admin_tenant_name - - if CONF.send_identity_headers: - # When registry is operating in trusted-auth mode - roles = [CONF.admin_role] - self.admin_context = context.RequestContext(user=admin_user, - tenant=admin_tenant, - auth_token=None, - roles=roles) - self.registry = registry.get_registry_client(self.admin_context) - else: - ctxt = context.RequestContext() - self.registry = registry.get_registry_client(ctxt) - admin_token = self.registry.auth_token - self.admin_context = context.RequestContext(user=admin_user, - tenant=admin_tenant, - auth_token=admin_token) + self.admin_context = context.get_admin_context(show_deleted=True) def add_location(self, image_id, location): """Adding image location to scrub queue. @@ -261,14 +185,12 @@ class ScrubDBQueue(object): def _get_images_page(self, marker): filters = {'deleted': True, - 'is_public': 'none', 'status': 'pending_delete'} - if marker: - return self.registry.get_images_detailed(filters=filters, - marker=marker) - else: - return self.registry.get_images_detailed(filters=filters) + return db_api.get_api().image_get_all(self.admin_context, + filters=filters, + marker=marker, + limit=REASONABLE_DB_PAGE_SIZE) def _get_all_images(self): """Generator to fetch all appropriate images, paging as needed.""" @@ -296,23 +218,23 @@ class ScrubDBQueue(object): deleted_at = image.get('deleted_at') if not deleted_at: continue - # NOTE: Strip off microseconds which may occur after the last '.,' # Example: 2012-07-07T19:14:34.974216 + deleted_at = timeutils.isotime(deleted_at) date_str = deleted_at.rsplit('.', 1)[0].rsplit(',', 1)[0] delete_time = calendar.timegm(time.strptime(date_str, - "%Y-%m-%dT%H:%M:%S")) + "%Y-%m-%dT%H:%M:%SZ")) if delete_time + self.scrub_time > time.time(): continue - for loc in image['location_data']: + for loc in image['locations']: if loc['status'] != 'pending_delete': continue if self.metadata_encryption_key: - uri = crypt.urlsafe_encrypt(self.metadata_encryption_key, - loc['url'], 64) + uri = crypt.urlsafe_decrypt(self.metadata_encryption_key, + loc['url']) else: uri = loc['url'] @@ -327,7 +249,7 @@ class ScrubDBQueue(object): :returns: a boolean value to inform including or not """ try: - image = self.registry.get_image(image_id) + image = db_api.get_api().image_get(self.admin_context, image_id) return image['status'] == 'pending_delete' except exception.NotFound: return False @@ -372,36 +294,9 @@ class Daemon(object): class Scrubber(object): def __init__(self, store_api): - LOG.info(_LI("Initializing scrubber with configuration: %s"), - six.text_type({'registry_host': CONF.registry_host, - 'registry_port': CONF.registry_port})) - + LOG.info(_LI("Initializing scrubber")) self.store_api = store_api - - registry.configure_registry_client() - registry.configure_registry_admin_creds() - - # Here we create a request context with credentials to support - # delayed delete when using multi-tenant backend storage - admin_user = CONF.admin_user - admin_tenant = CONF.admin_tenant_name - - if CONF.send_identity_headers: - # When registry is operating in trusted-auth mode - roles = [CONF.admin_role] - self.admin_context = context.RequestContext(user=admin_user, - tenant=admin_tenant, - auth_token=None, - roles=roles) - self.registry = registry.get_registry_client(self.admin_context) - else: - ctxt = context.RequestContext() - self.registry = registry.get_registry_client(ctxt) - auth_token = self.registry.auth_token - self.admin_context = context.RequestContext(user=admin_user, - tenant=admin_tenant, - auth_token=auth_token) - + self.admin_context = context.get_admin_context(show_deleted=True) self.db_queue = get_scrub_queue() self.pool = eventlet.greenpool.GreenPool(CONF.scrub_pool_size) @@ -444,9 +339,10 @@ class Scrubber(object): success = False if success: - image = self.registry.get_image(image_id) + image = db_api.get_api().image_get(self.admin_context, image_id) if image['status'] == 'pending_delete': - self.registry.update_image(image_id, {'status': 'deleted'}) + db_api.get_api().image_update(self.admin_context, image_id, + {'status': 'deleted'}) LOG.info(_LI("Image %s has been scrubbed successfully"), image_id) else: LOG.warn(_LW("One or more image locations couldn't be scrubbed " @@ -454,8 +350,6 @@ class Scrubber(object): " status") % image_id) def _delete_image_location_from_backend(self, image_id, loc_id, uri): - if CONF.metadata_encryption_key: - uri = crypt.urlsafe_decrypt(CONF.metadata_encryption_key, uri) try: LOG.debug("Scrubbing image %s from a location.", image_id) try: diff --git a/glance/tests/unit/test_scrubber.py b/glance/tests/unit/test_scrubber.py index 83cd184d16..50d25ed6a9 100644 --- a/glance/tests/unit/test_scrubber.py +++ b/glance/tests/unit/test_scrubber.py @@ -24,6 +24,7 @@ from oslo_config import cfg from six.moves import range from glance.common import exception +from glance.db.sqlalchemy import api as db_api from glance import scrubber from glance.tests import utils as test_utils @@ -51,9 +52,6 @@ class TestScrubber(test_utils.BaseTestCase): uri = 'file://some/path/%s' % uuid.uuid4() id = 'helloworldid' scrub = scrubber.Scrubber(glance_store) - scrub.registry = self.mox.CreateMockAnything() - scrub.registry.get_image(id).AndReturn({'status': 'pending_delete'}) - scrub.registry.update_image(id, {'status': 'deleted'}) self.mox.StubOutWithMock(glance_store, "delete_from_backend") glance_store.delete_from_backend( uri, @@ -62,21 +60,20 @@ class TestScrubber(test_utils.BaseTestCase): scrub._scrub_image(id, [(id, '-', uri)]) self.mox.VerifyAll() - def test_store_delete_successful(self): + @mock.patch.object(db_api, "image_get") + def test_store_delete_successful(self, mock_image_get): uri = 'file://some/path/%s' % uuid.uuid4() id = 'helloworldid' scrub = scrubber.Scrubber(glance_store) - scrub.registry = self.mox.CreateMockAnything() - scrub.registry.get_image(id).AndReturn({'status': 'pending_delete'}) - scrub.registry.update_image(id, {'status': 'deleted'}) self.mox.StubOutWithMock(glance_store, "delete_from_backend") glance_store.delete_from_backend(uri, mox.IgnoreArg()).AndReturn('') self.mox.ReplayAll() scrub._scrub_image(id, [(id, '-', uri)]) self.mox.VerifyAll() - def test_store_delete_store_exceptions(self): + @mock.patch.object(db_api, "image_get") + def test_store_delete_store_exceptions(self, mock_image_get): # While scrubbing image data, all store exceptions, other than # NotFound, cause image scrubbing to fail. Essentially, no attempt # would be made to update the status of image. @@ -86,7 +83,6 @@ class TestScrubber(test_utils.BaseTestCase): ex = glance_store.GlanceStoreException() scrub = scrubber.Scrubber(glance_store) - scrub.registry = self.mox.CreateMockAnything() self.mox.StubOutWithMock(glance_store, "delete_from_backend") glance_store.delete_from_backend( uri, @@ -95,7 +91,8 @@ class TestScrubber(test_utils.BaseTestCase): scrub._scrub_image(id, [(id, '-', uri)]) self.mox.VerifyAll() - def test_store_delete_notfound_exception(self): + @mock.patch.object(db_api, "image_get") + def test_store_delete_notfound_exception(self, mock_image_get): # While scrubbing image data, NotFound exception is ignored and image # scrubbing succeeds uri = 'file://some/path/%s' % uuid.uuid4() @@ -103,9 +100,6 @@ class TestScrubber(test_utils.BaseTestCase): ex = glance_store.NotFound(message='random') scrub = scrubber.Scrubber(glance_store) - scrub.registry = self.mox.CreateMockAnything() - scrub.registry.get_image(id).AndReturn({'status': 'pending_delete'}) - scrub.registry.update_image(id, {'status': 'deleted'}) self.mox.StubOutWithMock(glance_store, "delete_from_backend") glance_store.delete_from_backend(uri, mox.IgnoreArg()).AndRaise(ex) self.mox.ReplayAll() @@ -141,11 +135,12 @@ class TestScrubDBQueue(test_utils.BaseTestCase): image_pager = ImagePager(images) def make_get_images_detailed(pager): - def mock_get_images_detailed(filters, marker=None): + def mock_get_images_detailed(ctx, filters, marker=None, + limit=None): return pager() return mock_get_images_detailed - with patch.object(scrub_queue.registry, 'get_images_detailed') as ( + with patch.object(db_api, 'image_get_all') as ( _mock_get_images_detailed): _mock_get_images_detailed.side_effect = ( make_get_images_detailed(image_pager)) @@ -159,11 +154,12 @@ class TestScrubDBQueue(test_utils.BaseTestCase): image_pager = ImagePager(images, page_size=4) def make_get_images_detailed(pager): - def mock_get_images_detailed(filters, marker=None): + def mock_get_images_detailed(ctx, filters, marker=None, + limit=None): return pager() return mock_get_images_detailed - with patch.object(scrub_queue.registry, 'get_images_detailed') as ( + with patch.object(db_api, 'image_get_all') as ( _mock_get_images_detailed): _mock_get_images_detailed.side_effect = ( make_get_images_detailed(image_pager)) diff --git a/releasenotes/notes/scrubber-refactor-73ddbd61ebbf1e86.yaml b/releasenotes/notes/scrubber-refactor-73ddbd61ebbf1e86.yaml new file mode 100644 index 0000000000..c605353e87 --- /dev/null +++ b/releasenotes/notes/scrubber-refactor-73ddbd61ebbf1e86.yaml @@ -0,0 +1,8 @@ +others: + - | + The ``glance-scrubber`` utility, which is used to perfom offline deletion + of images when the Glance ``delayed_delete`` option is enabled, has been + refactored so that it no longer uses the Glance Registry API (and hence no + longer has a dependency on the Registry v1 Client). Configuration options + associated with connecting to the Glance registry are no longer required, + and operators may remove them from the glance-scrubber.conf file.