Deprecate removing old status and disable it by default

We no longer have to do it, now that we can remove statuses for removed nodes.

Change-Id: Iacc546d265270983c6a360a92073acde9d9b36c7
Closes-Bug: #1695858

example.conf

@@ -22,10 +22,12 @@
 # disable. (integer value)
 #timeout = 3600
 
-# For how much time (in seconds) to keep status information about
+# DEPRECATED: For how much time (in seconds) to keep status
-# nodes after introspection was finished for them. Default value is 1
+# information about nodes after introspection was finished for them.
-# week. (integer value)
+# Set to 0 (the default) to disable the timeout. (integer value)
-#node_status_keep_time = 604800
+# This option is deprecated for removal.
+# Its value may be silently ignored in the future.
+#node_status_keep_time = 0
 
 # Amount of time in seconds, after which repeat clean up of timed out
 # nodes and old nodes status information. (integer value)

ironic_inspector/conf.py

@@ -155,10 +155,12 @@ SERVICE_OPTS = [
                help=_('Timeout after which introspection is considered '
                       'failed, set to 0 to disable.')),
     cfg.IntOpt('node_status_keep_time',
-               default=604800,
+               default=0,
                help=_('For how much time (in seconds) to keep status '
                       'information about nodes after introspection was '
-                      'finished for them. Default value is 1 week.')),
+                      'finished for them. Set to 0 (the default) '
+                      'to disable the timeout.'),
+               deprecated_for_removal=True),
     cfg.IntOpt('clean_up_period',
                default=60,
                help=_('Amount of time in seconds, after which repeat clean up '

ironic_inspector/node_cache.py

@@ -864,22 +864,23 @@ def clean_up():
 
     :return: list of timed out node UUID's
     """
-    status_keep_threshold = (timeutils.utcnow() - datetime.timedelta(
+    if CONF.node_status_keep_time > 0:
-                             seconds=CONF.node_status_keep_time))
+        status_keep_threshold = (timeutils.utcnow() - datetime.timedelta(
+                                 seconds=CONF.node_status_keep_time))
+        with db.ensure_transaction() as session:
+            db.model_query(db.Node, session=session).filter(
+                db.Node.finished_at.isnot(None),
+                db.Node.finished_at < status_keep_threshold).delete()
 
-    with db.ensure_transaction() as session:
+    timeout = CONF.timeout
-        db.model_query(db.Node, session=session).filter(
+    if timeout <= 0:
-            db.Node.finished_at.isnot(None),
+        return []
-            db.Node.finished_at < status_keep_threshold).delete()
+    threshold = timeutils.utcnow() - datetime.timedelta(seconds=timeout)
+    uuids = [row.uuid for row in
+             db.model_query(db.Node.uuid).filter(
+                 db.Node.started_at < threshold,
+                 db.Node.finished_at.is_(None)).all()]
 
-        timeout = CONF.timeout
-        if timeout <= 0:
-            return []
-        threshold = timeutils.utcnow() - datetime.timedelta(seconds=timeout)
-        uuids = [row.uuid for row in
-                 db.model_query(db.Node.uuid, session=session).filter(
-                     db.Node.started_at < threshold,
-                     db.Node.finished_at.is_(None)).all()]
     if not uuids:
         return []
 

ironic_inspector/test/unit/test_node_cache.py

@@ -410,6 +410,18 @@ class TestNodeCacheCleanUp(test_base.NodeTest):
 
         self.assertEqual([], db.model_query(db.Node).all())
 
+    def test_old_status_disabled(self):
+        # Status clean up is disabled by default
+        session = db.get_session()
+        with session.begin():
+            db.model_query(db.Node).update(
+                {'finished_at': (datetime.datetime.utcnow() -
+                                 datetime.timedelta(days=10000))})
+
+        self.assertEqual([], node_cache.clean_up())
+
+        self.assertNotEqual([], db.model_query(db.Node).all())
+
 
 class TestNodeCacheGetNode(test_base.NodeTest):
     def test_ok(self):

releasenotes/notes/status-removal-fa1d9a98ffad9f60.yaml

@@ -0,0 +1,11 @@
+---
+upgrade:
+  - |
+    Old status records are no longer removed by default. They are still
+    removed if a node is removed from Ironic.
+deprecations:
+  - |
+    The ``node_status_keep_time`` configuration option is deprecated. Now that
+    we can remove status information about nodes removed from **ironic**, this
+    option does not make much sense, and maybe be confusing (see `bug 1695858
+    <https://bugs.launchpad.net/ironic-inspector/+bug/1695858>`_).