Merge "Mount all vfat partitions before calling grub2"

2 files changed, 73 insertions, 6 deletions

diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py
index 7362ebe3..e0138a7c 100644
--- a/ironic_python_agent/extensions/image.py
+++ b/ironic_python_agent/extensions/image.py
@@ -497,6 +497,13 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None,
         return
 
     try:
+        # Add /bin to PATH variable as grub requires it to find efibootmgr
+        # when running in uefi boot mode.
+        # Add /usr/sbin to PATH variable to ensure it is there as we do
+        # not use full path to grub binary anymore.
+        path_variable = os.environ.get('PATH', '')
+        path_variable = '%s:/bin:/usr/sbin:/sbin' % path_variable
+
         # Mount the partition and binds
         path = tempfile.mkdtemp()
         if efi_system_part_uuid:
@@ -533,12 +540,13 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None,
         if os.path.exists(os.path.join(path, 'usr/sbin/grub2-install')):
             binary_name = "grub2"
 
-        # Add /bin to PATH variable as grub requires it to find efibootmgr
-        # when running in uefi boot mode.
-        # Add /usr/sbin to PATH variable to ensure it is there as we do
-        # not use full path to grub binary anymore.
-        path_variable = os.environ.get('PATH', '')
-        path_variable = '%s:/bin:/usr/sbin:/sbin' % path_variable
+        # Mount all vfat partitions listed in the fstab of the root partition.
+        # This is to make sure grub2 finds all files it needs, as some of them
+        # may not be inside the root partition but in the ESP (like grub2env).
+        LOG.debug("Mounting all partitions inside the image ...")
+        utils.execute('chroot %(path)s /bin/sh -c "mount -a -t vfat"' %
+                      {'path': path}, shell=True,
+                      env_variables={'PATH': path_variable})
 
         if efi_partitions:
             if not os.path.exists(efi_partition_mount_point):
@@ -658,6 +666,16 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None,
             LOG.error(error_msg)
             raise errors.CommandExecutionError(error_msg)
 
+        # Umount the vfat partitions we may have mounted
+        LOG.debug("Unmounting all partitions inside the image ...")
+        try:
+            utils.execute('chroot %(path)s /bin/sh -c "umount -a -t vfat"' %
+                          {'path': path}, shell=True,
+                          env_variables={'PATH': path_variable})
+        except processutils.ProcessExecutionError as e:
+            LOG.warning("Unable to umount vfat partitions. Error: %(error)s",
+                        {'error': e})
+
         for fs in BIND_MOUNTS:
             try:
                 utils.execute('umount', path + fs, attempts=3,
diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py
index 9dbec95e..d84307fb 100644
--- a/ironic_python_agent/tests/unit/extensions/test_image.py
+++ b/ironic_python_agent/tests/unit/extensions/test_image.py
@@ -362,6 +362,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                               self.fake_dir + '/run'),
                     mock.call('mount', '-t', 'sysfs', 'none',
                               self.fake_dir + '/sys'),
+                    mock.call(('chroot %s /bin/sh -c "mount -a -t vfat"' %
+                              (self.fake_dir)), shell=True,
+                              env_variables={
+                                  'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
                     mock.call(('chroot %s /bin/sh -c '
                               '"grub-install %s"' %
                                (self.fake_dir, self.fake_dev)), shell=True,
@@ -373,6 +377,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                               shell=True,
                               env_variables={
                                   'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
+                    mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' %
+                              (self.fake_dir)), shell=True,
+                              env_variables={
+                                  'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
                     mock.call('umount', self.fake_dir + '/dev',
                               attempts=3, delay_on_retry=True),
                     mock.call('umount', self.fake_dir + '/proc',
@@ -413,6 +421,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                               self.fake_dir + '/run'),
                     mock.call('mount', '-t', 'sysfs', 'none',
                               self.fake_dir + '/sys'),
+                    mock.call(('chroot %s /bin/sh -c "mount -a -t vfat"' %
+                              (self.fake_dir)), shell=True,
+                              env_variables={
+                                  'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
                     mock.call(('chroot %s /bin/sh -c '
                               '"grub-install %s"' %
                                (self.fake_dir, self.fake_prep_boot_part)),
@@ -425,6 +437,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                               shell=True,
                               env_variables={
                                   'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
+                    mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' %
+                              (self.fake_dir)), shell=True,
+                              env_variables={
+                                  'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
                     mock.call('umount', self.fake_dir + '/dev',
                               attempts=3, delay_on_retry=True),
                     mock.call('umount', self.fake_dir + '/proc',
@@ -473,6 +489,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                               self.fake_dir + '/run'),
                     mock.call('mount', '-t', 'sysfs', 'none',
                               self.fake_dir + '/sys'),
+                    mock.call(('chroot %s /bin/sh -c "mount -a -t vfat"' %
+                              (self.fake_dir)), shell=True,
+                              env_variables={
+                                  'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
                     mock.call('mount', self.fake_efi_system_part,
                               self.fake_dir + '/boot/efi'),
                     mock.call(('chroot %s /bin/sh -c "grub-install"' %
@@ -497,6 +517,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                                   'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
                     mock.call('umount', self.fake_dir + '/boot/efi',
                               attempts=3, delay_on_retry=True),
+                    mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' %
+                              (self.fake_dir)), shell=True,
+                              env_variables={
+                                  'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
                     mock.call('umount', self.fake_dir + '/dev',
                               attempts=3, delay_on_retry=True),
                     mock.call('umount', self.fake_dir + '/proc',
@@ -550,6 +574,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                               self.fake_dir + '/run'),
                     mock.call('mount', '-t', 'sysfs', 'none',
                               self.fake_dir + '/sys'),
+                    mock.call(('chroot %s /bin/sh -c "mount -a -t vfat"' %
+                              (self.fake_dir)), shell=True,
+                              env_variables={
+                                  'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
                     mock.call('mount', self.fake_efi_system_part,
                               self.fake_dir + '/boot/efi'),
                     mock.call(('chroot %s /bin/sh -c "grub-install"' %
@@ -583,6 +611,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
         mock_get_part_uuid.side_effect = [self.fake_root_part,
                                           self.fake_efi_system_part]
         mock_is_md_device.side_effect = [False, False]
+        environ_mock.get.return_value = '/sbin'
         mock_md_get_raid_devices.return_value = {}
 
         def mount_raise_func(*args, **kwargs):
@@ -596,6 +625,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                           efi_system_part_uuid=self.fake_efi_system_part_uuid)
 
         expected = [mock.call('mount', '/dev/fake2', self.fake_dir),
+                    mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' %
+                              (self.fake_dir)), shell=True,
+                              env_variables={
+                                  'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
                     mock.call('umount', self.fake_dir + '/dev',
                               attempts=3, delay_on_retry=True),
                     mock.call('umount', self.fake_dir + '/proc',
@@ -849,6 +882,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                               self.fake_dir + '/run'),
                     mock.call('mount', '-t', 'sysfs', 'none',
                               self.fake_dir + '/sys'),
+                    mock.call(('chroot %s /bin/sh -c "mount -a -t vfat"' %
+                              (self.fake_dir)), shell=True,
+                              env_variables={
+                                  'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
                     mock.call('mount', '/dev/sda1',
                               self.fake_dir + '/boot/efi'),
                     mock.call(('chroot %s /bin/sh -c "grub-install"' %
@@ -887,6 +924,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                                   'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
                     mock.call('umount', self.fake_dir + '/boot/efi',
                               attempts=3, delay_on_retry=True),
+                    mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' %
+                              (self.fake_dir)), shell=True,
+                              env_variables={
+                                  'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
                     mock.call('umount', self.fake_dir + '/dev',
                               attempts=3, delay_on_retry=True),
                     mock.call('umount', self.fake_dir + '/proc',
@@ -953,6 +994,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                       self.fake_dir + '/run'),
             mock.call('mount', '-t', 'sysfs', 'none',
                       self.fake_dir + '/sys'),
+            mock.call(('chroot %s /bin/sh -c "mount -a -t vfat"' %
+                       (self.fake_dir)), shell=True,
+                      env_variables={
+                          'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
             mock.call(('chroot %s /bin/sh -c '
                        '"grub-install %s"' %
                        (self.fake_dir, '/dev/sda')), shell=True,
@@ -969,6 +1014,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                       shell=True,
                       env_variables={
                           'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
+            mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' %
+                      (self.fake_dir)), shell=True,
+                      env_variables={
+                          'PATH': '/sbin:/bin:/usr/sbin:/sbin'}),
             mock.call('umount', self.fake_dir + '/dev',
                       attempts=3, delay_on_retry=True),
             mock.call('umount', self.fake_dir + '/proc',