From 71a36c98ed036b00a1b5fbe2d3d622c645611a7c Mon Sep 17 00:00:00 2001 From: wangxiyuan Date: Mon, 5 Mar 2018 15:07:54 +0800 Subject: [PATCH] Pending-delete rollback ability Now there is no way to revert the pending-delete images. Once the admin operators want to revert the delete action, Glance should give them this kind of ability. This patch will enhance the glance-scrubber tool to support restoring the image from pending-delete to active. Change-Id: I11fe58403e3af102b63d15b3cc702e567e526bad blueprint: pending-delete-rollback --- doc/source/cli/glancescrubber.rst | 19 ++- glance/cmd/scrubber.py | 44 +++++- glance/db/simple/api.py | 11 ++ glance/db/sqlalchemy/api.py | 15 +++ glance/domain/__init__.py | 2 +- glance/scrubber.py | 18 +++ glance/tests/functional/test_scrubber.py | 127 ++++++++++++++++++ glance/tests/unit/test_db.py | 26 ++++ glance/tests/unit/test_scrubber.py | 15 +++ ...ding-delete-rollback-444ff94c0056bbdb.yaml | 8 ++ 10 files changed, 280 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/pending-delete-rollback-444ff94c0056bbdb.yaml diff --git a/doc/source/cli/glancescrubber.rst b/doc/source/cli/glancescrubber.rst index 0c83d08bac..b619af909b 100644 --- a/doc/source/cli/glancescrubber.rst +++ b/doc/source/cli/glancescrubber.rst @@ -19,9 +19,10 @@ DESCRIPTION =========== glance-scrubber is a utility that allows an operator to configure Glance for -the asynchronous deletion of images. Whether this makes sense for your -deployment depends upon the storage backend you are using and the size of -typical images handled by your Glance installation. +the asynchronous deletion of images or to revert the image's status from +`pending_delete` to `active`. Whether this makes sense for your deployment +depends upon the storage backend you are using and the size of typical images +handled by your Glance installation. An image in glance is really a combination of an image record (stored in the database) and a file of image data (stored in a storage backend). Under normal @@ -49,6 +50,11 @@ with ``delayed_delete`` enabled, you *must* run the glance-scrubber occasionally or your storage backend will eventually fill up with "deleted" image data. +The glance-scrubber can also revert a image to `active` if operators delete +the image by mistake and the pending-delete is enabled in Glance. Please make +sure the ``glance-scrubber`` is not running before restoring the image to avoid +image data inconsistency. + Configuration of glance-scrubber is done in the **glance-scrubber.conf** file. Options are explained in detail in comments in the sample configuration file, so we only point out a few of them here. @@ -75,6 +81,10 @@ so we only point out a few of them here. **glance-scrubber.conf** or the scrubber won't be able to determine the locations of your image data. +``restore`` + reset the specified image's status from'pending_delete' to 'active' when + the image is deleted by mistake. + ``[database]`` As of the Queens release of Glance (16.0.0), the glance-scrubber does not use the deprecated Glance registry, but instead contacts the Glance @@ -111,6 +121,9 @@ OPTIONS The inverse of --daemon. Runs the scrub operation once and then exits. This is the default. + **--restore ** + Restore the specified image status from 'pending_delete' to 'active'. + FILES ===== diff --git a/glance/cmd/scrubber.py b/glance/cmd/scrubber.py index 4abfce537e..3a08d558f9 100644 --- a/glance/cmd/scrubber.py +++ b/glance/cmd/scrubber.py @@ -28,6 +28,7 @@ eventlet.hubs.get_hub() eventlet.patcher.monkey_patch() import os +import subprocess import sys # If ../glance/__init__.py exists, add ../ to Python search path, so that @@ -43,6 +44,7 @@ from oslo_config import cfg from oslo_log import log as logging from glance.common import config +from glance.common import exception from glance import scrubber @@ -65,12 +67,52 @@ def main(): app = scrubber.Scrubber(glance_store) - if CONF.daemon: + if CONF.restore and CONF.daemon: + sys.exit("ERROR: The restore and daemon options should not be set " + "together. Please use either of them in one request.") + if CONF.restore: + # Try to check the glance-scrubber is running or not. + # 1. Try to find the pid file if scrubber is controlled by + # glance-control + # 2. Try to check the process name. + error_str = ("ERROR: The glance-scrubber process is running under " + "daemon. Please stop it first.") + pid_file = '/var/run/glance/glance-scrubber.pid' + if os.path.exists(os.path.abspath(pid_file)): + sys.exit(error_str) + + for glance_scrubber_name in ['glance-scrubber', + 'glance.cmd.scrubber']: + cmd = subprocess.Popen( + ['/usr/bin/pgrep', '-f', glance_scrubber_name], + stdout=subprocess.PIPE, shell=False) + pids, _ = cmd.communicate() + + # The response format of subprocess.Popen.communicate() is + # diffderent between py2 and py3. It's "string" in py2, but + # "bytes" in py3. + if isinstance(pids, bytes): + pids = pids.decode() + self_pid = os.getpid() + + if pids.count('\n') > 1 and str(self_pid) in pids: + # One process is self, so if the process number is > 1, it + # means that another glance-scrubber process is running. + sys.exit(error_str) + elif pids.count('\n') > 0 and str(self_pid) not in pids: + # If self is not in result and the pids number is still + # > 0, it means that the another glance-scrubber process is + # running. + sys.exit(error_str) + app.revert_image_status(CONF.restore) + elif CONF.daemon: server = scrubber.Daemon(CONF.wakeup_time) server.start(app) server.wait() else: app.run() + except (exception.ImageNotFound, exception.Conflict) as e: + sys.exit("ERROR: %s" % e) except RuntimeError as e: sys.exit("ERROR: %s" % e) diff --git a/glance/db/simple/api.py b/glance/db/simple/api.py index daab21ab11..1456878504 100644 --- a/glance/db/simple/api.py +++ b/glance/db/simple/api.py @@ -474,6 +474,17 @@ def image_get_all(context, filters=None, marker=None, limit=None, return res +def image_restore(context, image_id): + """Restore the pending-delete image to active.""" + image = _image_get(context, image_id) + if image['status'] != 'pending_delete': + msg = (_('cannot restore the image from %s to active (wanted ' + 'from_state=pending_delete)') % image['status']) + raise exception.Conflict(msg) + values = {'status': 'active', 'deleted': 0} + image_update(context, image_id, values) + + @log_call def image_property_create(context, values): image = _image_get(context, values['image_id']) diff --git a/glance/db/sqlalchemy/api.py b/glance/db/sqlalchemy/api.py index 57f46c00f8..44656e62da 100644 --- a/glance/db/sqlalchemy/api.py +++ b/glance/db/sqlalchemy/api.py @@ -164,6 +164,21 @@ def image_update(context, image_id, values, purge_props=False, return image +def image_restore(context, image_id): + """Restore the pending-delete image to active.""" + session = get_session() + with session.begin(): + image_ref = _image_get(context, image_id, session=session) + if image_ref.status != 'pending_delete': + msg = (_('cannot restore the image from %s to active (wanted ' + 'from_state=pending_delete)') % image_ref.status) + raise exception.Conflict(msg) + + query = session.query(models.Image).filter_by(id=image_id) + values = {'status': 'active', 'deleted': 0} + query.update(values, synchronize_session='fetch') + + @retry(retry_on_exception=_retry_on_deadlock, wait_fixed=500, stop_max_attempt_number=50) def image_destroy(context, image_id): diff --git a/glance/domain/__init__.py b/glance/domain/__init__.py index eaf7ac5a40..acf46ea637 100644 --- a/glance/domain/__init__.py +++ b/glance/domain/__init__.py @@ -107,7 +107,7 @@ class Image(object): 'importing': ('active', 'deleted', 'queued'), 'active': ('pending_delete', 'deleted', 'deactivated'), 'killed': ('deleted',), - 'pending_delete': ('deleted',), + 'pending_delete': ('deleted', 'active'), 'deleted': (), 'deactivated': ('active', 'deleted'), } diff --git a/glance/scrubber.py b/glance/scrubber.py index dfc3bef499..28cb912956 100644 --- a/glance/scrubber.py +++ b/glance/scrubber.py @@ -150,6 +150,21 @@ Possible values: Related options: * ``wakeup_time`` +""")), + cfg.StrOpt('restore', + metavar='', + help=_(""" +Restore the image status from 'pending_delete' to 'active'. + +This option is used by administrator to reset the image's status from +'pending_delete' to 'active' when the image is deleted by mistake and +'pending delete' feature is enabled in Glance. Please make sure the +glance-scrubber daemon is stopped before restoring the image to avoid image +data inconsistency. + +Possible values: + * image's uuid + """)) ] @@ -371,3 +386,6 @@ class Scrubber(object): {'id': image_id, 'exc': encodeutils.exception_to_unicode(e)}) raise + + def revert_image_status(self, image_id): + db_api.get_api().image_restore(self.admin_context, image_id) diff --git a/glance/tests/functional/test_scrubber.py b/glance/tests/functional/test_scrubber.py index a7dfb9f83f..91597051a8 100644 --- a/glance/tests/functional/test_scrubber.py +++ b/glance/tests/functional/test_scrubber.py @@ -233,6 +233,133 @@ class TestScrubber(functional.FunctionalTest): self.stop_server(self.scrubber_daemon) + def test_scrubber_restore_image(self): + self.cleanup() + self.start_servers(delayed_delete=True, daemon=False, + metadata_encryption_key='') + path = "http://%s:%d/v2/images" % ("127.0.0.1", self.api_port) + response, content = self._send_create_image_http_request(path) + self.assertEqual(http_client.CREATED, response.status) + image = jsonutils.loads(content) + self.assertEqual('queued', image['status']) + + file_path = "%s/%s/file" % (path, image['id']) + response, content = self._send_upload_image_http_request(file_path, + body='XXX') + self.assertEqual(http_client.NO_CONTENT, response.status) + + path = "%s/%s" % (path, image['id']) + response, content = self._send_http_request(path, 'GET') + image = jsonutils.loads(content) + self.assertEqual('active', image['status']) + + response, content = self._send_http_request(path, 'DELETE') + self.assertEqual(http_client.NO_CONTENT, response.status) + + image = self._get_pending_delete_image(image['id']) + self.assertEqual('pending_delete', image['status']) + + def _test_content(): + exe_cmd = "%s -m glance.cmd.scrubber" % sys.executable + cmd = ("%s --config-file %s --restore %s" % + (exe_cmd, self.scrubber_daemon.conf_file_name, image['id'])) + exitcode, out, err = execute(cmd, raise_error=False) + self.assertEqual(0, exitcode) + self.wait_for_scrubber_shutdown(_test_content) + + response, content = self._send_http_request(path, 'GET') + image = jsonutils.loads(content) + self.assertEqual('active', image['status']) + + self.stop_servers() + + def test_scrubber_restore_active_image_raise_error(self): + self.cleanup() + self.start_servers(delayed_delete=True, daemon=False, + metadata_encryption_key='') + + path = "http://%s:%d/v2/images" % ("127.0.0.1", self.api_port) + response, content = self._send_create_image_http_request(path) + self.assertEqual(http_client.CREATED, response.status) + image = jsonutils.loads(content) + self.assertEqual('queued', image['status']) + + file_path = "%s/%s/file" % (path, image['id']) + response, content = self._send_upload_image_http_request(file_path, + body='XXX') + self.assertEqual(http_client.NO_CONTENT, response.status) + + path = "%s/%s" % (path, image['id']) + response, content = self._send_http_request(path, 'GET') + image = jsonutils.loads(content) + self.assertEqual('active', image['status']) + + def _test_content(): + exe_cmd = "%s -m glance.cmd.scrubber" % sys.executable + cmd = ("%s --config-file %s --restore %s" % + (exe_cmd, self.scrubber_daemon.conf_file_name, image['id'])) + exitcode, out, err = execute(cmd, raise_error=False) + self.assertEqual(1, exitcode) + self.assertIn('cannot restore the image from active to active ' + '(wanted from_state=pending_delete)', str(err)) + self.wait_for_scrubber_shutdown(_test_content) + + self.stop_servers() + + def test_scrubber_restore_image_non_exist(self): + + def _test_content(): + scrubber = functional.ScrubberDaemon(self.test_dir, + self.policy_file) + scrubber.write_conf(daemon=False) + scrubber.needs_database = True + scrubber.create_database() + exe_cmd = "%s -m glance.cmd.scrubber" % sys.executable + cmd = ("%s --config-file %s --restore fake_image_id" % + (exe_cmd, scrubber.conf_file_name)) + exitcode, out, err = execute(cmd, raise_error=False) + self.assertEqual(1, exitcode) + self.assertIn('No image found with ID fake_image_id', str(err)) + + self.wait_for_scrubber_shutdown(_test_content) + + def test_scrubber_restore_image_with_daemon_raise_error(self): + + exe_cmd = "%s -m glance.cmd.scrubber" % sys.executable + cmd = ("%s --daemon --restore fake_image_id" % exe_cmd) + exitcode, out, err = execute(cmd, raise_error=False) + + self.assertEqual(1, exitcode) + self.assertIn('The restore and daemon options should not be set ' + 'together', str(err)) + + def test_scrubber_restore_image_with_daemon_running(self): + self.cleanup() + self.scrubber_daemon.start(daemon=True) + + exe_cmd = "%s -m glance.cmd.scrubber" % sys.executable + cmd = ("%s --restore fake_image_id" % exe_cmd) + exitcode, out, err = execute(cmd, raise_error=False) + self.assertEqual(1, exitcode) + self.assertIn('The glance-scrubber process is running under daemon', + str(err)) + + self.stop_server(self.scrubber_daemon) + + def wait_for_scrubber_shutdown(self, func): + # NOTE: sometimes the glance-scrubber process which is setup by the + # previous test can't be shutdown immediately, we need wait a second to + # make sure it's down. + for _ in range(15): + try: + func() + break + except AssertionError: + time.sleep(1) + continue + else: + self.fail('unexpected error occurred in glance-scrubber') + def wait_for_scrub(self, image_id): """ NOTE(jkoelker) The build servers sometimes take longer than 15 seconds diff --git a/glance/tests/unit/test_db.py b/glance/tests/unit/test_db.py index 55ff0b1388..2f32509ea1 100644 --- a/glance/tests/unit/test_db.py +++ b/glance/tests/unit/test_db.py @@ -402,6 +402,32 @@ class TestImageRepo(test_utils.BaseTestCase): exception.ImageNotFound, self.image_repo.remove, image) self.assertIn(fake_uuid, encodeutils.exception_to_unicode(exc)) + def test_restore_image_status(self): + image_id = uuid.uuid4() + image = _db_fixture(image_id, name='restore_test', size=256, + is_public=True, status='pending_delete') + self.db.image_create(self.context, image) + self.db.image_restore(self.context, image_id) + image = self.db.image_get(self.context, image_id) + self.assertEqual(image['status'], 'active') + + def test_restore_image_status_not_found(self): + image_id = uuid.uuid4() + self.assertRaises(exception.ImageNotFound, + self.db.image_restore, + self.context, + image_id) + + def test_restore_image_status_not_pending_delete(self): + image_id = uuid.uuid4() + image = _db_fixture(image_id, name='restore_test', size=256, + is_public=True, status='deleted') + self.db.image_create(self.context, image) + self.assertRaises(exception.Conflict, + self.db.image_restore, + self.context, + image_id) + class TestEncryptedLocations(test_utils.BaseTestCase): def setUp(self): diff --git a/glance/tests/unit/test_scrubber.py b/glance/tests/unit/test_scrubber.py index f009981787..cea4dbbe38 100644 --- a/glance/tests/unit/test_scrubber.py +++ b/glance/tests/unit/test_scrubber.py @@ -104,6 +104,21 @@ class TestScrubber(test_utils.BaseTestCase): self.assertRaises(exception.FailedToGetScrubberJobs, scrub._get_delete_jobs) + @mock.patch.object(db_api, "image_restore") + def test_scrubber_revert_image_status(self, mock_image_restore): + scrub = scrubber.Scrubber(glance_store) + scrub.revert_image_status('fake_id') + + mock_image_restore.side_effect = exception.ImageNotFound + self.assertRaises(exception.ImageNotFound, + scrub.revert_image_status, + 'fake_id') + + mock_image_restore.side_effect = exception.Conflict + self.assertRaises(exception.Conflict, + scrub.revert_image_status, + 'fake_id') + class TestScrubDBQueue(test_utils.BaseTestCase): diff --git a/releasenotes/notes/pending-delete-rollback-444ff94c0056bbdb.yaml b/releasenotes/notes/pending-delete-rollback-444ff94c0056bbdb.yaml new file mode 100644 index 0000000000..e60bf1a396 --- /dev/null +++ b/releasenotes/notes/pending-delete-rollback-444ff94c0056bbdb.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + ``glance-scrubber`` now support to restore the image's status from + `pending_delete` to `active`. The usage is + `glance-scrubber --restore `. Please make sure the + ``glance-scrubber`` daemon is stopped before restoring the image to avoid + image data inconsistency.