diff --git a/ceph/utils.py b/ceph/utils.py index d281a3b..5ff970b 100644 --- a/ceph/utils.py +++ b/ceph/utils.py @@ -66,7 +66,6 @@ from charmhelpers.contrib.storage.linux.ceph import ( from charmhelpers.contrib.storage.linux.utils import ( is_block_device, is_device_mounted, - zap_disk, ) from charmhelpers.contrib.openstack.utils import ( get_os_codename_install_source, @@ -870,7 +869,42 @@ def get_partition_list(dev): raise +def is_pristine_disk(dev): + """ + Read first 2048 bytes (LBA 0 - 3) of block device to determine whether it + is actually all zeros and safe for us to use. + + Existing partitioning tools does not discern between a failure to read from + block device, failure to understand a partition table and the fact that a + block device has no partition table. Since we need to be positive about + which is which we need to read the device directly and confirm ourselves. + + :param dev: Path to block device + :type dev: str + :returns: True all 2048 bytes == 0x0, False if not + :rtype: bool + """ + want_bytes = 2048 + + f = open(dev, 'rb') + data = f.read(want_bytes) + read_bytes = len(data) + if read_bytes != want_bytes: + log('{}: short read, got {} bytes expected {}.' + .format(dev, read_bytes, want_bytes), level=WARNING) + return False + + return all(byte == 0x0 for byte in data) + + def is_osd_disk(dev): + db = kv() + osd_devices = db.get('osd-devices', []) + if dev in osd_devices: + log('Device {} already processed by charm,' + ' skipping'.format(dev)) + return True + partitions = get_partition_list(dev) for partition in partitions: try: @@ -1296,15 +1330,6 @@ def update_monfs(): pass -def maybe_zap_journal(journal_dev): - if is_osd_disk(journal_dev): - log('Looks like {} is already an OSD data' - ' or journal, skipping.'.format(journal_dev)) - return - zap_disk(journal_dev) - log("Zapped journal device {}".format(journal_dev)) - - def get_partitions(dev): cmd = ['partx', '--raw', '--noheadings', dev] try: @@ -1366,20 +1391,18 @@ def get_devices(name): return set(devices) -def osdize(dev, osd_format, osd_journal, reformat_osd=False, - ignore_errors=False, encrypt=False, bluestore=False, - key_manager=CEPH_KEY_MANAGER): +def osdize(dev, osd_format, osd_journal, ignore_errors=False, encrypt=False, + bluestore=False, key_manager=CEPH_KEY_MANAGER): if dev.startswith('/dev'): osdize_dev(dev, osd_format, osd_journal, - reformat_osd, ignore_errors, encrypt, + ignore_errors, encrypt, bluestore, key_manager) else: osdize_dir(dev, encrypt, bluestore) -def osdize_dev(dev, osd_format, osd_journal, reformat_osd=False, - ignore_errors=False, encrypt=False, bluestore=False, - key_manager=CEPH_KEY_MANAGER): +def osdize_dev(dev, osd_format, osd_journal, ignore_errors=False, + encrypt=False, bluestore=False, key_manager=CEPH_KEY_MANAGER): """ Prepare a block device for use as a Ceph OSD @@ -1389,8 +1412,6 @@ def osdize_dev(dev, osd_format, osd_journal, reformat_osd=False, :param: dev: Full path to block device to use :param: osd_format: Format for OSD filesystem :param: osd_journal: List of block devices to use for OSD journals - :param: reformat_osd: Reformat devices that are not currently in use - which have been used previously :param: ignore_errors: Don't fail in the event of any errors during processing :param: encrypt: Encrypt block devices using 'key_manager' @@ -1418,7 +1439,7 @@ def osdize_dev(dev, osd_format, osd_journal, reformat_osd=False, log('Path {} is not a block device - bailing'.format(dev)) return - if is_osd_disk(dev) and not reformat_osd: + if is_osd_disk(dev): log('Looks like {} is already an' ' OSD data or journal, skipping.'.format(dev)) return @@ -1432,9 +1453,6 @@ def osdize_dev(dev, osd_format, osd_journal, reformat_osd=False, ' skipping.'.format(dev)) return - if reformat_osd: - zap_disk(dev) - if cmp_pkgrevno('ceph', '12.2.4') >= 0: cmd = _ceph_volume(dev, osd_journal, diff --git a/tox.ini b/tox.ini index f8388cf..11ead47 100644 --- a/tox.ini +++ b/tox.ini @@ -18,9 +18,11 @@ whitelist_externals = rm [testenv:py27] -basepython = python2.7 -deps = -r{toxinidir}/requirements.txt - -r{toxinidir}/test-requirements.txt +# ceph charms are Python3-only, but py27 unit test target +# is required by OpenStack Governance. Remove this shim as soon as +# permitted. http://governance.openstack.org/reference/cti/python_cti.html +whitelist_externals = true +commands = true [testenv:py34] basepython = python3.4 @@ -38,7 +40,7 @@ deps = -r{toxinidir}/requirements.txt -r{toxinidir}/test-requirements.txt [testenv:pep8] -basepython = python2.7 +basepython = python3 deps = -r{toxinidir}/requirements.txt -r{toxinidir}/test-requirements.txt commands = flake8 {posargs} ceph unit_tests setup.py diff --git a/unit_tests/test_utils.py b/unit_tests/test_utils.py index a7a2034..c7e0c03 100644 --- a/unit_tests/test_utils.py +++ b/unit_tests/test_utils.py @@ -69,7 +69,6 @@ class CephTestCase(unittest.TestCase): @patch.object(utils, 'kv') @patch.object(utils.subprocess, 'check_call') - @patch.object(utils, 'zap_disk') @patch.object(utils, '_ceph_disk') @patch.object(utils, 'is_active_bluestore_device') @patch.object(utils.os.path, 'exists') @@ -78,7 +77,7 @@ class CephTestCase(unittest.TestCase): @patch.object(utils, 'is_block_device') def test_osdize_dev_ceph_disk(self, _is_blk, _cmp, _mounted, _exists, _is_active_bluestore_device, _ceph_disk, - _zap_disk, _check_call, _kv): + _check_call, _kv): """Test that _ceph_disk is called for < Luminous 12.2.4""" db = MagicMock() _kv.return_value = db @@ -90,17 +89,15 @@ class CephTestCase(unittest.TestCase): _ceph_disk.return_value = ['ceph-disk', 'prepare'] _is_active_bluestore_device.return_value = False utils.osdize('/dev/sdb', osd_format='xfs', osd_journal=None, - reformat_osd=True, bluestore=False) + bluestore=False) _ceph_disk.assert_called_with('/dev/sdb', 'xfs', None, False, False) _check_call.assert_called_with(['ceph-disk', 'prepare']) - _zap_disk.assert_called_once() db.get.assert_called_with('osd-devices', []) db.set.assert_called_with('osd-devices', ['/dev/sdb']) db.flush.assert_called_once() @patch.object(utils, 'kv') @patch.object(utils.subprocess, 'check_call') - @patch.object(utils, 'zap_disk') @patch.object(utils, '_ceph_volume') @patch.object(utils, 'is_active_bluestore_device') @patch.object(utils.os.path, 'exists') @@ -109,7 +106,7 @@ class CephTestCase(unittest.TestCase): @patch.object(utils, 'is_block_device') def test_osdize_dev_ceph_volume(self, _is_blk, _cmp, _mounted, _exists, _is_active_bluestore_device, _ceph_volume, - _zap_disk, _check_call, _kv): + _check_call, _kv): """Test that _ceph_volume is called for >= Luminous 12.2.4""" db = MagicMock() _kv.return_value = db @@ -121,10 +118,9 @@ class CephTestCase(unittest.TestCase): _ceph_volume.return_value = ['ceph-volume', 'prepare'] _is_active_bluestore_device.return_value = False utils.osdize('/dev/sdb', osd_format='xfs', osd_journal=None, - reformat_osd=True, bluestore=False) + bluestore=False) _ceph_volume.assert_called_with('/dev/sdb', None, False, False, 'ceph') _check_call.assert_called_with(['ceph-volume', 'prepare']) - _zap_disk.assert_called_once() db.get.assert_called_with('osd-devices', []) db.set.assert_called_with('osd-devices', ['/dev/sdb']) db.flush.assert_called_once() @@ -136,7 +132,7 @@ class CephTestCase(unittest.TestCase): _kv.return_value = db db.get.return_value = ['/dev/sdb'] utils.osdize('/dev/sdb', osd_format='xfs', osd_journal=None, - reformat_osd=True, bluestore=False) + bluestore=False) db.get.assert_called_with('osd-devices', []) db.set.assert_not_called() @@ -1277,3 +1273,34 @@ class CephGetLVSTestCase(unittest.TestCase): _lvm.is_lvm_physical_volume.assert_called_with( '/dev/sdb' ) + + @patch.object(utils, 'log') + def test_is_pristine_disk(self, _log): + data = b'\0' * 2048 + fake_open = mock_open(read_data=data) + with patch('ceph.utils.open', fake_open): + result = utils.is_pristine_disk('/dev/vdz') + fake_open.assert_called_with('/dev/vdz', 'rb') + self.assertFalse(_log.called) + self.assertEqual(result, True) + + @patch.object(utils, 'log') + def test_is_pristine_disk_short_read(self, _log): + data = b'\0' * 2047 + fake_open = mock_open(read_data=data) + with patch('ceph.utils.open', fake_open): + result = utils.is_pristine_disk('/dev/vdz') + fake_open.assert_called_with('/dev/vdz', 'rb') + _log.assert_called_with( + '/dev/vdz: short read, got 2047 bytes expected 2048.', + level='WARNING') + self.assertEqual(result, False) + + def test_is_pristine_disk_dirty_disk(self): + data = b'\0' * 2047 + data = data + b'\42' + fake_open = mock_open(read_data=data) + with patch('ceph.utils.open', fake_open): + result = utils.is_pristine_disk('/dev/vdz') + fake_open.assert_called_with('/dev/vdz', 'rb') + self.assertEqual(result, False)