Gracefully handle NodeLocked exceptions during heartbeat

Currently if it happens we allow the exception to fall through, which
results in node's last_error populated. Since it's not supposed to be
a dangerous condition, only log a warning instead.

Change-Id: I3419824a94d305101b987ffa0bb901be158f916c
Story: #2002094
Task: #19785

ironic/drivers/modules/agent_base_vendor.py

@@ -290,7 +290,13 @@ class HeartbeatMixin(object):
                        'state': task.node.provision_state})
             return
 
-        task.upgrade_lock()
+        try:
+            task.upgrade_lock()
+        except exception.NodeLocked:
+            LOG.warning('Node %s is currently locked, skipping heartbeat '
+                        'processing (will retry on the next heartbeat)',
+                        task.node.uuid)
+            return
 
         node = task.node
         LOG.debug('Heartbeat from node %s', node.uuid)

ironic/tests/unit/drivers/modules/test_agent_base_vendor.py

@@ -107,6 +107,33 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
             self.assertEqual(0, rti_mock.call_count)
             self.assertEqual(0, cd_mock.call_count)
 
+    @mock.patch('time.sleep', lambda _t: None)
+    @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'continue_deploy',
+                       autospec=True)
+    @mock.patch.object(agent_base_vendor.HeartbeatMixin,
+                       'reboot_to_instance', autospec=True)
+    @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean',
+                       autospec=True)
+    def test_heartbeat_with_reservation(self, ncrc_mock, rti_mock, cd_mock):
+        # NOTE(pas-ha) checking only for states that are not noop
+        for state in (states.DEPLOYWAIT, states.CLEANWAIT):
+            for m in (ncrc_mock, rti_mock, cd_mock):
+                m.reset_mock()
+            self.node.provision_state = state
+            self.node.reservation = 'localhost'
+            self.node.save()
+            old_drv_info = self.node.driver_internal_info.copy()
+            agent_url = 'url-%s' % state
+            with task_manager.acquire(self.context, self.node.uuid,
+                                      shared=True) as task:
+                self.deploy.heartbeat(task, agent_url, '3.2.0')
+                self.assertTrue(task.shared)
+                self.assertEqual(old_drv_info, task.node.driver_internal_info)
+                self.assertIsNone(task.node.last_error)
+            self.assertEqual(0, ncrc_mock.call_count)
+            self.assertEqual(0, rti_mock.call_count)
+            self.assertEqual(0, cd_mock.call_count)
+
     @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'continue_deploy',
                        autospec=True)
     @mock.patch.object(agent_base_vendor.HeartbeatMixin,

releasenotes/notes/heartbeat-locked-6e53b68337d5a258.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    When a node is found to be locked when a heart beat arrives, no longer
+    records the error in node's ``last_error`` field, since it's not
+    an error in most cases.