diff options
Diffstat (limited to 'ironic')
27 files changed, 579 insertions, 205 deletions
diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 7162ca4d3..eb0eb7956 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -510,6 +510,69 @@ RELEASE_MAPPING = { 'VolumeTarget': ['1.0'], } }, + '21.2': { + 'api': '1.80', + 'rpc': '1.55', + 'objects': { + 'Allocation': ['1.1'], + 'BIOSSetting': ['1.1'], + 'Node': ['1.36'], + 'NodeHistory': ['1.0'], + 'NodeInventory': ['1.0'], + 'Conductor': ['1.3'], + 'Chassis': ['1.3'], + 'Deployment': ['1.0'], + 'DeployTemplate': ['1.1'], + 'Port': ['1.10'], + 'Portgroup': ['1.4'], + 'Trait': ['1.0'], + 'TraitList': ['1.0'], + 'VolumeConnector': ['1.0'], + 'VolumeTarget': ['1.0'], + } + }, + '21.3': { + 'api': '1.81', + 'rpc': '1.55', + 'objects': { + 'Allocation': ['1.1'], + 'BIOSSetting': ['1.1'], + 'Node': ['1.36'], + 'NodeHistory': ['1.0'], + 'NodeInventory': ['1.0'], + 'Conductor': ['1.3'], + 'Chassis': ['1.3'], + 'Deployment': ['1.0'], + 'DeployTemplate': ['1.1'], + 'Port': ['1.11'], + 'Portgroup': ['1.4'], + 'Trait': ['1.0'], + 'TraitList': ['1.0'], + 'VolumeConnector': ['1.0'], + 'VolumeTarget': ['1.0'], + } + }, + '21.4': { + 'api': '1.82', + 'rpc': '1.55', + 'objects': { + 'Allocation': ['1.1'], + 'BIOSSetting': ['1.1'], + 'Node': ['1.37'], + 'NodeHistory': ['1.0'], + 'NodeInventory': ['1.0'], + 'Conductor': ['1.3'], + 'Chassis': ['1.3'], + 'Deployment': ['1.0'], + 'DeployTemplate': ['1.1'], + 'Port': ['1.11'], + 'Portgroup': ['1.5'], + 'Trait': ['1.0'], + 'TraitList': ['1.0'], + 'VolumeConnector': ['1.0'], + 'VolumeTarget': ['1.0'], + } + }, 'master': { 'api': '1.82', 'rpc': '1.55', @@ -543,12 +606,11 @@ RELEASE_MAPPING = { # # Just after we do a new named release, delete the oldest named # release (that we are no longer supporting for a rolling upgrade). -# -# There should be at most two named mappings here. -# NOTE(mgoddard): remove yoga prior to the antelope release. RELEASE_MAPPING['yoga'] = RELEASE_MAPPING['20.1'] RELEASE_MAPPING['zed'] = RELEASE_MAPPING['21.1'] +RELEASE_MAPPING['antelope'] = RELEASE_MAPPING['21.4'] +RELEASE_MAPPING['2023.1'] = RELEASE_MAPPING['21.4'] # List of available versions with named versions first; 'master' is excluded. RELEASE_VERSIONS = sorted(set(RELEASE_MAPPING) - {'master'}, reverse=True) diff --git a/ironic/common/rpc_service.py b/ironic/common/rpc_service.py index b0eec7758..cb0f23c98 100644 --- a/ironic/common/rpc_service.py +++ b/ironic/common/rpc_service.py @@ -14,6 +14,7 @@ # License for the specific language governing permissions and limitations # under the License. +import datetime import signal import sys import time @@ -24,6 +25,7 @@ from oslo_log import log import oslo_messaging as messaging from oslo_service import service from oslo_utils import importutils +from oslo_utils import timeutils from ironic.common import context from ironic.common import rpc @@ -93,6 +95,26 @@ class RPCService(service.Service): 'transport': CONF.rpc_transport}) def stop(self): + initial_time = timeutils.utcnow() + extend_time = initial_time + datetime.timedelta( + seconds=CONF.hash_ring_reset_interval) + + try: + self.manager.del_host(deregister=self.deregister) + except Exception as e: + LOG.exception('Service error occurred when cleaning up ' + 'the RPC manager. Error: %s', e) + + if self.manager.get_online_conductor_count() > 1: + # Delay stopping the server until the hash ring has been + # reset on the cluster + stop_time = timeutils.utcnow() + if stop_time < extend_time: + stop_wait = max(0, (extend_time - stop_time).seconds) + LOG.info('Waiting %(stop_wait)s seconds for hash ring reset.', + {'stop_wait': stop_wait}) + time.sleep(stop_wait) + try: if self.rpcserver is not None: self.rpcserver.stop() @@ -100,11 +122,6 @@ class RPCService(service.Service): except Exception as e: LOG.exception('Service error occurred when stopping the ' 'RPC server. Error: %s', e) - try: - self.manager.del_host(deregister=self.deregister) - except Exception as e: - LOG.exception('Service error occurred when cleaning up ' - 'the RPC manager. Error: %s', e) super(RPCService, self).stop(graceful=True) LOG.info('Stopped RPC server for service %(service)s on host ' diff --git a/ironic/common/states.py b/ironic/common/states.py index 89b710189..f2238b41b 100644 --- a/ironic/common/states.py +++ b/ironic/common/states.py @@ -269,6 +269,9 @@ _FASTTRACK_LOOKUP_ALLOWED_STATES = (ENROLL, MANAGEABLE, AVAILABLE, FASTTRACK_LOOKUP_ALLOWED_STATES = frozenset(_FASTTRACK_LOOKUP_ALLOWED_STATES) """States where API lookups are permitted with fast track enabled.""" +FAILURE_STATES = frozenset((DEPLOYFAIL, CLEANFAIL, INSPECTFAIL, + RESCUEFAIL, UNRESCUEFAIL, ADOPTFAIL)) + ############## # Power states diff --git a/ironic/common/utils.py b/ironic/common/utils.py index 9ae88d4d6..793b4b501 100644 --- a/ironic/common/utils.py +++ b/ironic/common/utils.py @@ -26,6 +26,7 @@ import hashlib import ipaddress import os import re +import shlex import shutil import tempfile import time @@ -696,3 +697,30 @@ def stop_after_retries(option, group=None): return retry_state.attempt_number >= num_retries + 1 return should_stop + + +def is_loopback(hostname_or_ip): + """Check if the provided hostname or IP address is a loopback.""" + try: + return ipaddress.ip_address(hostname_or_ip).is_loopback + except ValueError: # host name + return hostname_or_ip in ('localhost', 'localhost.localdomain') + + +def parse_kernel_params(params): + """Parse kernel parameters into a dictionary. + + ``None`` is used as a value for parameters that are not in + the ``key=value`` format. + + :param params: kernel parameters as a space-delimited string. + """ + result = {} + for s in shlex.split(params): + try: + key, value = s.split('=', 1) + except ValueError: + result[s] = None + else: + result[key] = value + return result diff --git a/ironic/conductor/base_manager.py b/ironic/conductor/base_manager.py index 22ebd57f5..5c2e4ea95 100644 --- a/ironic/conductor/base_manager.py +++ b/ironic/conductor/base_manager.py @@ -334,6 +334,10 @@ class BaseConductorManager(object): self._started = False + def get_online_conductor_count(self): + """Return a count of currently online conductors""" + return len(self.dbapi.get_online_conductors()) + def _register_and_validate_hardware_interfaces(self, hardware_types): """Register and validate hardware interfaces for this conductor. diff --git a/ironic/conductor/cleaning.py b/ironic/conductor/cleaning.py index e59841a99..9e4edb809 100644 --- a/ironic/conductor/cleaning.py +++ b/ironic/conductor/cleaning.py @@ -248,12 +248,21 @@ def do_next_clean_step(task, step_index, disable_ramdisk=None): task.process_event(event) +def get_last_error(node): + last_error = _('By request, the clean operation was aborted') + if node.clean_step: + last_error += ( + _(' during or after the completion of step "%s"') + % conductor_steps.step_id(node.clean_step) + ) + return last_error + + @task_manager.require_exclusive_lock -def do_node_clean_abort(task, step_name=None): +def do_node_clean_abort(task): """Internal method to abort an ongoing operation. :param task: a TaskManager instance with an exclusive lock - :param step_name: The name of the clean step. """ node = task.node try: @@ -271,12 +280,13 @@ def do_node_clean_abort(task, step_name=None): set_fail_state=False) return + last_error = get_last_error(node) info_message = _('Clean operation aborted for node %s') % node.uuid - last_error = _('By request, the clean operation was aborted') - if step_name: - msg = _(' after the completion of step "%s"') % step_name - last_error += msg - info_message += msg + if node.clean_step: + info_message += ( + _(' during or after the completion of step "%s"') + % node.clean_step + ) node.last_error = last_error node.clean_step = None @@ -318,7 +328,7 @@ def continue_node_clean(task): target_state = None task.process_event('fail', target_state=target_state) - do_node_clean_abort(task, step_name) + do_node_clean_abort(task) return LOG.debug('The cleaning operation for node %(node)s was ' diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 8de34b76a..74e3192cf 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -1351,7 +1351,8 @@ class ConductorManager(base_manager.BaseConductorManager): callback=self._spawn_worker, call_args=(cleaning.do_node_clean_abort, task), err_handler=utils.provisioning_error_handler, - target_state=target_state) + target_state=target_state, + last_error=cleaning.get_last_error(node)) return if node.provision_state == states.RESCUEWAIT: diff --git a/ironic/conductor/task_manager.py b/ironic/conductor/task_manager.py index 509c9ce92..922e74cf6 100644 --- a/ironic/conductor/task_manager.py +++ b/ironic/conductor/task_manager.py @@ -527,7 +527,8 @@ class TaskManager(object): self.release_resources() def process_event(self, event, callback=None, call_args=None, - call_kwargs=None, err_handler=None, target_state=None): + call_kwargs=None, err_handler=None, target_state=None, + last_error=None): """Process the given event for the task's current state. :param event: the name of the event to process @@ -540,6 +541,8 @@ class TaskManager(object): prev_target_state) :param target_state: if specified, the target provision state for the node. Otherwise, use the target state from the fsm + :param last_error: last error to set on the node together with + the state transition. :raises: InvalidState if the event is not allowed by the associated state machine """ @@ -572,13 +575,15 @@ class TaskManager(object): # set up the async worker if callback: - # clear the error if we're going to start work in a callback - self.node.last_error = None + # update the error if we're going to start work in a callback + self.node.last_error = last_error if call_args is None: call_args = () if call_kwargs is None: call_kwargs = {} self.spawn_after(callback, *call_args, **call_kwargs) + elif last_error is not None: + self.node.last_error = last_error # publish the state transition by saving the Node self.node.save() diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 29f5f1d25..402ec2241 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -309,9 +309,11 @@ def node_power_action(task, new_state, timeout=None): # Set the target_power_state and clear any last_error, if we're # starting a new operation. This will expose to other processes - # and clients that work is in progress. - node['target_power_state'] = target_state - node['last_error'] = None + # and clients that work is in progress. Keep the last_error intact + # if the power action happens as a result of a failure. + node.target_power_state = target_state + if node.provision_state not in states.FAILURE_STATES: + node.last_error = None node.timestamp_driver_internal_info('last_power_state_change') # NOTE(dtantsur): wipe token on shutting down, otherwise a reboot in # fast-track (or an accidentally booted agent) will cause subsequent diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index d5f4a9d65..93a211fc3 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -1834,6 +1834,9 @@ class Connection(api.Connection): max_to_migrate = max_count or total_to_migrate for model in sql_models: + use_node_id = False + if (not hasattr(model, 'id') and hasattr(model, 'node_id')): + use_node_id = True version = mapping[model.__name__][0] num_migrated = 0 with _session_for_write() as session: @@ -1847,13 +1850,27 @@ class Connection(api.Connection): # max_to_migrate objects. ids = [] for obj in query.slice(0, max_to_migrate): - ids.append(obj['id']) - num_migrated = ( - session.query(model). - filter(sql.and_(model.id.in_(ids), - model.version != version)). - update({model.version: version}, - synchronize_session=False)) + if not use_node_id: + ids.append(obj['id']) + else: + # BIOSSettings, NodeTrait, NodeTag do not have id + # columns, fallback to node_id as they both have + # it. + ids.append(obj['node_id']) + if not use_node_id: + num_migrated = ( + session.query(model). + filter(sql.and_(model.id.in_(ids), + model.version != version)). + update({model.version: version}, + synchronize_session=False)) + else: + num_migrated = ( + session.query(model). + filter(sql.and_(model.node_id.in_(ids), + model.version != version)). + update({model.version: version}, + synchronize_session=False)) else: num_migrated = ( session.query(model). diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 13f91e9cd..fd83f9f08 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -1093,6 +1093,11 @@ def _cache_and_convert_image(task, instance_info, image_info=None): _, image_path = cache_instance_image(task.context, task.node, force_raw=force_raw) if force_raw or image_info is None: + if image_info is None: + initial_format = instance_info.get('image_disk_format') + else: + initial_format = image_info.get('disk_format') + if force_raw: instance_info['image_disk_format'] = 'raw' else: @@ -1108,21 +1113,29 @@ def _cache_and_convert_image(task, instance_info, image_info=None): # sha256. if image_info is None: os_hash_algo = instance_info.get('image_os_hash_algo') + hash_value = instance_info.get('image_os_hash_value') + old_checksum = instance_info.get('image_checksum') else: os_hash_algo = image_info.get('os_hash_algo') + hash_value = image_info.get('os_hash_value') + old_checksum = image_info.get('checksum') + + if initial_format != instance_info['image_disk_format']: + if not os_hash_algo or os_hash_algo == 'md5': + LOG.debug("Checksum algorithm for image %(image)s for node " + "%(node)s is set to '%(algo)s', changing to sha256", + {'algo': os_hash_algo, 'node': task.node.uuid, + 'image': image_path}) + os_hash_algo = 'sha256' + + LOG.debug('Recalculating checksum for image %(image)s for node ' + '%(node)s due to image conversion', + {'image': image_path, 'node': task.node.uuid}) + instance_info['image_checksum'] = None + hash_value = compute_image_checksum(image_path, os_hash_algo) + else: + instance_info['image_checksum'] = old_checksum - if not os_hash_algo or os_hash_algo == 'md5': - LOG.debug("Checksum algorithm for image %(image)s for node " - "%(node)s is set to '%(algo)s', changing to 'sha256'", - {'algo': os_hash_algo, 'node': task.node.uuid, - 'image': image_path}) - os_hash_algo = 'sha256' - - LOG.debug('Recalculating checksum for image %(image)s for node ' - '%(node)s due to image conversion', - {'image': image_path, 'node': task.node.uuid}) - instance_info['image_checksum'] = None - hash_value = compute_image_checksum(image_path, os_hash_algo) instance_info['image_os_hash_algo'] = os_hash_algo instance_info['image_os_hash_value'] = hash_value else: diff --git a/ironic/drivers/modules/inspect_utils.py b/ironic/drivers/modules/inspect_utils.py index 0089302c1..432345cf1 100644 --- a/ironic/drivers/modules/inspect_utils.py +++ b/ironic/drivers/modules/inspect_utils.py @@ -27,7 +27,7 @@ LOG = logging.getLogger(__name__) _OBJECT_NAME_PREFIX = 'inspector_data' -def create_ports_if_not_exist(task, macs): +def create_ports_if_not_exist(task, macs=None): """Create ironic ports from MAC addresses data dict. Creates ironic ports from MAC addresses data returned with inspection or @@ -36,8 +36,17 @@ def create_ports_if_not_exist(task, macs): pair. :param task: A TaskManager instance. - :param macs: A sequence of MAC addresses. + :param macs: A sequence of MAC addresses. If ``None``, fetched from + the task's management interface. """ + if macs is None: + macs = task.driver.management.get_mac_addresses(task) + if not macs: + LOG.warning("Not attempting to create any port as no NICs " + "were discovered in 'enabled' state for node %s", + task.node.uuid) + return + node = task.node for mac in macs: if not netutils.is_valid_mac(mac): diff --git a/ironic/drivers/modules/inspector/__init__.py b/ironic/drivers/modules/inspector/__init__.py new file mode 100644 index 000000000..bb2dd43c8 --- /dev/null +++ b/ironic/drivers/modules/inspector/__init__.py @@ -0,0 +1,15 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from ironic.drivers.modules.inspector.interface import Inspector + +__all__ = ['Inspector'] diff --git a/ironic/drivers/modules/inspector/client.py b/ironic/drivers/modules/inspector/client.py new file mode 100644 index 000000000..7e996492e --- /dev/null +++ b/ironic/drivers/modules/inspector/client.py @@ -0,0 +1,57 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Client helper for ironic-inspector.""" + +from keystoneauth1 import exceptions as ks_exception +import openstack + +from ironic.common import exception +from ironic.common.i18n import _ +from ironic.common import keystone +from ironic.conf import CONF + + +_INSPECTOR_SESSION = None + + +def _get_inspector_session(**kwargs): + global _INSPECTOR_SESSION + if not _INSPECTOR_SESSION: + if CONF.auth_strategy != 'keystone': + # NOTE(dtantsur): using set_default instead of set_override because + # the native keystoneauth option must have priority. + CONF.set_default('auth_type', 'none', group='inspector') + service_auth = keystone.get_auth('inspector') + _INSPECTOR_SESSION = keystone.get_session('inspector', + auth=service_auth, + **kwargs) + return _INSPECTOR_SESSION + + +def get_client(context): + """Helper to get inspector client instance.""" + session = _get_inspector_session() + # NOTE(dtantsur): openstacksdk expects config option groups to match + # service name, but we use just "inspector". + conf = dict(CONF) + conf['ironic-inspector'] = conf.pop('inspector') + # TODO(pas-ha) investigate possibility of passing user context here, + # similar to what neutron/glance-related code does + try: + return openstack.connection.Connection( + session=session, + oslo_conf=conf).baremetal_introspection + except ks_exception.DiscoveryFailure as exc: + raise exception.ConfigInvalid( + _("Could not contact ironic-inspector for version discovery: %s") + % exc) diff --git a/ironic/drivers/modules/inspector.py b/ironic/drivers/modules/inspector/interface.py index dbf171714..8792b7b88 100644 --- a/ironic/drivers/modules/inspector.py +++ b/ironic/drivers/modules/inspector/interface.py @@ -15,18 +15,13 @@ Modules required to work with ironic_inspector: https://pypi.org/project/ironic-inspector """ -import ipaddress -import shlex from urllib import parse as urlparse import eventlet -from keystoneauth1 import exceptions as ks_exception -import openstack from oslo_log import log as logging from ironic.common import exception from ironic.common.i18n import _ -from ironic.common import keystone from ironic.common import states from ironic.common import utils from ironic.conductor import periodics @@ -36,61 +31,22 @@ from ironic.conf import CONF from ironic.drivers import base from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import inspect_utils +from ironic.drivers.modules.inspector import client LOG = logging.getLogger(__name__) -_INSPECTOR_SESSION = None # Internal field to mark whether ironic or inspector manages boot for the node _IRONIC_MANAGES_BOOT = 'inspector_manage_boot' -def _get_inspector_session(**kwargs): - global _INSPECTOR_SESSION - if not _INSPECTOR_SESSION: - if CONF.auth_strategy != 'keystone': - # NOTE(dtantsur): using set_default instead of set_override because - # the native keystoneauth option must have priority. - CONF.set_default('auth_type', 'none', group='inspector') - service_auth = keystone.get_auth('inspector') - _INSPECTOR_SESSION = keystone.get_session('inspector', - auth=service_auth, - **kwargs) - return _INSPECTOR_SESSION - - -def _get_client(context): - """Helper to get inspector client instance.""" - session = _get_inspector_session() - # NOTE(dtantsur): openstacksdk expects config option groups to match - # service name, but we use just "inspector". - conf = dict(CONF) - conf['ironic-inspector'] = conf.pop('inspector') - # TODO(pas-ha) investigate possibility of passing user context here, - # similar to what neutron/glance-related code does - try: - return openstack.connection.Connection( - session=session, - oslo_conf=conf).baremetal_introspection - except ks_exception.DiscoveryFailure as exc: - raise exception.ConfigInvalid( - _("Could not contact ironic-inspector for version discovery: %s") - % exc) - - def _get_callback_endpoint(client): root = CONF.inspector.callback_endpoint_override or client.get_endpoint() if root == 'mdns': return root parts = urlparse.urlsplit(root) - is_loopback = False - try: - # ip_address requires a unicode string on Python 2 - is_loopback = ipaddress.ip_address(parts.hostname).is_loopback - except ValueError: # host name - is_loopback = (parts.hostname == 'localhost') - if is_loopback: + if utils.is_loopback(parts.hostname): raise exception.InvalidParameterValue( _('Loopback address %s cannot be used as an introspection ' 'callback URL') % parts.hostname) @@ -181,26 +137,14 @@ def _ironic_manages_boot(task, raise_exc=False): return True -def _parse_kernel_params(): - """Parse kernel params from the configuration.""" - result = {} - for s in shlex.split(CONF.inspector.extra_kernel_params): - try: - key, value = s.split('=', 1) - except ValueError: - result[s] = None - else: - result[key] = value - return result - - def _start_managed_inspection(task): """Start inspection managed by ironic.""" try: - client = _get_client(task.context) - endpoint = _get_callback_endpoint(client) - params = dict(_parse_kernel_params(), - **{'ipa-inspection-callback-url': endpoint}) + cli = client.get_client(task.context) + endpoint = _get_callback_endpoint(cli) + params = dict( + utils.parse_kernel_params(CONF.inspector.extra_kernel_params), + **{'ipa-inspection-callback-url': endpoint}) if utils.fast_track_enabled(task.node): params['ipa-api-url'] = deploy_utils.get_ironic_api_url() @@ -208,7 +152,7 @@ def _start_managed_inspection(task): with cond_utils.power_state_for_network_configuration(task): task.driver.network.add_inspection_network(task) task.driver.boot.prepare_ramdisk(task, ramdisk_params=params) - client.start_introspection(task.node.uuid, manage_boot=False) + cli.start_introspection(task.node.uuid, manage_boot=False) cond_utils.node_power_action(task, states.POWER_ON) except Exception as exc: LOG.exception('Unable to start managed inspection for node %(uuid)s: ' @@ -235,7 +179,7 @@ class Inspector(base.InspectInterface): :param task: a task from TaskManager. :raises: UnsupportedDriverExtension """ - _parse_kernel_params() + utils.parse_kernel_params(CONF.inspector.extra_kernel_params) if CONF.inspector.require_managed_boot: _ironic_manages_boot(task, raise_exc=True) @@ -250,16 +194,7 @@ class Inspector(base.InspectInterface): :raises: HardwareInspectionFailure on failure """ try: - enabled_macs = task.driver.management.get_mac_addresses(task) - if enabled_macs: - inspect_utils.create_ports_if_not_exist(task, enabled_macs) - else: - LOG.warning("Not attempting to create any port as no NICs " - "were discovered in 'enabled' state for node " - "%(node)s: %(mac_data)s", - {'mac_data': enabled_macs, - 'node': task.node.uuid}) - + inspect_utils.create_ports_if_not_exist(task) except exception.UnsupportedDriverExtension: LOG.debug('Pre-creating ports prior to inspection not supported' ' on node %s.', task.node.uuid) @@ -295,7 +230,7 @@ class Inspector(base.InspectInterface): node_uuid = task.node.uuid LOG.debug('Aborting inspection for node %(uuid)s using ' 'ironic-inspector', {'uuid': node_uuid}) - _get_client(task.context).abort_introspection(node_uuid) + client.get_client(task.context).abort_introspection(node_uuid) @periodics.node_periodic( purpose='checking hardware inspection status', @@ -310,7 +245,7 @@ class Inspector(base.InspectInterface): def _start_inspection(node_uuid, context): """Call to inspector to start inspection.""" try: - _get_client(context).start_introspection(node_uuid) + client.get_client(context).start_introspection(node_uuid) except Exception as exc: LOG.error('Error contacting ironic-inspector for inspection of node ' '%(node)s: %(cls)s: %(err)s', @@ -339,7 +274,7 @@ def _check_status(task): task.node.uuid) try: - inspector_client = _get_client(task.context) + inspector_client = client.get_client(task.context) status = inspector_client.get_introspection(node.uuid) except Exception: # NOTE(dtantsur): get_status should not normally raise diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index d56652b1e..27061737d 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -44,8 +44,8 @@ from ironic.common import indicator_states from ironic.common import policy from ironic.common import states from ironic.conductor import rpcapi +from ironic.conf import CONF from ironic.drivers.modules import inspect_utils -from ironic.drivers.modules import inspector from ironic import objects from ironic.objects import fields as obj_fields from ironic import tests as tests_root @@ -54,7 +54,6 @@ from ironic.tests.unit.api import base as test_api_base from ironic.tests.unit.api import utils as test_api_utils from ironic.tests.unit.objects import utils as obj_utils -CONF = inspector.CONF with open( os.path.join( diff --git a/ironic/tests/unit/common/test_release_mappings.py b/ironic/tests/unit/common/test_release_mappings.py index dad536257..e6f4479b9 100644 --- a/ironic/tests/unit/common/test_release_mappings.py +++ b/ironic/tests/unit/common/test_release_mappings.py @@ -44,7 +44,7 @@ NUMERIC_RELEASES = sorted( map(versionutils.convert_version_to_tuple, set(release_mappings.RELEASE_MAPPING) # Update the exceptions whenever needed - - {'master', 'zed', 'yoga'}), + - {'master', '2023.1', 'antelope', 'zed', 'yoga'}), reverse=True) diff --git a/ironic/tests/unit/common/test_rpc_service.py b/ironic/tests/unit/common/test_rpc_service.py index 8483bfb22..09446ecf8 100644 --- a/ironic/tests/unit/common/test_rpc_service.py +++ b/ironic/tests/unit/common/test_rpc_service.py @@ -10,24 +10,28 @@ # License for the specific language governing permissions and limitations # under the License. +import datetime +import time from unittest import mock from oslo_config import cfg import oslo_messaging from oslo_service import service as base_service +from oslo_utils import timeutils from ironic.common import context from ironic.common import rpc from ironic.common import rpc_service from ironic.conductor import manager from ironic.objects import base as objects_base -from ironic.tests import base +from ironic.tests.unit.db import base as db_base +from ironic.tests.unit.db import utils as db_utils CONF = cfg.CONF @mock.patch.object(base_service.Service, '__init__', lambda *_, **__: None) -class TestRPCService(base.TestCase): +class TestRPCService(db_base.DbTestCase): def setUp(self): super(TestRPCService, self).setUp() @@ -35,6 +39,7 @@ class TestRPCService(base.TestCase): mgr_module = "ironic.conductor.manager" mgr_class = "ConductorManager" self.rpc_svc = rpc_service.RPCService(host, mgr_module, mgr_class) + self.rpc_svc.manager.dbapi = self.dbapi @mock.patch.object(manager.ConductorManager, 'prepare_host', autospec=True) @mock.patch.object(oslo_messaging, 'Target', autospec=True) @@ -108,3 +113,75 @@ class TestRPCService(base.TestCase): self.assertFalse(self.rpc_svc._started) self.assertIn("boom", self.rpc_svc._failure) self.assertRaises(SystemExit, self.rpc_svc.wait_for_start) + + @mock.patch.object(timeutils, 'utcnow', autospec=True) + @mock.patch.object(time, 'sleep', autospec=True) + def test_stop_instant(self, mock_sleep, mock_utcnow): + # del_host returns instantly + mock_utcnow.return_value = datetime.datetime(2023, 2, 2, 21, 10, 0) + conductor1 = db_utils.get_test_conductor(hostname='fake_host') + with mock.patch.object(self.dbapi, 'get_online_conductors', + autospec=True) as mock_cond_list: + mock_cond_list.return_value = [conductor1] + self.rpc_svc.stop() + + # single conductor so exit immediately without waiting + mock_sleep.assert_not_called() + + @mock.patch.object(timeutils, 'utcnow', autospec=True) + @mock.patch.object(time, 'sleep', autospec=True) + def test_stop_after_full_reset_interval(self, mock_sleep, mock_utcnow): + # del_host returns instantly + mock_utcnow.return_value = datetime.datetime(2023, 2, 2, 21, 10, 0) + conductor1 = db_utils.get_test_conductor(hostname='fake_host') + conductor2 = db_utils.get_test_conductor(hostname='other_fake_host') + with mock.patch.object(self.dbapi, 'get_online_conductors', + autospec=True) as mock_cond_list: + # multiple conductors, so wait for hash_ring_reset_interval + mock_cond_list.return_value = [conductor1, conductor2] + self.rpc_svc.stop() + + # wait the total CONF.hash_ring_reset_interval 15 seconds + mock_sleep.assert_has_calls([mock.call(15)]) + + @mock.patch.object(timeutils, 'utcnow', autospec=True) + @mock.patch.object(time, 'sleep', autospec=True) + def test_stop_after_remaining_interval(self, mock_sleep, mock_utcnow): + mock_utcnow.return_value = datetime.datetime(2023, 2, 2, 21, 10, 0) + conductor1 = db_utils.get_test_conductor(hostname='fake_host') + conductor2 = db_utils.get_test_conductor(hostname='other_fake_host') + + # del_host returns after 5 seconds + mock_utcnow.side_effect = [ + datetime.datetime(2023, 2, 2, 21, 10, 0), + datetime.datetime(2023, 2, 2, 21, 10, 5), + ] + with mock.patch.object(self.dbapi, 'get_online_conductors', + autospec=True) as mock_cond_list: + # multiple conductors, so wait for hash_ring_reset_interval + mock_cond_list.return_value = [conductor1, conductor2] + self.rpc_svc.stop() + + # wait the remaining 10 seconds + mock_sleep.assert_has_calls([mock.call(10)]) + + @mock.patch.object(timeutils, 'utcnow', autospec=True) + @mock.patch.object(time, 'sleep', autospec=True) + def test_stop_slow(self, mock_sleep, mock_utcnow): + mock_utcnow.return_value = datetime.datetime(2023, 2, 2, 21, 10, 0) + conductor1 = db_utils.get_test_conductor(hostname='fake_host') + conductor2 = db_utils.get_test_conductor(hostname='other_fake_host') + + # del_host returns after 16 seconds + mock_utcnow.side_effect = [ + datetime.datetime(2023, 2, 2, 21, 10, 0), + datetime.datetime(2023, 2, 2, 21, 10, 16), + ] + with mock.patch.object(self.dbapi, 'get_online_conductors', + autospec=True) as mock_cond_list: + # multiple conductors, so wait for hash_ring_reset_interval + mock_cond_list.return_value = [conductor1, conductor2] + self.rpc_svc.stop() + + # no wait required, CONF.hash_ring_reset_interval already exceeded + mock_sleep.assert_not_called() diff --git a/ironic/tests/unit/conductor/test_cleaning.py b/ironic/tests/unit/conductor/test_cleaning.py index a4c3d57b6..34e805deb 100644 --- a/ironic/tests/unit/conductor/test_cleaning.py +++ b/ironic/tests/unit/conductor/test_cleaning.py @@ -1138,12 +1138,12 @@ class DoNodeCleanTestCase(db_base.DbTestCase): class DoNodeCleanAbortTestCase(db_base.DbTestCase): @mock.patch.object(fake.FakeDeploy, 'tear_down_cleaning', autospec=True) - def _test__do_node_clean_abort(self, step_name, tear_mock): + def _test_do_node_clean_abort(self, clean_step, tear_mock): node = obj_utils.create_test_node( self.context, driver='fake-hardware', - provision_state=states.CLEANFAIL, + provision_state=states.CLEANWAIT, target_provision_state=states.AVAILABLE, - clean_step={'step': 'foo', 'abortable': True}, + clean_step=clean_step, driver_internal_info={ 'agent_url': 'some url', 'agent_secret_token': 'token', @@ -1153,11 +1153,11 @@ class DoNodeCleanAbortTestCase(db_base.DbTestCase): 'skip_current_clean_step': True}) with task_manager.acquire(self.context, node.uuid) as task: - cleaning.do_node_clean_abort(task, step_name=step_name) + cleaning.do_node_clean_abort(task) self.assertIsNotNone(task.node.last_error) tear_mock.assert_called_once_with(task.driver.deploy, task) - if step_name: - self.assertIn(step_name, task.node.last_error) + if clean_step: + self.assertIn(clean_step['step'], task.node.last_error) # assert node's clean_step and metadata was cleaned up self.assertEqual({}, task.node.clean_step) self.assertNotIn('clean_step_index', @@ -1173,11 +1173,12 @@ class DoNodeCleanAbortTestCase(db_base.DbTestCase): self.assertNotIn('agent_secret_token', task.node.driver_internal_info) - def test__do_node_clean_abort(self): - self._test__do_node_clean_abort(None) + def test_do_node_clean_abort_early(self): + self._test_do_node_clean_abort(None) - def test__do_node_clean_abort_with_step_name(self): - self._test__do_node_clean_abort('foo') + def test_do_node_clean_abort_with_step(self): + self._test_do_node_clean_abort({'step': 'foo', 'interface': 'deploy', + 'abortable': True}) @mock.patch.object(fake.FakeDeploy, 'tear_down_cleaning', autospec=True) def test__do_node_clean_abort_tear_down_fail(self, tear_mock): diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 027418ba9..6a6f7e08f 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -2735,7 +2735,8 @@ class DoProvisioningActionTestCase(mgr_utils.ServiceSetUpMixin, # Node will be moved to tgt_prov_state after cleaning, not tested here self.assertEqual(states.CLEANFAIL, node.provision_state) self.assertEqual(tgt_prov_state, node.target_provision_state) - self.assertIsNone(node.last_error) + self.assertEqual('By request, the clean operation was aborted', + node.last_error) mock_spawn.assert_called_with( self.service, cleaning.do_node_clean_abort, mock.ANY) diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index e8e12cc9c..563ac0b70 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -196,7 +196,8 @@ class NodePowerActionTestCase(db_base.DbTestCase): node = obj_utils.create_test_node(self.context, uuid=uuidutils.generate_uuid(), driver='fake-hardware', - power_state=states.POWER_OFF) + power_state=states.POWER_OFF, + last_error='failed before') task = task_manager.TaskManager(self.context, node.uuid) get_power_mock.return_value = states.POWER_OFF @@ -209,6 +210,27 @@ class NodePowerActionTestCase(db_base.DbTestCase): self.assertIsNone(node['target_power_state']) self.assertIsNone(node['last_error']) + @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True) + def test_node_power_action_keep_last_error(self, get_power_mock): + """Test node_power_action to keep last_error for failed states.""" + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + driver='fake-hardware', + power_state=states.POWER_OFF, + provision_state=states.CLEANFAIL, + last_error='failed before') + task = task_manager.TaskManager(self.context, node.uuid) + + get_power_mock.return_value = states.POWER_OFF + + conductor_utils.node_power_action(task, states.POWER_ON) + + node.refresh() + get_power_mock.assert_called_once_with(mock.ANY, mock.ANY) + self.assertEqual(states.POWER_ON, node['power_state']) + self.assertIsNone(node['target_power_state']) + self.assertEqual('failed before', node['last_error']) + @mock.patch('ironic.objects.node.NodeSetPowerStateNotification', autospec=True) @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True) diff --git a/ironic/tests/unit/db/test_api.py b/ironic/tests/unit/db/test_api.py index 6142fdfae..2396b1253 100644 --- a/ironic/tests/unit/db/test_api.py +++ b/ironic/tests/unit/db/test_api.py @@ -226,6 +226,11 @@ class UpdateToLatestVersionsTestCase(base.DbTestCase): for i in range(0, num_nodes): node = utils.create_test_node(version=version, uuid=uuidutils.generate_uuid()) + # Create entries on the tables so we force field upgrades + utils.create_test_node_trait(node_id=node.id, trait='foo', + version='0.0') + utils.create_test_bios_setting(node_id=node.id, version='1.0') + nodes.append(node.uuid) for uuid in nodes: node = self.dbapi.get_node_by_uuid(uuid) @@ -238,10 +243,15 @@ class UpdateToLatestVersionsTestCase(base.DbTestCase): return nodes = self._create_nodes(5) + # Check/migrate 2, 10 remain. + self.assertEqual( + (10, 2), self.dbapi.update_to_latest_versions(self.context, 2)) + # Check/migrate 10, 8 migrated, 8 remain. self.assertEqual( - (5, 2), self.dbapi.update_to_latest_versions(self.context, 2)) + (8, 8), self.dbapi.update_to_latest_versions(self.context, 10)) + # Just make sure it is still 0, 0 in case more things are added. self.assertEqual( - (3, 3), self.dbapi.update_to_latest_versions(self.context, 10)) + (0, 0), self.dbapi.update_to_latest_versions(self.context, 10)) for uuid in nodes: node = self.dbapi.get_node_by_uuid(uuid) self.assertEqual(self.node_ver, node.version) @@ -250,10 +260,19 @@ class UpdateToLatestVersionsTestCase(base.DbTestCase): if self.node_version_same: # can't test if we don't have diff versions of the node return - - nodes = self._create_nodes(5) + vm_count = 5 + nodes = self._create_nodes(vm_count) + # NOTE(TheJulia): Under current testing, 5 node will result in 10 + # records implicitly needing to be migrated. + migrate_count = vm_count * 2 + self.assertEqual( + (migrate_count, migrate_count), + self.dbapi.update_to_latest_versions(self.context, + migrate_count)) self.assertEqual( - (5, 5), self.dbapi.update_to_latest_versions(self.context, 5)) + (0, 0), self.dbapi.update_to_latest_versions(self.context, + migrate_count)) + for uuid in nodes: node = self.dbapi.get_node_by_uuid(uuid) self.assertEqual(self.node_ver, node.version) diff --git a/ironic/tests/unit/drivers/modules/inspector/__init__.py b/ironic/tests/unit/drivers/modules/inspector/__init__.py new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/ironic/tests/unit/drivers/modules/inspector/__init__.py diff --git a/ironic/tests/unit/drivers/modules/inspector/test_client.py b/ironic/tests/unit/drivers/modules/inspector/test_client.py new file mode 100644 index 000000000..08f0fcd93 --- /dev/null +++ b/ironic/tests/unit/drivers/modules/inspector/test_client.py @@ -0,0 +1,65 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from unittest import mock + +from keystoneauth1 import exceptions as ks_exception +import openstack + +from ironic.common import context +from ironic.common import exception +from ironic.conf import CONF +from ironic.drivers.modules.inspector import client +from ironic.tests.unit.db import base as db_base + + +@mock.patch('ironic.common.keystone.get_auth', autospec=True, + return_value=mock.sentinel.auth) +@mock.patch('ironic.common.keystone.get_session', autospec=True, + return_value=mock.sentinel.session) +@mock.patch.object(openstack.connection, 'Connection', autospec=True) +class GetClientTestCase(db_base.DbTestCase): + + def setUp(self): + super(GetClientTestCase, self).setUp() + # NOTE(pas-ha) force-reset global inspector session object + client._INSPECTOR_SESSION = None + self.context = context.RequestContext(global_request_id='global') + + def test_get_client(self, mock_conn, mock_session, mock_auth): + client.get_client(self.context) + mock_conn.assert_called_once_with( + session=mock.sentinel.session, + oslo_conf=mock.ANY) + self.assertEqual(1, mock_auth.call_count) + self.assertEqual(1, mock_session.call_count) + + def test_get_client_standalone(self, mock_conn, mock_session, mock_auth): + self.config(auth_strategy='noauth') + client.get_client(self.context) + self.assertEqual('none', CONF.inspector.auth_type) + mock_conn.assert_called_once_with( + session=mock.sentinel.session, + oslo_conf=mock.ANY) + self.assertEqual(1, mock_auth.call_count) + self.assertEqual(1, mock_session.call_count) + + def test_get_client_connection_problem( + self, mock_conn, mock_session, mock_auth): + mock_conn.side_effect = ks_exception.DiscoveryFailure("") + self.assertRaises(exception.ConfigInvalid, + client.get_client, self.context) + mock_conn.assert_called_once_with( + session=mock.sentinel.session, + oslo_conf=mock.ANY) + self.assertEqual(1, mock_auth.call_count) + self.assertEqual(1, mock_session.call_count) diff --git a/ironic/tests/unit/drivers/modules/test_inspector.py b/ironic/tests/unit/drivers/modules/inspector/test_interface.py index 75ccc3ebf..f4dbdd4b0 100644 --- a/ironic/tests/unit/drivers/modules/test_inspector.py +++ b/ironic/tests/unit/drivers/modules/inspector/test_interface.py @@ -13,65 +13,19 @@ from unittest import mock import eventlet -from keystoneauth1 import exceptions as ks_exception -import openstack -from ironic.common import context from ironic.common import exception from ironic.common import states from ironic.common import utils from ironic.conductor import task_manager +from ironic.conf import CONF from ironic.drivers.modules import inspect_utils -from ironic.drivers.modules import inspector +from ironic.drivers.modules.inspector import client +from ironic.drivers.modules.inspector import interface as inspector from ironic.drivers.modules.redfish import utils as redfish_utils from ironic.tests.unit.db import base as db_base from ironic.tests.unit.objects import utils as obj_utils -CONF = inspector.CONF - - -@mock.patch('ironic.common.keystone.get_auth', autospec=True, - return_value=mock.sentinel.auth) -@mock.patch('ironic.common.keystone.get_session', autospec=True, - return_value=mock.sentinel.session) -@mock.patch.object(openstack.connection, 'Connection', autospec=True) -class GetClientTestCase(db_base.DbTestCase): - - def setUp(self): - super(GetClientTestCase, self).setUp() - # NOTE(pas-ha) force-reset global inspector session object - inspector._INSPECTOR_SESSION = None - self.context = context.RequestContext(global_request_id='global') - - def test__get_client(self, mock_conn, mock_session, mock_auth): - inspector._get_client(self.context) - mock_conn.assert_called_once_with( - session=mock.sentinel.session, - oslo_conf=mock.ANY) - self.assertEqual(1, mock_auth.call_count) - self.assertEqual(1, mock_session.call_count) - - def test__get_client_standalone(self, mock_conn, mock_session, mock_auth): - self.config(auth_strategy='noauth') - inspector._get_client(self.context) - self.assertEqual('none', inspector.CONF.inspector.auth_type) - mock_conn.assert_called_once_with( - session=mock.sentinel.session, - oslo_conf=mock.ANY) - self.assertEqual(1, mock_auth.call_count) - self.assertEqual(1, mock_session.call_count) - - def test__get_client_connection_problem( - self, mock_conn, mock_session, mock_auth): - mock_conn.side_effect = ks_exception.DiscoveryFailure("") - self.assertRaises(exception.ConfigInvalid, - inspector._get_client, self.context) - mock_conn.assert_called_once_with( - session=mock.sentinel.session, - oslo_conf=mock.ANY) - self.assertEqual(1, mock_auth.call_count) - self.assertEqual(1, mock_session.call_count) - class BaseTestCase(db_base.DbTestCase): def setUp(self): @@ -129,7 +83,7 @@ class CommonFunctionsTestCase(BaseTestCase): @mock.patch.object(eventlet, 'spawn_n', lambda f, *a, **kw: f(*a, **kw)) -@mock.patch('ironic.drivers.modules.inspector._get_client', autospec=True) +@mock.patch.object(client, 'get_client', autospec=True) class InspectHardwareTestCase(BaseTestCase): def test_validate_ok(self, mock_client): self.iface.validate(self.task) @@ -369,7 +323,7 @@ class InspectHardwareTestCase(BaseTestCase): self.task, 'power off', timeout=None) -@mock.patch('ironic.drivers.modules.inspector._get_client', autospec=True) +@mock.patch.object(client, 'get_client', autospec=True) class CheckStatusTestCase(BaseTestCase): def setUp(self): super(CheckStatusTestCase, self).setUp() @@ -593,7 +547,7 @@ class CheckStatusTestCase(BaseTestCase): mock_get_data.assert_not_called() -@mock.patch('ironic.drivers.modules.inspector._get_client', autospec=True) +@mock.patch.object(client, 'get_client', autospec=True) class InspectHardwareAbortTestCase(BaseTestCase): def test_abort_ok(self, mock_client): mock_abort = mock_client.return_value.abort_introspection diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 1177e9743..7220697cb 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -1940,7 +1940,7 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): self.node.save() self.checksum_mock = self.useFixture(fixtures.MockPatchObject( - fileutils, 'compute_file_checksum')).mock + fileutils, 'compute_file_checksum', autospec=True)).mock self.checksum_mock.return_value = 'fake-checksum' self.cache_image_mock = self.useFixture(fixtures.MockPatchObject( utils, 'cache_instance_image', autospec=True)).mock @@ -2012,9 +2012,25 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): image_info=self.image_info, expect_raw=True) self.assertIsNone(instance_info['image_checksum']) + self.assertEqual(instance_info['image_os_hash_algo'], 'sha512') + self.assertEqual(instance_info['image_os_hash_value'], + 'fake-checksum') self.assertEqual(instance_info['image_disk_format'], 'raw') - calls = [mock.call(image_path, algorithm='sha512')] - self.checksum_mock.assert_has_calls(calls) + self.checksum_mock.assert_called_once_with(image_path, + algorithm='sha512') + + def test_build_instance_info_already_raw(self): + cfg.CONF.set_override('force_raw_images', True) + self.image_info['disk_format'] = 'raw' + image_path, instance_info = self._test_build_instance_info( + image_info=self.image_info, expect_raw=True) + + self.assertEqual(instance_info['image_checksum'], 'aa') + self.assertEqual(instance_info['image_os_hash_algo'], 'sha512') + self.assertEqual(instance_info['image_os_hash_value'], + 'fake-sha512') + self.assertEqual(instance_info['image_disk_format'], 'raw') + self.checksum_mock.assert_not_called() def test_build_instance_info_force_raw_drops_md5(self): cfg.CONF.set_override('force_raw_images', True) @@ -2027,6 +2043,17 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): calls = [mock.call(image_path, algorithm='sha256')] self.checksum_mock.assert_has_calls(calls) + def test_build_instance_info_already_raw_keeps_md5(self): + cfg.CONF.set_override('force_raw_images', True) + self.image_info['os_hash_algo'] = 'md5' + self.image_info['disk_format'] = 'raw' + image_path, instance_info = self._test_build_instance_info( + image_info=self.image_info, expect_raw=True) + + self.assertEqual(instance_info['image_checksum'], 'aa') + self.assertEqual(instance_info['image_disk_format'], 'raw') + self.checksum_mock.assert_not_called() + @mock.patch.object(image_service.HttpImageService, 'validate_href', autospec=True) def test_build_instance_info_file_image(self, validate_href_mock): @@ -2035,7 +2062,6 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): i_info['image_source'] = 'file://image-ref' i_info['image_checksum'] = 'aa' i_info['root_gb'] = 10 - i_info['image_checksum'] = 'aa' driver_internal_info['is_whole_disk_image'] = True self.node.instance_info = i_info self.node.driver_internal_info = driver_internal_info @@ -2052,6 +2078,7 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): self.assertEqual(expected_url, info['image_url']) self.assertEqual('sha256', info['image_os_hash_algo']) self.assertEqual('fake-checksum', info['image_os_hash_value']) + self.assertEqual('raw', info['image_disk_format']) self.cache_image_mock.assert_called_once_with( task.context, task.node, force_raw=True) self.checksum_mock.assert_called_once_with( @@ -2068,7 +2095,6 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): i_info['image_source'] = 'http://image-ref' i_info['image_checksum'] = 'aa' i_info['root_gb'] = 10 - i_info['image_checksum'] = 'aa' driver_internal_info['is_whole_disk_image'] = True self.node.instance_info = i_info self.node.driver_internal_info = driver_internal_info @@ -2102,7 +2128,6 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): i_info['image_source'] = 'http://image-ref' i_info['image_checksum'] = 'aa' i_info['root_gb'] = 10 - i_info['image_checksum'] = 'aa' i_info['image_download_source'] = 'local' driver_internal_info['is_whole_disk_image'] = True self.node.instance_info = i_info @@ -2138,7 +2163,6 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): i_info['image_source'] = 'http://image-ref' i_info['image_checksum'] = 'aa' i_info['root_gb'] = 10 - i_info['image_checksum'] = 'aa' d_info['image_download_source'] = 'local' driver_internal_info['is_whole_disk_image'] = True self.node.instance_info = i_info @@ -2164,6 +2188,41 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): validate_href_mock.assert_called_once_with( mock.ANY, expected_url, False) + @mock.patch.object(image_service.HttpImageService, 'validate_href', + autospec=True) + def test_build_instance_info_local_image_already_raw(self, + validate_href_mock): + cfg.CONF.set_override('image_download_source', 'local', group='agent') + i_info = self.node.instance_info + driver_internal_info = self.node.driver_internal_info + i_info['image_source'] = 'http://image-ref' + i_info['image_checksum'] = 'aa' + i_info['root_gb'] = 10 + i_info['image_disk_format'] = 'raw' + driver_internal_info['is_whole_disk_image'] = True + self.node.instance_info = i_info + self.node.driver_internal_info = driver_internal_info + self.node.save() + + expected_url = ( + 'http://172.172.24.10:8080/agent_images/%s' % self.node.uuid) + + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + + info = utils.build_instance_info_for_deploy(task) + + self.assertEqual(expected_url, info['image_url']) + self.assertEqual('aa', info['image_checksum']) + self.assertEqual('raw', info['image_disk_format']) + self.assertIsNone(info['image_os_hash_algo']) + self.assertIsNone(info['image_os_hash_value']) + self.cache_image_mock.assert_called_once_with( + task.context, task.node, force_raw=True) + self.checksum_mock.assert_not_called() + validate_href_mock.assert_called_once_with( + mock.ANY, expected_url, False) + class TestStorageInterfaceUtils(db_base.DbTestCase): def setUp(self): diff --git a/ironic/tests/unit/drivers/modules/test_inspect_utils.py b/ironic/tests/unit/drivers/modules/test_inspect_utils.py index 7cb451473..473b0ee7c 100644 --- a/ironic/tests/unit/drivers/modules/test_inspect_utils.py +++ b/ironic/tests/unit/drivers/modules/test_inspect_utils.py @@ -23,14 +23,13 @@ from ironic.common import context as ironic_context from ironic.common import exception from ironic.common import swift from ironic.conductor import task_manager +from ironic.conf import CONF from ironic.drivers.modules import inspect_utils as utils -from ironic.drivers.modules import inspector from ironic import objects from ironic.tests.unit.db import base as db_base from ironic.tests.unit.objects import utils as obj_utils sushy = importutils.try_import('sushy') -CONF = inspector.CONF @mock.patch('time.sleep', lambda sec: None) |