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

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."""
+    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):

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):

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.