diff options
author | Dmitry Tantsur <dtantsur@protonmail.com> | 2021-01-20 12:11:54 +0100 |
---|---|---|
committer | Dmitry Tantsur <dtantsur@protonmail.com> | 2021-01-28 16:41:45 +0100 |
commit | 121b3348c813eb075ca38bd264a9315ee3acc2fe (patch) | |
tree | d5d142ae8da3cf56a4bb198c2d42ccc9ba5bbcd0 /ironic/tests | |
parent | fd34d3c437e15dd53c0acac39bdf600123fbb1de (diff) | |
download | ironic-121b3348c813eb075ca38bd264a9315ee3acc2fe.tar.gz |
Refactor vendor detection and add Redfish implementation
Get rid of the TODO in the code and prepare for more management
interfaces supporting detect_vendor(). Vendor detecting now runs
during transition to manageable and on power state sync (essentially
same as before but for all drivers not only IPMI).
Update the IPMI implementation to no longer hide exceptions since
they're not handled on the upper level. Simplify the regex and fix
the docstring.
Add the Redfish implementation as a foundation for future
vendor-specific changes.
Change-Id: Ie521cf2295613dde5842cbf9a053540a40be4b9c
Diffstat (limited to 'ironic/tests')
-rw-r--r-- | ironic/tests/unit/conductor/test_manager.py | 24 | ||||
-rw-r--r-- | ironic/tests/unit/conductor/test_utils.py | 73 | ||||
-rw-r--r-- | ironic/tests/unit/drivers/modules/redfish/test_management.py | 8 | ||||
-rw-r--r-- | ironic/tests/unit/drivers/modules/test_ipmitool.py | 21 |
4 files changed, 108 insertions, 18 deletions
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index b3d3f4251..c5fd84279 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -2981,6 +2981,7 @@ class DoNodeRescueTestCase(mgr_utils.CommonMixIn, mgr_utils.ServiceSetUpMixin, @mgr_utils.mock_record_keepalive class DoNodeVerifyTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): + @mock.patch.object(conductor_utils, 'node_cache_vendor', autospec=True) @mock.patch('ironic.objects.node.NodeCorrectedPowerStateNotification', autospec=True) @mock.patch('ironic.drivers.modules.fake.FakePower.get_power_state', @@ -2988,7 +2989,7 @@ class DoNodeVerifyTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): @mock.patch('ironic.drivers.modules.fake.FakePower.validate', autospec=True) def test__do_node_verify(self, mock_validate, mock_get_power_state, - mock_notif): + mock_notif, mock_cache_vendor): self._start_service() mock_get_power_state.return_value = states.POWER_OFF # Required for exception handling @@ -3003,6 +3004,7 @@ class DoNodeVerifyTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): with task_manager.acquire( self.context, node['id'], shared=False) as task: self.service._do_node_verify(task) + mock_cache_vendor.assert_called_once_with(task) self._stop_service() @@ -4672,6 +4674,8 @@ class ManagerDoSyncPowerStateTestCase(db_base.DbTestCase): super(ManagerDoSyncPowerStateTestCase, self).setUp() self.service = manager.ConductorManager('hostname', 'test-topic') self.driver = mock.Mock(spec_set=drivers_base.BareDriver) + self.driver.management.detect_vendor.side_effect = \ + exception.UnsupportedDriverExtension self.power = self.driver.power self.node = obj_utils.create_test_node( self.context, driver='fake-hardware', maintenance=False, @@ -4984,6 +4988,24 @@ class ManagerDoSyncPowerStateTestCase(db_base.DbTestCase): self.assertFalse(node_power_action.called) self.task.upgrade_lock.assert_called_once_with() + @mock.patch.object(nova, 'power_update', autospec=True) + def test_vendor_detection(self, mock_power_update, node_power_action): + self.driver.management.detect_vendor.side_effect = [ + "Fake Inc." + ] + self._do_sync_power_state(states.POWER_ON, states.POWER_OFF) + + self.assertFalse(self.power.validate.called) + self.power.get_power_state.assert_called_once_with(self.task) + self.assertFalse(node_power_action.called) + self.assertEqual(states.POWER_OFF, self.node.power_state) + # node_cache_vendor calls upgrade_lock, then power update does it once + # more (which is safe because TaskManager checks its state) + self.assertEqual(2, self.task.upgrade_lock.call_count) + mock_power_update.assert_called_once_with( + self.task.context, self.node.instance_uuid, states.POWER_OFF) + self.assertEqual("Fake Inc.", self.node.properties['vendor']) + @mock.patch.object(waiters, 'wait_for_all', new=mock.MagicMock(return_value=(0, 0))) diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index a41559898..8d0bf5f35 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -2195,3 +2195,76 @@ class StoreAgentCertificateTestCase(db_base.DbTestCase): self.assertEqual(self.fname, result) with open(self.fname, 'rt') as fp: self.assertEqual('cert text', fp.read()) + + +@mock.patch.object(fake.FakeManagement, 'detect_vendor', autospec=True, + return_value="Fake Inc.") +class CacheVendorTestCase(db_base.DbTestCase): + + def setUp(self): + super(CacheVendorTestCase, self).setUp() + self.node = obj_utils.create_test_node(self.context, + driver='fake-hardware', + properties={}) + + def test_ok(self, mock_detect): + with task_manager.acquire(self.context, self.node.id, + shared=True) as task: + conductor_utils.node_cache_vendor(task) + self.assertFalse(task.shared) + mock_detect.assert_called_once_with(task.driver.management, task) + + self.node.refresh() + self.assertEqual("Fake Inc.", self.node.properties['vendor']) + + def test_already_present(self, mock_detect): + self.node.properties = {'vendor': "Fake GmbH"} + self.node.save() + + with task_manager.acquire(self.context, self.node.id, + shared=True) as task: + conductor_utils.node_cache_vendor(task) + self.assertTrue(task.shared) + + self.node.refresh() + self.assertEqual("Fake GmbH", self.node.properties['vendor']) + self.assertFalse(mock_detect.called) + + def test_empty(self, mock_detect): + mock_detect.return_value = None + with task_manager.acquire(self.context, self.node.id, + shared=True) as task: + conductor_utils.node_cache_vendor(task) + self.assertTrue(task.shared) + mock_detect.assert_called_once_with(task.driver.management, task) + + self.node.refresh() + self.assertNotIn('vendor', self.node.properties) + + @mock.patch.object(conductor_utils.LOG, 'warning', autospec=True) + def test_unsupported(self, mock_log, mock_detect): + mock_detect.side_effect = exception.UnsupportedDriverExtension + + with task_manager.acquire(self.context, self.node.id, + shared=True) as task: + conductor_utils.node_cache_vendor(task) + self.assertTrue(task.shared) + mock_detect.assert_called_once_with(task.driver.management, task) + + self.node.refresh() + self.assertNotIn('vendor', self.node.properties) + self.assertFalse(mock_log.called) + + @mock.patch.object(conductor_utils.LOG, 'warning', autospec=True) + def test_failed(self, mock_log, mock_detect): + mock_detect.side_effect = RuntimeError + + with task_manager.acquire(self.context, self.node.id, + shared=True) as task: + conductor_utils.node_cache_vendor(task) + self.assertTrue(task.shared) + mock_detect.assert_called_once_with(task.driver.management, task) + + self.node.refresh() + self.assertNotIn('vendor', self.node.properties) + self.assertTrue(mock_log.called) diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py index a1fb4e8d0..dc67e17d6 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_management.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_management.py @@ -702,6 +702,14 @@ class RedfishManagementTestCase(db_base.DbTestCase): self.assertEqual(indicator_states.ON, state) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_detect_vendor(self, mock_get_system): + mock_get_system.return_value.manufacturer = "Fake GmbH" + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + response = task.driver.management.detect_vendor(task) + self.assertEqual("Fake GmbH", response) + @mock.patch.object(manager_utils, 'node_power_action', autospec=True) @mock.patch.object(deploy_utils, 'get_async_step_return_state', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index f4d54e74c..7a172ec2d 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py @@ -1541,16 +1541,14 @@ class IPMIToolPrivateMethodTestCase( self.assertEqual(expected, mock_exec.call_args_list) - @mock.patch.object(ipmi.IPMIManagement, 'detect_vendor', autospec=True) @mock.patch.object(ipmi, '_exec_ipmitool', autospec=True) @mock.patch('oslo_utils.eventletutils.EventletEvent.wait', autospec=True) - def test__soft_power_off(self, sleep_mock, mock_exec, mock_vendor): + def test__soft_power_off(self, sleep_mock, mock_exec): def side_effect(driver_info, command, **kwargs): resp_dict = {"power status": ["Chassis Power is off\n", None], "power soft": [None, None]} return resp_dict.get(command, ["Bad\n", None]) - mock_vendor.return_value = None mock_exec.side_effect = side_effect expected = [mock.call(self.info, "power soft"), @@ -1561,13 +1559,10 @@ class IPMIToolPrivateMethodTestCase( self.assertEqual(expected, mock_exec.call_args_list) self.assertEqual(states.POWER_OFF, state) - self.assertTrue(mock_vendor.called) - @mock.patch.object(ipmi.IPMIManagement, 'detect_vendor', autospec=True) @mock.patch.object(ipmi, '_exec_ipmitool', autospec=True) @mock.patch('oslo_utils.eventletutils.EventletEvent.wait', autospec=True) - def test__soft_power_off_max_retries(self, sleep_mock, mock_exec, - mock_vendor): + def test__soft_power_off_max_retries(self, sleep_mock, mock_exec): def side_effect(driver_info, command, **kwargs): resp_dict = {"power status": ["Chassis Power is on\n", None], @@ -1586,8 +1581,6 @@ class IPMIToolPrivateMethodTestCase( ipmi._soft_power_off, task, self.info, timeout=2) self.assertEqual(expected, mock_exec.call_args_list) - # Should be removed when detect_vendor automatic invocation is moved. - self.assertFalse(mock_vendor.called) @mock.patch.object(ipmi, '_power_status', autospec=True) @mock.patch.object(ipmi, '_exec_ipmitool', autospec=True) @@ -1661,9 +1654,8 @@ class IPMIToolDriverTestCase(Base): self.assertEqual(sorted(expected), sorted(task.driver.get_properties())) - @mock.patch.object(ipmi.IPMIManagement, 'detect_vendor', autospec=True) @mock.patch.object(ipmi, '_exec_ipmitool', autospec=True) - def test_get_power_state(self, mock_exec, mock_detect): + def test_get_power_state(self, mock_exec): returns = iter([["Chassis Power is off\n", None], ["Chassis Power is on\n", None], ["\n", None]]) @@ -1671,7 +1663,6 @@ class IPMIToolDriverTestCase(Base): mock.call(self.info, "power status", kill_on_timeout=True), mock.call(self.info, "power status", kill_on_timeout=True)] mock_exec.side_effect = returns - mock_detect.return_value = None with task_manager.acquire(self.context, self.node.uuid) as task: pstate = self.power.get_power_state(task) @@ -1684,20 +1675,16 @@ class IPMIToolDriverTestCase(Base): self.assertEqual(states.ERROR, pstate) self.assertEqual(mock_exec.call_args_list, expected) - self.assertEqual(3, mock_detect.call_count) - @mock.patch.object(ipmi.IPMIManagement, 'detect_vendor', autospec=True) @mock.patch.object(ipmi, '_exec_ipmitool', autospec=True) - def test_get_power_state_exception(self, mock_exec, mock_vendor): + def test_get_power_state_exception(self, mock_exec): mock_exec.side_effect = processutils.ProcessExecutionError("error") - mock_vendor.return_value = None with task_manager.acquire(self.context, self.node.uuid) as task: self.assertRaises(exception.IPMIFailure, self.power.get_power_state, task) mock_exec.assert_called_once_with(self.info, "power status", kill_on_timeout=True) - self.assertEqual(1, mock_vendor.call_count) @mock.patch.object(ipmi, '_power_on', autospec=True) @mock.patch.object(ipmi, '_power_off', autospec=True) |