diff options
author | Zuul <zuul@review.opendev.org> | 2019-08-16 22:59:23 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2019-08-16 22:59:23 +0000 |
commit | cdf5bca715cf5f1382614539426ec5536cb4e314 (patch) | |
tree | fceca78031e9b5ae84603fc355c6618d2dbe05fd | |
parent | f21f8b74d26bdb080517cc4e1d514a9307fa2692 (diff) | |
parent | 15cba675f23c49b5ebf1c3ff9fbdf8785d06f05e (diff) | |
download | ironic-cdf5bca715cf5f1382614539426ec5536cb4e314.tar.gz |
Merge "Check for deploy.deploy deploy step in heartbeat" into stable/stein
3 files changed, 175 insertions, 14 deletions
diff --git a/ironic/drivers/modules/agent_base_vendor.py b/ironic/drivers/modules/agent_base_vendor.py index 5ba8ff910..fff521673 100644 --- a/ironic/drivers/modules/agent_base_vendor.py +++ b/ironic/drivers/modules/agent_base_vendor.py @@ -250,6 +250,24 @@ class HeartbeatMixin(object): :returns: True if the deployment is completed. False otherwise """ + def in_core_deploy_step(self, task): + """Check if we are in the deploy.deploy deploy step. + + Assumes that we are in the DEPLOYWAIT state. + + :param task: a TaskManager instance + :returns: True if the current deploy step is deploy.deploy. + """ + # TODO(mgoddard): Remove this 'if' in the Train release, after the + # deprecation period for supporting drivers with no deploy steps. + if not task.node.driver_internal_info.get('deploy_steps'): + return True + + step = task.node.deploy_step + return (step + and step['interface'] == 'deploy' + and step['step'] == 'deploy') + def reboot_to_instance(self, task): """Method invoked after the deployment is completed. @@ -333,17 +351,22 @@ class HeartbeatMixin(object): LOG.debug('Heartbeat from node %(node)s in maintenance mode; ' 'not taking any action.', {'node': node.uuid}) return - elif (node.provision_state == states.DEPLOYWAIT - and not self.deploy_has_started(task)): - msg = _('Node failed to deploy.') - self.continue_deploy(task) - elif (node.provision_state == states.DEPLOYWAIT - and self.deploy_is_done(task)): - msg = _('Node failed to move to active state.') - self.reboot_to_instance(task) - elif (node.provision_state == states.DEPLOYWAIT - and self.deploy_has_started(task)): - node.touch_provisioning() + # NOTE(mgoddard): Only handle heartbeats during DEPLOYWAIT if we + # are currently in the core deploy.deploy step. Other deploy steps + # may cause the agent to boot, but we should not trigger deployment + # at that point. + elif node.provision_state == states.DEPLOYWAIT: + if self.in_core_deploy_step(task): + if not self.deploy_has_started(task): + msg = _('Node failed to deploy.') + self.continue_deploy(task) + elif self.deploy_is_done(task): + msg = _('Node failed to move to active state.') + self.reboot_to_instance(task) + else: + node.touch_provisioning() + else: + node.touch_provisioning() elif node.provision_state == states.CLEANWAIT: node.touch_provisioning() if not node.clean_step: diff --git a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py index c175032c8..5a21ffe28 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py @@ -80,6 +80,97 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): super(HeartbeatMixinTest, self).setUp() self.deploy = agent_base_vendor.HeartbeatMixin() + @mock.patch.object(agent_base_vendor.HeartbeatMixin, + 'in_core_deploy_step', autospec=True) + @mock.patch.object(agent_base_vendor.HeartbeatMixin, + 'deploy_has_started', autospec=True) + @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'continue_deploy', + autospec=True) + @mock.patch.object(agent_base_vendor.HeartbeatMixin, + 'reboot_to_instance', autospec=True) + def test_heartbeat_continue_deploy(self, rti_mock, cd_mock, + deploy_started_mock, + in_deploy_mock): + in_deploy_mock.return_value = True + deploy_started_mock.return_value = False + self.node.provision_state = states.DEPLOYWAIT + self.node.save() + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + self.deploy.heartbeat(task, 'url', '3.2.0') + self.assertFalse(task.shared) + self.assertEqual( + 'url', task.node.driver_internal_info['agent_url']) + self.assertEqual( + '3.2.0', + task.node.driver_internal_info['agent_version']) + cd_mock.assert_called_once_with(self.deploy, task) + self.assertFalse(rti_mock.called) + + @mock.patch.object(agent_base_vendor.HeartbeatMixin, + 'in_core_deploy_step', autospec=True) + @mock.patch.object(agent_base_vendor.HeartbeatMixin, + 'deploy_has_started', autospec=True) + @mock.patch.object(agent_base_vendor.HeartbeatMixin, + 'deploy_is_done', autospec=True) + @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'continue_deploy', + autospec=True) + @mock.patch.object(agent_base_vendor.HeartbeatMixin, + 'reboot_to_instance', autospec=True) + def test_heartbeat_reboot_to_instance(self, rti_mock, cd_mock, + deploy_is_done_mock, + deploy_started_mock, + in_deploy_mock): + in_deploy_mock.return_value = True + deploy_started_mock.return_value = True + deploy_is_done_mock.return_value = True + self.node.provision_state = states.DEPLOYWAIT + self.node.save() + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + self.deploy.heartbeat(task, 'url', '3.2.0') + self.assertFalse(task.shared) + self.assertEqual( + 'url', task.node.driver_internal_info['agent_url']) + self.assertEqual( + '3.2.0', + task.node.driver_internal_info['agent_version']) + self.assertFalse(cd_mock.called) + rti_mock.assert_called_once_with(self.deploy, task) + + @mock.patch.object(agent_base_vendor.HeartbeatMixin, + 'in_core_deploy_step', autospec=True) + @mock.patch.object(agent_base_vendor.HeartbeatMixin, + 'deploy_has_started', autospec=True) + @mock.patch.object(agent_base_vendor.HeartbeatMixin, + 'deploy_is_done', autospec=True) + @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'continue_deploy', + autospec=True) + @mock.patch.object(agent_base_vendor.HeartbeatMixin, + 'reboot_to_instance', autospec=True) + def test_heartbeat_not_in_core_deploy_step(self, rti_mock, cd_mock, + deploy_is_done_mock, + deploy_started_mock, + in_deploy_mock): + # Check that heartbeats do not trigger deployment actions when not in + # the deploy.deploy step. + in_deploy_mock.return_value = False + self.node.provision_state = states.DEPLOYWAIT + self.node.save() + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + self.deploy.heartbeat(task, 'url', '3.2.0') + self.assertFalse(task.shared) + self.assertEqual( + 'url', task.node.driver_internal_info['agent_url']) + self.assertEqual( + '3.2.0', + task.node.driver_internal_info['agent_version']) + self.assertFalse(deploy_started_mock.called) + self.assertFalse(deploy_is_done_mock.called) + self.assertFalse(cd_mock.called) + self.assertFalse(rti_mock.called) + @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'continue_deploy', autospec=True) @mock.patch.object(agent_base_vendor.HeartbeatMixin, @@ -158,13 +249,17 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): self.assertEqual(0, cd_mock.call_count) @mock.patch.object(agent_base_vendor.HeartbeatMixin, + 'in_core_deploy_step', autospec=True) + @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'deploy_has_started', autospec=True) @mock.patch.object(deploy_utils, 'set_failed_state', autospec=True) @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'deploy_is_done', autospec=True) @mock.patch.object(agent_base_vendor.LOG, 'exception', autospec=True) def test_heartbeat_deploy_done_fails(self, log_mock, done_mock, - failed_mock, deploy_started_mock): + failed_mock, deploy_started_mock, + in_deploy_mock): + in_deploy_mock.return_value = True deploy_started_mock.return_value = True done_mock.side_effect = Exception('LlamaException') with task_manager.acquire( @@ -180,6 +275,8 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): {'node': task.node.uuid}) @mock.patch.object(agent_base_vendor.HeartbeatMixin, + 'in_core_deploy_step', autospec=True) + @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'deploy_has_started', autospec=True) @mock.patch.object(deploy_utils, 'set_failed_state', autospec=True) @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'deploy_is_done', @@ -187,7 +284,9 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): @mock.patch.object(agent_base_vendor.LOG, 'exception', autospec=True) def test_heartbeat_deploy_done_raises_with_event(self, log_mock, done_mock, failed_mock, - deploy_started_mock): + deploy_started_mock, + in_deploy_mock): + in_deploy_mock.return_value = True deploy_started_mock.return_value = True with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: @@ -341,12 +440,16 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): task, 'Asynchronous exception: Node failed to perform ' 'rescue operation. Exception: some failure for node') + @mock.patch.object(agent_base_vendor.HeartbeatMixin, + 'in_core_deploy_step', autospec=True) @mock.patch.object(objects.node.Node, 'touch_provisioning', autospec=True) @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'deploy_has_started', autospec=True) def test_heartbeat_touch_provisioning_and_url_save(self, mock_deploy_started, - mock_touch): + mock_touch, + mock_in_deploy): + mock_in_deploy.return_value = True mock_deploy_started.return_value = True self.node.provision_state = states.DEPLOYWAIT @@ -381,6 +484,35 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): task.node.driver_internal_info['agent_last_heartbeat']) self.assertEqual(provision_state, task.node.provision_state) + def test_in_core_deploy_step(self): + self.node.deploy_step = { + 'interface': 'deploy', 'step': 'deploy', 'priority': 100} + info = self.node.driver_internal_info + info['deploy_steps'] = [self.node.deploy_step] + self.node.driver_internal_info = info + self.node.save() + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.assertTrue(self.deploy.in_core_deploy_step(task)) + + def test_in_core_deploy_step_no_steps_list(self): + # Need to handle drivers without deploy step support, remove in the + # Train release. + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.assertTrue(self.deploy.in_core_deploy_step(task)) + + def test_in_core_deploy_step_in_other_step(self): + self.node.deploy_step = { + 'interface': 'deploy', 'step': 'other-step', 'priority': 100} + info = self.node.driver_internal_info + info['deploy_steps'] = [self.node.deploy_step] + self.node.driver_internal_info = info + self.node.save() + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.assertFalse(self.deploy.in_core_deploy_step(task)) + class AgentRescueTests(AgentDeployMixinBaseTest): diff --git a/releasenotes/notes/fix-deploywait-errors-during-deploy-5a4279c0c1a6d4d9.yaml b/releasenotes/notes/fix-deploywait-errors-during-deploy-5a4279c0c1a6d4d9.yaml new file mode 100644 index 000000000..ea47511f4 --- /dev/null +++ b/releasenotes/notes/fix-deploywait-errors-during-deploy-5a4279c0c1a6d4d9.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes spurious deployment warnings being logged by the + ``ironic-conductor`` service indicating that the heartbeats from the + deployment ramdisk could not be processed in ``DEPLOYWAIT`` state. |