diff options
| -rw-r--r-- | examples/business-logic/example_business_logic.py | 7 | ||||
| -rw-r--r-- | ironic_python_agent/agent.py | 11 | ||||
| -rw-r--r-- | ironic_python_agent/extensions/image.py | 9 | ||||
| -rw-r--r-- | ironic_python_agent/extensions/standby.py | 34 | ||||
| -rw-r--r-- | ironic_python_agent/hardware.py | 37 | ||||
| -rw-r--r-- | ironic_python_agent/hardware_managers/cna.py | 7 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/extensions/test_image.py | 38 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/hardware_managers/test_cna.py | 4 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/test_agent.py | 2 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/test_hardware.py | 97 | ||||
| -rw-r--r-- | ironic_python_agent/utils.py | 3 | ||||
| -rw-r--r-- | tox.ini | 4 |
12 files changed, 129 insertions, 124 deletions
diff --git a/examples/business-logic/example_business_logic.py b/examples/business-logic/example_business_logic.py index 4fb7fc32..eac88a59 100644 --- a/examples/business-logic/example_business_logic.py +++ b/examples/business-logic/example_business_logic.py @@ -190,5 +190,8 @@ class ExampleBusinessLogicHardwareManager(hardware.HardwareManager): # Make sure to provide default values for optional arguments. def companyx_apply_something(self, node, ports, required_value, optional_value=None): - LOG.info('apply_something called with required_value={} and ' - 'optional_value={}'.format(required_value, optional_value)) + LOG.info('apply_something called with ' + 'required_value=%(required_value)s and ' + 'optional_value=%(optional_value)s', + {'required_value': required_value, + 'optional_value': optional_value}) diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index a9f8077f..c5b24560 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -125,17 +125,16 @@ class IronicPythonAgentHeartbeater(threading.Thread): self.heartbeat_forced = False self.previous_heartbeat = _time() except errors.HeartbeatConflictError: - LOG.warning('conflict error sending heartbeat to {}'.format( - self.agent.api_url)) + LOG.warning('conflict error sending heartbeat to %s', + self.agent.api_url) except Exception: - LOG.exception('error sending heartbeat to {}'.format( - self.agent.api_url)) + LOG.exception('error sending heartbeat to %s', self.agent.api_url) finally: interval_multiplier = random.uniform(self.min_jitter_multiplier, self.max_jitter_multiplier) self.interval = self.agent.heartbeat_timeout * interval_multiplier - log_msg = 'sleeping before next heartbeat, interval: {0}' - LOG.info(log_msg.format(self.interval)) + LOG.info('sleeping before next heartbeat, interval: %s', + self.interval) def force_heartbeat(self): self.heartbeat_forced = True diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index 84e86f17..1d439923 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -274,7 +274,7 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition, # Before updating let's get information about the bootorder LOG.debug("Getting information about boot order.") - utils.execute('efibootmgr') + utils.execute('efibootmgr', '-v') # NOTE(iurygregory): regex used to identify the Warning in the stderr after # we add the new entry. Example: # "efibootmgr: ** Warning ** : Boot0004 has same label ironic" @@ -303,7 +303,7 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition, 'dev': device}) # Update the nvram using efibootmgr # https://linux.die.net/man/8/efibootmgr - cmd = utils.execute('efibootmgr', '-c', '-d', device, + cmd = utils.execute('efibootmgr', '-v', '-c', '-d', device, '-p', efi_partition, '-w', '-L', label, '-l', v_efi_bl_path) for line in cmd[1].split('\n'): @@ -489,8 +489,9 @@ def _prepare_boot_partitions_for_softraid(device, holders, efi_part, # RAID the ESPs, metadata=1.0 is mandatory to be able to boot md_device = '/dev/md/esp' - LOG.debug("Creating md device {} for the ESPs on {}".format( - md_device, efi_partitions)) + LOG.debug("Creating md device %(md_device)s for the ESPs " + "on %(efi_partitions)s", + {'md_device': md_device, 'efi_partitions': efi_partitions}) utils.execute('mdadm', '--create', md_device, '--force', '--run', '--metadata=1.0', '--level', '1', '--raid-devices', len(efi_partitions), diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py index 970cf7de..e63a9572 100644 --- a/ironic_python_agent/extensions/standby.py +++ b/ironic_python_agent/extensions/standby.py @@ -202,7 +202,7 @@ def _write_whole_disk_image(image, image_info, device): command = ['qemu-img', 'convert', '-t', 'directsync', '-O', 'host_device', '-W', image, device] - LOG.info('Writing image with command: {}'.format(' '.join(command))) + LOG.info('Writing image with command: %s', ' '.join(command)) try: disk_utils.convert_image(image, device, out_format='host_device', cache='directsync', out_of_order=True) @@ -232,8 +232,9 @@ def _write_image(image_info, device, configdrive=None): else: _write_whole_disk_image(image, image_info, device) totaltime = time.time() - starttime - LOG.info('Image {} written to device {} in {} seconds'.format( - image, device, totaltime)) + LOG.info('Image %(image)s written to device %(device)s in %(totaltime)s ' + 'seconds', {'image': image, 'device': device, + 'totaltime': totaltime}) try: disk_utils.fix_gpt_partition(device, node_uuid=None) except exception.InstanceDeployFailure: @@ -329,7 +330,7 @@ class ImageDownload(object): details = [] for url in image_info['urls']: try: - LOG.info("Attempting to download image from {}".format(url)) + LOG.info("Attempting to download image from %s", url) self._request = _download_with_proxy(image_info, url, image_info['id']) except errors.ImageDownloadError as e: @@ -386,12 +387,16 @@ class ImageDownload(object): not match the checksum as reported by glance in image_info. """ checksum = self._hash_algo.hexdigest() - LOG.debug('Verifying image at {} against {} checksum ' - '{}'.format(image_location, self._hash_algo.name, checksum)) + LOG.debug('Verifying image at %(image_location)s against ' + '%(algo_name)s checksum %(checksum)s', + {'image_location': image_location, + 'algo_name': self._hash_algo.name, + 'checksum': checksum}) if checksum != self._expected_hash_value: - LOG.error(errors.ImageChecksumError.details_str.format( + error_msg = errors.ImageChecksumError.details_str.format( image_location, self._image_info['id'], - self._expected_hash_value, checksum)) + self._expected_hash_value, checksum) + LOG.error(error_msg) raise errors.ImageChecksumError(image_location, self._image_info['id'], self._expected_hash_value, @@ -434,8 +439,10 @@ def _download_image(image_info): break totaltime = time.time() - starttime - LOG.info("Image downloaded from {} in {} seconds".format(image_location, - totaltime)) + LOG.info("Image downloaded from %(image_location)s " + "in %(totaltime)s seconds", + {'image_location': image_location, + 'totaltime': totaltime}) image_download.verify_image(image_location) @@ -597,8 +604,8 @@ class StandbyExtension(base.BaseAgentExtension): break totaltime = time.time() - starttime - LOG.info("Image streamed onto device {} in {} " - "seconds".format(device, totaltime)) + LOG.info("Image streamed onto device %(device)s in %(totaltime)s " + "seconds", {'device': device, 'totaltime': totaltime}) # Verify if the checksum of the streamed image is correct image_download.verify_image(device) # Fix any gpt partition @@ -609,7 +616,8 @@ class StandbyExtension(base.BaseAgentExtension): pass # Fix the root partition UUID root_uuid = disk_utils.block_uuid(device) - LOG.info("{} UUID is now {}".format(device, root_uuid)) + LOG.info("%(device)s UUID is now %(root_uuid)s", + {'device': device, 'root_uuid': root_uuid}) self.partition_uuids['root uuid'] = root_uuid def _fix_up_partition_uuids(self, image_info, device): diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 87e5ea82..e963059f 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -127,6 +127,17 @@ def _udev_settle(): return +def _load_ipmi_modules(): + """Load kernel modules required for IPMI interaction. + + This is required to be called at least once before attempting to use + ipmitool or related tools. + """ + il_utils.try_execute('modprobe', 'ipmi_msghandler') + il_utils.try_execute('modprobe', 'ipmi_devintf') + il_utils.try_execute('modprobe', 'ipmi_si') + + def _check_for_iscsi(): """Connect iSCSI shared connected via iBFT or OF. @@ -445,13 +456,13 @@ def list_all_block_devices(block_type='disk', # lvm, part, rom, loop if devtype != block_type: if devtype is None or ignore_raid: - LOG.debug("Skipping: {!r}".format(line)) + LOG.debug("Skipping: %s", line) continue elif ('raid' in devtype and block_type in ['raid', 'disk']): LOG.debug( "TYPE detected to contain 'raid', signifying a " - "RAID volume. Found: {!r}".format(line)) + "RAID volume. Found: %s", line) elif (devtype == 'md' and (block_type == 'part' or block_type == 'md')): @@ -461,11 +472,11 @@ def list_all_block_devices(block_type='disk', # more detail. LOG.debug( "TYPE detected to contain 'md', signifying a " - "RAID partition. Found: {!r}".format(line)) + "RAID partition. Found: %s", line) else: LOG.debug( - "TYPE did not match. Wanted: {!r} but found: {!r}".format( - block_type, line)) + "TYPE did not match. Wanted: %(block_type)s but found: " + "%(line)s", {'block_type': block_type, 'line': line}) continue # Ensure all required columns are at least present, even if blank @@ -984,6 +995,7 @@ class GenericHardwareManager(HardwareManager): # Do some initialization before we declare ourself ready _check_for_iscsi() _md_scan_and_assemble() + _load_ipmi_modules() self.wait_for_disks() return HardwareSupport.GENERIC @@ -1765,11 +1777,6 @@ class GenericHardwareManager(HardwareManager): :return: IP address of lan channel or 0.0.0.0 in case none of them is configured properly """ - # These modules are rarely loaded automatically - il_utils.try_execute('modprobe', 'ipmi_msghandler') - il_utils.try_execute('modprobe', 'ipmi_devintf') - il_utils.try_execute('modprobe', 'ipmi_si') - try: # From all the channels 0-15, only 1-11 can be assigned to # different types of communication media and protocols and @@ -1807,11 +1814,6 @@ class GenericHardwareManager(HardwareManager): :return: MAC address of the first LAN channel or 00:00:00:00:00:00 in case none of them has one or is configured properly """ - # These modules are rarely loaded automatically - il_utils.try_execute('modprobe', 'ipmi_msghandler') - il_utils.try_execute('modprobe', 'ipmi_devintf') - il_utils.try_execute('modprobe', 'ipmi_si') - try: # From all the channels 0-15, only 1-11 can be assigned to # different types of communication media and protocols and @@ -1859,11 +1861,6 @@ class GenericHardwareManager(HardwareManager): configured properly. May return None value if it cannot interract with system tools or critical error occurs. """ - # These modules are rarely loaded automatically - il_utils.try_execute('modprobe', 'ipmi_msghandler') - il_utils.try_execute('modprobe', 'ipmi_devintf') - il_utils.try_execute('modprobe', 'ipmi_si') - null_address_re = re.compile(r'^::(/\d{1,3})*$') def get_addr(channel, dynamic=False): diff --git a/ironic_python_agent/hardware_managers/cna.py b/ironic_python_agent/hardware_managers/cna.py index 2d49d9e0..a4021c10 100644 --- a/ironic_python_agent/hardware_managers/cna.py +++ b/ironic_python_agent/hardware_managers/cna.py @@ -64,9 +64,10 @@ def _disable_embedded_lldp_agent_in_cna_card(): failed_dirs.append(inner_dir) continue if failed_dirs: - LOG.warning('Failed to disable the embedded LLDP on Intel CNA network ' - 'card. Addresses of failed pci devices: {}' - .format(str(failed_dirs).strip('[]'))) + warning_msg = ('Failed to disable the embedded LLDP on Intel ' + 'CNA network card. Addresses of failed pci ' + 'devices: %s', str(failed_dirs).strip('[]')) + LOG.warning(warning_msg) class IntelCnaHardwareManager(hardware.HardwareManager): diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 05e937b8..33056575 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -236,8 +236,8 @@ class TestImageExtension(base.IronicAgentTest): mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr'), - mock.call('efibootmgr', '-c', '-d', self.fake_dev, + mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', '\\EFI\\BOOT\\BOOTX64.EFI'), @@ -283,8 +283,8 @@ class TestImageExtension(base.IronicAgentTest): mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr'), - mock.call('efibootmgr', '-c', '-d', self.fake_dev, + mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', '\\EFI\\BOOT\\BOOTX64.EFI'), @@ -335,8 +335,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr'), - mock.call('efibootmgr', '-c', '-d', self.fake_dev, + mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', '\\EFI\\BOOT\\BOOTX64.EFI'), @@ -387,12 +387,12 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr'), - mock.call('efibootmgr', '-c', '-d', self.fake_dev, + mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', '\\EFI\\BOOT\\BOOTX64.EFI'), - mock.call('efibootmgr', '-c', '-d', self.fake_dev, + mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic2', '-l', '\\WINDOWS\\system32\\winload.efi'), @@ -2226,8 +2226,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr'), - mock.call('efibootmgr', '-c', '-d', self.fake_dev, + mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', '\\EFI\\BOOT\\BOOTX64.EFI'), @@ -2270,8 +2270,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr'), - mock.call('efibootmgr', '-c', '-d', self.fake_dev, + mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'Vendor String', '-l', '\\EFI\\vendor\\shimx64.efi'), @@ -2310,8 +2310,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock.call('udevadm', 'settle'), mock.call('mount', '/dev/fakenvme0p1', self.fake_dir + '/boot/efi'), - mock.call('efibootmgr'), - mock.call('efibootmgr', '-c', '-d', '/dev/fakenvme0', + mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', '-c', '-d', '/dev/fakenvme0', '-p', '1', '-w', '-L', 'ironic1', '-l', '\\EFI\\BOOT\\BOOTX64.EFI'), @@ -2350,8 +2350,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr'), - mock.call('efibootmgr', '-c', '-d', self.fake_dev, + mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', '\\EFI\\BOOT\\BOOTX64.EFI'), @@ -2457,8 +2457,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n self.fake_dev, self.fake_efi_system_part, self.fake_dir) - expected = [mock.call('efibootmgr'), - mock.call('efibootmgr', '-c', '-d', self.fake_dev, + expected = [mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', self.fake_efi_system_part, '-w', '-L', 'ironic1', '-l', '\\EFI\\BOOT\\BOOTX64.EFI')] diff --git a/ironic_python_agent/tests/unit/hardware_managers/test_cna.py b/ironic_python_agent/tests/unit/hardware_managers/test_cna.py index b38d6d20..29c456a7 100644 --- a/ironic_python_agent/tests/unit/hardware_managers/test_cna.py +++ b/ironic_python_agent/tests/unit/hardware_managers/test_cna.py @@ -112,8 +112,8 @@ class TestIntelCnaHardwareManager(base.IronicAgentTest): cna._disable_embedded_lldp_agent_in_cna_card() expected_log_message = ('Failed to disable the embedded LLDP on ' 'Intel CNA network card. Addresses of ' - 'failed pci devices: {}' - .format(str(listdir_dict).strip('[]'))) + 'failed pci devices: %s', + str(listdir_dict).strip('[]')) mock_log.warning.assert_called_once_with(expected_log_message) @mock.patch.object(cna, 'LOG', autospec=True) diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py index b020f692..9b95e695 100644 --- a/ironic_python_agent/tests/unit/test_agent.py +++ b/ironic_python_agent/tests/unit/test_agent.py @@ -159,6 +159,7 @@ class TestHeartbeater(ironic_agent_base.IronicAgentTest): @mock.patch.object(hardware, '_md_scan_and_assemble', lambda: None) @mock.patch.object(hardware, '_check_for_iscsi', lambda: None) +@mock.patch.object(hardware, '_load_ipmi_modules', lambda: None) @mock.patch.object(hardware.GenericHardwareManager, 'wait_for_disks', lambda self: None) class TestBaseAgent(ironic_agent_base.IronicAgentTest): @@ -959,6 +960,7 @@ class TestAgentStandalone(ironic_agent_base.IronicAgentTest): @mock.patch.object(hardware, '_md_scan_and_assemble', lambda: None) @mock.patch.object(hardware, '_check_for_iscsi', lambda: None) +@mock.patch.object(hardware, '_load_ipmi_modules', lambda: None) @mock.patch.object(hardware.GenericHardwareManager, 'wait_for_disks', lambda self: None) @mock.patch.object(socket, 'gethostbyname', autospec=True) diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 44b2a87d..143957eb 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -2246,41 +2246,35 @@ class TestGenericHardwareManager(base.IronicAgentTest): mock_dev_file.side_effect = reads self.assertTrue(self.hardware._is_read_only_device(device)) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_address(self, mocked_execute, mte): + def test_get_bmc_address(self, mocked_execute): mocked_execute.return_value = '192.1.2.3\n', '' self.assertEqual('192.1.2.3', self.hardware.get_bmc_address()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_address_virt(self, mocked_execute, mte): + def test_get_bmc_address_virt(self, mocked_execute): mocked_execute.side_effect = processutils.ProcessExecutionError() self.assertIsNone(self.hardware.get_bmc_address()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_address_zeroed(self, mocked_execute, mte): + def test_get_bmc_address_zeroed(self, mocked_execute): mocked_execute.return_value = '0.0.0.0\n', '' self.assertEqual('0.0.0.0', self.hardware.get_bmc_address()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_address_invalid(self, mocked_execute, mte): + def test_get_bmc_address_invalid(self, mocked_execute): # In case of invalid lan channel, stdout is empty and the error # on stderr is "Invalid channel" mocked_execute.return_value = '\n', 'Invalid channel: 55' self.assertEqual('0.0.0.0', self.hardware.get_bmc_address()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_address_random_error(self, mocked_execute, mte): + def test_get_bmc_address_random_error(self, mocked_execute): mocked_execute.return_value = '192.1.2.3\n', 'Random error message' self.assertEqual('192.1.2.3', self.hardware.get_bmc_address()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_address_iterate_channels(self, mocked_execute, mte): + def test_get_bmc_address_iterate_channels(self, mocked_execute): # For channel 1 we simulate unconfigured IP # and for any other we return a correct IP address def side_effect(*args, **kwargs): @@ -2295,57 +2289,49 @@ class TestGenericHardwareManager(base.IronicAgentTest): mocked_execute.side_effect = side_effect self.assertEqual('192.1.2.3', self.hardware.get_bmc_address()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_address_not_available(self, mocked_execute, mte): + def test_get_bmc_address_not_available(self, mocked_execute): mocked_execute.return_value = '', '' self.assertEqual('0.0.0.0', self.hardware.get_bmc_address()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_mac_not_available(self, mocked_execute, mte): + def test_get_bmc_mac_not_available(self, mocked_execute): mocked_execute.return_value = '', '' self.assertRaises(errors.IncompatibleHardwareMethodError, self.hardware.get_bmc_mac) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_mac(self, mocked_execute, mte): + def test_get_bmc_mac(self, mocked_execute): mocked_execute.return_value = '192.1.2.3\n01:02:03:04:05:06', '' self.assertEqual('01:02:03:04:05:06', self.hardware.get_bmc_mac()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_mac_virt(self, mocked_execute, mte): + def test_get_bmc_mac_virt(self, mocked_execute): mocked_execute.side_effect = processutils.ProcessExecutionError() self.assertIsNone(self.hardware.get_bmc_mac()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_mac_zeroed(self, mocked_execute, mte): + def test_get_bmc_mac_zeroed(self, mocked_execute): mocked_execute.return_value = '0.0.0.0\n00:00:00:00:00:00', '' self.assertRaises(errors.IncompatibleHardwareMethodError, self.hardware.get_bmc_mac) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_mac_invalid(self, mocked_execute, mte): + def test_get_bmc_mac_invalid(self, mocked_execute): # In case of invalid lan channel, stdout is empty and the error # on stderr is "Invalid channel" mocked_execute.return_value = '\n', 'Invalid channel: 55' self.assertRaises(errors.IncompatibleHardwareMethodError, self.hardware.get_bmc_mac) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_mac_random_error(self, mocked_execute, mte): + def test_get_bmc_mac_random_error(self, mocked_execute): mocked_execute.return_value = ('192.1.2.3\n00:00:00:00:00:02', 'Random error message') self.assertEqual('00:00:00:00:00:02', self.hardware.get_bmc_mac()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_mac_iterate_channels(self, mocked_execute, mte): + def test_get_bmc_mac_iterate_channels(self, mocked_execute): # For channel 1 we simulate unconfigured IP # and for any other we return a correct IP address def side_effect(*args, **kwargs): @@ -2363,15 +2349,13 @@ class TestGenericHardwareManager(base.IronicAgentTest): mocked_execute.side_effect = side_effect self.assertEqual('01:02:03:04:05:06', self.hardware.get_bmc_mac()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_v6address_not_enabled(self, mocked_execute, mte): + def test_get_bmc_v6address_not_enabled(self, mocked_execute): mocked_execute.side_effect = [('ipv4\n', '')] * 11 self.assertEqual('::/0', self.hardware.get_bmc_v6address()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_v6address_dynamic_address(self, mocked_execute, mte): + def test_get_bmc_v6address_dynamic_address(self, mocked_execute): mocked_execute.side_effect = [ ('ipv6\n', ''), (hws.IPMITOOL_LAN6_PRINT_DYNAMIC_ADDR, '') @@ -2379,9 +2363,8 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.assertEqual('2001:1234:1234:1234:1234:1234:1234:1234', self.hardware.get_bmc_v6address()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_v6address_static_address_both(self, mocked_execute, mte): + def test_get_bmc_v6address_static_address_both(self, mocked_execute): dynamic_disabled = \ hws.IPMITOOL_LAN6_PRINT_DYNAMIC_ADDR.replace('active', 'disabled') mocked_execute.side_effect = [ @@ -2392,15 +2375,13 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.assertEqual('2001:5678:5678:5678:5678:5678:5678:5678', self.hardware.get_bmc_v6address()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_v6address_virt(self, mocked_execute, mte): + def test_get_bmc_v6address_virt(self, mocked_execute): mocked_execute.side_effect = processutils.ProcessExecutionError() self.assertIsNone(self.hardware.get_bmc_v6address()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_v6address_invalid_enables(self, mocked_execute, mte): + def test_get_bmc_v6address_invalid_enables(self, mocked_execute): def side_effect(*args, **kwargs): if args[0].startswith('ipmitool lan6 print'): return '', 'Failed to get IPv6/IPv4 Addressing Enables' @@ -2408,9 +2389,8 @@ class TestGenericHardwareManager(base.IronicAgentTest): mocked_execute.side_effect = side_effect self.assertEqual('::/0', self.hardware.get_bmc_v6address()) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_v6address_invalid_get_address(self, mocked_execute, mte): + def test_get_bmc_v6address_invalid_get_address(self, mocked_execute): def side_effect(*args, **kwargs): if args[0].startswith('ipmitool lan6 print'): if args[0].endswith('dynamic_addr') \ @@ -2422,10 +2402,9 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.assertEqual('::/0', self.hardware.get_bmc_v6address()) @mock.patch.object(hardware, 'LOG', autospec=True) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) def test_get_bmc_v6address_ipmitool_invalid_stdout_format( - self, mocked_execute, mte, mocked_log): + self, mocked_execute, mocked_log): def side_effect(*args, **kwargs): if args[0].startswith('ipmitool lan6 print'): if args[0].endswith('dynamic_addr') \ @@ -2439,9 +2418,8 @@ class TestGenericHardwareManager(base.IronicAgentTest): 'command: %(e)s', mock.ANY) mocked_log.warning.assert_has_calls([one_call] * 14) - @mock.patch.object(il_utils, 'try_execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test_get_bmc_v6address_channel_7(self, mocked_execute, mte): + def test_get_bmc_v6address_channel_7(self, mocked_execute): def side_effect(*args, **kwargs): if not args[0].startswith('ipmitool lan6 print 7'): # ipv6 is not enabled for channels 1-6 @@ -4072,6 +4050,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.hardware._nvme_erase, block_device) +@mock.patch.object(hardware, '_load_ipmi_modules', autospec=True) @mock.patch.object(hardware.GenericHardwareManager, 'get_os_install_device', autospec=True) @mock.patch.object(hardware, '_md_scan_and_assemble', autospec=True) @@ -4084,7 +4063,8 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest): def test_evaluate_hw_waits_for_disks( self, mocked_sleep, mocked_check_for_iscsi, - mocked_md_assemble, mocked_get_inst_dev): + mocked_md_assemble, mocked_get_inst_dev, + mocked_load_ipmi_modules): mocked_get_inst_dev.side_effect = [ errors.DeviceNotFound('boom'), None @@ -4092,6 +4072,7 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest): result = self.hardware.evaluate_hardware_support() + self.assertTrue(mocked_load_ipmi_modules.called) self.assertTrue(mocked_check_for_iscsi.called) self.assertTrue(mocked_md_assemble.called) self.assertEqual(hardware.HardwareSupport.GENERIC, result) @@ -4102,7 +4083,8 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest): @mock.patch.object(hardware, 'LOG', autospec=True) def test_evaluate_hw_no_wait_for_disks( self, mocked_log, mocked_sleep, mocked_check_for_iscsi, - mocked_md_assemble, mocked_get_inst_dev): + mocked_md_assemble, mocked_get_inst_dev, + mocked_load_ipmi_modules): CONF.set_override('disk_wait_attempts', '0') result = self.hardware.evaluate_hardware_support() @@ -4116,7 +4098,8 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest): @mock.patch.object(hardware, 'LOG', autospec=True) def test_evaluate_hw_waits_for_disks_nonconfigured( self, mocked_log, mocked_sleep, mocked_check_for_iscsi, - mocked_md_assemble, mocked_get_inst_dev): + mocked_md_assemble, mocked_get_inst_dev, + mocked_load_ipmi_modules): mocked_get_inst_dev.side_effect = [ errors.DeviceNotFound('boom'), errors.DeviceNotFound('boom'), @@ -4147,7 +4130,8 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest): mocked_sleep, mocked_check_for_iscsi, mocked_md_assemble, - mocked_get_inst_dev): + mocked_get_inst_dev, + mocked_load_ipmi_modules): CONF.set_override('disk_wait_attempts', '1') mocked_get_inst_dev.side_effect = [ @@ -4167,7 +4151,8 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest): def test_evaluate_hw_disks_timeout_unconfigured(self, mocked_sleep, mocked_check_for_iscsi, mocked_md_assemble, - mocked_get_inst_dev): + mocked_get_inst_dev, + mocked_load_ipmi_modules): mocked_get_inst_dev.side_effect = errors.DeviceNotFound('boom') self.hardware.evaluate_hardware_support() mocked_sleep.assert_called_with(3) @@ -4175,7 +4160,8 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest): def test_evaluate_hw_disks_timeout_configured(self, mocked_sleep, mocked_check_for_iscsi, mocked_md_assemble, - mocked_root_dev): + mocked_root_dev, + mocked_load_ipmi_modules): CONF.set_override('disk_wait_delay', '5') mocked_root_dev.side_effect = errors.DeviceNotFound('boom') @@ -4184,7 +4170,8 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest): def test_evaluate_hw_disks_timeout( self, mocked_sleep, mocked_check_for_iscsi, - mocked_md_assemble, mocked_get_inst_dev): + mocked_md_assemble, mocked_get_inst_dev, + mocked_load_ipmi_modules): mocked_get_inst_dev.side_effect = errors.DeviceNotFound('boom') result = self.hardware.evaluate_hardware_support() self.assertEqual(hardware.HardwareSupport.GENERIC, result) @@ -4274,6 +4261,14 @@ class TestModuleFunctions(base.IronicAgentTest): mocked_execute.assert_has_calls([ mock.call('iscsistart', '-f')]) + @mock.patch.object(il_utils, 'try_execute', autospec=True) + def test__load_ipmi_modules(self, mocked_try_execute, me): + hardware._load_ipmi_modules() + mocked_try_execute.assert_has_calls([ + mock.call('modprobe', 'ipmi_msghandler'), + mock.call('modprobe', 'ipmi_devintf'), + mock.call('modprobe', 'ipmi_si')]) + def create_hdparm_info(supported=False, enabled=False, locked=False, frozen=False, enhanced_erase=False): diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py index 2fc60ff1..74ac328b 100644 --- a/ironic_python_agent/utils.py +++ b/ironic_python_agent/utils.py @@ -865,8 +865,7 @@ def create_partition_table(dev_name, partition_table_type): :raises: CommandExecutionError if an error is encountered while attempting to create the partition table. """ - LOG.info("Creating partition table on {}".format( - dev_name)) + LOG.info("Creating partition table on %s", dev_name) try: execute('parted', dev_name, '-s', '--', 'mklabel', partition_table_type) @@ -35,10 +35,10 @@ commands = stestr run {posargs} [testenv:pep8] usedevelop = False deps= - hacking>=3.1.0,<4.0.0 # Apache-2.0 + hacking>=4.1.0,<5.0.0 # Apache-2.0 bashate>=0.5.1 # Apache-2.0 flake8-import-order>=0.17.1 # LGPLv3 - pycodestyle>=2.0.0,<2.7.0 # MIT + pycodestyle>=2.0.0,<3.0.0 # MIT doc8>=0.8.1 # Apache-2.0 allowlist_externals = bash commands = |