Merge "Fix XClarity parameters discrepancy"

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.')),
 ]
 
 

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. "
+    'xclarity_manager_ip': _("IP address of the XClarity Controller."),
-                              "Required in driver_info."),
+    'xclarity_username': _("Username for the XClarity Controller "
-}
+                           "with administrator privileges."),
-
-COMMON_PROPERTIES = {
-    'xclarity_address': _("IP address of the XClarity node."),
-    'xclarity_username': _("Username for the XClarity 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,
+            ip=driver_info.get('xclarity_manager_ip'),
-            username=CONF.xclarity.username,
+            username=driver_info.get('xclarity_username'),
-            password=CONF.xclarity.password,
+            password=driver_info.get('xclarity_password'),
-            port=CONF.xclarity.port
+            port=driver_info.get('xclarity_port')
         )
     except xclarity_client_exceptions.XClarityError as exc:
-        msg = (_("Error getting connection to XClarity manager IP: %(ip)s. "
+        msg = (_("Error getting connection to XClarity address: %(ip)s. "
-                 "Error: %(exc)s"), {'ip': CONF.xclarity.manager_ip,
+                 "Error: %(exc)s"),
-                                     'exc': exc})
+               {'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

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,
+            client.set_node_boot_info(server_hardware_id,
-                                                    boot_info,
+                                      boot_info,
-                                                    xclarity_boot_device,
+                                      xclarity_boot_device,
-                                                    singleuse)
+                                      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)

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.parse_driver_info(task.node)
-        common.is_node_managed_by_xclarity(self.xclarity_client, 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(
+            power_state = client.get_node_power_status(server_hardware_id)
-                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,
+            client.set_node_power_status(server_hardware_id, power_state)
-                                                       power_state)
         except xclarity_client_exceptions.XClarityError as xclarity_exc:
             LOG.error(
                 "Error setting power state of node %(node_uuid)s to "

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',
     }
 

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(
+    def test_parse_driver_info_missing_driver_info_and_config(self):
-            self.context, driver='fake-xclarity',
+        del self.node.driver_info['xclarity_manager_ip']
-            properties=db_utils.get_test_xclarity_properties(),
+        del self.node.driver_info['xclarity_username']
-            driver_info=db_utils.get_test_xclarity_driver_info(),
+        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')

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.COMMON_PROPERTIES
-        expected = common.REQUIRED_ON_DRIVER_INFO
+        driver = management.XClarityManagement()
-        self.assertItemsEqual(expected,
+        self.assertEqual(expected, driver.get_properties())
-                              self.node.driver_info)
 
     @mock.patch.object(management.XClarityManagement, 'get_boot_device',
                        return_value='pxe')

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
+        expected = common.COMMON_PROPERTIES
-        self.assertItemsEqual(expected,
+        driver = power.XClarityPower()
-                              self.node.driver_info)
+        self.assertEqual(expected, driver.get_properties())
 
     @mock.patch.object(common, 'get_server_hardware_id',
                        spect_set=True, autospec=True)

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.