diff --git a/hooks/ceph_hooks.py b/hooks/ceph_hooks.py index dfea7363..3b8cfcdd 100755 --- a/hooks/ceph_hooks.py +++ b/hooks/ceph_hooks.py @@ -89,6 +89,7 @@ from charmhelpers.contrib.storage.linux.ceph import ( CephConfContext) from charmhelpers.contrib.storage.linux.utils import ( is_device_mounted, + is_block_device, ) from charmhelpers.contrib.charmsupport import nrpe from charmhelpers.contrib.hardening.harden import harden @@ -522,10 +523,19 @@ def get_devices(): if config('osd-devices'): for path in config('osd-devices').split(' '): path = path.strip() + # Ensure that only block devices + # are considered for evaluation as block devices. + # This avoids issues with relative directories + # being passed via configuration, and ensures that + # the path to a block device provided by the user + # is used, rather than its target which may change + # between reboots in the case of bcache devices. + if is_block_device(path): + devices.append(path) # Make sure its a device which is specified using an # absolute path so that the current working directory # or any relative path under this directory is not used - if os.path.isabs(path): + elif os.path.isabs(path): devices.append(os.path.realpath(path)) # List storage instances for the 'osd-devices' @@ -562,6 +572,32 @@ def upgrade_charm(): apt_install(packages=filter_installed_packages(ceph.determine_packages()), fatal=True) install_udev_rules() + remap_resolved_targets() + + +def remap_resolved_targets(): + '''Remap any previous fully resolved target devices to provided names''' + # NOTE(jamespage): Deal with any prior provided dev to + # target device resolution which occurred in prior + # releases of the charm - the user provided value + # should be used in preference to the target path + # to the block device as in some instances this + # is not consistent between reboots (bcache). + db = kv() + touched_devices = db.get('osd-devices', []) + osd_devices = get_devices() + for dev in osd_devices: + real_path = os.path.realpath(dev) + if real_path != dev and real_path in touched_devices: + log('Device {} already processed by charm using ' + 'actual device path {}, updating block device ' + 'usage with provided device path ' + 'and skipping'.format(dev, + real_path)) + touched_devices.remove(real_path) + touched_devices.append(dev) + db.set('osd-devices', touched_devices) + db.flush() @hooks.hook('nrpe-external-master-relation-joined', diff --git a/lib/ceph/utils.py b/lib/ceph/utils.py index 8a44afc5..4d6ac326 100644 --- a/lib/ceph/utils.py +++ b/lib/ceph/utils.py @@ -1699,8 +1699,10 @@ def is_mapped_luks_device(dev): :param: dev: A full path to a block device to be checked :returns: boolean: indicates whether a device is mapped """ - _, dirs, _ = next(os.walk('/sys/class/block/{}/holders/' - .format(os.path.basename(dev)))) + _, dirs, _ = next(os.walk( + '/sys/class/block/{}/holders/' + .format(os.path.basename(os.path.realpath(dev)))) + ) is_held = len(dirs) > 0 return is_held and is_luks_device(dev) diff --git a/unit_tests/test_ceph_hooks.py b/unit_tests/test_ceph_hooks.py index 9b660c3a..543d3564 100644 --- a/unit_tests/test_ceph_hooks.py +++ b/unit_tests/test_ceph_hooks.py @@ -395,26 +395,31 @@ class CephHooksTestCase(unittest.TestCase): call('ceph-osd@2'), ]) + @patch.object(ceph_hooks, 'is_block_device') @patch.object(ceph_hooks, 'storage_list') @patch.object(ceph_hooks, 'config') - def test_get_devices(self, mock_config, mock_storage_list): + def test_get_devices(self, mock_config, mock_storage_list, + mock_is_block_device): '''Devices returned as expected''' config = {'osd-devices': '/dev/vda /dev/vdb'} mock_config.side_effect = lambda key: config[key] mock_storage_list.return_value = [] + mock_is_block_device.return_value = True devices = ceph_hooks.get_devices() self.assertEqual(devices, ['/dev/vda', '/dev/vdb']) + @patch.object(ceph_hooks, 'is_block_device') @patch.object(ceph_hooks, 'get_blacklist') @patch.object(ceph_hooks, 'storage_list') @patch.object(ceph_hooks, 'config') def test_get_devices_blacklist(self, mock_config, mock_storage_list, - mock_get_blacklist): + mock_get_blacklist, mock_is_block_device): '''Devices returned as expected when blacklist in effect''' config = {'osd-devices': '/dev/vda /dev/vdb'} mock_config.side_effect = lambda key: config[key] mock_storage_list.return_value = [] mock_get_blacklist.return_value = ['/dev/vda'] + mock_is_block_device.return_value = True devices = ceph_hooks.get_devices() mock_storage_list.assert_called() mock_get_blacklist.assert_called() diff --git a/unit_tests/test_config.py b/unit_tests/test_config.py index c3ae347e..aa539859 100644 --- a/unit_tests/test_config.py +++ b/unit_tests/test_config.py @@ -34,6 +34,7 @@ with patch('charmhelpers.contrib.hardening.harden.harden') as mock_dec: TO_PATCH = [ 'config', + 'is_block_device', ] @@ -43,6 +44,13 @@ class GetDevicesTestCase(test_utils.CharmTestCase): super(GetDevicesTestCase, self).setUp(hooks, TO_PATCH) self.config.side_effect = self.test_config.get self.tmp_dir = tempfile.mkdtemp() + self.bd = { + os.path.join(self.tmp_dir, "device1"): True, + os.path.join(self.tmp_dir, "device1"): True, + os.path.join(self.tmp_dir, "link"): True, + os.path.join(self.tmp_dir, "device"): True, + } + self.is_block_device.side_effect = lambda x: self.bd.get(x, False) self.addCleanup(shutil.rmtree, self.tmp_dir) def test_get_devices_empty(self): @@ -93,11 +101,11 @@ class GetDevicesTestCase(test_utils.CharmTestCase): def test_get_devices_symlink(self): """ - If a symlink is specified in osd-devices, get_devices() resolves - it and returns the link target. + If a symlink is specified in osd-devices, get_devices() does not + resolve it and returns the symlink provided. """ device = os.path.join(self.tmp_dir, "device") link = os.path.join(self.tmp_dir, "link") os.symlink(device, link) self.test_config.set("osd-devices", link) - self.assertEqual([device], hooks.get_devices()) + self.assertEqual([link], hooks.get_devices())