diff --git a/ironic/drivers/modules/agent_base_vendor.py b/ironic/drivers/modules/agent_base_vendor.py index 9094d4374b..005ded1cde 100644 --- a/ironic/drivers/modules/agent_base_vendor.py +++ b/ironic/drivers/modules/agent_base_vendor.py @@ -707,16 +707,25 @@ class AgentDeployMixin(HeartbeatMixin): if not oob_power_off: try: self._client.power_off(node) - _wait_until_powered_off(task) except Exception as e: - LOG.warning('Failed to soft power off node %(node_uuid)s ' - 'in at least %(timeout)d seconds. ' + LOG.warning('Failed to soft power off node %(node_uuid)s. ' '%(cls)s: %(error)s', {'node_uuid': node.uuid, - 'timeout': (wait * (attempts - 1)) / 1000, 'cls': e.__class__.__name__, 'error': e}, exc_info=not isinstance( e, exception.IronicException)) + + # NOTE(dtantsur): in rare cases it may happen that the power + # off request comes through but we never receive the response. + # Check the power state before trying to force off. + try: + _wait_until_powered_off(task) + except Exception: + LOG.warning('Failed to soft power off node %(node_uuid)s ' + 'in at least %(timeout)d seconds. Forcing ' + 'hard power off and proceeding.', + {'node_uuid': node.uuid, + 'timeout': (wait * (attempts - 1)) / 1000}) manager_utils.node_power_action(task, states.POWER_OFF) else: # Flush the file system prior to hard rebooting the node 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 509d17720d..c03526fd09 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py @@ -877,7 +877,10 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.assertFalse(mock_collect.called) @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) + @mock.patch.object(time, 'sleep', lambda seconds: None) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) + @mock.patch.object(fake.FakePower, 'get_power_state', + spec=types.FunctionType) @mock.patch.object(agent_client.AgentClient, 'power_off', spec=types.FunctionType) @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' @@ -886,13 +889,15 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): 'configure_tenant_networks', spec_set=True, autospec=True) def test_reboot_and_finish_deploy_soft_poweroff_fails( self, configure_tenant_net_mock, remove_provisioning_net_mock, - power_off_mock, node_power_action_mock, mock_collect): + power_off_mock, get_power_state_mock, node_power_action_mock, + mock_collect): power_off_mock.side_effect = RuntimeError("boom") self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: + get_power_state_mock.return_value = states.POWER_ON self.deploy.reboot_and_finish_deploy(task) power_off_mock.assert_called_once_with(task.node) node_power_action_mock.assert_has_calls([ @@ -905,6 +910,42 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.assertEqual(states.NOSTATE, task.node.target_provision_state) self.assertFalse(mock_collect.called) + @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) + @mock.patch.object(time, 'sleep', lambda seconds: None) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) + @mock.patch.object(fake.FakePower, 'get_power_state', + spec=types.FunctionType) + @mock.patch.object(agent_client.AgentClient, 'power_off', + spec=types.FunctionType) + @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' + 'remove_provisioning_network', spec_set=True, autospec=True) + @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' + 'configure_tenant_networks', spec_set=True, autospec=True) + def test_reboot_and_finish_deploy_soft_poweroff_race( + self, configure_tenant_net_mock, remove_provisioning_net_mock, + power_off_mock, get_power_state_mock, node_power_action_mock, + mock_collect): + # Test the situation when soft power off works, but ironic doesn't + # learn about it. + power_off_mock.side_effect = RuntimeError("boom") + self.node.provision_state = states.DEPLOYING + self.node.target_provision_state = states.ACTIVE + self.node.save() + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + get_power_state_mock.side_effect = [states.POWER_ON, + states.POWER_OFF] + self.deploy.reboot_and_finish_deploy(task) + power_off_mock.assert_called_once_with(task.node) + node_power_action_mock.assert_called_once_with( + task, states.POWER_ON) + remove_provisioning_net_mock.assert_called_once_with(mock.ANY, + task) + configure_tenant_net_mock.assert_called_once_with(mock.ANY, task) + self.assertEqual(states.ACTIVE, task.node.provision_state) + self.assertEqual(states.NOSTATE, task.node.target_provision_state) + self.assertFalse(mock_collect.called) + @mock.patch.object(manager_utils, 'power_on_node_if_needed') @mock.patch.object(driver_utils, 'collect_ramdisk_logs', autospec=True) @mock.patch.object(time, 'sleep', lambda seconds: None) diff --git a/releasenotes/notes/agent-power-off-2115fcfaac030bd0.yaml b/releasenotes/notes/agent-power-off-2115fcfaac030bd0.yaml new file mode 100644 index 0000000000..74f84fbab3 --- /dev/null +++ b/releasenotes/notes/agent-power-off-2115fcfaac030bd0.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes a rare issue when agent successfully powers off a node after + deployment, but ironic never learns about it and does another reboot.