diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 0e4cbb28e7..f629f685fb 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -87,11 +87,6 @@ METRICS = metrics_utils.get_metrics_logger(__name__) SYNC_EXCLUDED_STATES = (states.DEPLOYWAIT, states.CLEANWAIT, states.ENROLL) -# NOTE(sambetts) This list is used to keep track of deprecation warnings that -# have already been issued for deploy drivers that do not accept the -# agent_version parameter and need updating. -_SEEN_AGENT_VERSION_DEPRECATIONS = [] - # NOTE(rloo) This is used to keep track of deprecation warnings that have # already been issued for deploy drivers that do not use deploy steps. _SEEN_NO_DEPLOY_STEP_DEPRECATIONS = set() @@ -3293,29 +3288,12 @@ class ConductorManager(base_manager.BaseConductorManager): if agent_version is None: agent_version = '3.0.0' - def heartbeat_with_deprecation(task, callback_url, agent_version): - global _SEEN_AGENT_VERSION_DEPRECATIONS - # FIXME(sambetts) Remove this try/except statement in Rocky making - # taking the agent_version in the deploy driver heartbeat function - # mandatory. - try: - task.driver.deploy.heartbeat(task, callback_url, agent_version) - except TypeError: - deploy_driver_name = task.driver.deploy.__class__.__name__ - if deploy_driver_name not in _SEEN_AGENT_VERSION_DEPRECATIONS: - LOG.warning('Deploy driver %s does not support ' - 'agent_version as part of the heartbeat ' - 'request, this will be required from Rocky ' - 'onward.', deploy_driver_name) - _SEEN_AGENT_VERSION_DEPRECATIONS.append(deploy_driver_name) - task.driver.deploy.heartbeat(task, callback_url) - # NOTE(dtantsur): we acquire a shared lock to begin with, drivers are # free to promote it to an exclusive one. with task_manager.acquire(context, node_id, shared=True, purpose='heartbeat') as task: task.spawn_after( - self._spawn_worker, heartbeat_with_deprecation, + self._spawn_worker, task.driver.deploy.heartbeat, task, callback_url, agent_version) @METRICS.timer('ConductorManager.vif_list') diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 5fb5c06d6b..b3564ad9ef 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -8892,7 +8892,7 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): autospec=True) @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', autospec=True) - def test_heartbeat(self, mock_spawn, mock_heartbeat): + def test_heartbeat_without_version(self, mock_spawn, mock_heartbeat): """Test heartbeating.""" node = obj_utils.create_test_node( self.context, driver='fake-hardware', @@ -8916,7 +8916,7 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): autospec=True) @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', autospec=True) - def test_heartbeat_agent_version(self, mock_spawn, mock_heartbeat): + def test_heartbeat_with_agent_version(self, mock_spawn, mock_heartbeat): """Test heartbeating.""" node = obj_utils.create_test_node( self.context, driver='fake-hardware', @@ -8937,58 +8937,6 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): mock_heartbeat.assert_called_with(mock.ANY, mock.ANY, 'http://callback', '1.4.1') - # NOTE(rloo): We cannot use autospec=True for FakeDeploy.heartbeat - # since we are testing whether our code makes a call to the old - # .heartbeat method that doesn't support 'agent_version' parameter. - @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat') - @mock.patch.object(manager, 'LOG', autospec=True) - @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', - autospec=True) - def test_heartbeat_agent_version_deprecated(self, mock_spawn, log_mock, - mock_heartbeat): - """Test heartbeating.""" - node = obj_utils.create_test_node( - self.context, driver='fake-hardware', - provision_state=states.DEPLOYING, - target_provision_state=states.ACTIVE) - - self._start_service() - - mock_spawn.reset_mock() - - def fake_spawn(conductor_obj, func, *args, **kwargs): - func(*args, **kwargs) - return mock.MagicMock() - mock_spawn.side_effect = fake_spawn - - mock_heartbeat.side_effect = [TypeError("Too many parameters"), - None, TypeError("Too many parameters"), - None] - - # NOTE(sambetts) Test to make sure deploy driver that doesn't support - # version yet falls back to old behaviour and logs a warning. - self.service.heartbeat( - self.context, node.uuid, 'http://callback', '1.4.1') - calls = [ - mock.call(mock.ANY, 'http://callback', '1.4.1'), - mock.call(mock.ANY, 'http://callback') - ] - mock_heartbeat.assert_has_calls(calls) - self.assertTrue(log_mock.warning.called) - - # NOTE(sambetts) Test to make sure that the deprecation warning isn't - # thrown again. - log_mock.reset_mock() - mock_heartbeat.reset_mock() - self.service.heartbeat( - self.context, node.uuid, 'http://callback', '1.4.1') - calls = [ - mock.call(mock.ANY, 'http://callback', '1.4.1'), - mock.call(mock.ANY, 'http://callback') - ] - mock_heartbeat.assert_has_calls(calls) - self.assertFalse(log_mock.warning.called) - @mgr_utils.mock_record_keepalive class DestroyVolumeConnectorTestCase(mgr_utils.ServiceSetUpMixin, diff --git a/releasenotes/notes/needs-agent-version-in-heartbeat-4e6806b679c53ec5.yaml b/releasenotes/notes/needs-agent-version-in-heartbeat-4e6806b679c53ec5.yaml new file mode 100644 index 0000000000..3dfb444e49 --- /dev/null +++ b/releasenotes/notes/needs-agent-version-in-heartbeat-4e6806b679c53ec5.yaml @@ -0,0 +1,5 @@ +--- +upgrade: + - | + The argument ``agent_version`` of the heartbeat interface is now mandatory + to all interfaces that inherit from HeartbeatMixin.