Merge "[OVN] Rate limit the "Disallow caching" log from hash ring" into stable/yoga

Author: Zuul

neutron/common/ovn/hash_ring_manager.py

@@ -35,6 +35,8 @@ def __init__(self, group_name):
         self._last_time_loaded = None
         self._check_hashring_startup = True
         self._group = group_name
+        # Flag to rate limit the caching log
+        self._prev_num_nodes = -1
         self.admin_ctx = context.get_admin_context()
 
     @property
@@ -56,11 +58,18 @@ def _wait_startup_before_caching(self):
             self.admin_ctx,
             constants.HASH_RING_CACHE_TIMEOUT, self._group, from_host=True)
 
-        if len(nodes) >= api_workers:
-            LOG.debug("Allow caching, nodes %s>=%s", len(nodes), api_workers)
+        num_nodes = len(nodes)
+        if num_nodes >= api_workers:
+            LOG.debug("Allow caching, nodes %s>=%s", num_nodes, api_workers)
             self._check_hashring_startup = False
             return False
-        LOG.debug("Disallow caching, nodes %s<%s", len(nodes), api_workers)
+
+        # NOTE(lucasagomes): We only log when the number of connected
+        # nodes are different to prevent this message from being spammed
+        if self._prev_num_nodes != num_nodes:
+            LOG.debug("Disallow caching, nodes %s<%s", num_nodes, api_workers)
+            self._prev_num_nodes = num_nodes
+
         return True
 
     def _load_hash_ring(self, refresh=False):

neutron/tests/unit/common/ovn/test_hash_ring_manager.py

@@ -110,14 +110,23 @@ def test_ring_rebalance(self):
         # The ring should re-balance and as it was before
         self._verify_hashes(hash_dict_before)
 
+    @mock.patch.object(hash_ring_manager.LOG, 'debug')
     @mock.patch.object(service, '_get_api_workers', return_value=2)
-    def test__wait_startup_before_caching(self, api_workers):
+    def test__wait_startup_before_caching(self, api_workers, mock_log):
         db_hash_ring.add_node(self.admin_ctx, HASH_RING_TEST_GROUP, 'node-1')
 
         # Assert it will return True until until we equal api_workers
         self.assertTrue(self.hash_ring_manager._wait_startup_before_caching)
         self.assertTrue(self.hash_ring_manager._check_hashring_startup)
 
+        # Call it again with the same number of workers to test the
+        # log rating
+        self.assertTrue(self.hash_ring_manager._wait_startup_before_caching)
+        self.assertTrue(self.hash_ring_manager._check_hashring_startup)
+
+        # Assert the cache message has been logged only once
+        self.assertEqual(1, mock_log.call_count)
+
         db_hash_ring.add_node(self.admin_ctx, HASH_RING_TEST_GROUP, 'node-2')
 
         # Assert it's now False. Waiting is not needed anymore