diff --git a/example.conf b/example.conf index fa29ca0bd..82185aab8 100644 --- a/example.conf +++ b/example.conf @@ -22,10 +22,12 @@ # disable. (integer value) #timeout = 3600 -# For how much time (in seconds) to keep status information about -# nodes after introspection was finished for them. Default value is 1 -# week. (integer value) -#node_status_keep_time = 604800 +# DEPRECATED: For how much time (in seconds) to keep status +# information about nodes after introspection was finished for them. +# Set to 0 (the default) to disable the timeout. (integer value) +# 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) diff --git a/ironic_inspector/conf.py b/ironic_inspector/conf.py index a67f93443..97f13609f 100644 --- a/ironic_inspector/conf.py +++ b/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 ' diff --git a/ironic_inspector/node_cache.py b/ironic_inspector/node_cache.py index 7ccaa5e55..d0da5634e 100644 --- a/ironic_inspector/node_cache.py +++ b/ironic_inspector/node_cache.py @@ -865,22 +865,23 @@ def clean_up(): :return: list of timed out node UUID's """ - status_keep_threshold = (timeutils.utcnow() - datetime.timedelta( - seconds=CONF.node_status_keep_time)) + if CONF.node_status_keep_time > 0: + 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: - db.model_query(db.Node, session=session).filter( - db.Node.finished_at.isnot(None), - db.Node.finished_at < status_keep_threshold).delete() + 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).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 [] diff --git a/ironic_inspector/test/unit/test_node_cache.py b/ironic_inspector/test/unit/test_node_cache.py index 2c185ce01..ff0f1f79f 100644 --- a/ironic_inspector/test/unit/test_node_cache.py +++ b/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): diff --git a/releasenotes/notes/status-removal-fa1d9a98ffad9f60.yaml b/releasenotes/notes/status-removal-fa1d9a98ffad9f60.yaml new file mode 100644 index 000000000..226102f4f --- /dev/null +++ b/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 + `_).