Bug 1901440

Created attachment 1733292 [details]
zipl actor to reproduce on x86_64 systems (assumes /boot is not a dedicated partition)

Created attachment 1733292 [details]
zipl actor to reproduce on x86_64 systems (assumes /boot is not a dedicated partition)

Description of problem:

When /boot is not a dedicated partition, on Z systems, the leapp upgrade fails after rebooting, when the actor "remove_boot_files" deletes the upgrade initramfs/vmlinuz from /boot:
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
ERROR    PID: 688 leapp.workflow.Preparation.remove_boot_files: Could not remove /boot/vmlinuz-upgrade.s390x: [Errno 2] No such file or directory: '/boot/vmlinuz-upgrade.s390x'.
ERROR    PID: 688 leapp.workflow.Preparation.remove_boot_files: Could not remove /boot/initramfs-upgrade.s390x.img: [Errno 2] No such file or directory: '/boot/initramfs-upgrade.s390x.img'.
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------

This is a consequence of a severe bug happening when the "zipl_convert_to_blscfg" actor executes: a bind mount of /boot is made onto /var/lib/leapp/el8userspace/boot, zipl runs, then a cleanup of the bind mount /var/lib/leapp/el8userspace/boot is performed *without* unmounting it, causing a deletion of the content of host's /boot:
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
DEBUG    PID: 527 leapp.workflow.Preparation.zipl_convert_to_blscfg: External command has started: ['rm', '-rf', u'/var/lib/leapp/el8userspace/boot']
DEBUG    PID: 527 leapp.workflow.Preparation.zipl_convert_to_blscfg: External command has finished: ['rm', '-rf', u'/var/lib/leapp/el8userspace/boot']
DEBUG    PID: 527 leapp.workflow.Preparation.zipl_convert_to_blscfg: External command has started: ['mount', '-o', 'bind', '/boot', u'/var/lib/leapp/el8userspace/boot']
DEBUG    PID: 527 leapp.workflow.Preparation.zipl_convert_to_blscfg: External command has finished: ['mount', '-o', 'bind', '/boot', u'/var/lib/leapp/el8userspace/boot']
DEBUG    PID: 527 leapp.workflow.Preparation.zipl_convert_to_blscfg: External command has started: ['systemd-nspawn', ...]
DEBUG    PID: 527 leapp.workflow.Preparation.zipl_convert_to_blscfg: External command has finished: ['systemd-nspawn', ...]
DEBUG    PID: 527 leapp.workflow.Preparation.zipl_convert_to_blscfg: External command has started: ['rm', '-rf', u'/var/lib/leapp/el8userspace/boot']
lib/leapp/el8userspace/boot': Device or resource busy^
DEBUG    PID: 527 leapp.workflow.Preparation.zipl_convert_to_blscfg: Command ['rm', '-rf', u'/var/lib/leapp/el8userspace/boot'] failed with exit code 1.
DEBUG    PID: 527 leapp.workflow.Preparation.zipl_convert_to_blscfg: External command has finished: ['rm', '-rf', u'/var/lib/leapp/el8userspace/boot']
WARNING  PID: 527 leapp.workflow.Preparation.zipl_convert_to_blscfg: Removing mount directory /var/lib/leapp/el8userspace/boot failed with: Command ['rm', '-rf', u'/var/lib/leapp/el8userspace/boot'] failed with exit code 1.
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------

The root cause for all this is Python2's bug https://bugs.python.org/issue29707: in a nutshell, "os.path.ismount(<mount>)" for Bind mounts is not working, in particular when the Bind mount is done on the same device (our case here), as shown in the example below (/mnt is hosted on /, /bind_mount also):

-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
# mkdir /bind_mount
# mount -o bind /mnt /bind_mount

# mount | grep bind
/dev/vda1 on /bind_mount type xfs (rw,relatime,seclabel,attr2,inode64,noquota)

# python2 -c 'import os; print os.path.ismount("/bind_mount")'
False
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------

Due to using unreliable os.path.ismount() in "mounting.py" library, this breaks:

/usr/share/leapp-repository/repositories/system_upgrade/el7toel8/libraries/mounting.py:
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
    def _cleanup(self):
        """ Cleanup operations """
        if os.path.exists(self.target) and os.path.ismount(self.target):        <<<<<<<<<<<<<<<< BREAKS HERE
            try:
                run(['umount', '-fl', self.target], split=False)
            except (OSError, CalledProcessError) as e:
                api.current_logger().warning('Unmounting %s failed with: %s', self.target, str(e))
        for directory in itertools.chain(self.additional_directories, (self.target,)):   <<<<<<< DELETE WHILE NOT UMOUNTED!
            try:
                run(['rm', '-rf', directory], split=False)
            except (OSError, CalledProcessError) as e:
                api.current_logger().warning('Removing mount directory %s failed with: %s', directory, str(e))
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------


Version-Release number of selected component (if applicable):

leapp-repository-0.12.0-2.el7_9.noarch


How reproducible:

Always on Z with /boot merged into /


Steps to Reproduce: Hack to reproduce on any x86_64 system with /boot merged into /

1. Hack /usr/share/leapp-repository/repositories/system_upgrade/el7toel8/actors/ziplconverttoblscfg/actor.py to execute on x86_64

-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
@@ -21,8 +21,8 @@ class ZiplConvertToBLSCFG(Actor):
     tags = (IPUWorkflowTag, PreparationPhaseTag)
 
     def process(self):
-        if not architecture.matches_architecture(architecture.ARCH_S390X):
-            return
+        #if not architecture.matches_architecture(architecture.ARCH_S390X):
+        #    return
         userspace = next(self.consume(TargetUserSpaceInfo), None)
         if not userspace:
             # actually this should not happen, but in such case, we want to still
@@ -44,15 +44,15 @@ class ZiplConvertToBLSCFG(Actor):
                 os.remove(userspace_zipl_conf)
             with mounting.NullMount(target=userspace.path) as userspace:
                 with userspace.nspawn() as context:
-                    context.copy_to('/etc/zipl.conf', '/etc/zipl.conf')
-                    # zipl needs this one as well
-                    context.copy_to('/etc/machine-id', '/etc/machine-id')
+                    #context.copy_to('/etc/zipl.conf', '/etc/zipl.conf')
+                    ## zipl needs this one as well
+                    #context.copy_to('/etc/machine-id', '/etc/machine-id')
                     try:
-                        context.call(['/usr/sbin/zipl-switch-to-blscfg'])
-                        if filecmp.cmp('/etc/zipl.conf', userspace_zipl_conf):
-                            # When the files are same, zipl failed - see the switch script
-                            raise OSError('Failed to convert the ZIPL configuration to BLS.')
-                        context.copy_from('/etc/zipl.conf', '/etc/zipl.conf')
+                        context.call(['/usr/bin/true'])
+                        #if filecmp.cmp('/etc/zipl.conf', userspace_zipl_conf):
+                        #    # When the files are same, zipl failed - see the switch script
+                        #    raise OSError('Failed to convert the ZIPL configuration to BLS.')
+                        #context.copy_from('/etc/zipl.conf', '/etc/zipl.conf')
                     except OSError as e:
                         self.log.error('Could not call zipl-switch-to-blscfg command.',
                                        exc_info=True)
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------

(File attached for convenience)

2. leapp upgrade and reboot

Actual results:

Dies deleting /boot/*-upgrade* files + content of /boot of host being deleted


Expected results:

No deletion of /boot content


Additional info:

This is only reproducible on zVM because the affected actor only runs on Z, but potentially could affect any future update of leapp-repository.