From 6da0268ebe9e69b891409236a88b8721ce2236eb Mon Sep 17 00:00:00 2001 From: Ruby Loo Date: Tue, 31 Oct 2017 10:14:25 -0400 Subject: [PATCH] Fix off-by-one error in warning This fixes an off-by-one error in a warning message. This is a follow-up to 3189c16a5e95ade468fa8bc37302eb9979f5a8c9. Change-Id: I89b56974c1b919f4c03498873d3ce9860d5644c5 Related-Bug: #1670916 --- ironic_python_agent/hardware.py | 10 ++++--- .../tests/unit/test_hardware.py | 26 ++++++++++++------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index f97d0c0b1..c6e510af6 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -392,6 +392,7 @@ class HardwareManager(object): if not CONF.disk_wait_attempts: return + max_waits = CONF.disk_wait_attempts - 1 for attempt in range(CONF.disk_wait_attempts): try: self.get_os_install_device() @@ -400,13 +401,16 @@ class HardwareManager(object): 'attempt %d of %d', attempt + 1, CONF.disk_wait_attempts) - if attempt < CONF.disk_wait_attempts - 1: + if attempt < max_waits: time.sleep(CONF.disk_wait_delay) else: break else: - LOG.warning('The root device was not detected in %d seconds', - CONF.disk_wait_delay * CONF.disk_wait_attempts) + if max_waits: + LOG.warning('The root device was not detected in %d seconds', + CONF.disk_wait_delay * max_waits) + else: + LOG.warning('The root device was not detected') def list_hardware_info(self): """Return full hardware inventory as a serializable dict. diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 273d75dc8..c5a698e54 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -1680,12 +1680,14 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.assertEqual(2, mocked_get_inst_dev.call_count) mocked_sleep.assert_called_once_with(CONF.disk_wait_delay) + @mock.patch.object(hardware, 'LOG', autospec=True) @mock.patch.object(hardware.GenericHardwareManager, 'get_os_install_device', autospec=True) @mock.patch.object(hardware, '_check_for_iscsi', autospec=True) @mock.patch.object(time, 'sleep', autospec=True) def test_evaluate_hw_no_wait_for_disks( - self, mocked_sleep, mocked_check_for_iscsi, mocked_get_inst_dev): + self, mocked_sleep, mocked_check_for_iscsi, mocked_get_inst_dev, + mocked_log): CONF.set_override('disk_wait_attempts', '0') result = self.hardware.evaluate_hardware_support() @@ -1694,13 +1696,15 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.assertEqual(hardware.HardwareSupport.GENERIC, result) self.assertFalse(mocked_get_inst_dev.called) self.assertFalse(mocked_sleep.called) + self.assertFalse(mocked_log.called) + @mock.patch.object(hardware, 'LOG', autospec=True) @mock.patch.object(hardware, '_check_for_iscsi', mock.Mock()) @mock.patch.object(hardware.GenericHardwareManager, 'get_os_install_device', autospec=True) @mock.patch.object(time, 'sleep', autospec=True) def test_evaluate_hw_waits_for_disks_nonconfigured( - self, mocked_sleep, mocked_get_inst_dev): + self, mocked_sleep, mocked_get_inst_dev, mocked_log): mocked_get_inst_dev.side_effect = [ errors.DeviceNotFound('boom'), errors.DeviceNotFound('boom'), @@ -1722,18 +1726,21 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.assertEqual(10, mocked_get_inst_dev.call_count) expected_calls = [mock.call(CONF.disk_wait_delay)] * 9 mocked_sleep.assert_has_calls(expected_calls) + mocked_log.warning.assert_called_once_with( + 'The root device was not detected in %d seconds', + CONF.disk_wait_delay * 9) + @mock.patch.object(hardware, 'LOG', autospec=True) @mock.patch.object(hardware, '_check_for_iscsi', mock.Mock()) @mock.patch.object(hardware.GenericHardwareManager, 'get_os_install_device', autospec=True) @mock.patch.object(time, 'sleep', autospec=True) def test_evaluate_hw_waits_for_disks_configured(self, mocked_sleep, - mocked_get_inst_dev): - CONF.set_override('disk_wait_attempts', '2') + mocked_get_inst_dev, + mocked_log): + CONF.set_override('disk_wait_attempts', '1') mocked_get_inst_dev.side_effect = [ - errors.DeviceNotFound('boom'), - errors.DeviceNotFound('boom'), errors.DeviceNotFound('boom'), errors.DeviceNotFound('boom'), None @@ -1742,9 +1749,10 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.hardware.evaluate_hardware_support() mocked_get_inst_dev.assert_called_with(mock.ANY) - self.assertEqual(2, mocked_get_inst_dev.call_count) - expected_calls = [mock.call(CONF.disk_wait_delay)] * 1 - mocked_sleep.assert_has_calls(expected_calls) + self.assertEqual(1, mocked_get_inst_dev.call_count) + self.assertFalse(mocked_sleep.called) + mocked_log.warning.assert_called_once_with( + 'The root device was not detected') @mock.patch.object(hardware, '_check_for_iscsi', mock.Mock()) @mock.patch.object(hardware.GenericHardwareManager,