Stop supporting incompatible heartbeat interfaces

agent_version is required for the heartbeat interfaces, we kept it for
quite a long time and now it's time to remove the support.

Change-Id: I36c3e4fbb465bb9163cf0a387e9efb36dba27a60
Story: 1602265
Task: 10478

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()
@@ -3279,29 +3274,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')

ironic/tests/unit/conductor/test_manager.py

@@ -8874,7 +8874,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',
@@ -8898,7 +8898,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',
@@ -8919,58 +8919,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,

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.