Move hash_ring refresh logic out of sync_local_state

As sync_local_state periodic task can be disabled, which will lose
out on hash ring refresh, so the refresh logic should be independent
of it. this patch adds a updated_at to HashRingManager to track of
when the ring was last refreshed, and when checks the ring is set,
also check the age and rebuild if older than the new config option
hash_ring_reset_interval which is default to 180s.

Change-Id: Ie46dbf93b920543f99e11774a29878aaf27c3400
Closes-Bug: #1506657

etc/ironic/ironic.conf.sample

@@ -67,6 +67,10 @@
 # (integer value)
 #hash_distribution_replicas=1
 
+# Interval (in seconds) between hash ring resets. (integer
+# value)
+#hash_ring_reset_interval=180
+
 
 #
 # Options defined in ironic.common.images

ironic/common/hash_ring.py

@@ -16,6 +16,7 @@
 import bisect
 import hashlib
 import threading
+import time
 
 from oslo_config import cfg
 import six
@@ -47,6 +48,9 @@ hash_opts = [
                       'conductor services to prepare deployment environments '
                       'and potentially allow the Ironic cluster to recover '
                       'more quickly if a conductor instance is terminated.')),
+    cfg.IntOpt('hash_ring_reset_interval',
+               default=180,
+               help=_('Interval (in seconds) between hash ring resets.')),
 ]
 
 CONF = cfg.CONF
@@ -166,17 +170,21 @@ class HashRingManager(object):
 
     def __init__(self):
         self.dbapi = dbapi.get_instance()
+        self.updated_at = time.time()
 
     @property
     def ring(self):
+        interval = CONF.hash_ring_reset_interval
+        limit = time.time() - interval
         # Hot path, no lock
-        if self._hash_rings is not None:
+        if self._hash_rings is not None and self.updated_at >= limit:
             return self._hash_rings
 
         with self._lock:
-            if self._hash_rings is None:
+            if self._hash_rings is None or self.updated_at < limit:
                 rings = self._load_hash_rings()
                 self.__class__._hash_rings = rings
+                self.updated_at = time.time()
             return self._hash_rings
 
     def _load_hash_rings(self):

ironic/conductor/manager.py

@@ -1406,7 +1406,6 @@ class ConductorManager(periodic_task.PeriodicTasks):
         The ensuing actions could include preparing a PXE environment,
         updating the DHCP server, and so on.
         """
-        self.ring_manager.reset()
         filters = {'reserved': False,
                    'maintenance': False,
                    'provision_state': states.ACTIVE}

ironic/tests/unit/common/test_hash_ring.py

@@ -14,6 +14,7 @@
 #    under the License.
 
 import hashlib
+import time
 
 import mock
 from oslo_config import cfg
@@ -249,3 +250,14 @@ class HashRingManagerTestCase(db_base.DbTestCase):
         self.assertRaises(exception.DriverNotFound,
                           self.ring_manager.__getitem__,
                           'driver1')
+
+    def test_hash_ring_manager_refresh(self):
+        CONF.set_override('hash_ring_reset_interval', 30)
+        # Initialize the ring manager to make _hash_rings not None, then
+        # hash ring will refresh only when time interval exceeded.
+        self.assertRaises(exception.DriverNotFound,
+                          self.ring_manager.__getitem__,
+                          'driver1')
+        self.register_conductors()
+        self.ring_manager.updated_at = time.time() - 30
+        self.ring_manager.__getitem__('driver1')

ironic/tests/unit/conductor/test_manager.py

@@ -3830,7 +3830,6 @@ class ManagerSyncLocalStateTestCase(_CommonMixIn, tests_db_base.DbTestCase):
         self._assert_get_nodeinfo_args(get_nodeinfo_mock)
         mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver)
         self.assertFalse(acquire_mock.called)
-        self.service.ring_manager.reset.assert_called_once_with()
 
     def test_already_mapped(self, get_nodeinfo_mock, mapped_mock,
                             acquire_mock):
@@ -3846,7 +3845,6 @@ class ManagerSyncLocalStateTestCase(_CommonMixIn, tests_db_base.DbTestCase):
         self._assert_get_nodeinfo_args(get_nodeinfo_mock)
         mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver)
         self.assertFalse(acquire_mock.called)
-        self.service.ring_manager.reset.assert_called_once_with()
 
     def test_good(self, get_nodeinfo_mock, mapped_mock, acquire_mock):
         get_nodeinfo_mock.return_value = self._get_nodeinfo_list_response()