openstack
/
charms.ceph
Code Issues Proposed changes

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:
Frode Nordahl 2018-06-01 12:18:17 +02:00
parent 20ba3c876a
commit 3550df998f
No known key found for this signature in database
GPG Key ID: 6A5D59A3BA48373F
3 changed files with 83 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
ceph/utils.py
View File

@ -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
View File

@ -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

45
unit_tests/test_utils.py
View File

@ -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)