From 68cea19d99e6f6016b17df87c186144ccda3de76 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Wed, 2 Sep 2020 09:37:16 -0700 Subject: [PATCH] Minor agent version code cleanup Internal to the conductor, we have long determiend that if no agent version is submitted by the client, that the client is a version 3.0.0 agent. With agent token, we're effectively requiring 6.1.0 or later to be leveraged, so all agents should be sending versions. An agent not sending versions is thus unsupported and would no longer work without agent token support. Redudnant code has thus been removed. Additionally the conductor utility is_agent_token_supported has been removed since it is now redundant. Change-Id: Id6c8d1df08c3ac7af61ed7d05d274f3099003582 --- ironic/conductor/manager.py | 7 ++++- ironic/conductor/utils.py | 12 -------- ironic/tests/unit/conductor/test_manager.py | 29 ++++++++++++------- ironic/tests/unit/conductor/test_utils.py | 8 ----- ...gent-version-cleanup-842e3919a366b9d6.yaml | 8 +++++ 5 files changed, 32 insertions(+), 32 deletions(-) create mode 100644 releasenotes/notes/minor-agent-version-cleanup-842e3919a366b9d6.yaml diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 7860a125ce..4d5b0f608c 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -3120,7 +3120,12 @@ class ConductorManager(base_manager.BaseConductorManager): LOG.debug('RPC heartbeat called for node %s', node_id) if agent_version is None: - agent_version = '3.0.0' + LOG.error('Node %s transmitted no version information which ' + 'indicates the agent is incompatible with the ironic ' + 'services and must be upgraded.', node_id) + raise exception.InvalidParameterValue( + _('Agent did not transmit a version, and a version is ' + 'required. Please update the agent being used.')) # NOTE(dtantsur): we acquire a shared lock to begin with, drivers are # free to promote it to an exclusive one. diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index ae481283c1..473c3d4783 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -15,7 +15,6 @@ import contextlib import crypt import datetime -from distutils.version import StrictVersion import functools import os import secrets @@ -1173,17 +1172,6 @@ def is_agent_token_valid(node, token): return known_token == token -def is_agent_token_supported(agent_version): - # NOTE(TheJulia): This is hoped that 6.x supports - # agent token capabilities and realistically needs to be updated - # once that version of IPA is out there in some shape or form. - # This allows us to gracefully allow older agent's that were - # launched via pre-generated agent_tokens, to still work - # and could likely be removed at some point down the road. - version = str(agent_version).replace('.dev', 'b', 1) - return StrictVersion(version) > StrictVersion('6.1.0') - - def is_agent_token_pregenerated(node): """Determines if the token was generated for out of band configuration. diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index a45fb82197..63a44b0a4d 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -7243,10 +7243,15 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): mock_spawn.side_effect = self._fake_spawn - self.service.heartbeat(self.context, node.uuid, 'http://callback', - agent_token='magic') - mock_heartbeat.assert_called_with(mock.ANY, mock.ANY, - 'http://callback', '3.0.0', None) + exc = self.assertRaises( + messaging.rpc.ExpectedException, + self.service.heartbeat, + self.context, node.uuid, 'http://callback', + agent_token='magic') + expected_string = ('Agent did not transmit a version, and a version ' + 'is required. Please update the agent being used.') + self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) + self.assertEqual(expected_string, str(exc.exc_info[1])) @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', autospec=True) @@ -7315,9 +7320,9 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): mock_spawn.side_effect = self._fake_spawn self.service.heartbeat(self.context, node.uuid, 'http://callback', - agent_token='a secret') + '6.1.0', agent_token='a secret') mock_heartbeat.assert_called_with(mock.ANY, mock.ANY, - 'http://callback', '3.0.0', None) + 'http://callback', '6.1.0', None) @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', autospec=True) @@ -7339,9 +7344,9 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): mock_spawn.side_effect = self._fake_spawn self.service.heartbeat(self.context, node.uuid, 'http://callback', - agent_token='a secret') + '6.1.0', agent_token='a secret') mock_heartbeat.assert_called_with(mock.ANY, mock.ANY, - 'http://callback', '3.0.0', None) + 'http://callback', '6.1.0', None) @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', autospec=True) @@ -7442,7 +7447,8 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): exc = self.assertRaises(messaging.rpc.ExpectedException, self.service.heartbeat, self.context, node.uuid, 'http://callback', - agent_token='a secret') + agent_token='a secret', + agent_version='3.0.0') self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) self.assertIn('TLS is required', str(exc.exc_info[1])) self.assertFalse(mock_heartbeat.called) @@ -7469,9 +7475,10 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): mock_spawn.side_effect = self._fake_spawn self.service.heartbeat(self.context, node.uuid, 'http://callback', - agent_token='a secret', agent_verify_ca='abcd') + agent_version='6.1.0', agent_token='a secret', + agent_verify_ca='abcd') mock_heartbeat.assert_called_with( - mock.ANY, mock.ANY, 'http://callback', '3.0.0', + mock.ANY, mock.ANY, 'http://callback', '6.1.0', '/path/to/crt') diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index 2b1ea74cc1..a449bf8de0 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -2087,14 +2087,6 @@ class AgentTokenUtilsTestCase(tests_base.TestCase): conductor_utils.add_secret_token(self.node) self.assertTrue(conductor_utils.is_agent_token_present(self.node)) - def test_is_agent_token_supported(self): - self.assertTrue( - conductor_utils.is_agent_token_supported('6.1.1.dev39')) - self.assertTrue( - conductor_utils.is_agent_token_supported('6.2.1')) - self.assertFalse( - conductor_utils.is_agent_token_supported('6.0.0')) - class GetAttachedVifTestCase(db_base.DbTestCase): diff --git a/releasenotes/notes/minor-agent-version-cleanup-842e3919a366b9d6.yaml b/releasenotes/notes/minor-agent-version-cleanup-842e3919a366b9d6.yaml new file mode 100644 index 0000000000..c03a4e32ba --- /dev/null +++ b/releasenotes/notes/minor-agent-version-cleanup-842e3919a366b9d6.yaml @@ -0,0 +1,8 @@ +--- +other: + - | + The ironic conductor internal logic has been updated to return an error if + no agent version has been submitted during a heartbeat. This is because + versions have been transmitted by the agents for quite some time and + support for the default use of agent token forces all agents to be + updated. As such redundant code been removed and tests updated accordingly.