diff options
29 files changed, 856 insertions, 377 deletions
diff --git a/devstack/lib/ironic b/devstack/lib/ironic index f99dd994d..475f14e74 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -3084,6 +3084,7 @@ function ironic_configure_tempest { if [[ $IRONIC_VM_VOLUME_COUNT -gt 1 ]]; then iniset $TEMPEST_CONFIG baremetal_feature_enabled software_raid True + iniset $TEMPEST_CONFIG baremetal_feature_enabled deploy_time_raid True fi # Enabled features diff --git a/doc/source/install/standalone.rst b/doc/source/install/standalone.rst index a0aa46cbc..553c58306 100644 --- a/doc/source/install/standalone.rst +++ b/doc/source/install/standalone.rst @@ -342,7 +342,7 @@ be booted. #. Store the URL to the ISO image to ``instance_info/boot_iso``, instead of a ``kernel`` or ``ramdisk`` setting:: - openstack barmetal node set $NODE_UUID \ + openstack baremetal node set $NODE_UUID \ --instance-info boot_iso=$BOOT_ISO_URL #. Deploy the node:: diff --git a/driver-requirements.txt b/driver-requirements.txt index bb016f26e..539d27ba9 100644 --- a/driver-requirements.txt +++ b/driver-requirements.txt @@ -4,7 +4,7 @@ # python projects they should package as optional dependencies for Ironic. # These are available on pypi -proliantutils>=2.9.1 +proliantutils>=2.9.5 pysnmp>=4.3.0,<5.0.0 python-scciclient>=0.8.0 python-dracclient>=3.1.0,<5.0.0 diff --git a/ironic/api/controllers/v1/__init__.py b/ironic/api/controllers/v1/__init__.py index b0be184f9..a944dec69 100644 --- a/ironic/api/controllers/v1/__init__.py +++ b/ironic/api/controllers/v1/__init__.py @@ -18,8 +18,9 @@ Version 1 of the Ironic API Specification can be found at doc/source/webapi/v1.rst """ +from http import client as http_client + import pecan -from pecan import rest from webob import exc from ironic import api @@ -39,7 +40,7 @@ from ironic.api.controllers.v1 import utils from ironic.api.controllers.v1 import versions from ironic.api.controllers.v1 import volume from ironic.api.controllers import version -from ironic.api import expose +from ironic.api import method from ironic.common.i18n import _ BASE_VERSION = versions.BASE_VERSION @@ -57,205 +58,161 @@ def max_version(): versions.min_version_string(), versions.max_version_string()) -class MediaType(base.Base): - """A media type representation.""" - - base = str - type = str - - def __init__(self, base, type): - self.base = base - self.type = type - - -class V1(base.Base): - """The representation of the version 1 of the API.""" - - id = str - """The ID of the version, also acts as the release number""" - - media_types = [MediaType] - """An array of supported media types for this version""" - - links = None - """Links that point to a specific URL for this version and documentation""" - - chassis = None - """Links to the chassis resource""" - - nodes = None - """Links to the nodes resource""" - - ports = None - """Links to the ports resource""" - - portgroups = None - """Links to the portgroups resource""" - - drivers = None - """Links to the drivers resource""" - - volume = None - """Links to the volume resource""" - - lookup = None - """Links to the lookup resource""" - - heartbeat = None - """Links to the heartbeat resource""" - - conductors = None - """Links to the conductors resource""" - - allocations = None - """Links to the allocations resource""" - - deploy_templates = None - """Links to the deploy_templates resource""" - - version = None - """Version discovery information.""" - - events = None - """Links to the events resource""" - - @staticmethod - def convert(): - v1 = V1() - v1.id = "v1" - v1.links = [link.make_link('self', api.request.public_url, - 'v1', '', bookmark=True), - link.make_link('describedby', - 'https://docs.openstack.org', - '/ironic/latest/contributor/', - 'webapi.html', - bookmark=True, type='text/html') - ] - v1.media_types = [MediaType('application/json', - 'application/vnd.openstack.ironic.v1+json')] - v1.chassis = [link.make_link('self', api.request.public_url, - 'chassis', ''), - link.make_link('bookmark', - api.request.public_url, - 'chassis', '', - bookmark=True) - ] - v1.nodes = [link.make_link('self', api.request.public_url, - 'nodes', ''), - link.make_link('bookmark', - api.request.public_url, - 'nodes', '', - bookmark=True) - ] - v1.ports = [link.make_link('self', api.request.public_url, - 'ports', ''), - link.make_link('bookmark', - api.request.public_url, - 'ports', '', - bookmark=True) - ] - if utils.allow_portgroups(): - v1.portgroups = [ - link.make_link('self', api.request.public_url, - 'portgroups', ''), - link.make_link('bookmark', api.request.public_url, - 'portgroups', '', bookmark=True) - ] - v1.drivers = [link.make_link('self', api.request.public_url, - 'drivers', ''), - link.make_link('bookmark', - api.request.public_url, - 'drivers', '', - bookmark=True) - ] - if utils.allow_volume(): - v1.volume = [ - link.make_link('self', - api.request.public_url, - 'volume', ''), - link.make_link('bookmark', - api.request.public_url, - 'volume', '', - bookmark=True) - ] - if utils.allow_ramdisk_endpoints(): - v1.lookup = [link.make_link('self', api.request.public_url, - 'lookup', ''), - link.make_link('bookmark', - api.request.public_url, - 'lookup', '', - bookmark=True) - ] - v1.heartbeat = [link.make_link('self', - api.request.public_url, - 'heartbeat', ''), - link.make_link('bookmark', - api.request.public_url, - 'heartbeat', '', - bookmark=True) - ] - if utils.allow_expose_conductors(): - v1.conductors = [link.make_link('self', - api.request.public_url, - 'conductors', ''), - link.make_link('bookmark', - api.request.public_url, - 'conductors', '', - bookmark=True) - ] - if utils.allow_allocations(): - v1.allocations = [link.make_link('self', - api.request.public_url, - 'allocations', ''), - link.make_link('bookmark', - api.request.public_url, - 'allocations', '', - bookmark=True) - ] - if utils.allow_expose_events(): - v1.events = [link.make_link('self', api.request.public_url, - 'events', ''), - link.make_link('bookmark', - api.request.public_url, - 'events', '', - bookmark=True) - ] - if utils.allow_deploy_templates(): - v1.deploy_templates = [ - link.make_link('self', - api.request.public_url, - 'deploy_templates', ''), - link.make_link('bookmark', - api.request.public_url, - 'deploy_templates', '', - bookmark=True) - ] - v1.version = version.default_version() - return v1 - - -class Controller(rest.RestController): +def v1(): + v1 = { + 'id': "v1", + 'links': [ + link.make_link('self', api.request.public_url, + 'v1', '', bookmark=True), + link.make_link('describedby', + 'https://docs.openstack.org', + '/ironic/latest/contributor/', + 'webapi.html', + bookmark=True, type='text/html') + ], + 'media_types': { + 'base': 'application/json', + 'type': 'application/vnd.openstack.ironic.v1+json' + }, + 'chassis': [ + link.make_link('self', api.request.public_url, + 'chassis', ''), + link.make_link('bookmark', + api.request.public_url, + 'chassis', '', + bookmark=True) + ], + 'nodes': [ + link.make_link('self', api.request.public_url, + 'nodes', ''), + link.make_link('bookmark', + api.request.public_url, + 'nodes', '', + bookmark=True) + ], + 'ports': [ + link.make_link('self', api.request.public_url, + 'ports', ''), + link.make_link('bookmark', + api.request.public_url, + 'ports', '', + bookmark=True) + ], + 'drivers': [ + link.make_link('self', api.request.public_url, + 'drivers', ''), + link.make_link('bookmark', + api.request.public_url, + 'drivers', '', + bookmark=True) + ], + 'version': version.default_version() + } + if utils.allow_portgroups(): + v1['portgroups'] = [ + link.make_link('self', api.request.public_url, + 'portgroups', ''), + link.make_link('bookmark', api.request.public_url, + 'portgroups', '', bookmark=True) + ] + if utils.allow_volume(): + v1['volume'] = [ + link.make_link('self', + api.request.public_url, + 'volume', ''), + link.make_link('bookmark', + api.request.public_url, + 'volume', '', + bookmark=True) + ] + if utils.allow_ramdisk_endpoints(): + v1['lookup'] = [ + link.make_link('self', api.request.public_url, + 'lookup', ''), + link.make_link('bookmark', + api.request.public_url, + 'lookup', '', + bookmark=True) + ] + v1['heartbeat'] = [ + link.make_link('self', + api.request.public_url, + 'heartbeat', ''), + link.make_link('bookmark', + api.request.public_url, + 'heartbeat', '', + bookmark=True) + ] + if utils.allow_expose_conductors(): + v1['conductors'] = [ + link.make_link('self', + api.request.public_url, + 'conductors', ''), + link.make_link('bookmark', + api.request.public_url, + 'conductors', '', + bookmark=True) + ] + if utils.allow_allocations(): + v1['allocations'] = [ + link.make_link('self', + api.request.public_url, + 'allocations', ''), + link.make_link('bookmark', + api.request.public_url, + 'allocations', '', + bookmark=True) + ] + if utils.allow_expose_events(): + v1['events'] = [ + link.make_link('self', api.request.public_url, + 'events', ''), + link.make_link('bookmark', + api.request.public_url, + 'events', '', + bookmark=True) + ] + if utils.allow_deploy_templates(): + v1['deploy_templates'] = [ + link.make_link('self', + api.request.public_url, + 'deploy_templates', ''), + link.make_link('bookmark', + api.request.public_url, + 'deploy_templates', '', + bookmark=True) + ] + return v1 + + +class Controller(object): """Version 1 API controller root.""" - nodes = node.NodesController() - ports = port.PortsController() - portgroups = portgroup.PortgroupsController() - chassis = chassis.ChassisController() - drivers = driver.DriversController() - volume = volume.VolumeController() - lookup = ramdisk.LookupController() - heartbeat = ramdisk.HeartbeatController() - conductors = conductor.ConductorsController() - allocations = allocation.AllocationsController() - events = event.EventsController() - deploy_templates = deploy_template.DeployTemplatesController() - - @expose.expose(V1) - def get(self): - # NOTE: The reason why convert() it's being called for every + _subcontroller_map = { + 'nodes': node.NodesController(), + 'ports': port.PortsController(), + 'portgroups': portgroup.PortgroupsController(), + 'chassis': chassis.ChassisController(), + 'drivers': driver.DriversController(), + 'volume': volume.VolumeController(), + 'lookup': ramdisk.LookupController(), + 'heartbeat': ramdisk.HeartbeatController(), + 'conductors': conductor.ConductorsController(), + 'allocations': allocation.AllocationsController(), + 'events': event.EventsController(), + 'deploy_templates': deploy_template.DeployTemplatesController() + } + + @method.expose() + def index(self): + # NOTE: The reason why v1() it's being called for every # request is because we need to get the host url from # the request object to make the links. - return V1.convert() + self._add_version_attributes() + if api.request.method != "GET": + pecan.abort(http_client.METHOD_NOT_ALLOWED) + + return v1() def _check_version(self, version, headers=None): if headers is None: @@ -279,8 +236,7 @@ class Controller(rest.RestController): 'max': versions.max_version_string()}, headers=headers) - @pecan.expose() - def _route(self, args, request=None): + def _add_version_attributes(self): v = base.Version(api.request.headers, versions.min_version_string(), versions.max_version_string()) @@ -295,7 +251,15 @@ class Controller(rest.RestController): api.response.headers[base.Version.string] = str(v) api.request.version = v - return super(Controller, self)._route(args, request) + @pecan.expose() + def _lookup(self, primary_key, *remainder): + self._add_version_attributes() + + controller = self._subcontroller_map.get(primary_key) + if not controller: + pecan.abort(http_client.NOT_FOUND) + + return controller, remainder __all__ = ('Controller',) diff --git a/ironic/api/controllers/v1/collection.py b/ironic/api/controllers/v1/collection.py index 6e1b1faf3..c669b9309 100644 --- a/ironic/api/controllers/v1/collection.py +++ b/ironic/api/controllers/v1/collection.py @@ -19,6 +19,38 @@ from ironic.api.controllers import link from ironic.api import types as atypes +def has_next(collection, limit): + """Return whether collection has more items.""" + return len(collection) and len(collection) == limit + + +def get_next(collection, limit, url=None, key_field='uuid', **kwargs): + """Return a link to the next subset of the collection.""" + if not has_next(collection, limit): + return None + + fields = kwargs.pop('fields', None) + # NOTE(saga): If fields argument is present in kwargs and not None. It + # is a list so convert it into a comma seperated string. + if fields: + kwargs['fields'] = ','.join(fields) + q_args = ''.join(['%s=%s&' % (key, kwargs[key]) for key in kwargs]) + + last_item = collection[-1] + # handle items which are either objects or dicts + if hasattr(last_item, key_field): + marker = getattr(last_item, key_field) + else: + marker = last_item.get(key_field) + + next_args = '?%(args)slimit=%(limit)d&marker=%(marker)s' % { + 'args': q_args, 'limit': limit, + 'marker': marker} + + return link.make_link('next', api.request.public_url, + url, next_args)['href'] + + class Collection(base.Base): next = str @@ -34,23 +66,13 @@ class Collection(base.Base): def has_next(self, limit): """Return whether collection has more items.""" - return len(self.collection) and len(self.collection) == limit + return has_next(self.collection, limit) def get_next(self, limit, url=None, **kwargs): """Return a link to the next subset of the collection.""" - if not self.has_next(limit): - return atypes.Unset - resource_url = url or self._type - fields = kwargs.pop('fields', None) - # NOTE(saga): If fields argument is present in kwargs and not None. It - # is a list so convert it into a comma seperated string. - if fields: - kwargs['fields'] = ','.join(fields) - q_args = ''.join(['%s=%s&' % (key, kwargs[key]) for key in kwargs]) - next_args = '?%(args)slimit=%(limit)d&marker=%(marker)s' % { - 'args': q_args, 'limit': limit, - 'marker': getattr(self.collection[-1], self.get_key_field())} - - return link.make_link('next', api.request.public_url, - resource_url, next_args)['href'] + the_next = get_next(self.collection, limit, url=resource_url, + key_field=self.get_key_field(), **kwargs) + if the_next is None: + return atypes.Unset + return the_next diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index f75029eaa..cb20fcfd8 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -606,12 +606,13 @@ class ConductorManager(base_manager.BaseConductorManager): node_id, purpose='node rescue') as task: node = task.node - # Record of any pre-existing agent_url should be removed. - utils.remove_agent_url(node) if node.maintenance: raise exception.NodeInMaintenance(op=_('rescuing'), node=node.uuid) + # Record of any pre-existing agent_url should be removed. + utils.wipe_token_and_url(task) + # driver validation may check rescue_password, so save it on the # node early i_info = node.instance_info @@ -758,6 +759,9 @@ class ConductorManager(base_manager.BaseConductorManager): handle_failure(e, _('Failed to unrescue. Exception: %s'), log_func=LOG.exception) + + utils.wipe_token_and_url(task) + if next_state == states.ACTIVE: task.process_event('done') else: @@ -1228,6 +1232,8 @@ class ConductorManager(base_manager.BaseConductorManager): error = (_('Failed to validate power driver interface for node ' '%(node)s. Error: %(msg)s') % {'node': node.uuid, 'msg': e}) + log_traceback = not isinstance(e, exception.IronicException) + LOG.error(error, exc_info=log_traceback) else: try: power_state = task.driver.power.get_power_state(task) @@ -1235,6 +1241,8 @@ class ConductorManager(base_manager.BaseConductorManager): error = (_('Failed to get power state for node ' '%(node)s. Error: %(msg)s') % {'node': node.uuid, 'msg': e}) + log_traceback = not isinstance(e, exception.IronicException) + LOG.error(error, exc_info=log_traceback) if error is None: if power_state != node.power_state: @@ -1246,7 +1254,6 @@ class ConductorManager(base_manager.BaseConductorManager): else: task.process_event('done') else: - LOG.error(error) node.last_error = error task.process_event('fail') diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index c64dc9f5a..b30fdee5e 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -451,16 +451,23 @@ def cleaning_error_handler(task, msg, tear_down_cleaning=True, task.process_event('fail', target_state=target_state) +def wipe_token_and_url(task): + """Remove agent URL and token from the task.""" + info = task.node.driver_internal_info + info.pop('agent_secret_token', None) + info.pop('agent_secret_token_pregenerated', None) + # Remove agent_url since it will be re-asserted + # upon the next deployment attempt. + info.pop('agent_url', None) + task.node.driver_internal_info = info + + def wipe_deploy_internal_info(task): """Remove temporary deployment fields from driver_internal_info.""" - info = task.node.driver_internal_info if not fast_track_able(task): - info.pop('agent_secret_token', None) - info.pop('agent_secret_token_pregenerated', None) - # Remove agent_url since it will be re-asserted - # upon the next deployment attempt. - info.pop('agent_url', None) + wipe_token_and_url(task) # Clear any leftover metadata about deployment. + info = task.node.driver_internal_info info['deploy_steps'] = None info.pop('agent_cached_deploy_steps', None) info.pop('deploy_step_index', None) @@ -473,11 +480,9 @@ def wipe_deploy_internal_info(task): def wipe_cleaning_internal_info(task): """Remove temporary cleaning fields from driver_internal_info.""" - info = task.node.driver_internal_info if not fast_track_able(task): - info.pop('agent_url', None) - info.pop('agent_secret_token', None) - info.pop('agent_secret_token_pregenerated', None) + wipe_token_and_url(task) + info = task.node.driver_internal_info info['clean_steps'] = None info.pop('agent_cached_clean_steps', None) info.pop('clean_step_index', None) diff --git a/ironic/drivers/hardware_type.py b/ironic/drivers/hardware_type.py index 0ec71210e..df5f43782 100644 --- a/ironic/drivers/hardware_type.py +++ b/ironic/drivers/hardware_type.py @@ -42,19 +42,23 @@ class AbstractHardwareType(object, metaclass=abc.ABCMeta): # Required hardware interfaces - @abc.abstractproperty + @property + @abc.abstractmethod def supported_boot_interfaces(self): """List of supported boot interfaces.""" - @abc.abstractproperty + @property + @abc.abstractmethod def supported_deploy_interfaces(self): """List of supported deploy interfaces.""" - @abc.abstractproperty + @property + @abc.abstractmethod def supported_management_interfaces(self): """List of supported management interfaces.""" - @abc.abstractproperty + @property + @abc.abstractmethod def supported_power_interfaces(self): """List of supported power interfaces.""" diff --git a/ironic/drivers/modules/agent_base.py b/ironic/drivers/modules/agent_base.py index 49debc5a9..ee9753dcb 100644 --- a/ironic/drivers/modules/agent_base.py +++ b/ironic/drivers/modules/agent_base.py @@ -72,7 +72,14 @@ VENDOR_PROPERTIES = { 'deploy_forces_oob_reboot': _( 'Whether Ironic should force a reboot of the Node via the out-of-band ' 'channel after deployment is complete. Provides compatibility with ' - 'older deploy ramdisks. Defaults to False. Optional.') + 'older deploy ramdisks. Defaults to False. Optional.'), + 'agent_verify_ca': _( + 'Either a Boolean value, a path to a CA_BUNDLE file or directory with ' + 'certificates of trusted CAs. If set to True ironic will verify ' + 'the agent\'s certificate; if False the driver will ignore verifying ' + 'the SSL certificate. If it\'s a path the driver will use the ' + 'specified certificate or one of the certificates in the ' + 'directory. Defaults to True. Optional'), } __HEARTBEAT_RECORD_ONLY = (states.ENROLL, states.MANAGEABLE, states.AVAILABLE, diff --git a/ironic/drivers/modules/agent_client.py b/ironic/drivers/modules/agent_client.py index d1f3d6759..090007d8f 100644 --- a/ironic/drivers/modules/agent_client.py +++ b/ironic/drivers/modules/agent_client.py @@ -76,6 +76,9 @@ class AgentClient(object): 'params': params, }) + def _get_verify(self, node): + return node.driver_info.get('agent_verify_ca', True) + def _raise_if_typeerror(self, result, node, method): error = result.get('command_error') if error and error.get('type') == 'TypeError': @@ -168,6 +171,7 @@ class AgentClient(object): try: response = self.session.post( url, params=request_params, data=body, + verify=self._get_verify(node), timeout=CONF.agent.command_timeout) except (requests.ConnectionError, requests.Timeout) as e: msg = (_('Failed to connect to the agent running on node %(node)s ' @@ -261,6 +265,7 @@ class AgentClient(object): def _get(): try: return self.session.get(url, + verify=self._get_verify(node), timeout=CONF.agent.command_timeout) except (requests.ConnectionError, requests.Timeout) as e: msg = (_('Failed to connect to the agent running on node ' diff --git a/ironic/drivers/modules/ilo/management.py b/ironic/drivers/modules/ilo/management.py index 45bac59a1..75b5934ea 100644 --- a/ironic/drivers/modules/ilo/management.py +++ b/ironic/drivers/modules/ilo/management.py @@ -524,12 +524,43 @@ class IloManagement(base.ManagementInterface): @base.clean_step(priority=0, abortable=False, argsinfo=_FIRMWARE_UPDATE_SUM_ARGSINFO) def update_firmware_sum(self, task, **kwargs): - """Updates the firmware using Smart Update Manager (SUM). + """Clean step to update the firmware using Smart Update Manager (SUM) :param task: a TaskManager object. :raises: NodeCleaningFailure, on failure to execute of clean step. + :returns: states.CLEANWAIT to signify the step will be completed async + """ + return self._do_update_firmware_sum(task, **kwargs) + + @METRICS.timer('IloManagement.update_firmware_sum') + @base.deploy_step(priority=0, argsinfo=_FIRMWARE_UPDATE_SUM_ARGSINFO) + def flash_firmware_sum(self, task, **kwargs): + """Deploy step to Update the firmware using Smart Update Manager (SUM). + + :param task: a TaskManager object. + :raises: InstanceDeployFailure, on failure to execute of deploy step. + :returns: states.DEPLOYWAIT to signify the step will be completed + async + """ + return self._do_update_firmware_sum(task, **kwargs) + + def _do_update_firmware_sum(self, task, **kwargs): + """Update the firmware using Smart Update Manager (SUM). + + :param task: a TaskManager object. + :raises: NodeCleaningFailure or InstanceDeployFailure, on failure to + execute of clean or deploy step respectively. + :returns: states.CLEANWAIT or states.DEPLOYWAIT to signify the step + will be completed async for clean or deploy step respectively. """ node = task.node + if node.provision_state == states.DEPLOYING: + step = node.deploy_step + step_type = 'deploy' + else: + step = node.clean_step + step_type = 'clean' + # The arguments are validated and sent to the ProliantHardwareManager # to perform SUM based firmware update clean step. firmware_processor.get_and_validate_firmware_image_info(kwargs, @@ -538,24 +569,25 @@ class IloManagement(base.ManagementInterface): url = kwargs['url'] if urlparse.urlparse(url).scheme == 'swift': url = firmware_processor.get_swift_url(urlparse.urlparse(url)) - node.clean_step['args']['url'] = url + step['args']['url'] = url # Insert SPP ISO into virtual media CDROM ilo_common.attach_vmedia(node, 'CDROM', url) - step = node.clean_step - return agent_base.execute_clean_step(task, step) + return agent_base.execute_step(task, step, step_type) @staticmethod + @agent_base.post_deploy_step_hook( + interface='management', step='flash_firmware_sum') @agent_base.post_clean_step_hook( interface='management', step='update_firmware_sum') def _update_firmware_sum_final(task, command): - """Clean step hook after SUM based firmware update operation. + """Deploy/Clean step hook after SUM based firmware update operation. - This method is invoked as a post clean step hook by the Ironic - conductor once firmware update operaion is completed. The clean logs - are collected and stored according to the configured storage backend - when the node is configured to collect the logs. + This method is invoked as a post deploy/clean step hook by the Ironic + conductor once firmware update operaion is completed. The deploy/clean + logs are collected and stored according to the configured storage + backend when the node is configured to collect the logs. :param task: a TaskManager instance. :param command: A command result structure of the SUM based firmware @@ -565,12 +597,16 @@ class IloManagement(base.ManagementInterface): if not _should_collect_logs(command): return + if task.node.provision_state == states.DEPLOYWAIT: + log_data = command['command_result']['deploy_result']['Log Data'] + label = command['command_result']['deploy_step']['step'] + else: + log_data = command['command_result']['clean_result']['Log Data'] + label = command['command_result']['clean_step']['step'] + node = task.node try: - driver_utils.store_ramdisk_logs( - node, - command['command_result']['clean_result']['Log Data'], - label='update_firmware_sum') + driver_utils.store_ramdisk_logs(node, log_data, label=label) except exception.SwiftOperationError as e: LOG.error('Failed to store the logs from the node %(node)s ' 'for "update_firmware_sum" clean step in Swift. ' diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 41c83be61..8a4843136 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -673,7 +673,7 @@ class ISCSIDeploy(agent_base.AgentDeployMixin, agent_base.AgentBaseMixin, return states.DEPLOYWAIT @METRICS.timer('ISCSIDeploy.write_image') - @base.deploy_step(priority=90) + @base.deploy_step(priority=80) @task_manager.require_exclusive_lock def write_image(self, task): """Method invoked when deployed using iSCSI. @@ -701,7 +701,7 @@ class ISCSIDeploy(agent_base.AgentDeployMixin, agent_base.AgentBaseMixin, node.save() @METRICS.timer('ISCSIDeploy.prepare_instance_boot') - @base.deploy_step(priority=80) + @base.deploy_step(priority=60) def prepare_instance_boot(self, task): if not task.driver.storage.should_write_image(task): task.driver.boot.prepare_instance(task) diff --git a/ironic/drivers/modules/snmp.py b/ironic/drivers/modules/snmp.py index 008886b90..ab6c3ade4 100644 --- a/ironic/drivers/modules/snmp.py +++ b/ironic/drivers/modules/snmp.py @@ -554,15 +554,18 @@ class SNMPDriverSimple(SNMPDriverBase): super(SNMPDriverSimple, self).__init__(*args, **kwargs) self.oid = self._snmp_oid() - @abc.abstractproperty + @property + @abc.abstractmethod def oid_device(self): """Device dependent portion of the power state object OID.""" - @abc.abstractproperty + @property + @abc.abstractmethod def value_power_on(self): """Value representing power on state.""" - @abc.abstractproperty + @property + @abc.abstractmethod def value_power_off(self): """Value representing power off state.""" diff --git a/ironic/tests/unit/api/controllers/v1/test_root.py b/ironic/tests/unit/api/controllers/v1/test_root.py index b3e58b817..78d3053e4 100644 --- a/ironic/tests/unit/api/controllers/v1/test_root.py +++ b/ironic/tests/unit/api/controllers/v1/test_root.py @@ -17,6 +17,7 @@ from unittest import mock from webob import exc as webob_exc from ironic.api.controllers import v1 as v1_api +from ironic.api.controllers.v1 import versions from ironic.tests import base as test_base from ironic.tests.unit.api import base as api_base @@ -28,6 +29,130 @@ class TestV1Routing(api_base.BaseApiTest): mock.ANY, mock.ANY) + def test_min_version(self): + response = self.get_json( + '/', + headers={ + 'Accept': 'application/json', + 'X-OpenStack-Ironic-API-Version': + versions.min_version_string() + }) + self.assertEqual({ + 'id': 'v1', + 'links': [ + {'href': 'http://localhost/v1/', 'rel': 'self'}, + {'href': 'https://docs.openstack.org//ironic/latest' + '/contributor//webapi.html', + 'rel': 'describedby', 'type': 'text/html'} + ], + 'media_types': { + 'base': 'application/json', + 'type': 'application/vnd.openstack.ironic.v1+json' + }, + 'version': { + 'id': 'v1', + 'links': [{'href': 'http://localhost/v1/', 'rel': 'self'}], + 'status': 'CURRENT', + 'min_version': versions.min_version_string(), + 'version': versions.max_version_string() + }, + 'chassis': [ + {'href': 'http://localhost/v1/chassis/', 'rel': 'self'}, + {'href': 'http://localhost/chassis/', 'rel': 'bookmark'} + ], + 'nodes': [ + {'href': 'http://localhost/v1/nodes/', 'rel': 'self'}, + {'href': 'http://localhost/nodes/', 'rel': 'bookmark'} + ], + 'ports': [ + {'href': 'http://localhost/v1/ports/', 'rel': 'self'}, + {'href': 'http://localhost/ports/', 'rel': 'bookmark'} + ], + 'drivers': [ + {'href': 'http://localhost/v1/drivers/', 'rel': 'self'}, + {'href': 'http://localhost/drivers/', 'rel': 'bookmark'} + ], + }, response) + + def test_max_version(self): + response = self.get_json( + '/', + headers={ + 'Accept': 'application/json', + 'X-OpenStack-Ironic-API-Version': + versions.max_version_string() + }) + self.assertEqual({ + 'id': 'v1', + 'links': [ + {'href': 'http://localhost/v1/', 'rel': 'self'}, + {'href': 'https://docs.openstack.org//ironic/latest' + '/contributor//webapi.html', + 'rel': 'describedby', 'type': 'text/html'} + ], + 'media_types': { + 'base': 'application/json', + 'type': 'application/vnd.openstack.ironic.v1+json' + }, + 'version': { + 'id': 'v1', + 'links': [{'href': 'http://localhost/v1/', 'rel': 'self'}], + 'status': 'CURRENT', + 'min_version': versions.min_version_string(), + 'version': versions.max_version_string() + }, + 'allocations': [ + {'href': 'http://localhost/v1/allocations/', 'rel': 'self'}, + {'href': 'http://localhost/allocations/', 'rel': 'bookmark'} + ], + 'chassis': [ + {'href': 'http://localhost/v1/chassis/', 'rel': 'self'}, + {'href': 'http://localhost/chassis/', 'rel': 'bookmark'} + ], + 'conductors': [ + {'href': 'http://localhost/v1/conductors/', 'rel': 'self'}, + {'href': 'http://localhost/conductors/', 'rel': 'bookmark'} + ], + 'deploy_templates': [ + {'href': 'http://localhost/v1/deploy_templates/', + 'rel': 'self'}, + {'href': 'http://localhost/deploy_templates/', + 'rel': 'bookmark'} + ], + 'drivers': [ + {'href': 'http://localhost/v1/drivers/', 'rel': 'self'}, + {'href': 'http://localhost/drivers/', 'rel': 'bookmark'} + ], + 'events': [ + {'href': 'http://localhost/v1/events/', 'rel': 'self'}, + {'href': 'http://localhost/events/', 'rel': 'bookmark'} + ], + 'heartbeat': [ + {'href': 'http://localhost/v1/heartbeat/', 'rel': 'self'}, + {'href': 'http://localhost/heartbeat/', 'rel': 'bookmark'} + ], + 'lookup': [ + {'href': 'http://localhost/v1/lookup/', 'rel': 'self'}, + {'href': 'http://localhost/lookup/', 'rel': 'bookmark'} + ], + 'nodes': [ + {'href': 'http://localhost/v1/nodes/', 'rel': 'self'}, + {'href': 'http://localhost/nodes/', 'rel': 'bookmark'} + ], + 'portgroups': [ + {'href': 'http://localhost/v1/portgroups/', 'rel': 'self'}, + {'href': 'http://localhost/portgroups/', 'rel': 'bookmark'} + ], + 'ports': [ + {'href': 'http://localhost/v1/ports/', 'rel': 'self'}, + {'href': 'http://localhost/ports/', 'rel': 'bookmark'} + ], + 'volume': [ + {'href': 'http://localhost/v1/volume/', 'rel': 'self'}, + {'href': 'http://localhost/volume/', 'rel': 'bookmark'} + ] + }, response) + class TestCheckVersions(test_base.TestCase): diff --git a/ironic/tests/unit/api/test_root.py b/ironic/tests/unit/api/test_root.py index 9a512d7ad..b784762f3 100644 --- a/ironic/tests/unit/api/test_root.py +++ b/ironic/tests/unit/api/test_root.py @@ -44,9 +44,10 @@ class TestRoot(base.BaseApiTest): self.assertNotIn('<html', response.json['error_message']) def test_no_html_errors2(self): - response = self.delete('/v1', expect_errors=True) + response = self.delete('/', expect_errors=True) self.assertEqual(http_client.METHOD_NOT_ALLOWED, response.status_int) - self.assertIn('Not Allowed', response.json['error_message']) + self.assertIn('malformed or otherwise incorrect', + response.json['error_message']) self.assertNotIn('<html', response.json['error_message']) @@ -68,8 +69,8 @@ class TestV1Root(base.BaseApiTest): expected_resources = (['chassis', 'drivers', 'nodes', 'ports'] + additional_expected_resources) self.assertEqual(sorted(expected_resources), sorted(actual_resources)) - self.assertIn({'type': 'application/vnd.openstack.ironic.v1+json', - 'base': 'application/json'}, data['media_types']) + self.assertEqual({'type': 'application/vnd.openstack.ironic.v1+json', + 'base': 'application/json'}, data['media_types']) version1 = data['version'] self.assertEqual('v1', version1['id']) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index f1b0990bf..7ab03e175 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -2767,11 +2767,14 @@ class DoNodeRescueTestCase(mgr_utils.CommonMixIn, mgr_utils.ServiceSetUpMixin, @mock.patch('ironic.conductor.task_manager.acquire', autospec=True) def test_do_node_rescue(self, mock_acquire): self._start_service() + dii = {'agent_secret_token': 'token', + 'agent_url': 'http://url', + 'other field': 'value'} task = self._create_task( node_attrs=dict(driver='fake-hardware', provision_state=states.ACTIVE, instance_info={}, - driver_internal_info={'agent_url': 'url'})) + driver_internal_info=dii)) mock_acquire.side_effect = self._get_acquire_side_effect(task) self.service.do_node_rescue(self.context, task.node.uuid, "password") @@ -2782,7 +2785,8 @@ class DoNodeRescueTestCase(mgr_utils.CommonMixIn, mgr_utils.ServiceSetUpMixin, err_handler=conductor_utils.spawn_rescue_error_handler) self.assertIn('rescue_password', task.node.instance_info) self.assertIn('hashed_rescue_password', task.node.instance_info) - self.assertNotIn('agent_url', task.node.driver_internal_info) + self.assertEqual({'other field': 'value'}, + task.node.driver_internal_info) def test_do_node_rescue_invalid_state(self): self._start_service() @@ -2985,16 +2989,22 @@ class DoNodeRescueTestCase(mgr_utils.CommonMixIn, mgr_utils.ServiceSetUpMixin, autospec=True) def test__do_node_unrescue(self, mock_unrescue): self._start_service() + dii = {'agent_url': 'http://url', + 'agent_secret_token': 'token', + 'other field': 'value'} node = obj_utils.create_test_node(self.context, driver='fake-hardware', provision_state=states.UNRESCUING, target_provision_state=states.ACTIVE, - instance_info={}) + instance_info={}, + driver_internal_info=dii) with task_manager.TaskManager(self.context, node.uuid) as task: mock_unrescue.return_value = states.ACTIVE self.service._do_node_unrescue(task) node.refresh() self.assertEqual(states.ACTIVE, node.provision_state) self.assertEqual(states.NOSTATE, node.target_provision_state) + self.assertEqual({'other field': 'value'}, + node.driver_internal_info) @mock.patch.object(manager, 'LOG', autospec=True) @mock.patch('ironic.drivers.modules.fake.FakeRescue.unrescue', @@ -5759,7 +5769,7 @@ class ManagerTestProperties(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): enabled_power_interfaces=['ipmitool'], enabled_management_interfaces=['ipmitool'], enabled_console_interfaces=['ipmitool-socat']) - expected = ['ipmi_address', 'ipmi_terminal_port', + expected = ['agent_verify_ca', 'ipmi_address', 'ipmi_terminal_port', 'ipmi_password', 'ipmi_port', 'ipmi_priv_level', 'ipmi_username', 'ipmi_bridging', 'ipmi_transit_channel', 'ipmi_transit_address', 'ipmi_target_channel', @@ -5774,7 +5784,7 @@ class ManagerTestProperties(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): def test_driver_properties_snmp(self): self.config(enabled_hardware_types='snmp', enabled_power_interfaces=['snmp']) - expected = ['deploy_kernel', 'deploy_ramdisk', + expected = ['agent_verify_ca', 'deploy_kernel', 'deploy_ramdisk', 'force_persistent_boot_device', 'rescue_kernel', 'rescue_ramdisk', 'snmp_driver', 'snmp_address', 'snmp_port', 'snmp_version', @@ -5795,9 +5805,9 @@ class ManagerTestProperties(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): enabled_boot_interfaces=['ilo-virtual-media'], enabled_inspect_interfaces=['ilo'], enabled_console_interfaces=['ilo']) - expected = ['ilo_address', 'ilo_username', 'ilo_password', - 'client_port', 'client_timeout', 'ilo_deploy_iso', - 'console_port', 'ilo_change_password', + expected = ['agent_verify_ca', 'ilo_address', 'ilo_username', + 'ilo_password', 'client_port', 'client_timeout', + 'ilo_deploy_iso', 'console_port', 'ilo_change_password', 'ca_file', 'snmp_auth_user', 'snmp_auth_prot_password', 'snmp_auth_priv_password', 'snmp_auth_protocol', 'snmp_auth_priv_protocol', 'deploy_forces_oob_reboot'] @@ -5825,7 +5835,7 @@ class ManagerTestHardwareTypeProperties(mgr_utils.ServiceSetUpMixin, self.assertEqual(sorted(expected), sorted(properties)) def test_hardware_type_properties_manual_management(self): - expected = ['deploy_kernel', 'deploy_ramdisk', + expected = ['agent_verify_ca', 'deploy_kernel', 'deploy_ramdisk', 'force_persistent_boot_device', 'deploy_forces_oob_reboot', 'rescue_kernel', 'rescue_ramdisk'] self._check_hardware_type_properties('manual-management', expected) diff --git a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py index cef1fabd4..17ab45786 100644 --- a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py +++ b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py @@ -576,7 +576,7 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase): def test_get_properties(self): self.assertEqual( set(list(ansible_deploy.COMMON_PROPERTIES) - + ['deploy_forces_oob_reboot']), + + ['agent_verify_ca', 'deploy_forces_oob_reboot']), set(self.driver.get_properties())) @mock.patch.object(deploy_utils, 'check_for_missing_params', diff --git a/ironic/tests/unit/drivers/modules/ilo/test_management.py b/ironic/tests/unit/drivers/modules/ilo/test_management.py index e7cb060c9..26b683058 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_management.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_management.py @@ -827,177 +827,388 @@ class IloManagementTestCase(test_common.BaseIloTest): @mock.patch.object(ilo_common, 'attach_vmedia', spec_set=True, autospec=True) - @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) - def test_update_firmware_sum_mode_with_component( - self, execute_mock, attach_vmedia_mock): + @mock.patch.object(agent_base, 'execute_step', autospec=True) + def _test_write_firmware_sum_mode_with_component( + self, execute_mock, attach_vmedia_mock, step_type='clean'): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - execute_mock.return_value = states.CLEANWAIT # | GIVEN | firmware_update_args = { 'url': 'http://any_url', 'checksum': 'xxxx', 'component': ['CP02345.scexe', 'CP02567.exe']} - clean_step = {'step': 'update_firmware', - 'interface': 'management', - 'args': firmware_update_args} - task.node.clean_step = clean_step + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + step['step'] = 'update_firmware_sum' + task.node.provision_state = states.CLEANING + execute_mock.return_value = states.CLEANWAIT + task.node.clean_step = step + func = task.driver.management.update_firmware_sum + exp_ret_state = states.CLEANWAIT + else: + step['step'] = 'flash_firmware_sum' + task.node.provision_state = states.DEPLOYING + execute_mock.return_value = states.DEPLOYWAIT + task.node.deploy_step = step + func = task.driver.management.flash_firmware_sum + exp_ret_state = states.DEPLOYWAIT # | WHEN | - return_value = task.driver.management.update_firmware_sum( - task, **firmware_update_args) + return_value = func(task, **firmware_update_args) # | THEN | attach_vmedia_mock.assert_any_call( task.node, 'CDROM', 'http://any_url') - self.assertEqual(states.CLEANWAIT, return_value) - execute_mock.assert_called_once_with(task, clean_step) + self.assertEqual(exp_ret_state, return_value) + execute_mock.assert_called_once_with(task, step, step_type) + + def test_update_firmware_sum_mode_with_component(self): + self._test_write_firmware_sum_mode_with_component(step_type='clean') + + def test_flash_firmware_sum_mode_with_component(self): + self._test_write_firmware_sum_mode_with_component(step_type='deploy') @mock.patch.object(ilo_common, 'attach_vmedia', spec_set=True, autospec=True) @mock.patch.object(ilo_management.firmware_processor, 'get_swift_url', autospec=True) - @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) - def test_update_firmware_sum_mode_swift_url( - self, execute_mock, swift_url_mock, attach_vmedia_mock): + @mock.patch.object(agent_base, 'execute_step', autospec=True) + def _test_write_firmware_sum_mode_swift_url( + self, execute_mock, swift_url_mock, attach_vmedia_mock, + step_type='clean'): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - swift_url_mock.return_value = "http://path-to-file" - execute_mock.return_value = states.CLEANWAIT # | GIVEN | + swift_url_mock.return_value = "http://path-to-file" firmware_update_args = { 'url': 'swift://container/object', 'checksum': 'xxxx', 'components': ['CP02345.scexe', 'CP02567.exe']} - clean_step = {'step': 'update_firmware', - 'interface': 'management', - 'args': firmware_update_args} - task.node.clean_step = clean_step + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + task.node.provision_state = states.CLEANING + execute_mock.return_value = states.CLEANWAIT + step['step'] = 'update_firmware_sum', + task.node.clean_step = step + func = task.driver.management.update_firmware_sum + exp_ret_state = states.CLEANWAIT + args_data = task.node.clean_step['args'] + else: + task.node.provision_state = states.DEPLOYING + execute_mock.return_value = states.DEPLOYWAIT + step['step'] = 'flash_firmware_sum', + task.node.deploy_step = step + func = task.driver.management.flash_firmware_sum + exp_ret_state = states.DEPLOYWAIT + args_data = task.node.deploy_step['args'] # | WHEN | - return_value = task.driver.management.update_firmware_sum( - task, **firmware_update_args) + return_value = func(task, **firmware_update_args) # | THEN | attach_vmedia_mock.assert_any_call( task.node, 'CDROM', 'http://path-to-file') - self.assertEqual(states.CLEANWAIT, return_value) - self.assertEqual(task.node.clean_step['args']['url'], - "http://path-to-file") + self.assertEqual(exp_ret_state, return_value) + self.assertEqual(args_data['url'], "http://path-to-file") + + def test_write_firmware_sum_mode_swift_url_clean(self): + self._test_write_firmware_sum_mode_swift_url(step_type='clean') + + def test_write_firmware_sum_mode_swift_url_deploy(self): + self._test_write_firmware_sum_mode_swift_url(step_type='deploy') @mock.patch.object(ilo_common, 'attach_vmedia', spec_set=True, autospec=True) - @mock.patch.object(agent_base, 'execute_clean_step', autospec=True) - def test_update_firmware_sum_mode_without_component( - self, execute_mock, attach_vmedia_mock): + @mock.patch.object(agent_base, 'execute_step', autospec=True) + def _test_write_firmware_sum_mode_without_component( + self, execute_mock, attach_vmedia_mock, step_type='clean'): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - execute_mock.return_value = states.CLEANWAIT # | GIVEN | firmware_update_args = { 'url': 'any_valid_url', 'checksum': 'xxxx'} - clean_step = {'step': 'update_firmware', - 'interface': 'management', - 'args': firmware_update_args} - task.node.clean_step = clean_step + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + task.node.provision_state = states.CLEANING + execute_mock.return_value = states.CLEANWAIT + step['step'] = 'update_firmware_sum' + task.node.clean_step = step + func = task.driver.management.update_firmware_sum + exp_ret_state = states.CLEANWAIT + else: + task.node.provision_state = states.DEPLOYING + execute_mock.return_value = states.DEPLOYWAIT + step['step'] = 'flash_firmware_sum' + task.node.deploy_step = step + func = task.driver.management.flash_firmware_sum + exp_ret_state = states.DEPLOYWAIT # | WHEN | - return_value = task.driver.management.update_firmware_sum( - task, **firmware_update_args) + return_value = func(task, **firmware_update_args) # | THEN | attach_vmedia_mock.assert_any_call( task.node, 'CDROM', 'any_valid_url') - self.assertEqual(states.CLEANWAIT, return_value) - execute_mock.assert_called_once_with(task, clean_step) + self.assertEqual(exp_ret_state, return_value) + execute_mock.assert_called_once_with(task, step, step_type) - def test_update_firmware_sum_mode_invalid_component(self): + def test_write_firmware_sum_mode_without_component_clean(self): + self._test_write_firmware_sum_mode_without_component( + step_type='clean') + + def test_write_firmware_sum_mode_without_component_deploy(self): + self._test_write_firmware_sum_mode_without_component( + step_type='deploy') + + def _test_write_firmware_sum_mode_invalid_component(self, + step_type='clean'): + # | GIVEN | + firmware_update_args = { + 'url': 'any_valid_url', + 'checksum': 'xxxx', + 'components': ['CP02345']} with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - # | GIVEN | - firmware_update_args = { - 'url': 'any_valid_url', - 'checksum': 'xxxx', - 'components': ['CP02345']} # | WHEN & THEN | + if step_type == 'clean': + func = task.driver.management.update_firmware_sum + else: + func = task.driver.management.flash_firmware_sum self.assertRaises(exception.InvalidParameterValue, - task.driver.management.update_firmware_sum, - task, - **firmware_update_args) + func, task, **firmware_update_args) + + def test_write_firmware_sum_mode_invalid_component_clean(self): + self._test_write_firmware_sum_mode_invalid_component( + step_type='clean') + + def test_write_firmware_sum_mode_invalid_component_deploy(self): + self._test_write_firmware_sum_mode_invalid_component( + step_type='deploy') @mock.patch.object(driver_utils, 'store_ramdisk_logs') - def test__update_firmware_sum_final_with_logs(self, store_mock): + def _test__write_firmware_sum_final_with_logs(self, store_mock, + step_type='clean'): self.config(deploy_logs_collect='always', group='agent') - command = {'command_status': 'SUCCEEDED', - 'command_result': { - 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} - } + firmware_update_args = { + 'url': 'any_valid_url', + 'checksum': 'xxxx'} + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + step['step'] = 'update_firmware_sum' + node_state = states.CLEANWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'clean_step': step, + } + } + exp_label = 'update_firmware_sum' + else: + step['step'] = 'flash_firmware_sum' + node_state = states.DEPLOYWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'deploy_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'deploy_step': step, + } + } + exp_label = 'flash_firmware_sum' with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + task.node.provision_state = node_state task.driver.management._update_firmware_sum_final( task, command) store_mock.assert_called_once_with(task.node, 'aaaabbbbcccdddd', - label='update_firmware_sum') + label=exp_label) + + def test__write_firmware_sum_final_with_logs_clean(self): + self._test__write_firmware_sum_final_with_logs(step_type='clean') + + def test__write_firmware_sum_final_with_logs_deploy(self): + self._test__write_firmware_sum_final_with_logs(step_type='deploy') @mock.patch.object(driver_utils, 'store_ramdisk_logs') - def test__update_firmware_sum_final_without_logs(self, store_mock): + def _test__write_firmware_sum_final_without_logs(self, store_mock, + step_type='clean'): self.config(deploy_logs_collect='on_failure', group='agent') - command = {'command_status': 'SUCCEEDED', - 'command_result': { - 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} - } + firmware_update_args = { + 'url': 'any_valid_url', + 'checksum': 'xxxx'} + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + step['step'] = 'update_firmware_sum' + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'clean_step': step, + } + } + else: + step['step'] = 'flash_firmware_sum' + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'deploy_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'deploy_step': step, + } + } with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.driver.management._update_firmware_sum_final( task, command) self.assertFalse(store_mock.called) + def test__write_firmware_sum_final_without_logs_clean(self): + self._test__write_firmware_sum_final_without_logs(step_type='clean') + + def test__write_firmware_sum_final_without_logs_deploy(self): + self._test__write_firmware_sum_final_without_logs(step_type='deploy') + @mock.patch.object(ilo_management, 'LOG', spec_set=True, autospec=True) @mock.patch.object(driver_utils, 'store_ramdisk_logs') - def test__update_firmware_sum_final_swift_error(self, store_mock, - log_mock): + def _test__write_firmware_sum_final_swift_error(self, store_mock, + log_mock, + step_type='clean'): self.config(deploy_logs_collect='always', group='agent') - command = {'command_status': 'SUCCEEDED', - 'command_result': { - 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} - } + firmware_update_args = { + 'url': 'any_valid_url', + 'checksum': 'xxxx'} + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + step['step'] = 'update_firmware_sum' + node_state = states.CLEANWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'clean_step': step, + } + } + else: + step['step'] = 'flash_firmware_sum' + node_state = states.DEPLOYWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'deploy_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'deploy_step': step, + } + } store_mock.side_effect = exception.SwiftOperationError('Error') with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + task.node.provision_state = node_state task.driver.management._update_firmware_sum_final( task, command) self.assertTrue(log_mock.error.called) + def test__write_firmware_sum_final_swift_error_clean(self): + self._test__write_firmware_sum_final_swift_error(step_type='clean') + + def test__write_firmware_sum_final_swift_error_deploy(self): + self._test__write_firmware_sum_final_swift_error(step_type='deploy') + @mock.patch.object(ilo_management, 'LOG', spec_set=True, autospec=True) @mock.patch.object(driver_utils, 'store_ramdisk_logs') - def test__update_firmware_sum_final_environment_error(self, store_mock, - log_mock): + def _test__write_firmware_sum_final_environment_error(self, store_mock, + log_mock, + step_type='clean'): self.config(deploy_logs_collect='always', group='agent') - command = {'command_status': 'SUCCEEDED', - 'command_result': { - 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} - } + firmware_update_args = { + 'url': 'any_valid_url', + 'checksum': 'xxxx'} + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + step['step'] = 'update_firmware_sum' + node_state = states.CLEANWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'clean_step': step, + } + } + else: + step['step'] = 'flash_firmware_sum' + node_state = states.DEPLOYWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'deploy_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'deploy_step': step, + } + } store_mock.side_effect = EnvironmentError('Error') with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + task.node.provision_state = node_state task.driver.management._update_firmware_sum_final( task, command) self.assertTrue(log_mock.exception.called) + def test__write_firmware_sum_final_environment_error_clean(self): + self._test__write_firmware_sum_final_environment_error( + step_type='clean') + + def test__write_firmware_sum_final_environment_error_deploy(self): + self._test__write_firmware_sum_final_environment_error( + step_type='deploy') + @mock.patch.object(ilo_management, 'LOG', spec_set=True, autospec=True) @mock.patch.object(driver_utils, 'store_ramdisk_logs') - def test__update_firmware_sum_final_unknown_exception(self, store_mock, - log_mock): + def _test__write_firmware_sum_final_unknown_exception(self, store_mock, + log_mock, + step_type='clean'): self.config(deploy_logs_collect='always', group='agent') - command = {'command_status': 'SUCCEEDED', - 'command_result': { - 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} - } + firmware_update_args = { + 'url': 'any_valid_url', + 'checksum': 'xxxx'} + step = {'interface': 'management', + 'args': firmware_update_args} + if step_type == 'clean': + step['step'] = 'update_firmware_sum' + node_state = states.CLEANWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'clean_step': step, + } + } + else: + step['step'] = 'flash_firmware_sum' + node_state = states.DEPLOYWAIT + command = { + 'command_status': 'SUCCEEDED', + 'command_result': { + 'deploy_result': {'Log Data': 'aaaabbbbcccdddd'}, + 'deploy_step': step, + } + } store_mock.side_effect = Exception('Error') with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + task.node.provision_state = node_state task.driver.management._update_firmware_sum_final( task, command) self.assertTrue(log_mock.exception.called) + def test__write_firmware_sum_final_unknown_exception_clean(self): + self._test__write_firmware_sum_final_unknown_exception( + step_type='clean') + + def test__write_firmware_sum_final_unknown_exception_deploy(self): + self._test__write_firmware_sum_final_unknown_exception( + step_type='deploy') + @mock.patch.object(ilo_common, 'get_ilo_object', spec_set=True, autospec=True) def test_set_iscsi_boot_target_with_auth(self, get_ilo_object_mock): diff --git a/ironic/tests/unit/drivers/modules/storage/test_cinder.py b/ironic/tests/unit/drivers/modules/storage/test_cinder.py index 6ec9c317f..165ce7c43 100644 --- a/ironic/tests/unit/drivers/modules/storage/test_cinder.py +++ b/ironic/tests/unit/drivers/modules/storage/test_cinder.py @@ -368,7 +368,7 @@ class CinderInterfaceTestCase(db_base.DbTestCase): @mock.patch.object(cinder_common, 'detach_volumes', autospec=True) @mock.patch.object(cinder_common, 'attach_volumes', autospec=True) - @mock.patch.object(cinder, 'LOG') + @mock.patch.object(cinder, 'LOG', autospec=True) def test_attach_detach_volumes_no_volumes(self, mock_log, mock_attach, mock_detach): with task_manager.acquire(self.context, self.node.id) as task: @@ -400,7 +400,8 @@ class CinderInterfaceTestCase(db_base.DbTestCase): @mock.patch.object(cinder_common, 'detach_volumes', autospec=True) @mock.patch.object(cinder_common, 'attach_volumes', autospec=True) @mock.patch.object(cinder, 'LOG', autospec=True) - @mock.patch.object(objects.volume_target.VolumeTarget, 'list_by_volume_id') + @mock.patch.object(objects.volume_target.VolumeTarget, 'list_by_volume_id', + autospec=True) def test_attach_detach_called_with_target_and_connector(self, mock_target_list, mock_log, @@ -514,7 +515,7 @@ class CinderInterfaceTestCase(db_base.DbTestCase): self.assertEqual(1, mock_log.warning.call_count) @mock.patch.object(cinder_common, 'detach_volumes', autospec=True) - @mock.patch.object(cinder, 'LOG') + @mock.patch.object(cinder, 'LOG', autospec=True) def test_detach_volumes_failure_raises_exception(self, mock_log, mock_detach): diff --git a/ironic/tests/unit/drivers/modules/test_agent_client.py b/ironic/tests/unit/drivers/modules/test_agent_client.py index 69aa03495..1f8cc1f6a 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_client.py +++ b/ironic/tests/unit/drivers/modules/test_agent_client.py @@ -62,13 +62,15 @@ class MockNode(object): 'hardware_manager_version': {'generic': '1'} } self.instance_info = {} + self.driver_info = {} def as_dict(self, secure=False): assert secure, 'agent_client must pass secure=True' return { 'uuid': self.uuid, 'driver_internal_info': self.driver_internal_info, - 'instance_info': self.instance_info + 'instance_info': self.instance_info, + 'driver_info': self.driver_info, } @@ -117,7 +119,8 @@ class TestAgentClient(base.TestCase): url, data=body, params={'wait': 'false'}, - timeout=60) + timeout=60, + verify=True) def test__command_fail_json(self): response_text = 'this be not json matey!' @@ -137,7 +140,8 @@ class TestAgentClient(base.TestCase): url, data=body, params={'wait': 'false'}, - timeout=60) + timeout=60, + verify=True) def test__command_fail_post(self): error = 'Boom' @@ -177,7 +181,8 @@ class TestAgentClient(base.TestCase): url, data=mock.ANY, params={'wait': 'false'}, - timeout=60) + timeout=60, + verify=True) self.assertEqual(3, self.client.session.post.call_count) def test__command_error_code(self): @@ -198,7 +203,8 @@ class TestAgentClient(base.TestCase): url, data=body, params={'wait': 'false'}, - timeout=60) + timeout=60, + verify=True) def test__command_error_code_okay_error_typeerror_embedded(self): response_data = {"faultstring": "you dun goofd", @@ -218,7 +224,29 @@ class TestAgentClient(base.TestCase): url, data=body, params={'wait': 'false'}, - timeout=60) + timeout=60, + verify=True) + + def test__command_verify(self): + response_data = {'status': 'ok'} + self.client.session.post.return_value = MockResponse(response_data) + method = 'standby.run_image' + image_info = {'image_id': 'test_image'} + params = {'image_info': image_info} + + self.node.driver_info['agent_verify_ca'] = '/path/to/agent.crt' + + url = self.client._get_command_url(self.node) + body = self.client._get_command_body(method, params) + + response = self.client._command(self.node, method, params) + self.assertEqual(response, response_data) + self.client.session.post.assert_called_once_with( + url, + data=body, + params={'wait': 'false'}, + timeout=60, + verify='/path/to/agent.crt') @mock.patch('time.sleep', lambda seconds: None) def test__command_poll(self): @@ -247,8 +275,10 @@ class TestAgentClient(base.TestCase): url, data=body, params={'wait': 'false'}, - timeout=60) - self.client.session.get.assert_called_with(url, timeout=60) + timeout=60, + verify=True) + self.client.session.get.assert_called_with(url, timeout=60, + verify=True) def test_get_commands_status(self): with mock.patch.object(self.client.session, 'get', @@ -262,7 +292,8 @@ class TestAgentClient(base.TestCase): '%(agent_url)s/%(api_version)s/commands' % { 'agent_url': agent_url, 'api_version': CONF.agent.agent_api_version}, - timeout=CONF.agent.command_timeout) + timeout=CONF.agent.command_timeout, + verify=True) def test_get_commands_status_retries(self): res = mock.MagicMock(spec_set=['json']) @@ -281,6 +312,23 @@ class TestAgentClient(base.TestCase): retry_connection=False) self.assertEqual(1, self.client.session.get.call_count) + def test_get_commands_status_verify(self): + self.node.driver_info['agent_verify_ca'] = '/path/to/agent.crt' + + with mock.patch.object(self.client.session, 'get', + autospec=True) as mock_get: + res = mock.MagicMock(spec_set=['json']) + res.json.return_value = {'commands': []} + mock_get.return_value = res + self.assertEqual([], self.client.get_commands_status(self.node)) + agent_url = self.node.driver_internal_info.get('agent_url') + mock_get.assert_called_once_with( + '%(agent_url)s/%(api_version)s/commands' % { + 'agent_url': agent_url, + 'api_version': CONF.agent.agent_api_version}, + timeout=CONF.agent.command_timeout, + verify='/path/to/agent.crt') + def test_prepare_image(self): self.client._command = mock.MagicMock(spec_set=[]) image_info = {'image_id': 'image'} @@ -500,7 +548,8 @@ class TestAgentClient(base.TestCase): data=body, params={'wait': 'false', 'agent_token': 'magical'}, - timeout=60) + timeout=60, + verify=True) class TestAgentClientAttempts(base.TestCase): @@ -553,7 +602,8 @@ class TestAgentClientAttempts(base.TestCase): self.client._get_command_url(self.node), data=self.client._get_command_body(method, params), params={'wait': 'false'}, - timeout=60) + timeout=60, + verify=True) @mock.patch.object(retrying.time, 'sleep', autospec=True) def test__command_succeed_after_one_timeout(self, mock_sleep): @@ -574,4 +624,5 @@ class TestAgentClientAttempts(base.TestCase): self.client._get_command_url(self.node), data=self.client._get_command_body(method, params), params={'wait': 'false'}, - timeout=60) + timeout=60, + verify=True) diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index 0923df714..b6eb12771 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -616,7 +616,8 @@ class ISCSIDeployTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: props = task.driver.deploy.get_properties() - self.assertEqual(['deploy_forces_oob_reboot'], list(props)) + self.assertEqual({'agent_verify_ca', 'deploy_forces_oob_reboot'}, + set(props)) @mock.patch.object(iscsi_deploy, 'validate', autospec=True) @mock.patch.object(deploy_utils, 'validate_capabilities', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/xclarity/test_management.py b/ironic/tests/unit/drivers/modules/xclarity/test_management.py index 6e993f93f..f63933c90 100644 --- a/ironic/tests/unit/drivers/modules/xclarity/test_management.py +++ b/ironic/tests/unit/drivers/modules/xclarity/test_management.py @@ -61,7 +61,7 @@ class XClarityManagementDriverTestCase(db_base.DbTestCase): self.assertEqual(expected, driver.get_properties()) @mock.patch.object(management.XClarityManagement, 'get_boot_device', - return_value='pxe') + return_value='pxe', autospec=True) def test_set_boot_device(self, mock_get_boot_device, mock_get_xc_client): with task_manager.acquire(self.context, self.node.uuid) as task: @@ -95,7 +95,8 @@ class XClarityManagementDriverTestCase(db_base.DbTestCase): @mock.patch.object( management.XClarityManagement, 'get_boot_device', - return_value={'boot_device': 'pxe', 'persistent': False}) + return_value={'boot_device': 'pxe', 'persistent': False}, + autospec=True) def test_get_boot_device(self, mock_get_boot_device, mock_get_xc_client): reference = {'boot_device': 'pxe', 'persistent': False} with task_manager.acquire(self.context, self.node.uuid) as task: diff --git a/ironic/tests/unit/drivers/modules/xclarity/test_power.py b/ironic/tests/unit/drivers/modules/xclarity/test_power.py index af4b85155..e12ee837a 100644 --- a/ironic/tests/unit/drivers/modules/xclarity/test_power.py +++ b/ironic/tests/unit/drivers/modules/xclarity/test_power.py @@ -66,10 +66,10 @@ class XClarityPowerDriverTestCase(db_base.DbTestCase): mock_validate_driver_info.assert_called_with(task.node) @mock.patch.object(power.XClarityPower, 'get_power_state', - return_value=STATE_POWER_ON) + return_value=STATE_POWER_ON, autospec=True) def test_get_power_state(self, mock_get_power_state, mock_get_xc_client): with task_manager.acquire(self.context, self.node.uuid) as task: - result = power.XClarityPower.get_power_state(task) + result = power.XClarityPower.get_power_state(self, task) self.assertEqual(STATE_POWER_ON, result) @mock.patch.object(common, 'translate_xclarity_power_state', @@ -89,9 +89,9 @@ class XClarityPowerDriverTestCase(db_base.DbTestCase): task) self.assertFalse(mock_translate_state.called) - @mock.patch.object(power.LOG, 'warning') + @mock.patch.object(power.LOG, 'warning', autospec=True) @mock.patch.object(power.XClarityPower, 'get_power_state', - return_value=states.POWER_ON) + return_value=states.POWER_ON, autospec=True) def test_set_power(self, mock_set_power_state, mock_log, mock_get_xc_client): with task_manager.acquire(self.context, self.node.uuid) as task: @@ -100,9 +100,9 @@ class XClarityPowerDriverTestCase(db_base.DbTestCase): self.assertEqual(expected, states.POWER_ON) self.assertFalse(mock_log.called) - @mock.patch.object(power.LOG, 'warning') + @mock.patch.object(power.LOG, 'warning', autospec=True) @mock.patch.object(power.XClarityPower, 'get_power_state', - return_value=states.POWER_ON) + return_value=states.POWER_ON, autospec=True) def test_set_power_timeout(self, mock_set_power_state, mock_log, mock_get_xc_client): with task_manager.acquire(self.context, self.node.uuid) as task: @@ -126,19 +126,21 @@ class XClarityPowerDriverTestCase(db_base.DbTestCase): task.driver.power.set_power_state, task, states.POWER_OFF) - @mock.patch.object(power.LOG, 'warning') - @mock.patch.object(power.XClarityPower, 'set_power_state') + @mock.patch.object(power.LOG, 'warning', autospec=True) + @mock.patch.object(power.XClarityPower, 'set_power_state', autospec=True) def test_reboot(self, mock_set_power_state, mock_log, mock_get_xc_client): with task_manager.acquire(self.context, self.node.uuid) as task: task.driver.power.reboot(task) - mock_set_power_state.assert_called_with(task, states.REBOOT) + mock_set_power_state.assert_called_with( + mock.ANY, task, states.REBOOT) self.assertFalse(mock_log.called) - @mock.patch.object(power.LOG, 'warning') - @mock.patch.object(power.XClarityPower, 'set_power_state') + @mock.patch.object(power.LOG, 'warning', autospec=True) + @mock.patch.object(power.XClarityPower, 'set_power_state', autospec=True) def test_reboot_timeout(self, mock_set_power_state, mock_log, mock_get_xc_client): with task_manager.acquire(self.context, self.node.uuid) as task: task.driver.power.reboot(task, timeout=55) - mock_set_power_state.assert_called_with(task, states.REBOOT) + mock_set_power_state.assert_called_with( + mock.ANY, task, states.REBOOT) self.assertTrue(mock_log.called) diff --git a/ironic/tests/unit/drivers/test_generic.py b/ironic/tests/unit/drivers/test_generic.py index 6a1d07863..d1acb59b8 100644 --- a/ironic/tests/unit/drivers/test_generic.py +++ b/ironic/tests/unit/drivers/test_generic.py @@ -68,7 +68,8 @@ class ManualManagementHardwareTestCase(db_base.DbTestCase): def test_get_properties(self): # These properties are from vendor (agent) and boot (pxe) interfaces expected_prop_keys = [ - 'deploy_forces_oob_reboot', 'deploy_kernel', 'deploy_ramdisk', + 'agent_verify_ca', 'deploy_forces_oob_reboot', + 'deploy_kernel', 'deploy_ramdisk', 'force_persistent_boot_device', 'rescue_kernel', 'rescue_ramdisk'] hardware_type = driver_factory.get_hardware_type("manual-management") properties = hardware_type.get_properties() diff --git a/releasenotes/notes/add-ilo-inband-deploy-step-update-firmware-using-sum-cfee84a19120dd3c.yaml b/releasenotes/notes/add-ilo-inband-deploy-step-update-firmware-using-sum-cfee84a19120dd3c.yaml new file mode 100644 index 000000000..fe1956e92 --- /dev/null +++ b/releasenotes/notes/add-ilo-inband-deploy-step-update-firmware-using-sum-cfee84a19120dd3c.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + Adds inband deploy step ``flash_firmware_sum`` to the ``management`` + interface of the ``ilo`` and ``ilo5`` hardware types. The required + minimum version for the proliantutils library is 2.9.5. +other: + - | + The proliantutils library version 2.9.5 enables ``ssacli`` based + in-band deploy step ``apply_configuration`` of ``agent`` RAID + interface for ``ilo`` and ``ilo5`` hardware types. diff --git a/releasenotes/notes/agent-verify-ca-ddbfbb0f27198d82.yaml b/releasenotes/notes/agent-verify-ca-ddbfbb0f27198d82.yaml new file mode 100644 index 000000000..3f945f22d --- /dev/null +++ b/releasenotes/notes/agent-verify-ca-ddbfbb0f27198d82.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Adds a new ``driver_info`` parameter ``agent_verify_ca`` that allows + specifying a file with certificates to use when accessing IPA. Set + to ``False`` to disable certificate validation. diff --git a/releasenotes/notes/unrescue-token-ae664a17343e0610.yaml b/releasenotes/notes/unrescue-token-ae664a17343e0610.yaml new file mode 100644 index 000000000..7ce3273e7 --- /dev/null +++ b/releasenotes/notes/unrescue-token-ae664a17343e0610.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Removes stale agent token on rescue and unrescue operations. Previously it + would cause subsequent rescue operations to fail. @@ -135,8 +135,6 @@ per-file-ignores = ironic/tests/unit/drivers/modules/test_console_utils.py:H210 ironic/tests/unit/drivers/modules/ilo/*:H210 ironic/tests/unit/drivers/modules/irmc/*:H210 - ironic/tests/unit/drivers/modules/xclarity/*:H210 - ironic/tests/unit/drivers/modules/storage/*:H210 [hacking] import_exceptions = testtools.matchers, ironic.common.i18n diff --git a/zuul.d/ironic-jobs.yaml b/zuul.d/ironic-jobs.yaml index 6a8d8b7dd..91f45a2c7 100644 --- a/zuul.d/ironic-jobs.yaml +++ b/zuul.d/ironic-jobs.yaml @@ -642,6 +642,7 @@ IRONIC_ENABLED_BOOT_INTERFACES: pxe IRONIC_IPXE_ENABLED: False IRONIC_BOOT_MODE: uefi + IRONIC_RAMDISK_TYPE: tinyipa IRONIC_AUTOMATED_CLEAN_ENABLED: False IRONIC_DEFAULT_BOOT_OPTION: netboot IRONIC_VM_SPECS_RAM: 4096 |