Rework time handling in periodic tasks.

Update time calculations in the periodic task handling to use the
timeutils module from oslo. This provides benefits for unit testing,
since we can easily override the time functions to simulate time
differences without having to actually sleep and make the unit tests
run unnecessarily long.

Resolves bug 1098979.

Change-Id: I1e6a0a0b1622a3f8c37c42376f5261f5f2dbf6fe

2 files changed, 80 insertions, 25 deletions

diff --git a/nova/manager.py b/nova/manager.py
index cc7d464ca..d6a62c509 100644
--- a/nova/manager.py
+++ b/nova/manager.py
@@ -53,8 +53,7 @@ This module provides Manager, a base class for managers.
 
 """
 
-import time
-
+import datetime
 import eventlet
 from oslo.config import cfg
 
@@ -63,6 +62,7 @@ from nova import exception
 from nova.openstack.common import log as logging
 from nova.openstack.common.plugin import pluginmanager
 from nova.openstack.common.rpc import dispatcher as rpc_dispatcher
+from nova.openstack.common import timeutils
 from nova.scheduler import rpcapi as scheduler_rpcapi
 
 
@@ -114,10 +114,11 @@ def periodic_task(*args, **kwargs):
 
         # Control frequency
         f._periodic_spacing = kwargs.pop('spacing', 0)
-        if kwargs.pop('run_immediately', False):
+        f._periodic_immediate = kwargs.pop('run_immediately', False)
+        if f._periodic_immediate:
             f._periodic_last_run = None
         else:
-            f._periodic_last_run = time.time()
+            f._periodic_last_run = timeutils.utcnow()
         return f
 
     # NOTE(sirp): The `if` is necessary to allow the decorator to be used with
@@ -220,22 +221,22 @@ class Manager(base.Base):
         for task_name, task in self._periodic_tasks:
             full_task_name = '.'.join([self.__class__.__name__, task_name])
 
+            now = timeutils.utcnow()
+            spacing = self._periodic_spacing[task_name]
+            last_run = self._periodic_last_run[task_name]
+
             # If a periodic task is _nearly_ due, then we'll run it early
-            if self._periodic_spacing[task_name] is None:
-                wait = 0
-            elif self._periodic_last_run[task_name] is None:
-                wait = 0
-            else:
-                due = (self._periodic_last_run[task_name] +
-                       self._periodic_spacing[task_name])
-                wait = max(0, due - time.time())
-                if wait > 0.2:
-                    if wait < idle_for:
-                        idle_for = wait
+            if spacing is not None and last_run is not None:
+                due = last_run + datetime.timedelta(seconds=spacing)
+                if not timeutils.is_soon(due, 0.2):
+                    idle_for = min(idle_for, timeutils.delta_seconds(now, due))
                     continue
 
+            if spacing is not None:
+                idle_for = min(idle_for, spacing)
+
             LOG.debug(_("Running periodic task %(full_task_name)s"), locals())
-            self._periodic_last_run[task_name] = time.time()
+            self._periodic_last_run[task_name] = timeutils.utcnow()
 
             try:
                 task(self, context)
@@ -244,10 +245,6 @@ class Manager(base.Base):
                     raise
                 LOG.exception(_("Error during %(full_task_name)s: %(e)s"),
                               locals())
-
-            if (not self._periodic_spacing[task_name] is None and
-                self._periodic_spacing[task_name] < idle_for):
-                idle_for = self._periodic_spacing[task_name]
             eventlet.sleep(0)
 
         return idle_for
diff --git a/nova/tests/test_periodic_tasks.py b/nova/tests/test_periodic_tasks.py
index 2539332a9..a4f594b3a 100644
--- a/nova/tests/test_periodic_tasks.py
+++ b/nova/tests/test_periodic_tasks.py
@@ -15,11 +15,12 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import time
+import datetime
 
 from testtools import matchers
 
 from nova import manager
+from nova.openstack.common import timeutils
 from nova import test
 
 
@@ -79,17 +80,74 @@ class Manager(test.TestCase):
         self.assertAlmostEqual(60, idle, 1)
 
     def test_periodic_tasks_idle_calculation(self):
+        fake_time = datetime.datetime(3000, 1, 1)
+        timeutils.set_time_override(fake_time)
+
         class Manager(manager.Manager):
             @manager.periodic_task(spacing=10)
-            def bar(self):
+            def bar(self, context):
                 return 'bar'
 
         m = Manager()
+
+        # Ensure initial values are correct
+        self.assertEqual(1, len(m._periodic_tasks))
+        task_name, task = m._periodic_tasks[0]
+
+        # Test task values
+        self.assertEqual('bar', task_name)
+        self.assertEqual(10, task._periodic_spacing)
+        self.assertEqual(True, task._periodic_enabled)
+        self.assertEqual(False, task._periodic_external_ok)
+        self.assertEqual(False, task._periodic_immediate)
+        self.assertNotEqual(None, task._periodic_last_run)
+
+        # Test the manager's representation of those values
+        self.assertEqual(10, m._periodic_spacing[task_name])
+        self.assertNotEqual(None, m._periodic_last_run[task_name])
+
+        timeutils.advance_time_delta(datetime.timedelta(seconds=5))
         m.periodic_tasks(None)
-        time.sleep(0.1)
+
+        timeutils.advance_time_delta(datetime.timedelta(seconds=5))
+        idle = m.periodic_tasks(None)
+        self.assertAlmostEqual(10, idle, 1)
+
+    def test_periodic_tasks_immediate_runs_now(self):
+        fake_time = datetime.datetime(3000, 1, 1)
+        timeutils.set_time_override(fake_time)
+
+        class Manager(manager.Manager):
+            @manager.periodic_task(spacing=10, run_immediately=True)
+            def bar(self, context):
+                return 'bar'
+
+        m = Manager()
+
+        # Ensure initial values are correct
+        self.assertEqual(1, len(m._periodic_tasks))
+        task_name, task = m._periodic_tasks[0]
+
+        # Test task values
+        self.assertEqual('bar', task_name)
+        self.assertEqual(10, task._periodic_spacing)
+        self.assertEqual(True, task._periodic_enabled)
+        self.assertEqual(False, task._periodic_external_ok)
+        self.assertEqual(True, task._periodic_immediate)
+        self.assertEqual(None, task._periodic_last_run)
+
+        # Test the manager's representation of those values
+        self.assertEqual(10, m._periodic_spacing[task_name])
+        self.assertEqual(None, m._periodic_last_run[task_name])
+
+        idle = m.periodic_tasks(None)
+        self.assertEqual(datetime.datetime(3000, 1, 1, 0, 0),
+                         m._periodic_last_run[task_name])
+        self.assertAlmostEqual(10, idle, 1)
+
+        timeutils.advance_time_delta(datetime.timedelta(seconds=5))
         idle = m.periodic_tasks(None)
-        self.assertThat(idle, matchers.GreaterThan(9.7))
-        self.assertThat(idle, matchers.LessThan(9.9))
+        self.assertAlmostEqual(5, idle, 1)
 
     def test_periodic_tasks_disabled(self):
         class Manager(manager.Manager):