checknfs: do not check systemd mounts #1127

MichalHe · 2023-09-14T11:49:08Z

Optional, keeping this code breaks nothing, however, the inhibitor cannot be reproduced.

Systemd mounts contain only block devices. Therefore, the list can never contain NFS shares at all and the check is redundant. This is apparent if one reads storagescanner/libraries/storagescanner.py:L251. This patch, therefore, removes the check for systemd mount alltogether.

@pirat89 @fernflower @matejmatuska

github-actions · 2023-09-14T11:49:23Z

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 mergeable.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

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

Packit will automatically schedule regression tests for this PR's build and latest upstream leapp build. If you need a different version of leapp from PR#42, use /packit test oamg/leapp#42

It is possible to schedule specific on-demand tests as well. Currently 2 test sets are supported, beaker-minimal and kernel-rt, both can be used to be run on all upgrade paths or just a couple of specific ones.
To launch on-demand tests with packit:

/packit test --labels kernel-rt to schedule kernel-rt tests set for all upgrade paths
/packit test --labels beaker-minimal-8.9to9.3,kernel-rt-8.9to9.3 to schedule kernel-rt and beaker-minimal test sets for 8.9->9.3 upgrade path

[Deprecated] To launch on-demand regression testing public members of oamg organization can leave the following comment:

/rerun to schedule basic regression tests using this pr build and latest upstream leapp build as artifacts
/rerun 42 to schedule basic regression tests using this pr build and leapp*PR42* as artifacts
/rerun-sst to schedule sst tests using this pr build and latest upstream leapp build as artifacts
/rerun-sst 42 to schedule sst tests using this pr build and leapp*PR42* 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 contact leapp-infra.

fernflower · 2023-09-15T11:57:21Z

I have used your pr's build for some unrelated upgrade testing and ended up with a crash

2023-09-15 07:48:24.63  INFO     PID: 4727 leapp.workflow.Checks: Executing actor check_nfs 
Process Process-370:
Traceback (most recent call last):
  File "/usr/lib64/python3.6/multiprocessing/process.py", line 258, in _bootstrap
    self.run()
  File "/usr/lib64/python3.6/multiprocessing/process.py", line 93, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/lib/python3.6/site-packages/leapp/repository/actor_definition.py", line 74, in _do_run
    actor_instance.run(*args, **kwargs)
  File "/usr/lib/python3.6/site-packages/leapp/actors/__init__.py", line 289, in run
    self.process(*args)
  File "/etc/leapp/repos.d/system_upgrade/common/actors/checknfs/actor.py", line 44, in process
    if any(fstab_nfs_mounts, nfs_mounts):
TypeError: any() takes exactly one argument (2 given)
2023-09-15 07:48:24.183 ERROR    PID: 4727 leapp.workflow.Checks: Actor check_nfs has crashed: Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/leapp/repository/actor_definition.py", line 74, in _do_run
    actor_instance.run(*args, **kwargs)
  File "/usr/lib/python3.6/site-packages/leapp/actors/__init__.py", line 289, in run
    self.process(*args)
  File "/etc/leapp/repos.d/system_upgrade/common/actors/checknfs/actor.py", line 44, in process
    if any(fstab_nfs_mounts, nfs_mounts):
TypeError: any() takes exactly one argument (2 given)

I believe the rest of the tests failed for this particular reason as well

fernflower

Please fix the traceback

MichalHe · 2023-09-15T18:45:57Z

@fernflower Thanks, should be fixed.

fernflower · 2023-09-18T10:42:29Z

/packit build

fernflower · 2023-09-19T05:39:14Z

/packit build

fernflower · 2023-09-19T13:07:21Z

@MichalHe I am not an expert, so I still have to do some research, but some recipes "how to mount NFS share using systemd" like https://sleeplessbeastie.eu/2019/09/23/how-to-mount-nfs-share-using-systemd/ have caught my eye. I will need to study those before I can properly review this pr.

stale

MichalHe · 2023-09-19T13:57:19Z

@fernflower It isn't that you cannot mount a NFS share using systemd-mount, its that the systemd_mounts entry contains only block devices, see storagescanner, see https://stackoverflow.com/questions/58914555/nfs-mount-point-as-a-disk-device-linux. The patch is not critical since it just removes dead code.

Probably a good way on how to reproduce the "issue" is to setup a box that has NFS mounted via systemd-mount, and checking whether the mount will be present in systemd_mounts (should not be). The upgrade will be still inhibited due to active NFS mount (found in output of mount).

fernflower · 2023-10-02T13:25:31Z

/packit build

pirat89 · 2023-10-06T12:16:37Z

/packit test

pirat89

It make sense. code lgtm. waiting for tests (restarted as they stucked)

Systemd mounts contain only *block* devices. Therefore, the list can never contain NFS shares at all and the check is redundant. This is apparent if one reads storagescanner/libraries/storagescanner.py:L251. This patch, therefore, removes the check for systemd mount alltogether.

pirat89 · 2023-11-03T15:24:29Z

Rebased. hopefully tests will pass now.

## Packaging - Requires xfsprogs and e2fsprogs (oamg#1154) - Bump leapp-repository-dependencies to 10 (oamg#1154) ## Upgrade handling ### Fixes - Detect changes in openssl default configuration file and restore it to the default to the target default during the upgrade to reduce risk of potential issues (oamg#1131) - Do not try to download data files anymore when missing as the service is obsoleted since the data is part of installed packages (oamg#1120) - Drop the invalid `tuv` target channel (oamg#1130) - Fix handling of symlinks under /etc/pki when managing certificates (oamg#1135, oamg#1160, oamg#1166) - Fix semanage import issue (oamg#1164) - Fix the issue of going out of bounds in the isccfg parser (oamg#1124) - Fix traceback when saving the rhsm facts results and the /etc/rhsm/facts directory doesn’t exist yet (oamg#1132) - Handle the upgrade better when a proxy is configured in YUM/DNF configutations (oamg#1143) - Load all rpm repository substitutions that dnf knows about, not just releasever since repofiles may use the other substitutions too (oamg#1134) - Minor updates of generated reports (oamg#1151) - Print nice error msg when device and driver deprecation data is malformed (oamg#1168) - Report information about required manual steps after the upgrade when openssl-ibmca is installed (oamg#1131) - Update error messages and reports when installed upgrade data files are malformed or missing (oamg#1120) - [IPU 7 -> 8] Fix the upgrade of the RH Satellite server when tomcat is installed (oamg#1150) - [IPU 8 -> 9] Fix the upgrade from RHEL 8.9+ when the release is locked by subscription-manager (oamg#1136, oamg#1138) ### Enhancements - Update upgrade paths: (oamg#1146, oamg#1147, oamg#1175) - RHEL 7.9 -> 8.10, 8.8 (default: 8.10) - RHEL with SAPAHA 7.9 -> 8.10, 8.8 (default: 8.8) - RHEL w/o SAP HANA 8.8 -> 9.2 - RHEL w/o SAP HANA 8.10 -> 9.4 - Added possibility to define DNF configuration for the target system (oamg#1143) - Code cleaning: drop redundant and invalid NFS checks (oamg#1127) - Default to NO_RHSM mode when subscription-manager is not found (oamg#1133) - Detect customized configuration of dynamic linker (oamg#1118) - Detect possible unexpected RPM GPG keys has been installed during RPM transaction (oamg#1101) - Drop obsoleted upgrade paths that relates to releases: 8.6, 8.9, 9.0, 9.3 (oamg#1175) - Ignore Leapp related PES events (oamg#1153) - Introduce generic transition of systemd services states during the IPU (oamg#1060, oamg#1174) - Introduce possibility to upgrade with local repositories (oamg#1099) - Introduced some changes getting us closer to possibility of IPU for Centos (Stream) systems (oamg#1140) - Report the upgrade customisations and modifications of the upgrade tooling (oamg#1148) - Simplify handling of upgrades on systems using RHUI, reducing the maintenance burden for cloud providers (oamg#1057) - Update the leapp upgrade data files - bump data stream to "3.0" (oamg#1163, oamg#1165, oamg#1170) - [IPU 8 -> 9] Enable upgrades RHEL 8 -> 9 using RHUI on Alibaba cloud (oamg#1137, oamg#1165, oamg#1172) ## Additional changes interesting for devels - Introduced new functions returning a list of packages related to upgrade - see the rpms library (oamg#1156) - Make detection of installed signed packages distribution agnostic - covers RHEL & CentOS (oamg#876) - Model InstalledRedHatSignedRPM is deprecated, replaced by DistributionSignedRPM (oamg#876)

## Packaging - Requires xfsprogs and e2fsprogs (oamg#1154) - Bump leapp-repository-dependencies to 10 (oamg#1154) ## Upgrade handling ### Fixes - Detect changes in openssl default configuration file and restore it to the default to the target default during the upgrade to reduce risk of potential issues (oamg#1131) - Do not try to download data files anymore when missing as the service is obsoleted since the data is part of installed packages (oamg#1120) - Drop the invalid `tuv` target channel (oamg#1130) - Fix handling of symlinks under /etc/pki when managing certificates (oamg#1135, oamg#1160, oamg#1166) - Fix semanage import issue (oamg#1164) - Fix the issue of going out of bounds in the isccfg parser (oamg#1124) - Fix traceback when saving the rhsm facts results and the /etc/rhsm/facts directory doesn’t exist yet (oamg#1132) - Handle the upgrade better when a proxy is configured in YUM/DNF configutations (oamg#1143) - Load all rpm repository substitutions that dnf knows about, not just releasever since repofiles may use the other substitutions too (oamg#1134) - Minor updates of generated reports (oamg#1151) - Print nice error msg when device and driver deprecation data is malformed (oamg#1168) - Report information about required manual steps after the upgrade when openssl-ibmca is installed (oamg#1131) - Update error messages and reports when installed upgrade data files are malformed or missing (oamg#1120) - [IPU 7 -> 8] Fix the upgrade of the RH Satellite server when tomcat is installed (oamg#1150) - [IPU 8 -> 9] Fix the upgrade from RHEL 8.9+ when the release is locked by subscription-manager (oamg#1136, oamg#1138) ### Enhancements - Update upgrade paths: (oamg#1146, oamg#1147, oamg#1175) - RHEL 7.9 -> 8.10, 8.8 (default: 8.10) - RHEL with SAPAHA 7.9 -> 8.10, 8.8 (default: 8.8) - RHEL w/o SAP HANA 8.8 -> 9.2 - RHEL w/o SAP HANA 8.10 -> 9.4 - Added possibility to define DNF configuration for the target system (oamg#1143) - Code cleaning: drop redundant and invalid NFS checks (oamg#1127) - Default to NO_RHSM mode when subscription-manager is not found (oamg#1133) - Detect customized configuration of dynamic linker (oamg#1118) - Detect possible unexpected RPM GPG keys has been installed during RPM transaction (oamg#1101) - Drop obsoleted upgrade paths that relates to releases: 8.6, 8.9, 9.0, 9.3 (oamg#1175) - Ignore Leapp related PES events (oamg#1153) - Introduce generic transition of systemd services states during the IPU (oamg#1060, oamg#1174) - Introduce possibility to upgrade with local repositories (oamg#1099) - Introduced some changes getting us closer to possibility of IPU for Centos (Stream) systems (oamg#1140) - Report the upgrade customisations and modifications of the upgrade tooling (oamg#1148) - Simplify handling of upgrades on systems using RHUI, reducing the maintenance burden for cloud providers (oamg#1057) - Update the leapp upgrade data files - bump data stream to "3.0" (oamg#1163, oamg#1165, oamg#1170) - [IPU 8 -> 9] Enable upgrades RHEL 8 -> 9 using RHUI on Alibaba cloud (oamg#1137, oamg#1165, oamg#1172) - Unify breakpoints inside the upgrade initramfs for the easier troubleshooting (oamg#1157) ## Additional changes interesting for devels - Introduced new functions returning a list of packages related to upgrade - see the rpms library (oamg#1156) - Make detection of installed signed packages distribution agnostic - covers RHEL & CentOS (oamg#876) - Model InstalledRedHatSignedRPM is deprecated, replaced by DistributionSignedRPM (oamg#876)

## Packaging - Requires xfsprogs and e2fsprogs (#1154) - Bump leapp-repository-dependencies to 10 (#1154) ## Upgrade handling ### Fixes - Detect changes in openssl default configuration file and restore it to the default to the target default during the upgrade to reduce risk of potential issues (#1131) - Do not try to download data files anymore when missing as the service is obsoleted since the data is part of installed packages (#1120) - Drop the invalid `tuv` target channel (#1130) - Fix handling of symlinks under /etc/pki when managing certificates (#1135, #1160, #1166) - Fix semanage import issue (#1164) - Fix the issue of going out of bounds in the isccfg parser (#1124) - Fix traceback when saving the rhsm facts results and the /etc/rhsm/facts directory doesn’t exist yet (#1132) - Handle the upgrade better when a proxy is configured in YUM/DNF configutations (#1143) - Load all rpm repository substitutions that dnf knows about, not just releasever since repofiles may use the other substitutions too (#1134) - Minor updates of generated reports (#1151) - Print nice error msg when device and driver deprecation data is malformed (#1168) - Report information about required manual steps after the upgrade when openssl-ibmca is installed (#1131) - Update error messages and reports when installed upgrade data files are malformed or missing (#1120) - [IPU 7 -> 8] Fix the upgrade of the RH Satellite server when tomcat is installed (#1150) - [IPU 8 -> 9] Fix the upgrade from RHEL 8.9+ when the release is locked by subscription-manager (#1136, #1138) ### Enhancements - Update upgrade paths: (#1146, #1147, #1175) - RHEL 7.9 -> 8.10, 8.8 (default: 8.10) - RHEL with SAPAHA 7.9 -> 8.10, 8.8 (default: 8.8) - RHEL w/o SAP HANA 8.8 -> 9.2 - RHEL w/o SAP HANA 8.10 -> 9.4 - Added possibility to define DNF configuration for the target system (#1143) - Code cleaning: drop redundant and invalid NFS checks (#1127) - Default to NO_RHSM mode when subscription-manager is not found (#1133) - Detect customized configuration of dynamic linker (#1118) - Detect possible unexpected RPM GPG keys has been installed during RPM transaction (#1101) - Drop obsoleted upgrade paths that relates to releases: 8.6, 8.9, 9.0, 9.3 (#1175) - Ignore Leapp related PES events (#1153) - Introduce generic transition of systemd services states during the IPU (#1060, #1174) - Introduce possibility to upgrade with local repositories (#1099) - Introduced some changes getting us closer to possibility of IPU for Centos (Stream) systems (#1140) - Report the upgrade customisations and modifications of the upgrade tooling (#1148) - Simplify handling of upgrades on systems using RHUI, reducing the maintenance burden for cloud providers (#1057) - Update the leapp upgrade data files - bump data stream to "3.0" (#1163, #1165, #1170) - [IPU 8 -> 9] Enable upgrades RHEL 8 -> 9 using RHUI on Alibaba cloud (#1137, #1165, #1172) - Unify breakpoints inside the upgrade initramfs for the easier troubleshooting (#1157) ## Additional changes interesting for devels - Introduced new functions returning a list of packages related to upgrade - see the rpms library (#1156) - Make detection of installed signed packages distribution agnostic - covers RHEL & CentOS (#876) - Model InstalledRedHatSignedRPM is deprecated, replaced by DistributionSignedRPM (#876)

## Packaging - Requires xfsprogs and e2fsprogs (oamg#1154) - Bump leapp-repository-dependencies to 10 (oamg#1154) ## Upgrade handling ### Fixes - Detect changes in openssl default configuration file and restore it to the default to the target default during the upgrade to reduce risk of potential issues (oamg#1131) - Do not try to download data files anymore when missing as the service is obsoleted since the data is part of installed packages (oamg#1120) - Drop the invalid `tuv` target channel (oamg#1130) - Fix handling of symlinks under /etc/pki when managing certificates (oamg#1135, oamg#1160, oamg#1166) - Fix semanage import issue (oamg#1164) - Fix the issue of going out of bounds in the isccfg parser (oamg#1124) - Fix traceback when saving the rhsm facts results and the /etc/rhsm/facts directory doesn’t exist yet (oamg#1132) - Handle the upgrade better when a proxy is configured in YUM/DNF configutations (oamg#1143) - Load all rpm repository substitutions that dnf knows about, not just releasever since repofiles may use the other substitutions too (oamg#1134) - Minor updates of generated reports (oamg#1151) - Print nice error msg when device and driver deprecation data is malformed (oamg#1168) - Report information about required manual steps after the upgrade when openssl-ibmca is installed (oamg#1131) - Update error messages and reports when installed upgrade data files are malformed or missing (oamg#1120) - [IPU 7 -> 8] Fix the upgrade of the RH Satellite server when tomcat is installed (oamg#1150) - [IPU 8 -> 9] Fix the upgrade from RHEL 8.9+ when the release is locked by subscription-manager (oamg#1136, oamg#1138) ### Enhancements - Update upgrade paths: (oamg#1146, oamg#1147, oamg#1175) - RHEL 7.9 -> 8.10, 8.8 (default: 8.10) - RHEL with SAPAHA 7.9 -> 8.10, 8.8 (default: 8.8) - RHEL w/o SAP HANA 8.8 -> 9.2 - RHEL w/o SAP HANA 8.10 -> 9.4 - Added possibility to define DNF configuration for the target system (oamg#1143) - Code cleaning: drop redundant and invalid NFS checks (oamg#1127) - Default to NO_RHSM mode when subscription-manager is not found (oamg#1133) - Detect customized configuration of dynamic linker (oamg#1118) - Detect possible unexpected RPM GPG keys has been installed during RPM transaction (oamg#1101) - Drop obsoleted upgrade paths that relates to releases: 8.6, 8.9, 9.0, 9.3 (oamg#1175) - Ignore Leapp related PES events (oamg#1153) - Introduce generic transition of systemd services states during the IPU (oamg#1060, oamg#1174) - Introduce possibility to upgrade with local repositories (oamg#1099) - Introduced some changes getting us closer to possibility of IPU for Centos (Stream) systems (oamg#1140) - Report the upgrade customisations and modifications of the upgrade tooling (oamg#1148) - Simplify handling of upgrades on systems using RHUI, reducing the maintenance burden for cloud providers (oamg#1057) - Update the leapp upgrade data files - bump data stream to "3.0" (oamg#1163, oamg#1165, oamg#1170) - [IPU 8 -> 9] Enable upgrades RHEL 8 -> 9 using RHUI on Alibaba cloud (oamg#1137, oamg#1165, oamg#1172) - Unify breakpoints inside the upgrade initramfs for the easier troubleshooting (oamg#1157) ## Additional changes interesting for devels - Introduced new functions returning a list of packages related to upgrade - see the rpms library (oamg#1156) - Make detection of installed signed packages distribution agnostic - covers RHEL & CentOS (oamg#876) - Model InstalledRedHatSignedRPM is deprecated, replaced by DistributionSignedRPM (oamg#876) (cherry picked from commit 6421225)

fernflower previously requested changes Sep 15, 2023

View reviewed changes

MichalHe force-pushed the remove_systemd_nfs branch from a7bfe11 to 9be831e Compare September 15, 2023 18:45

fernflower mentioned this pull request Sep 19, 2023

Packit config for leapp oamg/leapp#827

Merged

pirat89 approved these changes Oct 6, 2023

View reviewed changes

pirat89 added this to the 8.10/9.4 milestone Oct 6, 2023

pirat89 mentioned this pull request Oct 10, 2023

make pylint happy #1129

Merged

pirat89 force-pushed the remove_systemd_nfs branch from 9be831e to ec54476 Compare November 3, 2023 15:24

pirat89 merged commit bf866cb into oamg:master Nov 10, 2023

pirat89 added the changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant label Nov 10, 2023

pirat89 mentioned this pull request Feb 13, 2024

Release 0.20.0 #1177

Merged

yuravk mentioned this pull request Aug 9, 2024

Update to version 0.20.0 AlmaLinux/leapp-repository#119

Merged

MichalHe deleted the remove_systemd_nfs branch January 13, 2025 08:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

checknfs: do not check systemd mounts #1127

checknfs: do not check systemd mounts #1127

Uh oh!

MichalHe commented Sep 14, 2023

Uh oh!

github-actions bot commented Sep 14, 2023

Uh oh!

fernflower commented Sep 15, 2023

Uh oh!

fernflower left a comment

Uh oh!

MichalHe commented Sep 15, 2023

Uh oh!

fernflower commented Sep 18, 2023

Uh oh!

fernflower commented Sep 19, 2023

Uh oh!

fernflower commented Sep 19, 2023

Uh oh!

MichalHe commented Sep 19, 2023 •

edited

Loading

Uh oh!

fernflower commented Oct 2, 2023

Uh oh!

pirat89 commented Oct 6, 2023

Uh oh!

pirat89 left a comment

Uh oh!

pirat89 commented Nov 3, 2023

Uh oh!

Uh oh!

checknfs: do not check systemd mounts #1127

checknfs: do not check systemd mounts #1127

Uh oh!

Conversation

MichalHe commented Sep 14, 2023

Uh oh!

github-actions bot commented Sep 14, 2023

Thank you for contributing to the Leapp project!

Uh oh!

fernflower commented Sep 15, 2023

Uh oh!

fernflower left a comment

Choose a reason for hiding this comment

Uh oh!

MichalHe commented Sep 15, 2023

Uh oh!

fernflower commented Sep 18, 2023

Uh oh!

fernflower commented Sep 19, 2023

Uh oh!

fernflower commented Sep 19, 2023

Uh oh!

MichalHe commented Sep 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fernflower commented Oct 2, 2023

Uh oh!

pirat89 commented Oct 6, 2023

Uh oh!

pirat89 left a comment

Choose a reason for hiding this comment

Uh oh!

pirat89 commented Nov 3, 2023

Uh oh!

Uh oh!

MichalHe commented Sep 19, 2023 •

edited

Loading