diff --git a/ironic/conf/xclarity.py b/ironic/conf/xclarity.py index a595126ba5..2bca905a7d 100644 --- a/ironic/conf/xclarity.py +++ b/ironic/conf/xclarity.py @@ -18,14 +18,24 @@ from ironic.common.i18n import _ opts = [ cfg.StrOpt('manager_ip', - help=_('IP address of XClarity controller.')), + help=_('IP address of the XClarity Controller. ' + 'Configuration here is deprecated and will be removed ' + 'in the Stein release. Please update the driver_info ' + 'field to use "xclarity_manager_ip" instead')), cfg.StrOpt('username', - help=_('Username to access the XClarity controller.')), + help=_('Username for the XClarity Controller. ' + 'Configuration here is deprecated and will be removed ' + 'in the Stein release. Please update the driver_info ' + 'field to use "xclarity_username" instead')), cfg.StrOpt('password', - help=_('Password for XClarity controller username.')), + help=_('Password for XClarity Controller username. ' + 'Configuration here is deprecated and will be removed ' + 'in the Stein release. Please update the driver_info ' + 'field to use "xclarity_password" instead')), cfg.PortOpt('port', default=443, - help=_('Port to be used for XClarity operations.')), + help=_('Port to be used for XClarity Controller ' + 'connection.')), ] diff --git a/ironic/drivers/modules/xclarity/common.py b/ironic/drivers/modules/xclarity/common.py index 1ffdfe248c..d216195148 100644 --- a/ironic/drivers/modules/xclarity/common.py +++ b/ironic/drivers/modules/xclarity/common.py @@ -18,6 +18,7 @@ from oslo_utils import importutils from ironic.common import exception from ironic.common.i18n import _ from ironic.common import states +from ironic.common import utils from ironic.conf import CONF LOG = logging.getLogger(__name__) @@ -28,45 +29,102 @@ xclarity_client_exceptions = importutils.try_import( 'xclarity_client.exceptions') REQUIRED_ON_DRIVER_INFO = { - 'xclarity_hardware_id': _("XClarity Server Hardware ID. " - "Required in driver_info."), -} - -COMMON_PROPERTIES = { - 'xclarity_address': _("IP address of the XClarity node."), - 'xclarity_username': _("Username for the XClarity with administrator " - "privileges."), + 'xclarity_manager_ip': _("IP address of the XClarity Controller."), + 'xclarity_username': _("Username for the XClarity Controller " + "with administrator privileges."), 'xclarity_password': _("Password for xclarity_username."), - 'xclarity_port': _("Port to be used for xclarity_username."), + 'xclarity_hardware_id': _("Server Hardware ID managed by XClarity."), } +OPTIONAL_ON_DRIVER_INFO = { + 'xclarity_port': _("Port to be used for XClarity Controller connection. " + "Optional"), +} + +COMMON_PROPERTIES = {} COMMON_PROPERTIES.update(REQUIRED_ON_DRIVER_INFO) +COMMON_PROPERTIES.update(OPTIONAL_ON_DRIVER_INFO) def get_properties(): return COMMON_PROPERTIES -def get_xclarity_client(): +def parse_driver_info(node): + """Parse a node's driver_info values. + + Parses the driver_info of the node, reads default values + and returns a dict containing the combination of both. + + :param node: an ironic node object to get informatin from. + :returns: a dict containing information parsed from driver_info. + :raises: InvalidParameterValue if some required information + is missing on the node or inputs is invalid. + """ + driver_info = node.driver_info + parsed_driver_info = {} + + error_msgs = [] + for param in REQUIRED_ON_DRIVER_INFO: + if param == "xclarity_hardware_id": + try: + parsed_driver_info[param] = str(driver_info[param]) + except KeyError: + error_msgs.append(_("'%s' not provided to XClarity.") % param) + except UnicodeEncodeError: + error_msgs.append(_("'%s' contains non-ASCII symbol.") % param) + else: + # corresponding config names don't have 'xclarity_' prefix + if param in driver_info: + parsed_driver_info[param] = str(driver_info[param]) + elif param not in driver_info and\ + CONF.xclarity.get(param[len('xclarity_'):]) is not None: + parsed_driver_info[param] = str( + CONF.xclarity.get(param[len('xclarity_'):])) + LOG.warning('The configuration [xclarity]/%(config)s ' + 'is deprecated and will be removed in the ' + 'Stein release. Please update the node ' + '%(node_uuid)s driver_info field to use ' + '"%(field)s" instead', + {'config': param[len('xclarity_'):], + 'node_uuid': node.uuid, 'field': param}) + else: + error_msgs.append(_("'%s' not provided to XClarity.") % param) + + port = driver_info.get('xclarity_port', CONF.xclarity.get('port')) + parsed_driver_info['xclarity_port'] = utils.validate_network_port( + port, 'xclarity_port') + + if error_msgs: + msg = (_('The following errors were encountered while parsing ' + 'driver_info:\n%s') % '\n'.join(error_msgs)) + raise exception.InvalidParameterValue(msg) + + return parsed_driver_info + + +def get_xclarity_client(node): """Generates an instance of the XClarity client. Generates an instance of the XClarity client using the imported xclarity_client library. + :param node: an ironic node object. :returns: an instance of the XClarity client :raises: XClarityError if can't get to the XClarity client """ + driver_info = parse_driver_info(node) try: xclarity_client = client.Client( - ip=CONF.xclarity.manager_ip, - username=CONF.xclarity.username, - password=CONF.xclarity.password, - port=CONF.xclarity.port + ip=driver_info.get('xclarity_manager_ip'), + username=driver_info.get('xclarity_username'), + password=driver_info.get('xclarity_password'), + port=driver_info.get('xclarity_port') ) except xclarity_client_exceptions.XClarityError as exc: - msg = (_("Error getting connection to XClarity manager IP: %(ip)s. " - "Error: %(exc)s"), {'ip': CONF.xclarity.manager_ip, - 'exc': exc}) + msg = (_("Error getting connection to XClarity address: %(ip)s. " + "Error: %(exc)s"), + {'ip': driver_info.get('xclarity_manager_ip'), 'exc': exc}) raise exception.XClarityError(error=msg) return xclarity_client @@ -118,17 +176,3 @@ def translate_xclarity_power_action(power_action): } return power_action_map[power_action] - - -def is_node_managed_by_xclarity(xclarity_client, node): - """Determines whether dynamic allocation is enabled for a specifc node. - - :param: xclarity_client: an instance of the XClarity client - :param: node: node object to get information from - :returns: Boolean depending on whether node is managed by XClarity - """ - try: - hardware_id = get_server_hardware_id(node) - return xclarity_client.is_node_managed(hardware_id) - except exception.MissingParameterValue: - return False diff --git a/ironic/drivers/modules/xclarity/management.py b/ironic/drivers/modules/xclarity/management.py index 2ae03f0be5..7a3b704259 100644 --- a/ironic/drivers/modules/xclarity/management.py +++ b/ironic/drivers/modules/xclarity/management.py @@ -47,20 +47,21 @@ SUPPORTED_BOOT_DEVICES = [ class XClarityManagement(base.ManagementInterface): - def __init__(self): - super(XClarityManagement, self).__init__() - self.xclarity_client = common.get_xclarity_client() def get_properties(self): return common.COMMON_PROPERTIES @METRICS.timer('XClarityManagement.validate') def validate(self, task): - """It validates if the node is being used by XClarity. + """Validate the driver-specific info supplied. + + This method validates if the 'driver_info' property of the supplied + task's node contains the required information for this driver to + manage the node. :param task: a task from TaskManager. """ - common.is_node_managed_by_xclarity(self.xclarity_client, task.node) + common.parse_driver_info(task.node) @METRICS.timer('XClarityManagement.get_supported_boot_devices') def get_supported_boot_devices(self, task): @@ -97,16 +98,18 @@ class XClarityManagement(base.ManagementInterface): :raises: InvalidParameterValue if the boot device is unknown :raises: XClarityError if the communication with XClarity fails """ - server_hardware_id = common.get_server_hardware_id(task.node) + node = task.node + client = common.get_xclarity_client(node) + server_hardware_id = common.get_server_hardware_id(node) try: boot_info = ( - self.xclarity_client.get_node_all_boot_info( + client.get_node_all_boot_info( server_hardware_id) ) except xclarity_client_exceptions.XClarityError as xclarity_exc: LOG.error( "Error getting boot device from XClarity for node %(node)s. " - "Error: %(error)s", {'node': task.node.uuid, + "Error: %(error)s", {'node': node.uuid, 'error': xclarity_exc}) raise exception.XClarityError(error=xclarity_exc) @@ -182,7 +185,9 @@ class XClarityManagement(base.ManagementInterface): :param task: a TaskManager instance. :param singleuse: if this device will be used only once at next boot """ - boot_info = self.xclarity_client.get_node_all_boot_info( + node = task.node + client = common.get_xclarity_client(node) + boot_info = client.get_node_all_boot_info( server_hardware_id) xclarity_boot_device = self._translate_ironic_to_xclarity( new_primary_boot_device) @@ -190,7 +195,7 @@ class XClarityManagement(base.ManagementInterface): LOG.debug( ("Setting boot device to %(device)s for XClarity " "node %(node)s"), - {'device': xclarity_boot_device, 'node': task.node.uuid} + {'device': xclarity_boot_device, 'node': node.uuid} ) for item in boot_info['bootOrder']['bootOrderList']: if singleuse and item['bootType'] == 'SingleUse': @@ -205,15 +210,15 @@ class XClarityManagement(base.ManagementInterface): item['currentBootOrderDevices'] = current try: - self.xclarity_client.set_node_boot_info(server_hardware_id, - boot_info, - xclarity_boot_device, - singleuse) + client.set_node_boot_info(server_hardware_id, + boot_info, + xclarity_boot_device, + singleuse) except xclarity_client_exceptions.XClarityError as xclarity_exc: LOG.error( ('Error setting boot device %(boot_device)s for the XClarity ' 'node %(node)s. Error: %(error)s'), - {'boot_device': xclarity_boot_device, 'node': task.node.uuid, + {'boot_device': xclarity_boot_device, 'node': node.uuid, 'error': xclarity_exc} ) raise exception.XClarityError(error=xclarity_exc) diff --git a/ironic/drivers/modules/xclarity/power.py b/ironic/drivers/modules/xclarity/power.py index eaaea67461..9eb6b110fc 100644 --- a/ironic/drivers/modules/xclarity/power.py +++ b/ironic/drivers/modules/xclarity/power.py @@ -31,21 +31,21 @@ xclarity_client_exceptions = importutils.try_import( class XClarityPower(base.PowerInterface): - def __init__(self): - super(XClarityPower, self).__init__() - self.xclarity_client = common.get_xclarity_client() def get_properties(self): return common.get_properties() @METRICS.timer('XClarityPower.validate') def validate(self, task): - """It validates if the node is being used by XClarity. + """Validate the driver-specific info supplied. + + This method validates if the 'driver_info' property of the supplied + task's node contains the required information for this driver to + manage the power state of the node. :param task: a task from TaskManager. """ - - common.is_node_managed_by_xclarity(self.xclarity_client, task.node) + common.parse_driver_info(task.node) @METRICS.timer('XClarityPower.get_power_state') def get_power_state(self, task): @@ -57,15 +57,16 @@ class XClarityPower(base.PowerInterface): :raises: XClarityError if fails to retrieve power state of XClarity resource """ - server_hardware_id = common.get_server_hardware_id(task.node) + node = task.node + client = common.get_xclarity_client(node) + server_hardware_id = common.get_server_hardware_id(node) try: - power_state = self.xclarity_client.get_node_power_status( - server_hardware_id) + power_state = client.get_node_power_status(server_hardware_id) except xclarity_client_exceptions.XClarityException as xclarity_exc: LOG.error( ("Error getting power state for node %(node)s. Error: " "%(error)s"), - {'node': task.node.uuid, 'error': xclarity_exc} + {'node': node.uuid, 'error': xclarity_exc} ) raise exception.XClarityError(error=xclarity_exc) return common.translate_xclarity_power_state(power_state) @@ -95,14 +96,15 @@ class XClarityPower(base.PowerInterface): if target_power_state == states.POWER_OFF: power_state = states.POWER_ON - server_hardware_id = common.get_server_hardware_id(task.node) + node = task.node + client = common.get_xclarity_client(node) + server_hardware_id = common.get_server_hardware_id(node) LOG.debug("Setting power state of node %(node_uuid)s to " "%(power_state)s", - {'node_uuid': task.node.uuid, 'power_state': power_state}) + {'node_uuid': node.uuid, 'power_state': power_state}) try: - self.xclarity_client.set_node_power_status(server_hardware_id, - power_state) + client.set_node_power_status(server_hardware_id, power_state) except xclarity_client_exceptions.XClarityError as xclarity_exc: LOG.error( "Error setting power state of node %(node_uuid)s to " diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py index 5aea3fccd8..ca0eaab60b 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -506,6 +506,10 @@ def get_test_xclarity_properties(): def get_test_xclarity_driver_info(): return { + 'xclarity_manager_ip': "1.2.3.4", + 'xclarity_username': "USERID", + 'xclarity_password': "fake", + 'xclarity_port': 443, 'xclarity_hardware_id': 'fake_sh_id', } diff --git a/ironic/tests/unit/drivers/modules/xclarity/test_common.py b/ironic/tests/unit/drivers/modules/xclarity/test_common.py index 0b2c1bf682..a7253bf8dc 100644 --- a/ironic/tests/unit/drivers/modules/xclarity/test_common.py +++ b/ironic/tests/unit/drivers/modules/xclarity/test_common.py @@ -16,29 +16,91 @@ import mock from oslo_utils import importutils +from ironic.common import exception from ironic.drivers.modules.xclarity import common from ironic.tests.unit.db import base as db_base from ironic.tests.unit.db import utils as db_utils from ironic.tests.unit.objects import utils as obj_utils +xclarity_client = importutils.try_import('xclarity_client.client') xclarity_exceptions = importutils.try_import('xclarity_client.exceptions') xclarity_constants = importutils.try_import('xclarity_client.constants') +INFO_DICT = db_utils.get_test_xclarity_driver_info() + class XClarityCommonTestCase(db_base.DbTestCase): def setUp(self): super(XClarityCommonTestCase, self).setUp() + self.config(enabled_hardware_types=['xclarity'], + enabled_power_interfaces=['xclarity'], + enabled_management_interfaces=['xclarity']) + self.node = obj_utils.create_test_node( + self.context, driver='xclarity', + properties=db_utils.get_test_xclarity_properties(), + driver_info=INFO_DICT) - self.config(manager_ip='1.2.3.4', group='xclarity') + def test_parse_driver_info(self): + info = common.parse_driver_info(self.node) + self.assertEqual(INFO_DICT['xclarity_manager_ip'], + info['xclarity_manager_ip']) + self.assertEqual(INFO_DICT['xclarity_username'], + info['xclarity_username']) + self.assertEqual(INFO_DICT['xclarity_password'], + info['xclarity_password']) + self.assertEqual(INFO_DICT['xclarity_port'], info['xclarity_port']) + self.assertEqual(INFO_DICT['xclarity_hardware_id'], + info['xclarity_hardware_id']) + + def test_parse_driver_info_missing_hardware_id(self): + del self.node.driver_info['xclarity_hardware_id'] + self.assertRaises(exception.InvalidParameterValue, + common.parse_driver_info, self.node) + + def test_parse_driver_info_get_param_from_config(self): + del self.node.driver_info['xclarity_manager_ip'] + del self.node.driver_info['xclarity_username'] + del self.node.driver_info['xclarity_password'] + self.config(manager_ip='5.6.7.8', group='xclarity') self.config(username='user', group='xclarity') self.config(password='password', group='xclarity') + info = common.parse_driver_info(self.node) + self.assertEqual('5.6.7.8', info['xclarity_manager_ip']) + self.assertEqual('user', info['xclarity_username']) + self.assertEqual('password', info['xclarity_password']) - self.node = obj_utils.create_test_node( - self.context, driver='fake-xclarity', - properties=db_utils.get_test_xclarity_properties(), - driver_info=db_utils.get_test_xclarity_driver_info(), - ) + def test_parse_driver_info_missing_driver_info_and_config(self): + del self.node.driver_info['xclarity_manager_ip'] + del self.node.driver_info['xclarity_username'] + del self.node.driver_info['xclarity_password'] + e = self.assertRaises(exception.InvalidParameterValue, + common.parse_driver_info, self.node) + self.assertIn('xclarity_manager_ip', str(e)) + self.assertIn('xclarity_username', str(e)) + self.assertIn('xclarity_password', str(e)) + + def test_parse_driver_info_invalid_port(self): + self.node.driver_info['xclarity_port'] = 'asd' + self.assertRaises(exception.InvalidParameterValue, + common.parse_driver_info, self.node) + self.node.driver_info['xclarity_port'] = '65536' + self.assertRaises(exception.InvalidParameterValue, + common.parse_driver_info, self.node) + self.node.driver_info['xclarity_port'] = 'invalid' + self.assertRaises(exception.InvalidParameterValue, + common.parse_driver_info, self.node) + self.node.driver_info['xclarity_port'] = '-1' + self.assertRaises(exception.InvalidParameterValue, + common.parse_driver_info, self.node) + + @mock.patch.object(xclarity_client, 'Client', autospec=True) + def test_get_xclarity_client(self, mock_xclarityclient): + expected_call = mock.call(ip='1.2.3.4', password='fake', port=443, + username='USERID') + common.get_xclarity_client(self.node) + + self.assertEqual(mock_xclarityclient.mock_calls, [expected_call]) def test_get_server_hardware_id(self): driver_info = self.node.driver_info @@ -46,19 +108,3 @@ class XClarityCommonTestCase(db_base.DbTestCase): self.node.driver_info = driver_info result = common.get_server_hardware_id(self.node) self.assertEqual(result, 'test') - - @mock.patch.object(common, 'get_server_hardware_id', - spec_set=True, autospec=True) - @mock.patch.object(common, 'get_xclarity_client', - spec_set=True, autospec=True) - def test_check_node_managed_by_xclarity(self, mock_xc_client, - mock_validate_driver_info): - driver_info = self.node.driver_info - driver_info['xclarity_hardware_id'] = 'abcd' - self.node.driver_info = driver_info - - xclarity_client = mock_xc_client() - mock_validate_driver_info.return_value = '12345' - common.is_node_managed_by_xclarity(xclarity_client, - self.node) - xclarity_client.is_node_managed.assert_called_once_with('12345') diff --git a/ironic/tests/unit/drivers/modules/xclarity/test_management.py b/ironic/tests/unit/drivers/modules/xclarity/test_management.py index 8563ebad2f..36210fe382 100644 --- a/ironic/tests/unit/drivers/modules/xclarity/test_management.py +++ b/ironic/tests/unit/drivers/modules/xclarity/test_management.py @@ -59,10 +59,9 @@ class XClarityManagementDriverTestCase(db_base.DbTestCase): mock_validate.assert_called_with(task.node) def test_get_properties(self, mock_get_xc_client): - - expected = common.REQUIRED_ON_DRIVER_INFO - self.assertItemsEqual(expected, - self.node.driver_info) + expected = common.COMMON_PROPERTIES + driver = management.XClarityManagement() + self.assertEqual(expected, driver.get_properties()) @mock.patch.object(management.XClarityManagement, 'get_boot_device', return_value='pxe') diff --git a/ironic/tests/unit/drivers/modules/xclarity/test_power.py b/ironic/tests/unit/drivers/modules/xclarity/test_power.py index c459e34274..dcca3353d9 100644 --- a/ironic/tests/unit/drivers/modules/xclarity/test_power.py +++ b/ironic/tests/unit/drivers/modules/xclarity/test_power.py @@ -56,9 +56,9 @@ class XClarityPowerDriverTestCase(db_base.DbTestCase): driver_info=db_utils.get_test_xclarity_driver_info()) def test_get_properties(self, mock_get_xc_client): - expected = common.REQUIRED_ON_DRIVER_INFO - self.assertItemsEqual(expected, - self.node.driver_info) + expected = common.COMMON_PROPERTIES + driver = power.XClarityPower() + self.assertEqual(expected, driver.get_properties()) @mock.patch.object(common, 'get_server_hardware_id', spect_set=True, autospec=True) diff --git a/releasenotes/notes/deprecate-xclarity-config-af9b753f96779f42.yaml b/releasenotes/notes/deprecate-xclarity-config-af9b753f96779f42.yaml new file mode 100644 index 0000000000..d9d21e9cbb --- /dev/null +++ b/releasenotes/notes/deprecate-xclarity-config-af9b753f96779f42.yaml @@ -0,0 +1,19 @@ +--- +features: + - | + Adds new parameter fields to driver_info, which will become + mandatory in Stein release: + + * ``xclarity_manager_ip``: IP address of the XClarity Controller. + * ``xclarity_username``: Username for the XClarity Controller. + * ``xclarity_password``: Password for XClarity Controller username. + * ``xclarity_port``: Port to be used for XClarity Controller connection. +deprecations: + - | + Configuration options ``[xclarity]/manager_ip``, ``[xclarity]/username``, + and ``[xclarity]/password`` are deprecated and will be removed in + the Stein release. +fixes: + - | + Fixes an issue where parameters required in driver_info and + descriptions in documentation are different.