From e74e783128daca56c05b7b6568e91cd6fc908f96 Mon Sep 17 00:00:00 2001 From: Dmitry Guryanov Date: Thu, 19 May 2016 19:02:23 +0300 Subject: [PATCH] change configdrive format to ConfigDrive version 2 We put configdrive with is9660 filesystem to a partition on a hard disk. New hard disks may have 4K sectors, but blocksize of iso9660 fs is 2K so it will not work. To fix this bug we should use another filesystem (ext2) and another config drive format (files, directory structure), because NoCloud format, which is currently used support only vfat and iso9660 filesystems. Change-Id: Ia0f244f19bab3dfaceef8a092ad03667675a5557 Closes-Bug: #1544818 (cherry picked from commit 8f5c03dcacfd20d278c4c1389762638fb14e1a02) --- cloud-init-templates/meta_data_json.jinja2 | 4 + fuel_agent/manager.py | 109 ++++++++----- fuel_agent/tests/test_fs_utils.py | 9 ++ fuel_agent/tests/test_manager.py | 168 ++++++++++++++------- fuel_agent/utils/fs.py | 6 + 5 files changed, 205 insertions(+), 91 deletions(-) create mode 100644 cloud-init-templates/meta_data_json.jinja2 diff --git a/cloud-init-templates/meta_data_json.jinja2 b/cloud-init-templates/meta_data_json.jinja2 new file mode 100644 index 00000000..14e8a50f --- /dev/null +++ b/cloud-init-templates/meta_data_json.jinja2 @@ -0,0 +1,4 @@ +{ + "hostname": "{{ common.hostname }}", + "uuid": "some-unused-id" +} diff --git a/fuel_agent/manager.py b/fuel_agent/manager.py index bb37777f..d114f990 100644 --- a/fuel_agent/manager.py +++ b/fuel_agent/manager.py @@ -16,6 +16,7 @@ from io import open import os import shutil import signal +import tempfile from oslo_config import cfg import six @@ -320,43 +321,78 @@ class Manager(object): if not found_images: fu.make_fs(fs.type, fs.options, fs.label, fs.device) + def _make_configdrive_image(self, src_files): + bs = 4096 + configdrive_device = self.driver.partition_scheme.configdrive_device() + size = utils.execute('blockdev', '--getsize64', configdrive_device)[0] + size = int(size.strip()) + + utils.execute('truncate', '--size=%d' % size, CONF.config_drive_path) + fu.make_fs( + fs_type='ext2', + fs_options=' -b %d -F ' % bs, + fs_label='config-2', + dev=six.text_type(CONF.config_drive_path)) + + mount_point = tempfile.mkdtemp(dir=CONF.tmp_path) + try: + fu.mount_fs('ext2', CONF.config_drive_path, mount_point) + for file_path in src_files: + name = os.path.basename(file_path) + if os.path.isdir(file_path): + shutil.copytree(file_path, os.path.join(mount_point, name)) + else: + shutil.copy2(file_path, mount_point) + except Exception as exc: + LOG.error('Error copying files to configdrive: %s', exc) + raise + finally: + fu.umount_fs(mount_point) + os.rmdir(mount_point) + + def _prepare_configdrive_files(self): + # see data sources part of cloud-init documentation + # for directory structure + cd_root = tempfile.mkdtemp(dir=CONF.tmp_path) + cd_latest = os.path.join(cd_root, 'openstack', 'latest') + md_output_path = os.path.join(cd_latest, 'meta_data.json') + ud_output_path = os.path.join(cd_latest, 'user_data') + os.makedirs(cd_latest) + + cc_output_path = os.path.join(CONF.tmp_path, 'cloud_config.txt') + bh_output_path = os.path.join(CONF.tmp_path, 'boothook.txt') + + tmpl_dir = CONF.nc_template_path + utils.render_and_save( + tmpl_dir, + self.driver.configdrive_scheme.template_names('cloud_config'), + self.driver.configdrive_scheme.template_data(), + cc_output_path + ) + utils.render_and_save( + tmpl_dir, + self.driver.configdrive_scheme.template_names('boothook'), + self.driver.configdrive_scheme.template_data(), + bh_output_path + ) + utils.render_and_save( + tmpl_dir, + self.driver.configdrive_scheme.template_names('meta_data_json'), + self.driver.configdrive_scheme.template_data(), + md_output_path + ) + + utils.execute( + 'write-mime-multipart', '--output=%s' % ud_output_path, + '%s:text/cloud-boothook' % bh_output_path, + '%s:text/cloud-config' % cc_output_path) + return [os.path.join(cd_root, 'openstack')] + def do_configdrive(self): LOG.debug('--- Creating configdrive (do_configdrive) ---') if CONF.prepare_configdrive: - cc_output_path = os.path.join(CONF.tmp_path, 'cloud_config.txt') - bh_output_path = os.path.join(CONF.tmp_path, 'boothook.txt') - # NOTE:file should be strictly named as 'user-data' - # the same is for meta-data as well - ud_output_path = os.path.join(CONF.tmp_path, 'user-data') - md_output_path = os.path.join(CONF.tmp_path, 'meta-data') - - tmpl_dir = CONF.nc_template_path - utils.render_and_save( - tmpl_dir, - self.driver.configdrive_scheme.template_names('cloud_config'), - self.driver.configdrive_scheme.template_data(), - cc_output_path - ) - utils.render_and_save( - tmpl_dir, - self.driver.configdrive_scheme.template_names('boothook'), - self.driver.configdrive_scheme.template_data(), - bh_output_path - ) - utils.render_and_save( - tmpl_dir, - self.driver.configdrive_scheme.template_names('meta_data'), - self.driver.configdrive_scheme.template_data(), - md_output_path - ) - - utils.execute( - 'write-mime-multipart', '--output=%s' % ud_output_path, - '%s:text/cloud-boothook' % bh_output_path, - '%s:text/cloud-config' % cc_output_path) - utils.execute('genisoimage', '-output', CONF.config_drive_path, - '-volid', 'cidata', '-joliet', '-rock', - ud_output_path, md_output_path) + files = self._prepare_configdrive_files() + self._make_configdrive_image(files) if CONF.prepare_configdrive or os.path.isfile(CONF.config_drive_path): self._add_configdrive_image() @@ -369,10 +405,13 @@ class Manager(object): 'configdrive device not found') size = os.path.getsize(CONF.config_drive_path) md5 = utils.calculate_md5(CONF.config_drive_path, size) + + fs_type = fu.get_fs_type(CONF.config_drive_path) + self.driver.image_scheme.add_image( uri='file://%s' % CONF.config_drive_path, target_device=configdrive_device, - format='iso9660', + format=fs_type, container='raw', size=size, md5=md5, diff --git a/fuel_agent/tests/test_fs_utils.py b/fuel_agent/tests/test_fs_utils.py index 5904c728..62dd3f1a 100644 --- a/fuel_agent/tests/test_fs_utils.py +++ b/fuel_agent/tests/test_fs_utils.py @@ -169,6 +169,15 @@ class TestFSUtils(unittest2.TestCase): mock_mkdtemp.assert_called_once_with(dir=None, suffix='') mock_mount.assert_called_once_with('ext4', '/dev/fake', '/tmp/dir') + def test_get_fs_type(self, mock_exec): + output = "megafs\n" + mock_exec.return_value = (output, '') + ret = fu.get_fs_type('/dev/sda4') + mock_exec.assert_called_once_with('blkid', '-o', 'value', + '-s', 'TYPE', '-c', '/dev/null', + '/dev/sda4') + self.assertEqual(ret, 'megafs') + class TestFSRetry(unittest2.TestCase): diff --git a/fuel_agent/tests/test_manager.py b/fuel_agent/tests/test_manager.py index 16528015..5b70750a 100644 --- a/fuel_agent/tests/test_manager.py +++ b/fuel_agent/tests/test_manager.py @@ -496,24 +496,18 @@ class TestManager(unittest2.TestCase): mock.call('xfs', '', '', '/dev/mapper/image-glance')] self.assertEqual(mock_fu_mf_expected_calls, mock_fu_mf.call_args_list) - @mock.patch('fuel_agent.drivers.nailgun.Nailgun.parse_image_meta', - return_value={}) - @mock.patch('fuel_agent.drivers.nailgun.Nailgun.parse_operating_system') - @mock.patch.object(utils, 'calculate_md5') - @mock.patch('os.path.getsize') - @mock.patch('yaml.load') - @mock.patch.object(utils, 'init_http_request') + @mock.patch('tempfile.mkdtemp') + @mock.patch('os.makedirs') @mock.patch.object(utils, 'execute') @mock.patch.object(utils, 'render_and_save') - @mock.patch.object(hu, 'list_block_devices') - def test_do_configdrive(self, mock_lbd, mock_u_ras, mock_u_e, - mock_http_req, mock_yaml, mock_get_size, mock_md5, - mock_parse_os, mock_image_meta): - mock_get_size.return_value = 123 - mock_md5.return_value = 'fakemd5' - mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE - self.assertEqual(1, len(self.mgr.driver.image_scheme.images)) - self.mgr.do_configdrive() + def test_prepare_configdrive_files(self, mock_u_ras, mock_u_e, + mock_makedirs, mock_mkdtemp): + mock_mkdtemp.return_value = '/tmp/qwe' + ret = self.mgr._prepare_configdrive_files() + self.assertEqual(ret, ['/tmp/qwe/openstack']) + mock_mkdtemp.assert_called_once_with(dir=CONF.tmp_path) + mock_makedirs.assert_called_once_with('/tmp/qwe/openstack/latest') + mock_u_ras_expected_calls = [ mock.call(CONF.nc_template_path, ['cloud_config_pro_fi-le.jinja2', @@ -528,50 +522,103 @@ class TestManager(unittest2.TestCase): 'boothook.jinja2'], mock.ANY, '%s/%s' % (CONF.tmp_path, 'boothook.txt')), mock.call(CONF.nc_template_path, - ['meta_data_pro_fi-le.jinja2', - 'meta_data_pro.jinja2', - 'meta_data_pro_fi.jinja2', - 'meta_data.jinja2'], - mock.ANY, '%s/%s' % (CONF.tmp_path, 'meta-data'))] + ['meta_data_json_pro_fi-le.jinja2', + 'meta_data_json_pro.jinja2', + 'meta_data_json_pro_fi.jinja2', + 'meta_data_json.jinja2'], + mock.ANY, '/tmp/qwe/openstack/latest/meta_data.json')] self.assertEqual(mock_u_ras_expected_calls, mock_u_ras.call_args_list) - mock_u_e_expected_calls = [ - mock.call('write-mime-multipart', - '--output=%s' % ('%s/%s' % (CONF.tmp_path, 'user-data')), - '%s:text/cloud-boothook' % ('%s/%s' % (CONF.tmp_path, - 'boothook.txt')), - '%s:text/cloud-config' % ('%s/%s' % (CONF.tmp_path, - 'cloud_config.txt')) - ), - mock.call('genisoimage', '-output', CONF.config_drive_path, - '-volid', 'cidata', '-joliet', '-rock', - '%s/%s' % (CONF.tmp_path, 'user-data'), - '%s/%s' % (CONF.tmp_path, 'meta-data'))] - self.assertEqual(mock_u_e_expected_calls, mock_u_e.call_args_list) + mock_u_e.assert_called_once_with( + 'write-mime-multipart', + '--output=/tmp/qwe/openstack/latest/user_data', + '%s/%s:text/cloud-boothook' % (CONF.tmp_path, 'boothook.txt'), + '%s/%s:text/cloud-config' % (CONF.tmp_path, 'cloud_config.txt')) + + @mock.patch('fuel_agent.manager.fu', create=True) + @mock.patch('os.path.isdir') + @mock.patch('os.rmdir') + @mock.patch('shutil.copy2') + @mock.patch('shutil.copytree') + @mock.patch('tempfile.mkdtemp') + @mock.patch.object(hu, 'list_block_devices') + @mock.patch.object(utils, 'execute') + def test_make_configdrive_image(self, mock_u_e, mock_lbd, mock_mkdtemp, + mock_copytree, mock_copy2, mock_rmdir, + mock_isdir, mock_fu): + mock_u_e.side_effect = [(' 795648', ''), None] + mock_isdir.side_effect = [True, False] + mock_mkdtemp.return_value = '/tmp/mount_point' + mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE + + self.mgr._make_configdrive_image(['/tmp/openstack', '/tmp/somefile']) + + mock_u_e_calls = [ + mock.call('blockdev', '--getsize64', '/dev/sda7'), + mock.call('truncate', '--size=795648', CONF.config_drive_path)] + + self.assertEqual(mock_u_e_calls, mock_u_e.call_args_list, + str(mock_u_e.call_args_list)) + + mock_fu.make_fs.assert_called_with(fs_type='ext2', + fs_options=' -b 4096 -F ', + fs_label='config-2', + dev=CONF.config_drive_path) + mock_fu.mount_fs.assert_called_with('ext2', + CONF.config_drive_path, + '/tmp/mount_point') + mock_fu.umount_fs.assert_called_with('/tmp/mount_point') + mock_rmdir.assert_called_with('/tmp/mount_point') + mock_copy2.assert_called_with('/tmp/somefile', '/tmp/mount_point') + mock_copytree.assert_called_with('/tmp/openstack', + '/tmp/mount_point/openstack') + + @mock.patch.object(fu, 'get_fs_type') + @mock.patch.object(utils, 'calculate_md5') + @mock.patch('os.path.getsize') + @mock.patch.object(hu, 'list_block_devices') + def test_add_configdrive_image(self, mock_lbd, mock_getsize, + mock_calc_md5, mock_get_fs_type): + mock_get_fs_type.return_value = 'ext999' + mock_calc_md5.return_value = 'fakemd5' + mock_getsize.return_value = 123 + self.mgr._add_configdrive_image() + self.assertEqual(2, len(self.mgr.driver.image_scheme.images)) cf_drv_img = self.mgr.driver.image_scheme.images[-1] self.assertEqual('file://%s' % CONF.config_drive_path, cf_drv_img.uri) - self.assertEqual('/dev/sda7', - self.mgr.driver.partition_scheme.configdrive_device()) - self.assertEqual('iso9660', cf_drv_img.format) + self.assertEqual('/dev/sda7', cf_drv_img.target_device) + self.assertEqual('ext999', cf_drv_img.format) self.assertEqual('raw', cf_drv_img.container) self.assertEqual('fakemd5', cf_drv_img.md5) self.assertEqual(123, cf_drv_img.size) - @mock.patch('yaml.load') - @mock.patch.object(utils, 'init_http_request') @mock.patch.object(objects.PartitionScheme, 'configdrive_device') - @mock.patch.object(utils, 'execute') - @mock.patch.object(utils, 'render_and_save') + @mock.patch.object(utils, 'calculate_md5') + @mock.patch('os.path.getsize') @mock.patch.object(hu, 'list_block_devices') - def test_do_configdrive_no_configdrive_device(self, mock_lbd, mock_u_ras, - mock_u_e, mock_p_ps_cd, - mock_http_req, mock_yaml): - mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE + def test_add_configdrive_image_no_configdrive_device(self, mock_lbd, + mock_getsize, + mock_calc_md5, + mock_p_ps_cd): + mock_calc_md5.return_value = 'fakemd5' + mock_getsize.return_value = 123 mock_p_ps_cd.return_value = None self.assertRaises(errors.WrongPartitionSchemeError, - self.mgr.do_configdrive) + self.mgr._add_configdrive_image) + def test_do_configdrive(self): + with mock.patch.multiple(self.mgr, + _prepare_configdrive_files=mock.DEFAULT, + _make_configdrive_image=mock.DEFAULT, + _add_configdrive_image=mock.DEFAULT) as mocks: + mocks['_prepare_configdrive_files'].return_value = 'x' + self.mgr.do_configdrive() + mocks['_prepare_configdrive_files'].assert_called_once_with() + mocks['_make_configdrive_image'].assert_called_once_with('x') + mocks['_add_configdrive_image'].assert_called_once_with() + + @mock.patch.object(fu, 'get_fs_type') @mock.patch('fuel_agent.manager.Manager.move_files_to_their_places') @mock.patch.object(manager.os.path, 'exists') @mock.patch.object(hu, 'is_block_device') @@ -590,12 +637,13 @@ class TestManager(unittest2.TestCase): def test_do_copyimage(self, mock_lbd, mock_u_ras, mock_u_e, mock_au_c, mock_au_h, mock_au_l, mock_au_g, mock_fu_ef, mock_http_req, mock_yaml, mock_get_size, mock_md5, - mock_ibd, mock_os_path, mock_mfttp): + mock_ibd, mock_os_path, mock_mfttp, + mock_get_fs_type): mock_os_path.return_value = True mock_ibd.return_value = True mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE mock_au_c.return_value = FakeChain() - self.mgr.do_configdrive() + self.mgr._add_configdrive_image() self.mgr.do_copyimage() imgs = self.mgr.driver.image_scheme.images self.assertEqual(2, len(imgs)) @@ -619,6 +667,7 @@ class TestManager(unittest2.TestCase): self.assertEqual(mock_fu_ef_expected_calls, mock_fu_ef.call_args_list) self.assertTrue(mock_mfttp.called) + @mock.patch.object(fu, 'get_fs_type') @mock.patch.object(manager.os.path, 'exists') @mock.patch.object(hu, 'is_block_device') @mock.patch.object(utils, 'calculate_md5') @@ -638,16 +687,18 @@ class TestManager(unittest2.TestCase): mock_au_l, mock_au_g, mock_fu_ef, mock_http_req, mock_yaml, mock_get_size, mock_md5, - mock_ibd, mock_os_path): + mock_ibd, mock_os_path, + mock_get_fs_type): mock_os_path.return_value = False mock_ibd.return_value = True mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE mock_au_c.return_value = FakeChain() - self.mgr.do_configdrive() + self.mgr._add_configdrive_image() with self.assertRaisesRegexp(errors.WrongDeviceError, 'TARGET processor .* does not exist'): self.mgr.do_copyimage() + @mock.patch.object(fu, 'get_fs_type') @mock.patch.object(manager.os.path, 'exists') @mock.patch.object(hu, 'is_block_device') @mock.patch.object(utils, 'calculate_md5') @@ -668,16 +719,18 @@ class TestManager(unittest2.TestCase): mock_au_g, mock_fu_ef, mock_http_req, mock_yaml, mock_get_size, mock_md5, - mock_ibd, mock_os_path): + mock_ibd, mock_os_path, + mock_get_fs_type): mock_os_path.return_value = True mock_ibd.return_value = False mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE mock_au_c.return_value = FakeChain() - self.mgr.do_configdrive() + self.mgr._add_configdrive_image() msg = 'TARGET processor .* is not a block device' with self.assertRaisesRegexp(errors.WrongDeviceError, msg): self.mgr.do_copyimage() + @mock.patch.object(fu, 'get_fs_type') @mock.patch('fuel_agent.manager.Manager.move_files_to_their_places') @mock.patch.object(manager.os.path, 'exists') @mock.patch.object(hu, 'is_block_device') @@ -697,7 +750,8 @@ class TestManager(unittest2.TestCase): mock_au_c, mock_au_h, mock_au_l, mock_au_g, mock_fu_ef, mock_http_req, mock_yaml, mock_get_size, mock_md5, - mock_ibd, mock_os_path, mock_mfttp): + mock_ibd, mock_os_path, mock_mfttp, + mock_get_fs_type): mock_os_path.return_value = True mock_ibd.return_value = True mock_get_size.return_value = 123 @@ -706,7 +760,7 @@ class TestManager(unittest2.TestCase): mock_au_c.return_value = FakeChain() self.mgr.driver.image_scheme.images[0].size = 1234 self.mgr.driver.image_scheme.images[0].md5 = 'really_fakemd5' - self.mgr.do_configdrive() + self.mgr._add_configdrive_image() self.assertEqual(2, len(self.mgr.driver.image_scheme.images)) self.mgr.do_copyimage() expected_md5_calls = [mock.call('/tmp/config-drive.img', 123), @@ -715,6 +769,7 @@ class TestManager(unittest2.TestCase): self.assertEqual(expected_md5_calls, mock_md5.call_args_list) self.assertTrue(mock_mfttp.called) + @mock.patch.object(fu, 'get_fs_type') @mock.patch.object(hu, 'is_block_device') @mock.patch.object(manager.os.path, 'exists') @mock.patch.object(utils, 'calculate_md5') @@ -733,7 +788,8 @@ class TestManager(unittest2.TestCase): mock_au_c, mock_au_h, mock_au_l, mock_au_g, mock_fu_ef, mock_http_req, mock_yaml, mock_get_size, mock_md5, - mock_os_path, mock_ibd): + mock_os_path, mock_ibd, + mock_get_fs_type): mock_os_path.return_value = True mock_ibd.return_value = True mock_get_size.return_value = 123 @@ -742,7 +798,7 @@ class TestManager(unittest2.TestCase): mock_au_c.return_value = FakeChain() self.mgr.driver.image_scheme.images[0].size = 1234 self.mgr.driver.image_scheme.images[0].md5 = 'fakemd5' - self.mgr.do_configdrive() + self.mgr._add_configdrive_image() self.assertEqual(2, len(self.mgr.driver.image_scheme.images)) self.assertRaises(errors.ImageChecksumMismatchError, self.mgr.do_copyimage) diff --git a/fuel_agent/utils/fs.py b/fuel_agent/utils/fs.py index d83b8596..efb2a4a2 100644 --- a/fuel_agent/utils/fs.py +++ b/fuel_agent/utils/fs.py @@ -126,3 +126,9 @@ def mount_fs_temp(fs_type, fs_dev, tmpdir=None, suffix=''): mount_point = tempfile.mkdtemp(dir=tmpdir, suffix=suffix) mount_fs(fs_type, fs_dev, mount_point) return mount_point + + +def get_fs_type(device): + output = utils.execute('blkid', '-o', 'value', '-s', 'TYPE', + '-c', '/dev/null', device)[0] + return output.strip()