diff --git a/glance/common/exception.py b/glance/common/exception.py index 4a0fda8059..1ea6df8efa 100644 --- a/glance/common/exception.py +++ b/glance/common/exception.py @@ -292,6 +292,11 @@ class ImageSizeLimitExceeded(GlanceException): message = _("The provided image is too large.") +class FailedToGetScrubberJobs(GlanceException): + message = _("Scrubber encountered an error while trying to fetch " + "scrub jobs.") + + class ImageMemberLimitExceeded(LimitExceeded): message = _("The limit has been exceeded on the number of allowed image " "members for this image. Attempted: %(attempted)s, " diff --git a/glance/scrubber.py b/glance/scrubber.py index eb604521b1..1894c7f2d0 100644 --- a/glance/scrubber.py +++ b/glance/scrubber.py @@ -27,7 +27,7 @@ from glance.common import crypt from glance.common import exception from glance import context import glance.db as db_api -from glance.i18n import _, _LE, _LI, _LW +from glance.i18n import _, _LC, _LE, _LI, _LW import glance.registry.client.v1.api as registry LOG = logging.getLogger(__name__) @@ -409,9 +409,12 @@ class Scrubber(object): try: records = self.db_queue.get_all_locations() except Exception as err: - LOG.error(_LE("Can not get scrub jobs from queue: %s") % - encodeutils.exception_to_unicode(err)) - return {} + # Note(dharinic): spawn_n, in Daemon mode will log the + # exception raised. Otherwise, exit 1 will occur. + msg = (_LC("Can not get scrub jobs from queue: %s") % + encodeutils.exception_to_unicode(err)) + LOG.critical(msg) + raise exception.FailedToGetScrubberJobs() delete_jobs = {} for image_id, loc_id, loc_uri in records: diff --git a/glance/tests/functional/__init__.py b/glance/tests/functional/__init__.py index cd6c8f8ec4..fa56be54c1 100644 --- a/glance/tests/functional/__init__.py +++ b/glance/tests/functional/__init__.py @@ -603,6 +603,10 @@ class FunctionalTest(test_utils.BaseTestCase): self.api_protocol = 'http' self.api_port, api_sock = test_utils.get_unused_port_and_socket() self.registry_port, reg_sock = test_utils.get_unused_port_and_socket() + # NOTE: Scrubber is enabled by default for the functional tests. + # Please disbale it by explicitly setting 'self.include_scrubber' to + # False in the test SetUps that do not require Scrubber to run. + self.include_scrubber = True self.tracecmd = tracecmd_osmap.get(platform.system()) @@ -799,11 +803,11 @@ class FunctionalTest(test_utils.BaseTestCase): self.start_with_retry(self.api_server, 'api_port', 3, **kwargs) - exitcode, out, err = self.scrubber_daemon.start(**kwargs) - - self.assertEqual(0, exitcode, - "Failed to spin up the Scrubber daemon. " - "Got: %s" % err) + if self.include_scrubber: + exitcode, out, err = self.scrubber_daemon.start(**kwargs) + self.assertEqual(0, exitcode, + "Failed to spin up the Scrubber daemon. " + "Got: %s" % err) def ping_server(self, port): """ @@ -919,7 +923,8 @@ class FunctionalTest(test_utils.BaseTestCase): # Spin down the API and default registry server self.stop_server(self.api_server, 'API server') self.stop_server(self.registry_server, 'Registry server') - self.stop_server(self.scrubber_daemon, 'Scrubber daemon') + if self.include_scrubber: + self.stop_server(self.scrubber_daemon, 'Scrubber daemon') self._reset_database(self.registry_server.sql_connection) diff --git a/glance/tests/functional/test_scrubber.py b/glance/tests/functional/test_scrubber.py index 90d6ef5073..f6d8f00ce4 100644 --- a/glance/tests/functional/test_scrubber.py +++ b/glance/tests/functional/test_scrubber.py @@ -274,6 +274,33 @@ class TestScrubber(functional.FunctionalTest): self.stop_servers() + def test_scrubber_app_queue_errors_not_daemon(self): + """ + test that the glance-scrubber exits with an exit code > 0 when it + fails to lookup images, indicating a configuration error when not + in daemon mode. + + Related-Bug: #1548289 + """ + # Don't start the registry server to cause intended failure + # Don't start the api server to save time + exitcode, out, err = self.scrubber_daemon.start( + delayed_delete=True, daemon=False, registry_port=28890) + self.assertEqual(0, exitcode, + "Failed to spin up the Scrubber daemon. " + "Got: %s" % err) + + # Run the Scrubber + exe_cmd = "%s -m glance.cmd.scrubber" % sys.executable + cmd = ("%s --config-file %s" % + (exe_cmd, self.scrubber_daemon.conf_file_name)) + exitcode, out, err = execute(cmd, raise_error=False) + + self.assertEqual(1, exitcode) + self.assertIn('Can not get scrub jobs from queue', err) + + self.stop_server(self.scrubber_daemon, 'Scrubber daemon') + def wait_for_scrub(self, path, headers=None): """ NOTE(jkoelker) The build servers sometimes take longer than 15 seconds diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py index 3420b7f5d8..b6b737c791 100644 --- a/glance/tests/functional/v2/test_images.py +++ b/glance/tests/functional/v2/test_images.py @@ -40,6 +40,7 @@ class TestImages(functional.FunctionalTest): def setUp(self): super(TestImages, self).setUp() self.cleanup() + self.include_scrubber = False self.api_server.deployment_flavor = 'noauth' self.api_server.data_api = 'glance.db.sqlalchemy.api' for i in range(3): @@ -3028,6 +3029,7 @@ class TestImagesIPv6(functional.FunctionalTest): # Setting up monkey patch (2), after object is ready... self.ping_server_ipv4 = self.ping_server self.ping_server = self.ping_server_ipv6 + self.include_scrubber = False def tearDown(self): # Cleaning up monkey patch (2). @@ -3083,6 +3085,7 @@ class TestImageDirectURLVisibility(functional.FunctionalTest): def setUp(self): super(TestImageDirectURLVisibility, self).setUp() self.cleanup() + self.include_scrubber = False self.api_server.deployment_flavor = 'noauth' def _url(self, path): @@ -3292,6 +3295,7 @@ class TestImageLocationSelectionStrategy(functional.FunctionalTest): def setUp(self): super(TestImageLocationSelectionStrategy, self).setUp() self.cleanup() + self.include_scrubber = False self.api_server.deployment_flavor = 'noauth' for i in range(3): ret = test_utils.start_http_server("foo_image_id%d" % i, @@ -3395,6 +3399,7 @@ class TestImageMembers(functional.FunctionalTest): def setUp(self): super(TestImageMembers, self).setUp() self.cleanup() + self.include_scrubber = False self.api_server.deployment_flavor = 'fakeauth' self.registry_server.deployment_flavor = 'fakeauth' self.start_servers(**self.__dict__.copy()) @@ -3761,6 +3766,7 @@ class TestQuotas(functional.FunctionalTest): def setUp(self): super(TestQuotas, self).setUp() self.cleanup() + self.include_scrubber = False self.api_server.deployment_flavor = 'noauth' self.registry_server.deployment_flavor = 'trusted-auth' self.user_storage_quota = 100 diff --git a/glance/tests/functional/v2/test_tasks.py b/glance/tests/functional/v2/test_tasks.py index 674dd53e58..b04340b67d 100644 --- a/glance/tests/functional/v2/test_tasks.py +++ b/glance/tests/functional/v2/test_tasks.py @@ -143,3 +143,4 @@ class TestTasksWithRegistry(TestTasks): self.api_server.data_api = ( 'glance.tests.functional.v2.registry_data_api') self.registry_server.deployment_flavor = 'trusted-auth' + self.include_scrubber = False diff --git a/glance/tests/unit/test_scrubber.py b/glance/tests/unit/test_scrubber.py index cf8cb0c7d9..7f64a4c359 100644 --- a/glance/tests/unit/test_scrubber.py +++ b/glance/tests/unit/test_scrubber.py @@ -16,12 +16,14 @@ import uuid import glance_store +import mock from mock import patch from mox3 import mox from oslo_config import cfg # NOTE(jokke): simplified transition to py3, behaves like py2 xrange from six.moves import range +from glance.common import exception from glance import scrubber from glance.tests import utils as test_utils @@ -110,6 +112,16 @@ class TestScrubber(test_utils.BaseTestCase): scrub._scrub_image(id, [(id, '-', uri)]) self.mox.VerifyAll() + def test_scrubber_exits(self): + # Checks for Scrubber exits when it is not able to fetch jobs from + # the queue + scrub_jobs = scrubber.ScrubDBQueue.get_all_locations + scrub_jobs = mock.MagicMock() + scrub_jobs.side_effect = exception.NotFound + scrub = scrubber.Scrubber(glance_store) + self.assertRaises(exception.FailedToGetScrubberJobs, + scrub._get_delete_jobs) + class TestScrubDBQueue(test_utils.BaseTestCase): diff --git a/releasenotes/notes/scrubber-exit-e5d77f6f1a38ffb7.yaml b/releasenotes/notes/scrubber-exit-e5d77f6f1a38ffb7.yaml new file mode 100644 index 0000000000..8e91f49729 --- /dev/null +++ b/releasenotes/notes/scrubber-exit-e5d77f6f1a38ffb7.yaml @@ -0,0 +1,12 @@ +--- +fixes: + - | + Please note a change in the Scrubber's behavior in case + of job fetching errors: + + * If configured to work in daemon mode, the Scrubber + will log an error message at level critical, but + will not exit the process. + * If configured to work in non-daemon mode, the Scrubber + will log an error message at level critical and exit + with status one.