Update multipath actors for el8toel9 #886

bmarzins · 2022-04-28T21:16:21Z

This patchset adds versions of the multipath actors for the el8toel9 repo. They are heavily based on the the el7toel8 actors, but simpler, since there is much less work to do on this upgrade. I left the produce_copy_to_target_task() code that was added to mutipathconfread the same as in the el7toel8 repo. If you want me to deal with the TODOs in it, just let me know.

github-actions · 2022-04-28T21:16:32Z

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergable.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

review please to notify leapp developers of review request
/packit copr-build to submit a public copr build using packit

To launch regression testing public members of oamg organization can leave the following comment:

/rerun to schedule tests using this pr build and leappmaster as artifacts
/rerun 42 to schedule tests using this pr build and leappPR42 as artifacts

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra.

bmarzins · 2022-04-28T21:28:14Z

/packit copr-build

bmarzins · 2022-04-28T21:55:43Z

/packit build

bmarzins · 2022-04-28T23:15:47Z

/packit build

bmarzins · 2022-04-28T23:45:08Z

review please

bmarzins · 2022-04-28T23:45:50Z

I'm not sure why the packit builds are failing, or what to do about it. Any help would be appreciated.

leapp-bot · 2022-04-29T09:00:26Z

This PR has been linked in issue tracker (#OAMG-6809).

pirat89 · 2022-04-29T14:44:58Z

/packit build

pirat89 · 2022-04-29T15:02:29Z

@bmarzins builds created. I guess it was a temporary glitch.

pirat89 · 2022-04-29T15:04:18Z

thanks again for the awesome PR!! I believe someone will take a look during the week.

fernflower

Can't help myself - that is one of the most thoroughly test-covered prs I have seen so far, thank you for your diligence!
Left some suggestions after a quick review.

fernflower · 2022-05-04T09:56:44Z

repos/system_upgrade/el8toel9/actors/multipathconfread/tests/test_multipath_conf_read_8to9.py

+TEST_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'files')
+
+
+def build_config(val):


Imho passing 5 arguments to this helper like build_config(pathname, config_dir, enable_foreign_exists, invalid_regexes_exists, allow_usb_exists) is cleaner and more self-speaking than one list-ike val there is now.
Sorry if I'm missing something.

fernflower · 2022-05-04T10:03:27Z

repos/system_upgrade/el8toel9/actors/multipathconfread/tests/test_multipath_conf_read_8to9.py

+    return conf
+
+
+def test_parse_default_rhel8():


Tests L87-L138 seem very much alike. How about introducing a mapping like

test_map = {'default_rhel8.conf': default_rhel8_conf, 'converted_the_things.conf': converted_the_things_conf, 'compicated.conf': complicated_conf, ...}

And the have a single test

def_test_parse_configs(): for config_name, expected_data in test_mapping.items(): config = multipathconfread._parse_config(os.path.join(TEST_DIR, config_name)) assert config assert_config(config, expected_data)

WDYT?

That sounds fine.

fernflower · 2022-05-04T10:07:16Z

.../system_upgrade/el8toel9/actors/multipathconfupdate/tests/test_multipath_conf_update_8to9.py

+converted_data = {}
+
+
+def build_config(val):


Same suggestion as above - 5 distinct arguments for this helper.

fernflower · 2022-05-04T10:08:06Z

.../system_upgrade/el8toel9/actors/multipathconfupdate/tests/test_multipath_conf_update_8to9.py

+            assert expected_data is None
+
+
+def test_default_rhel8(setup_test):


Same suggestion as above - instead of N similar tests have a mapping and just 1.

This reverts commit 1c171ec. the multipath actors need to do different things in the el8toel9 upgrade than in the el7toel8 upgrade, so leave the old actors in the el7toel8 repo. After reverting, the code was relinted.

Check config files and make necessary changes to retain RHEL-8 default behavior. This involves adding 'enable_foreign ""' and 'allow_usb_devices yes' to the defaults section of /etc/multipath.conf if these options don't already exist, and changing any regex values of "*" to ".*" in any multipath config file. Reports are generated for all changes made. In order to share code with el7toel8, move the multipathutil library to the system_upgrade/common repo.

bmarzins · 2022-05-05T05:23:24Z

I've updated the code to include all the suggestions from the review, although I'm not sure if I'm supposed to be the one to Resolve the Conversation, or not.

fernflower · 2022-05-10T10:12:06Z

/rerun

fernflower · 2022-05-10T10:13:15Z

/rerun

github-actions · 2022-05-10T10:18:37Z

Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/4392073

github-actions · 2022-05-10T10:18:45Z

Testing Farm request for RHEL-7.9-ZStream/4392073 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

github-actions · 2022-05-10T10:18:48Z

Testing Farm request for RHEL-8.6.0-Nightly/4392073 regression testing has been created.
Once finished, results should be available here.
Full pipeline log.

vinzenz · 2022-05-19T07:16:42Z

Very nice PR @bmarzins - If you can confirm your QE verified it, we can merge it.

bmarzins · 2022-05-19T14:43:35Z

I'm not sure what the QE process is supposed to be. I haven't sent this to the primary multipath QE person. I did personally run all the unit tests, run a number of scenarios through all the actors using snactor and saving the output, and run a couple of upgrades using the vagrant box with multipath installed on it.

Rezney · 2022-06-03T11:07:30Z

Very nice PR @bmarzins - If you can confirm your QE verified it, we can merge it.

We discussed the verification offline.

## Packaging - Provide and require leapp-repository-dependencies 7 (oamg#952) - Provide `leapp-command(<CMD>)` for each CLI command provided by leapp-repository (oamg#947) - Require dracut, kmod, procps-ng on RHEL 8+ (oamg#952) - Require leapp-framework >= 3.1 (oamg#905, oamg#927) ## Upgrade handling ### Fixes - Do not create the upgrade bootloader entry when the dnf dry-run actor fails (oamg#912) - Do not inhibit in-place upgrades in case LUKS volumes are Ceph OSDs (oamg#735) - Fix & improve application of custom selinux rules to be less error prone and do not override changes done by RPM scriptlets (oamg#925) - Fix detection of deprecated devices (and drivers) regarding the PCI address (oamg#881) - Fix detection of deprecated kernel modules (oamg#874) - Fix the false positive NFS storage detection on NFS servers (oamg#888) - Fix the issues on systems with the LANGUAGE environment variable (oamg#887) - Fix the root directory scan to deal with non-utf8 filenames (oamg#927) - Skip comment lines when parsing the GRUB configuration file (oamg#883) - Stop propagating the “debug” and ”enforcing=0” kernel cmdline options into the target kernel cmdline options (oamg#938, oamg#950) - [IPU 7 -> 8] Fix the upgrade of the Satellite server (oamg#875, oamg#878, oamg#879 oamg#890, oamg#899, oamg#916, 934) - [IPU 7 -> 8] Fix SSSD: Prune old cache files (the format of data is incompatible) (oamg#922) - [IPU 8 -> 9] Enable the CRB repository for the upgrade only if enabled on the source system (oamg#942) - [IPU 8 -> 9] Drop obsoleted actor blocking upgrade on z16 (oamg#892) - [IPU 8 -> 9] Fix cloud provider detection on AWS (oamg#920) - [IPU 8 -> 9] Fix detention of the latest kernel on RHEL 8+ systems (oamg#909) - [IPU 8 -> 9] Fix issues caused by leapp artifacts from previous in-place upgrades (oamg#889) - [IPU 8 -> 9] Fix issues with false positive switch to emergency console during the upgrade (oamg#906) - [IPU 8 -> 9] Fix swap page size on aarch64 (oamg#937, oamg#948) - [IPU 8 -> 9] Fix the VDO scanner to skip partitions unrelated to VDO and adjust error messages (oamg#919) ### Enhancements - Add 8.7 & 9.1 Beta & GA product certificates (oamg#891) - Detect /var/lib/leapp being mounted in a non-persistent fashion (oamg#921) - Detect /var/lib/leapp mounted with the noexec option (oamg#908) - Improve the report msg when NFS partitions are discovered providing info about concrete mountpoints (oamg#806) - Inform about necessary migrations related to bacula-director (oamg#896) - [IPU 7 -> 8] The default upgrade path for RHEL SAP is 7.9 -> 8.6 (oamg#939) - [IPU 7 -> 8] Detect and fix missing newline at the end of /etc/default/grub (oamg#945) - [IPU 7 -> 8] Handle upgrades of SAP Apps systems on Azure (oamg#926) - [IPU 7 -> 8] Handle upgrades on RHUI Google Cloud (oamg#897, oamg#946) - [IPU 8 -> 9] Support upgrade path RHEL 8.7 -> 9.0 and RHEL SAP 8.6 -> 9.0 (oamg#903, oamg#894) - [IPU 8 -> 9] Add actors covering removal of NIS components on RHEL 9 (oamg#851) - [IPU 8 -> 9] Add checks for obsolete .NET versions (oamg#867) - [IPU 8 -> 9] Allow specifying the report schema v1.2.0 (oamg#872) - [IPU 8 -> 9] Check and handle upgrades with custom crypto policies (oamg#898) - [IPU 8 -> 9] Check and migrate OpenSSH configuration (oamg#864, oamg#860) - [IPU 8 -> 9] Check and migrate multipath configuration the upgrade (oamg#886) - [IPU 8 -> 9] Check minimum memory requirements (oamg#935) - [IPU 8 -> 9] Enable Base and SAP In-place upgrades on Azure (oamg#943) - [IPU 8 -> 9] Enable in-place upgrades in Azure RHEL 8 base images using RHUI (oamg#918) - [IPU 8 -> 9] Handle upgrades of SAP systems on AWS (oamg#924) - [IPU 8 -> 9] Inhibit upgrade when NVIDIA driver is detected (oamg#880) - [IPU 8 -> 9] Migrate blocklisted CAs (oamg#882) - [IPU 8 -> 9] Migrate the OpenSSL configuration (oamg#900) - [IPU 8 -> 9] Report changes around SCP and SFTP (oamg#863, oamg#893) ## Additional changes interesting for devels - Extend LsblkEntry model in StorageInfo by kernel name and size of partition in bytes (oamg#919) - Mass refactoring: Fix imports in actors and libraries to follow project guidelines (oamg#932) - Mass refactoring: Replace use of deprecated `reporting.(Tags|Flags)` by `reporting.Groups` (oamg#932) - PESEventScanner actor has been fully refactored (oamg#856, oamg#941) - Use library function is_inhibitor to check for failures (oamg#905) Signed-off-by: Petr Stodulka <pstodulk@redhat.com>

## Packaging - Provide and require leapp-repository-dependencies 7 (#952) - Provide `leapp-command(<CMD>)` for each CLI command provided by leapp-repository (#947) - Require dracut, kmod, procps-ng on RHEL 8+ (#952) - Require leapp-framework >= 3.1 (#905, #927) ## Upgrade handling ### Fixes - Do not create the upgrade bootloader entry when the dnf dry-run actor fails (#912) - Do not inhibit in-place upgrades in case LUKS volumes are Ceph OSDs (#735) - Fix & improve application of custom selinux rules to be less error prone and do not override changes done by RPM scriptlets (#925) - Fix detection of deprecated devices (and drivers) regarding the PCI address (#881) - Fix detection of deprecated kernel modules (#874) - Fix the false positive NFS storage detection on NFS servers (#888) - Fix the issues on systems with the LANGUAGE environment variable (#887) - Fix the root directory scan to deal with non-utf8 filenames (#927) - Skip comment lines when parsing the GRUB configuration file (#883) - Stop propagating the “debug” and ”enforcing=0” kernel cmdline options into the target kernel cmdline options (#938, #950) - [IPU 7 -> 8] Fix the upgrade of the Satellite server (#875, #878, #879 #890, #899, #916, 934) - [IPU 7 -> 8] Fix SSSD: Prune old cache files (the format of data is incompatible) (#922) - [IPU 8 -> 9] Enable the CRB repository for the upgrade only if enabled on the source system (#942) - [IPU 8 -> 9] Drop obsoleted actor blocking upgrade on z16 (#892) - [IPU 8 -> 9] Fix cloud provider detection on AWS (#920) - [IPU 8 -> 9] Fix detention of the latest kernel on RHEL 8+ systems (#909) - [IPU 8 -> 9] Fix issues caused by leapp artifacts from previous in-place upgrades (#889) - [IPU 8 -> 9] Fix issues with false positive switch to emergency console during the upgrade (#906) - [IPU 8 -> 9] Fix swap page size on aarch64 (#937, #948) - [IPU 8 -> 9] Fix the VDO scanner to skip partitions unrelated to VDO and adjust error messages (#919) ### Enhancements - Add 8.7 & 9.1 Beta & GA product certificates (#891) - Detect /var/lib/leapp being mounted in a non-persistent fashion (#921) - Detect /var/lib/leapp mounted with the noexec option (#908) - Improve the report msg when NFS partitions are discovered providing info about concrete mountpoints (#806) - Inform about necessary migrations related to bacula-director (#896) - [IPU 7 -> 8] The default upgrade path for RHEL SAP is 7.9 -> 8.6 (#939) - [IPU 7 -> 8] Detect and fix missing newline at the end of /etc/default/grub (#945) - [IPU 7 -> 8] Handle upgrades of SAP Apps systems on Azure (#926) - [IPU 7 -> 8] Handle upgrades on RHUI Google Cloud (#897, #946) - [IPU 8 -> 9] Support upgrade path RHEL 8.7 -> 9.0 and RHEL SAP 8.6 -> 9.0 (#903, #894) - [IPU 8 -> 9] Add actors covering removal of NIS components on RHEL 9 (#851) - [IPU 8 -> 9] Add checks for obsolete .NET versions (#867) - [IPU 8 -> 9] Allow specifying the report schema v1.2.0 (#872) - [IPU 8 -> 9] Check and handle upgrades with custom crypto policies (#898) - [IPU 8 -> 9] Check and migrate OpenSSH configuration (#864, #860) - [IPU 8 -> 9] Check and migrate multipath configuration the upgrade (#886) - [IPU 8 -> 9] Check minimum memory requirements (#935) - [IPU 8 -> 9] Enable Base and SAP In-place upgrades on Azure (#943) - [IPU 8 -> 9] Enable in-place upgrades in Azure RHEL 8 base images using RHUI (#918) - [IPU 8 -> 9] Handle upgrades of SAP systems on AWS (#924) - [IPU 8 -> 9] Inhibit upgrade when NVIDIA driver is detected (#880) - [IPU 8 -> 9] Migrate blocklisted CAs (#882) - [IPU 8 -> 9] Migrate the OpenSSL configuration (#900) - [IPU 8 -> 9] Report changes around SCP and SFTP (#863, #893) ## Additional changes interesting for devels - Extend LsblkEntry model in StorageInfo by kernel name and size of partition in bytes (#919) - Mass refactoring: Fix imports in actors and libraries to follow project guidelines (#932) - Mass refactoring: Replace use of deprecated `reporting.(Tags|Flags)` by `reporting.Groups` (#932) - PESEventScanner actor has been fully refactored (#856, #941) - Use library function is_inhibitor to check for failures (#905) Signed-off-by: Petr Stodulka <pstodulk@redhat.com>

bmarzins marked this pull request as ready for review April 28, 2022 23:15

fernflower reviewed May 4, 2022

View reviewed changes

bmarzins added 2 commits May 5, 2022 00:17

Revert "Move "multipathconfread" into common repository"

6a79e0f

This reverts commit 1c171ec. the multipath actors need to do different things in the el8toel9 upgrade than in the el7toel8 upgrade, so leave the old actors in the el7toel8 repo. After reverting, the code was relinted.

bmarzins force-pushed the el8toel9_mpath branch from 2bba385 to 6f4ff75 Compare May 5, 2022 05:18

Rezney approved these changes Jun 3, 2022

View reviewed changes

Rezney merged commit 2f3d8e5 into oamg:master Jun 3, 2022

pirat89 added enhancement New feature or request changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant labels Jul 22, 2022

pirat89 mentioned this pull request Aug 23, 2022

# Release 0.17.0 #953

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update multipath actors for el8toel9 #886

Update multipath actors for el8toel9 #886

bmarzins commented Apr 28, 2022 •

edited

github-actions bot commented Apr 28, 2022

bmarzins commented Apr 28, 2022

bmarzins commented Apr 28, 2022

bmarzins commented Apr 28, 2022

bmarzins commented Apr 28, 2022

bmarzins commented Apr 28, 2022

leapp-bot commented Apr 29, 2022

pirat89 commented Apr 29, 2022

pirat89 commented Apr 29, 2022

pirat89 commented Apr 29, 2022

fernflower left a comment

fernflower May 4, 2022

bmarzins May 5, 2022

fernflower May 4, 2022

bmarzins May 5, 2022

fernflower May 4, 2022

bmarzins May 5, 2022

fernflower May 4, 2022

bmarzins May 5, 2022

bmarzins commented May 5, 2022

fernflower commented May 10, 2022

fernflower commented May 10, 2022

github-actions bot commented May 10, 2022

github-actions bot commented May 10, 2022

github-actions bot commented May 10, 2022

vinzenz commented May 19, 2022

bmarzins commented May 19, 2022

Rezney commented Jun 3, 2022

		TEST_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'files')


		def build_config(val):

		assert expected_data is None


		def test_default_rhel8(setup_test):

Update multipath actors for el8toel9 #886

Update multipath actors for el8toel9 #886

Conversation

bmarzins commented Apr 28, 2022 • edited

github-actions bot commented Apr 28, 2022

Thank you for contributing to the Leapp project!

bmarzins commented Apr 28, 2022

bmarzins commented Apr 28, 2022

bmarzins commented Apr 28, 2022

bmarzins commented Apr 28, 2022

bmarzins commented Apr 28, 2022

leapp-bot commented Apr 29, 2022

pirat89 commented Apr 29, 2022

pirat89 commented Apr 29, 2022

pirat89 commented Apr 29, 2022

fernflower left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmarzins commented May 5, 2022

fernflower commented May 10, 2022

fernflower commented May 10, 2022

github-actions bot commented May 10, 2022

github-actions bot commented May 10, 2022

github-actions bot commented May 10, 2022

vinzenz commented May 19, 2022

bmarzins commented May 19, 2022

Rezney commented Jun 3, 2022

bmarzins commented Apr 28, 2022 •

edited