diff --git a/ironic_python_agent/inspector.py b/ironic_python_agent/inspector.py index 681484fb2..ec83a8794 100644 --- a/ironic_python_agent/inspector.py +++ b/ironic_python_agent/inspector.py @@ -98,9 +98,6 @@ def inspect(): LOG.info('stopping inspection, as inspector returned an error') return - # Optionally update IPMI credentials - setup_ipmi_credentials(resp) - LOG.info('inspection finished successfully') return resp.get('uuid') @@ -127,36 +124,6 @@ def call_inspector(data, failures): return resp.json() -def setup_ipmi_credentials(resp): - """Setup IPMI credentials, if requested. - - :param resp: JSON response from inspector. - """ - if not resp.get('ipmi_setup_credentials'): - LOG.info('setting IPMI credentials was not requested') - return - - user, password = resp['ipmi_username'], resp['ipmi_password'] - LOG.debug('setting IPMI credentials: user %s', user) - - commands = [ - ('user', 'set', 'name', '2', user), - ('user', 'set', 'password', '2', password), - ('user', 'enable', '2'), - ('channel', 'setaccess', '1', '2', - 'link=on', 'ipmi=on', 'callin=on', 'privilege=4'), - ] - - for cmd in commands: - try: - utils.execute('ipmitool', *cmd) - except processutils.ProcessExecutionError: - LOG.exception('failed to update IPMI credentials') - raise errors.InspectionError('failed to update IPMI credentials') - - LOG.info('successfully set IPMI credentials: user %s', user) - - def _normalize_mac(mac): """Convert MAC to a well-known format aa:bb:cc:dd:ee:ff.""" if '-' in mac: @@ -237,12 +204,11 @@ def collect_default(data, failures): else: data['root_disk'] = root_disk LOG.debug('default root device is %s', root_disk.name) - # Both boot interface and IPMI address might not be present, - # we don't count it as failure + # The boot interface might not be present, we don't count it as failure. + # TODO(dtantsur): stop using the boot_interface field. data['boot_interface'] = inventory['boot'].pxe_interface LOG.debug('boot devices was %s', data['boot_interface']) - data['ipmi_address'] = inventory.get('bmc_address') - LOG.debug('BMC IP address: %s', data['ipmi_address']) + LOG.debug('BMC IP address: %s', inventory.get('bmc_address')) def collect_logs(data, failures): diff --git a/ironic_python_agent/tests/unit/test_inspector.py b/ironic_python_agent/tests/unit/test_inspector.py index a6c3ac5f6..edd204511 100644 --- a/ironic_python_agent/tests/unit/test_inspector.py +++ b/ironic_python_agent/tests/unit/test_inspector.py @@ -59,7 +59,6 @@ class TestMisc(base.IronicAgentTest): ['foobar']) -@mock.patch.object(inspector, 'setup_ipmi_credentials', autospec=True) @mock.patch.object(inspector, 'call_inspector', new_callable=AcceptingFailure) @mock.patch.object(stevedore, 'NamedExtensionManager', autospec=True) class TestInspect(base.IronicAgentTest): @@ -71,7 +70,7 @@ class TestInspect(base.IronicAgentTest): self.mock_ext = mock.Mock(spec=['plugin', 'name'], plugin=self.mock_collect) - def test_ok(self, mock_ext_mgr, mock_call, mock_setup_ipmi): + def test_ok(self, mock_ext_mgr, mock_call): mock_ext_mgr.return_value = [self.mock_ext] mock_call.return_value = {'uuid': 'uuid1'} @@ -80,9 +79,8 @@ class TestInspect(base.IronicAgentTest): self.mock_collect.assert_called_with_failure() mock_call.assert_called_with_failure() self.assertEqual('uuid1', result) - mock_setup_ipmi.assert_called_once_with(mock_call.return_value) - def test_collectors_option(self, mock_ext_mgr, mock_call, mock_setup_ipmi): + def test_collectors_option(self, mock_ext_mgr, mock_call): CONF.set_override('inspection_collectors', 'foo,bar') mock_ext_mgr.return_value = [ mock.Mock(spec=['name', 'plugin'], plugin=AcceptingFailure()), @@ -95,7 +93,7 @@ class TestInspect(base.IronicAgentTest): fake_ext.plugin.assert_called_with_failure() mock_call.assert_called_with_failure() - def test_collector_failed(self, mock_ext_mgr, mock_call, mock_setup_ipmi): + def test_collector_failed(self, mock_ext_mgr, mock_call): mock_ext_mgr.return_value = [self.mock_ext] self.mock_collect.side_effect = RuntimeError('boom') @@ -104,18 +102,16 @@ class TestInspect(base.IronicAgentTest): self.mock_collect.assert_called_with_failure() mock_call.assert_called_with_failure(expect_error=True) - self.assertFalse(mock_setup_ipmi.called) - def test_extensions_failed(self, mock_ext_mgr, mock_call, mock_setup_ipmi): + def test_extensions_failed(self, mock_ext_mgr, mock_call): CONF.set_override('inspection_collectors', 'foo,bar') mock_ext_mgr.side_effect = RuntimeError('boom') self.assertRaisesRegex(RuntimeError, 'boom', inspector.inspect) mock_call.assert_called_with_failure(expect_error=True) - self.assertFalse(mock_setup_ipmi.called) - def test_inspector_error(self, mock_ext_mgr, mock_call, mock_setup_ipmi): + def test_inspector_error(self, mock_ext_mgr, mock_call): mock_call.return_value = None mock_ext_mgr.return_value = [self.mock_ext] @@ -124,7 +120,6 @@ class TestInspect(base.IronicAgentTest): self.mock_collect.assert_called_with_failure() mock_call.assert_called_with_failure() self.assertIsNone(result) - self.assertFalse(mock_setup_ipmi.called) @mock.patch.object(requests, 'post', autospec=True) @@ -171,58 +166,6 @@ class TestCallInspector(base.IronicAgentTest): self.assertIsNone(res) -@mock.patch.object(utils, 'execute', autospec=True) -class TestSetupIpmiCredentials(base.IronicAgentTest): - def setUp(self): - super(TestSetupIpmiCredentials, self).setUp() - self.resp = {'ipmi_username': 'user', - 'ipmi_password': 'pwd', - 'ipmi_setup_credentials': True} - - def test_disabled(self, mock_call): - del self.resp['ipmi_setup_credentials'] - - inspector.setup_ipmi_credentials(self.resp) - - self.assertFalse(mock_call.called) - - def test_ok(self, mock_call): - inspector.setup_ipmi_credentials(self.resp) - - expected = [ - mock.call('ipmitool', 'user', 'set', 'name', '2', 'user'), - mock.call('ipmitool', 'user', 'set', 'password', '2', 'pwd'), - mock.call('ipmitool', 'user', 'enable', '2'), - mock.call('ipmitool', 'channel', 'setaccess', '1', '2', - 'link=on', 'ipmi=on', 'callin=on', 'privilege=4'), - ] - self.assertEqual(expected, mock_call.call_args_list) - - def test_user_failed(self, mock_call): - mock_call.side_effect = processutils.ProcessExecutionError() - - self.assertRaises(errors.InspectionError, - inspector.setup_ipmi_credentials, - self.resp) - - mock_call.assert_called_once_with('ipmitool', 'user', 'set', 'name', - '2', 'user') - - def test_password_failed(self, mock_call): - mock_call.side_effect = iter((None, - processutils.ProcessExecutionError)) - - self.assertRaises(errors.InspectionError, - inspector.setup_ipmi_credentials, - self.resp) - - expected = [ - mock.call('ipmitool', 'user', 'set', 'name', '2', 'user'), - mock.call('ipmitool', 'user', 'set', 'password', '2', 'pwd') - ] - self.assertEqual(expected, mock_call.call_args_list) - - class BaseDiscoverTest(base.IronicAgentTest): def setUp(self): super(BaseDiscoverTest, self).setUp() @@ -269,10 +212,9 @@ class TestCollectDefault(BaseDiscoverTest): inspector.collect_default(self.data, self.failures) - for key in ('memory', 'interfaces', 'cpu', 'disks'): + for key in ('memory', 'interfaces', 'cpu', 'disks', 'bmc_address'): self.assertTrue(self.data['inventory'][key]) - self.assertEqual('1.2.3.4', self.data['ipmi_address']) self.assertEqual('boot:if', self.data['boot_interface']) self.assertEqual(self.inventory['disks'][0].name, self.data['root_disk'].name) @@ -286,10 +228,9 @@ class TestCollectDefault(BaseDiscoverTest): inspector.collect_default(self.data, self.failures) - for key in ('memory', 'interfaces', 'cpu'): + for key in ('memory', 'interfaces', 'cpu', 'bmc_address'): self.assertTrue(self.data['inventory'][key]) - self.assertEqual('1.2.3.4', self.data['ipmi_address']) self.assertEqual('boot:if', self.data['boot_interface']) self.assertNotIn('root_disk', self.data) diff --git a/releasenotes/notes/ipmi-cleanup-a4454f6851d81c4d.yaml b/releasenotes/notes/ipmi-cleanup-a4454f6851d81c4d.yaml new file mode 100644 index 000000000..c4a493df3 --- /dev/null +++ b/releasenotes/notes/ipmi-cleanup-a4454f6851d81c4d.yaml @@ -0,0 +1,8 @@ +--- +upgrade: + - | + Support for setting IPMI credentials during inspection was removed. This + feature is not supported by ironic-inspector since the Pike release. + - | + The ``ipmi_address`` field is no longer sent as part of the inspection + process. The ``inventory[bmc_address]`` field should be used instead.