From 30c3fb9353d69944f815eb1be6a2c8b1e858fe2b Mon Sep 17 00:00:00 2001 From: Chris Glass Date: Fri, 6 May 2016 06:51:50 +0000 Subject: [PATCH] Resolve links before using path as block device If the charm code is passed symlinks to block devices (as is often the case with newer MAAS substrate versions), resolve links before attempting to use the block device for storage. Charmhelpers were updated as well. Testing done: - Unit tests pass - Tests pass - Multiple Openstack Autopilot deployments pass Change-Id: If966239502d0752c86e46f3f0aee96f43828aa08 Closes-Bug: 1577408 Signed-off-by: Chris Glass --- .../section-keystone-authtoken-mitaka | 12 +++++++++ charmhelpers/contrib/storage/linux/ceph.py | 15 +++++++++-- charmhelpers/contrib/storage/linux/utils.py | 10 +++---- lib/misc_utils.py | 11 ++++++++ unit_tests/test_misc_utils.py | 26 +++++++++++++++++++ 5 files changed, 67 insertions(+), 7 deletions(-) create mode 100644 charmhelpers/contrib/openstack/templates/section-keystone-authtoken-mitaka create mode 100644 unit_tests/test_misc_utils.py diff --git a/charmhelpers/contrib/openstack/templates/section-keystone-authtoken-mitaka b/charmhelpers/contrib/openstack/templates/section-keystone-authtoken-mitaka new file mode 100644 index 0000000..dd6f364 --- /dev/null +++ b/charmhelpers/contrib/openstack/templates/section-keystone-authtoken-mitaka @@ -0,0 +1,12 @@ +{% if auth_host -%} +[keystone_authtoken] +auth_uri = {{ service_protocol }}://{{ service_host }}:{{ service_port }} +auth_url = {{ auth_protocol }}://{{ auth_host }}:{{ auth_port }} +auth_type = password +project_domain_name = default +user_domain_name = default +project_name = {{ admin_tenant_name }} +username = {{ admin_user }} +password = {{ admin_password }} +signing_dir = {{ signing_dir }} +{% endif -%} diff --git a/charmhelpers/contrib/storage/linux/ceph.py b/charmhelpers/contrib/storage/linux/ceph.py index 1b4b1de..d008081 100644 --- a/charmhelpers/contrib/storage/linux/ceph.py +++ b/charmhelpers/contrib/storage/linux/ceph.py @@ -166,12 +166,19 @@ class Pool(object): """ # read-only is easy, writeback is much harder mode = get_cache_mode(self.service, cache_pool) + version = ceph_version() if mode == 'readonly': check_call(['ceph', '--id', self.service, 'osd', 'tier', 'cache-mode', cache_pool, 'none']) check_call(['ceph', '--id', self.service, 'osd', 'tier', 'remove', self.name, cache_pool]) elif mode == 'writeback': - check_call(['ceph', '--id', self.service, 'osd', 'tier', 'cache-mode', cache_pool, 'forward']) + pool_forward_cmd = ['ceph', '--id', self.service, 'osd', 'tier', + 'cache-mode', cache_pool, 'forward'] + if version >= '10.1': + # Jewel added a mandatory flag + pool_forward_cmd.append('--yes-i-really-mean-it') + + check_call(pool_forward_cmd) # Flush the cache and wait for it to return check_call(['rados', '--id', self.service, '-p', cache_pool, 'cache-flush-evict-all']) check_call(['ceph', '--id', self.service, 'osd', 'tier', 'remove-overlay', self.name]) @@ -221,6 +228,10 @@ class ReplicatedPool(Pool): self.name, str(self.pg_num)] try: check_call(cmd) + # Set the pool replica size + update_pool(client=self.service, + pool=self.name, + settings={'size': str(self.replicas)}) except CalledProcessError: raise @@ -604,7 +615,7 @@ def pool_exists(service, name): except CalledProcessError: return False - return name in out + return name in out.split() def get_osds(service): diff --git a/charmhelpers/contrib/storage/linux/utils.py b/charmhelpers/contrib/storage/linux/utils.py index 1e57941..4e35c29 100644 --- a/charmhelpers/contrib/storage/linux/utils.py +++ b/charmhelpers/contrib/storage/linux/utils.py @@ -64,8 +64,8 @@ def is_device_mounted(device): :returns: boolean: True if the path represents a mounted device, False if it doesn't. ''' - is_partition = bool(re.search(r".*[0-9]+\b", device)) - out = check_output(['mount']).decode('UTF-8') - if is_partition: - return bool(re.search(device + r"\b", out)) - return bool(re.search(device + r"[0-9]*\b", out)) + try: + out = check_output(['lsblk', '-P', device]).decode('UTF-8') + except: + return False + return bool(re.search(r'MOUNTPOINT=".+"', out)) diff --git a/lib/misc_utils.py b/lib/misc_utils.py index 5414be5..f014179 100644 --- a/lib/misc_utils.py +++ b/lib/misc_utils.py @@ -1,3 +1,5 @@ +import os + from charmhelpers.contrib.storage.linux.utils import ( is_block_device, zap_disk, @@ -41,6 +43,15 @@ def ensure_block_device(block_device): :returns: str: Full path of ensured block device. ''' + if block_device.startswith("/"): + # Resolve non-relative link(s) if device is a symlink to a real device. + # This fixes/prevents bug #1577408. + real_device = os.path.realpath(block_device) + if real_device != block_device: + log('Block device "{}" is a symlink to "{}". Using "{}".'.format( + block_device, real_device, real_device), level=INFO) + block_device = real_device + _none = ['None', 'none', None] if (block_device in _none): log('prepare_storage(): Missing required input: ' diff --git a/unit_tests/test_misc_utils.py b/unit_tests/test_misc_utils.py new file mode 100644 index 0000000..d1c484b --- /dev/null +++ b/unit_tests/test_misc_utils.py @@ -0,0 +1,26 @@ +import os +import tempfile +import unittest +import shutil + +from mock import patch + +from lib.misc_utils import ensure_block_device + + +class EnsureBlockDeviceTestCase(unittest.TestCase): + + @patch("lib.misc_utils.is_block_device") + def test_symlinks_are_resolved(self, mock_function): + """ + Ensure symlinks pointing to block devices are resolved when passed to + ensure_block_device. + """ + # Create a temporary symlink pointing to /dev/null + temp_dir = tempfile.mkdtemp() + link_path = os.path.join(temp_dir, "null_link") + os.symlink("/dev/null", link_path) + result = ensure_block_device(link_path) + assert mock_function.called + self.assertEqual("/dev/null", result) + shutil.rmtree(temp_dir)