From 9e4b5ad8dc83e4d48359f6c165a5cc85f2916eac Mon Sep 17 00:00:00 2001 From: Andrey Tykhonov Date: Tue, 20 Oct 2015 18:00:38 +0300 Subject: [PATCH] Make several attempts to attach image file If one starts 2 or more instances of fuel-agent simultaneously, then fuel-agent could fail on loop device allocation. With this patch it makes several attempts to attach temporary image file to loop device. Change-Id: I502eb07a69a2d813157d7511fc03032671e98196 Closes-Bug: #1506071 --- fuel_agent/manager.py | 50 ++++++++++++++++------------ fuel_agent/objects/image.py | 1 + fuel_agent/tests/test_build_utils.py | 44 ++++++++++++++++++++++++ fuel_agent/tests/test_manager.py | 22 +++++++----- fuel_agent/utils/build.py | 42 +++++++++++++++++++++++ 5 files changed, 129 insertions(+), 30 deletions(-) diff --git a/fuel_agent/manager.py b/fuel_agent/manager.py index 10cdd8b..fc2aa69 100644 --- a/fuel_agent/manager.py +++ b/fuel_agent/manager.py @@ -88,6 +88,11 @@ opts = [ # isn't theoretically present anymore. help='Maximum allowed loop devices count to use' ), + cfg.IntOpt( + 'max_allowed_attempts_attach_image', + default=10, + help='Maximum allowed attempts to attach image file to loop device' + ), cfg.IntOpt( 'sparse_file_size', # XXX: Apparently Fuel configures the node root filesystem to span @@ -608,13 +613,12 @@ class Manager(object): # to be able to shrink them and move in the end image.img_tmp_file = img_tmp_file - LOG.debug('Looking for a free loop device') - image.target_device.name = bu.get_free_loop_device( - loop_device_major_number=CONF.loop_device_major_number, - max_loop_devices_count=CONF.max_loop_devices_count) - - LOG.debug('Attaching temporary image file to free loop device') - bu.attach_file_to_loop(img_tmp_file, str(image.target_device)) + image.target_device.name = \ + bu.attach_file_to_free_loop_device( + img_tmp_file, + max_loop_devices_count=CONF.max_loop_devices_count, + loop_device_major_number=CONF.loop_device_major_number, + max_attempts=CONF.max_allowed_attempts_attach_image) # find fs with the same loop device object # as image.target_device @@ -794,22 +798,24 @@ class Manager(object): LOG.debug('Finally: umounting chroot tree %s', chroot) self.umount_target(chroot, pseudo=False) for image in self.driver.image_scheme.images: - LOG.debug('Finally: detaching loop device: %s', - str(image.target_device)) - try: - bu.deattach_loop(str(image.target_device)) - except errors.ProcessExecutionError as e: - LOG.warning('Error occured while trying to detach ' - 'loop device %s. Error message: %s', - str(image.target_device), e) + if image.target_device.name: + LOG.debug('Finally: detaching loop device: %s', + image.target_device.name) + try: + bu.deattach_loop(image.target_device.name) + except errors.ProcessExecutionError as e: + LOG.warning('Error occured while trying to detach ' + 'loop device %s. Error message: %s', + image.target_device.name, e) - LOG.debug('Finally: removing temporary file: %s', - image.img_tmp_file) - try: - os.unlink(image.img_tmp_file) - except OSError: - LOG.debug('Finally: file %s seems does not exist ' - 'or can not be removed', image.img_tmp_file) + if image.img_tmp_file: + LOG.debug('Finally: removing temporary file: %s', + image.img_tmp_file) + try: + os.unlink(image.img_tmp_file) + except OSError: + LOG.debug('Finally: file %s seems does not exist ' + 'or can not be removed', image.img_tmp_file) LOG.debug('Finally: removing chroot directory: %s', chroot) try: os.rmdir(chroot) diff --git a/fuel_agent/objects/image.py b/fuel_agent/objects/image.py index 583b032..a143dce 100644 --- a/fuel_agent/objects/image.py +++ b/fuel_agent/objects/image.py @@ -34,6 +34,7 @@ class Image(object): self.container = container self.size = size self.md5 = md5 + self.img_tmp_file = None class ImageScheme(object): diff --git a/fuel_agent/tests/test_build_utils.py b/fuel_agent/tests/test_build_utils.py index 6cb5b0d..00dab13 100644 --- a/fuel_agent/tests/test_build_utils.py +++ b/fuel_agent/tests/test_build_utils.py @@ -502,3 +502,47 @@ class BuildUtilsTestCase(unittest2.TestCase): def test_containerize_bad_container(self): self.assertRaises(errors.WrongImageDataError, bu.containerize, 'file', 'fake') + + @mock.patch('fuel_agent.utils.build.get_free_loop_device') + @mock.patch('fuel_agent.utils.build.attach_file_to_loop') + def test_do_build_image_retries_attach_image_max_attempts_exceeded( + self, mock_attach_file, mock_get_free_loop_device): + + mock_attach_file.side_effect = errors.ProcessExecutionError() + + with self.assertRaises(errors.NoFreeLoopDevices): + bu.attach_file_to_free_loop_device( + mock.sentinel, max_loop_devices_count=255, + loop_device_major_number=7, max_attempts=3) + + self.assertEqual(mock_attach_file.call_count, 3) + + @mock.patch('fuel_agent.utils.build.get_free_loop_device') + @mock.patch('fuel_agent.utils.build.attach_file_to_loop') + def test_do_build_image_retries_attach_image( + self, mock_attach_file, mock_get_free_loop_device): + + mock_attach_file.side_effect = \ + [errors.ProcessExecutionError(), + errors.ProcessExecutionError(), + True] + free_loop_device = '/dev/loop0' + mock_get_free_loop_device.return_value = free_loop_device + loop_device_major_number = 7 + max_loop_devices_count = 255 + max_attempts = 3 + filename = mock.sentinel + + loop_device = bu.attach_file_to_free_loop_device( + filename, max_loop_devices_count=max_loop_devices_count, + loop_device_major_number=loop_device_major_number, + max_attempts=max_attempts) + + self.assertEqual(free_loop_device, loop_device) + self.assertEqual( + [mock.call(loop_device_major_number=loop_device_major_number, + max_loop_devices_count=max_loop_devices_count)] * 3, + mock_get_free_loop_device.call_args_list) + self.assertEqual( + [mock.call(filename, '/dev/loop0')] * 3, + mock_attach_file.call_args_list) diff --git a/fuel_agent/tests/test_manager.py b/fuel_agent/tests/test_manager.py index cb84585..01b0dbb 100644 --- a/fuel_agent/tests/test_manager.py +++ b/fuel_agent/tests/test_manager.py @@ -846,7 +846,8 @@ class TestImageBuild(unittest2.TestCase): mock_os.path.basename.side_effect = ['img.img.gz', 'img-boot.img.gz'] mock_bu.create_sparse_tmp_file.side_effect = \ ['/tmp/img', '/tmp/img-boot'] - mock_bu.get_free_loop_device.side_effect = ['/dev/loop0', '/dev/loop1'] + mock_bu.attach_file_to_free_loop_device.side_effect = [ + '/dev/loop0', '/dev/loop1'] mock_mkdtemp.return_value = '/tmp/imgdir' getsize_side = [20, 2, 10, 1] mock_os.path.getsize.side_effect = getsize_side @@ -867,13 +868,18 @@ class TestImageBuild(unittest2.TestCase): size=CONF.sparse_file_size)] * 2, mock_bu.create_sparse_tmp_file.call_args_list) self.assertEqual( - [mock.call(loop_device_major_number=CONF.loop_device_major_number, - max_loop_devices_count=CONF.max_loop_devices_count), - ] * 2, - mock_bu.get_free_loop_device.call_args_list) - self.assertEqual([mock.call('/tmp/img', '/dev/loop0'), - mock.call('/tmp/img-boot', '/dev/loop1')], - mock_bu.attach_file_to_loop.call_args_list) + [mock.call( + '/tmp/img', + loop_device_major_number=CONF.loop_device_major_number, + max_loop_devices_count=CONF.max_loop_devices_count, + max_attempts=CONF.max_allowed_attempts_attach_image), + mock.call( + '/tmp/img-boot', + loop_device_major_number=CONF.loop_device_major_number, + max_loop_devices_count=CONF.max_loop_devices_count, + max_attempts=CONF.max_allowed_attempts_attach_image) + ], + mock_bu.attach_file_to_free_loop_device.call_args_list) self.assertEqual([mock.call(fs_type='ext4', fs_options='', fs_label='', dev='/dev/loop0'), mock.call(fs_type='ext2', fs_options='', diff --git a/fuel_agent/utils/build.py b/fuel_agent/utils/build.py index 8535394..728b646 100644 --- a/fuel_agent/utils/build.py +++ b/fuel_agent/utils/build.py @@ -455,3 +455,45 @@ def containerize(filename, container, chunk_size=1048576): raise errors.WrongImageDataError( 'Error while image initialization: ' 'unsupported image container: {container}'.format(container=container)) + + +def attach_file_to_free_loop_device(filename, max_loop_devices_count=255, + loop_device_major_number=7, + max_attempts=1): + """Find free loop device and try to attach `filename` to it. + + If attaching fails then retry again. Max allowed attempts is + `max_attempts`. + + Returns loop device to which file is attached. Otherwise, raises + errors.NoFreeLoopDevices. + """ + loop_device = None + for i in range(0, max_attempts): + try: + LOG.debug('Looking for a free loop device') + loop_device = get_free_loop_device( + loop_device_major_number=loop_device_major_number, + max_loop_devices_count=max_loop_devices_count) + + log_msg = "Attaching image file '{0}' to free loop device '{1}'" + LOG.debug(log_msg.format(filename, loop_device)) + attach_file_to_loop(filename, loop_device) + break + except errors.ProcessExecutionError: + log_msg = "Couldn't attach image file '{0}' to loop device '{1}'." + LOG.debug(log_msg.format(filename, loop_device)) + + if i == max_attempts - 1: + log_msg = ("Maximum allowed attempts ({0}) to attach image " + "file '{1}' to loop device '{2}' is exceeded.") + LOG.debug(log_msg.format(max_attempts, filename, loop_device)) + raise errors.NoFreeLoopDevices('Free loop device not found.') + else: + log_msg = ("Trying again to attach image file '{0}' " + "to free loop device '{1}'. " + "Attempt #{2} out of {3}") + LOG.debug(log_msg.format(filename, loop_device, + i + 1, max_attempts)) + + return loop_device