[OVN] Rate limit the "Disallow caching" log from hash ring

This patch adds a condition prior to logging the "Disallow caching" log
message from the hash ring.

Prior to this patch, this message was logged when the number of nodes
connected to the hash ring was different from the number of API workers
at Neutron's startup. This is because the hash ring waits until all API
workers are connected to build the hash ring cache.

With this patch, we will only log the message once (per worker) until
the number of connected nodes changes. When nodes connect and the cache
is built the _wait_startup_before_caching() is no longer used until the
service is restarted again.

Change-Id: I4f62b723083215483a2277cfcb798506671e1a2d
Closes-Bug: 1989480
Signed-off-by: Lucas Alvares Gomes <lucasagomes@gmail.com>
(cherry picked from commit 9655466763282f47a06862a47f7f31b48130277e)

2 files changed, 22 insertions, 4 deletions

diff --git a/neutron/common/ovn/hash_ring_manager.py b/neutron/common/ovn/hash_ring_manager.py
index c27f3ae936..f746d3effd 100644
--- a/neutron/common/ovn/hash_ring_manager.py
+++ b/neutron/common/ovn/hash_ring_manager.py
@@ -36,6 +36,8 @@ class HashRingManager(object):
         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
@@ -57,11 +59,18 @@ class HashRingManager(object):
             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):
diff --git a/neutron/tests/unit/common/ovn/test_hash_ring_manager.py b/neutron/tests/unit/common/ovn/test_hash_ring_manager.py
index d3a2c41a9b..3c67b72686 100644
--- a/neutron/tests/unit/common/ovn/test_hash_ring_manager.py
+++ b/neutron/tests/unit/common/ovn/test_hash_ring_manager.py
@@ -110,14 +110,23 @@ class TestHashRingManager(testlib_api.SqlTestCaseLight):
         # 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