From 714f1cc86b6a9e73f2377eb244a420f144f303aa Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Wed, 12 Jun 2024 17:18:47 +0200
Subject: [PATCH] Cache openstacksdk Connections to avoid leaking memory

The current code never closes connection. Newer openstacksdk versions
register Connection.close with the atexit mechanism, so any connections
that are not explicitly closed stay in memory forever.

Change-Id: I18bbb460cbaa4f58f9e736c071571c38ced35892
(cherry picked from commit 48a3bf605b1d110b505d30e208b6ea64a121f848)
---
 ironic_inspector/common/ironic.py             | 32 ++++++++++++++-----
 .../test/unit/test_common_ironic.py           |  8 +++--
 .../notes/sdk-2-leak-500f3669afb6713e.yaml    |  5 +++
 3 files changed, 35 insertions(+), 10 deletions(-)
 create mode 100644 releasenotes/notes/sdk-2-leak-500f3669afb6713e.yaml

diff --git a/ironic_inspector/common/ironic.py b/ironic_inspector/common/ironic.py
index e09ae3595..9d4a111de 100644
--- a/ironic_inspector/common/ironic.py
+++ b/ironic_inspector/common/ironic.py
@@ -18,6 +18,7 @@ import netaddr
 import openstack
 from openstack import exceptions as os_exc
 from oslo_config import cfg
+from oslo_utils import excutils
 import tenacity
 
 from ironic_inspector.common.i18n import _
@@ -35,6 +36,7 @@ VALID_STATES = frozenset(['enroll', 'manageable', 'inspecting', 'inspect wait',
 VALID_ACTIVE_STATES = frozenset(['active', 'rescue'])
 
 _IRONIC_SESSION = None
+_CONNECTION = None
 
 
 class NotFound(utils.Error):
@@ -55,15 +57,29 @@ def _get_ironic_session():
 
 def get_client(token=None):
     """Get an ironic client connection."""
-    session = _get_ironic_session()
+    global _CONNECTION
+
+    if _CONNECTION is None:
+        try:
+            session = _get_ironic_session()
+            _CONNECTION = openstack.connection.Connection(
+                session=session, oslo_conf=CONF)
+        except Exception as exc:
+            LOG.error('Failed to create an openstack connection: %s', exc)
+            raise
 
     try:
-        return openstack.connection.Connection(
-            session=session, oslo_conf=CONF).baremetal
+        return _CONNECTION.baremetal
     except Exception as exc:
-        LOG.error('Failed to establish a connection with ironic, '
-                  'reason: %s', exc)
-        raise
+        with excutils.save_and_reraise_exception():
+            LOG.error('Failed to connect to Ironic: %s', exc)
+            # Force creating a new connection on the next retry
+            try:
+                _CONNECTION.close()
+            except Exception as exc2:
+                LOG.error('Unable to close an openstack connection, '
+                          'a memory leak is possible. Error: %s', exc2)
+            _CONNECTION = None
 
 
 def reset_ironic_session():
@@ -71,8 +87,8 @@ def reset_ironic_session():
 
     Mostly useful for unit tests.
     """
-    global _IRONIC_SESSION
-    _IRONIC_SESSION = None
+    global _IRONIC_SESSION, _CONNECTION
+    _CONNECTION = _IRONIC_SESSION = None
 
 
 def get_ipmi_address(node):
diff --git a/ironic_inspector/test/unit/test_common_ironic.py b/ironic_inspector/test/unit/test_common_ironic.py
index 7857ff35d..bf538a916 100644
--- a/ironic_inspector/test/unit/test_common_ironic.py
+++ b/ironic_inspector/test/unit/test_common_ironic.py
@@ -32,8 +32,12 @@ class TestGetClientBase(base.BaseTest):
         ir_utils.reset_ironic_session()
 
     def test_get_client(self, mock_connection, mock_session):
-        ir_utils.get_client()
-        self.assertEqual(1, mock_session.call_count)
+        for i in range(3):
+            cli = ir_utils.get_client()
+            self.assertIs(mock_connection.return_value.baremetal, cli)
+        mock_session.assert_called_once_with('ironic')
+        mock_connection.assert_called_once_with(
+            session=mock_session.return_value, oslo_conf=ir_utils.CONF)
 
 
 class TestGetIpmiAddress(base.BaseTest):
diff --git a/releasenotes/notes/sdk-2-leak-500f3669afb6713e.yaml b/releasenotes/notes/sdk-2-leak-500f3669afb6713e.yaml
new file mode 100644
index 000000000..c62adc805
--- /dev/null
+++ b/releasenotes/notes/sdk-2-leak-500f3669afb6713e.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixes memory leak with openstacksdk 2.0 and newer. This version requires
+    connections to be explicitly closed, otherwise they stay in memory forever.