From 161374f763f14f27bf0ff3f3c46eb420e44d77df Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Tue, 14 Feb 2023 15:09:34 +1300 Subject: [PATCH] Expose conductor online boolean for accurate alive Currently the online database column is not considered when displaying the "baremetal conductor list" Alive status. This means that when a conductor is stopped gracefully it will be shown as (inaccurately) alive for the duration of [conductor]graceful_timeout. This change adds the online field to the alive evaluation, so the conductor must be online *and* have a recent heartbeat. Change-Id: Ic5a8d56ec236faca1b9797bd0d3e42c956469fab --- ironic/api/controllers/v1/conductor.py | 3 ++- ironic/common/release_mappings.py | 2 +- ironic/objects/conductor.py | 4 +++- .../tests/unit/api/controllers/v1/test_conductor.py | 12 +++++++++++- ironic/tests/unit/db/utils.py | 1 + ironic/tests/unit/objects/test_objects.py | 2 +- .../notes/accurate_alive-d2687bca802211a4.yaml | 10 ++++++++++ 7 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/accurate_alive-d2687bca802211a4.yaml diff --git a/ironic/api/controllers/v1/conductor.py b/ironic/api/controllers/v1/conductor.py index f5663f5cfb..a523750fa9 100644 --- a/ironic/api/controllers/v1/conductor.py +++ b/ironic/api/controllers/v1/conductor.py @@ -40,8 +40,9 @@ def convert_with_links(rpc_conductor, fields=None, sanitize=True): link_resource='conductors', link_resource_args=rpc_conductor.hostname ) - conductor['alive'] = not timeutils.is_older_than( + recent_heartbeat = not timeutils.is_older_than( rpc_conductor.updated_at, CONF.conductor.heartbeat_timeout) + conductor['alive'] = rpc_conductor.online and recent_heartbeat if fields is not None: api_utils.check_for_invalid_fields(fields, conductor) diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 32dce9a1dd..ffa3541874 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -713,7 +713,7 @@ RELEASE_MAPPING = { 'Node': ['1.40', '1.39', '1.38', '1.37'], 'NodeHistory': ['1.0'], 'NodeInventory': ['1.0'], - 'Conductor': ['1.3'], + 'Conductor': ['1.4'], 'Chassis': ['1.3'], 'Deployment': ['1.0'], 'DeployTemplate': ['1.1'], diff --git a/ironic/objects/conductor.py b/ironic/objects/conductor.py index 7271525d3d..a11e19df8b 100644 --- a/ironic/objects/conductor.py +++ b/ironic/objects/conductor.py @@ -31,7 +31,8 @@ class Conductor(base.IronicObject, object_base.VersionedObjectDictCompat): # Version 1.2: Add register_hardware_interfaces() and # unregister_all_hardware_interfaces() # Version 1.3: Add conductor_group field. - VERSION = '1.3' + # Version 1.4: Add online field. + VERSION = '1.4' dbapi = db_api.get_instance() @@ -40,6 +41,7 @@ class Conductor(base.IronicObject, object_base.VersionedObjectDictCompat): 'drivers': object_fields.ListOfStringsField(nullable=True), 'hostname': object_fields.StringField(), 'conductor_group': object_fields.StringField(), + 'online': object_fields.BooleanField(), } @classmethod diff --git a/ironic/tests/unit/api/controllers/v1/test_conductor.py b/ironic/tests/unit/api/controllers/v1/test_conductor.py index 0429b6cb6a..c313b02728 100644 --- a/ironic/tests/unit/api/controllers/v1/test_conductor.py +++ b/ironic/tests/unit/api/controllers/v1/test_conductor.py @@ -99,7 +99,7 @@ class TestListConductors(test_api_base.BaseApiTest): self.assertTrue(data['alive']) @mock.patch.object(timeutils, 'utcnow', autospec=True) - def test_get_one_conductor_offline(self, mock_utcnow): + def test_get_one_conductor_offline_old_heartbeat(self, mock_utcnow): self.config(heartbeat_timeout=10, group='conductor') _time = datetime.datetime(2000, 1, 1, 0, 0) @@ -120,6 +120,16 @@ class TestListConductors(test_api_base.BaseApiTest): self.assertEqual(data['hostname'], 'rocky.rocks') self.assertFalse(data['alive']) + def test_get_one_conductor_offline_unregistered(self): + conductor = obj_utils.create_test_conductor( + self.context, hostname='rocky.rocks') + conductor.unregister() + + data = self.get_json( + '/conductors/rocky.rocks', + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertFalse(data['alive']) + def test_get_one_with_invalid_api(self): response = self.get_json( '/conductors/rocky.rocks', diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py index 8e72608cb8..f7d7cf1b24 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -415,6 +415,7 @@ def get_test_conductor(**kw): 'conductor_group': kw.get('conductor_group', ''), 'created_at': kw.get('created_at', timeutils.utcnow()), 'updated_at': kw.get('updated_at', timeutils.utcnow()), + 'online': kw.get('online', True) } diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index ceb69d90db..1222e347f4 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -681,7 +681,7 @@ expected_object_fingerprints = { 'Chassis': '1.3-d656e039fd8ae9f34efc232ab3980905', 'Port': '1.11-97bf15b61224f26c65e90f007d78bfd2', 'Portgroup': '1.5-df4dc15967f67114d51176a98a901a83', - 'Conductor': '1.3-d3f53e853b4d58cae5bfbd9a8341af4a', + 'Conductor': '1.4-a9703208fdab5fab8f1cec420be1b4a7', 'EventType': '1.1-aa2ba1afd38553e3880c267404e8d370', 'NotificationPublisher': '1.0-51a09397d6c0687771fb5be9a999605d', 'NodePayload': '1.16-9298b3aba63ab2b9c3359afd90fb9230', diff --git a/releasenotes/notes/accurate_alive-d2687bca802211a4.yaml b/releasenotes/notes/accurate_alive-d2687bca802211a4.yaml new file mode 100644 index 0000000000..87ccaa25e8 --- /dev/null +++ b/releasenotes/notes/accurate_alive-d2687bca802211a4.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Previously the ``conductors`` ``online`` database column is not considered + when displaying the "baremetal conductor list" ``Alive`` status. This means + that when a conductor is stopped gracefully it will be shown as + (inaccurately) alive for the duration of ``[conductor]graceful_timeout``. + + A conductor is now considered alive if ``online`` is true and there is a + recent enough heartbeat. \ No newline at end of file