Remove the reformat option, add check of device pristinity
As a part of the mitigation of LP Bug #1698154 remove the reformat option and initialization of journal devices entirely. Ceph charms can rely on actions to have operator administratively execute zap_disk. There should not be a code path that could accidentally execute zap_disk without administrative intervention. Make py27 test noop Flip pep8 test to py3 Partial-Bug: #1698154 Change-Id: I90a866aa138d18e4242783c42d4c7c587f696d7d
This commit is contained in:
parent
20ba3c876a
commit
3550df998f
|
@ -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,
|
||||
|
|
10
tox.ini
10
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
|
||||
|
|
|
@ -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)
|
||||
|
|
Loading…
Reference in New Issue