From 886ef0b2243ffd0fc243ee01b5c9c58b622788f7 Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Tue, 5 Feb 2019 12:20:25 +0100 Subject: [PATCH] Ensure we populate osd-devices with existing devices If an older version of ceph-osd is deployed and then upgraded to a version that keeps track of bootstrapped OSDs, then the list of osd-devices never gets updated with the pre-existing devices. This change allows us to add existing, mounted Ceph OSDs to the osd-devices entry in the local KV storage. Change-Id: I940b108d914b39b55013a4617c3d17ff7122df60 Closes-Bug: #1814597 --- ceph/utils.py | 143 ++++++++++++++++++++------------------- unit_tests/test_utils.py | 24 ++++++- 2 files changed, 96 insertions(+), 71 deletions(-) diff --git a/ceph/utils.py b/ceph/utils.py index ee1719d..98320ac 100644 --- a/ceph/utils.py +++ b/ceph/utils.py @@ -1446,77 +1446,82 @@ def osdize_dev(dev, osd_format, osd_journal, ignore_errors=False, db = kv() osd_devices = db.get('osd-devices', []) - if dev in osd_devices: - log('Device {} already processed by charm,' - ' skipping'.format(dev)) - return - - if not os.path.exists(dev): - log('Path {} does not exist - bailing'.format(dev)) - return - - if not is_block_device(dev): - log('Path {} is not a block device - bailing'.format(dev)) - return - - if is_osd_disk(dev): - log('Looks like {} is already an' - ' OSD data or journal, skipping.'.format(dev)) - return - - if is_device_mounted(dev): - log('Looks like {} is in use, skipping.'.format(dev)) - return - - if is_active_bluestore_device(dev): - log('{} is in use as an active bluestore block device,' - ' skipping.'.format(dev)) - return - - if is_mapped_luks_device(dev): - log('{} is a mapped LUKS device,' - ' skipping.'.format(dev)) - return - - if cmp_pkgrevno('ceph', '12.2.4') >= 0: - cmd = _ceph_volume(dev, - osd_journal, - encrypt, - bluestore, - key_manager) - else: - cmd = _ceph_disk(dev, - osd_format, - osd_journal, - encrypt, - bluestore) - try: - status_set('maintenance', 'Initializing device {}'.format(dev)) - log("osdize cmd: {}".format(cmd)) - subprocess.check_call(cmd) - except subprocess.CalledProcessError: - try: - lsblk_output = subprocess.check_output( - ['lsblk', '-P']).decode('UTF-8') - except subprocess.CalledProcessError as e: - log("Couldn't get lsblk output: {}".format(e), ERROR) - if ignore_errors: - log('Unable to initialize device: {}'.format(dev), WARNING) - if lsblk_output: - log('lsblk output: {}'.format(lsblk_output), DEBUG) - else: - log('Unable to initialize device: {}'.format(dev), ERROR) - if lsblk_output: - log('lsblk output: {}'.format(lsblk_output), WARNING) - raise + if dev in osd_devices: + log('Device {} already processed by charm,' + ' skipping'.format(dev)) + return - # NOTE: Record processing of device only on success to ensure that - # the charm only tries to initialize a device of OSD usage - # once during its lifetime. - osd_devices.append(dev) - db.set('osd-devices', osd_devices) - db.flush() + if not os.path.exists(dev): + log('Path {} does not exist - bailing'.format(dev)) + return + + if not is_block_device(dev): + log('Path {} is not a block device - bailing'.format(dev)) + return + + if is_osd_disk(dev): + log('Looks like {} is already an' + ' OSD data or journal, skipping.'.format(dev)) + if is_device_mounted(dev): + osd_devices.append(dev) + return + + if is_device_mounted(dev): + log('Looks like {} is in use, skipping.'.format(dev)) + return + + if is_active_bluestore_device(dev): + log('{} is in use as an active bluestore block device,' + ' skipping.'.format(dev)) + osd_devices.append(dev) + return + + if is_mapped_luks_device(dev): + log('{} is a mapped LUKS device,' + ' skipping.'.format(dev)) + return + + if cmp_pkgrevno('ceph', '12.2.4') >= 0: + cmd = _ceph_volume(dev, + osd_journal, + encrypt, + bluestore, + key_manager) + else: + cmd = _ceph_disk(dev, + osd_format, + osd_journal, + encrypt, + bluestore) + + try: + status_set('maintenance', 'Initializing device {}'.format(dev)) + log("osdize cmd: {}".format(cmd)) + subprocess.check_call(cmd) + except subprocess.CalledProcessError: + try: + lsblk_output = subprocess.check_output( + ['lsblk', '-P']).decode('UTF-8') + except subprocess.CalledProcessError as e: + log("Couldn't get lsblk output: {}".format(e), ERROR) + if ignore_errors: + log('Unable to initialize device: {}'.format(dev), WARNING) + if lsblk_output: + log('lsblk output: {}'.format(lsblk_output), DEBUG) + else: + log('Unable to initialize device: {}'.format(dev), ERROR) + if lsblk_output: + log('lsblk output: {}'.format(lsblk_output), WARNING) + raise + + # NOTE: Record processing of device only on success to ensure that + # the charm only tries to initialize a device of OSD usage + # once during its lifetime. + osd_devices.append(dev) + finally: + db.set('osd-devices', osd_devices) + db.flush() def _ceph_disk(dev, osd_format, osd_journal, encrypt=False, bluestore=False): diff --git a/unit_tests/test_utils.py b/unit_tests/test_utils.py index 6c66d64..838192c 100644 --- a/unit_tests/test_utils.py +++ b/unit_tests/test_utils.py @@ -140,7 +140,27 @@ class CephTestCase(unittest.TestCase): utils.osdize('/dev/sdb', osd_format='xfs', osd_journal=None, bluestore=False) db.get.assert_called_with('osd-devices', []) - db.set.assert_not_called() + db.set.assert_called_with('osd-devices', ['/dev/sdb']) + + @patch.object(utils, 'kv') + @patch.object(utils.os.path, 'exists') + @patch.object(utils, 'is_device_mounted') + @patch.object(utils, 'is_block_device') + @patch.object(utils, 'is_osd_disk') + def test_osdize_dev_already_processed_without_kv(self, _is_osd, _is_blk, + _mounted, _exists, _kv): + """Ensure that previously processed disks are skipped""" + db = MagicMock() + _kv.return_value = db + db.get.return_value = [] + _exists.return_value = True + _is_osd.return_value = True + _mounted.return_value = True + _is_blk.return_value = True + utils.osdize('/dev/sdb', osd_format='xfs', osd_journal=None, + bluestore=False) + db.get.assert_called_with('osd-devices', []) + db.set.assert_called_with('osd-devices', ['/dev/sdb']) @patch.object(utils, 'kv') @patch.object(utils.subprocess, 'check_call') @@ -170,7 +190,7 @@ class CephTestCase(unittest.TestCase): utils.osdize('/dev/sdb', encrypt=True, osd_format=None, osd_journal=None, bluestore=True, key_manager='vault') db.get.assert_called_with('osd-devices', []) - db.set.assert_not_called() + db.set.assert_called_with('osd-devices', []) @patch.object(utils, 'kv') @patch.object(utils.subprocess, 'check_call')