From 51a73e11c2f6bad0302cefcd8380f6ccbc757907 Mon Sep 17 00:00:00 2001 From: Imre Farkas Date: Mon, 30 Nov 2015 11:49:43 +0100 Subject: [PATCH] DRAC: switch to python-dracclient on vendor-passthru DRAC specific code from Ironic is moving to its own project, to python-dracclient project. This patch continues refactoring code in Ironic to use the new library. Note that this is a backwards incompatible change. In addition to the dependency change, return values of the following methods changed in ironic.drivers.modules.drac.vendor_passthru: - set_bios_config returns the 'commit_required' key - commit_bios_config returns the 'job_id' and 'reboot_required' keys - abandon_bios_config returns nothing Change-Id: Iaa4cee0dae8980817803ca55cdb53789b4051bc2 Partial-Bug: #1454492 Depends-On: I574696e460f08cececc47a69b5c2b090a6cd44a2 --- ironic/drivers/fake.py | 2 + ironic/drivers/modules/drac/bios.py | 391 +++--------------- ironic/drivers/modules/drac/management.py | 89 ---- .../drivers/modules/drac/vendor_passthru.py | 87 ++-- .../unit/drivers/modules/drac/test_bios.py | 258 +++++------- .../drivers/modules/drac/test_management.py | 121 ------ 6 files changed, 211 insertions(+), 737 deletions(-) diff --git a/ironic/drivers/fake.py b/ironic/drivers/fake.py index 4c7bfe4363..98178947fe 100644 --- a/ironic/drivers/fake.py +++ b/ironic/drivers/fake.py @@ -29,6 +29,7 @@ from ironic.drivers.modules.cimc import management as cimc_mgmt from ironic.drivers.modules.cimc import power as cimc_power from ironic.drivers.modules.drac import management as drac_mgmt from ironic.drivers.modules.drac import power as drac_power +from ironic.drivers.modules.drac import vendor_passthru as drac_vendor from ironic.drivers.modules import fake from ironic.drivers.modules import iboot from ironic.drivers.modules.ilo import inspect as ilo_inspect @@ -190,6 +191,7 @@ class FakeDracDriver(base.BaseDriver): self.power = drac_power.DracPower() self.deploy = fake.FakeDeploy() self.management = drac_mgmt.DracManagement() + self.vendor = drac_vendor.DracVendorPassthru() class FakeSNMPDriver(base.BaseDriver): diff --git a/ironic/drivers/modules/drac/bios.py b/ironic/drivers/modules/drac/bios.py index 2c19066595..6360c0bc6b 100644 --- a/ironic/drivers/modules/drac/bios.py +++ b/ironic/drivers/modules/drac/bios.py @@ -12,211 +12,27 @@ # under the License. """ -DRAC Bios specific methods +DRAC BIOS configuration specific methods """ -import re -from xml.etree import ElementTree as ET - from oslo_log import log as logging -from oslo_utils import excutils +from oslo_utils import importutils from ironic.common import exception -from ironic.common.i18n import _, _LE, _LW -from ironic.conductor import task_manager -from ironic.drivers.modules.drac import client as wsman_client -from ironic.drivers.modules.drac import management -from ironic.drivers.modules.drac import resource_uris +from ironic.common.i18n import _LE +from ironic.drivers.modules.drac import common as drac_common +from ironic.drivers.modules.drac import job as drac_job + +drac_exceptions = importutils.try_import('dracclient.exceptions') LOG = logging.getLogger(__name__) -def _val_or_none(item): - """Test to see if an XML element should be treated as None. - - If the element contains an XML Schema namespaced nil attribute that - has a value of True, return None. Otherwise, return whatever the - text of the element is. - - :param item: an XML element. - :returns: None or the test of the XML element. - """ - - if item is None: - return - itemnil = item.attrib.get('{%s}nil' % resource_uris.CIM_XmlSchema) - if itemnil == "true": - return - else: - return item.text - - -def _parse_common(item, ns): - """Parse common values that all attributes must have. - - :param item: an XML element. - :param ns: the namespace to search. - :returns: a dictionary containing the parsed attributes of the element. - :raises: DracOperationFailed if the given element had no AttributeName - value. - """ - searches = {'current_value': './{%s}CurrentValue' % ns, - 'read_only': './{%s}IsReadOnly' % ns, - 'pending_value': './{%s}PendingValue' % ns} - LOG.debug("Handing %(ns)s for %(xml)s", { - 'ns': ns, - 'xml': ET.tostring(item), - }) - name = item.findtext('./{%s}AttributeName' % ns) - if not name: - raise exception.DracOperationFailed( - message=_('Item has no name: "%s"') % ET.tostring(item)) - res = {} - res['name'] = name - - for k in searches: - if k == 'read_only': - res[k] = item.findtext(searches[k]) == 'true' - else: - res[k] = _val_or_none(item.find(searches[k])) - return res - - -def _format_error_msg(invalid_attribs_msgs, read_only_keys): - """Format a combined error message. - - This method creates a combined error message from a list of error messages - and a list of read-only keys. - - :param invalid_attribs_msgs: a list of invalid attribute error messages. - :param read_only_keys: a list of read only keys that were attempted to be - written to. - :returns: a formatted error message. - """ - msg = '\n'.join(invalid_attribs_msgs) - if invalid_attribs_msgs and read_only_keys: - msg += '\n' - if read_only_keys: - msg += (_('Cannot set read-only BIOS settings "%r"') % read_only_keys) - return msg - - -def parse_enumeration(item, ns): - """Parse an attribute that has a set of distinct values. - - :param item: an XML element. - :param ns: the namespace to search. - :returns: a dictionary containing the parsed attributes of the element. - :raises: DracOperationFailed if the given element had no AttributeName - value. - """ - res = _parse_common(item, ns) - res['possible_values'] = sorted( - [v.text for v in item.findall('./{%s}PossibleValues' % ns)]) - - return res - - -def parse_string(item, ns): - """Parse an attribute that should be a freeform string. - - :param item: an XML element. - :param ns: the namespace to search. - :returns: a dictionary containing the parsed attributes of the element. - :raises: DracOperationFailed if the given element had no AttributeName - value. - """ - res = _parse_common(item, ns) - searches = {'min_length': './{%s}MinLength' % ns, - 'max_length': './{%s}MaxLength' % ns, - 'pcre_regex': './{%s}ValueExpression' % ns} - for k in searches: - if k == 'pcre_regex': - res[k] = _val_or_none(item.find(searches[k])) - else: - res[k] = int(item.findtext(searches[k])) - - # Workaround for a BIOS bug in one of the 13 gen boxes - badval = re.compile(r"MAX_ASSET_TAG_LEN") - if (res['pcre_regex'] is not None and - res['name'] == 'AssetTag' and - badval.search(res['pcre_regex'])): - res['pcre_regex'] = badval.sub("%d" % res['max_length'], - res['pcre_regex']) - return res - - -def parse_integer(item, ns): - """Parse an attribute that should be an integer. - - :param item: an XML element. - :param ns: the namespace to search. - :returns: a dictionary containing the parsed attributes of the element. - :raises: DracOperationFailed if the given element had no AttributeName - value. - """ - res = _parse_common(item, ns) - for k in ['current_value', 'pending_value']: - if res[k]: - res[k] = int(res[k]) - searches = {'lower_bound': './{%s}LowerBound' % ns, - 'upper_bound': './{%s}UpperBound' % ns} - for k in searches: - res[k] = int(item.findtext(searches[k])) - - return res - - -def _get_config(node, resource): - """Helper for get_config. - - Handles getting BIOS config values for a single namespace - - :param node: an ironic node object. - :param resource: the namespace. - :returns: a dictionary that maps the name of each attribute to a dictionary - of values of that attribute. - :raises: InvalidParameterValue if some information required to connnect - to the DRAC is missing on the node or the value of one or more - required parameters is invalid. - :raises: DracClientError on an error from pywsman library. - :raises: DracOperationFailed if the specified resource is unknown. - """ - res = {} - client = wsman_client.get_wsman_client(node) - try: - doc = client.wsman_enumerate(resource) - except exception.DracClientError as exc: - with excutils.save_and_reraise_exception(): - LOG.error(_LE('DRAC driver failed to get BIOS settings ' - 'for resource %(resource)s ' - 'from node %(node_uuid)s. ' - 'Reason: %(error)s.'), - {'node_uuid': node.uuid, - 'resource': resource, - 'error': exc}) - items = doc.find('.//{%s}Items' % resource_uris.CIM_WSMAN) - for item in items: - if resource == resource_uris.DCIM_BIOSEnumeration: - attribute = parse_enumeration(item, resource) - elif resource == resource_uris.DCIM_BIOSString: - attribute = parse_string(item, resource) - elif resource == resource_uris.DCIM_BIOSInteger: - attribute = parse_integer(item, resource) - else: - raise exception.DracOperationFailed( - message=_('Unknown namespace %(ns)s for item: "%(item)s"') % { - 'item': ET.tostring(item), 'ns': resource}) - res[attribute['name']] = attribute - return res - - def get_config(node): - """Get the BIOS configuration from a Dell server using WSMAN + """Get the BIOS configuration. :param node: an ironic node object. - :raises: DracClientError on an error from pywsman. - :raises: DracOperationFailed when a BIOS setting cannot be parsed. + :raises: DracOperationError on an error from python-dracclient. :returns: a dictionary containing BIOS settings in the form of: {'EnumAttrib': {'name': 'EnumAttrib', 'current_value': 'Value', @@ -267,166 +83,85 @@ def get_config(node): Integer attributes also have the following parameters: :lower_bound: is the minimum value the attribute can have. :upper_bound: is the maximum value the attribute can have. - """ - res = {} - for ns in [resource_uris.DCIM_BIOSEnumeration, - resource_uris.DCIM_BIOSString, - resource_uris.DCIM_BIOSInteger]: - attribs = _get_config(node, ns) - if not set(res).isdisjoint(set(attribs)): - raise exception.DracOperationFailed( - message=_('Colliding attributes %r') % ( - set(res) & set(attribs))) - res.update(attribs) - return res + + client = drac_common.get_drac_client(node) + + try: + return client.list_bios_settings() + except drac_exceptions.BaseClientException as exc: + LOG.error(_LE('DRAC driver failed to get the BIOS settings for node ' + '%(node_uuid)s. Reason: %(error)s.'), + {'node_uuid': node.uuid, + 'error': exc}) + raise exception.DracOperationError(error=exc) -@task_manager.require_exclusive_lock def set_config(task, **kwargs): """Sets the pending_value parameter for each of the values passed in. - :param task: an ironic task object. + :param task: a TaskManager instance containing the node to act on. :param kwargs: a dictionary of {'AttributeName': 'NewValue'} - :raises: DracOperationFailed if any new values are invalid. - :raises: DracOperationFailed if any of the attributes are read-only. - :raises: DracOperationFailed if any of the attributes cannot be set for - any other reason. - :raises: DracClientError on an error from the pywsman library. - :returns: A boolean indicating whether commit_config needs to be - called to make the changes. - + :raises: DracOperationError on an error from python-dracclient. + :returns: A dictionary containing the commit_required key with a boolean + value indicating whether commit_bios_config() needs to be called + to make the changes. """ node = task.node - management.check_for_config_job(node) - current = get_config(node) - unknown_keys = set(kwargs) - set(current) - if unknown_keys: - LOG.warning(_LW('Ignoring unknown BIOS attributes "%r"'), - unknown_keys) + drac_job.validate_job_queue(node) - candidates = set(kwargs) - unknown_keys - read_only_keys = [] - unchanged_attribs = [] - invalid_attribs_msgs = [] - attrib_names = [] + client = drac_common.get_drac_client(node) + if 'http_method' in kwargs: + del kwargs['http_method'] - for k in candidates: - if str(kwargs[k]) == str(current[k]['current_value']): - unchanged_attribs.append(k) - elif current[k]['read_only']: - read_only_keys.append(k) - else: - if 'possible_values' in current[k]: - if str(kwargs[k]) not in current[k]['possible_values']: - m = _('Attribute %(attr)s cannot be set to value %(val)s.' - ' It must be in %(ok)r') % { - 'attr': k, - 'val': kwargs[k], - 'ok': current[k]['possible_values']} - invalid_attribs_msgs.append(m) - continue - if ('pcre_regex' in current[k] and - current[k]['pcre_regex'] is not None): - regex = re.compile(current[k]['pcre_regex']) - if regex.search(str(kwargs[k])) is None: - # TODO(victor-lowther) - # Leave untranslated for now until the unicode - # issues that the test suite exposes are straightened out. - m = ('Attribute %(attr)s cannot be set to value %(val)s.' - ' It must match regex %(re)s.') % { - 'attr': k, - 'val': kwargs[k], - 're': current[k]['pcre_regex']} - invalid_attribs_msgs.append(m) - continue - if 'lower_bound' in current[k]: - lower = current[k]['lower_bound'] - upper = current[k]['upper_bound'] - val = int(kwargs[k]) - if val < lower or val > upper: - m = _('Attribute %(attr)s cannot be set to value %(val)d.' - ' It must be between %(lower)d and %(upper)d.') % { - 'attr': k, - 'val': val, - 'lower': lower, - 'upper': upper} - invalid_attribs_msgs.append(m) - continue - attrib_names.append(k) - - if unchanged_attribs: - LOG.warning(_LW('Ignoring unchanged BIOS settings %r'), - unchanged_attribs) - - if invalid_attribs_msgs or read_only_keys: - raise exception.DracOperationFailed( - _format_error_msg(invalid_attribs_msgs, read_only_keys)) - - if not attrib_names: - return False - - client = wsman_client.get_wsman_client(node) - selectors = {'CreationClassName': 'DCIM_BIOSService', - 'Name': 'DCIM:BIOSService', - 'SystemCreationClassName': 'DCIM_ComputerSystem', - 'SystemName': 'DCIM:ComputerSystem'} - properties = {'Target': 'BIOS.Setup.1-1', - 'AttributeName': attrib_names, - 'AttributeValue': map(lambda k: kwargs[k], attrib_names)} - doc = client.wsman_invoke(resource_uris.DCIM_BIOSService, - 'SetAttributes', - selectors, - properties) - # Yes, we look for RebootRequired. In this context, that actually means - # that we need to create a lifecycle controller config job and then reboot - # so that the lifecycle controller can commit the BIOS config changes that - # we have proposed. - set_results = doc.findall( - './/{%s}RebootRequired' % resource_uris.DCIM_BIOSService) - return any(str(res.text) == 'Yes' for res in set_results) + try: + return client.set_bios_settings(kwargs) + except drac_exceptions.BaseClientException as exc: + LOG.error(_LE('DRAC driver failed to set the BIOS settings for node ' + '%(node_uuid)s. Reason: %(error)s.'), + {'node_uuid': node.uuid, + 'error': exc}) + raise exception.DracOperationError(error=exc) -@task_manager.require_exclusive_lock def commit_config(task, reboot=False): """Commits pending changes added by set_config - :param task: is the ironic task for running the config job. + :param task: a TaskManager instance containing the node to act on. :param reboot: indicates whether a reboot job should be automatically created with the config job. - :raises: DracClientError on an error from pywsman library. - :raises: DracPendingConfigJobExists if the job is already created. - :raises: DracOperationFailed if the client received response with an - error message. - :raises: DracUnexpectedReturnValue if the client received a response - with unexpected return value - + :raises: DracOperationError on an error from python-dracclient. + :returns: the job_id key with the id of the newly created config job. """ node = task.node - management.check_for_config_job(node) - management.create_config_job(node, reboot) + drac_job.validate_job_queue(node) + + client = drac_common.get_drac_client(node) + + try: + return client.commit_pending_bios_changes(reboot) + except drac_exceptions.BaseClientException as exc: + LOG.error(_LE('DRAC driver failed to commit the pending BIOS changes ' + 'for node %(node_uuid)s. Reason: %(error)s.'), + {'node_uuid': node.uuid, + 'error': exc}) + raise exception.DracOperationError(error=exc) -@task_manager.require_exclusive_lock def abandon_config(task): """Abandons uncommitted changes added by set_config - :param task: is the ironic task for abandoning the changes. - :raises: DracClientError on an error from pywsman library. - :raises: DracOperationFailed on error reported back by DRAC. - :raises: DracUnexpectedReturnValue if the drac did not report success. - + :param task: a TaskManager instance containing the node to act on. + :raises: DracOperationError on an error from python-dracclient. """ node = task.node - client = wsman_client.get_wsman_client(node) - selectors = {'CreationClassName': 'DCIM_BIOSService', - 'Name': 'DCIM:BIOSService', - 'SystemCreationClassName': 'DCIM_ComputerSystem', - 'SystemName': 'DCIM:ComputerSystem'} - properties = {'Target': 'BIOS.Setup.1-1'} + client = drac_common.get_drac_client(node) - client.wsman_invoke(resource_uris.DCIM_BIOSService, - 'DeletePendingConfiguration', - selectors, - properties, - wsman_client.RET_SUCCESS) + try: + client.abandon_pending_bios_changes() + except drac_exceptions.BaseClientException as exc: + LOG.error(_LE('DRAC driver failed to delete the pending BIOS ' + 'settings for node %(node_uuid)s. Reason: %(error)s.'), + {'node_uuid': node.uuid, + 'error': exc}) + raise exception.DracOperationError(error=exc) diff --git a/ironic/drivers/modules/drac/management.py b/ironic/drivers/modules/drac/management.py index 2c4e5d08f5..ae0ea6c224 100644 --- a/ironic/drivers/modules/drac/management.py +++ b/ironic/drivers/modules/drac/management.py @@ -20,7 +20,6 @@ DRAC management interface """ from oslo_log import log as logging -from oslo_utils import excutils from oslo_utils import importutils from ironic.common import boot_devices @@ -29,11 +28,8 @@ from ironic.common.i18n import _ from ironic.common.i18n import _LE from ironic.conductor import task_manager from ironic.drivers import base -from ironic.drivers.modules.drac import client as drac_client from ironic.drivers.modules.drac import common as drac_common from ironic.drivers.modules.drac import job as drac_job -from ironic.drivers.modules.drac import resource_uris - drac_exceptions = importutils.try_import('dracclient.exceptions') @@ -45,11 +41,6 @@ _BOOT_DEVICES_MAP = { boot_devices.CDROM: 'Optical', } -TARGET_DEVICE = 'BIOS.Setup.1-1' - -# RebootJobType constants -_GRACEFUL_REBOOT_WITH_FORCED_SHUTDOWN = '3' - # BootMode constants PERSISTENT_BOOT_MODE = 'IPL' NON_PERSISTENT_BOOT_MODE = 'OneTime' @@ -127,86 +118,6 @@ def set_boot_device(node, device, persistent=False): raise exception.DracOperationError(error=exc) -# TODO(ifarkas): delete this during BIOS vendor_passthru refactor -def create_config_job(node, reboot=False): - """Create a configuration job. - - This method is used to apply the pending values created by - set_boot_device(). - - :param node: an ironic node object. - :param reboot: indicates whether a reboot job should be automatically - created with the config job. - :raises: DracClientError if the client received unexpected response. - :raises: DracOperationFailed if the client received response with an - error message. - :raises: DracUnexpectedReturnValue if the client received a response - with unexpected return value. - """ - client = drac_client.get_wsman_client(node) - selectors = {'CreationClassName': 'DCIM_BIOSService', - 'Name': 'DCIM:BIOSService', - 'SystemCreationClassName': 'DCIM_ComputerSystem', - 'SystemName': 'DCIM:ComputerSystem'} - properties = {'Target': TARGET_DEVICE, - 'ScheduledStartTime': 'TIME_NOW'} - - if reboot: - properties['RebootJobType'] = _GRACEFUL_REBOOT_WITH_FORCED_SHUTDOWN - - try: - client.wsman_invoke(resource_uris.DCIM_BIOSService, - 'CreateTargetedConfigJob', selectors, properties, - drac_client.RET_CREATED) - except exception.DracRequestFailed as exc: - with excutils.save_and_reraise_exception(): - LOG.error(_LE('DRAC driver failed to create config job for node ' - '%(node_uuid)s. The changes are not applied. ' - 'Reason: %(error)s.'), - {'node_uuid': node.uuid, 'error': exc}) - - -# TODO(ifarkas): delete this during BIOS vendor_passthru refactor -def check_for_config_job(node): - """Check if a configuration job is already created. - - :param node: an ironic node object. - :raises: DracClientError on an error from pywsman library. - :raises: DracPendingConfigJobExists if the job is already created. - - """ - client = drac_client.get_wsman_client(node) - try: - doc = client.wsman_enumerate(resource_uris.DCIM_LifecycleJob) - except exception.DracClientError as exc: - with excutils.save_and_reraise_exception(): - LOG.error(_LE('DRAC driver failed to list the configuration jobs ' - 'for node %(node_uuid)s. Reason: %(error)s.'), - {'node_uuid': node.uuid, 'error': exc}) - - items = drac_common.find_xml(doc, 'DCIM_LifecycleJob', - resource_uris.DCIM_LifecycleJob, - find_all=True) - for i in items: - name = drac_common.find_xml(i, 'Name', resource_uris.DCIM_LifecycleJob) - if TARGET_DEVICE not in name.text: - continue - - job_status = drac_common.find_xml( - i, 'JobStatus', resource_uris.DCIM_LifecycleJob).text - # If job is already completed or failed we can - # create another one. - # The 'Completed with Errors' JobStatus can be returned by - # configuration jobs that set NIC or BIOS attributes. - # Job Control Documentation: http://goo.gl/o1dDD3 (Section 7.2.3.2) - if job_status.lower() not in ('completed', 'completed with errors', - 'failed'): - job_id = drac_common.find_xml(i, 'InstanceID', - resource_uris.DCIM_LifecycleJob).text - raise exception.DracPendingConfigJobExists(job_id=job_id, - target=TARGET_DEVICE) - - class DracManagement(base.ManagementInterface): def get_properties(self): diff --git a/ironic/drivers/modules/drac/vendor_passthru.py b/ironic/drivers/modules/drac/vendor_passthru.py index fa96fe927a..4322605933 100644 --- a/ironic/drivers/modules/drac/vendor_passthru.py +++ b/ironic/drivers/modules/drac/vendor_passthru.py @@ -15,6 +15,7 @@ DRAC VendorPassthruBios Driver """ +from ironic.conductor import task_manager from ironic.drivers import base from ironic.drivers.modules.drac import bios from ironic.drivers.modules.drac import common as drac_common @@ -24,97 +25,89 @@ class DracVendorPassthru(base.VendorInterface): """Interface for DRAC specific BIOS configuration methods.""" def get_properties(self): - """Returns the driver_info properties. - - This method returns the driver_info properties for this driver. - - :returns: a dictionary of propery names and their descriptions. - """ + """Return the properties of the interface.""" return drac_common.COMMON_PROPERTIES def validate(self, task, **kwargs): - """Validates the driver_info of a node. + """Validate the driver-specific info supplied. - This method validates the driver_info associated with the node that is - associated with the task. + This method validates whether the 'driver_info' property of the + supplied node contains the required information for this driver to + manage the power state of the node. - :param task: the ironic task used to identify the node. + :param task: a TaskManager instance containing the node to act on. :param kwargs: not used. - :raises: InvalidParameterValue if mandatory information is missing on - the node or any driver_info is invalid. - :returns: a dict containing information from driver_info - and default values. + :raises: InvalidParameterValue if required driver_info attribute + is missing or invalid on the node. """ return drac_common.parse_driver_info(task.node) @base.passthru(['GET'], async=False) def get_bios_config(self, task, **kwargs): - """Get BIOS settings. + """Get the BIOS configuration. This method is used to retrieve the BIOS settings from a node. - :param task: the ironic task used to identify the node. + :param task: a TaskManager instance containing the node to act on. :param kwargs: not used. - :raises: DracClientError on an error from pywsman. - :raises: DracOperationFailed when a BIOS setting cannot be parsed. + :raises: DracOperationError on an error from python-dracclient. :returns: a dictionary containing BIOS settings. """ - return bios.get_config(task.node) + bios_attrs = {} + for name, bios_attr in bios.get_config(task.node).items(): + # NOTE(ifarkas): call from python-dracclient returns list of + # namedtuples, converting it to dict here. + bios_attrs[name] = bios_attr.__dict__ + + return bios_attrs @base.passthru(['POST'], async=False) + @task_manager.require_exclusive_lock def set_bios_config(self, task, **kwargs): """Change BIOS settings. This method is used to change the BIOS settings on a node. - :param task: the ironic task used to identify the node. + :param task: a TaskManager instance containing the node to act on. :param kwargs: a dictionary of {'AttributeName': 'NewValue'} - :raises: DracOperationFailed if any of the attributes cannot be set for - any reason. - :raises: DracClientError on an error from the pywsman library. - :returns: A dictionary containing the commit_needed key with a boolean - value indicating whether commit_config() needs to be called - to make the changes. + :raises: DracOperationError on an error from python-dracclient. + :returns: A dictionary containing the commit_required key with a + Boolean value indicating whether commit_bios_config() needs + to be called to make the changes. """ - return {'commit_needed': bios.set_config(task, **kwargs)} + return bios.set_config(task, **kwargs) @base.passthru(['POST'], async=False) + @task_manager.require_exclusive_lock def commit_bios_config(self, task, reboot=False, **kwargs): """Commit a BIOS configuration job. This method is used to commit a BIOS configuration job. submitted through set_bios_config(). - :param task: the ironic task for running the config job. + :param task: a TaskManager instance containing the node to act on. :param reboot: indicates whether a reboot job should be automatically created with the config job. - :param kwargs: additional arguments sent via vendor passthru. - :raises: DracClientError on an error from pywsman library. - :raises: DracPendingConfigJobExists if the job is already created. - :raises: DracOperationFailed if the client received response with an - error message. - :raises: DracUnexpectedReturnValue if the client received a response - with unexpected return value - :returns: A dictionary containing the committing key with no return - value, and the reboot_needed key with a value of True. + :param kwargs: not used. + :raises: DracOperationError on an error from python-dracclient. + :returns: A dictionary containing the job_id key with the id of the + newly created config job, and the reboot_required key + indicating whether to node needs to be rebooted to start the + config job. """ - bios.commit_config(task, reboot=reboot) - return {'committing': None, 'reboot_needed': not reboot} + job_id = bios.commit_config(task, reboot=reboot) + return {'job_id': job_id, 'reboot_required': not reboot} @base.passthru(['DELETE'], async=False) + @task_manager.require_exclusive_lock def abandon_bios_config(self, task, **kwargs): """Abandon a BIOS configuration job. - This method is used to abandon a BIOS configuration job previously + This method is used to abandon a BIOS configuration previously submitted through set_bios_config(). - :param task: the ironic task for abandoning the changes. + :param task: a TaskManager instance containing the node to act on. :param kwargs: not used. - :raises: DracClientError on an error from pywsman library. - :raises: DracOperationFailed on error reported back by DRAC. - :raises: DracUnexpectedReturnValue if the drac did not report success. - :returns: A dictionary containing the abandoned key with no return - value. + :raises: DracOperationError on an error from python-dracclient. """ bios.abandon_config(task) - return {'abandoned': None} diff --git a/ironic/tests/unit/drivers/modules/drac/test_bios.py b/ironic/tests/unit/drivers/modules/drac/test_bios.py index e039fbb684..7cfbd6fcb3 100644 --- a/ironic/tests/unit/drivers/modules/drac/test_bios.py +++ b/ironic/tests/unit/drivers/modules/drac/test_bios.py @@ -16,196 +16,150 @@ # under the License. """ -Test class for DRAC BIOS interface +Test class for DRAC BIOS configuration specific methods """ +from dracclient import exceptions as drac_exceptions import mock from ironic.common import exception from ironic.conductor import task_manager -from ironic.drivers.modules.drac import bios -from ironic.drivers.modules.drac import client as drac_client -from ironic.drivers.modules.drac import management as drac_mgmt -from ironic.drivers.modules.drac import resource_uris +from ironic.drivers.modules.drac import common as drac_common from ironic.tests.unit.conductor import mgr_utils from ironic.tests.unit.db import base as db_base from ironic.tests.unit.db import utils as db_utils -from ironic.tests.unit.drivers.modules.drac import bios_wsman_mock -from ironic.tests.unit.drivers.modules.drac import utils as test_utils from ironic.tests.unit.objects import utils as obj_utils -from six.moves.urllib.parse import unquote -FAKE_DRAC = db_utils.get_test_drac_info() +INFO_DICT = db_utils.get_test_drac_info() -def _base_config(responses=[]): - for resource in [resource_uris.DCIM_BIOSEnumeration, - resource_uris.DCIM_BIOSString, - resource_uris.DCIM_BIOSInteger]: - xml_root = test_utils.mock_wsman_root( - bios_wsman_mock.Enumerations[resource]['XML']) - responses.append(xml_root) - return responses - - -def _set_config(responses=[]): - ccj_xml = test_utils.build_soap_xml([{'DCIM_LifecycleJob': - {'Name': 'fake'}}], - resource_uris.DCIM_LifecycleJob) - responses.append(test_utils.mock_wsman_root(ccj_xml)) - return _base_config(responses) - - -def _mock_pywsman_responses(client, responses): - mpw = client.Client.return_value - mpw.enumerate.side_effect = responses - return mpw - - -@mock.patch.object(drac_client, 'pywsman') -class DracBiosTestCase(db_base.DbTestCase): +class DracBIOSConfigurationTestCase(db_base.DbTestCase): def setUp(self): - super(DracBiosTestCase, self).setUp() + super(DracBIOSConfigurationTestCase, self).setUp() mgr_utils.mock_the_extension_manager(driver='fake_drac') self.node = obj_utils.create_test_node(self.context, driver='fake_drac', - driver_info=FAKE_DRAC) + driver_info=INFO_DICT) - def test_get_config(self, client): - _mock_pywsman_responses(client, _base_config()) - expected = {} - for resource in [resource_uris.DCIM_BIOSEnumeration, - resource_uris.DCIM_BIOSString, - resource_uris.DCIM_BIOSInteger]: - expected.update(bios_wsman_mock.Enumerations[resource]['Dict']) - result = bios.get_config(self.node) - self.assertEqual(expected, result) + patch_get_drac_client = mock.patch.object( + drac_common, 'get_drac_client', spec_set=True, autospec=True) + mock_get_drac_client = patch_get_drac_client.start() + self.mock_client = mock.Mock() + mock_get_drac_client.return_value = self.mock_client + self.addCleanup(patch_get_drac_client.stop) + + proc_virt_attr = { + 'name': 'ProcVirtualization', + 'current_value': 'Enabled', + 'pending_value': None, + 'read_only': False, + 'possible_values': ['Enabled', 'Disabled']} + self.bios_attrs = { + 'ProcVirtualization': mock.Mock(**proc_virt_attr) + } + + def test_get_config(self): + self.mock_client.list_bios_settings.return_value = self.bios_attrs + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + bios_config = task.driver.vendor.get_bios_config(task) + + self.mock_client.list_bios_settings.assert_called_once_with() + self.assertIn('ProcVirtualization', bios_config) + + def test_get_config_fail(self): + exc = drac_exceptions.BaseClientException('boom') + self.mock_client.list_bios_settings.side_effect = exc + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + self.assertRaises(exception.DracOperationError, + task.driver.vendor.get_bios_config, task) + + self.mock_client.list_bios_settings.assert_called_once_with() + + def test_set_config(self): + self.mock_client.list_jobs.return_value = [] - def test_set_config_empty(self, client): - _mock_pywsman_responses(client, _set_config()) with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - res = bios.set_config(task) - self.assertFalse(res) + task.driver.vendor.set_bios_config(task, + ProcVirtualization='Enabled') + + self.mock_client.list_jobs.assert_called_once_with( + only_unfinished=True) + self.mock_client.set_bios_settings.assert_called_once_with( + {'ProcVirtualization': 'Enabled'}) + + def test_set_config_fail(self): + self.mock_client.list_jobs.return_value = [] + exc = drac_exceptions.BaseClientException('boom') + self.mock_client.set_bios_settings.side_effect = exc - def test_set_config_nochange(self, client): - _mock_pywsman_responses(client, _set_config()) with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - task.node = self.node - res = bios.set_config(task, - MemTest='Disabled', - ProcCStates='Disabled', - SystemModelName='PowerEdge R630', - AssetTag=None, - Proc1NumCores=8, - AcPwrRcvryUserDelay=60) - self.assertFalse(res) + self.assertRaises(exception.DracOperationError, + task.driver.vendor.set_bios_config, task, + ProcVirtualization='Enabled') + + self.mock_client.set_bios_settings.assert_called_once_with( + {'ProcVirtualization': 'Enabled'}) + + def test_commit_config(self): + self.mock_client.list_jobs.return_value = [] - def test_set_config_ro(self, client): - _mock_pywsman_responses(client, _set_config()) with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - task.node = self.node - self.assertRaises(exception.DracOperationFailed, - bios.set_config, task, - ProcCStates="Enabled") + task.driver.vendor.commit_bios_config(task) + + self.mock_client.list_jobs.assert_called_once_with( + only_unfinished=True) + self.mock_client.commit_pending_bios_changes.assert_called_once_with( + False) + + def test_commit_config_with_reboot(self): + self.mock_client.list_jobs.return_value = [] - def test_set_config_enum_invalid(self, client): - _mock_pywsman_responses(client, _set_config()) with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - task.node = self.node - self.assertRaises(exception.DracOperationFailed, - bios.set_config, task, - MemTest="Never") + task.driver.vendor.commit_bios_config(task, reboot=True) + + self.mock_client.list_jobs.assert_called_once_with( + only_unfinished=True) + self.mock_client.commit_pending_bios_changes.assert_called_once_with( + True) + + def test_commit_config_fail(self): + self.mock_client.list_jobs.return_value = [] + exc = drac_exceptions.BaseClientException('boom') + self.mock_client.commit_pending_bios_changes.side_effect = exc - def test_set_config_string_toolong(self, client): - _mock_pywsman_responses(client, _set_config()) - tag = ('Never have I seen such a silly long asset tag! ' - 'It is really rather ridiculous, don\'t you think?') with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - task.node = self.node - self.assertRaises(exception.DracOperationFailed, - bios.set_config, task, - AssetTag=tag) + self.assertRaises(exception.DracOperationError, + task.driver.vendor.commit_bios_config, task) - def test_set_config_string_nomatch(self, client): - _mock_pywsman_responses(client, _set_config()) - tag = unquote('%80') + self.mock_client.list_jobs.assert_called_once_with( + only_unfinished=True) + self.mock_client.commit_pending_bios_changes.assert_called_once_with( + False) + + def test_abandon_config(self): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - task.node = self.node - self.assertRaises(exception.DracOperationFailed, - bios.set_config, task, - AssetTag=tag) + task.driver.vendor.abandon_bios_config(task) + + self.mock_client.abandon_pending_bios_changes.assert_called_once_with() + + def test_abandon_config_fail(self): + exc = drac_exceptions.BaseClientException('boom') + self.mock_client.abandon_pending_bios_changes.side_effect = exc - def test_set_config_integer_toosmall(self, client): - _mock_pywsman_responses(client, _set_config()) with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - task.node = self.node - self.assertRaises(exception.DracOperationFailed, - bios.set_config, task, - AcPwrRcvryUserDelay=0) + self.assertRaises(exception.DracOperationError, + task.driver.vendor.abandon_bios_config, task) - def test_set_config_integer_toobig(self, client): - _mock_pywsman_responses(client, _set_config()) - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - task.node = self.node - self.assertRaises(exception.DracOperationFailed, - bios.set_config, task, - AcPwrRcvryUserDelay=600) - - def test_set_config_needreboot(self, client): - mock_pywsman = _mock_pywsman_responses(client, _set_config()) - invoke_xml = test_utils.mock_wsman_root( - bios_wsman_mock.Invoke_Commit) - # TODO(victor-lowther) This needs more work. - # Specifically, we will need to verify that - # invoke was handed the XML blob we expected. - mock_pywsman.invoke.return_value = invoke_xml - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - task.node = self.node - res = bios.set_config(task, - AssetTag="An Asset Tag", - MemTest="Enabled") - self.assertTrue(res) - - @mock.patch.object(drac_mgmt, 'check_for_config_job', - spec_set=True, autospec=True) - @mock.patch.object(drac_mgmt, 'create_config_job', spec_set=True, - autospec=True) - def test_commit_config(self, mock_ccj, mock_cfcj, client): - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - task.node = self.node - bios.commit_config(task) - mock_cfcj.assert_called_once_with(self.node) - mock_ccj.assert_called_once_with(self.node, False) - - @mock.patch.object(drac_mgmt, 'check_for_config_job', - spec_set=True, autospec=True) - @mock.patch.object(drac_mgmt, 'create_config_job', spec_set=True, - autospec=True) - def test_commit_config_with_reboot(self, mock_ccj, mock_cfcj, client): - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - task.node = self.node - bios.commit_config(task, reboot=True) - mock_cfcj.assert_called_once_with(self.node) - mock_ccj.assert_called_once_with(self.node, True) - - @mock.patch.object(drac_client.Client, 'wsman_invoke', spec_set=True, - autospec=True) - def test_abandon_config(self, mock_wi, client): - _mock_pywsman_responses(client, _set_config()) - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - task.node = self.node - bios.abandon_config(task) - self.assertTrue(mock_wi.called) + self.mock_client.abandon_pending_bios_changes.assert_called_once_with() diff --git a/ironic/tests/unit/drivers/modules/drac/test_management.py b/ironic/tests/unit/drivers/modules/drac/test_management.py index 23e8a49310..0a42077c84 100644 --- a/ironic/tests/unit/drivers/modules/drac/test_management.py +++ b/ironic/tests/unit/drivers/modules/drac/test_management.py @@ -24,17 +24,12 @@ import mock import ironic.common.boot_devices from ironic.common import exception from ironic.conductor import task_manager -from ironic.drivers.modules.drac import client as drac_client from ironic.drivers.modules.drac import common as drac_common from ironic.drivers.modules.drac import job as drac_job from ironic.drivers.modules.drac import management as drac_mgmt -from ironic.drivers.modules.drac import resource_uris from ironic.tests.unit.conductor import mgr_utils from ironic.tests.unit.db import base as db_base from ironic.tests.unit.db import utils as db_utils -from ironic.tests.unit.drivers.modules.drac import utils as test_utils -from ironic.tests.unit.drivers import third_party_driver_mock_specs \ - as mock_specs from ironic.tests.unit.objects import utils as obj_utils INFO_DICT = db_utils.get_test_drac_info() @@ -271,119 +266,3 @@ class DracManagementTestCase(db_base.DbTestCase): shared=False) as task: self.assertRaises(NotImplementedError, task.driver.management.get_sensors_data, task) - - -# TODO(ifarkas): delete this during BIOS vendor_passthru refactor -@mock.patch.object(drac_client, 'pywsman', spec_set=mock_specs.PYWSMAN_SPEC) -class DracConfigJobMethodsTestCase(db_base.DbTestCase): - - def setUp(self): - super(DracConfigJobMethodsTestCase, self).setUp() - mgr_utils.mock_the_extension_manager(driver='fake_drac') - self.node = obj_utils.create_test_node(self.context, - driver='fake_drac', - driver_info=INFO_DICT) - - def test__check_for_config_job(self, mock_client_pywsman): - result_xml = test_utils.build_soap_xml( - [{'DCIM_LifecycleJob': {'Name': 'fake'}}], - resource_uris.DCIM_LifecycleJob) - - mock_xml = test_utils.mock_wsman_root(result_xml) - mock_pywsman = mock_client_pywsman.Client.return_value - mock_pywsman.enumerate.return_value = mock_xml - - result = drac_mgmt.check_for_config_job(self.node) - - self.assertIsNone(result) - mock_pywsman.enumerate.assert_called_once_with( - mock.ANY, mock.ANY, resource_uris.DCIM_LifecycleJob) - - def test__check_for_config_job_already_exist(self, mock_client_pywsman): - result_xml = test_utils.build_soap_xml( - [{'DCIM_LifecycleJob': {'Name': 'BIOS.Setup.1-1', - 'JobStatus': 'scheduled', - 'InstanceID': 'fake'}}], - resource_uris.DCIM_LifecycleJob) - - mock_xml = test_utils.mock_wsman_root(result_xml) - mock_pywsman = mock_client_pywsman.Client.return_value - mock_pywsman.enumerate.return_value = mock_xml - - self.assertRaises(exception.DracPendingConfigJobExists, - drac_mgmt.check_for_config_job, self.node) - mock_pywsman.enumerate.assert_called_once_with( - mock.ANY, mock.ANY, resource_uris.DCIM_LifecycleJob) - - def test__check_for_config_job_not_exist(self, mock_client_pywsman): - job_statuses = ["Completed", "Completed with Errors", "Failed"] - for job_status in job_statuses: - result_xml = test_utils.build_soap_xml( - [{'DCIM_LifecycleJob': {'Name': 'BIOS.Setup.1-1', - 'JobStatus': job_status, - 'InstanceID': 'fake'}}], - resource_uris.DCIM_LifecycleJob) - - mock_xml = test_utils.mock_wsman_root(result_xml) - mock_pywsman = mock_client_pywsman.Client.return_value - mock_pywsman.enumerate.return_value = mock_xml - - try: - drac_mgmt.check_for_config_job(self.node) - except (exception.DracClientError, - exception.DracPendingConfigJobExists): - self.fail("Failed to detect completed job due to " - "\"{}\" job status".format(job_status)) - - def test_create_config_job(self, mock_client_pywsman): - result_xml = test_utils.build_soap_xml( - [{'ReturnValue': drac_client.RET_CREATED}], - resource_uris.DCIM_BIOSService) - - mock_xml = test_utils.mock_wsman_root(result_xml) - mock_pywsman = mock_client_pywsman.Client.return_value - mock_pywsman.invoke.return_value = mock_xml - - result = drac_mgmt.create_config_job(self.node) - - self.assertIsNone(result) - mock_pywsman.invoke.assert_called_once_with( - mock.ANY, resource_uris.DCIM_BIOSService, - 'CreateTargetedConfigJob', None) - - def test_create_config_job_with_reboot(self, mock_client_pywsman): - result_xml = test_utils.build_soap_xml( - [{'ReturnValue': drac_client.RET_CREATED}], - resource_uris.DCIM_BIOSService) - mock_xml = test_utils.mock_wsman_root(result_xml) - mock_pywsman = mock_client_pywsman.Client.return_value - mock_pywsman.invoke.return_value = mock_xml - - mock_pywsman_clientopts = ( - mock_client_pywsman.ClientOptions.return_value) - - result = drac_mgmt.create_config_job(self.node, reboot=True) - - self.assertIsNone(result) - mock_pywsman_clientopts.add_property.assert_has_calls([ - mock.call('RebootJobType', '3'), - ]) - mock_pywsman.invoke.assert_called_once_with( - mock.ANY, resource_uris.DCIM_BIOSService, - 'CreateTargetedConfigJob', None) - - def test_create_config_job_error(self, mock_client_pywsman): - result_xml = test_utils.build_soap_xml( - [{'ReturnValue': drac_client.RET_ERROR, - 'Message': 'E_FAKE'}], - resource_uris.DCIM_BIOSService) - - mock_xml = test_utils.mock_wsman_root(result_xml) - mock_pywsman = mock_client_pywsman.Client.return_value - mock_pywsman.invoke.return_value = mock_xml - - self.assertRaises(exception.DracOperationFailed, - drac_mgmt.create_config_job, self.node) - mock_pywsman.invoke.assert_called_once_with( - mock.ANY, resource_uris.DCIM_BIOSService, - 'CreateTargetedConfigJob', None)