Handling scrubber's exit in non-daemon mode.

Currently, Scrubber exits with a status code of zero
upon job fetching errors both in daemon and non daemon
mode. It is more appropriate to have scrubber exit with
status code 1 if it is not in daemon mode.

This patch causes scrubber to exit with status code 1
(RuntimeError)if the scrubber is not running in daemon
mode, facilitating accurate exits.

Co-Authored-By: Steve Lewis <steve.lewis@rackspace.com>
Co-Authored-By: Dharini Chandrasekar <dharini.chandrasekar@intel.com>

Closes-Bug: #1548289

Change-Id: Ib75676dc90349f008395fb118978f4e37fa876ea

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, "

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") %
+            # Note(dharinic): spawn_n, in Daemon mode will log the
-                      encodeutils.exception_to_unicode(err))
+            # exception raised. Otherwise, exit 1 will occur.
-            return {}
+            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:

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)
+        if self.include_scrubber:
-
+            exitcode, out, err = self.scrubber_daemon.start(**kwargs)
-        self.assertEqual(0, exitcode,
+            self.assertEqual(0, exitcode,
-                         "Failed to spin up the Scrubber daemon. "
+                             "Failed to spin up the Scrubber daemon. "
-                         "Got: %s" % err)
+                             "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)
 

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

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

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

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):
 

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.