diff options
75 files changed, 1409 insertions, 1008 deletions
@@ -2,3 +2,4 @@ # <preferred e-mail> <other e-mail 1> # <preferred e-mail> <other e-mail 2> Joe Gordon <joe.gordon0@gmail.com> <jogo@cloudscaling.com> +Aeva Black <aeva.online@gmail.com> <devananda.vdv@gmail.com> diff --git a/devstack/lib/ironic b/devstack/lib/ironic index 8d00d8fff..bc28bb39e 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -572,7 +572,7 @@ if [[ "$IRONIC_BOOT_MODE" == "uefi" ]]; then fi # TODO(dtantsur): change this when we change the default value. -IRONIC_DEFAULT_BOOT_OPTION=${IRONIC_DEFAULT_BOOT_OPTION:-netboot} +IRONIC_DEFAULT_BOOT_OPTION=${IRONIC_DEFAULT_BOOT_OPTION:-local} if [ $IRONIC_DEFAULT_BOOT_OPTION != "netboot" ] && [ $IRONIC_DEFAULT_BOOT_OPTION != "local" ]; then die $LINENO "Supported values for IRONIC_DEFAULT_BOOT_OPTION are 'netboot' and 'local' only." fi @@ -1639,7 +1639,7 @@ function create_ironic_accounts { "$IRONIC_SERVICE_PROTOCOL://$IRONIC_HOSTPORT" # Create ironic service user - # TODO(deva): make this work with the 'service' role + # TODO(tenbrae): make this work with the 'service' role # https://bugs.launchpad.net/ironic/+bug/1605398 create_service_user "ironic" "admin" diff --git a/doc/source/admin/drivers/idrac.rst b/doc/source/admin/drivers/idrac.rst index 43988551b..72dcffcf6 100644 --- a/doc/source/admin/drivers/idrac.rst +++ b/doc/source/admin/drivers/idrac.rst @@ -586,3 +586,37 @@ the Ironic power state poll interval to 70 seconds. See .. _Ironic_RAID: https://docs.openstack.org/ironic/latest/admin/raid.html .. _iDRAC: https://www.dell.com/idracmanuals + +Vendor passthru timeout +----------------------- + +When iDRAC is not ready and executing vendor passthru commands, they take more +time as waiting for iDRAC to become ready again and then time out, for example: + +.. code-block:: bash + + openstack baremetal node passthru call --http-method GET \ + aed58dca-1b25-409a-a32f-3a817d59e1e0 list_unfinished_jobs + Timed out waiting for a reply to message ID 547ce7995342418c99ef1ea4a0054572 (HTTP 500) + +To avoid this need to increase timeout for messaging in ``/etc/ironic/ironic.conf`` +and restart Ironic API service. + +.. code-block:: ini + + [DEFAULT] + rpc_response_timeout = 600 + +Timeout when powering off +------------------------- + +Some servers might be slow when soft powering off and time out. The default retry count +is 6, resulting in 30 seconds timeout (the default retry interval set by +``post_deploy_get_power_state_retry_interval`` is 5 seconds). +To resolve this issue, increase the timeout to 90 seconds by setting the retry count to +18 as follows: + +.. code-block:: ini + + [agent] + post_deploy_get_power_state_retries = 18 diff --git a/doc/source/admin/raid.rst b/doc/source/admin/raid.rst index b8796ce5e..40a5e4bb1 100644 --- a/doc/source/admin/raid.rst +++ b/doc/source/admin/raid.rst @@ -365,11 +365,6 @@ There are certain limitations to be aware of: * Only the mandatory properties (plus the required ``controller`` property) from `Target RAID configuration`_ are currently supported. -* There is no way to select the disks which are used to set up the software - RAID, so the Ironic Python Agent will use all available disks. This seems - appropriate for servers with 2 or 4 disks, but needs to be considered when - disk arrays are attached. - * The number of created Software RAID devices must be 1 or 2. If there is only one Software RAID device, it has to be a RAID-1. If there are two, the first one has to be a RAID-1, while the RAID level for the second one can diff --git a/doc/source/contributor/jobs-description.rst b/doc/source/contributor/jobs-description.rst index 4f4bd4475..24b4fa217 100644 --- a/doc/source/contributor/jobs-description.rst +++ b/doc/source/contributor/jobs-description.rst @@ -29,19 +29,19 @@ The description of each jobs that runs in the CI when you submit a patch for * - ironic-grenade-dsvm-multinode-multitenant - Deploys Ironic in a multinode DevStack and runs upgrade for all enabled services. - * - ironic-tempest-ipa-partition-pxe_ipmitool-tinyipa-python3 - - Deploys Ironic in DevStack under Python3, configured to use tinyipa + * - ironic-tempest-ipa-partition-pxe_ipmitool + - Deploys Ironic in DevStack under Python3, configured to use dib ramdisk partition image with `pxe` boot and `ipmi` driver. Runs tempest tests that match the regex `ironic_tempest_plugin.tests.scenario` and deploy 1 virtual baremetal. - * - ironic-tempest-ipa-partition-redfish-tinyipa - - Deploys Ironic in DevStack, configured to use tinyipa ramdisk partition + * - ironic-tempest-partition-bios-redfish-pxe + - Deploys Ironic in DevStack, configured to use dib ramdisk partition image with `pxe` boot and `redfish` driver. Runs tempest tests that match the regex `ironic_tempest_plugin.tests.scenario`, also deploys 1 virtual baremetal. - * - ironic-tempest-ipa-partition-uefi-pxe_ipmitool-tinyipa - - Deploys Ironic in DevStack, configured to use tinyipa ramdisk partition + * - ironic-tempest-ipa-partition-uefi-pxe_ipmitool + - Deploys Ironic in DevStack, configured to use dib ramdisk partition image with `uefi` boot and `ipmi` driver. Runs tempest tests that match the regex `ironic_tempest_plugin.tests.scenario`, also deploys 1 virtual @@ -59,14 +59,14 @@ The description of each jobs that runs in the CI when you submit a patch for `pxe` boot and `ipmi` driver. Runs tempest tests that match the regex `ironic_tempest_plugin.tests.scenario` and deploys 1 virtual baremetal. - * - ironic-tempest-ipa-wholedisk-bios-agent_ipmitool-tinyipa-indirect - - Deploys Ironic in DevStack, configured to use a pre-build tinyipa + * - ironic-tempest-ipa-wholedisk-bios-agent_ipmitool-indirect + - Deploys Ironic in DevStack, configured to use a pre-built dib ramdisk wholedisk image that is downloaded from http url, `pxe` boot and `ipmi` driver. Runs tempest tests that match the regex `ironic_tempest_plugin.tests.scenario` and deploys 1 virtual baremetal. - * - ironic-tempest-ipa-partition-bios-agent_ipmitool-tinyipa-indirect - - Deploys Ironic in DevStack, configured to use a pre-build tinyipa + * - ironic-tempest-ipa-partition-bios-agent_ipmitool-indirect + - Deploys Ironic in DevStack, configured to use a pre-built dib ramdisk partition image that is downloaded from http url, `pxe` boot and `ipmi` driver. Runs tempest tests that match the regex @@ -84,8 +84,8 @@ The description of each jobs that runs in the CI when you submit a patch for * - ironic-tox-bandit - Runs bandit security tests in a tox environment to find known issues in the Ironic code. - * - ironic-tempest-ipa-wholedisk-bios-pxe_snmp-tinyipa - - Deploys Ironic in DevStack, configured to use a pre-build tinyipa + * - ironic-tempest-ipa-wholedisk-bios-pxe_snmp + - Deploys Ironic in DevStack, configured to use a pre-built dib ramdisk wholedisk image that is downloaded from a Swift temporary url, `pxe` boot and `snmp` driver. Runs tempest tests that match the regex @@ -96,7 +96,7 @@ The description of each jobs that runs in the CI when you submit a patch for Swift temporary url, `pxe` boot and `ipmi` driver. Runs tempest tests that match the regex `InspectorBasicTest` and deploys 1 virtual baremetal. - * - bifrost-integration-tinyipa-ubuntu-xenial + * - bifrost-integration-tinyipa-ubuntu-bionic - Tests the integration between Ironic and Bifrost. * - metalsmith-integration-glance-localboot-centos7 - Tests the integration between Ironic and Metalsmith using Glance as diff --git a/ironic/api/controllers/v1/driver.py b/ironic/api/controllers/v1/driver.py index efe6d31d6..52bd673a6 100644 --- a/ironic/api/controllers/v1/driver.py +++ b/ironic/api/controllers/v1/driver.py @@ -367,7 +367,7 @@ class DriversController(rest.RestController): @expose.expose(DriverList, str, types.boolean) def get_all(self, type=None, detail=None): """Retrieve a list of drivers.""" - # FIXME(deva): formatting of the auto-generated REST API docs + # FIXME(tenbrae): formatting of the auto-generated REST API docs # will break from a single-line doc string. # This is a result of a bug in sphinxcontrib-pecanwsme # https://github.com/dreamhost/sphinxcontrib-pecanwsme/issues/8 diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 2faf738a5..fc873a622 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -1265,8 +1265,8 @@ class Node(base.APIBase): retired_reason = wsme.wsattr(str) """Indicates the reason for a node's retirement.""" - # NOTE(deva): "conductor_affinity" shouldn't be presented on the - # API because it's an internal value. Don't add it here. + # NOTE(tenbrae): "conductor_affinity" shouldn't be presented on the + # API because it's an internal value. Don't add it here. def __init__(self, **kwargs): self.fields = [] @@ -1399,10 +1399,10 @@ class Node(base.APIBase): :type fields: list of str """ cdict = api.request.context.to_policy_values() - # NOTE(deva): the 'show_password' policy setting name exists for legacy - # purposes and can not be changed. Changing it will cause - # upgrade problems for any operators who have customized - # the value of this field + # NOTE(tenbrae): the 'show_password' policy setting name exists for + # legacy purposes and can not be changed. Changing it will + # cause upgrade problems for any operators who have + # customized the value of this field show_driver_secrets = policy.check("show_password", cdict, cdict) show_instance_secrets = policy.check("show_instance_secrets", cdict, cdict) @@ -1421,7 +1421,7 @@ class Node(base.APIBase): if not show_instance_secrets and self.instance_info != wtypes.Unset: self.instance_info = strutils.mask_dict_password( self.instance_info, "******") - # NOTE(deva): agent driver may store a swift temp_url on the + # NOTE(tenbrae): agent driver may store a swift temp_url on the # instance_info, which shouldn't be exposed to non-admin users. # Now that ironic supports additional policies, we need to hide # it here, based on this policy. @@ -2231,8 +2231,8 @@ class NodesController(rest.RestController): msg = _("Allocation UUID cannot be specified, use allocations API") raise exception.Invalid(msg) - # NOTE(deva): get_topic_for checks if node.driver is in the hash ring - # and raises NoValidHost if it is not. + # NOTE(tenbrae): get_topic_for checks if node.driver is in the hash + # ring and raises NoValidHost if it is not. # We need to ensure that node has a UUID before it can # be mapped onto the hash ring. if not node.uuid: @@ -2241,7 +2241,7 @@ class NodesController(rest.RestController): try: topic = api.request.rpcapi.get_topic_for(node) except exception.NoValidHost as e: - # NOTE(deva): convert from 404 to 400 because client can see + # NOTE(tenbrae): convert from 404 to 400 because client can see # list of available drivers and shouldn't request # one that doesn't exist. e.code = http_client.BAD_REQUEST @@ -2400,14 +2400,14 @@ class NodesController(rest.RestController): node_dict['chassis_uuid'] = node_dict.pop('chassis_id', None) node = Node(**api_utils.apply_jsonpatch(node_dict, patch)) self._update_changed_fields(node, rpc_node) - # NOTE(deva): we calculate the rpc topic here in case node.driver + # NOTE(tenbrae): we calculate the rpc topic here in case node.driver # has changed, so that update is sent to the # new conductor, not the old one which may fail to # load the new driver. try: topic = api.request.rpcapi.get_topic_for(rpc_node) except exception.NoValidHost as e: - # NOTE(deva): convert from 404 to 400 because client can see + # NOTE(tenbrae): convert from 404 to 400 because client can see # list of available drivers and shouldn't request # one that doesn't exist. e.code = http_client.BAD_REQUEST diff --git a/ironic/cmd/api.py b/ironic/cmd/api.py index 1e8dbcdf6..38ec7bd1f 100644 --- a/ironic/cmd/api.py +++ b/ironic/cmd/api.py @@ -23,6 +23,7 @@ from oslo_config import cfg from oslo_log import log try: from oslo_reports import guru_meditation_report as gmr + from oslo_reports import opts as gmr_opts except ImportError: gmr = None @@ -41,6 +42,7 @@ def main(): ironic_service.prepare_service(sys.argv) if gmr is not None: + gmr_opts.set_defaults(CONF) gmr.TextGuruMeditation.setup_autorun(version) else: LOG.debug('Guru meditation reporting is disabled ' diff --git a/ironic/cmd/conductor.py b/ironic/cmd/conductor.py index d8fbcfec9..4164db18a 100644 --- a/ironic/cmd/conductor.py +++ b/ironic/cmd/conductor.py @@ -25,6 +25,7 @@ from oslo_config import cfg from oslo_log import log try: from oslo_reports import guru_meditation_report as gmr + from oslo_reports import opts as gmr_opts except ImportError: gmr = None from oslo_service import service @@ -49,14 +50,6 @@ def warn_about_unsafe_shred_parameters(conf): 'Secure Erase. This is a possible SECURITY ISSUE!') -def warn_about_missing_default_boot_option(conf): - if not conf.deploy.default_boot_option: - LOG.warning('The default value of default_boot_option ' - 'configuration will change eventually from ' - '"netboot" to "local". It is recommended to set ' - 'an explicit value for it during the transition period') - - def warn_about_agent_token_deprecation(conf): if not conf.require_agent_token: LOG.warning('The ``[DEFAULT]require_agent_token`` option is not ' @@ -70,7 +63,6 @@ def warn_about_agent_token_deprecation(conf): def issue_startup_warnings(conf): warn_about_unsafe_shred_parameters(conf) - warn_about_missing_default_boot_option(conf) warn_about_agent_token_deprecation(conf) @@ -87,7 +79,8 @@ def main(): ironic_service.prepare_service(sys.argv) if gmr is not None: - gmr.TextGuruMeditation.setup_autorun(version) + gmr_opts.set_defaults(CONF) + gmr.TextGuruMeditation.setup_autorun(version, conf=CONF) else: LOG.debug('Guru meditation reporting is disabled ' 'because oslo.reports is not installed') diff --git a/ironic/common/driver_factory.py b/ironic/common/driver_factory.py index 3d9f42bd8..323b299d7 100644 --- a/ironic/common/driver_factory.py +++ b/ironic/common/driver_factory.py @@ -294,9 +294,9 @@ class BaseDriverFactory(object): This is subclassed to load both main drivers and extra interfaces. """ - # NOTE(deva): loading the _extension_manager as a class member will break - # stevedore when it loads a driver, because the driver will - # import this file (and thus instantiate another factory). + # NOTE(tenbrae): loading the _extension_manager as a class member will + # break stevedore when it loads a driver, because the driver + # will import this file (and thus instantiate another factory). # Instead, we instantiate a NameDispatchExtensionManager only # once, the first time DriverFactory.__init__ is called. _extension_manager = None @@ -322,12 +322,12 @@ class BaseDriverFactory(object): def get_driver(self, name): return self[name].obj - # NOTE(deva): Use lockutils to avoid a potential race in eventlet + # NOTE(tenbrae): Use lockutils to avoid a potential race in eventlet # that might try to create two driver factories. @classmethod @lockutils.synchronized(EM_SEMAPHORE) def _init_extension_manager(cls): - # NOTE(deva): In case multiple greenthreads queue up on this lock + # NOTE(tenbrae): In case multiple greenthreads queue up on this lock # before _extension_manager is initialized, prevent # creation of multiple NameDispatchExtensionManagers. if cls._extension_manager: @@ -356,8 +356,8 @@ class BaseDriverFactory(object): 'configuration file.', ', '.join(duplicated_drivers)) - # NOTE(deva): Drivers raise "DriverLoadError" if they are unable to be - # loaded, eg. due to missing external dependencies. + # NOTE(tenbrae): Drivers raise "DriverLoadError" if they are unable to + # be loaded, eg. due to missing external dependencies. # We capture that exception, and, only if it is for an # enabled driver, raise it from here. If enabled driver # raises other exception type, it is wrapped in @@ -365,7 +365,7 @@ class BaseDriverFactory(object): # caused it, and raised. If the exception is for a # non-enabled driver, we suppress it. def _catch_driver_not_found(mgr, ep, exc): - # NOTE(deva): stevedore loads plugins *before* evaluating + # NOTE(tenbrae): stevedore loads plugins *before* evaluating # _check_func, so we need to check here, too. if ep.name in cls._enabled_driver_list: if not isinstance(exc, exception.DriverLoadError): diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 26147edde..221a21ca4 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -52,7 +52,8 @@ class TemporaryFailure(IronicException): class NotAcceptable(IronicException): - # TODO(deva): We need to set response headers in the API for this exception + # TODO(tenbrae): We need to set response headers in the API + # for this exception _msg_fmt = _("Request not acceptable.") code = http_client.NOT_ACCEPTABLE diff --git a/ironic/common/policy.py b/ironic/common/policy.py index 8b7495875..3f009e9c8 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -41,7 +41,7 @@ default_policies = [ 'is_public_api:True', description='Internal flag for public API routes'), # Generic default to hide passwords in node driver_info - # NOTE(deva): the 'show_password' policy setting hides secrets in + # NOTE(tenbrae): the 'show_password' policy setting hides secrets in # driver_info. However, the name exists for legacy # purposes and can not be changed. Changing it will cause # upgrade problems for any operators who have customized @@ -74,7 +74,7 @@ default_policies = [ description='Owner of allocation'), ] -# NOTE(deva): to follow policy-in-code spec, we define defaults for +# NOTE(tenbrae): to follow policy-in-code spec, we define defaults for # the granular policies in code, rather than in policy.json. # All of these may be overridden by configuration, but we can # depend on their existence throughout the code. @@ -581,7 +581,7 @@ def init_enforcer(policy_file=None, rules=None, if _ENFORCER: return - # NOTE(deva): Register defaults for policy-in-code here so that they are + # NOTE(tenbrae): Register defaults for policy-in-code here so that they are # loaded exactly once - when this module-global is initialized. # Defining these in the relevant API modules won't work # because API classes lack singletons and don't use globals. @@ -622,7 +622,7 @@ def get_oslo_policy_enforcer(): return get_enforcer() -# NOTE(deva): We can't call these methods from within decorators because the +# NOTE(tenbrae): We can't call these methods from within decorators because the # 'target' and 'creds' parameter must be fetched from the call time # context-local pecan.request magic variable, but decorators are compiled # at module-load time. diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index c67c8cbfa..cbd7b7f0f 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -213,7 +213,7 @@ RELEASE_MAPPING = { 'VolumeTarget': ['1.0'], } }, - 'master': { + '15.0': { 'api': '1.65', 'rpc': '1.50', 'objects': { @@ -230,6 +230,23 @@ RELEASE_MAPPING = { 'VolumeTarget': ['1.0'], } }, + 'master': { + 'api': '1.65', + 'rpc': '1.50', + 'objects': { + 'Allocation': ['1.1'], + 'Node': ['1.34'], + 'Conductor': ['1.3'], + 'Chassis': ['1.3'], + 'DeployTemplate': ['1.1'], + 'Port': ['1.9'], + 'Portgroup': ['1.4'], + 'Trait': ['1.0'], + 'TraitList': ['1.0'], + 'VolumeConnector': ['1.0'], + 'VolumeTarget': ['1.0'], + } + }, } # NOTE(xek): Assign each named release to the appropriate semver. @@ -247,6 +264,7 @@ RELEASE_MAPPING = { # NOTE(mgoddard): remove Train prior to the Victoria release. RELEASE_MAPPING['train'] = RELEASE_MAPPING['13.0'] +RELEASE_MAPPING['ussuri'] = RELEASE_MAPPING['15.0'] # 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/states.py b/ironic/common/states.py index a3ec95d5a..cefdbd838 100644 --- a/ironic/common/states.py +++ b/ironic/common/states.py @@ -38,7 +38,7 @@ LOG = logging.getLogger(__name__) # Provisioning states ##################### -# TODO(deva): add add'l state mappings here +# TODO(tenbrae): add add'l state mappings here VERBS = { 'active': 'deploy', 'deleted': 'delete', @@ -315,7 +315,7 @@ for state in STABLE_STATES: machine.add_state(VERIFYING, target=MANAGEABLE, **watchers) # Add deploy* states -# NOTE(deva): Juno shows a target_provision_state of DEPLOYDONE +# NOTE(tenbrae): Juno shows a target_provision_state of DEPLOYDONE # this is changed in Kilo to ACTIVE machine.add_state(DEPLOYING, target=ACTIVE, **watchers) machine.add_state(DEPLOYWAIT, target=ACTIVE, **watchers) @@ -354,7 +354,7 @@ machine.add_transition(DEPLOYING, DEPLOYFAIL, 'fail') # A failed deployment may be retried # ironic/conductor/manager.py:do_node_deploy() machine.add_transition(DEPLOYFAIL, DEPLOYING, 'rebuild') -# NOTE(deva): Juno allows a client to send "active" to initiate a rebuild +# NOTE(tenbrae): Juno allows a client to send "active" to initiate a rebuild machine.add_transition(DEPLOYFAIL, DEPLOYING, 'deploy') # A deployment may also wait on external callbacks diff --git a/ironic/conductor/base_manager.py b/ironic/conductor/base_manager.py index 8ef568d83..cd63d875c 100644 --- a/ironic/conductor/base_manager.py +++ b/ironic/conductor/base_manager.py @@ -118,7 +118,8 @@ class BaseConductorManager(object): _check_enabled_interfaces() - # NOTE(deva): these calls may raise DriverLoadError or DriverNotFound + # NOTE(tenbrae): these calls may raise DriverLoadError or + # DriverNotFound # NOTE(vdrok): Instantiate network and storage interface factory on # startup so that all the interfaces are loaded at the very # beginning, and failures prevent the conductor from starting. @@ -296,6 +297,8 @@ class BaseConductorManager(object): return self._shutdown = True self._keepalive_evt.set() + # clear all locks held by this conductor before deregistering + self.dbapi.clear_node_reservations_for_conductor(self.host) if deregister: try: # Inform the cluster that this conductor is shutting down. diff --git a/ironic/conductor/deployments.py b/ironic/conductor/deployments.py index 26df423aa..152bbce43 100644 --- a/ironic/conductor/deployments.py +++ b/ironic/conductor/deployments.py @@ -281,8 +281,8 @@ def do_next_deploy_step(task, step_index, conductor_id): # Check if the step is done or not. The step should return # states.DEPLOYWAIT if the step is still being executed, or # None if the step is done. - # NOTE(deva): Some drivers may return states.DEPLOYWAIT - # eg. if they are waiting for a callback + # NOTE(tenbrae): Some drivers may return states.DEPLOYWAIT + # eg. if they are waiting for a callback if result == states.DEPLOYWAIT: # Kill this worker, the async step will make an RPC call to # continue_node_deploy() to continue deploying diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 4cc0167a2..4a588c874 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -1004,13 +1004,13 @@ class ConductorManager(base_manager.BaseConductorManager): node.last_error = _("Failed to tear down. Error: %s") % e task.process_event('error') else: - # NOTE(deva): When tear_down finishes, the deletion is done, + # NOTE(tenbrae): When tear_down finishes, the deletion is done, # cleaning will start next LOG.info('Successfully unprovisioned node %(node)s with ' 'instance %(instance)s.', {'node': node.uuid, 'instance': node.instance_uuid}) finally: - # NOTE(deva): there is no need to unset conductor_affinity + # NOTE(tenbrae): there is no need to unset conductor_affinity # because it is a reference to the most recent conductor which # deployed a node, and does not limit any future actions. # But we do need to clear the instance-related fields. @@ -1463,7 +1463,7 @@ class ConductorManager(base_manager.BaseConductorManager): with task_manager.acquire(context, node_uuid, purpose='power state sync', shared=True) as task: - # NOTE(deva): we should not acquire a lock on a node in + # NOTE(tenbrae): we should not acquire a lock on a node in # DEPLOYWAIT/CLEANWAIT, as this could cause # an error within a deploy ramdisk POSTing back # at the same time. @@ -1883,7 +1883,7 @@ class ConductorManager(base_manager.BaseConductorManager): try: with task_manager.acquire(context, node_uuid, purpose='node take over') as task: - # NOTE(deva): now that we have the lock, check again to + # NOTE(tenbrae): now that we have the lock, check again to # avoid racing with deletes and other state changes node = task.node if (node.maintenance @@ -2235,7 +2235,7 @@ class ConductorManager(base_manager.BaseConductorManager): try: if enabled: task.driver.console.start_console(task) - # TODO(deva): We should be updating conductor_affinity here + # TODO(tenbrae): We should be updating conductor_affinity here # but there is no support for console sessions in # take_over() right now. else: @@ -3114,7 +3114,7 @@ class ConductorManager(base_manager.BaseConductorManager): 'Invalid or missing agent token received.') else: LOG.warning('Out of date agent detected for node ' - '%(node)s. Agent version %(version) ' + '%(node)s. Agent version %(version)s ' 'reported. Support for this version is ' 'deprecated.', {'node': task.node.uuid, diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index f3a64824b..4b1929b02 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -132,7 +132,7 @@ class ConductorAPI(object): serializer=serializer) use_groups = self.client.can_send_version('1.47') - # NOTE(deva): this is going to be buggy + # NOTE(tenbrae): this is going to be buggy self.ring_manager = hash_ring.HashRingManager(use_groups=use_groups) def get_conductor_for(self, node): diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index e537219c6..ca399b651 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -369,7 +369,7 @@ def provisioning_error_handler(e, node, provision_state, """ if isinstance(e, exception.NoFreeConductorWorker): - # NOTE(deva): there is no need to clear conductor_affinity + # NOTE(tenbrae): there is no need to clear conductor_affinity # because it isn't updated on a failed deploy node.provision_state = provision_state node.target_provision_state = target_provision_state @@ -499,7 +499,7 @@ def deploying_error_handler(task, logmsg, errmsg=None, traceback=False, node.last_error = cleanup_err node.save() - # NOTE(deva): there is no need to clear conductor_affinity + # NOTE(tenbrae): there is no need to clear conductor_affinity task.process_event('fail') diff --git a/ironic/conf/deploy.py b/ironic/conf/deploy.py index 092238ec8..500324f1c 100644 --- a/ironic/conf/deploy.py +++ b/ironic/conf/deploy.py @@ -80,11 +80,11 @@ opts = [ cfg.StrOpt('default_boot_option', choices=[('netboot', _('boot from a network')), ('local', _('local boot'))], + default='local', help=_('Default boot option to use when no boot option is ' - 'requested in node\'s driver_info. Currently the ' - 'default is "netboot", but it will be changed to ' - '"local" in the future. It is recommended to set ' - 'an explicit value for this option.')), + 'requested in node\'s driver_info. Defaults to ' + '"local". Prior to the Ussuri release, the default ' + 'was "netboot".')), cfg.StrOpt('default_boot_mode', choices=[(boot_modes.UEFI, _('UEFI boot mode')), (boot_modes.LEGACY_BIOS, _('Legacy BIOS boot mode'))], diff --git a/ironic/db/sqlalchemy/alembic/versions/5674c57409b9_replace_nostate_with_available.py b/ironic/db/sqlalchemy/alembic/versions/5674c57409b9_replace_nostate_with_available.py index 00f73e3da..1cccd1842 100644 --- a/ironic/db/sqlalchemy/alembic/versions/5674c57409b9_replace_nostate_with_available.py +++ b/ironic/db/sqlalchemy/alembic/versions/5674c57409b9_replace_nostate_with_available.py @@ -31,10 +31,10 @@ node = table('nodes', column('provision_state', String(15))) -# NOTE(deva): We must represent the states as static strings in this migration -# file, rather than import ironic.common.states, because that file may change -# in the future. This migration script must still be able to be run with -# future versions of the code and still produce the same results. +# NOTE(tenbrae): We must represent the states as static strings in this +# migration file, rather than import ironic.common.states, because that file +# may change in the future. This migration script must still be able to be +# run with future versions of the code and still produce the same results. AVAILABLE = 'available' diff --git a/ironic/db/sqlalchemy/models.py b/ironic/db/sqlalchemy/models.py index e1df97ace..fa56c2611 100644 --- a/ironic/db/sqlalchemy/models.py +++ b/ironic/db/sqlalchemy/models.py @@ -128,7 +128,7 @@ class Node(Base): table_args()) id = Column(Integer, primary_key=True) uuid = Column(String(36)) - # NOTE(deva): we store instance_uuid directly on the node so that we can + # NOTE(tenbrae): we store instance_uuid directly on the node so that we can # filter on it more efficiently, even though it is # user-settable, and would otherwise be in node.properties. instance_uuid = Column(String(36), nullable=True) @@ -152,12 +152,12 @@ class Node(Base): raid_config = Column(db_types.JsonEncodedDict) target_raid_config = Column(db_types.JsonEncodedDict) - # NOTE(deva): this is the host name of the conductor which has + # NOTE(tenbrae): this is the host name of the conductor which has # acquired a TaskManager lock on the node. # We should use an INT FK (conductors.id) in the future. reservation = Column(String(255), nullable=True) - # NOTE(deva): this is the id of the last conductor which prepared local + # NOTE(tenbrae): this is the id of the last conductor which prepared local # state for the node (eg, a PXE config file). # When affinity and the hash ring's mapping do not match, # this indicates that a conductor should rebuild local state. diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 787ea21fe..9a9bb94b3 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -563,6 +563,12 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface): # 'instance_info' to 'local for backward compatibility. # TODO(stendulker): Fail here once the default boot # option is local. + # NOTE(TheJulia): Fixing the default boot mode only + # masks the failure as the lack of a user definition + # can be perceived as both an invalid configuration and + # reliance upon the default configuration. The reality + # being that in most scenarios, users do not want network + # booting, so the changed default should be valid. with excutils.save_and_reraise_exception(reraise=False) as ctx: instance_info = node.instance_info capabilities = utils.parse_instance_info_capabilities(node) diff --git a/ironic/drivers/modules/agent_base.py b/ironic/drivers/modules/agent_base.py index 70ea04411..be7a7dd73 100644 --- a/ironic/drivers/modules/agent_base.py +++ b/ironic/drivers/modules/agent_base.py @@ -459,6 +459,85 @@ class HeartbeatMixin(object): 'maintenance mode; not taking any action.', {'node': node.uuid}) + def _heartbeat_deploy_wait(self, task): + msg = _('Failed checking if deploy is done') + node = task.node + try: + # NOTE(mgoddard): Only handle heartbeats during DEPLOYWAIT if we + # are currently in the core deploy.deploy step. Other deploy steps + # may cause the agent to boot, but we should not trigger deployment + # at that point if the driver is polling for completion of a step. + if self.in_core_deploy_step(task): + if not self.deploy_has_started(task): + msg = _('Node failed to deploy') + self.continue_deploy(task) + elif self.deploy_is_done(task): + msg = _('Node failed to move to active state') + self.reboot_to_instance(task) + else: + node.touch_provisioning() + else: + # The exceptions from RPC are not possible as we using cast + # here + # Check if the driver is polling for completion of a step, + # via the 'deployment_polling' flag. + polling = node.driver_internal_info.get( + 'deployment_polling', False) + if not polling: + manager_utils.notify_conductor_resume_deploy(task) + node.touch_provisioning() + except Exception as e: + last_error = _('%(msg)s. Error: %(exc)s') % {'msg': msg, 'exc': e} + LOG.exception('Asynchronous exception for node %(node)s: %(err)s', + {'node': task.node.uuid, 'err': last_error}) + # Do not call the error handler is the node is already DEPLOYFAIL + if node.provision_state in (states.DEPLOYING, states.DEPLOYWAIT): + deploy_utils.set_failed_state( + task, last_error, collect_logs=bool(self._client)) + + def _heartbeat_clean_wait(self, task): + node = task.node + msg = _('Failed checking if cleaning is done') + try: + node.touch_provisioning() + if not node.clean_step: + LOG.debug('Node %s just booted to start cleaning.', + node.uuid) + msg = _('Node failed to start the first cleaning step') + # First, cache the clean steps + self.refresh_clean_steps(task) + # Then set/verify node clean steps and start cleaning + conductor_steps.set_node_cleaning_steps(task) + # The exceptions from RPC are not possible as we using cast + # here + manager_utils.notify_conductor_resume_clean(task) + else: + msg = _('Node failed to check cleaning progress') + # Check if the driver is polling for completion of a step, + # via the 'cleaning_polling' flag. + polling = node.driver_internal_info.get( + 'cleaning_polling', False) + if not polling: + self.continue_cleaning(task) + except Exception as e: + last_error = _('%(msg)s. Error: %(exc)s') % {'msg': msg, 'exc': e} + LOG.exception('Asynchronous exception for node %(node)s: %(err)s', + {'node': task.node.uuid, 'err': last_error}) + if node.provision_state in (states.CLEANING, states.CLEANWAIT): + manager_utils.cleaning_error_handler(task, last_error) + + def _heartbeat_rescue_wait(self, task): + msg = _('Node failed to perform rescue operation') + try: + self._finalize_rescue(task) + except Exception as e: + last_error = _('%(msg)s. Error: %(exc)s') % {'msg': msg, 'exc': e} + LOG.exception('Asynchronous exception for node %(node)s: %(err)s', + {'node': task.node.uuid, 'err': last_error}) + if task.node.provision_state in (states.RESCUING, + states.RESCUEWAIT): + manager_utils.rescuing_error_handler(task, last_error) + @METRICS.timer('HeartbeatMixin.heartbeat') def heartbeat(self, task, callback_url, agent_version): """Process a heartbeat. @@ -508,71 +587,12 @@ class HeartbeatMixin(object): if node.maintenance: return self._heartbeat_in_maintenance(task) - # Async call backs don't set error state on their own - # TODO(jimrollenhagen) improve error messages here - msg = _('Failed checking if deploy is done.') - try: - # NOTE(mgoddard): Only handle heartbeats during DEPLOYWAIT if we - # are currently in the core deploy.deploy step. Other deploy steps - # may cause the agent to boot, but we should not trigger deployment - # at that point if the driver is polling for completion of a step. - if node.provision_state == states.DEPLOYWAIT: - if self.in_core_deploy_step(task): - if not self.deploy_has_started(task): - msg = _('Node failed to deploy.') - self.continue_deploy(task) - elif self.deploy_is_done(task): - msg = _('Node failed to move to active state.') - self.reboot_to_instance(task) - else: - node.touch_provisioning() - else: - # The exceptions from RPC are not possible as we using cast - # here - # Check if the driver is polling for completion of a step, - # via the 'deployment_polling' flag. - polling = node.driver_internal_info.get( - 'deployment_polling', False) - if not polling: - manager_utils.notify_conductor_resume_deploy(task) - node.touch_provisioning() - elif node.provision_state == states.CLEANWAIT: - node.touch_provisioning() - if not node.clean_step: - LOG.debug('Node %s just booted to start cleaning.', - node.uuid) - msg = _('Node failed to start the first cleaning step.') - # First, cache the clean steps - self.refresh_clean_steps(task) - # Then set/verify node clean steps and start cleaning - conductor_steps.set_node_cleaning_steps(task) - # The exceptions from RPC are not possible as we using cast - # here - manager_utils.notify_conductor_resume_clean(task) - else: - msg = _('Node failed to check cleaning progress.') - # Check if the driver is polling for completion of a step, - # via the 'cleaning_polling' flag. - polling = node.driver_internal_info.get( - 'cleaning_polling', False) - if not polling: - self.continue_cleaning(task) - elif (node.provision_state == states.RESCUEWAIT): - msg = _('Node failed to perform rescue operation.') - self._finalize_rescue(task) - except Exception as e: - err_info = {'msg': msg, 'e': e} - last_error = _('Asynchronous exception: %(msg)s ' - 'Exception: %(e)s for node') % err_info - errmsg = last_error + ' %(node)s' - LOG.exception(errmsg, {'node': node.uuid}) - if node.provision_state in (states.CLEANING, states.CLEANWAIT): - manager_utils.cleaning_error_handler(task, last_error) - elif node.provision_state in (states.DEPLOYING, states.DEPLOYWAIT): - deploy_utils.set_failed_state( - task, last_error, collect_logs=bool(self._client)) - elif node.provision_state in (states.RESCUING, states.RESCUEWAIT): - manager_utils.rescuing_error_handler(task, last_error) + if node.provision_state == states.DEPLOYWAIT: + self._heartbeat_deploy_wait(task) + elif node.provision_state == states.CLEANWAIT: + self._heartbeat_clean_wait(task) + elif node.provision_state == states.RESCUEWAIT: + self._heartbeat_rescue_wait(task) def _finalize_rescue(self, task): """Call ramdisk to prepare rescue mode and verify result. @@ -671,7 +691,7 @@ class AgentDeployMixin(HeartbeatMixin): if missing: _raise(step_type, _( 'agent get_%(type)s_steps for node %(node)s returned ' - 'an invalid %(type)s step. Keys: %(keys)s are missing' + 'an invalid %(type)s step. Keys: %(keys)s are missing ' 'from step: %(step)s.') % ({'node': node.uuid, 'keys': missing, 'step': step, @@ -704,6 +724,67 @@ class AgentDeployMixin(HeartbeatMixin): """ return execute_step(task, step, 'clean') + def _process_version_mismatch(self, task, step_type): + node = task.node + # For manual clean, the target provision state is MANAGEABLE, whereas + # for automated cleaning, it is (the default) AVAILABLE. + manual_clean = node.target_provision_state == states.MANAGEABLE + + # Cache the new clean steps (and 'hardware_manager_version') + try: + self.refresh_steps(task, step_type) + except exception.NodeCleaningFailure as e: + msg = (_('Could not continue cleaning on node ' + '%(node)s: %(err)s.') % + {'node': node.uuid, 'err': e}) + LOG.exception(msg) + return manager_utils.cleaning_error_handler(task, msg) + except exception.InstanceDeployFailure as e: + msg = (_('Could not continue deployment on node ' + '%(node)s: %(err)s.') % + {'node': node.uuid, 'err': e}) + LOG.exception(msg) + return manager_utils.deploying_error_handler(task, msg) + + if manual_clean: + # Don't restart manual cleaning if agent reboots to a new + # version. Both are operator actions, unlike automated + # cleaning. Manual clean steps are not necessarily idempotent + # like automated clean steps and can be even longer running. + LOG.info('During manual cleaning, node %(node)s detected ' + 'a clean version mismatch. Re-executing and ' + 'continuing from current step %(step)s.', + {'node': node.uuid, 'step': node.clean_step}) + + driver_internal_info = node.driver_internal_info + driver_internal_info['skip_current_clean_step'] = False + node.driver_internal_info = driver_internal_info + node.save() + else: + # Restart the process, agent must have rebooted to new version + LOG.info('During %(type)s, node %(node)s detected a ' + '%(type)s version mismatch. Resetting %(type)s steps ' + 'and rebooting the node.', + {'type': step_type, 'node': node.uuid}) + try: + conductor_steps.set_node_cleaning_steps(task) + except exception.NodeCleaningFailure as e: + msg = (_('Could not restart automated cleaning on node ' + '%(node)s after step %(step)s: %(err)s.') % + {'node': node.uuid, 'err': e, + 'step': node.clean_step}) + LOG.exception(msg) + return manager_utils.cleaning_error_handler(task, msg) + except exception.InstanceDeployFailure as e: + msg = (_('Could not restart deployment on node ' + '%(node)s after step %(step)s: %(err)s.') % + {'node': node.uuid, 'err': e, + 'step': node.deploy_step}) + LOG.exception(msg) + return manager_utils.deploying_error_handler(task, msg) + + manager_utils.notify_conductor_resume_operation(task, step_type) + @METRICS.timer('AgentDeployMixin.process_next_step') def process_next_step(self, task, step_type, **kwargs): """Start the next clean/deploy step if the previous one is complete. @@ -724,9 +805,6 @@ class AgentDeployMixin(HeartbeatMixin): assert step_type in ('clean', 'deploy') node = task.node - # For manual clean, the target provision state is MANAGEABLE, whereas - # for automated cleaning, it is (the default) AVAILABLE. - manual_clean = node.target_provision_state == states.MANAGEABLE agent_commands = self._client.get_commands_status(task.node) if not agent_commands: @@ -770,61 +848,7 @@ class AgentDeployMixin(HeartbeatMixin): return manager_utils.cleaning_error_handler(task, msg) elif command.get('command_status') in ('CLEAN_VERSION_MISMATCH', 'DEPLOY_VERSION_MISMATCH'): - # Cache the new clean steps (and 'hardware_manager_version') - try: - self.refresh_steps(task, step_type) - except exception.NodeCleaningFailure as e: - msg = (_('Could not continue cleaning on node ' - '%(node)s: %(err)s.') % - {'node': node.uuid, 'err': e}) - LOG.exception(msg) - return manager_utils.cleaning_error_handler(task, msg) - except exception.InstanceDeployFailure as e: - msg = (_('Could not continue deployment on node ' - '%(node)s: %(err)s.') % - {'node': node.uuid, 'err': e}) - LOG.exception(msg) - return manager_utils.deploying_error_handler(task, msg) - - if manual_clean: - # Don't restart manual cleaning if agent reboots to a new - # version. Both are operator actions, unlike automated - # cleaning. Manual clean steps are not necessarily idempotent - # like automated clean steps and can be even longer running. - LOG.info('During manual cleaning, node %(node)s detected ' - 'a clean version mismatch. Re-executing and ' - 'continuing from current step %(step)s.', - {'node': node.uuid, 'step': node.clean_step}) - - driver_internal_info = node.driver_internal_info - driver_internal_info['skip_current_clean_step'] = False - node.driver_internal_info = driver_internal_info - node.save() - else: - # Restart the process, agent must have rebooted to new version - LOG.info('During %(type)s, node %(node)s detected a ' - '%(type)s version mismatch. Resetting %(type)s steps ' - 'and rebooting the node.', - {'type': step_type, 'node': node.uuid}) - try: - conductor_steps.set_node_cleaning_steps(task) - except exception.NodeCleaningFailure as e: - msg = (_('Could not restart automated cleaning on node ' - '%(node)s after step %(step)s: %(err)s.') % - {'node': node.uuid, 'err': e, - 'step': node.clean_step}) - LOG.exception(msg) - return manager_utils.cleaning_error_handler(task, msg) - except exception.InstanceDeployFailure as e: - msg = (_('Could not restart deployment on node ' - '%(node)s after step %(step)s: %(err)s.') % - {'node': node.uuid, 'err': e, - 'step': node.deploy_step}) - LOG.exception(msg) - return manager_utils.deploying_error_handler(task, msg) - - manager_utils.notify_conductor_resume_operation(task, step_type) - + self._process_version_mismatch(task, step_type) elif command.get('command_status') == 'SUCCEEDED': step_hook = _get_post_step_hook(node, step_type) if step_hook is not None: @@ -1013,6 +1037,9 @@ class AgentDeployMixin(HeartbeatMixin): on encountering error while setting the boot device on the node. """ node = task.node + # Almost never taken into account on agent side, just used for softraid + # Can be useful with whole_disk_images + target_boot_mode = boot_mode_utils.get_boot_mode(task.node) LOG.debug('Configuring local boot for node %s', node.uuid) # If the target RAID configuration is set to 'software' for the @@ -1067,7 +1094,9 @@ class AgentDeployMixin(HeartbeatMixin): result = self._client.install_bootloader( node, root_uuid=root_uuid, efi_system_part_uuid=efi_system_part_uuid, - prep_boot_part_uuid=prep_boot_part_uuid) + prep_boot_part_uuid=prep_boot_part_uuid, + target_boot_mode=target_boot_mode + ) if result['command_status'] == 'FAILED': if not whole_disk_image: msg = (_("Failed to install a bootloader when " diff --git a/ironic/drivers/modules/agent_client.py b/ironic/drivers/modules/agent_client.py index 2644fa46d..4da9bc69e 100644 --- a/ironic/drivers/modules/agent_client.py +++ b/ironic/drivers/modules/agent_client.py @@ -45,7 +45,7 @@ class AgentClient(object): raise exception.IronicException(_('Agent driver requires ' 'agent_url in ' 'driver_internal_info')) - return ('%(agent_url)s/%(api_version)s/commands' % + return ('%(agent_url)s/%(api_version)s/commands/' % {'agent_url': agent_url, 'api_version': CONF.agent.agent_api_version}) @@ -259,12 +259,14 @@ class AgentClient(object): wait=True) @METRICS.timer('AgentClient.install_bootloader') - def install_bootloader(self, node, root_uuid, efi_system_part_uuid=None, + def install_bootloader(self, node, root_uuid, target_boot_mode, + efi_system_part_uuid=None, prep_boot_part_uuid=None): """Install a boot loader on the image. :param node: A node object. :param root_uuid: The UUID of the root partition. + :param target_boot_mode: The target deployment boot mode. :param efi_system_part_uuid: The UUID of the efi system partition where the bootloader will be installed to, only used for uefi boot mode. @@ -279,7 +281,9 @@ class AgentClient(object): """ params = {'root_uuid': root_uuid, 'efi_system_part_uuid': efi_system_part_uuid, - 'prep_boot_part_uuid': prep_boot_part_uuid} + 'prep_boot_part_uuid': prep_boot_part_uuid, + 'target_boot_mode': target_boot_mode + } # NOTE(TheJulia): This command explicitly sends a larger timeout # factor to the _command call such that the agent ramdisk has enough @@ -289,11 +293,34 @@ class AgentClient(object): # compatible. We could at least begin to delineate the commands apart # over the next cycle or two so we don't need a command timeout # extension factor. - return self._command(node=node, - method='image.install_bootloader', - params=params, - wait=True, - command_timeout_factor=2) + try: + return self._command(node=node, + method='image.install_bootloader', + params=params, + wait=True, + command_timeout_factor=2) + except exception.AgentAPIError: + # NOTE(arne_wiebalck): If we require to pass 'uefi' as the boot + # mode, but find that the IPA does not yet support the additional + # 'target_boot_mode' parameter, we need to fail. For 'bios' boot + # mode on the other hand we can retry without the parameter, + # since 'bios' is the default value the IPA will use. + if target_boot_mode == 'uefi': + LOG.error('Unable to pass UEFI boot mode to an out of date ' + 'agent ramdisk. Please contact the administrator ' + 'to update the ramdisk to contain an ' + 'ironic-python-agent version of at least 6.0.0.') + raise + else: + params = {'root_uuid': root_uuid, + 'efi_system_part_uuid': efi_system_part_uuid, + 'prep_boot_part_uuid': prep_boot_part_uuid + } + return self._command(node=node, + method='image.install_bootloader', + params=params, + wait=True, + command_timeout_factor=2) @METRICS.timer('AgentClient.get_clean_steps') def get_clean_steps(self, node, ports): diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 582c33423..56ae0cbdf 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -16,7 +16,9 @@ import os import re +import shutil import time +from urllib import parse as urlparse from ironic_lib import metrics_utils from ironic_lib import utils as il_utils @@ -276,7 +278,7 @@ def set_failed_state(task, msg, collect_logs=True): 'should be removed from Ironic or put in maintenance ' 'mode until the problem is resolved.' % node.uuid) LOG.exception(msg2) - # NOTE(deva): node_power_action() erases node.last_error + # NOTE(tenbrae): node_power_action() erases node.last_error # so we need to set it here. node.last_error = msg node.save() @@ -497,7 +499,8 @@ def validate_image_properties(ctx, deploy_info, properties): def get_default_boot_option(): """Gets the default boot option.""" - return CONF.deploy.default_boot_option or 'netboot' + # TODO(TheJulia): Deprecated: Remove after Ussuri. + return CONF.deploy.default_boot_option def get_boot_option(node): @@ -514,7 +517,8 @@ def get_boot_option(node): if is_software_raid(node): return 'local' capabilities = utils.parse_instance_info_capabilities(node) - return capabilities.get('boot_option', get_default_boot_option()).lower() + return capabilities.get('boot_option', + CONF.deploy.default_boot_option).lower() def is_software_raid(node): @@ -822,6 +826,35 @@ def direct_deploy_should_convert_raw_image(node): return CONF.force_raw_images and CONF.agent.stream_raw_images +def copy_image_to_web_server(source_file_path, destination): + """Copies the given image to the http web server. + + This method copies the given image to the http_root location. + It enables read-write access to the image else the deploy fails + as the image file at the web_server url is inaccessible. + + :param source_file_path: The absolute path of the image file + which needs to be copied to the + web server root. + :param destination: The name of the file that + will contain the copied image. + :raises: ImageUploadFailed exception if copying the source + file to the web server fails. + :returns: image url after the source image is uploaded. + + """ + + image_url = urlparse.urljoin(CONF.deploy.http_url, destination) + image_path = os.path.join(CONF.deploy.http_root, destination) + try: + shutil.copyfile(source_file_path, image_path) + except IOError as exc: + raise exception.ImageUploadFailed(image_name=destination, + web_server=CONF.deploy.http_url, + reason=exc) + return image_url + + @image_cache.cleanup(priority=50) class InstanceImageCache(image_cache.ImageCache): diff --git a/ironic/drivers/modules/drac/power.py b/ironic/drivers/modules/drac/power.py index 3ace62f7d..a42b4d5d9 100644 --- a/ironic/drivers/modules/drac/power.py +++ b/ironic/drivers/modules/drac/power.py @@ -15,6 +15,8 @@ DRAC power interface """ +import time + from ironic_lib import metrics_utils from oslo_log import log as logging from oslo_utils import importutils @@ -43,6 +45,10 @@ if drac_constants: REVERSE_POWER_STATES = dict((v, k) for (k, v) in POWER_STATES.items()) +POWER_STATE_TRIES = 15 +POWER_STATE_SLEEP = 2 +POWER_STATE_CHANGE_FAIL = 'The command failed to set RequestedState' + def _get_power_state(node): """Returns the current power state of the node. @@ -101,18 +107,60 @@ def _set_power_state(node, power_state): _commit_boot_list_change(node) client = drac_common.get_drac_client(node) - target_power_state = REVERSE_POWER_STATES[power_state] - - try: - client.set_power_state(target_power_state) - except drac_exceptions.BaseClientException as exc: - LOG.error('DRAC driver failed to set power state for node ' - '%(node_uuid)s to %(power_state)s. ' - 'Reason: %(error)s.', - {'node_uuid': node.uuid, - 'power_state': power_state, - 'error': exc}) - raise exception.DracOperationError(error=exc) + tries = POWER_STATE_TRIES + + # Cases have been seen where the iDRAC returns a SYS021 error even when + # the server is in the right power state and a valid power state change + # is attempted. Retry in this case. + while tries > 0: + # The iDRAC will return a SYS021 error if the server is powered off + # and a reboot is requested. In this situation, convert the requested + # reboot into a power on to avoid this error. To minimize the chance + # of a race condition, it is critical to do this check immediately + # before sending the power state change command. This keeps the + # window during which the server could change power states without us + # knowing about it as small as possible. + calc_power_state = power_state + if power_state == states.REBOOT: + current_power_state = _get_power_state(node) + # If the server is not on, then power it on instead of rebooting + if current_power_state != states.POWER_ON: + calc_power_state = states.POWER_ON + + target_power_state = REVERSE_POWER_STATES[calc_power_state] + + try: + client.set_power_state(target_power_state) + break + except drac_exceptions.BaseClientException as exc: + if (power_state == states.REBOOT + and POWER_STATE_CHANGE_FAIL in str(exc) + and tries > 0): + LOG.warning('DRAC driver failed to set power state for node ' + '%(node_uuid)s to %(calc_power_state)s. ' + 'Reason: %(error)s. Retrying...', + {'node_uuid': node.uuid, + 'calc_power_state': calc_power_state, + 'error': exc}) + tries -= 1 + time.sleep(POWER_STATE_SLEEP) + else: + LOG.error('DRAC driver failed to set power state for node ' + '%(node_uuid)s to %(calc_power_state)s. ' + 'Reason: %(error)s.', + {'node_uuid': node.uuid, + 'calc_power_state': calc_power_state, + 'error': exc}) + raise exception.DracOperationError(error=exc) + + if tries <= 0: + error_msg = (_('DRAC driver timed out while trying to set the power ' + 'state for node %(node_uuid)s to ' + '%(calc_power_state)s.') % + {'node_uuid': node.uuid, + 'calc_power_state': calc_power_state}) + LOG.error(error_msg) + raise exception.DracOperationError(error_msg) class DracRedfishPower(redfish_power.RedfishPower): @@ -199,13 +247,7 @@ class DracWSManPower(base.PowerInterface): "timeout=%(timeout)s", {'timeout': timeout}) - current_power_state = _get_power_state(task.node) - if current_power_state == states.POWER_ON: - target_power_state = states.REBOOT - else: - target_power_state = states.POWER_ON - - _set_power_state(task.node, target_power_state) + _set_power_state(task.node, states.REBOOT) class DracPower(DracWSManPower): diff --git a/ironic/drivers/modules/drac/raid.py b/ironic/drivers/modules/drac/raid.py index 4e5cbce18..b5f30d641 100644 --- a/ironic/drivers/modules/drac/raid.py +++ b/ironic/drivers/modules/drac/raid.py @@ -680,7 +680,7 @@ def _calculate_volume_props(logical_disk, physical_disks, free_space_mb): error_msg = _('invalid number of physical disks was provided') raise exception.DracOperationError(error=error_msg) - disks_per_span = len(selected_disks) / spans_count + disks_per_span = int(len(selected_disks) / spans_count) # Best practice is to not pass span_length and span_depth when creating a # RAID10. The iDRAC will dynamically calculate these values using maximum diff --git a/ironic/drivers/modules/ilo/boot.py b/ironic/drivers/modules/ilo/boot.py index b6142bb8b..8c93ccc97 100644 --- a/ironic/drivers/modules/ilo/boot.py +++ b/ironic/drivers/modules/ilo/boot.py @@ -16,8 +16,6 @@ Boot Interface for iLO drivers and its supporting methods. """ import os -import tempfile -from urllib import parse as urlparse from ironic_lib import metrics_utils from ironic_lib import utils as ironic_utils @@ -40,6 +38,7 @@ from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.ilo import common as ilo_common from ironic.drivers.modules import ipxe from ironic.drivers.modules import pxe +from ironic.drivers.modules import virtual_media_base LOG = logging.getLogger(__name__) @@ -47,16 +46,45 @@ METRICS = metrics_utils.get_metrics_logger(__name__) CONF = cfg.CONF -REQUIRED_PROPERTIES = { +DEPLOY_ISO_PROPERTIES = { 'ilo_deploy_iso': _("UUID (from Glance) of the deployment ISO. " "Required.") } -RESCUE_PROPERTIES = { + +DEPLOY_RAMDISK_PROPERTIES = { + 'deploy_kernel': _("URL or Glance UUID of the deployment kernel. " + "Required."), + 'deploy_ramdisk': _("URL or Glance UUID of the ramdisk that is " + "mounted at boot time. Required.") +} + +OPTIONAL_PROPERTIES = { + 'bootloader': _("URL or Glance UUID of the EFI system partition " + "image containing EFI boot loader. This image will be " + "used by ironic when building UEFI-bootable ISO " + "out of kernel and ramdisk. Required for UEFI " + "boot from partition images.") +} + +RESCUE_ISO_PROPERTIES = { 'ilo_rescue_iso': _("UUID (from Glance) of the rescue ISO. Only " "required if rescue mode is being used and ironic is " "managing booting the rescue ramdisk.") } -COMMON_PROPERTIES = REQUIRED_PROPERTIES + +RESCUE_RAMDISK_PROPERTIES = { + 'rescue_kernel': _("URL or Glance UUID of the rescue kernel. " + "Required."), + 'rescue_ramdisk': _("URL or Glance UUID of the ramdisk that is " + "mounted at boot time. Required.") +} + +COMMON_PROPERTIES = DEPLOY_ISO_PROPERTIES + +KERNEL_RAMDISK_LABELS = { + 'deploy': DEPLOY_RAMDISK_PROPERTIES, + 'rescue': RESCUE_RAMDISK_PROPERTIES +} def parse_driver_info(node, mode='deploy'): @@ -77,24 +105,35 @@ def parse_driver_info(node, mode='deploy'): """ info = node.driver_info d_info = {} - if mode == 'rescue': + if mode == 'rescue' and info.get('ilo_rescue_iso'): d_info['ilo_rescue_iso'] = info.get('ilo_rescue_iso') - else: + elif mode == 'deploy' and info.get('ilo_deploy_iso'): d_info['ilo_deploy_iso'] = info.get('ilo_deploy_iso') + else: + params_to_check = KERNEL_RAMDISK_LABELS[mode] - error_msg = (_("Error validating iLO virtual media for %s. Some " - "parameters were missing in node's driver_info") % mode) - deploy_utils.check_for_missing_params(d_info, error_msg) + d_info = {option: info.get(option) + for option in params_to_check} - return d_info + if not any(d_info.values()): + # NOTE(dtantsur): avoid situation when e.g. deploy_kernel comes + # from driver_info but deploy_ramdisk comes from configuration, + # since it's a sign of a potential operator's mistake. + d_info = {k: getattr(CONF.conductor, k) + for k in params_to_check} + + error_msg = (_("Error validating iLO virtual media for %(mode)s. " + "Either 'ilo_%(mode)s_iso' is missing or " + "DEPLOY_RAMDISK_PROPERTIES or RESCUE_RAMDISK_PROPERTIES " + "were missing in node's driver_info.") % {'mode': mode}) + deploy_utils.check_for_missing_params(d_info, error_msg) -def _get_boot_iso_object_name(node): - """Returns the boot iso object name for a given node. + d_info.update( + {option: info.get(option, getattr(CONF.conductor, option, None)) + for option in OPTIONAL_PROPERTIES}) - :param node: the node for which object name is to be provided. - """ - return "boot-%s" % node.uuid + return d_info def _get_boot_iso(task, root_uuid): @@ -171,12 +210,6 @@ def _get_boot_iso(task, root_uuid): LOG.debug("Found boot_iso %s in Glance", boot_iso_uuid) return boot_iso_uuid - if not kernel_href or not ramdisk_href: - LOG.error("Unable to find kernel or ramdisk for " - "image %(image)s to generate boot ISO for %(node)s", - {'image': image_href, 'node': task.node.uuid}) - return - # NOTE(rameshg87): Functionality to share the boot ISOs created for # similar instances (instances with same deployed image) is # not implemented as of now. Creation/Deletion of such a shared boot ISO @@ -186,43 +219,15 @@ def _get_boot_iso(task, root_uuid): # Option 3 - Create boot_iso from kernel/ramdisk, upload to Swift # or web server and provide its name. deploy_iso_uuid = deploy_info['ilo_deploy_iso'] - boot_mode = boot_mode_utils.get_boot_mode(task.node) - boot_iso_object_name = _get_boot_iso_object_name(task.node) - kernel_params = "" - if deploy_utils.get_boot_option(task.node) == "ramdisk": - i_info = task.node.instance_info - kernel_params = "root=/dev/ram0 text " - kernel_params += i_info.get("ramdisk_kernel_arguments", "") - else: - kernel_params = CONF.pxe.pxe_append_params - with tempfile.NamedTemporaryFile(dir=CONF.tempdir) as fileobj: - boot_iso_tmp_file = fileobj.name - images.create_boot_iso(task.context, boot_iso_tmp_file, - kernel_href, ramdisk_href, - deploy_iso_href=deploy_iso_uuid, - root_uuid=root_uuid, - kernel_params=kernel_params, - boot_mode=boot_mode) - - if CONF.ilo.use_web_server_for_images: - boot_iso_url = ( - ilo_common.copy_image_to_web_server(boot_iso_tmp_file, - boot_iso_object_name)) - driver_internal_info = task.node.driver_internal_info - driver_internal_info['boot_iso_created_in_web_server'] = True - task.node.driver_internal_info = driver_internal_info - task.node.save() - LOG.debug("Created boot_iso %(boot_iso)s for node %(node)s", - {'boot_iso': boot_iso_url, 'node': task.node.uuid}) - return boot_iso_url - else: - container = CONF.ilo.swift_ilo_container - swift_api = swift.SwiftAPI() - swift_api.create_object(container, boot_iso_object_name, - boot_iso_tmp_file) - LOG.debug("Created boot_iso %s in Swift", boot_iso_object_name) - return 'swift:%s' % boot_iso_object_name + use_web_server = CONF.ilo.use_web_server_for_images + container = CONF.ilo.swift_ilo_container + + return virtual_media_base.prepare_iso_image( + task, kernel_href, ramdisk_href, + deploy_iso_href=deploy_iso_uuid, + root_uuid=root_uuid, use_web_server=use_web_server, + container=container) def _clean_up_boot_iso_for_instance(node): @@ -230,25 +235,21 @@ def _clean_up_boot_iso_for_instance(node): :param node: an ironic node object. """ - ilo_boot_iso = node.instance_info.get('ilo_boot_iso') - if not ilo_boot_iso: - return - if ilo_boot_iso.startswith('swift'): + boot_iso_object_name = virtual_media_base.get_iso_image_name(node) + + if CONF.ilo.use_web_server_for_images: + boot_iso_path = os.path.join( + CONF.deploy.http_root, boot_iso_object_name) + ironic_utils.unlink_without_raise(boot_iso_path) + else: swift_api = swift.SwiftAPI() container = CONF.ilo.swift_ilo_container - boot_iso_object_name = _get_boot_iso_object_name(node) try: swift_api.delete_object(container, boot_iso_object_name) except exception.SwiftOperationError as e: LOG.exception("Failed to clean up boot ISO for node " "%(node)s. Error: %(error)s.", {'node': node.uuid, 'error': e}) - elif CONF.ilo.use_web_server_for_images: - result = urlparse.urlparse(ilo_boot_iso) - ilo_boot_iso_name = os.path.basename(result.path) - boot_iso_path = os.path.join( - CONF.deploy.http_root, ilo_boot_iso_name) - ironic_utils.unlink_without_raise(boot_iso_path) def _parse_deploy_info(node): @@ -284,19 +285,7 @@ def _validate_driver_info(task): """ node = task.node ilo_common.parse_driver_info(node) - if 'ilo_deploy_iso' not in node.driver_info: - raise exception.MissingParameterValue(_( - "Missing 'ilo_deploy_iso' parameter in node's 'driver_info'.")) - deploy_iso = node.driver_info['ilo_deploy_iso'] - if not service_utils.is_glance_image(deploy_iso): - try: - image_service.HttpImageService().validate_href(deploy_iso) - except exception.ImageRefValidationFailed: - raise exception.InvalidParameterValue(_( - "Virtual media boot accepts only Glance images or " - "HTTP(S) as driver_info['ilo_deploy_iso']. Either '%s' " - "is not a glance UUID or not a valid HTTP(S) URL or " - "the given URL is not reachable.") % deploy_iso) + parse_driver_info(node) def _validate_instance_image_info(task): @@ -518,11 +507,19 @@ class IloVirtualMediaBoot(base.BootInterface): deploy_nic_mac = deploy_utils.get_single_nic_with_vif_port_id(task) ramdisk_params['BOOTIF'] = deploy_nic_mac - if node.provision_state == states.RESCUING: + if (node.driver_info.get('ilo_rescue_iso') + and node.provision_state == states.RESCUING): iso = node.driver_info['ilo_rescue_iso'] - else: + elif node.driver_info.get('ilo_deploy_iso'): iso = node.driver_info['ilo_deploy_iso'] - + else: + mode = deploy_utils.rescue_or_deploy_mode(node) + d_info = parse_driver_info(node, mode) + use_web_server = CONF.ilo.use_web_server_for_images + container = CONF.ilo.swift_ilo_container + iso = virtual_media_base.prepare_deploy_iso( + task, ramdisk_params, mode, d_info, + use_web_server=use_web_server, container=container) ilo_common.setup_vmedia(task, iso, ramdisk_params) @METRICS.timer('IloVirtualMediaBoot.prepare_instance') @@ -636,6 +633,10 @@ class IloVirtualMediaBoot(base.BootInterface): :returns: None :raises: IloOperationError, if some operation on iLO failed. """ + info = task.node.driver_info + + if not info.get('ilo_deploy_iso') and not info.get('ilo_rescue_iso'): + _clean_up_boot_iso_for_instance(task.node) ilo_common.cleanup_vmedia_boot(task) def _configure_vmedia_boot(self, task, root_uuid): diff --git a/ironic/drivers/modules/ilo/common.py b/ironic/drivers/modules/ilo/common.py index 62c6bbb23..eb99c9568 100644 --- a/ironic/drivers/modules/ilo/common.py +++ b/ironic/drivers/modules/ilo/common.py @@ -17,7 +17,6 @@ Common functionalities shared between different iLO modules. """ import os -import shutil import tempfile from urllib import parse as urlparse @@ -134,17 +133,12 @@ def copy_image_to_web_server(source_file_path, destination): :returns: image url after the source image is uploaded. """ + LOG.warning( + 'This method is obsolete and will be removed in next release. ' + 'Please use deploy_utils.copy_image_to_web_server() instead.') - image_url = urlparse.urljoin(CONF.deploy.http_url, destination) - image_path = os.path.join(CONF.deploy.http_root, destination) - try: - shutil.copyfile(source_file_path, image_path) - except IOError as exc: - raise exception.ImageUploadFailed(image_name=destination, - web_server=CONF.deploy.http_url, - reason=exc) - os.chmod(image_path, 0o644) - return image_url + return deploy_utils.copy_image_to_web_server(source_file_path, + destination) def remove_image_from_web_server(object_name): @@ -420,8 +414,8 @@ def _prepare_floppy_image(task, params): images.create_vfat_image(vfat_image_tmpfile, parameters=params) object_name = _get_floppy_image_name(task.node) if CONF.ilo.use_web_server_for_images: - image_url = copy_image_to_web_server(vfat_image_tmpfile, - object_name) + image_url = deploy_utils.copy_image_to_web_server( + vfat_image_tmpfile, object_name) else: image_url = copy_image_to_swift(vfat_image_tmpfile, object_name) diff --git a/ironic/drivers/modules/ilo/firmware_processor.py b/ironic/drivers/modules/ilo/firmware_processor.py index 53282311e..ca704f5b8 100644 --- a/ironic/drivers/modules/ilo/firmware_processor.py +++ b/ironic/drivers/modules/ilo/firmware_processor.py @@ -32,6 +32,7 @@ from ironic.common import exception from ironic.common.i18n import _ from ironic.common import image_service from ironic.common import swift +from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.ilo import common as ilo_common # Supported components for firmware update when invoked through manual clean @@ -370,7 +371,7 @@ def _extract_fw_from_file(node, target_file): "firmware file %(firmware_image)s on web server ...", {'firmware_image': fw_image_location, 'node': node.uuid}) - fw_image_uploaded_url = ilo_common.copy_image_to_web_server( + fw_image_uploaded_url = deploy_utils.copy_image_to_web_server( fw_image_location, fw_image_filename) fw_image_location_obj.fw_image_location = fw_image_uploaded_url diff --git a/ironic/drivers/modules/inspector.py b/ironic/drivers/modules/inspector.py index 30d34d6a1..be0a5e72b 100644 --- a/ironic/drivers/modules/inspector.py +++ b/ironic/drivers/modules/inspector.py @@ -17,12 +17,12 @@ Modules required to work with ironic_inspector: import ipaddress import shlex +from urllib import parse as urlparse import eventlet from futurist import periodics import openstack from oslo_log import log as logging -from six.moves.urllib import parse as urlparse from ironic.common import exception from ironic.common.i18n import _ diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index da3db756e..5747d156a 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -504,7 +504,7 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None, while True: num_tries = num_tries - 1 - # NOTE(deva): ensure that no communications are sent to a BMC more + # NOTE(tenbrae): ensure that no communications are sent to a BMC more # often than once every min_command_interval seconds. time_till_next_poll = CONF.ipmi.min_command_interval - ( time.time() - LAST_CMD_TIME.get(driver_info['address'], 0)) @@ -870,7 +870,7 @@ class IPMIPower(base.PowerInterface): """ _parse_driver_info(task.node) - # NOTE(deva): don't actually touch the BMC in validate because it is + # NOTE(tenbrae): don't actually touch the BMC in validate because it is # called too often, and BMCs are too fragile. # This is a temporary measure to mitigate problems while # 1314954 and 1314961 are resolved. diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index f842a04fd..931211ee3 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -20,7 +20,6 @@ from urllib import parse as urlparse from ironic_lib import utils as ironic_utils from oslo_log import log -from oslo_serialization import base64 from oslo_utils import importutils from ironic.common import boot_devices @@ -36,6 +35,7 @@ from ironic.drivers import base from ironic.drivers.modules import boot_mode_utils from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.redfish import utils as redfish_utils +from ironic.drivers.modules import virtual_media_base LOG = log.getLogger(__name__) @@ -392,164 +392,17 @@ class RedfishVirtualMediaBoot(base.BootInterface): return image_url - @staticmethod - def _get_iso_image_name(node): - """Returns the boot iso image name for a given node. - - :param node: the node for which image name is to be provided. - """ - return "boot-%s" % node.uuid - @classmethod def _cleanup_iso_image(cls, task): """Deletes the ISO if it was created for the instance. :param task: an ironic node object. """ - iso_object_name = cls._get_iso_image_name(task.node) + iso_object_name = virtual_media_base.get_iso_image_name(task.node) cls._unpublish_image(iso_object_name) @classmethod - def _prepare_iso_image(cls, task, kernel_href, ramdisk_href, - bootloader_href=None, configdrive=None, - root_uuid=None, params=None): - """Prepare an ISO to boot the node. - - Build bootable ISO out of `kernel_href` and `ramdisk_href` (and - `bootloader` if it's UEFI boot), then push built image up to Swift and - return a temporary URL. - - :param task: a TaskManager instance containing the node to act on. - :param kernel_href: URL or Glance UUID of the kernel to use - :param ramdisk_href: URL or Glance UUID of the ramdisk to use - :param bootloader_href: URL or Glance UUID of the EFI bootloader - image to use when creating UEFI bootbable ISO - :param configdrive: URL to or a compressed blob of a ISO9660 or - FAT-formatted OpenStack config drive image. This image will be - written onto the built ISO image. Optional. - :param root_uuid: optional uuid of the root partition. - :param params: a dictionary containing 'parameter name'->'value' - mapping to be passed to kernel command line. - :returns: bootable ISO HTTP URL. - :raises: MissingParameterValue, if any of the required parameters are - missing. - :raises: InvalidParameterValue, if any of the parameters have invalid - value. - :raises: ImageCreationFailed, if creating ISO image failed. - """ - if not kernel_href or not ramdisk_href: - raise exception.InvalidParameterValue(_( - "Unable to find kernel or ramdisk for " - "building ISO for %(node)s") % - {'node': task.node.uuid}) - - i_info = task.node.instance_info - - if deploy_utils.get_boot_option(task.node) == "ramdisk": - kernel_params = "root=/dev/ram0 text " - kernel_params += i_info.get("ramdisk_kernel_arguments", "") - - else: - kernel_params = i_info.get( - 'kernel_append_params', CONF.redfish.kernel_append_params) - - if params: - kernel_params = ' '.join( - (kernel_params, ' '.join( - '%s=%s' % kv for kv in params.items()))) - - boot_mode = boot_mode_utils.get_boot_mode_for_deploy(task.node) - - LOG.debug("Trying to create %(boot_mode)s ISO image for node %(node)s " - "with kernel %(kernel_href)s, ramdisk %(ramdisk_href)s, " - "bootloader %(bootloader_href)s and kernel params %(params)s" - "", {'node': task.node.uuid, - 'boot_mode': boot_mode, - 'kernel_href': kernel_href, - 'ramdisk_href': ramdisk_href, - 'bootloader_href': bootloader_href, - 'params': kernel_params}) - - with tempfile.NamedTemporaryFile( - dir=CONF.tempdir, suffix='.iso') as boot_fileobj: - - with tempfile.NamedTemporaryFile( - dir=CONF.tempdir, suffix='.img') as cfgdrv_fileobj: - - configdrive_href = configdrive - - if configdrive: - parsed_url = urlparse.urlparse(configdrive) - if not parsed_url.scheme: - cfgdrv_blob = base64.decode_as_bytes(configdrive) - - with open(cfgdrv_fileobj.name, 'wb') as f: - f.write(cfgdrv_blob) - - configdrive_href = urlparse.urlunparse( - ('file', '', cfgdrv_fileobj.name, '', '', '')) - - LOG.info("Burning configdrive %(url)s to boot ISO image " - "for node %(node)s", {'url': configdrive_href, - 'node': task.node.uuid}) - - boot_iso_tmp_file = boot_fileobj.name - images.create_boot_iso( - task.context, boot_iso_tmp_file, - kernel_href, ramdisk_href, - esp_image_href=bootloader_href, - configdrive_href=configdrive_href, - root_uuid=root_uuid, - kernel_params=kernel_params, - boot_mode=boot_mode) - - iso_object_name = cls._get_iso_image_name(task.node) - - image_url = cls._publish_image( - boot_iso_tmp_file, iso_object_name) - - LOG.debug("Created ISO %(name)s in object store for node %(node)s, " - "exposed as temporary URL " - "%(url)s", {'node': task.node.uuid, - 'name': iso_object_name, - 'url': image_url}) - - return image_url - - @classmethod - def _prepare_deploy_iso(cls, task, params, mode): - """Prepare deploy or rescue ISO image - - Build bootable ISO out of - `[driver_info]/deploy_kernel`/`[driver_info]/deploy_ramdisk` or - `[driver_info]/rescue_kernel`/`[driver_info]/rescue_ramdisk` - and `[driver_info]/bootloader`, then push built image up to Glance - and return temporary Swift URL to the image. - - :param task: a TaskManager instance containing the node to act on. - :param params: a dictionary containing 'parameter name'->'value' - mapping to be passed to kernel command line. - :param mode: either 'deploy' or 'rescue'. - :returns: bootable ISO HTTP URL. - :raises: MissingParameterValue, if any of the required parameters are - missing. - :raises: InvalidParameterValue, if any of the parameters have invalid - value. - :raises: ImageCreationFailed, if creating ISO image failed. - """ - node = task.node - - d_info = cls._parse_driver_info(node) - - kernel_href = d_info.get('%s_kernel' % mode) - ramdisk_href = d_info.get('%s_ramdisk' % mode) - bootloader_href = d_info.get('bootloader') - - return cls._prepare_iso_image( - task, kernel_href, ramdisk_href, bootloader_href, params=params) - - @classmethod def _prepare_boot_iso(cls, task, root_uuid=None): """Prepare boot ISO image @@ -598,9 +451,19 @@ class RedfishVirtualMediaBoot(base.BootInterface): bootloader_href = d_info.get('bootloader') - return cls._prepare_iso_image( - task, kernel_href, ramdisk_href, bootloader_href, - root_uuid=root_uuid) + if CONF.redfish.use_swift: + use_web_server = False + container = CONF.redfish.swift_container + timeout = CONF.redfish.swift_object_expiry_timeout + else: + use_web_server = True + container = None + timeout = None + + return virtual_media_base.prepare_iso_image( + task, kernel_href, ramdisk_href, bootloader_href=bootloader_href, + root_uuid=root_uuid, timeout=timeout, + use_web_server=use_web_server, container=container) def get_properties(self): """Return the properties of the interface. @@ -753,7 +616,16 @@ class RedfishVirtualMediaBoot(base.BootInterface): mode = deploy_utils.rescue_or_deploy_mode(node) - iso_ref = self._prepare_deploy_iso(task, ramdisk_params, mode) + if CONF.redfish.use_swift: + use_web_server = False + container = CONF.redfish.swift_container + else: + use_web_server = True + container = None + + iso_ref = virtual_media_base.prepare_deploy_iso( + task, ramdisk_params, mode, d_info, + use_web_server=use_web_server, container=container) self._eject_vmedia(task, sushy.VIRTUAL_MEDIA_CD) self._insert_vmedia(task, iso_ref, sushy.VIRTUAL_MEDIA_CD) diff --git a/ironic/drivers/modules/redfish/management.py b/ironic/drivers/modules/redfish/management.py index 3db4b7de2..4026da48c 100644 --- a/ironic/drivers/modules/redfish/management.py +++ b/ironic/drivers/modules/redfish/management.py @@ -126,10 +126,17 @@ class RedfishManagement(base.ManagementInterface): """ system = redfish_utils.get_system(task.node) + desired_persistence = BOOT_DEVICE_PERSISTENT_MAP_REV[persistent] + current_persistence = system.boot.get('enabled') + + # NOTE(etingof): this can be racy, esp if BMC is not RESTful + enabled = (desired_persistence + if desired_persistence != current_persistence else None) + try: system.set_system_boot_options( - BOOT_DEVICE_MAP_REV[device], - enabled=BOOT_DEVICE_PERSISTENT_MAP_REV[persistent]) + BOOT_DEVICE_MAP_REV[device], enabled=enabled) + except sushy.exceptions.SushyError as e: error_msg = (_('Redfish set boot device failed for node ' '%(node)s. Error: %(error)s') % diff --git a/ironic/drivers/modules/virtual_media_base.py b/ironic/drivers/modules/virtual_media_base.py new file mode 100644 index 000000000..9bda27f61 --- /dev/null +++ b/ironic/drivers/modules/virtual_media_base.py @@ -0,0 +1,173 @@ +# 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. + +import tempfile + +from oslo_log import log + +from ironic.common import exception +from ironic.common.i18n import _ +from ironic.common import images +from ironic.common import swift +from ironic.conf import CONF +from ironic.drivers.modules import boot_mode_utils +from ironic.drivers.modules import deploy_utils + +LOG = log.getLogger(__name__) + + +def get_iso_image_name(node): + """Returns the boot iso image name for a given node. + + :param node: the node for which image name is to be provided. + """ + return "boot-%s" % node.uuid + + +def prepare_iso_image(task, kernel_href, ramdisk_href, deploy_iso_href=None, + bootloader_href=None, root_uuid=None, + kernel_params=None, timeout=None, + use_web_server=False, container=None): + """Prepare an ISO to boot the node. + + Build bootable ISO out of `kernel_href` and `ramdisk_href` (and + `bootloader` if it's UEFI boot), then push built image up to Swift and + return a temporary URL. + + :param task: a TaskManager instance containing the node to act on. + :param kernel_href: URL or Glance UUID of the kernel to use + :param ramdisk_href: URL or Glance UUID of the ramdisk to use + :param bootloader_href: URL or Glance UUID of the EFI bootloader + image to use when creating UEFI bootbable ISO + :param root_uuid: optional uuid of the root partition. + :param kernel_params: a dictionary containing 'parameter name'->'value' + mapping to be passed to kernel command line. + :param timeout: swift object expiry timeout + :returns: bootable ISO HTTP URL. + :raises: MissingParameterValue, if any of the required parameters are + missing. + :raises: InvalidParameterValue, if any of the parameters have invalid + value. + :raises: ImageCreationFailed, if creating ISO image failed. + """ + if not kernel_href or not ramdisk_href: + raise exception.InvalidParameterValue(_( + "Unable to find kernel or ramdisk for " + "building ISO for %(node)s") % + {'node': task.node.uuid}) + + boot_mode = boot_mode_utils.get_boot_mode_for_deploy(task.node) + + LOG.debug("Trying to create %(boot_mode)s ISO image for node %(node)s " + "with kernel %(kernel_href)s, ramdisk %(ramdisk_href)s, " + "bootloader %(bootloader_href)s and kernel params %(params)s" + "", {'node': task.node.uuid, + 'boot_mode': boot_mode, + 'kernel_href': kernel_href, + 'ramdisk_href': ramdisk_href, + 'bootloader_href': bootloader_href, + 'params': kernel_params}) + + with tempfile.NamedTemporaryFile( + dir=CONF.tempdir, suffix='.iso') as fileobj: + boot_iso_tmp_file = fileobj.name + images.create_boot_iso(task.context, boot_iso_tmp_file, + kernel_href, ramdisk_href, + deploy_iso_href=deploy_iso_href, + esp_image_href=bootloader_href, + root_uuid=root_uuid, + kernel_params=kernel_params, + boot_mode=boot_mode) + + iso_object_name = get_iso_image_name(task.node) + + if use_web_server: + boot_iso_url = ( + deploy_utils.copy_image_to_web_server(boot_iso_tmp_file, + iso_object_name)) + driver_internal_info = task.node.driver_internal_info + driver_internal_info['boot_iso_created_in_web_server'] = True + task.node.driver_internal_info = driver_internal_info + task.node.save() + LOG.debug("Created boot_iso %(boot_iso)s for node %(node)s", + {'boot_iso': boot_iso_url, 'node': task.node.uuid}) + return boot_iso_url + else: + swift_api = swift.SwiftAPI() + + object_headers = None + if task.node.driver == 'redfish': + object_headers = {'X-Delete-After': str(timeout)} + + swift_api.create_object(container, iso_object_name, + boot_iso_tmp_file, + object_headers=object_headers) + + LOG.debug("Created ISO %(name)s in Swift for node %(node)s", + {'node': task.node.uuid, 'name': iso_object_name}) + + if task.node.driver == 'redfish': + boot_iso_url = swift_api.get_temp_url( + container, iso_object_name, timeout) + return boot_iso_url + else: + return 'swift:%s' % iso_object_name + + +def prepare_deploy_iso(task, params, mode, driver_info, + use_web_server=False, container=None): + """Prepare deploy or rescue ISO image + + Build bootable ISO out of + `[driver_info]/deploy_kernel`/`[driver_info]/deploy_ramdisk` or + `[driver_info]/rescue_kernel`/`[driver_info]/rescue_ramdisk` + and `[driver_info]/bootloader`, then push built image up to Glance + and return temporary Swift URL to the image. + + :param task: a TaskManager instance containing the node to act on. + :param params: a dictionary containing 'parameter name'->'value' + mapping to be passed to kernel command line. + :param mode: either 'deploy' or 'rescue'. + :param driver_info: a dictionary containing driver_info values. + :returns: bootable ISO HTTP URL. + :raises: MissingParameterValue, if any of the required parameters are + missing. + :raises: InvalidParameterValue, if any of the parameters have invalid + value. + :raises: ImageCreationFailed, if creating ISO image failed. + """ + + kernel_href = driver_info.get('%s_kernel' % mode) + ramdisk_href = driver_info.get('%s_ramdisk' % mode) + bootloader_href = driver_info.get('bootloader') + timeout = None + + if deploy_utils.get_boot_option(task.node) == "ramdisk": + i_info = task.node.instance_info + kernel_params = "root=/dev/ram0 text " + kernel_params += i_info.get("ramdisk_kernel_arguments", "") + elif task.node.driver == 'redfish': + kernel_params = CONF.redfish.kernel_append_params + timeout = CONF.redfish.swift_object_expiry_timeout + else: + kernel_params = CONF.pxe.pxe_append_params + + if params: + kernel_params = ' '.join( + (kernel_params, ' '.join( + '%s=%s' % kv for kv in params.items()))) + + return prepare_iso_image(task, kernel_href, ramdisk_href, + bootloader_href=bootloader_href, + kernel_params=kernel_params, timeout=timeout, + use_web_server=use_web_server, + container=container) diff --git a/ironic/tests/unit/__init__.py b/ironic/tests/unit/__init__.py index 2abf56519..a83ef31e8 100644 --- a/ironic/tests/unit/__init__.py +++ b/ironic/tests/unit/__init__.py @@ -22,7 +22,7 @@ :platform: Unix """ -# TODO(deva): move eventlet imports to ironic.__init__ once we move to PBR +# TODO(tenbrae): move eventlet imports to ironic.__init__ once we move to PBR import eventlet from oslo_config import cfg diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 323de9e07..18637d3a7 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -5062,10 +5062,10 @@ class TestPut(test_api_base.BaseApiTest): self.assertEqual(urlparse.urlparse(ret.location).path, expected_location) - # NOTE(deva): this test asserts API functionality which is not part of + # NOTE(tenbrae): this test asserts API functionality which is not part of # the new-ironic-state-machine in Kilo. It is retained for backwards # compatibility with Juno. - # TODO(deva): add a deprecation-warning to the REST result + # TODO(tenbrae): add a deprecation-warning to the REST result # and check for it here. def test_provision_with_deploy_after_deployfail(self): node = self.node diff --git a/ironic/tests/unit/cmd/test_conductor.py b/ironic/tests/unit/cmd/test_conductor.py index 2da3db2a2..8de4ebb40 100644 --- a/ironic/tests/unit/cmd/test_conductor.py +++ b/ironic/tests/unit/cmd/test_conductor.py @@ -50,8 +50,3 @@ class ConductorStartTestCase(db_base.DbTestCase): 'deploy') conductor.warn_about_unsafe_shred_parameters(cfg.CONF) self.assertTrue(log_mock.warning.called) - - @mock.patch.object(conductor, 'LOG', autospec=True) - def test_warn_on_missing_default_boot_option(self, log_mock): - conductor.warn_about_missing_default_boot_option(cfg.CONF) - self.assertTrue(log_mock.warning.called) diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py index 449fafe9d..6950bfe8a 100644 --- a/ironic/tests/unit/common/test_pxe_utils.py +++ b/ironic/tests/unit/common/test_pxe_utils.py @@ -998,9 +998,17 @@ class PXEInterfacesTestCase(db_base.DbTestCase): def test_get_instance_image_info(self): # Tests when 'is_whole_disk_image' exists in driver_internal_info + # NOTE(TheJulia): The method being tested is primarily geared for + # only netboot operation as the information should only need to be + # looked up again during network booting. + self.config(group="deploy", default_boot_option="netboot") self._test_get_instance_image_info() def test_get_instance_image_info_without_is_whole_disk_image(self): + # NOTE(TheJulia): The method being tested is primarily geared for + # only netboot operation as the information should only need to be + # looked up again during network booting. + self.config(group="deploy", default_boot_option="netboot") # Tests when 'is_whole_disk_image' doesn't exists in # driver_internal_info del self.node.driver_internal_info['is_whole_disk_image'] diff --git a/ironic/tests/unit/conductor/test_base_manager.py b/ironic/tests/unit/conductor/test_base_manager.py index f8ba77c3f..49cacbff5 100644 --- a/ironic/tests/unit/conductor/test_base_manager.py +++ b/ironic/tests/unit/conductor/test_base_manager.py @@ -64,6 +64,17 @@ class StartStopTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): node.refresh() self.assertIsNone(node.reservation) + def test_stop_clears_conductor_locks(self): + node = obj_utils.create_test_node(self.context, + reservation=self.hostname) + node.save() + self._start_service() + res = objects.Conductor.get_by_hostname(self.context, self.hostname) + self.assertEqual(self.hostname, res['hostname']) + self.service.del_host() + node.refresh() + self.assertIsNone(node.reservation) + def test_stop_unregisters_conductor(self): self._start_service() res = objects.Conductor.get_by_hostname(self.context, self.hostname) diff --git a/ironic/tests/unit/conductor/test_deployments.py b/ironic/tests/unit/conductor/test_deployments.py index f41797c1e..13b35c26c 100644 --- a/ironic/tests/unit/conductor/test_deployments.py +++ b/ironic/tests/unit/conductor/test_deployments.py @@ -53,7 +53,7 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): self.service.conductor.id) node.refresh() self.assertEqual(states.DEPLOYFAIL, node.provision_state) - # NOTE(deva): failing a deploy does not clear the target state + # NOTE(tenbrae): failing a deploy does not clear the target state # any longer. Instead, it is cleared when the instance # is deleted. self.assertEqual(states.ACTIVE, node.target_provision_state) @@ -78,7 +78,7 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): self.service.conductor.id) node.refresh() self.assertEqual(states.DEPLOYFAIL, node.provision_state) - # NOTE(deva): failing a deploy does not clear the target state + # NOTE(tenbrae): failing a deploy does not clear the target state # any longer. Instead, it is cleared when the instance # is deleted. self.assertEqual(states.ACTIVE, node.target_provision_state) @@ -101,7 +101,7 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): deployments.do_node_deploy(task, self.service.conductor.id) node.refresh() self.assertEqual(states.DEPLOYFAIL, node.provision_state) - # NOTE(deva): failing a deploy does not clear the target state + # NOTE(tenbrae): failing a deploy does not clear the target state # any longer. Instead, it is cleared when the instance # is deleted. self.assertEqual(states.ACTIVE, node.target_provision_state) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 115dd9a6e..37580706b 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -2097,12 +2097,12 @@ class DoNodeTearDownTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): for state in valid_rescue_states: self._test_do_node_tear_down_from_state(state, True) - # NOTE(deva): partial tear-down was broken. A node left in a state of - # DELETING could not have tear_down called on it a second time - # Thus, I have removed the unit test, which faultily asserted - # only that a node could be left in a state of incomplete - # deletion -- not that such a node's deletion could later be - # completed. + # NOTE(tenbrae): partial tear-down was broken. A node left in a state of + # DELETING could not have tear_down called on it a second + # time Thus, I have removed the unit test, which faultily + # asserted only that a node could be left in a state of + # incomplete deletion -- not that such a node's deletion + # could later be completed. @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', autospec=True) diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py index e18f1b33a..a7b720f4b 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -163,9 +163,10 @@ def get_test_node(**kw): "local_gb": "10", "memory_mb": "4096", } - # NOTE(deva): API unit tests confirm that sensitive fields in instance_info - # and driver_info will get scrubbed from the API response - # but other fields (eg, 'foo') do not. + # NOTE(tenbrae): API unit tests confirm that sensitive fields in + # instance_info and driver_info will get scrubbed + # from the API response but other fields + # (eg, 'foo') do not. fake_instance_info = { "configdrive": "TG9yZW0gaXBzdW0gZG9sb3Igc2l0IGFtZXQ=", "image_url": "http://example.com/test_image_url", diff --git a/ironic/tests/unit/drivers/__init__.py b/ironic/tests/unit/drivers/__init__.py index 7a25e0c7f..43fd02823 100644 --- a/ironic/tests/unit/drivers/__init__.py +++ b/ironic/tests/unit/drivers/__init__.py @@ -13,9 +13,9 @@ # License for the specific language governing permissions and limitations # under the License. -# NOTE(deva): since __init__ is loaded before the files in the same directory, -# and some third-party driver tests may need to have their -# external libraries mocked, we load the file which does that -# mocking here -- in the __init__. +# NOTE(tenbrae): since __init__ is loaded before the files in the same +# directory, and some third-party driver tests may need to have +# their external libraries mocked, we load the file which does +# that mocking here -- in the __init__. from ironic.tests.unit.drivers import third_party_driver_mocks # noqa diff --git a/ironic/tests/unit/drivers/modules/drac/test_power.py b/ironic/tests/unit/drivers/modules/drac/test_power.py index 27cc75981..4c442ba6f 100644 --- a/ironic/tests/unit/drivers/modules/drac/test_power.py +++ b/ironic/tests/unit/drivers/modules/drac/test_power.py @@ -144,3 +144,58 @@ class DracPowerTestCase(test_utils.BaseDracTest): drac_power_state = drac_power.REVERSE_POWER_STATES[states.POWER_ON] mock_client.set_power_state.assert_called_once_with(drac_power_state) + + @mock.patch('time.sleep') + def test_reboot_retries_success(self, mock_sleep, mock_get_drac_client): + mock_client = mock_get_drac_client.return_value + mock_client.get_power_state.return_value = drac_constants.POWER_OFF + exc = drac_exceptions.DRACOperationFailed( + drac_messages=['The command failed to set RequestedState']) + mock_client.set_power_state.side_effect = [exc, None] + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.driver.power.reboot(task) + + drac_power_state = drac_power.REVERSE_POWER_STATES[states.POWER_ON] + self.assertEqual(2, mock_client.set_power_state.call_count) + mock_client.set_power_state.assert_has_calls( + [mock.call(drac_power_state), + mock.call(drac_power_state)]) + + @mock.patch('time.sleep') + def test_reboot_retries_fail(self, mock_sleep, mock_get_drac_client): + mock_client = mock_get_drac_client.return_value + mock_client.get_power_state.return_value = drac_constants.POWER_OFF + exc = drac_exceptions.DRACOperationFailed( + drac_messages=['The command failed to set RequestedState']) + mock_client.set_power_state.side_effect = exc + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.assertRaises(exception.DracOperationError, + task.driver.power.reboot, task) + + self.assertEqual(drac_power.POWER_STATE_TRIES, + mock_client.set_power_state.call_count) + + @mock.patch('time.sleep') + def test_reboot_retries_power_change_success(self, mock_sleep, + mock_get_drac_client): + mock_client = mock_get_drac_client.return_value + mock_client.get_power_state.side_effect = [drac_constants.POWER_OFF, + drac_constants.POWER_ON] + exc = drac_exceptions.DRACOperationFailed( + drac_messages=['The command failed to set RequestedState']) + mock_client.set_power_state.side_effect = [exc, None] + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.driver.power.reboot(task) + + self.assertEqual(2, mock_client.set_power_state.call_count) + drac_power_state1 = drac_power.REVERSE_POWER_STATES[states.POWER_ON] + drac_power_state2 = drac_power.REVERSE_POWER_STATES[states.REBOOT] + mock_client.set_power_state.assert_has_calls( + [mock.call(drac_power_state1), + mock.call(drac_power_state2)]) diff --git a/ironic/tests/unit/drivers/modules/ilo/test_boot.py b/ironic/tests/unit/drivers/modules/ilo/test_boot.py index 1e133a540..f0f2528b6 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_boot.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_boot.py @@ -15,9 +15,6 @@ """Test class for boot methods used by iLO modules.""" -import io -import tempfile - from ironic_lib import utils as ironic_utils import mock from oslo_config import cfg @@ -39,6 +36,7 @@ from ironic.drivers.modules.ilo import management as ilo_management from ironic.drivers.modules import ipxe from ironic.drivers.modules import pxe from ironic.drivers.modules.storage import noop as noop_storage +from ironic.drivers.modules import virtual_media_base from ironic.drivers import utils as driver_utils from ironic.tests.unit.drivers.modules.ilo import test_common @@ -50,13 +48,44 @@ class IloBootCommonMethodsTestCase(test_common.BaseIloTest): boot_interface = 'ilo-virtual-media' - def test_parse_driver_info(self): + def test_parse_driver_info_deploy_iso(self): self.node.driver_info['ilo_deploy_iso'] = 'deploy-iso' - expected_driver_info = {'ilo_deploy_iso': 'deploy-iso'} + expected_driver_info = {'bootloader': None, + 'ilo_deploy_iso': 'deploy-iso'} actual_driver_info = ilo_boot.parse_driver_info(self.node) self.assertEqual(expected_driver_info, actual_driver_info) + def test_parse_driver_info_rescue_iso(self): + self.node.driver_info['ilo_rescue_iso'] = 'rescue-iso' + expected_driver_info = {'bootloader': None, + 'ilo_rescue_iso': 'rescue-iso'} + + actual_driver_info = ilo_boot.parse_driver_info(self.node, 'rescue') + self.assertEqual(expected_driver_info, actual_driver_info) + + def test_parse_driver_info_deploy(self): + self.node.driver_info['deploy_kernel'] = 'kernel' + self.node.driver_info['deploy_ramdisk'] = 'ramdisk' + self.node.driver_info['bootloader'] = 'bootloader' + expected_driver_info = {'deploy_kernel': 'kernel', + 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader'} + + actual_driver_info = ilo_boot.parse_driver_info(self.node) + self.assertEqual(expected_driver_info, actual_driver_info) + + def test_parse_driver_info_rescue(self): + self.node.driver_info['rescue_kernel'] = 'kernel' + self.node.driver_info['rescue_ramdisk'] = 'ramdisk' + self.node.driver_info['bootloader'] = 'bootloader' + expected_driver_info = {'rescue_kernel': 'kernel', + 'rescue_ramdisk': 'ramdisk', + 'bootloader': 'bootloader'} + + actual_driver_info = ilo_boot.parse_driver_info(self.node, 'rescue') + self.assertEqual(expected_driver_info, actual_driver_info) + def test_parse_driver_info_exc(self): self.assertRaises(exception.MissingParameterValue, ilo_boot.parse_driver_info, self.node) @@ -66,11 +95,6 @@ class IloBootPrivateMethodsTestCase(test_common.BaseIloTest): boot_interface = 'ilo-virtual-media' - def test__get_boot_iso_object_name(self): - boot_iso_actual = ilo_boot._get_boot_iso_object_name(self.node) - boot_iso_expected = "boot-%s" % self.node.uuid - self.assertEqual(boot_iso_expected, boot_iso_actual) - @mock.patch.object(image_service.HttpImageService, 'validate_href', spec_set=True, autospec=True) def test__get_boot_iso_http_url(self, service_mock): @@ -128,238 +152,52 @@ class IloBootPrivateMethodsTestCase(test_common.BaseIloTest): boot_iso_expected = u'glance://uui\u0111' self.assertEqual(boot_iso_expected, boot_iso_actual) - @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', - spec_set=True, autospec=True) - @mock.patch.object(ilo_boot.LOG, 'error', spec_set=True, autospec=True) - @mock.patch.object(images, 'get_image_properties', spec_set=True, - autospec=True) - @mock.patch.object(ilo_boot, '_parse_deploy_info', spec_set=True, - autospec=True) - def test__get_boot_iso_uefi_no_glance_image(self, - deploy_info_mock, - image_props_mock, - log_mock, - boot_mode_mock): - deploy_info_mock.return_value = {'image_source': 'image-uuid', - 'ilo_deploy_iso': 'deploy_iso_uuid'} - image_props_mock.return_value = {'boot_iso': None, - 'kernel_id': None, - 'ramdisk_id': None} - properties = {'capabilities': 'boot_mode:uefi'} - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - task.node.properties = properties - boot_iso_result = ilo_boot._get_boot_iso(task, 'root-uuid') - deploy_info_mock.assert_called_once_with(task.node) - image_props_mock.assert_called_once_with( - task.context, 'image-uuid', - ['boot_iso', 'kernel_id', 'ramdisk_id']) - self.assertTrue(log_mock.called) - self.assertFalse(boot_mode_mock.called) - self.assertIsNone(boot_iso_result) - - @mock.patch.object(tempfile, 'NamedTemporaryFile', spec_set=True, - autospec=True) - @mock.patch.object(images, 'create_boot_iso', spec_set=True, autospec=True) - @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) - @mock.patch.object(ilo_boot, '_get_boot_iso_object_name', spec_set=True, - autospec=True) - @mock.patch.object(driver_utils, 'get_node_capability', spec_set=True, + @mock.patch.object(virtual_media_base, 'prepare_iso_image', spec_set=True, autospec=True) @mock.patch.object(images, 'get_image_properties', spec_set=True, autospec=True) @mock.patch.object(ilo_boot, '_parse_deploy_info', spec_set=True, autospec=True) - def test__get_boot_iso_create(self, deploy_info_mock, image_props_mock, - capability_mock, boot_object_name_mock, - swift_api_mock, - create_boot_iso_mock, tempfile_mock): - CONF.ilo.swift_ilo_container = 'ilo-cont' - CONF.pxe.pxe_append_params = 'kernel-params' - - swift_obj_mock = swift_api_mock.return_value - fileobj_mock = mock.MagicMock(spec=io.BytesIO) - fileobj_mock.name = 'tmpfile' - mock_file_handle = mock.MagicMock(spec=io.BytesIO) - mock_file_handle.__enter__.return_value = fileobj_mock - tempfile_mock.return_value = mock_file_handle - + def test__get_boot_iso_create(self, deploy_info_mock, + image_props_mock, prepare_iso_mock): deploy_info_mock.return_value = {'image_source': 'image-uuid', 'ilo_deploy_iso': 'deploy_iso_uuid'} image_props_mock.return_value = {'boot_iso': None, 'kernel_id': 'kernel_uuid', 'ramdisk_id': 'ramdisk_uuid'} - boot_object_name_mock.return_value = 'abcdef' - create_boot_iso_mock.return_value = '/path/to/boot-iso' - capability_mock.return_value = 'uefi' - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - boot_iso_actual = ilo_boot._get_boot_iso(task, 'root-uuid') - deploy_info_mock.assert_called_once_with(task.node) - image_props_mock.assert_called_once_with( - task.context, 'image-uuid', - ['boot_iso', 'kernel_id', 'ramdisk_id']) - boot_object_name_mock.assert_called_once_with(task.node) - create_boot_iso_mock.assert_called_once_with( - task.context, 'tmpfile', 'kernel_uuid', 'ramdisk_uuid', - deploy_iso_href='deploy_iso_uuid', - root_uuid='root-uuid', - kernel_params='kernel-params', - boot_mode='uefi') - swift_obj_mock.create_object.assert_called_once_with('ilo-cont', - 'abcdef', - 'tmpfile') - boot_iso_expected = 'swift:abcdef' - self.assertEqual(boot_iso_expected, boot_iso_actual) - - @mock.patch.object(ilo_common, 'copy_image_to_web_server', spec_set=True, - autospec=True) - @mock.patch.object(tempfile, 'NamedTemporaryFile', spec_set=True, - autospec=True) - @mock.patch.object(images, 'create_boot_iso', spec_set=True, autospec=True) - @mock.patch.object(ilo_boot, '_get_boot_iso_object_name', spec_set=True, - autospec=True) - @mock.patch.object(driver_utils, 'get_node_capability', spec_set=True, - autospec=True) - @mock.patch.object(images, 'get_image_properties', spec_set=True, - autospec=True) - @mock.patch.object(ilo_boot, '_parse_deploy_info', spec_set=True, - autospec=True) - def test__get_boot_iso_recreate_boot_iso_use_webserver( - self, deploy_info_mock, image_props_mock, - capability_mock, boot_object_name_mock, - create_boot_iso_mock, tempfile_mock, - copy_file_mock): - CONF.ilo.swift_ilo_container = 'ilo-cont' - CONF.ilo.use_web_server_for_images = True - CONF.deploy.http_url = "http://10.10.1.30/httpboot" - CONF.deploy.http_root = "/httpboot" - CONF.pxe.pxe_append_params = 'kernel-params' - - fileobj_mock = mock.MagicMock(spec=io.BytesIO) - fileobj_mock.name = 'tmpfile' - mock_file_handle = mock.MagicMock(spec=io.BytesIO) - mock_file_handle.__enter__.return_value = fileobj_mock - tempfile_mock.return_value = mock_file_handle - - ramdisk_href = "http://10.10.1.30/httpboot/ramdisk" - kernel_href = "http://10.10.1.30/httpboot/kernel" - deploy_info_mock.return_value = {'image_source': 'image-uuid', - 'ilo_deploy_iso': 'deploy_iso_uuid'} - image_props_mock.return_value = {'boot_iso': None, - 'kernel_id': kernel_href, - 'ramdisk_id': ramdisk_href} - boot_object_name_mock.return_value = 'new_boot_iso' - create_boot_iso_mock.return_value = '/path/to/boot-iso' - capability_mock.return_value = 'uefi' - copy_file_mock.return_value = "http://10.10.1.30/httpboot/new_boot_iso" + prepare_iso_mock.return_value = 'swift:boot-iso' with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - driver_internal_info = task.node.driver_internal_info - driver_internal_info['boot_iso_created_in_web_server'] = True - instance_info = task.node.instance_info - old_boot_iso = 'http://10.10.1.30/httpboot/old_boot_iso' - instance_info['ilo_boot_iso'] = old_boot_iso boot_iso_actual = ilo_boot._get_boot_iso(task, 'root-uuid') deploy_info_mock.assert_called_once_with(task.node) image_props_mock.assert_called_once_with( task.context, 'image-uuid', ['boot_iso', 'kernel_id', 'ramdisk_id']) - boot_object_name_mock.assert_called_once_with(task.node) - create_boot_iso_mock.assert_called_once_with( - task.context, 'tmpfile', kernel_href, ramdisk_href, + prepare_iso_mock.assert_called_once_with( + task, 'kernel_uuid', 'ramdisk_uuid', deploy_iso_href='deploy_iso_uuid', - root_uuid='root-uuid', - kernel_params='kernel-params', - boot_mode='uefi') - boot_iso_expected = 'http://10.10.1.30/httpboot/new_boot_iso' + root_uuid='root-uuid', use_web_server=False, + container='ironic_ilo_container') + boot_iso_expected = 'swift:boot-iso' self.assertEqual(boot_iso_expected, boot_iso_actual) - copy_file_mock.assert_called_once_with(fileobj_mock.name, - 'new_boot_iso') - @mock.patch.object(ilo_common, 'copy_image_to_web_server', spec_set=True, - autospec=True) - @mock.patch.object(tempfile, 'NamedTemporaryFile', spec_set=True, - autospec=True) - @mock.patch.object(images, 'create_boot_iso', spec_set=True, autospec=True) - @mock.patch.object(ilo_boot, '_get_boot_iso_object_name', spec_set=True, - autospec=True) - @mock.patch.object(driver_utils, 'get_node_capability', spec_set=True, - autospec=True) - @mock.patch.object(images, 'get_image_properties', spec_set=True, - autospec=True) - @mock.patch.object(ilo_boot, '_parse_deploy_info', spec_set=True, - autospec=True) - def test__get_boot_iso_create_use_webserver_true_ramdisk_webserver( - self, deploy_info_mock, image_props_mock, - capability_mock, boot_object_name_mock, - create_boot_iso_mock, tempfile_mock, - copy_file_mock): - CONF.ilo.swift_ilo_container = 'ilo-cont' - CONF.ilo.use_web_server_for_images = True - CONF.deploy.http_url = "http://10.10.1.30/httpboot" - CONF.deploy.http_root = "/httpboot" - CONF.pxe.pxe_append_params = 'kernel-params' - - fileobj_mock = mock.MagicMock(spec=io.BytesIO) - fileobj_mock.name = 'tmpfile' - mock_file_handle = mock.MagicMock(spec=io.BytesIO) - mock_file_handle.__enter__.return_value = fileobj_mock - tempfile_mock.return_value = mock_file_handle - - ramdisk_href = "http://10.10.1.30/httpboot/ramdisk" - kernel_href = "http://10.10.1.30/httpboot/kernel" - deploy_info_mock.return_value = {'image_source': 'image-uuid', - 'ilo_deploy_iso': 'deploy_iso_uuid'} - image_props_mock.return_value = {'boot_iso': None, - 'kernel_id': kernel_href, - 'ramdisk_id': ramdisk_href} - boot_object_name_mock.return_value = 'abcdef' - create_boot_iso_mock.return_value = '/path/to/boot-iso' - capability_mock.return_value = 'uefi' - copy_file_mock.return_value = "http://10.10.1.30/httpboot/abcdef" - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - boot_iso_actual = ilo_boot._get_boot_iso(task, 'root-uuid') - deploy_info_mock.assert_called_once_with(task.node) - image_props_mock.assert_called_once_with( - task.context, 'image-uuid', - ['boot_iso', 'kernel_id', 'ramdisk_id']) - boot_object_name_mock.assert_called_once_with(task.node) - create_boot_iso_mock.assert_called_once_with( - task.context, 'tmpfile', kernel_href, ramdisk_href, - deploy_iso_href='deploy_iso_uuid', - root_uuid='root-uuid', - kernel_params='kernel-params', - boot_mode='uefi') - boot_iso_expected = 'http://10.10.1.30/httpboot/abcdef' - self.assertEqual(boot_iso_expected, boot_iso_actual) - copy_file_mock.assert_called_once_with(fileobj_mock.name, - 'abcdef') - - @mock.patch.object(ilo_boot, '_get_boot_iso_object_name', spec_set=True, - autospec=True) + @mock.patch.object(virtual_media_base, 'get_iso_image_name', + spec_set=True, autospec=True) @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) def test__clean_up_boot_iso_for_instance(self, swift_mock, boot_object_name_mock): swift_obj_mock = swift_mock.return_value CONF.ilo.swift_ilo_container = 'ilo-cont' boot_object_name_mock.return_value = 'boot-object' - i_info = self.node.instance_info - i_info['ilo_boot_iso'] = 'swift:bootiso' - self.node.instance_info = i_info - self.node.save() ilo_boot._clean_up_boot_iso_for_instance(self.node) swift_obj_mock.delete_object.assert_called_once_with('ilo-cont', 'boot-object') @mock.patch.object(ilo_boot.LOG, 'exception', spec_set=True, autospec=True) - @mock.patch.object(ilo_boot, '_get_boot_iso_object_name', spec_set=True, - autospec=True) + @mock.patch.object(virtual_media_base, 'get_iso_image_name', + spec_set=True, autospec=True) @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) def test__clean_up_boot_iso_for_instance_exc(self, swift_mock, boot_object_name_mock, @@ -369,10 +207,6 @@ class IloBootPrivateMethodsTestCase(test_common.BaseIloTest): swift_obj_mock.delete_object.side_effect = exc CONF.ilo.swift_ilo_container = 'ilo-cont' boot_object_name_mock.return_value = 'boot-object' - i_info = self.node.instance_info - i_info['ilo_boot_iso'] = 'swift:bootiso' - self.node.instance_info = i_info - self.node.save() ilo_boot._clean_up_boot_iso_for_instance(self.node) swift_obj_mock.delete_object.assert_called_once_with('ilo-cont', 'boot-object') @@ -380,25 +214,17 @@ class IloBootPrivateMethodsTestCase(test_common.BaseIloTest): @mock.patch.object(ironic_utils, 'unlink_without_raise', spec_set=True, autospec=True) - def test__clean_up_boot_iso_for_instance_on_webserver(self, unlink_mock): - + @mock.patch.object(virtual_media_base, 'get_iso_image_name', + spec_set=True, autospec=True) + def test__clean_up_boot_iso_for_instance_on_webserver( + self, boot_object_name_mock, unlink_mock): + boot_object_name_mock.return_value = 'boot-object' CONF.ilo.use_web_server_for_images = True CONF.deploy.http_root = "/webserver" - i_info = self.node.instance_info - i_info['ilo_boot_iso'] = 'http://x.y.z.a/webserver/boot-object' - self.node.instance_info = i_info - self.node.save() boot_iso_path = "/webserver/boot-object" ilo_boot._clean_up_boot_iso_for_instance(self.node) unlink_mock.assert_called_once_with(boot_iso_path) - @mock.patch.object(ilo_boot, '_get_boot_iso_object_name', spec_set=True, - autospec=True) - def test__clean_up_boot_iso_for_instance_no_boot_iso( - self, boot_object_name_mock): - ilo_boot._clean_up_boot_iso_for_instance(self.node) - self.assertFalse(boot_object_name_mock.called) - @mock.patch.object(ilo_boot, 'parse_driver_info', spec_set=True, autospec=True) @mock.patch.object(deploy_utils, 'get_image_instance_info', @@ -412,68 +238,15 @@ class IloBootPrivateMethodsTestCase(test_common.BaseIloTest): @mock.patch.object(ilo_common, 'parse_driver_info', spec_set=True, autospec=True) - def test__validate_driver_info_MissingParam(self, mock_parse_driver_info): - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - self.assertRaisesRegex(exception.MissingParameterValue, - "Missing 'ilo_deploy_iso'", - ilo_boot._validate_driver_info, task) - mock_parse_driver_info.assert_called_once_with(task.node) - - @mock.patch.object(service_utils, 'is_glance_image', spec_set=True, - autospec=True) - @mock.patch.object(ilo_common, 'parse_driver_info', spec_set=True, - autospec=True) - def test__validate_driver_info_valid_uuid(self, mock_parse_driver_info, - mock_is_glance_image): - mock_is_glance_image.return_value = True - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - deploy_iso = '8a81759a-f29b-454b-8ab3-161c6ca1882c' - task.node.driver_info['ilo_deploy_iso'] = deploy_iso - ilo_boot._validate_driver_info(task) - mock_parse_driver_info.assert_called_once_with(task.node) - mock_is_glance_image.assert_called_once_with(deploy_iso) - - @mock.patch.object(image_service.HttpImageService, 'validate_href', - spec_set=True, autospec=True) - @mock.patch.object(service_utils, 'is_glance_image', spec_set=True, - autospec=True) - @mock.patch.object(ilo_common, 'parse_driver_info', spec_set=True, - autospec=True) - def test__validate_driver_info_InvalidParam(self, mock_parse_driver_info, - mock_is_glance_image, - mock_validate_href): - deploy_iso = 'http://abc.org/image/qcow2' - mock_validate_href.side_effect = exception.ImageRefValidationFailed( - image_href='http://abc.org/image/qcow2', reason='fail') - mock_is_glance_image.return_value = False - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - task.node.driver_info['ilo_deploy_iso'] = deploy_iso - self.assertRaisesRegex(exception.InvalidParameterValue, - "Virtual media boot accepts", - ilo_boot._validate_driver_info, task) - mock_parse_driver_info.assert_called_once_with(task.node) - mock_validate_href.assert_called_once_with(mock.ANY, deploy_iso) - - @mock.patch.object(image_service.HttpImageService, 'validate_href', - spec_set=True, autospec=True) - @mock.patch.object(service_utils, 'is_glance_image', spec_set=True, - autospec=True) - @mock.patch.object(ilo_common, 'parse_driver_info', spec_set=True, + @mock.patch.object(ilo_boot, 'parse_driver_info', spec_set=True, autospec=True) - def test__validate_driver_info_valid_url(self, mock_parse_driver_info, - mock_is_glance_image, - mock_validate_href): - deploy_iso = 'http://abc.org/image/deploy.iso' - mock_is_glance_image.return_value = False + def test__validate_driver_info(self, mock_driver_info, + mock_parse_driver_info): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: - task.node.driver_info['ilo_deploy_iso'] = deploy_iso ilo_boot._validate_driver_info(task) mock_parse_driver_info.assert_called_once_with(task.node) - mock_validate_href.assert_called_once_with(mock.ANY, deploy_iso) + mock_driver_info.assert_called_once_with(task.node) @mock.patch.object(deploy_utils, 'validate_image_properties', spec_set=True, autospec=True) @@ -1044,10 +817,13 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): @mock.patch.object(ilo_common, 'cleanup_vmedia_boot', spec_set=True, autospec=True) - def test_clean_up_ramdisk(self, cleanup_vmedia_mock): + @mock.patch.object(ilo_boot, '_clean_up_boot_iso_for_instance', + spec_set=True, autospec=True) + def test_clean_up_ramdisk(self, cleanup_iso_mock, cleanup_vmedia_mock): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.driver.boot.clean_up_ramdisk(task) + cleanup_iso_mock.assert_called_once_with(task.node) cleanup_vmedia_mock.assert_called_once_with(task) @mock.patch.object(deploy_utils, 'is_iscsi_boot', @@ -1105,6 +881,8 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): is_iscsi_boot_mock): self.node.driver_internal_info = {'root_uuid_or_disk_id': ( "12312642-09d3-467f-8e09-12385826a123")} + self.node.instance_info = { + 'capabilities': {'boot_option': 'netboot'}} self.node.save() is_iscsi_boot_mock.return_value = False with task_manager.acquire(self.context, self.node.uuid, @@ -1226,7 +1004,7 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): def test_validate_rescue_no_rescue_ramdisk(self): with task_manager.acquire(self.context, self.node.uuid) as task: self.assertRaisesRegex(exception.MissingParameterValue, - 'Missing.*ilo_rescue_iso', + 'Either.*ilo_rescue_iso*', task.driver.boot.validate_rescue, task) diff --git a/ironic/tests/unit/drivers/modules/ilo/test_common.py b/ironic/tests/unit/drivers/modules/ilo/test_common.py index 466f35a0c..02ba34715 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_common.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_common.py @@ -19,7 +19,6 @@ import builtins import hashlib import io import os -import shutil import tempfile from ironic_lib import utils as ironic_utils @@ -341,7 +340,7 @@ class IloCommonMethodsTestCase(BaseIloTest): 'ilo_cont', object_name, timeout) self.assertEqual('temp-url', temp_url) - @mock.patch.object(ilo_common, 'copy_image_to_web_server', + @mock.patch.object(deploy_utils, 'copy_image_to_web_server', spec_set=True, autospec=True) @mock.patch.object(images, 'create_vfat_image', spec_set=True, autospec=True) @@ -808,42 +807,6 @@ class IloCommonMethodsTestCase(BaseIloTest): task, False) ilo_mock_object.set_secure_boot_mode.assert_called_once_with(False) - @mock.patch.object(os, 'chmod', spec_set=True, - autospec=True) - @mock.patch.object(shutil, 'copyfile', spec_set=True, - autospec=True) - def test_copy_image_to_web_server(self, copy_mock, - chmod_mock): - CONF.deploy.http_url = "http://x.y.z.a/webserver/" - CONF.deploy.http_root = "/webserver" - expected_url = "http://x.y.z.a/webserver/image-UUID" - source = 'tmp_image_file' - destination = "image-UUID" - image_path = "/webserver/image-UUID" - actual_url = ilo_common.copy_image_to_web_server(source, destination) - self.assertEqual(expected_url, actual_url) - copy_mock.assert_called_once_with(source, image_path) - chmod_mock.assert_called_once_with(image_path, 0o644) - - @mock.patch.object(os, 'chmod', spec_set=True, - autospec=True) - @mock.patch.object(shutil, 'copyfile', spec_set=True, - autospec=True) - def test_copy_image_to_web_server_fails(self, copy_mock, - chmod_mock): - CONF.deploy.http_url = "http://x.y.z.a/webserver/" - CONF.deploy.http_root = "/webserver" - source = 'tmp_image_file' - destination = "image-UUID" - image_path = "/webserver/image-UUID" - exc = exception.ImageUploadFailed('reason') - copy_mock.side_effect = exc - self.assertRaises(exception.ImageUploadFailed, - ilo_common.copy_image_to_web_server, - source, destination) - copy_mock.assert_called_once_with(source, image_path) - self.assertFalse(chmod_mock.called) - @mock.patch.object(ilo_common, 'ironic_utils', autospec=True) def test_remove_image_from_web_server(self, utils_mock): # | GIVEN | diff --git a/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py b/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py index 89aa96f9f..680cacefd 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py @@ -22,6 +22,7 @@ import mock from oslo_utils import importutils from ironic.common import exception +from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.ilo import common as ilo_common from ironic.drivers.modules.ilo import firmware_processor as ilo_fw_processor from ironic.tests import base @@ -481,10 +482,12 @@ class FirmwareProcessorTestCase(base.TestCase): utils_mock.process_firmware_image.assert_called_once_with( any_target_file, ilo_object_mock) + @mock.patch.object(deploy_utils, 'copy_image_to_web_server', + autospec=True) @mock.patch.object(ilo_fw_processor, 'ilo_common', autospec=True) @mock.patch.object(ilo_fw_processor, 'proliantutils_utils', autospec=True) def test__extract_fw_from_file_doesnt_upload_firmware( - self, utils_mock, ilo_common_mock): + self, utils_mock, ilo_common_mock, copy_mock): # | GIVEN | node_mock = mock.MagicMock(uuid='fake_node_uuid') any_target_file = 'any_target_file' @@ -493,7 +496,7 @@ class FirmwareProcessorTestCase(base.TestCase): # | WHEN | ilo_fw_processor._extract_fw_from_file(node_mock, any_target_file) # | THEN | - ilo_common_mock.copy_image_to_web_server.assert_not_called() + copy_mock.assert_not_called() @mock.patch.object(ilo_fw_processor, 'ilo_common', autospec=True) @mock.patch.object(ilo_fw_processor, 'proliantutils_utils', autospec=True) @@ -513,10 +516,12 @@ class FirmwareProcessorTestCase(base.TestCase): # | THEN | _remove_mock.assert_called_once_with(location_obj) + @mock.patch.object(deploy_utils, 'copy_image_to_web_server', + autospec=True) @mock.patch.object(ilo_fw_processor, 'ilo_common', autospec=True) @mock.patch.object(ilo_fw_processor, 'proliantutils_utils', autospec=True) def test__extract_fw_from_file_uploads_firmware_to_webserver( - self, utils_mock, ilo_common_mock): + self, utils_mock, ilo_common_mock, copy_mock): # | GIVEN | node_mock = mock.MagicMock(uuid='fake_node_uuid') any_target_file = 'any_target_file' @@ -526,15 +531,17 @@ class FirmwareProcessorTestCase(base.TestCase): # | WHEN | ilo_fw_processor._extract_fw_from_file(node_mock, any_target_file) # | THEN | - ilo_common_mock.copy_image_to_web_server.assert_called_once_with( + copy_mock.assert_called_once_with( 'some_location/some_fw_file', 'some_fw_file') + @mock.patch.object(deploy_utils, 'copy_image_to_web_server', + autospec=True) @mock.patch.object(ilo_fw_processor, 'ilo_common', autospec=True) @mock.patch.object(ilo_fw_processor, 'proliantutils_utils', autospec=True) @mock.patch.object(ilo_fw_processor, '_remove_webserver_based_me', autospec=True) def test__extract_fw_from_file_sets_loc_obj_remove_to_webserver( - self, _remove_mock, utils_mock, ilo_common_mock): + self, _remove_mock, utils_mock, ilo_common_mock, copy_mock): # | GIVEN | node_mock = mock.MagicMock(uuid='fake_node_uuid') any_target_file = 'any_target_file' diff --git a/ironic/tests/unit/drivers/modules/irmc/test_boot.py b/ironic/tests/unit/drivers/modules/irmc/test_boot.py index 6831759a7..fd83b8cc8 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_boot.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_boot.py @@ -1074,6 +1074,8 @@ class IRMCVirtualMediaBootTestCase(test_common.BaseIRMCTest): autospec=True) def test_prepare_instance_partition_image( self, _cleanup_vmedia_boot_mock, _configure_vmedia_mock): + self.node.instance_info = { + 'capabilities': {'boot_option': 'netboot'}} self.node.driver_internal_info = {'root_uuid_or_disk_id': "some_uuid"} self.node.save() with task_manager.acquire(self.context, self.node.uuid, @@ -1149,7 +1151,7 @@ class IRMCVirtualMediaBootTestCase(test_common.BaseIRMCTest): self.node.target_provision_state = states.ACTIVE self.node.instance_info = { 'capabilities': { - "secure_boot": "true" + "secure_boot": "true", 'boot_option': 'netboot' } } self.node.save() @@ -1177,7 +1179,7 @@ class IRMCVirtualMediaBootTestCase(test_common.BaseIRMCTest): self.node.target_provision_state = states.ACTIVE self.node.instance_info = { 'capabilities': { - "secure_boot": "false" + "secure_boot": "false", 'boot_option': 'netboot' } } self.node.save() @@ -1202,6 +1204,11 @@ class IRMCVirtualMediaBootTestCase(test_common.BaseIRMCTest): self.node.driver_internal_info = {'root_uuid_or_disk_id': "12312642"} self.node.provision_state = states.DEPLOYING self.node.target_provision_state = states.ACTIVE + self.node.instance_info = { + 'capabilities': { + 'boot_option': 'netboot' + } + } self.node.save() with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py index c1d86486a..ec8ac746c 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -27,6 +27,7 @@ from ironic.drivers.modules import boot_mode_utils from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.redfish import boot as redfish_boot from ironic.drivers.modules.redfish import utils as redfish_utils +from ironic.drivers.modules import virtual_media_base from ironic.tests.unit.db import base as db_base from ironic.tests.unit.db import utils as db_utils from ironic.tests.unit.objects import utils as obj_utils @@ -339,113 +340,10 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock_unpublish.assert_called_once_with(object_name) - @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, - '_publish_image', autospec=True) - @mock.patch.object(images, 'create_boot_iso', autospec=True) - def test__prepare_iso_image_uefi( - self, mock_create_boot_iso, mock__publish_image): - with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: - task.node.instance_info.update(deploy_boot_mode='uefi') - - expected_url = 'https://a.b/c.f?e=f' - - mock__publish_image.return_value = expected_url - - url = task.driver.boot._prepare_iso_image( - task, 'http://kernel/img', 'http://ramdisk/img', - 'http://bootloader/img', root_uuid=task.node.uuid) - - object_name = 'boot-%s' % task.node.uuid - - mock__publish_image.assert_called_once_with( - mock.ANY, object_name) - - mock_create_boot_iso.assert_called_once_with( - mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img', - boot_mode='uefi', esp_image_href='http://bootloader/img', - configdrive_href=mock.ANY, - kernel_params='nofb nomodeset vga=normal', - root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123') - - self.assertEqual(expected_url, url) - - @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, - '_publish_image', autospec=True) - @mock.patch.object(images, 'create_boot_iso', autospec=True) - def test__prepare_iso_image_bios( - self, mock_create_boot_iso, mock__publish_image): - with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: - - expected_url = 'https://a.b/c.f?e=f' - - mock__publish_image.return_value = expected_url - - url = task.driver.boot._prepare_iso_image( - task, 'http://kernel/img', 'http://ramdisk/img', - bootloader_href=None, root_uuid=task.node.uuid) - - object_name = 'boot-%s' % task.node.uuid - - mock__publish_image.assert_called_once_with( - mock.ANY, object_name) - - mock_create_boot_iso.assert_called_once_with( - mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img', - boot_mode=None, esp_image_href=None, - configdrive_href=mock.ANY, - kernel_params='nofb nomodeset vga=normal', - root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123') - - self.assertEqual(expected_url, url) - - @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, - '_publish_image', autospec=True) - @mock.patch.object(images, 'create_boot_iso', autospec=True) - def test__prepare_iso_image_kernel_params( - self, mock_create_boot_iso, mock__publish_image): - with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: - kernel_params = 'network-config=base64-cloudinit-blob' - - task.node.instance_info.update(kernel_append_params=kernel_params) - - task.driver.boot._prepare_iso_image( - task, 'http://kernel/img', 'http://ramdisk/img', - bootloader_href=None, root_uuid=task.node.uuid) - - mock_create_boot_iso.assert_called_once_with( - mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img', - boot_mode=None, esp_image_href=None, - configdrive_href=mock.ANY, - kernel_params=kernel_params, - root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123') - - @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, - '_prepare_iso_image', autospec=True) - def test__prepare_deploy_iso(self, mock__prepare_iso_image): - with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: - - task.node.driver_info.update( - {'deploy_kernel': 'kernel', - 'deploy_ramdisk': 'ramdisk', - 'bootloader': 'bootloader'} - ) - - task.node.instance_info.update(deploy_boot_mode='uefi') - - task.driver.boot._prepare_deploy_iso(task, {}, 'deploy') - - mock__prepare_iso_image.assert_called_once_with( - mock.ANY, 'kernel', 'ramdisk', 'bootloader', params={}) - - @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, - '_prepare_iso_image', autospec=True) + @mock.patch.object(virtual_media_base, 'prepare_iso_image', autospec=True) @mock.patch.object(images, 'create_boot_iso', autospec=True) def test__prepare_boot_iso(self, mock_create_boot_iso, - mock__prepare_iso_image): + mock_prepare_iso_image): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: task.node.driver_info.update( @@ -462,9 +360,11 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): task.driver.boot._prepare_boot_iso( task, root_uuid=task.node.uuid) - mock__prepare_iso_image.assert_called_once_with( + mock_prepare_iso_image.assert_called_once_with( mock.ANY, 'http://kernel/img', 'http://ramdisk/img', - 'bootloader', root_uuid=task.node.uuid) + bootloader_href='bootloader', root_uuid=task.node.uuid, + timeout=900, use_web_server=False, + container='ironic_redfish_container') @mock.patch.object(redfish_utils, 'parse_driver_info', autospec=True) @mock.patch.object(deploy_utils, 'validate_image_properties', @@ -557,8 +457,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): @mock.patch.object(redfish_boot.manager_utils, 'node_set_boot_device', autospec=True) - @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, - '_prepare_deploy_iso', autospec=True) + @mock.patch.object(virtual_media_base, 'prepare_deploy_iso', + autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_eject_vmedia', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, @@ -571,14 +471,14 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): def test_prepare_ramdisk_with_params( self, mock_boot_mode_utils, mock_node_power_action, mock__parse_driver_info, mock__insert_vmedia, mock__eject_vmedia, - mock__prepare_deploy_iso, mock_node_set_boot_device): + mock_prepare_deploy_iso, mock_node_set_boot_device): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.provision_state = states.DEPLOYING mock__parse_driver_info.return_value = {} - mock__prepare_deploy_iso.return_value = 'image-url' + mock_prepare_deploy_iso.return_value = 'image-url' task.driver.boot.prepare_ramdisk(task, {}) @@ -597,8 +497,9 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): 'ipa-debug': '1', } - mock__prepare_deploy_iso.assert_called_once_with( - task, expected_params, 'deploy') + mock_prepare_deploy_iso.assert_called_once_with( + task, expected_params, 'deploy', {}, + use_web_server=False, container='ironic_redfish_container') mock_node_set_boot_device.assert_called_once_with( task, boot_devices.CDROM, False) @@ -607,8 +508,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): @mock.patch.object(redfish_boot.manager_utils, 'node_set_boot_device', autospec=True) - @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, - '_prepare_deploy_iso', autospec=True) + @mock.patch.object(virtual_media_base, 'prepare_deploy_iso', + autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_eject_vmedia', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, @@ -621,14 +522,14 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): def test_prepare_ramdisk_no_debug( self, mock_boot_mode_utils, mock_node_power_action, mock__parse_driver_info, mock__insert_vmedia, mock__eject_vmedia, - mock__prepare_deploy_iso, mock_node_set_boot_device): + mock_prepare_deploy_iso, mock_node_set_boot_device): self.config(debug=False) with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: task.node.provision_state = states.DEPLOYING mock__parse_driver_info.return_value = {} - mock__prepare_deploy_iso.return_value = 'image-url' + mock_prepare_deploy_iso.return_value = 'image-url' task.driver.boot.prepare_ramdisk(task, {}) @@ -646,8 +547,9 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): 'ipa-agent-token': mock.ANY, } - mock__prepare_deploy_iso.assert_called_once_with( - task, expected_params, 'deploy') + mock_prepare_deploy_iso.assert_called_once_with( + task, expected_params, 'deploy', {}, + use_web_server=False, container='ironic_redfish_container') mock_node_set_boot_device.assert_called_once_with( task, boot_devices.CDROM, False) @@ -658,8 +560,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_prepare_floppy_image', autospec=True) - @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, - '_prepare_deploy_iso', autospec=True) + @mock.patch.object(virtual_media_base, 'prepare_deploy_iso', + autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_has_vmedia_device', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, @@ -674,7 +576,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): def test_prepare_ramdisk_with_floppy( self, mock_boot_mode_utils, mock_node_power_action, mock__parse_driver_info, mock__insert_vmedia, mock__eject_vmedia, - mock__has_vmedia_device, mock__prepare_deploy_iso, + mock__has_vmedia_device, mock_prepare_deploy_iso, mock__prepare_floppy_image, mock_node_set_boot_device): with task_manager.acquire(self.context, self.node.uuid, @@ -687,7 +589,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock__has_vmedia_device.return_value = True mock__prepare_floppy_image.return_value = 'floppy-image-url' - mock__prepare_deploy_iso.return_value = 'cd-image-url' + mock_prepare_deploy_iso.return_value = 'cd-image-url' task.driver.boot.prepare_ramdisk(task, {}) @@ -720,8 +622,11 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): 'ipa-agent-token': mock.ANY, } - mock__prepare_deploy_iso.assert_called_once_with( - task, expected_params, 'deploy') + expected_d_info = {'config_via_floppy': True} + + mock_prepare_deploy_iso.assert_called_once_with( + task, expected_params, 'deploy', expected_d_info, + use_web_server=False, container='ironic_redfish_container') mock_node_set_boot_device.assert_called_once_with( task, boot_devices.CDROM, False) diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py index 09d903f91..c8a37951a 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_management.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_management.py @@ -127,6 +127,31 @@ class RedfishManagementTestCase(db_base.DbTestCase): fake_system.set_system_boot_options.reset_mock() mock_get_system.reset_mock() + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_set_boot_device_persistency_no_change(self, mock_get_system): + fake_system = mock.Mock() + mock_get_system.return_value = fake_system + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + expected_values = [ + (True, sushy.BOOT_SOURCE_ENABLED_CONTINUOUS), + (False, sushy.BOOT_SOURCE_ENABLED_ONCE) + ] + + for target, expected in expected_values: + fake_system.boot.get.return_value = expected + + task.driver.management.set_boot_device( + task, boot_devices.PXE, persistent=target) + + fake_system.set_system_boot_options.assert_called_once_with( + sushy.BOOT_SOURCE_TARGET_PXE, enabled=None) + mock_get_system.assert_called_once_with(task.node) + + # Reset mocks + fake_system.set_system_boot_options.reset_mock() + mock_get_system.reset_mock() + @mock.patch.object(sushy, 'Sushy', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_set_boot_device_fail(self, mock_get_system, mock_sushy): diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 354e04159..590a22e08 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -1362,6 +1362,8 @@ class TestAgentDeploy(db_base.DbTestCase): power_on_node_if_needed_mock, resume_mock): check_deploy_mock.return_value = None + self.node.instance_info = { + 'capabilities': {'boot_option': 'netboot'}} uuid_mock.return_value = 'root_uuid' self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE diff --git a/ironic/tests/unit/drivers/modules/test_agent_base.py b/ironic/tests/unit/drivers/modules/test_agent_base.py index 62b2e892f..30a0cb9dd 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base.py @@ -384,9 +384,10 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): failed_mock.assert_called_once_with( task, mock.ANY, collect_logs=True) log_mock.assert_called_once_with( - 'Asynchronous exception: Failed checking if deploy is done. ' - 'Exception: LlamaException for node %(node)s', - {'node': task.node.uuid}) + 'Asynchronous exception for node %(node)s: %(err)s', + {'err': 'Failed checking if deploy is done. ' + 'Error: LlamaException', + 'node': task.node.uuid}) @mock.patch.object(agent_base.HeartbeatMixin, 'in_core_deploy_step', autospec=True) @@ -420,9 +421,10 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): # deploy_utils.set_failed_state anymore self.assertFalse(failed_mock.called) log_mock.assert_called_once_with( - 'Asynchronous exception: Failed checking if deploy is done. ' - 'Exception: LlamaException for node %(node)s', - {'node': task.node.uuid}) + 'Asynchronous exception for node %(node)s: %(err)s', + {'err': 'Failed checking if deploy is done. ' + 'Error: LlamaException', + 'node': task.node.uuid}) @mock.patch.object(objects.node.Node, 'touch_provisioning', autospec=True) @mock.patch.object(agent_base.HeartbeatMixin, @@ -574,8 +576,8 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): mock_finalize.assert_called_once_with(mock.ANY, task) mock_rescue_err_handler.assert_called_once_with( - task, 'Asynchronous exception: Node failed to perform ' - 'rescue operation. Exception: some failure for node') + task, 'Node failed to perform ' + 'rescue operation. Error: some failure') @mock.patch.object(agent_base.HeartbeatMixin, 'in_core_deploy_step', autospec=True) @@ -1082,7 +1084,10 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): @mock.patch.object(agent_client.AgentClient, 'install_bootloader', autospec=True) @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) - def test_configure_local_boot(self, try_set_boot_device_mock, + @mock.patch.object(boot_mode_utils, 'get_boot_mode', autospec=True, + return_value='whatever') + def test_configure_local_boot(self, boot_mode_mock, + try_set_boot_device_mock, install_bootloader_mock): install_bootloader_mock.return_value = { 'command_status': 'SUCCESS', 'command_error': None} @@ -1092,17 +1097,24 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.deploy.configure_local_boot(task, root_uuid='some-root-uuid') try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK, persistent=True) + boot_mode_mock.assert_called_once_with(task.node) install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', - efi_system_part_uuid=None, prep_boot_part_uuid=None) + efi_system_part_uuid=None, prep_boot_part_uuid=None, + target_boot_mode='whatever' + ) @mock.patch.object(agent_client.AgentClient, 'install_bootloader', autospec=True) @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) - def test_configure_local_boot_with_prep(self, try_set_boot_device_mock, + @mock.patch.object(boot_mode_utils, 'get_boot_mode', autospec=True, + return_value='whatever') + def test_configure_local_boot_with_prep(self, boot_mode_mock, + try_set_boot_device_mock, install_bootloader_mock): install_bootloader_mock.return_value = { 'command_status': 'SUCCESS', 'command_error': None} + with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: task.node.driver_internal_info['is_whole_disk_image'] = False @@ -1110,14 +1122,20 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): prep_boot_part_uuid='fake-prep') try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK, persistent=True) + boot_mode_mock.assert_called_once_with(task.node) install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', - efi_system_part_uuid=None, prep_boot_part_uuid='fake-prep') + efi_system_part_uuid=None, prep_boot_part_uuid='fake-prep', + target_boot_mode='whatever' + ) @mock.patch.object(agent_client.AgentClient, 'install_bootloader', autospec=True) @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) - def test_configure_local_boot_uefi(self, try_set_boot_device_mock, + @mock.patch.object(boot_mode_utils, 'get_boot_mode', autospec=True, + return_value='uefi') + def test_configure_local_boot_uefi(self, boot_mode_mock, + try_set_boot_device_mock, install_bootloader_mock): install_bootloader_mock.return_value = { 'command_status': 'SUCCESS', 'command_error': None} @@ -1129,10 +1147,13 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): efi_system_part_uuid='efi-system-part-uuid') try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK, persistent=True) + boot_mode_mock.assert_called_once_with(task.node) install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', efi_system_part_uuid='efi-system-part-uuid', - prep_boot_part_uuid=None) + prep_boot_part_uuid=None, + target_boot_mode='uefi' + ) @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) @mock.patch.object(agent_client.AgentClient, 'install_bootloader', @@ -1178,7 +1199,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid=None, efi_system_part_uuid='efi-system-part-uuid', - prep_boot_part_uuid=None) + prep_boot_part_uuid=None, target_boot_mode='uefi') @mock.patch.object(image_service, 'GlanceImageService', autospec=True) @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) @@ -1242,7 +1263,8 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): # check if the root_uuid comes from the driver_internal_info install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid=root_uuid, - efi_system_part_uuid=None, prep_boot_part_uuid=None) + efi_system_part_uuid=None, prep_boot_part_uuid=None, + target_boot_mode='bios') try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK, persistent=True) @@ -1324,8 +1346,11 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): autospec=True) @mock.patch.object(agent_client.AgentClient, 'install_bootloader', autospec=True) + @mock.patch.object(boot_mode_utils, 'get_boot_mode', autospec=True, + return_value='whatever') def test_configure_local_boot_boot_loader_install_fail( - self, install_bootloader_mock, collect_logs_mock): + self, boot_mode_mock, install_bootloader_mock, + collect_logs_mock): install_bootloader_mock.return_value = { 'command_status': 'FAILED', 'command_error': 'boom'} self.node.provision_state = states.DEPLOYING @@ -1337,9 +1362,12 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.assertRaises(exception.InstanceDeployFailure, self.deploy.configure_local_boot, task, root_uuid='some-root-uuid') + boot_mode_mock.assert_called_once_with(task.node) install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', - efi_system_part_uuid=None, prep_boot_part_uuid=None) + efi_system_part_uuid=None, prep_boot_part_uuid=None, + target_boot_mode='whatever' + ) collect_logs_mock.assert_called_once_with(mock.ANY, task.node) self.assertEqual(states.DEPLOYFAIL, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) @@ -1349,9 +1377,11 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) @mock.patch.object(agent_client.AgentClient, 'install_bootloader', autospec=True) + @mock.patch.object(boot_mode_utils, 'get_boot_mode', autospec=True, + return_value='whatever') def test_configure_local_boot_set_boot_device_fail( - self, install_bootloader_mock, try_set_boot_device_mock, - collect_logs_mock): + self, boot_mode_mock, install_bootloader_mock, + try_set_boot_device_mock, collect_logs_mock): install_bootloader_mock.return_value = { 'command_status': 'SUCCESS', 'command_error': None} try_set_boot_device_mock.side_effect = RuntimeError('error') @@ -1365,9 +1395,11 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.deploy.configure_local_boot, task, root_uuid='some-root-uuid', prep_boot_part_uuid=None) + boot_mode_mock.assert_called_once_with(task.node) install_bootloader_mock.assert_called_once_with( mock.ANY, task.node, root_uuid='some-root-uuid', - efi_system_part_uuid=None, prep_boot_part_uuid=None) + efi_system_part_uuid=None, prep_boot_part_uuid=None, + target_boot_mode='whatever') try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK, persistent=True) collect_logs_mock.assert_called_once_with(mock.ANY, task.node) diff --git a/ironic/tests/unit/drivers/modules/test_agent_client.py b/ironic/tests/unit/drivers/modules/test_agent_client.py index 790425e1e..65bb58172 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_client.py +++ b/ironic/tests/unit/drivers/modules/test_agent_client.py @@ -70,7 +70,8 @@ class TestAgentClient(base.TestCase): def test__get_command_url(self): command_url = self.client._get_command_url(self.node) - expected = self.node.driver_internal_info['agent_url'] + '/v1/commands' + expected = ('%s/v1/commands/' + % self.node.driver_internal_info['agent_url']) self.assertEqual(expected, command_url) def test__get_command_url_fail(self): @@ -276,11 +277,12 @@ class TestAgentClient(base.TestCase): self.client._command = mock.MagicMock(spec_set=[]) params = {'root_uuid': root_uuid, 'efi_system_part_uuid': efi_system_part_uuid, - 'prep_boot_part_uuid': prep_boot_part_uuid} + 'prep_boot_part_uuid': prep_boot_part_uuid, + 'target_boot_mode': 'hello'} self.client.install_bootloader( self.node, root_uuid, efi_system_part_uuid=efi_system_part_uuid, - prep_boot_part_uuid=prep_boot_part_uuid) + prep_boot_part_uuid=prep_boot_part_uuid, target_boot_mode='hello') self.client._command.assert_called_once_with( command_timeout_factor=2, node=self.node, method='image.install_bootloader', params=params, @@ -290,7 +292,7 @@ class TestAgentClient(base.TestCase): self._test_install_bootloader(root_uuid='fake-root-uuid', efi_system_part_uuid='fake-efi-uuid') - def test_install_bootloaderi_with_prep(self): + def test_install_bootloader_with_prep(self): self._test_install_bootloader(root_uuid='fake-root-uuid', efi_system_part_uuid='fake-efi-uuid', prep_boot_part_uuid='fake-prep-uuid') diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 20e578bfb..197a54d46 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -15,6 +15,7 @@ # under the License. import os +import shutil import tempfile import fixtures @@ -701,7 +702,7 @@ class OtherFunctionTestCase(db_base.DbTestCase): def test_get_boot_option_default_value(self): self.node.instance_info = {} result = utils.get_boot_option(self.node) - self.assertEqual("netboot", result) + self.assertEqual("local", result) def test_get_boot_option_overridden_default_value(self): cfg.CONF.set_override('default_boot_option', 'local', 'deploy') @@ -1241,6 +1242,38 @@ class AgentMethodsTestCase(db_base.DbTestCase): self.assertTrue( utils.direct_deploy_should_convert_raw_image(self.node)) + @mock.patch.object(shutil, 'copyfile', spec_set=True, + autospec=True) + def test_copy_image_to_web_server(self, copy_mock): + cfg.CONF.deploy.http_url = "http://x.y.z.a/webserver/" + cfg.CONF.deploy.http_root = "/webserver" + expected_url = "http://x.y.z.a/webserver/image-UUID" + source = 'tmp_image_file' + destination = "image-UUID" + image_path = "/webserver/image-UUID" + actual_url = utils.copy_image_to_web_server(source, destination) + self.assertEqual(expected_url, actual_url) + copy_mock.assert_called_once_with(source, image_path) + + @mock.patch.object(os, 'chmod', spec_set=True, + autospec=True) + @mock.patch.object(shutil, 'copyfile', spec_set=True, + autospec=True) + def test_copy_image_to_web_server_fails(self, copy_mock, + chmod_mock): + cfg.CONF.deploy.http_url = "http://x.y.z.a/webserver/" + cfg.CONF.deploy.http_root = "/webserver" + source = 'tmp_image_file' + destination = "image-UUID" + image_path = "/webserver/image-UUID" + exc = exception.ImageUploadFailed('reason') + copy_mock.side_effect = exc + self.assertRaises(exception.ImageUploadFailed, + utils.copy_image_to_web_server, + source, destination) + copy_mock.assert_called_once_with(source, image_path) + self.assertFalse(chmod_mock.called) + class ValidateImagePropertiesTestCase(db_base.DbTestCase): diff --git a/ironic/tests/unit/drivers/modules/test_ipxe.py b/ironic/tests/unit/drivers/modules/test_ipxe.py index 4b1d81670..cbec1bb0d 100644 --- a/ironic/tests/unit/drivers/modules/test_ipxe.py +++ b/ironic/tests/unit/drivers/modules/test_ipxe.py @@ -179,9 +179,11 @@ class iPXEBootTestCase(db_base.DbTestCase): @mock.patch.object(image_service.GlanceImageService, 'show', autospec=True) def test_validate_fail_no_image_kernel_ramdisk_props(self, mock_glance): + instance_info = {"boot_option": "netboot"} mock_glance.return_value = {'properties': {}} with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: + task.node.instance_info['capabilities'] = instance_info self.assertRaises(exception.MissingParameterValue, task.driver.boot.validate, task) @@ -629,6 +631,7 @@ class iPXEBootTestCase(db_base.DbTestCase): dhcp_factory_mock.return_value = provider_mock image_info = {'kernel': ('', '/path/to/kernel'), 'ramdisk': ('', '/path/to/ramdisk')} + instance_info = {"boot_option": "netboot"} get_image_info_mock.return_value = image_info with task_manager.acquire(self.context, self.node.uuid) as task: dhcp_opts = pxe_utils.dhcp_options_for_instance( @@ -638,6 +641,7 @@ class iPXEBootTestCase(db_base.DbTestCase): pxe_config_path = pxe_utils.get_pxe_config_file_path( task.node.uuid, ipxe_enabled=True) task.node.properties['capabilities'] = 'boot_mode:bios' + task.node.instance_info['capabilities'] = instance_info task.node.driver_internal_info['root_uuid_or_disk_id'] = ( "30212642-09d3-467f-8e09-21685826ab50") task.node.driver_internal_info['is_whole_disk_image'] = False @@ -671,6 +675,7 @@ class iPXEBootTestCase(db_base.DbTestCase): dhcp_factory_mock.return_value = provider_mock image_info = {'kernel': ('', '/path/to/kernel'), 'ramdisk': ('', '/path/to/ramdisk')} + instance_info = {"boot_option": "netboot"} get_image_info_mock.return_value = image_info self.node.provision_state = states.ACTIVE self.node.save() @@ -682,6 +687,7 @@ class iPXEBootTestCase(db_base.DbTestCase): pxe_config_path = pxe_utils.get_pxe_config_file_path( task.node.uuid, ipxe_enabled=True) task.node.properties['capabilities'] = 'boot_mode:bios' + task.node.instance_info['capabilities'] = instance_info task.node.driver_internal_info['root_uuid_or_disk_id'] = ( "30212642-09d3-467f-8e09-21685826ab50") task.node.driver_internal_info['is_whole_disk_image'] = False @@ -715,12 +721,14 @@ class iPXEBootTestCase(db_base.DbTestCase): image_info = {'kernel': ('', '/path/to/kernel'), 'ramdisk': ('', '/path/to/ramdisk')} get_image_info_mock.return_value = image_info + instance_info = {"boot_option": "netboot"} with task_manager.acquire(self.context, self.node.uuid) as task: dhcp_opts = pxe_utils.dhcp_options_for_instance( task, ipxe_enabled=True, ip_version=4) dhcp_opts += pxe_utils.dhcp_options_for_instance( task, ipxe_enabled=True, ip_version=6) task.node.properties['capabilities'] = 'boot_mode:bios' + task.node.instance_info['capabilities'] = instance_info task.node.driver_internal_info['is_whole_disk_image'] = False task.driver.boot.prepare_instance(task) @@ -748,12 +756,14 @@ class iPXEBootTestCase(db_base.DbTestCase): provider_mock = mock.MagicMock() dhcp_factory_mock.return_value = provider_mock get_image_info_mock.return_value = {} + instance_info = {"boot_option": "netboot"} with task_manager.acquire(self.context, self.node.uuid) as task: dhcp_opts = pxe_utils.dhcp_options_for_instance( task, ipxe_enabled=True) dhcp_opts += pxe_utils.dhcp_options_for_instance( task, ipxe_enabled=True, ip_version=6) task.node.properties['capabilities'] = 'boot_mode:bios' + task.node.instance_info['capabilities'] = instance_info task.node.driver_internal_info['is_whole_disk_image'] = True task.driver.boot.prepare_instance(task) get_image_info_mock.assert_called_once_with( diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index ab887e96b..e723b3518 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -423,7 +423,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): def test_get_deploy_info_boot_option_default(self): ret_val = self._test_get_deploy_info() - self.assertEqual('netboot', ret_val['boot_option']) + self.assertEqual('local', ret_val['boot_option']) def test_get_deploy_info_netboot_specified(self): capabilities = {'capabilities': {'boot_option': 'netboot'}} @@ -1001,11 +1001,14 @@ class ISCSIDeployTestCase(db_base.DbTestCase): def test_continue_deploy_netboot(self, do_agent_iscsi_deploy_mock, reboot_and_finish_deploy_mock): + self.node.instance_info = { + 'capabilities': {'boot_option': 'netboot'}} self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE self.node.save() uuid_dict_returned = {'root uuid': 'some-root-uuid'} do_agent_iscsi_deploy_mock.return_value = uuid_dict_returned + self.node.save() with task_manager.acquire(self.context, self.node.uuid) as task: with mock.patch.object( task.driver.boot, 'prepare_instance') as m_prep_instance: @@ -1395,7 +1398,7 @@ class PhysicalWorkTestCase(tests_base.TestCase): 'configdrive': deploy_args['configdrive'], # boot_option defaults to 'netboot' if # not set - 'boot_option': deploy_args['boot_option'] or 'netboot', + 'boot_option': deploy_args['boot_option'] or 'local', 'boot_mode': deploy_args['boot_mode'], 'disk_label': deploy_args['disk_label'], 'cpu_arch': deploy_args['cpu_arch'] or '' @@ -1762,7 +1765,7 @@ class PhysicalWorkTestCase(tests_base.TestCase): ephemeral_format, image_path, node_uuid, configdrive=None, preserve_ephemeral=False, - boot_option="netboot", + boot_option="local", boot_mode="bios", disk_label=None, cpu_arch="")] diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index f5ef8cd26..ed4fa2b63 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/ironic/tests/unit/drivers/modules/test_pxe.py @@ -181,9 +181,11 @@ class PXEBootTestCase(db_base.DbTestCase): @mock.patch.object(image_service.GlanceImageService, 'show', autospec=True) def test_validate_fail_no_image_kernel_ramdisk_props(self, mock_glance): + instance_info = {"boot_option": "netboot"} mock_glance.return_value = {'properties': {}} with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: + task.node.instance_info['capabilities'] = instance_info self.assertRaises(exception.MissingParameterValue, task.driver.boot.validate, task) @@ -564,7 +566,8 @@ class PXEBootTestCase(db_base.DbTestCase): task.node.driver_internal_info['root_uuid_or_disk_id'] = ( "30212642-09d3-467f-8e09-21685826ab50") task.node.driver_internal_info['is_whole_disk_image'] = False - + task.node.instance_info = { + 'capabilities': {'boot_option': 'netboot'}} task.driver.boot.prepare_instance(task) get_image_info_mock.assert_called_once_with( @@ -594,6 +597,7 @@ class PXEBootTestCase(db_base.DbTestCase): dhcp_factory_mock.return_value = provider_mock image_info = {'kernel': ('', '/path/to/kernel'), 'ramdisk': ('', '/path/to/ramdisk')} + instance_info = {"boot_option": "netboot"} get_image_info_mock.return_value = image_info self.node.provision_state = states.ACTIVE self.node.save() @@ -608,7 +612,7 @@ class PXEBootTestCase(db_base.DbTestCase): task.node.driver_internal_info['root_uuid_or_disk_id'] = ( "30212642-09d3-467f-8e09-21685826ab50") task.node.driver_internal_info['is_whole_disk_image'] = False - + task.node.instance_info['capabilities'] = instance_info task.driver.boot.prepare_instance(task) get_image_info_mock.assert_called_once_with( @@ -637,6 +641,7 @@ class PXEBootTestCase(db_base.DbTestCase): dhcp_factory_mock.return_value = provider_mock image_info = {'kernel': ('', '/path/to/kernel'), 'ramdisk': ('', '/path/to/ramdisk')} + instance_info = {"boot_option": "netboot"} get_image_info_mock.return_value = image_info with task_manager.acquire(self.context, self.node.uuid) as task: dhcp_opts = pxe_utils.dhcp_options_for_instance( @@ -644,6 +649,7 @@ class PXEBootTestCase(db_base.DbTestCase): dhcp_opts += pxe_utils.dhcp_options_for_instance( task, ipxe_enabled=False, ip_version=6) task.node.properties['capabilities'] = 'boot_mode:bios' + task.node.instance_info['capabilities'] = instance_info task.node.driver_internal_info['is_whole_disk_image'] = False task.driver.boot.prepare_instance(task) @@ -669,12 +675,14 @@ class PXEBootTestCase(db_base.DbTestCase): provider_mock = mock.MagicMock() dhcp_factory_mock.return_value = provider_mock get_image_info_mock.return_value = {} + instance_info = {"boot_option": "netboot"} with task_manager.acquire(self.context, self.node.uuid) as task: dhcp_opts = pxe_utils.dhcp_options_for_instance( task, ipxe_enabled=False) dhcp_opts += pxe_utils.dhcp_options_for_instance( task, ipxe_enabled=False, ip_version=6) task.node.properties['capabilities'] = 'boot_mode:bios' + task.node.instance_info['capabilities'] = instance_info task.node.driver_internal_info['is_whole_disk_image'] = True task.driver.boot.prepare_instance(task) get_image_info_mock.assert_called_once_with(task, diff --git a/ironic/tests/unit/drivers/modules/test_virtual_media_base.py b/ironic/tests/unit/drivers/modules/test_virtual_media_base.py new file mode 100644 index 000000000..c1385b047 --- /dev/null +++ b/ironic/tests/unit/drivers/modules/test_virtual_media_base.py @@ -0,0 +1,232 @@ +# 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. + +import tempfile + +import mock +from oslo_config import cfg +import six + +from ironic.common import images +from ironic.common import swift +from ironic.conductor import task_manager +from ironic.drivers.modules import deploy_utils +from ironic.drivers.modules import virtual_media_base +from ironic.drivers import utils as driver_utils +from ironic.tests.unit.db import base as db_base +from ironic.tests.unit.db import utils as db_utils +from ironic.tests.unit.objects import utils as object_utils + +if six.PY3: + import io + file = io.BytesIO + +INFO_DICT = db_utils.get_test_redfish_info() + +CONF = cfg.CONF + + +class VirtualMediaCommonMethodsTestCase(db_base.DbTestCase): + + def setUp(self): + super(VirtualMediaCommonMethodsTestCase, self).setUp() + self.config(enabled_hardware_types=['ilo', 'fake-hardware'], + enabled_boot_interfaces=['ilo-pxe', 'ilo-virtual-media', + 'fake'], + enabled_bios_interfaces=['ilo', 'no-bios'], + enabled_power_interfaces=['ilo', 'fake'], + enabled_management_interfaces=['ilo', 'fake'], + enabled_inspect_interfaces=['ilo', 'fake', 'no-inspect'], + enabled_console_interfaces=['ilo', 'fake', 'no-console'], + enabled_vendor_interfaces=['ilo', 'fake', 'no-vendor']) + self.node = object_utils.create_test_node( + self.context, boot_interface='ilo-virtual-media', + deploy_interface='direct') + + def test_get_iso_image_name(self): + boot_iso_actual = virtual_media_base.get_iso_image_name(self.node) + boot_iso_expected = "boot-%s" % self.node.uuid + self.assertEqual(boot_iso_expected, boot_iso_actual) + + @mock.patch.object(tempfile, 'NamedTemporaryFile', spec_set=True, + autospec=True) + @mock.patch.object(images, 'create_boot_iso', spec_set=True, autospec=True) + @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) + @mock.patch.object(virtual_media_base, 'get_iso_image_name', + spec_set=True, autospec=True) + @mock.patch.object(driver_utils, 'get_node_capability', spec_set=True, + autospec=True) + def test__prepare_iso_image_uefi(self, capability_mock, + iso_image_name_mock, swift_api_mock, + create_boot_iso_mock, tempfile_mock): + CONF.ilo.swift_ilo_container = 'ilo-cont' + CONF.ilo.use_web_server_for_images = False + + swift_obj_mock = swift_api_mock.return_value + fileobj_mock = mock.MagicMock(spec=file) + fileobj_mock.name = 'tmpfile' + mock_file_handle = mock.MagicMock(spec=file) + mock_file_handle.__enter__.return_value = fileobj_mock + tempfile_mock.return_value = mock_file_handle + iso_image_name_mock.return_value = 'abcdef' + create_boot_iso_mock.return_value = '/path/to/boot-iso' + capability_mock.return_value = 'uefi' + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + boot_iso_actual = virtual_media_base.prepare_iso_image( + task, 'kernel_uuid', 'ramdisk_uuid', + deploy_iso_href='deploy_iso_uuid', + bootloader_href='bootloader_uuid', + root_uuid='root-uuid', + kernel_params='kernel-params', + timeout=None, + container=CONF.ilo.swift_ilo_container, + use_web_server=CONF.ilo.use_web_server_for_images) + iso_image_name_mock.assert_called_once_with(task.node) + create_boot_iso_mock.assert_called_once_with( + task.context, 'tmpfile', 'kernel_uuid', 'ramdisk_uuid', + deploy_iso_href='deploy_iso_uuid', + esp_image_href='bootloader_uuid', + root_uuid='root-uuid', + kernel_params='kernel-params', + boot_mode='uefi') + swift_obj_mock.create_object.assert_called_once_with( + 'ilo-cont', 'abcdef', 'tmpfile', None) + boot_iso_expected = 'swift:abcdef' + self.assertEqual(boot_iso_expected, boot_iso_actual) + + @mock.patch.object(tempfile, 'NamedTemporaryFile', spec_set=True, + autospec=True) + @mock.patch.object(images, 'create_boot_iso', spec_set=True, autospec=True) + @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) + @mock.patch.object(virtual_media_base, 'get_iso_image_name', + spec_set=True, autospec=True) + @mock.patch.object(driver_utils, 'get_node_capability', spec_set=True, + autospec=True) + def test__prepare_iso_image_bios(self, capability_mock, + iso_image_name_mock, swift_api_mock, + create_boot_iso_mock, tempfile_mock): + CONF.ilo.swift_ilo_container = 'ilo-cont' + CONF.ilo.use_web_server_for_images = False + + swift_obj_mock = swift_api_mock.return_value + fileobj_mock = mock.MagicMock(spec=file) + fileobj_mock.name = 'tmpfile' + mock_file_handle = mock.MagicMock(spec=file) + mock_file_handle.__enter__.return_value = fileobj_mock + tempfile_mock.return_value = mock_file_handle + iso_image_name_mock.return_value = 'abcdef' + create_boot_iso_mock.return_value = '/path/to/boot-iso' + capability_mock.return_value = 'bios' + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + boot_iso_actual = virtual_media_base.prepare_iso_image( + task, 'kernel_uuid', 'ramdisk_uuid', + deploy_iso_href='deploy_iso_uuid', + root_uuid='root-uuid', + kernel_params='kernel-params', + timeout=None, + container=CONF.ilo.swift_ilo_container, + use_web_server=CONF.ilo.use_web_server_for_images) + iso_image_name_mock.assert_called_once_with(task.node) + create_boot_iso_mock.assert_called_once_with( + task.context, 'tmpfile', 'kernel_uuid', 'ramdisk_uuid', + deploy_iso_href='deploy_iso_uuid', + esp_image_href=None, + root_uuid='root-uuid', + kernel_params='kernel-params', + boot_mode='bios') + swift_obj_mock.create_object.assert_called_once_with( + 'ilo-cont', 'abcdef', 'tmpfile', None) + boot_iso_expected = 'swift:abcdef' + self.assertEqual(boot_iso_expected, boot_iso_actual) + + @mock.patch.object(deploy_utils, 'copy_image_to_web_server', spec_set=True, + autospec=True) + @mock.patch.object(tempfile, 'NamedTemporaryFile', spec_set=True, + autospec=True) + @mock.patch.object(images, 'create_boot_iso', spec_set=True, autospec=True) + @mock.patch.object(virtual_media_base, 'get_iso_image_name', + spec_set=True, autospec=True) + @mock.patch.object(driver_utils, 'get_node_capability', spec_set=True, + autospec=True) + def test__prepare_iso_image_use_webserver(self, capability_mock, + iso_image_name_mock, + create_boot_iso_mock, + tempfile_mock, copy_file_mock): + CONF.ilo.use_web_server_for_images = True + CONF.deploy.http_url = "http://10.10.1.30/httpboot" + CONF.deploy.http_root = "/httpboot" + CONF.pxe.pxe_append_params = 'kernel-params' + + fileobj_mock = mock.MagicMock(spec=file) + fileobj_mock.name = 'tmpfile' + mock_file_handle = mock.MagicMock(spec=file) + mock_file_handle.__enter__.return_value = fileobj_mock + tempfile_mock.return_value = mock_file_handle + + ramdisk_href = "http://10.10.1.30/httpboot/ramdisk" + kernel_href = "http://10.10.1.30/httpboot/kernel" + iso_image_name_mock.return_value = 'new_boot_iso' + create_boot_iso_mock.return_value = '/path/to/boot-iso' + capability_mock.return_value = 'uefi' + copy_file_mock.return_value = "http://10.10.1.30/httpboot/new_boot_iso" + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + driver_internal_info = task.node.driver_internal_info + driver_internal_info['boot_iso_created_in_web_server'] = True + boot_iso_actual = virtual_media_base.prepare_iso_image( + task, kernel_href, ramdisk_href, + deploy_iso_href='deploy_iso_uuid', + bootloader_href='bootloader_uuid', + root_uuid='root-uuid', + kernel_params='kernel-params', + use_web_server=CONF.ilo.use_web_server_for_images) + iso_image_name_mock.assert_called_once_with(task.node) + create_boot_iso_mock.assert_called_once_with( + task.context, 'tmpfile', kernel_href, ramdisk_href, + deploy_iso_href='deploy_iso_uuid', + esp_image_href='bootloader_uuid', + root_uuid='root-uuid', + kernel_params='kernel-params', + boot_mode='uefi') + boot_iso_expected = 'http://10.10.1.30/httpboot/new_boot_iso' + self.assertEqual(boot_iso_expected, boot_iso_actual) + copy_file_mock.assert_called_once_with(fileobj_mock.name, + 'new_boot_iso') + + @mock.patch.object(virtual_media_base, 'prepare_iso_image', spec_set=True, + autospec=True) + def test_prepare_deploy_iso(self, prepare_iso_mock): + driver_info = {'deploy_kernel': 'kernel', 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader'} + CONF.pxe.pxe_append_params = 'kernel-params' + timeout = None + container = 'container' + prepare_iso_mock.return_value = ( + 'swift:boot-b5451849-e088-4a4c-aa5f-4d97b3371dec') + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + deploy_iso_actual = virtual_media_base.prepare_deploy_iso( + task, {}, 'deploy', driver_info, use_web_server=False, + container=container) + prepare_iso_mock.assert_called_once_with( + task, 'kernel', 'ramdisk', bootloader_href='bootloader', + kernel_params=CONF.pxe.pxe_append_params, timeout=timeout, + use_web_server=False, container='container') + deploy_iso_expected = ( + 'swift:boot-b5451849-e088-4a4c-aa5f-4d97b3371dec') + self.assertEqual(deploy_iso_expected, deploy_iso_actual) diff --git a/ironic/tests/unit/drivers/third_party_driver_mocks.py b/ironic/tests/unit/drivers/third_party_driver_mocks.py index 3f3346a3d..4dcd45e51 100644 --- a/ironic/tests/unit/drivers/third_party_driver_mocks.py +++ b/ironic/tests/unit/drivers/third_party_driver_mocks.py @@ -106,6 +106,17 @@ if not dracclient: sys.modules['dracclient.exceptions'] = dracclient.exceptions dracclient.exceptions.BaseClientException = type('BaseClientException', (Exception,), {}) + + dracclient.exceptions.DRACRequestFailed = type( + 'DRACRequestFailed', (dracclient.exceptions.BaseClientException,), {}) + + class DRACOperationFailed(dracclient.exceptions.DRACRequestFailed): + def __init__(self, **kwargs): + super(DRACOperationFailed, self).__init__( + 'DRAC operation failed. Messages: %(drac_messages)s' % kwargs) + + dracclient.exceptions.DRACOperationFailed = DRACOperationFailed + # Now that the external library has been mocked, if anything had already # loaded any of the drivers, reload them. if 'ironic.drivers.modules.drac' in sys.modules: diff --git a/ironic/tests/unit/policy_fixture.py b/ironic/tests/unit/policy_fixture.py index 4fe3b5faa..4e07c2554 100644 --- a/ironic/tests/unit/policy_fixture.py +++ b/ironic/tests/unit/policy_fixture.py @@ -22,7 +22,7 @@ from ironic.common import policy as ironic_policy CONF = cfg.CONF -# NOTE(deva): We ship a default that always masks passwords, but for testing +# NOTE(tenbrae): We ship a default that always masks passwords, but for testing # we need to override that default to ensure passwords can be # made visible by operators that choose to do so. policy_data = """ diff --git a/releasenotes/notes/add-iso-less-vmedia-ilo-e93002e335cb21e1.yaml b/releasenotes/notes/add-iso-less-vmedia-ilo-e93002e335cb21e1.yaml new file mode 100644 index 000000000..6636afe41 --- /dev/null +++ b/releasenotes/notes/add-iso-less-vmedia-ilo-e93002e335cb21e1.yaml @@ -0,0 +1,8 @@ +features: + - | + Add functionality to perform virtual media boot without + user-built deploy/rescue/boot ISO images for hardware type ilo. + Instead, ironic will build necessary images out of common + kernel/ramdisk pair (though user needs to provide ESP image). + User provided deploy/rescue/boot ISO images are + also supported. diff --git a/releasenotes/notes/bug-2004265-cd9056868295f374.yaml b/releasenotes/notes/bug-2004265-cd9056868295f374.yaml new file mode 100644 index 000000000..5d25b5ea8 --- /dev/null +++ b/releasenotes/notes/bug-2004265-cd9056868295f374.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes 'Invalid parameter value for SpanLength' when configuring RAID + using Python 3. This passed incorrect data type to iDRAC, e.g., instead + of `2` it passed `2.0`. + See `story 2004265 <https://storyboard.openstack.org/#!/story/2004265>`_. diff --git a/releasenotes/notes/change-default-boot-option-to-local-8c326077770ab672.yaml b/releasenotes/notes/change-default-boot-option-to-local-8c326077770ab672.yaml new file mode 100644 index 000000000..7e7cd49be --- /dev/null +++ b/releasenotes/notes/change-default-boot-option-to-local-8c326077770ab672.yaml @@ -0,0 +1,8 @@ +--- +upgrade: + - | + The default value of ``[deploy]/default_boot_option`` is changed from + ``netboot`` to ``local``. + - Due to the default boot option change, partition images without ``grub2`` + will be unable to be deployed without the ``boot_option`` for the node + to be explicitly set to ``netboot``. diff --git a/releasenotes/notes/fifteen-0da3cca48dceab8b.yaml b/releasenotes/notes/fifteen-0da3cca48dceab8b.yaml new file mode 100644 index 000000000..0841d8f64 --- /dev/null +++ b/releasenotes/notes/fifteen-0da3cca48dceab8b.yaml @@ -0,0 +1,11 @@ +--- +prelude: > + The Ironic Developers are proud to announce the release of Ironic 15.0! + + This release contains a number of changes that have been sought by + operators and users of Ironic for some time, including support for UEFI + booting a software RAID system, improved Ironic/Ironic Python Agent + security, multi-tenancy constructs, a hardware retirement mechanism, + stateful DHCPv6, and numerous fixes. + + We sincerely hope you enjoy! diff --git a/releasenotes/notes/fix-gmr-37332a12065c09dc.yaml b/releasenotes/notes/fix-gmr-37332a12065c09dc.yaml new file mode 100644 index 000000000..4256fa4e0 --- /dev/null +++ b/releasenotes/notes/fix-gmr-37332a12065c09dc.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes state report via Guru Meditation Reports that did not work + previously because of empty ``log_dir`` and no way to configure + this configuration option.
\ No newline at end of file diff --git a/releasenotes/notes/idrac-fix-reboot-failure-c740e765ff41bcf0.yaml b/releasenotes/notes/idrac-fix-reboot-failure-c740e765ff41bcf0.yaml new file mode 100644 index 000000000..d5b922c70 --- /dev/null +++ b/releasenotes/notes/idrac-fix-reboot-failure-c740e765ff41bcf0.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixed a bug where rebooting a node managed by the ``idrac`` hardware + type when using the WS-MAN power interface sometimes fails with a + ``The command failed to set RequestedState`` error. See bug `2007487 + <https://storyboard.openstack.org/#!/story/2007487>`_ for details. diff --git a/releasenotes/notes/improve-redfish-set-boot-device-e38e9e9442ab5750.yaml b/releasenotes/notes/improve-redfish-set-boot-device-e38e9e9442ab5750.yaml new file mode 100644 index 000000000..a2ff17280 --- /dev/null +++ b/releasenotes/notes/improve-redfish-set-boot-device-e38e9e9442ab5750.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Makes management interface of ``redfish`` hardware type not changing + current boot frequency if currently set is the same as the desired one. + The goal is to avoid touching potentially faulty BMC option whenever + possible. diff --git a/releasenotes/notes/release-reservation-on-conductor-stop-6ebbcdf92da57ca6.yaml b/releasenotes/notes/release-reservation-on-conductor-stop-6ebbcdf92da57ca6.yaml new file mode 100644 index 000000000..1d583a88c --- /dev/null +++ b/releasenotes/notes/release-reservation-on-conductor-stop-6ebbcdf92da57ca6.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes an issue where a node may be locked from changes if a conductor's + hostname case is changed before restarting the conductor service. clean + up the reservation once the conductor stopped. diff --git a/releasenotes/notes/software-raid-with-uefi-5b88e6c5af9ea743.yaml b/releasenotes/notes/software-raid-with-uefi-5b88e6c5af9ea743.yaml new file mode 100644 index 000000000..179d2db1a --- /dev/null +++ b/releasenotes/notes/software-raid-with-uefi-5b88e6c5af9ea743.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Adds support for bootable software RAID with UEFI boot mode. diff --git a/test-requirements.txt b/test-requirements.txt index 26e52b3fc..d6c84f380 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -10,6 +10,7 @@ mock>=3.0.0 # BSD Babel!=2.4.0,>=2.3.4 # BSD PyMySQL>=0.7.6 # MIT License iso8601>=0.1.11 # MIT +oslo.reports>=1.18.0 # Apache-2.0 oslotest>=3.2.0 # Apache-2.0 stestr>=1.0.0 # Apache-2.0 psycopg2>=2.7.3 # LGPL/ZPL diff --git a/tools/config/ironic-config-generator.conf b/tools/config/ironic-config-generator.conf index def7f4f70..a14a0ec32 100644 --- a/tools/config/ironic-config-generator.conf +++ b/tools/config/ironic-config-generator.conf @@ -17,6 +17,7 @@ namespace = oslo.middleware.http_proxy_to_wsgi namespace = oslo.concurrency namespace = oslo.policy namespace = oslo.log +namespace = oslo.reports namespace = oslo.service.service namespace = oslo.service.periodic_task namespace = oslo.service.sslutils @@ -116,7 +116,7 @@ filename = *.py,app.wsgi exclude = .venv,.git,.tox,dist,doc,*lib/python*,*egg,build import-order-style = pep8 application-import-names = ironic -max-complexity=20 +max-complexity=18 # [H106] Don't put vim configuration in source files. # [H203] Use assertIs(Not)None to check for None. # [H204] Use assert(Not)Equal to check for equality. diff --git a/zuul.d/ironic-jobs.yaml b/zuul.d/ironic-jobs.yaml index 8b721d6ea..b381a1282 100644 --- a/zuul.d/ironic-jobs.yaml +++ b/zuul.d/ironic-jobs.yaml @@ -151,6 +151,7 @@ IRONIC_ENABLED_POWER_INTERFACES: redfish IRONIC_ENABLED_MANAGEMENT_INTERFACES: redfish IRONIC_AUTOMATED_CLEAN_ENABLED: False + IRONIC_DEFAULT_BOOT_OPTION: netboot - job: name: ironic-tempest-partition-uefi-redfish-vmedia @@ -198,6 +199,7 @@ IRONIC_ENABLED_BOOT_INTERFACES: "fake,pxe" IRONIC_IPXE_ENABLED: False IRONIC_AUTOMATED_CLEAN_ENABLED: False + IRONIC_DEFAULT_BOOT_OPTION: netboot devstack_services: mysql: False postgresql: True @@ -248,12 +250,22 @@ IRONIC_BOOT_MODE: uefi IRONIC_VM_SPECS_RAM: 3096 IRONIC_AUTOMATED_CLEAN_ENABLED: False + IRONIC_DEFAULT_BOOT_OPTION: netboot - job: name: ironic-tempest-ipa-partition-pxe_ipmitool description: ironic-tempest-ipa-partition-pxe_ipmitool parent: ironic-base timeout: 5400 + vars: + devstack_localrc: + # This test runs cleaning by default, and with a larger + # IPA image means that it takes longer to boot for deploy + # and cleaning. As such, CI performance variations can + # cause this job to fail easily due to the extra steps + # and boot cycle of the cleaning operation. + IRONIC_TEMPEST_BUILD_TIMEOUT: 850 + IRONIC_DEFAULT_BOOT_OPTION: netboot - job: name: ironic-tempest-bfv @@ -297,6 +309,7 @@ IRONIC_AUTOMATED_CLEAN_ENABLED: False SWIFT_ENABLE_TEMPURLS: True SWIFT_TEMPURL_KEY: secretkey + IRONIC_DEFAULT_BOOT_OPTION: netboot devstack_plugins: ironic-inspector: https://opendev.org/openstack/ironic-inspector devstack_services: @@ -329,6 +342,7 @@ IRONIC_AUTOMATED_CLEAN_ENABLED: False IRONIC_DEFAULT_RESCUE_INTERFACE: no-rescue IRONIC_ENABLED_RESCUE_INTERFACES: "fake,no-rescue" + IRONIC_DEFAULT_BOOT_OPTION: netboot - job: name: ironic-tempest-functional-python3 @@ -561,6 +575,7 @@ IRONIC_IPXE_ENABLED: False IRONIC_BOOT_MODE: uefi IRONIC_AUTOMATED_CLEAN_ENABLED: False + IRONIC_DEFAULT_BOOT_OPTION: netboot - job: # Security testing for known issues |