Don't send heartbeats if Manager reports a problem

This commit adds is_working method to base Manager class that can be
used to indicate that service has some problems and isn't running
correctly. This is used to block refreshing Service heartbeats if
Manager will return False from is_working.

The method is implemented in c-vol Manager to return initialization
state of the underlying volume driver.

Closes-Bug: 1446750
DocImpact

Change-Id: I809b2b777a0f84d12f39f2233920ec961f74f846

cinder/manager.py

@@ -112,6 +112,14 @@ class Manager(base.Base, periodic_task.PeriodicTasks):
             config[key] = CONF.get(key, None)
         return config
 
+    def is_working(self):
+        """Method indicating if service is working correctly.
+
+        This method is supposed to be overriden by subclasses and return if
+        manager is working correctly.
+        """
+        return True
+
 
 class SchedulerDependentManager(Manager):
     """Periodically send capability updates to the Scheduler services.

cinder/service.py

@@ -287,6 +287,14 @@ class Service(service.Service):
 
     def report_state(self):
         """Update the state of this service in the datastore."""
+        if not self.manager.is_working():
+            # NOTE(dulek): If manager reports a problem we're not sending
+            # heartbeats - to indicate that service is actually down.
+            LOG.error(_LE('Manager for service %s is reporting problems, skip '
+                          'sending heartbeat. Service will appear "down".'),
+                      self.binary)
+            return
+
         ctxt = context.get_admin_context()
         zone = CONF.storage_availability_zone
         state_catalog = {}

cinder/tests/unit/test_service.py

@@ -149,6 +149,7 @@ class ServiceTestCase(test.TestCase):
             serv.start()
             serv.report_state()
             self.assertTrue(serv.model_disconnected)
+            self.assertFalse(mock_db.service_update.called)
 
     def test_report_state_newly_connected(self):
         host = 'foo'
@@ -176,6 +177,33 @@ class ServiceTestCase(test.TestCase):
             serv.report_state()
 
             self.assertFalse(serv.model_disconnected)
+            self.assertTrue(mock_db.service_update.called)
+
+    def test_report_state_manager_not_working(self):
+        host = 'foo'
+        binary = 'bar'
+        topic = 'test'
+        service_ref = {'host': host,
+                       'binary': binary,
+                       'topic': topic,
+                       'report_count': 0,
+                       'availability_zone': 'nova',
+                       'id': 1}
+        with mock.patch.object(service, 'db') as mock_db:
+            mock_db.service_get.return_value = service_ref
+
+            serv = service.Service(
+                host,
+                binary,
+                topic,
+                'cinder.tests.unit.test_service.FakeManager'
+            )
+            serv.manager.is_working = mock.Mock(return_value=False)
+            serv.start()
+            serv.report_state()
+
+            serv.manager.is_working.assert_called_once_with()
+            self.assertFalse(mock_db.service_update.called)
 
     def test_service_with_long_report_interval(self):
         self.override_config('service_down_time', 10)

cinder/tests/unit/test_volume.py

@@ -330,6 +330,14 @@ class VolumeTestCase(BaseVolumeTestCase):
             manager.init_host()
         self.assertEqual(0, mock_add_p_task.call_count)
 
+    def test_is_working(self):
+        # By default we have driver mocked to be initialized...
+        self.assertTrue(self.volume.is_working())
+
+        # ...lets switch it and check again!
+        self.volume.driver._initialized = False
+        self.assertFalse(self.volume.is_working())
+
     def test_create_volume_fails_with_creating_and_downloading_status(self):
         """Test init_host in case of volume.
 

cinder/volume/manager.py

@@ -394,6 +394,15 @@ class VolumeManager(manager.SchedulerDependentManager):
                  resource={'type': 'driver',
                            'id': self.driver.__class__.__name__})
 
+    def is_working(self):
+        """Return if Manager is ready to accept requests.
+
+        This is to inform Service class that in case of volume driver
+        initialization failure the manager is actually down and not ready to
+        accept any requests.
+        """
+        return self.driver.initialized
+
     def create_volume(self, context, volume_id, request_spec=None,
                       filter_properties=None, allow_reschedule=True,
                       snapshot_id=None, image_id=None, source_volid=None,