From ebd7b076a5d470bf8fe25b329f8da7ed31eea836 Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Tue, 6 Oct 2015 14:50:55 -0400 Subject: [PATCH] Use mount -t sysfs to avoid host /sys dependencies This patch updates the _install_grub2 function in image.py so that we use 'mount -t sysfs' instead of bind mounting the hosts /sys into the chroot. This resolves issues which can occur with unmounting the chroot mounts where 'target is busy' errors may occur when you are debugging things with a serial console. Change-Id: I5c9a6546d048cbf54695329d9744fda55127bd30 Closes-bug: #1503385 --- ironic_python_agent/extensions/image.py | 11 +++++++++- .../tests/unit/extensions/test_image.py | 20 +++++++++---------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index a61a0fbec..6e9abc271 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -31,7 +31,7 @@ from ironic_python_agent import utils LOG = log.getLogger(__name__) -BIND_MOUNTS = ('/dev', '/sys', '/proc') +BIND_MOUNTS = ('/dev', '/proc') def _get_partition(device, uuid): @@ -97,6 +97,8 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None): for fs in BIND_MOUNTS: utils.execute('mount', '-o', 'bind', fs, path + fs) + utils.execute('mount', '-t', 'sysfs', 'none', path + '/sys') + if efi_partition: if not os.path.exists(efi_partition_mount_point): os.makedirs(efi_partition_mount_point) @@ -157,6 +159,13 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None): umount_binds_fail = True LOG.warning(umount_warn_msg, {'path': path + fs, 'error': e}) + try: + utils.execute('umount', path + '/sys', attempts=3, + delay_on_retry=True) + except processutils.ProcessExecutionError as e: + umount_binds_fail = True + LOG.warning(umount_warn_msg, {'path': path + '/sys', 'error': e}) + # If umounting the binds succeed then we can try to delete it if not umount_binds_fail: try: diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 7d38e5ea1..20a64e4fa 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -79,10 +79,10 @@ class TestImageExtension(test_base.BaseTestCase): expected = [mock.call('mount', '/dev/fake2', self.fake_dir), mock.call('mount', '-o', 'bind', '/dev', self.fake_dir + '/dev'), - mock.call('mount', '-o', 'bind', '/sys', - self.fake_dir + '/sys'), mock.call('mount', '-o', 'bind', '/proc', self.fake_dir + '/proc'), + mock.call('mount', '-t', 'sysfs', 'none', + self.fake_dir + '/sys'), mock.call(('chroot %s /bin/bash -c ' '"/usr/sbin/grub-install %s"' % (self.fake_dir, self.fake_dev)), shell=True, @@ -94,10 +94,10 @@ class TestImageExtension(test_base.BaseTestCase): env_variables={'PATH': '/sbin:/bin'}), mock.call('umount', self.fake_dir + '/dev', attempts=3, delay_on_retry=True), - mock.call('umount', self.fake_dir + '/sys', - attempts=3, delay_on_retry=True), mock.call('umount', self.fake_dir + '/proc', attempts=3, delay_on_retry=True), + mock.call('umount', self.fake_dir + '/sys', + attempts=3, delay_on_retry=True), mock.call('umount', self.fake_dir, attempts=3, delay_on_retry=True)] mock_execute.assert_has_calls(expected) @@ -122,10 +122,10 @@ class TestImageExtension(test_base.BaseTestCase): expected = [mock.call('mount', '/dev/fake2', self.fake_dir), mock.call('mount', '-o', 'bind', '/dev', self.fake_dir + '/dev'), - mock.call('mount', '-o', 'bind', '/sys', - self.fake_dir + '/sys'), mock.call('mount', '-o', 'bind', '/proc', self.fake_dir + '/proc'), + mock.call('mount', '-t', 'sysfs', 'none', + self.fake_dir + '/sys'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), mock.call(('chroot %s /bin/bash -c ' @@ -141,10 +141,10 @@ class TestImageExtension(test_base.BaseTestCase): attempts=3, delay_on_retry=True), mock.call('umount', self.fake_dir + '/dev', attempts=3, delay_on_retry=True), - mock.call('umount', self.fake_dir + '/sys', - attempts=3, delay_on_retry=True), mock.call('umount', self.fake_dir + '/proc', attempts=3, delay_on_retry=True), + mock.call('umount', self.fake_dir + '/sys', + attempts=3, delay_on_retry=True), mock.call('umount', self.fake_dir, attempts=3, delay_on_retry=True)] mkdir_mock.assert_called_once_with(self.fake_dir + '/boot/efi') @@ -178,10 +178,10 @@ class TestImageExtension(test_base.BaseTestCase): expected = [mock.call('mount', '/dev/fake2', self.fake_dir), mock.call('mount', '-o', 'bind', '/dev', self.fake_dir + '/dev'), - mock.call('mount', '-o', 'bind', '/sys', - self.fake_dir + '/sys'), mock.call('mount', '-o', 'bind', '/proc', self.fake_dir + '/proc'), + mock.call('mount', '-t', 'sysfs', 'none', + self.fake_dir + '/sys'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), mock.call(('chroot %s /bin/bash -c '