Handling corner cases in dynamic looping call

If the periodic task for dynamic looping call returns no suggestion for
delay, then we should use periodic_interval_max. If
periodic_interval_max is not specified, we should raise RuntimeError.
Otherwise, passing 'None' as a value, causes Exception at eventlet's
sleep() call.

Change-Id: Ida3e75bc64132d6b5920fa94657aa962e2fe9f53
Closes-bug: #1489998

oslo_service/loopingcall.py

@@ -166,6 +166,10 @@ class DynamicLoopingCall(LoopingCallBase):
 
     _RUN_ONLY_ONE_MESSAGE = _("A dynamic interval looping call can only run"
                               " one function at a time")
+    _TASK_MISSING_SLEEP_VALUE_MESSAGE = _(
+        "A dynamic interval looping call should supply either an"
+        " interval or periodic_interval_max"
+    )
 
     _KIND = _('Dynamic interval looping call')
 
@@ -173,9 +177,20 @@ class DynamicLoopingCall(LoopingCallBase):
               stop_on_exception=True):
         def _idle_for(suggested_delay, elapsed):
             delay = suggested_delay
-            if periodic_interval_max is not None:
-                delay = min(delay, periodic_interval_max)
+            if delay is None:
+                if periodic_interval_max is not None:
+                    delay = periodic_interval_max
+                else:
+                    # Note(suro-patz): An application used to receive a
+                    #     TypeError thrown from eventlet layer, before
+                    #     this RuntimeError was introduced.
+                    raise RuntimeError(
+                        self._TASK_MISSING_SLEEP_VALUE_MESSAGE)
+            else:
+                if periodic_interval_max is not None:
+                    delay = min(delay, periodic_interval_max)
             return delay
+
         return self._start(_idle_for, initial_delay=initial_delay,
                            stop_on_exception=stop_on_exception)
 

oslo_service/tests/test_loopingcall.py

@@ -192,6 +192,30 @@ class DynamicLoopingCallTestCase(test_base.BaseTestCase):
         timer = loopingcall.DynamicLoopingCall(self._wait_for_zero)
         self.assertFalse(timer.start().wait())
 
+    def _timeout_task_without_any_return(self):
+        pass
+
+    def test_timeout_task_without_return_and_max_periodic(self):
+        timer = loopingcall.DynamicLoopingCall(
+            self._timeout_task_without_any_return
+        )
+        self.assertRaises(RuntimeError, timer.start().wait)
+
+    def _timeout_task_without_return_but_with_done(self):
+        if self.num_runs == 0:
+            raise loopingcall.LoopingCallDone(False)
+        else:
+            self.num_runs = self.num_runs - 1
+
+    @mock.patch('eventlet.greenthread.sleep')
+    def test_timeout_task_without_return(self, sleep_mock):
+        self.num_runs = 1
+        timer = loopingcall.DynamicLoopingCall(
+            self._timeout_task_without_return_but_with_done
+        )
+        timer.start(periodic_interval_max=5).wait()
+        sleep_mock.assert_has_calls([mock.call(5)])
+
     @mock.patch('eventlet.greenthread.sleep')
     def test_interval_adjustment(self, sleep_mock):
         self.num_runs = 2