diff options
37 files changed, 631 insertions, 130 deletions
diff --git a/devstack/lib/ironic b/devstack/lib/ironic index f78acc6dc..ca9aeab0e 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -1425,6 +1425,17 @@ function configure_ironic_conductor { configure_rootwrap ironic + # additional rootwrap config from ironic-lib + local ironic_lib_prefix + if use_library_from_git "ironic-lib"; then + ironic_lib_prefix=${GITDIR["ironic-lib"]} + else + # pip uses default python 'data' path + ironic_lib_prefix=$(python -c "import sysconfig; \ + print(sysconfig.get_path('data'))") + fi + sudo install -o root -g root -m 644 $ironic_lib_prefix/etc/ironic/rootwrap.d/*.filters /etc/ironic/rootwrap.d + # set up drivers / hardware types iniset $IRONIC_CONF_FILE DEFAULT enabled_hardware_types $IRONIC_ENABLED_HARDWARE_TYPES diff --git a/doc/source/install/creating-images.rst b/doc/source/install/creating-images.rst index f6155142f..10a8c16df 100644 --- a/doc/source/install/creating-images.rst +++ b/doc/source/install/creating-images.rst @@ -26,6 +26,9 @@ the end user. There are two types of user images: Building user images ^^^^^^^^^^^^^^^^^^^^ +disk-image-builder +------------------ + The `disk-image-builder`_ can be used to create user images required for deployment and the actual OS which the user is going to run. @@ -63,3 +66,34 @@ If you want to use Fedora image, replace ``ubuntu`` with ``fedora`` in the chosen command. .. _disk-image-builder: https://docs.openstack.org/diskimage-builder/latest/ + +Virtual machine +--------------- + +Virtual machine software can also be used to build user images. There are +different software options available, qemu-kvm is usually a good choice on +linux platform, it supports emulating many devices and even building images +for architectures other than the host machine by software emulation. +VirtualBox is another good choice for non-linux host. + +The procedure varies depending on the software used, but the steps for +building an image are similar, the user creates a virtual machine, and +installs the target system just like what is done for a real hardware. The +system can be highly customized like partition layout, drivers or software +shipped, etc. + +Usually libvirt and its management tools are used to make interaction with +qemu-kvm easier, for example, to create a virtual machine with +``virt-install``:: + + $ virt-install --name centos8 --ram 4096 --vcpus=2 -f centos8.qcow2 \ + > --cdrom CentOS-8-x86_64-1905-dvd1.iso + +Graphic frontend like ``virt-manager`` can also be utilized. + +The disk file can be used as user image after the system is set up and powered +off. The path of the disk file varies depending on the software used, usually +it's stored in a user-selected part of the local file system. For qemu-kvm or +GUI frontend building upon it, it's typically stored at +``/var/lib/libvirt/images``. + diff --git a/etc/ironic/rootwrap.d/ironic-lib.filters b/etc/ironic/rootwrap.d/ironic-lib.filters deleted file mode 100644 index 342ab69eb..000000000 --- a/etc/ironic/rootwrap.d/ironic-lib.filters +++ /dev/null @@ -1,28 +0,0 @@ -# An ironic-lib.filters to be used with rootwrap command. -# The following commands should be used in filters for disk manipulation. -# This file should be owned by (and only-writable by) the root user. - -# NOTE: this file is a copy of ironic-lib.filters from the ironic-lib -# repository that should ultimately be remove. At this point, we still -# need it to avoid gate breakage and preserve compatibily with existing -# installation. - -[Filters] -# ironic_lib/disk_utils.py -blkid: CommandFilter, blkid, root -blockdev: CommandFilter, blockdev, root -hexdump: CommandFilter, hexdump, root -lsblk: CommandFilter, lsblk, root -qemu-img: CommandFilter, qemu-img, root -wipefs: CommandFilter, wipefs, root -sgdisk: CommandFilter, sgdisk, root -partprobe: CommandFilter, partprobe, root - -# ironic_lib/utils.py -mkswap: CommandFilter, mkswap, root -mkfs: CommandFilter, mkfs, root -dd: CommandFilter, dd, root - -# ironic_lib/disk_partitioner.py -fuser: CommandFilter, fuser, root -parted: CommandFilter, parted, root diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 582ed0a6d..905795bc8 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -2111,8 +2111,24 @@ class NodesController(rest.RestController): self._validate_patch(patch, reset_interfaces) context = api.request.context - rpc_node = api_utils.check_node_policy_and_retrieve( - 'baremetal:node:update', node_ident, with_suffix=True) + + # deal with attribute-specific policy rules + policy_checks = [] + generic_update = False + for p in patch: + if p['path'].startswith('/instance_info'): + policy_checks.append('baremetal:node:update_instance_info') + elif p['path'].startswith('/extra'): + policy_checks.append('baremetal:node:update_extra') + else: + generic_update = True + + # always do at least one check + if generic_update or policy_checks == []: + policy_checks.append('baremetal:node:update') + + rpc_node = api_utils.check_multiple_node_policies_and_retrieve( + policy_checks, node_ident, with_suffix=True) remove_inst_uuid_patch = [{'op': 'remove', 'path': '/instance_uuid'}] if rpc_node.maintenance and patch == remove_inst_uuid_patch: diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 3c012ff42..5c3349b0f 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -1235,6 +1235,30 @@ def check_allocation_policy_and_retrieve(policy_name, allocation_ident): return rpc_allocation +def check_multiple_node_policies_and_retrieve(policy_names, + node_ident, + with_suffix=False): + """Check if the specified policies authorize this request on a node. + + :param: policy_names: List of policy names to check. + :param: node_ident: the UUID or logical name of a node. + :param: with_suffix: whether the RPC node should include the suffix + + :raises: HTTPForbidden if the policy forbids access. + :raises: NodeNotFound if the node is not found. + :return: RPC node identified by node_ident + """ + rpc_node = None + for policy_name in policy_names: + if rpc_node is None: + rpc_node = check_node_policy_and_retrieve(policy_names[0], + node_ident, + with_suffix) + else: + check_owner_policy('node', policy_name, rpc_node['owner']) + return rpc_node + + def check_list_policy(object_type, owner=None): """Check if the list policy authorizes this request on an object. diff --git a/ironic/common/images.py b/ironic/common/images.py index b80a47a34..4273d25e2 100644 --- a/ironic/common/images.py +++ b/ironic/common/images.py @@ -222,12 +222,12 @@ def create_isolinux_image_for_bios(output_file, kernel, ramdisk, raise exception.ImageCreationFailed(image_type='iso', error=e) -def create_isolinux_image_for_uefi(output_file, kernel, ramdisk, - deploy_iso=None, esp_image=None, - kernel_params=None): - """Creates an isolinux image on the specified file. +def create_esp_image_for_uefi(output_file, kernel, ramdisk, + deploy_iso=None, esp_image=None, + kernel_params=None): + """Creates an ESP image on the specified file. - Copies the provided kernel, ramdisk and EFI system partition image to + Copies the provided kernel, ramdisk and EFI system partition image (ESP) to a directory, generates the grub configuration file using kernel parameters and then generates a bootable ISO image for UEFI. @@ -317,6 +317,10 @@ def create_isolinux_image_for_uefi(output_file, kernel, ramdisk, raise exception.ImageCreationFailed(image_type='iso', error=e) +# NOTE(etingof): backward compatibility +create_isolinux_image_for_uefi = create_esp_image_for_uefi + + def fetch(context, image_href, path, force_raw=False): # TODO(vish): Improve context handling and add owner and auth data # when it is added to glance. Right now there is no @@ -489,12 +493,12 @@ def create_boot_iso(context, output_filename, kernel_href, elif CONF.esp_image: esp_image_path = CONF.esp_image - create_isolinux_image_for_uefi(output_filename, - kernel_path, - ramdisk_path, - deploy_iso=deploy_iso_path, - esp_image=esp_image_path, - kernel_params=params) + create_esp_image_for_uefi(output_filename, + kernel_path, + ramdisk_path, + deploy_iso=deploy_iso_path, + esp_image=esp_image_path, + kernel_params=params) else: create_isolinux_image_for_bios(output_filename, kernel_path, diff --git a/ironic/common/policy.py b/ironic/common/policy.py index 16243f0f1..828fbef73 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -105,6 +105,16 @@ node_policies = [ 'Update Node records', [{'path': '/nodes/{node_ident}', 'method': 'PATCH'}]), policy.DocumentedRuleDefault( + 'baremetal:node:update_extra', + 'rule:baremetal:node:update', + 'Update Node extra field', + [{'path': '/nodes/{node_ident}', 'method': 'PATCH'}]), + policy.DocumentedRuleDefault( + 'baremetal:node:update_instance_info', + 'rule:baremetal:node:update', + 'Update Node instance_info field', + [{'path': '/nodes/{node_ident}', 'method': 'PATCH'}]), + policy.DocumentedRuleDefault( 'baremetal:node:update_owner_provisioned', 'rule:is_admin', 'Update Node owner even when Node is provisioned', diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 6d0f8381f..aca403313 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -2959,7 +2959,8 @@ class ConductorManager(base_manager.BaseConductorManager): # either tokens are required and they are present, # or a token is present in general and needs to be # validated. - if token_required or utils.is_agent_token_present(task.node): + if (token_required + or (utils.is_agent_token_present(task.node) and agent_token)): if not utils.is_agent_token_valid(task.node, agent_token): LOG.error('Invalid agent_token receieved for node ' '%(node)s', {'node': node_id}) diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 7f9cea147..2d97d655c 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -1010,13 +1010,22 @@ def get_node_next_deploy_steps(task, skip_current_step=True): skip_current_step=skip_current_step) -def add_secret_token(node): - """Adds a secret token to driver_internal_info for IPA verification.""" +def add_secret_token(node, pregenerated=False): + """Adds a secret token to driver_internal_info for IPA verification. + + :param node: Node object + :param pregenerated: Boolean value, default False, which indicates if + the token should be marked as "pregenerated" in + order to facilitate virtual media booting where + the token is embedded into the configuration. + """ characters = string.ascii_letters + string.digits token = ''.join( random.SystemRandom().choice(characters) for i in range(128)) i_info = node.driver_internal_info i_info['agent_secret_token'] = token + if pregenerated: + i_info['agent_secret_token_pregenerated'] = True node.driver_internal_info = i_info diff --git a/ironic/conf/conductor.py b/ironic/conf/conductor.py index 3ed0fc4f4..8b37b5467 100644 --- a/ironic/conf/conductor.py +++ b/ironic/conf/conductor.py @@ -209,7 +209,7 @@ opts = [ help=_('Timeout (in seconds) of soft reboot and soft power ' 'off operation. This value always has to be positive.')), cfg.IntOpt('power_state_change_timeout', - min=2, default=30, + min=2, default=60, help=_('Number of seconds to wait for power operations to ' 'complete, i.e., so that a baremetal node is in the ' 'desired power state. If timed out, the power operation ' diff --git a/ironic/conf/ipmi.py b/ironic/conf/ipmi.py index ec4d76e99..9545bde17 100644 --- a/ironic/conf/ipmi.py +++ b/ironic/conf/ipmi.py @@ -53,6 +53,12 @@ opts = [ default=[], help=_('Additional errors ipmitool may encounter, ' 'specific to the environment it is run in.')), + cfg.BoolOpt('debug', + default=False, + help=_('Enables all ipmi commands to be executed with an ' + 'additional debugging output. This is a separate ' + 'option as ipmitool can log a substantial amount ' + 'of misleading text when in this mode.')), ] diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 00c8a9021..44d4abab1 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -208,7 +208,6 @@ class AgentDeployMixin(agent_base.AgentDeployMixin): image_info = { 'id': image_source.split('/')[-1], 'urls': [node.instance_info['image_url']], - 'checksum': node.instance_info['image_checksum'], # NOTE(comstud): Older versions of ironic do not set # 'disk_format' nor 'container_format', so we use .get() # to maintain backwards compatibility in case code was @@ -219,6 +218,9 @@ class AgentDeployMixin(agent_base.AgentDeployMixin): 'stream_raw_images': CONF.agent.stream_raw_images, } + if node.instance_info.get('image_checksum'): + image_info['checksum'] = node.instance_info['image_checksum'] + if (node.instance_info.get('image_os_hash_algo') and node.instance_info.get('image_os_hash_value')): image_info['os_hash_algo'] = node.instance_info[ @@ -414,6 +416,10 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface): params = {} image_source = node.instance_info.get('image_source') + image_checksum = node.instance_info.get('image_checksum') + os_hash_algo = node.instance_info.get('image_os_hash_algo') + os_hash_value = node.instance_info.get('image_os_hash_value') + params['instance_info.image_source'] = image_source error_msg = _('Node %s failed to validate deploy image info. Some ' 'parameters were missing') % node.uuid @@ -421,10 +427,25 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface): deploy_utils.check_for_missing_params(params, error_msg) if not service_utils.is_glance_image(image_source): - if not node.instance_info.get('image_checksum'): + + def _raise_missing_checksum_exception(node): raise exception.MissingParameterValue(_( - "image_source's image_checksum must be provided in " - "instance_info for node %s") % node.uuid) + 'image_source\'s "image_checksum", or ' + '"image_os_hash_algo" and "image_os_hash_value" ' + 'must be provided in instance_info for ' + 'node %s') % node.uuid) + + if os_hash_value and not os_hash_algo: + # We are missing a piece of information, + # so we still need to raise an error. + _raise_missing_checksum_exception(node) + elif not os_hash_value and os_hash_algo: + # We have the hash setting, but not the hash. + _raise_missing_checksum_exception(node) + elif not os_hash_value and not image_checksum: + # We are lacking the original image_checksum, + # so we raise the error. + _raise_missing_checksum_exception(node) validate_http_provisioning_configuration(node) diff --git a/ironic/drivers/modules/agent_client.py b/ironic/drivers/modules/agent_client.py index d9d14c380..32427c6e7 100644 --- a/ironic/drivers/modules/agent_client.py +++ b/ironic/drivers/modules/agent_client.py @@ -82,6 +82,9 @@ class AgentClient(object): request_params = { 'wait': str(wait).lower() } + agent_token = node.driver_internal_info.get('agent_secret_token') + if agent_token: + request_params['agent_token'] = agent_token LOG.debug('Executing agent command %(method)s for node %(node)s', {'node': node.uuid, 'method': method}) diff --git a/ironic/drivers/modules/ansible/playbooks/callback_plugins/ironic_log.py b/ironic/drivers/modules/ansible/playbooks/callback_plugins/ironic_log.py index b8e4c3c69..fd98b9d6c 100644 --- a/ironic/drivers/modules/ansible/playbooks/callback_plugins/ironic_log.py +++ b/ironic/drivers/modules/ansible/playbooks/callback_plugins/ironic_log.py @@ -34,7 +34,7 @@ def parse_callback_config(): 'use_journal': True, 'use_syslog': False} try: - config.readfp(open(basename + ".ini")) + config.read_file(open(basename + ".ini")) if config.has_option('ironic', 'config_file'): callback_config['ironic_config'] = config.get( 'ironic', 'config_file') diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index db4b00638..301f84ed7 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -1245,7 +1245,7 @@ def build_instance_info_for_deploy(task): os_hash_algo = 'sha256' LOG.debug('Recalculating checksum for image %(image)s due to ' 'image conversion.', {'image': image_path}) - instance_info['image_checksum'] = 'md5-not-supported' + instance_info['image_checksum'] = None hash_value = compute_image_checksum(image_path, os_hash_algo) instance_info['image_os_hash_algo'] = os_hash_algo instance_info['image_os_hash_value'] = hash_value diff --git a/ironic/drivers/modules/ilo/boot.py b/ironic/drivers/modules/ilo/boot.py index bd5ab0d04..b6142bb8b 100644 --- a/ironic/drivers/modules/ilo/boot.py +++ b/ironic/drivers/modules/ilo/boot.py @@ -509,6 +509,13 @@ class IloVirtualMediaBoot(base.BootInterface): # during boot. ilo_common.eject_vmedia_devices(task) + # NOTE(TheJulia): Since we're deploying, cleaning, or rescuing, + # with virtual media boot, we should generate a token! + manager_utils.add_secret_token(task.node, pregenerated=True) + ramdisk_params['ipa-agent-token'] = \ + task.node.driver_internal_info['agent_secret_token'] + task.node.save() + 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: diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index 44779f1ca..d3682bfd3 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -446,7 +446,7 @@ def _get_ipmitool_args(driver_info, pw_file=None): args.append('-f') args.append(pw_file) - if CONF.debug: + if CONF.ipmi.debug: args.append('-v') # ensure all arguments are strings diff --git a/ironic/drivers/modules/irmc/boot.py b/ironic/drivers/modules/irmc/boot.py index c7bb45c78..e23ee4961 100644 --- a/ironic/drivers/modules/irmc/boot.py +++ b/ironic/drivers/modules/irmc/boot.py @@ -990,6 +990,13 @@ class IRMCVirtualMediaBoot(base.BootInterface, IRMCVolumeBootMixIn): {'node': task.node.uuid}) return + # NOTE(TheJulia): Since we're deploying, cleaning, or rescuing, + # with virtual media boot, we should generate a token! + manager_utils.add_secret_token(task.node, pregenerated=True) + ramdisk_params['ipa-agent-token'] = \ + task.node.driver_internal_info['agent_secret_token'] + task.node.save() + deploy_nic_mac = deploy_utils.get_single_nic_with_vif_port_id(task) ramdisk_params['BOOTIF'] = deploy_nic_mac diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index f3538b6eb..005f77d37 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -682,6 +682,13 @@ class RedfishVirtualMediaBoot(base.BootInterface): states.INSPECTING): return + # NOTE(TheJulia): Since we're deploying, cleaning, or rescuing, + # with virtual media boot, we should generate a token! + manager_utils.add_secret_token(node, pregenerated=True) + node.save() + ramdisk_params['ipa-agent-token'] = \ + node.driver_internal_info['agent_secret_token'] + manager_utils.node_power_action(task, states.POWER_OFF) d_info = self._parse_driver_info(node) diff --git a/ironic/tests/unit/api/controllers/v1/test_expose.py b/ironic/tests/unit/api/controllers/v1/test_expose.py index 0f2323d78..c2370fc61 100644 --- a/ironic/tests/unit/api/controllers/v1/test_expose.py +++ b/ironic/tests/unit/api/controllers/v1/test_expose.py @@ -53,6 +53,8 @@ class TestExposedAPIMethodsCheckPolicy(test_base.TestCase): self.assertTrue( ('api_utils.check_node_policy_and_retrieve' in src) or ('api_utils.check_list_policy' in src) or + ('api_utils.check_multiple_node_policies_and_retrieve' in + src) or ('self._get_node_and_topic' in src) or ('api_utils.check_port_policy_and_retrieve' in src) or ('api_utils.check_port_list_policy' in src) or diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index db571ffa5..429fab1b8 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -3400,6 +3400,164 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + def test_patch_policy_update(self, mock_cmnpar): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + mock_cmnpar.return_value = node + self.mock_update_node.return_value = node + headers = {api_base.Version.string: str(api_v1.max_version())} + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/description', + 'value': 'foo', + 'op': 'replace'}], + headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + mock_cmnpar.assert_called_once_with( + ['baremetal:node:update'], node.uuid, with_suffix=True) + + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + def test_patch_policy_update_none(self, mock_cmnpar): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + mock_cmnpar.return_value = node + self.mock_update_node.return_value = node + headers = {api_base.Version.string: str(api_v1.max_version())} + response = self.patch_json('/nodes/%s' % node.uuid, + [], + headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + mock_cmnpar.assert_called_once_with( + ['baremetal:node:update'], node.uuid, with_suffix=True) + + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + def test_patch_policy_update_extra(self, mock_cmnpar): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + mock_cmnpar.return_value = node + self.mock_update_node.return_value = node + headers = {api_base.Version.string: str(api_v1.max_version())} + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/extra/foo', + 'value': 'bar', + 'op': 'add'}], + headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + mock_cmnpar.assert_called_once_with( + ['baremetal:node:update_extra'], node.uuid, with_suffix=True) + + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + def test_patch_policy_update_instance_info(self, mock_cmnpar): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + mock_cmnpar.return_value = node + self.mock_update_node.return_value = node + headers = {api_base.Version.string: str(api_v1.max_version())} + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/instance_info/foo', + 'value': 'bar', + 'op': 'add'}], + headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + mock_cmnpar.assert_called_once_with( + ['baremetal:node:update_instance_info'], + node.uuid, with_suffix=True) + + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + def test_patch_policy_update_generic_and_extra(self, mock_cmnpar): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + mock_cmnpar.return_value = node + self.mock_update_node.return_value = node + headers = {api_base.Version.string: str(api_v1.max_version())} + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/description', + 'value': 'foo', + 'op': 'replace'}, + {'path': '/extra/foo', + 'value': 'bar', + 'op': 'add'}], + headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + mock_cmnpar.assert_called_once_with( + ['baremetal:node:update_extra', 'baremetal:node:update'], + node.uuid, with_suffix=True) + + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + def test_patch_policy_update_generic_and_instance_info(self, mock_cmnpar): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + mock_cmnpar.return_value = node + self.mock_update_node.return_value = node + headers = {api_base.Version.string: str(api_v1.max_version())} + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/description', + 'value': 'foo', + 'op': 'replace'}, + {'path': '/instance_info/foo', + 'value': 'bar', + 'op': 'add'}], + headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + mock_cmnpar.assert_called_once_with( + ['baremetal:node:update_instance_info', 'baremetal:node:update'], + node.uuid, with_suffix=True) + + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + def test_patch_policy_update_extra_and_instance_info(self, mock_cmnpar): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + mock_cmnpar.return_value = node + self.mock_update_node.return_value = node + headers = {api_base.Version.string: str(api_v1.max_version())} + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/extra/foo', + 'value': 'bar', + 'op': 'add'}, + {'path': '/instance_info/foo', + 'value': 'bar', + 'op': 'add'}], + headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + mock_cmnpar.assert_called_once_with( + ['baremetal:node:update_extra', + 'baremetal:node:update_instance_info'], + node.uuid, with_suffix=True) + + @mock.patch.object(api_utils, 'check_multiple_node_policies_and_retrieve') + def test_patch_policy_update_generic_extra_instance_info( + self, mock_cmnpar): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + mock_cmnpar.return_value = node + self.mock_update_node.return_value = node + headers = {api_base.Version.string: str(api_v1.max_version())} + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/description', + 'value': 'foo', + 'op': 'replace'}, + {'path': '/extra/foo', + 'value': 'bar', + 'op': 'add'}, + {'path': '/instance_info/foo', + 'value': 'bar', + 'op': 'add'}], + headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + mock_cmnpar.assert_called_once_with( + ['baremetal:node:update_extra', + 'baremetal:node:update_instance_info', + 'baremetal:node:update'], + node.uuid, with_suffix=True) + def _create_node_locally(node): driver_factory.check_and_update_node_interfaces(node) diff --git a/ironic/tests/unit/api/controllers/v1/test_utils.py b/ironic/tests/unit/api/controllers/v1/test_utils.py index ab7c02827..077ddc302 100644 --- a/ironic/tests/unit/api/controllers/v1/test_utils.py +++ b/ironic/tests/unit/api/controllers/v1/test_utils.py @@ -1016,6 +1016,68 @@ class TestCheckAllocationPolicyAndRetrieve(base.TestCase): ) +class TestCheckMultipleNodePoliciesAndRetrieve(base.TestCase): + def setUp(self): + super(TestCheckMultipleNodePoliciesAndRetrieve, self).setUp() + self.valid_node_uuid = uuidutils.generate_uuid() + self.node = test_api_utils.post_get_test_node() + self.node['owner'] = '12345' + + @mock.patch.object(utils, 'check_node_policy_and_retrieve') + @mock.patch.object(utils, 'check_owner_policy') + def test_check_multiple_node_policies_and_retrieve( + self, mock_cop, mock_cnpar + ): + mock_cnpar.return_value = self.node + mock_cop.return_value = True + + rpc_node = utils.check_multiple_node_policies_and_retrieve( + ['fake_policy_1', 'fake_policy_2'], self.valid_node_uuid + ) + mock_cnpar.assert_called_once_with('fake_policy_1', + self.valid_node_uuid, False) + mock_cop.assert_called_once_with( + 'node', 'fake_policy_2', '12345') + self.assertEqual(self.node, rpc_node) + + @mock.patch.object(utils, 'check_node_policy_and_retrieve') + @mock.patch.object(utils, 'check_owner_policy') + def test_check_multiple_node_policies_and_retrieve_first_fail( + self, mock_cop, mock_cnpar + ): + mock_cnpar.side_effect = exception.HTTPForbidden(resource='fake') + mock_cop.return_value = True + + self.assertRaises( + exception.HTTPForbidden, + utils.check_multiple_node_policies_and_retrieve, + ['fake_policy_1', 'fake_policy_2'], + self.valid_node_uuid + ) + mock_cnpar.assert_called_once_with('fake_policy_1', + self.valid_node_uuid, False) + mock_cop.assert_not_called() + + @mock.patch.object(utils, 'check_node_policy_and_retrieve') + @mock.patch.object(utils, 'check_owner_policy') + def test_check_node_policy_and_retrieve_no_node( + self, mock_cop, mock_cnpar + ): + mock_cnpar.return_value = self.node + mock_cop.side_effect = exception.HTTPForbidden(resource='fake') + + self.assertRaises( + exception.HTTPForbidden, + utils.check_multiple_node_policies_and_retrieve, + ['fake_policy_1', 'fake_policy_2'], + self.valid_node_uuid + ) + mock_cnpar.assert_called_once_with('fake_policy_1', + self.valid_node_uuid, False) + mock_cop.assert_called_once_with( + 'node', 'fake_policy_2', '12345') + + class TestCheckListPolicy(base.TestCase): @mock.patch.object(api, 'request', spec_set=["context", "version"]) @mock.patch.object(policy, 'authorize', spec=True) diff --git a/ironic/tests/unit/common/test_images.py b/ironic/tests/unit/common/test_images.py index 78514d1ea..ddfd5b150 100644 --- a/ironic/tests/unit/common/test_images.py +++ b/ironic/tests/unit/common/test_images.py @@ -485,7 +485,7 @@ class FsImageTestCase(base.TestCase): @mock.patch.object(images, '_mount_deploy_iso', autospec=True) @mock.patch.object(utils, 'tempdir', autospec=True) @mock.patch.object(images, '_generate_cfg', autospec=True) - def test_create_isolinux_image_for_uefi_with_deploy_iso( + def test_create_esp_image_for_uefi_with_deploy_iso( self, gen_cfg_mock, tempdir_mock, mount_mock, execute_mock, write_to_file_mock, create_root_fs_mock, umount_mock): @@ -517,11 +517,11 @@ class FsImageTestCase(base.TestCase): mount_mock.return_value = (uefi_path_info, e_img_rel_path, grub_rel_path) - images.create_isolinux_image_for_uefi('tgt_file', - 'path/to/kernel', - 'path/to/ramdisk', - deploy_iso='path/to/deploy_iso', - kernel_params=params) + images.create_esp_image_for_uefi('tgt_file', + 'path/to/kernel', + 'path/to/ramdisk', + deploy_iso='path/to/deploy_iso', + kernel_params=params) mount_mock.assert_called_once_with('path/to/deploy_iso', 'mountdir') create_root_fs_mock.assert_called_once_with('tmpdir', files_info) gen_cfg_mock.assert_any_call(params, CONF.grub_config_template, @@ -537,7 +537,7 @@ class FsImageTestCase(base.TestCase): @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'tempdir', autospec=True) @mock.patch.object(images, '_generate_cfg', autospec=True) - def test_create_isolinux_image_for_uefi_with_esp_image( + def test_create_esp_image_for_uefi_with_esp_image( self, gen_cfg_mock, tempdir_mock, execute_mock, create_root_fs_mock, write_to_file_mock): @@ -564,7 +564,7 @@ class FsImageTestCase(base.TestCase): tempdir_mock.side_effect = mock_file_handle, mock_file_handle1 mountdir_grub_cfg_path = 'tmpdir' + grub_cfg_file - images.create_isolinux_image_for_uefi( + images.create_esp_image_for_uefi( 'tgt_file', 'path/to/kernel', 'path/to/ramdisk', esp_image='sourceabspath/to/efiboot.img', kernel_params=params) @@ -643,11 +643,9 @@ class FsImageTestCase(base.TestCase): @mock.patch.object(utils, 'tempdir', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(os, 'walk', autospec=True) - def test_create_isolinux_image_uefi_rootfs_fails(self, walk_mock, - utils_mock, - tempdir_mock, - create_root_fs_mock, - umount_mock): + def test_create_esp_image_uefi_rootfs_fails( + self, walk_mock, utils_mock, tempdir_mock, + create_root_fs_mock, umount_mock): mock_file_handle = mock.MagicMock(spec=io.BytesIO) mock_file_handle.__enter__.return_value = 'tmpdir' @@ -657,7 +655,7 @@ class FsImageTestCase(base.TestCase): create_root_fs_mock.side_effect = IOError self.assertRaises(exception.ImageCreationFailed, - images.create_isolinux_image_for_uefi, + images.create_esp_image_for_uefi, 'tgt_file', 'path/to/kernel', 'path/to/ramdisk', @@ -686,14 +684,9 @@ class FsImageTestCase(base.TestCase): @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(images, '_mount_deploy_iso', autospec=True) @mock.patch.object(images, '_generate_cfg', autospec=True) - def test_create_isolinux_image_mkisofs_fails(self, - gen_cfg_mock, - mount_mock, - utils_mock, - tempdir_mock, - write_to_file_mock, - create_root_fs_mock, - umount_mock): + def test_create_esp_image_mkisofs_fails( + self, gen_cfg_mock, mount_mock, utils_mock, tempdir_mock, + write_to_file_mock, create_root_fs_mock, umount_mock): mock_file_handle = mock.MagicMock(spec=io.BytesIO) mock_file_handle.__enter__.return_value = 'tmpdir' mock_file_handle1 = mock.MagicMock(spec=io.BytesIO) @@ -703,7 +696,7 @@ class FsImageTestCase(base.TestCase): utils_mock.side_effect = processutils.ProcessExecutionError self.assertRaises(exception.ImageCreationFailed, - images.create_isolinux_image_for_uefi, + images.create_esp_image_for_uefi, 'tgt_file', 'path/to/kernel', 'path/to/ramdisk', @@ -731,7 +724,7 @@ class FsImageTestCase(base.TestCase): 'tgt_file', 'path/to/kernel', 'path/to/ramdisk') - @mock.patch.object(images, 'create_isolinux_image_for_uefi', autospec=True) + @mock.patch.object(images, 'create_esp_image_for_uefi', autospec=True) @mock.patch.object(images, 'fetch', autospec=True) @mock.patch.object(utils, 'tempdir', autospec=True) def test_create_boot_iso_for_uefi_deploy_iso( @@ -759,7 +752,7 @@ class FsImageTestCase(base.TestCase): deploy_iso='tmpdir/deploy_iso-uuid', esp_image=None, kernel_params=params) - @mock.patch.object(images, 'create_isolinux_image_for_uefi', autospec=True) + @mock.patch.object(images, 'create_esp_image_for_uefi', autospec=True) @mock.patch.object(images, 'fetch', autospec=True) @mock.patch.object(utils, 'tempdir', autospec=True) def test_create_boot_iso_for_uefi_esp_image( @@ -787,7 +780,7 @@ class FsImageTestCase(base.TestCase): deploy_iso=None, esp_image='tmpdir/efiboot-uuid', kernel_params=params) - @mock.patch.object(images, 'create_isolinux_image_for_uefi', autospec=True) + @mock.patch.object(images, 'create_esp_image_for_uefi', autospec=True) @mock.patch.object(images, 'fetch', autospec=True) @mock.patch.object(utils, 'tempdir', autospec=True) def test_create_boot_iso_for_uefi_deploy_iso_for_hrefs( @@ -815,7 +808,7 @@ class FsImageTestCase(base.TestCase): deploy_iso='tmpdir/deploy_iso-href', esp_image=None, kernel_params=params) - @mock.patch.object(images, 'create_isolinux_image_for_uefi', autospec=True) + @mock.patch.object(images, 'create_esp_image_for_uefi', autospec=True) @mock.patch.object(images, 'fetch', autospec=True) @mock.patch.object(utils, 'tempdir', autospec=True) def test_create_boot_iso_for_uefi_esp_image_for_hrefs( diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 49d0585e6..285317f31 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -6890,6 +6890,30 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): autospec=True) @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', autospec=True) + def test_heartbeat_with_agent_pregenerated_token( + self, mock_spawn, mock_heartbeat): + """Test heartbeating.""" + node = obj_utils.create_test_node( + self.context, driver='fake-hardware', + provision_state=states.DEPLOYING, + target_provision_state=states.ACTIVE, + driver_internal_info={'agent_secret_token': 'a secret'}) + + self._start_service() + + mock_spawn.reset_mock() + + mock_spawn.side_effect = self._fake_spawn + self.service.heartbeat( + self.context, node.uuid, 'http://callback', '6.0.1', + agent_token=None) + mock_heartbeat.assert_called_with(mock.ANY, mock.ANY, + 'http://callback', '6.0.1') + + @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', + autospec=True) + @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', + autospec=True) def test_heartbeat_with_no_required_agent_token(self, mock_spawn, mock_heartbeat): """Tests that we kill the heartbeat attempt very early on.""" @@ -7017,32 +7041,6 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): autospec=True) @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', autospec=True) - def test_heartbeat_invalid_when_token_on_file_older_agent( - self, mock_spawn, mock_heartbeat): - """Heartbeat rejected if a token is on file.""" - self.config(require_agent_token=False) - node = obj_utils.create_test_node( - self.context, driver='fake-hardware', - provision_state=states.DEPLOYING, - target_provision_state=states.ACTIVE, - driver_internal_info={'agent_secret_token': 'a secret'}) - - self._start_service() - - mock_spawn.reset_mock() - - mock_spawn.side_effect = self._fake_spawn - - self.assertRaises(exception.InvalidParameterValue, - self.service.heartbeat, self.context, - node.uuid, 'http://callback', - agent_token=None, agent_version='4.0.0') - self.assertFalse(mock_heartbeat.called) - - @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', - autospec=True) - @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', - autospec=True) def test_heartbeat_invalid_newer_version( self, mock_spawn, mock_heartbeat): """Heartbeat rejected if client should be sending a token.""" diff --git a/ironic/tests/unit/drivers/modules/ilo/test_boot.py b/ironic/tests/unit/drivers/modules/ilo/test_boot.py index 46d5195ad..1e133a540 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_boot.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_boot.py @@ -821,7 +821,8 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): prepare_node_for_deploy_mock.assert_called_once_with(task) eject_mock.assert_called_once_with(task) - expected_ramdisk_opts = {'a': 'b', 'BOOTIF': '12:34:56:78:90:ab'} + expected_ramdisk_opts = {'a': 'b', 'BOOTIF': '12:34:56:78:90:ab', + 'ipa-agent-token': mock.ANY} get_nic_mock.assert_called_once_with(task) setup_vmedia_mock.assert_called_once_with(task, iso, expected_ramdisk_opts) diff --git a/ironic/tests/unit/drivers/modules/irmc/test_boot.py b/ironic/tests/unit/drivers/modules/irmc/test_boot.py index 4de54c2e3..6831759a7 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_boot.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_boot.py @@ -991,7 +991,9 @@ class IRMCVirtualMediaBootTestCase(test_common.BaseIRMCTest): shared=False) as task: task.driver.boot.prepare_ramdisk(task, ramdisk_params) - expected_ramdisk_opts = {'a': 'b', 'BOOTIF': '12:34:56:78:90:ab'} + expected_ramdisk_opts = {'a': 'b', 'BOOTIF': '12:34:56:78:90:ab', + 'ipa-agent-token': mock.ANY} + get_single_nic_with_vif_port_id_mock.assert_called_once_with( task) _setup_vmedia_mock.assert_called_once_with( diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py index 4a759b6cf..aa072fa0f 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -552,6 +552,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): self.assertRaises(exception.UnsupportedDriverExtension, task.driver.boot.validate_inspection, task) + @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(redfish_boot.RedfishVirtualMediaBoot, @@ -560,15 +562,16 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): '_insert_vmedia', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_parse_driver_info', autospec=True) - @mock.patch.object(redfish_boot, 'manager_utils', autospec=True) + @mock.patch.object(redfish_boot.manager_utils, 'node_power_action', + autospec=True) @mock.patch.object(redfish_boot, 'boot_mode_utils', autospec=True) def test_prepare_ramdisk_with_params( - self, mock_boot_mode_utils, mock_manager_utils, + self, mock_boot_mode_utils, mock_node_power_action, mock__parse_driver_info, mock__insert_vmedia, mock__eject_vmedia, - mock__prepare_deploy_iso): + mock__prepare_deploy_iso, mock_node_set_boot_device): with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: + shared=False) as task: task.node.provision_state = states.DEPLOYING mock__parse_driver_info.return_value = {} @@ -576,7 +579,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): task.driver.boot.prepare_ramdisk(task, {}) - mock_manager_utils.node_power_action.assert_called_once_with( + mock_node_power_action.assert_called_once_with( task, states.POWER_OFF) mock__eject_vmedia.assert_called_once_with( @@ -587,17 +590,20 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): expected_params = { 'BOOTIF': None, + 'ipa-agent-token': mock.ANY, 'ipa-debug': '1', } mock__prepare_deploy_iso.assert_called_once_with( task, expected_params, 'deploy') - mock_manager_utils.node_set_boot_device.assert_called_once_with( + mock_node_set_boot_device.assert_called_once_with( task, boot_devices.CDROM, False) mock_boot_mode_utils.sync_boot_mode.assert_called_once_with(task) + @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(redfish_boot.RedfishVirtualMediaBoot, @@ -606,12 +612,13 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): '_insert_vmedia', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_parse_driver_info', autospec=True) - @mock.patch.object(redfish_boot, 'manager_utils', autospec=True) + @mock.patch.object(redfish_boot.manager_utils, 'node_power_action', + autospec=True) @mock.patch.object(redfish_boot, 'boot_mode_utils', autospec=True) def test_prepare_ramdisk_no_debug( - self, mock_boot_mode_utils, mock_manager_utils, + self, mock_boot_mode_utils, mock_node_power_action, mock__parse_driver_info, mock__insert_vmedia, mock__eject_vmedia, - mock__prepare_deploy_iso): + 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: @@ -622,7 +629,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): task.driver.boot.prepare_ramdisk(task, {}) - mock_manager_utils.node_power_action.assert_called_once_with( + mock_node_power_action.assert_called_once_with( task, states.POWER_OFF) mock__eject_vmedia.assert_called_once_with( @@ -633,16 +640,19 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): expected_params = { 'BOOTIF': None, + 'ipa-agent-token': mock.ANY, } mock__prepare_deploy_iso.assert_called_once_with( task, expected_params, 'deploy') - mock_manager_utils.node_set_boot_device.assert_called_once_with( + mock_node_set_boot_device.assert_called_once_with( task, boot_devices.CDROM, False) mock_boot_mode_utils.sync_boot_mode.assert_called_once_with(task) + @mock.patch.object(redfish_boot.manager_utils, 'node_set_boot_device', + autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_prepare_floppy_image', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, @@ -655,16 +665,17 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): '_insert_vmedia', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_parse_driver_info', autospec=True) - @mock.patch.object(redfish_boot, 'manager_utils', autospec=True) + @mock.patch.object(redfish_boot.manager_utils, 'node_power_action', + autospec=True) @mock.patch.object(redfish_boot, 'boot_mode_utils', autospec=True) def test_prepare_ramdisk_with_floppy( - self, mock_boot_mode_utils, mock_manager_utils, + 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__prepare_floppy_image): + mock__prepare_floppy_image, mock_node_set_boot_device): with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: + shared=False) as task: task.node.provision_state = states.DEPLOYING mock__parse_driver_info.return_value = { @@ -677,7 +688,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): task.driver.boot.prepare_ramdisk(task, {}) - mock_manager_utils.node_power_action.assert_called_once_with( + mock_node_power_action.assert_called_once_with( task, states.POWER_OFF) mock__has_vmedia_device.assert_called_once_with( @@ -703,12 +714,13 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): 'BOOTIF': None, 'boot_method': 'vmedia', 'ipa-debug': '1', + 'ipa-agent-token': mock.ANY, } mock__prepare_deploy_iso.assert_called_once_with( task, expected_params, 'deploy') - mock_manager_utils.node_set_boot_device.assert_called_once_with( + mock_node_set_boot_device.assert_called_once_with( task, boot_devices.CDROM, False) mock_boot_mode_utils.sync_boot_mode.assert_called_once_with(task) diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 7dd21d32f..81790a28a 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -257,6 +257,75 @@ class TestAgentDeploy(db_base.DbTestCase): pxe_boot_validate_mock.assert_called_once_with( task.driver.boot, task) + @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) + def test_validate_nonglance_image_no_checksum_os_algo( + self, pxe_boot_validate_mock): + i_info = self.node.instance_info + i_info['image_source'] = 'http://image-ref' + i_info['image_os_hash_value'] = 'az' + del i_info['image_checksum'] + self.node.instance_info = i_info + self.node.save() + + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.assertRaises(exception.MissingParameterValue, + self.driver.validate, task) + pxe_boot_validate_mock.assert_called_once_with( + task.driver.boot, task) + + @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) + def test_validate_nonglance_image_no_os_image_hash( + self, pxe_boot_validate_mock, autospec=True): + i_info = self.node.instance_info + i_info['image_source'] = 'http://image-ref' + i_info['image_os_hash_algo'] = 'magicalgo' + self.node.instance_info = i_info + self.node.save() + + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.assertRaises(exception.MissingParameterValue, + self.driver.validate, task) + pxe_boot_validate_mock.assert_called_once_with( + task.driver.boot, task) + + @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) + def test_validate_nonglance_image_no_os_algo( + self, pxe_boot_validate_mock): + i_info = self.node.instance_info + i_info['image_source'] = 'http://image-ref' + i_info['image_os_hash_value'] = 'az' + self.node.instance_info = i_info + self.node.save() + + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.assertRaises(exception.MissingParameterValue, + self.driver.validate, task) + pxe_boot_validate_mock.assert_called_once_with( + task.driver.boot, task) + + @mock.patch.object(images, 'image_show', autospec=True) + @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) + def test_validate_nonglance_image_no_os_checksum( + self, pxe_boot_validate_mock, show_mock): + i_info = self.node.instance_info + i_info['image_source'] = 'http://image-ref' + del i_info['image_checksum'] + i_info['image_os_hash_algo'] = 'whacky-algo-1' + i_info['image_os_hash_value'] = '1234567890' + self.node.instance_info = i_info + self.node.save() + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.driver.validate(task) + pxe_boot_validate_mock.assert_called_once_with( + task.driver.boot, task) + show_mock.assert_called_once_with(self.context, + 'http://image-ref') + @mock.patch.object(agent, 'validate_http_provisioning_configuration', autospec=True) @mock.patch.object(images, 'image_show', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/test_agent_client.py b/ironic/tests/unit/drivers/modules/test_agent_client.py index 9d1af1201..a39477f02 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_client.py +++ b/ironic/tests/unit/drivers/modules/test_agent_client.py @@ -346,6 +346,28 @@ class TestAgentClient(base.TestCase): self.node) self.assertFalse(self.client._command.called) + def test__command_agent_client(self): + response_data = {'status': 'ok'} + response_text = json.dumps(response_data) + self.client.session.post.return_value = MockResponse(response_text) + method = 'standby.run_image' + image_info = {'image_id': 'test_image'} + params = {'image_info': image_info} + i_info = self.node.driver_internal_info + i_info['agent_secret_token'] = 'magical' + self.node.driver_internal_info = i_info + url = self.client._get_command_url(self.node) + body = self.client._get_command_body(method, params) + + response = self.client._command(self.node, method, params) + self.assertEqual(response, response_data) + self.client.session.post.assert_called_once_with( + url, + data=body, + params={'wait': 'false', + 'agent_token': 'magical'}, + timeout=60) + class TestAgentClientAttempts(base.TestCase): def setUp(self): diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 96b45936b..3d5ee4e1c 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -2543,7 +2543,7 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): image_path, instance_info = self._test_build_instance_info( image_info=self.image_info, expect_raw=True) - self.assertEqual('md5-not-supported', instance_info['image_checksum']) + self.assertIsNone(instance_info['image_checksum']) self.assertEqual(instance_info['image_disk_format'], 'raw') calls = [mock.call(image_path, algorithm='sha512')] self.checksum_mock.assert_has_calls(calls) @@ -2554,7 +2554,7 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): image_path, instance_info = self._test_build_instance_info( image_info=self.image_info, expect_raw=True) - self.assertEqual('md5-not-supported', instance_info['image_checksum']) + self.assertIsNone(instance_info['image_checksum']) self.assertEqual(instance_info['image_disk_format'], 'raw') calls = [mock.call(image_path, algorithm='sha256')] self.checksum_mock.assert_has_calls(calls) diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index 60b9afe43..6f7b2c513 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py @@ -459,6 +459,7 @@ class Base(db_base.DbTestCase): enabled_console_interfaces=['fake', 'ipmitool-socat', 'ipmitool-shellinabox', 'no-console']) + self.config(debug=True, group="ipmi") self.node = obj_utils.create_test_node( self.context, console_interface='ipmitool-socat', @@ -2622,7 +2623,7 @@ class IPMIToolShellinaboxTestCase(db_base.DbTestCase): ipmi_cmd = self.console._get_ipmi_cmd(driver_info, 'pw_file') expected_ipmi_cmd = ("/:%(uid)s:%(gid)s:HOME:ipmitool " "-I lanplus -H %(address)s -L ADMINISTRATOR " - "-U %(user)s -f pw_file -v" % + "-U %(user)s -f pw_file" % {'uid': os.getuid(), 'gid': os.getgid(), 'address': driver_info['address'], 'user': driver_info['username']}) @@ -2636,7 +2637,7 @@ class IPMIToolShellinaboxTestCase(db_base.DbTestCase): ipmi_cmd = self.console._get_ipmi_cmd(driver_info, 'pw_file') expected_ipmi_cmd = ("/:%(uid)s:%(gid)s:HOME:ipmitool " "-I lanplus -H %(address)s -L ADMINISTRATOR " - "-f pw_file -v" % + "-f pw_file" % {'uid': os.getuid(), 'gid': os.getgid(), 'address': driver_info['address']}) self.assertEqual(expected_ipmi_cmd, ipmi_cmd) @@ -2806,7 +2807,7 @@ class IPMIToolSocatDriverTestCase(IPMIToolShellinaboxTestCase): ipmi_cmd = self.console._get_ipmi_cmd(driver_info, 'pw_file') expected_ipmi_cmd = ("ipmitool -I lanplus -H %(address)s " "-L ADMINISTRATOR -U %(user)s " - "-f pw_file -v" % + "-f pw_file" % {'address': driver_info['address'], 'user': driver_info['username']}) self.assertEqual(expected_ipmi_cmd, ipmi_cmd) @@ -2819,7 +2820,7 @@ class IPMIToolSocatDriverTestCase(IPMIToolShellinaboxTestCase): ipmi_cmd = self.console._get_ipmi_cmd(driver_info, 'pw_file') expected_ipmi_cmd = ("ipmitool -I lanplus -H %(address)s " "-L ADMINISTRATOR " - "-f pw_file -v" % + "-f pw_file" % {'address': driver_info['address']}) self.assertEqual(expected_ipmi_cmd, ipmi_cmd) diff --git a/releasenotes/notes/conductor-power-sync-timeout-extension-fa5e7b5fdd679d84.yaml b/releasenotes/notes/conductor-power-sync-timeout-extension-fa5e7b5fdd679d84.yaml new file mode 100644 index 000000000..7696bfb60 --- /dev/null +++ b/releasenotes/notes/conductor-power-sync-timeout-extension-fa5e7b5fdd679d84.yaml @@ -0,0 +1,9 @@ +--- +other: + - | + The ``[conductor]power_state_change_timeout`` default value has been + extended to ``60`` seconds from ``30`` seconds. This is due to some + API interfaces with Redfish, may cache the power state and thus may + take longer than thirty seconds to update after a change has been + requested. Please see `here <https://github.com/metal3-io/ironic-image/issues/143>`_ + for more information. diff --git a/releasenotes/notes/drop-ironic-lib-rootwrap-filters-f9224173289c1e30.yaml b/releasenotes/notes/drop-ironic-lib-rootwrap-filters-f9224173289c1e30.yaml new file mode 100644 index 000000000..b5d796d09 --- /dev/null +++ b/releasenotes/notes/drop-ironic-lib-rootwrap-filters-f9224173289c1e30.yaml @@ -0,0 +1,6 @@ +--- +other: + - | + The rootwrap filter file called "ironic-lib.filters" is no longer part + of Ironic. The same file is available from the ironic-lib module which is + already an install requirement. diff --git a/releasenotes/notes/image_checksum_optional-381acf9e441d2a58.yaml b/releasenotes/notes/image_checksum_optional-381acf9e441d2a58.yaml new file mode 100644 index 000000000..892534b59 --- /dev/null +++ b/releasenotes/notes/image_checksum_optional-381acf9e441d2a58.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Adds the cability for the ``instance_info\image_checksum`` value + to be optional in stand-alone deployments if the + ``instance_info\image_os_hash_algo`` and + ``instance_info\image_os_hash_value`` fields are populated. diff --git a/releasenotes/notes/ipmi-debug-1c7e090c6cc71903.yaml b/releasenotes/notes/ipmi-debug-1c7e090c6cc71903.yaml new file mode 100644 index 000000000..d0feb47f4 --- /dev/null +++ b/releasenotes/notes/ipmi-debug-1c7e090c6cc71903.yaml @@ -0,0 +1,13 @@ +--- +features: + - | + Adds a new ``[ipmi]debug`` option that allows users to explicitly turn + IPMI command debugging on, as opposed to relying upon the system debug + setting ``[DEFAULT]debug``. Users wishing to continue to log this output + should set ``[ipmi]debug`` to ``True`` in their ironic.conf. +upgrade: + - Debug logging control has been moved to the ``[ipmi]debug`` configuration + setting as opposed to the "conductor" ``[DEFAULT]debug`` setting as + the existing ``ipmitool`` output can be extremely misleading for users. + Operators who wish to continue to log ``ipmitool`` verbose output in their + logs should explicitly set the ``[ipmi]debug`` command to True. diff --git a/releasenotes/notes/node-update-instance-info-extra-policies-862b2a70b941cf39.yaml b/releasenotes/notes/node-update-instance-info-extra-policies-862b2a70b941cf39.yaml new file mode 100644 index 000000000..ce9855239 --- /dev/null +++ b/releasenotes/notes/node-update-instance-info-extra-policies-862b2a70b941cf39.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + Adds ``baremetal:node:update_extra`` and ``baremetal:node:instance_info`` + policies to allow finer-grained policy control over node updates. In order + to use standalone Ironic to provision a node, a user must be able to + update these attributes, and a lessee should not be able to update all + node attributes. diff --git a/releasenotes/notes/rename-iso-builder-func-46694ed6ded84f4a.yaml b/releasenotes/notes/rename-iso-builder-func-46694ed6ded84f4a.yaml new file mode 100644 index 000000000..8bc40b621 --- /dev/null +++ b/releasenotes/notes/rename-iso-builder-func-46694ed6ded84f4a.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Renames misleadingly named `images.create_isolinux_image_for_uefi` + function into `images.create_esp_image_for_uefi`. The new name + reflects what's actually going on under the hood. |