Skip to content

Commit

Permalink
Resolve links before using path as block device
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
Chris Glass committed May 6, 2016
1 parent 0f912d9 commit 30c3fb9
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -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 -%}
15 changes: 13 additions & 2 deletions charmhelpers/contrib/storage/linux/ceph.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,19 @@ def remove_cache_tier(self, cache_pool):
"""
# 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])
Expand Down Expand Up @@ -221,6 +228,10 @@ def create(self):
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

Expand Down Expand Up @@ -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):
Expand Down
10 changes: 5 additions & 5 deletions charmhelpers/contrib/storage/linux/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
11 changes: 11 additions & 0 deletions lib/misc_utils.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import os

from charmhelpers.contrib.storage.linux.utils import (
is_block_device,
zap_disk,
Expand Down Expand Up @@ -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: '
Expand Down
26 changes: 26 additions & 0 deletions unit_tests/test_misc_utils.py
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit 30c3fb9

Please sign in to comment.