diff --git a/fuel_agent/drivers/nailgun.py b/fuel_agent/drivers/nailgun.py index 060be84..bf21705 100644 --- a/fuel_agent/drivers/nailgun.py +++ b/fuel_agent/drivers/nailgun.py @@ -19,6 +19,9 @@ from fuel_agent import errors from fuel_agent import objects from fuel_agent.openstack.common import log as logging from fuel_agent.utils import hardware_utils as hu +from fuel_agent.utils import utils + +import yaml LOG = logging.getLogger(__name__) @@ -350,6 +353,28 @@ class Nailgun(object): LOG.debug('--- Preparing image scheme ---') data = self.data image_scheme = objects.ImageScheme() + #FIXME(agordeev): this piece of code for fetching additional image + # meta data should be factored out of this particular nailgun driver + # into more common and absract data getter which should be able to deal + # with various data sources (local file, http(s), etc.) and different + # data formats ('blob', json, yaml, etc.). + # So, the manager will combine and manipulate all those multiple data + # getter instances. + # Also, the initial data source should be set to sort out chicken/egg + # problem. Command line option may be useful for such a case. + # BUG: https://bugs.launchpad.net/fuel/+bug/1430418 + metadata_url = data['ks_meta']['image_data']['/']['uri'].\ + split('.')[0] + '.yaml' + image_meta = {} + raw_image_meta = None + try: + raw_image_meta = yaml.load( + utils.init_http_request(metadata_url).text) + except Exception as e: + LOG.exception(e) + LOG.debug('Failed to fetch/decode image meta data') + if raw_image_meta: + [image_meta.update(img_info) for img_info in raw_image_meta] # We assume for every file system user may provide a separate # file system image. For example if partitioning scheme has # /, /boot, /var/lib file systems then we will try to get images @@ -372,5 +397,7 @@ class Nailgun(object): # from provision.json, but currently it is hard coded. format=image_data['format'], container=image_data['container'], + size=image_meta.get(fs.mount, {}).get('size'), + md5=image_meta.get(fs.mount, {}).get('md5'), ) return image_scheme diff --git a/fuel_agent/errors.py b/fuel_agent/errors.py index aec2182..4b15c73 100644 --- a/fuel_agent/errors.py +++ b/fuel_agent/errors.py @@ -142,3 +142,7 @@ class HttpUrlConnectionError(BaseError): class HttpUrlInvalidContentLength(BaseError): pass + + +class ImageChecksumMismatchError(BaseError): + pass diff --git a/fuel_agent/manager.py b/fuel_agent/manager.py index 0da626d..b564b3d 100644 --- a/fuel_agent/manager.py +++ b/fuel_agent/manager.py @@ -242,11 +242,15 @@ class Manager(object): raise errors.WrongPartitionSchemeError( 'Error while trying to get configdrive device: ' 'configdrive device not found') + size = os.path.getsize(CONF.config_drive_path) + md5 = utils.calculate_md5(CONF.config_drive_path, size) self.image_scheme.add_image( uri='file://%s' % CONF.config_drive_path, target_device=configdrive_device, format='iso9660', - container='raw' + container='raw', + size=size, + md5=md5, ) def do_copyimage(self): @@ -275,6 +279,22 @@ class Manager(object): LOG.debug('Launching image processing chain') processing.process() + if image.size and image.md5: + LOG.debug('Trying to compare image checksum') + actual_md5 = utils.calculate_md5(image.target_device, + image.size) + if actual_md5 == image.md5: + LOG.debug('Checksum matches successfully: md5=%s' % + actual_md5) + else: + raise errors.ImageChecksumMismatchError( + 'Actual checksum %s mismatches with expected %s for ' + 'file %s' % (actual_md5, image.md5, + image.target_device)) + else: + LOG.debug('Skipping image checksum comparing. ' + 'Ether size or hash have been missed') + LOG.debug('Extending image file systems') if image.format in ('ext2', 'ext3', 'ext4', 'xfs'): LOG.debug('Extending %s %s' % diff --git a/fuel_agent/objects/image.py b/fuel_agent/objects/image.py index 581dc9e..583b032 100644 --- a/fuel_agent/objects/image.py +++ b/fuel_agent/objects/image.py @@ -19,7 +19,7 @@ class Image(object): SUPPORTED_CONTAINERS = ['raw', 'gzip'] def __init__(self, uri, target_device, - format, container, size=None): + format, container, size=None, md5=None): # uri is something like # http://host:port/path/to/image.img or # file:///tmp/image.img @@ -33,6 +33,7 @@ class Image(object): 'unsupported image container') self.container = container self.size = size + self.md5 = md5 class ImageScheme(object): diff --git a/fuel_agent/tests/test_artifact_utils.py b/fuel_agent/tests/test_artifact_utils.py index 29af053..a93f66e 100644 --- a/fuel_agent/tests/test_artifact_utils.py +++ b/fuel_agent/tests/test_artifact_utils.py @@ -34,8 +34,9 @@ class TestTarget(test_base.BaseTestCase): def test_target_next(self): self.assertRaises(StopIteration, self.tgt.next) + @mock.patch('os.fsync') @mock.patch.object(au.Target, '__iter__') - def test_target_target(self, mock_iter): + def test_target_target(self, mock_iter, mock_os_sync): mock_iter.return_value = iter(['chunk1', 'chunk2', 'chunk3']) m = mock.mock_open() with mock.patch('six.moves.builtins.open', m): diff --git a/fuel_agent/tests/test_manager.py b/fuel_agent/tests/test_manager.py index 2cad03d..4e498ac 100644 --- a/fuel_agent/tests/test_manager.py +++ b/fuel_agent/tests/test_manager.py @@ -38,8 +38,10 @@ class TestManager(test_base.BaseTestCase): super(TestManager, self).setUp() self.mgr = manager.Manager(test_nailgun.PROVISION_SAMPLE_DATA) + @mock.patch('yaml.load') + @mock.patch.object(utils, 'init_http_request') @mock.patch.object(hu, 'list_block_devices') - def test_do_parsing(self, mock_lbd): + def test_do_parsing(self, mock_lbd, mock_http_req, mock_yaml): mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE self.mgr.do_parsing() #NOTE(agordeev): there's no need for deeper assertions as all schemes @@ -139,10 +141,17 @@ class TestManager(test_base.BaseTestCase): mock.call('xfs', '', '', '/dev/mapper/image-glance')] self.assertEqual(mock_fu_mf_expected_calls, mock_fu_mf.call_args_list) + @mock.patch.object(utils, 'calculate_md5') + @mock.patch('os.path.getsize') + @mock.patch('yaml.load') + @mock.patch.object(utils, 'init_http_request') @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): + def test_do_configdrive(self, mock_lbd, mock_u_ras, mock_u_e, + mock_http_req, mock_yaml, mock_get_size, mock_md5): + mock_get_size.return_value = 123 + mock_md5.return_value = 'fakemd5' mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE self.mgr.do_parsing() self.assertEqual(1, len(self.mgr.image_scheme.images)) @@ -188,19 +197,28 @@ class TestManager(test_base.BaseTestCase): self.mgr.partition_scheme.configdrive_device()) self.assertEqual('iso9660', 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(partition.PartitionScheme, 'configdrive_device') @mock.patch.object(utils, 'execute') @mock.patch.object(utils, 'render_and_save') @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_u_e, mock_p_ps_cd, + mock_http_req, mock_yaml): mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE self.mgr.do_parsing() mock_p_ps_cd.return_value = None self.assertRaises(errors.WrongPartitionSchemeError, self.mgr.do_configdrive) + @mock.patch.object(utils, 'calculate_md5') + @mock.patch('os.path.getsize') + @mock.patch('yaml.load') + @mock.patch.object(utils, 'init_http_request') @mock.patch.object(fu, 'extend_fs') @mock.patch.object(au, 'GunzipStream') @mock.patch.object(au, 'LocalFile') @@ -210,7 +228,8 @@ class TestManager(test_base.BaseTestCase): @mock.patch.object(utils, 'render_and_save') @mock.patch.object(hu, 'list_block_devices') 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_au_h, mock_au_l, mock_au_g, mock_fu_ef, + mock_http_req, mock_yaml, mock_get_size, mock_md5): class FakeChain(object): processors = [] @@ -246,3 +265,82 @@ class TestManager(test_base.BaseTestCase): mock_fu_ef_expected_calls = [ mock.call('ext4', '/dev/mapper/os-root')] self.assertEqual(mock_fu_ef_expected_calls, mock_fu_ef.call_args_list) + + @mock.patch.object(utils, 'calculate_md5') + @mock.patch('os.path.getsize') + @mock.patch('yaml.load') + @mock.patch.object(utils, 'init_http_request') + @mock.patch.object(fu, 'extend_fs') + @mock.patch.object(au, 'GunzipStream') + @mock.patch.object(au, 'LocalFile') + @mock.patch.object(au, 'HttpUrl') + @mock.patch.object(au, 'Chain') + @mock.patch.object(utils, 'execute') + @mock.patch.object(utils, 'render_and_save') + @mock.patch.object(hu, 'list_block_devices') + def test_do_copyimage_md5_matches(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): + + class FakeChain(object): + processors = [] + + def append(self, thing): + self.processors.append(thing) + + def process(self): + pass + + mock_get_size.return_value = 123 + mock_md5.side_effect = ['fakemd5', 'really_fakemd5', 'fakemd5'] + mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE + mock_au_c.return_value = FakeChain() + self.mgr.do_parsing() + self.mgr.image_scheme.images[0].size = 1234 + self.mgr.image_scheme.images[0].md5 = 'really_fakemd5' + self.mgr.do_configdrive() + self.assertEqual(2, len(self.mgr.image_scheme.images)) + self.mgr.do_copyimage() + expected_md5_calls = [mock.call('/tmp/config-drive.img', 123), + mock.call('/dev/mapper/os-root', 1234), + mock.call('/dev/sda7', 123)] + self.assertEqual(expected_md5_calls, mock_md5.call_args_list) + + @mock.patch.object(utils, 'calculate_md5') + @mock.patch('os.path.getsize') + @mock.patch('yaml.load') + @mock.patch.object(utils, 'init_http_request') + @mock.patch.object(fu, 'extend_fs') + @mock.patch.object(au, 'GunzipStream') + @mock.patch.object(au, 'LocalFile') + @mock.patch.object(au, 'HttpUrl') + @mock.patch.object(au, 'Chain') + @mock.patch.object(utils, 'execute') + @mock.patch.object(utils, 'render_and_save') + @mock.patch.object(hu, 'list_block_devices') + def test_do_copyimage_md5_mismatch(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): + + class FakeChain(object): + processors = [] + + def append(self, thing): + self.processors.append(thing) + + def process(self): + pass + + mock_get_size.return_value = 123 + mock_md5.side_effect = ['fakemd5', 'really_fakemd5', 'fakemd5'] + mock_lbd.return_value = test_nailgun.LIST_BLOCK_DEVICES_SAMPLE + mock_au_c.return_value = FakeChain() + self.mgr.do_parsing() + self.mgr.image_scheme.images[0].size = 1234 + self.mgr.image_scheme.images[0].md5 = 'fakemd5' + self.mgr.do_configdrive() + self.assertEqual(2, len(self.mgr.image_scheme.images)) + self.assertRaises(errors.ImageChecksumMismatchError, + self.mgr.do_copyimage) diff --git a/fuel_agent/tests/test_nailgun.py b/fuel_agent/tests/test_nailgun.py index b8b79c2..6da5a6e 100644 --- a/fuel_agent/tests/test_nailgun.py +++ b/fuel_agent/tests/test_nailgun.py @@ -15,10 +15,13 @@ import mock from oslotest import base as test_base +import yaml + from fuel_agent.drivers import nailgun from fuel_agent import errors from fuel_agent.objects import image from fuel_agent.utils import hardware_utils as hu +from fuel_agent.utils import utils CEPH_JOURNAL = { @@ -576,8 +579,10 @@ class TestNailgun(test_base.BaseTestCase): self.assertEqual(1, len(p_scheme.mds)) self.assertEqual(3, len(p_scheme.parteds)) + @mock.patch('yaml.load') + @mock.patch.object(utils, 'init_http_request') @mock.patch.object(hu, 'list_block_devices') - def test_image_scheme(self, mock_lbd): + def test_image_scheme(self, mock_lbd, mock_http_req, mock_yaml): mock_lbd.return_value = LIST_BLOCK_DEVICES_SAMPLE p_scheme = self.drv.partition_scheme() i_scheme = self.drv.image_scheme(p_scheme) @@ -601,6 +606,40 @@ class TestNailgun(test_base.BaseTestCase): expected_images[i].format) self.assertEqual(img.container, expected_images[i].container) + self.assertIsNone(img.size) + self.assertIsNone(img.md5) + + @mock.patch.object(utils, 'init_http_request') + @mock.patch.object(hu, 'list_block_devices') + def test_image_scheme_with_checksums(self, mock_lbd, mock_http_req): + fake_image_meta = [{'/': {'md5': 'fakeroot', 'size': 1}}] + prop_mock = mock.PropertyMock(return_value=yaml.dump(fake_image_meta)) + type(mock_http_req.return_value).text = prop_mock + mock_lbd.return_value = LIST_BLOCK_DEVICES_SAMPLE + p_scheme = self.drv.partition_scheme() + i_scheme = self.drv.image_scheme(p_scheme) + expected_images = [] + for fs in p_scheme.fss: + if fs.mount not in PROVISION_SAMPLE_DATA['ks_meta']['image_data']: + continue + i_data = PROVISION_SAMPLE_DATA['ks_meta']['image_data'][fs.mount] + expected_images.append(image.Image( + uri=i_data['uri'], + target_device=fs.device, + format=i_data['format'], + container=i_data['container'], + )) + expected_images = sorted(expected_images, key=lambda x: x.uri) + for i, img in enumerate(sorted(i_scheme.images, key=lambda x: x.uri)): + self.assertEqual(img.uri, expected_images[i].uri) + self.assertEqual(img.target_device, + expected_images[i].target_device) + self.assertEqual(img.format, + expected_images[i].format) + self.assertEqual(img.container, + expected_images[i].container) + self.assertEqual(img.size, fake_image_meta[0]['/']['size']) + self.assertEqual(img.md5, fake_image_meta[0]['/']['md5']) def test_getlabel(self): self.assertEqual('', self.drv._getlabel(None)) diff --git a/fuel_agent/tests/test_utils.py b/fuel_agent/tests/test_utils.py index 82dcf46..24335ad 100644 --- a/fuel_agent/tests/test_utils.py +++ b/fuel_agent/tests/test_utils.py @@ -86,6 +86,23 @@ class ExecuteTestCase(testtools.TestCase): 'fake_file_name') mock_open.assert_called_once_with('fake_file_name', 'w') + def test_calculate_md5_ok(self): + # calculated by 'printf %10000s | md5sum' + with mock.patch('six.moves.builtins.open', + mock.mock_open(read_data=' ' * 10000), create=True): + self.assertEqual('f38898bb69bb02bccb9594dfe471c5c0', + utils.calculate_md5('fake', 10000)) + self.assertEqual('6934d9d33cd2d0c005994e7d96d2e0d9', + utils.calculate_md5('fake', 1000)) + self.assertEqual('1e68934346ee57858834a205017af8b7', + utils.calculate_md5('fake', 100)) + self.assertEqual('41b394758330c83757856aa482c79977', + utils.calculate_md5('fake', 10)) + self.assertEqual('7215ee9c7d9dc229d2921a40e899ec5f', + utils.calculate_md5('fake', 1)) + self.assertEqual('d41d8cd98f00b204e9800998ecf8427e', + utils.calculate_md5('fake', 0)) + @mock.patch.object(requests, 'get') def test_init_http_request_ok(self, mock_req): utils.init_http_request('fake_url') diff --git a/fuel_agent/utils/artifact_utils.py b/fuel_agent/utils/artifact_utils.py index 122bcde..8938113 100644 --- a/fuel_agent/utils/artifact_utils.py +++ b/fuel_agent/utils/artifact_utils.py @@ -13,6 +13,7 @@ # limitations under the License. import abc +import os import tarfile import tempfile import zlib @@ -56,6 +57,8 @@ class Target(object): count += 1 LOG.debug('Flushing file: %s' % filename) f.flush() + # ensure data to be written to disk + os.fsync(f.fileno()) LOG.debug('File is written: %s' % filename) diff --git a/fuel_agent/utils/utils.py b/fuel_agent/utils/utils.py index 6258bb1..de802ca 100644 --- a/fuel_agent/utils/utils.py +++ b/fuel_agent/utils/utils.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +import hashlib import locale import math import os @@ -48,6 +49,11 @@ u_opts = [ default=2.0, help='Delay in seconds before the next http request retry', ), + cfg.IntOpt( + 'read_chunk_size', + default=1048576, + help='Block size of data to read for calculating checksum', + ), ] CONF = cfg.CONF @@ -143,6 +149,25 @@ def render_and_save(tmpl_dir, tmpl_names, tmpl_data, file_name): 'templated data to {0}'.format(file_name)) +def calculate_md5(filename, size): + hash = hashlib.md5() + processed = 0 + with open(filename, "rb") as f: + while processed < size: + block = f.read(CONF.read_chunk_size) + if block: + block_len = len(block) + if processed + block_len < size: + hash.update(block) + processed += block_len + else: + hash.update(block[:size - processed]) + break + else: + break + return hash.hexdigest() + + def init_http_request(url, byte_range=0): LOG.debug('Trying to initialize http request object %s, byte range: %s' % (url, byte_range)) diff --git a/requirements.txt b/requirements.txt index 9e92ee7..50dba68 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,3 +10,4 @@ Jinja2 stevedore>=0.15 requests>=1.2.3 urllib3>=1.7 +PyYAML==3.10