From f9937c90079ed60237b10e6c39e19cc4f6d8f938 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Thu, 7 Dec 2017 16:25:38 +0000 Subject: [PATCH] Extend the cinder lvm thin pool with new devices If a new device is added to the block-device configuration option then add the new device to the cinder thin pool if it exists. From pike+ if the lvm backend is being used then it defaults to using a thin provisioning pool. This patch makes sure any new devices are used to extend that pool. Change-Id: I851e4eb58dbdc122086d53fe346ea045c6044f50 Partial-Bug: #1735931 --- .../contrib/openstack/amulet/deployment.py | 12 +++-- .../contrib/openstack/amulet/utils.py | 9 ++-- hooks/charmhelpers/contrib/openstack/utils.py | 2 + .../contrib/storage/linux/ceph.py | 8 +-- .../charmhelpers/contrib/storage/linux/lvm.py | 50 +++++++++++++++++++ hooks/charmhelpers/core/host.py | 2 + .../charmhelpers/core/host_factory/ubuntu.py | 1 + hooks/cinder_utils.py | 18 ++++++- .../contrib/openstack/amulet/deployment.py | 12 +++-- .../contrib/openstack/amulet/utils.py | 9 ++-- tests/charmhelpers/core/host.py | 2 + .../charmhelpers/core/host_factory/ubuntu.py | 1 + unit_tests/test_cinder_utils.py | 9 +++- 13 files changed, 114 insertions(+), 21 deletions(-) diff --git a/hooks/charmhelpers/contrib/openstack/amulet/deployment.py b/hooks/charmhelpers/contrib/openstack/amulet/deployment.py index e37f2834..5afbbd87 100644 --- a/hooks/charmhelpers/contrib/openstack/amulet/deployment.py +++ b/hooks/charmhelpers/contrib/openstack/amulet/deployment.py @@ -13,6 +13,7 @@ # limitations under the License. import logging +import os import re import sys import six @@ -185,7 +186,7 @@ class OpenStackAmuletDeployment(AmuletDeployment): self.d.configure(service, config) def _auto_wait_for_status(self, message=None, exclude_services=None, - include_only=None, timeout=1800): + include_only=None, timeout=None): """Wait for all units to have a specific extended status, except for any defined as excluded. Unless specified via message, any status containing any case of 'ready' will be considered a match. @@ -215,7 +216,10 @@ class OpenStackAmuletDeployment(AmuletDeployment): :param timeout: Maximum time in seconds to wait for status match :returns: None. Raises if timeout is hit. """ - self.log.info('Waiting for extended status on units...') + if not timeout: + timeout = int(os.environ.get('AMULET_SETUP_TIMEOUT', 1800)) + self.log.info('Waiting for extended status on units for {}s...' + ''.format(timeout)) all_services = self.d.services.keys() @@ -252,9 +256,9 @@ class OpenStackAmuletDeployment(AmuletDeployment): service_messages = {service: message for service in services} # Check for idleness - self.d.sentry.wait() + self.d.sentry.wait(timeout=timeout) # Check for error states and bail early - self.d.sentry.wait_for_status(self.d.juju_env, services) + self.d.sentry.wait_for_status(self.d.juju_env, services, timeout=timeout) # Check for ready messages self.d.sentry.wait_for_messages(service_messages, timeout=timeout) diff --git a/hooks/charmhelpers/contrib/openstack/amulet/utils.py b/hooks/charmhelpers/contrib/openstack/amulet/utils.py index b71b2b19..87f364d1 100644 --- a/hooks/charmhelpers/contrib/openstack/amulet/utils.py +++ b/hooks/charmhelpers/contrib/openstack/amulet/utils.py @@ -858,9 +858,12 @@ class OpenStackAmuletUtils(AmuletUtils): :returns: List of pool name, object count, kb disk space used """ df = self.get_ceph_df(sentry_unit) - pool_name = df['pools'][pool_id]['name'] - obj_count = df['pools'][pool_id]['stats']['objects'] - kb_used = df['pools'][pool_id]['stats']['kb_used'] + for pool in df['pools']: + if pool['id'] == pool_id: + pool_name = pool['name'] + obj_count = pool['stats']['objects'] + kb_used = pool['stats']['kb_used'] + self.log.debug('Ceph {} pool (ID {}): {} objects, ' '{} kb used'.format(pool_name, pool_id, obj_count, kb_used)) diff --git a/hooks/charmhelpers/contrib/openstack/utils.py b/hooks/charmhelpers/contrib/openstack/utils.py index 8a541d40..9e5af342 100644 --- a/hooks/charmhelpers/contrib/openstack/utils.py +++ b/hooks/charmhelpers/contrib/openstack/utils.py @@ -392,6 +392,8 @@ def get_swift_codename(version): releases = UBUNTU_OPENSTACK_RELEASE release = [k for k, v in six.iteritems(releases) if codename in v] ret = subprocess.check_output(['apt-cache', 'policy', 'swift']) + if six.PY3: + ret = ret.decode('UTF-8') if codename in ret or release[0] in ret: return codename elif len(codenames) == 1: diff --git a/hooks/charmhelpers/contrib/storage/linux/ceph.py b/hooks/charmhelpers/contrib/storage/linux/ceph.py index 39231612..0d9bacfd 100644 --- a/hooks/charmhelpers/contrib/storage/linux/ceph.py +++ b/hooks/charmhelpers/contrib/storage/linux/ceph.py @@ -377,12 +377,12 @@ def get_mon_map(service): try: return json.loads(mon_status) except ValueError as v: - log("Unable to parse mon_status json: {}. Error: {}".format( - mon_status, v.message)) + log("Unable to parse mon_status json: {}. Error: {}" + .format(mon_status, str(v))) raise except CalledProcessError as e: - log("mon_status command failed with message: {}".format( - e.message)) + log("mon_status command failed with message: {}" + .format(str(e))) raise diff --git a/hooks/charmhelpers/contrib/storage/linux/lvm.py b/hooks/charmhelpers/contrib/storage/linux/lvm.py index 7f2a0604..79a7a245 100644 --- a/hooks/charmhelpers/contrib/storage/linux/lvm.py +++ b/hooks/charmhelpers/contrib/storage/linux/lvm.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import functools from subprocess import ( CalledProcessError, check_call, @@ -101,3 +102,52 @@ def create_lvm_volume_group(volume_group, block_device): :block_device: str: Full path of PV-initialized block device. ''' check_call(['vgcreate', volume_group, block_device]) + + +def list_logical_volumes(select_criteria=None, path_mode=False): + ''' + List logical volumes + + :param select_criteria: str: Limit list to those volumes matching this + criteria (see 'lvs -S help' for more details) + :param path_mode: bool: return logical volume name in 'vg/lv' format, this + format is required for some commands like lvextend + :returns: [str]: List of logical volumes + ''' + lv_diplay_attr = 'lv_name' + if path_mode: + # Parsing output logic relies on the column order + lv_diplay_attr = 'vg_name,' + lv_diplay_attr + cmd = ['lvs', '--options', lv_diplay_attr, '--noheadings'] + if select_criteria: + cmd.extend(['--select', select_criteria]) + lvs = [] + for lv in check_output(cmd).decode('UTF-8').splitlines(): + if not lv: + continue + if path_mode: + lvs.append('/'.join(lv.strip().split())) + else: + lvs.append(lv.strip()) + return lvs + + +list_thin_logical_volume_pools = functools.partial( + list_logical_volumes, + select_criteria='lv_attr =~ ^t') + +list_thin_logical_volumes = functools.partial( + list_logical_volumes, + select_criteria='lv_attr =~ ^V') + + +def extend_logical_volume_by_device(lv_name, block_device): + ''' + Extends the size of logical volume lv_name by the amount of free space on + physical volume block_device. + + :param lv_name: str: name of logical volume to be extended (vg/lv format) + :param block_device: str: name of block_device to be allocated to lv_name + ''' + cmd = ['lvextend', lv_name, block_device] + check_call(cmd) diff --git a/hooks/charmhelpers/core/host.py b/hooks/charmhelpers/core/host.py index 5cc5c86b..fd14d60f 100644 --- a/hooks/charmhelpers/core/host.py +++ b/hooks/charmhelpers/core/host.py @@ -549,6 +549,8 @@ def write_file(path, content, owner='root', group='root', perms=0o444): with open(path, 'wb') as target: os.fchown(target.fileno(), uid, gid) os.fchmod(target.fileno(), perms) + if six.PY3 and isinstance(content, six.string_types): + content = content.encode('UTF-8') target.write(content) return # the contents were the same, but we might still need to change the diff --git a/hooks/charmhelpers/core/host_factory/ubuntu.py b/hooks/charmhelpers/core/host_factory/ubuntu.py index d8dc378a..99451b59 100644 --- a/hooks/charmhelpers/core/host_factory/ubuntu.py +++ b/hooks/charmhelpers/core/host_factory/ubuntu.py @@ -20,6 +20,7 @@ UBUNTU_RELEASES = ( 'yakkety', 'zesty', 'artful', + 'bionic', ) diff --git a/hooks/cinder_utils.py b/hooks/cinder_utils.py index 0bfb7e86..acc8bb25 100644 --- a/hooks/cinder_utils.py +++ b/hooks/cinder_utils.py @@ -80,9 +80,11 @@ from charmhelpers.contrib.storage.linux.lvm import ( create_lvm_physical_volume, create_lvm_volume_group, deactivate_lvm_volume_group, + extend_logical_volume_by_device, is_lvm_physical_volume, + list_lvm_volume_group, + list_thin_logical_volume_pools, remove_lvm_physical_volume, - list_lvm_volume_group ) from charmhelpers.contrib.storage.linux.loopback import ( @@ -565,7 +567,19 @@ def configure_lvm_storage(block_devices, volume_group, overwrite=False, # Extend the volume group as required for new_device in new_devices: extend_lvm_volume_group(volume_group, new_device) - + thin_pools = list_thin_logical_volume_pools(path_mode=True) + if len(thin_pools) == 0: + juju_log("No thin pools found") + elif len(thin_pools) == 1: + juju_log("Thin pool {} found, extending with {}".format( + thin_pools[0], + new_device)) + extend_logical_volume_by_device(thin_pools[0], new_device) + else: + juju_log("Multiple thin pools ({}) found, " + "skipping auto extending with {}".format( + ','.join(thin_pools), + new_device)) log_lvm_info() diff --git a/tests/charmhelpers/contrib/openstack/amulet/deployment.py b/tests/charmhelpers/contrib/openstack/amulet/deployment.py index e37f2834..5afbbd87 100644 --- a/tests/charmhelpers/contrib/openstack/amulet/deployment.py +++ b/tests/charmhelpers/contrib/openstack/amulet/deployment.py @@ -13,6 +13,7 @@ # limitations under the License. import logging +import os import re import sys import six @@ -185,7 +186,7 @@ class OpenStackAmuletDeployment(AmuletDeployment): self.d.configure(service, config) def _auto_wait_for_status(self, message=None, exclude_services=None, - include_only=None, timeout=1800): + include_only=None, timeout=None): """Wait for all units to have a specific extended status, except for any defined as excluded. Unless specified via message, any status containing any case of 'ready' will be considered a match. @@ -215,7 +216,10 @@ class OpenStackAmuletDeployment(AmuletDeployment): :param timeout: Maximum time in seconds to wait for status match :returns: None. Raises if timeout is hit. """ - self.log.info('Waiting for extended status on units...') + if not timeout: + timeout = int(os.environ.get('AMULET_SETUP_TIMEOUT', 1800)) + self.log.info('Waiting for extended status on units for {}s...' + ''.format(timeout)) all_services = self.d.services.keys() @@ -252,9 +256,9 @@ class OpenStackAmuletDeployment(AmuletDeployment): service_messages = {service: message for service in services} # Check for idleness - self.d.sentry.wait() + self.d.sentry.wait(timeout=timeout) # Check for error states and bail early - self.d.sentry.wait_for_status(self.d.juju_env, services) + self.d.sentry.wait_for_status(self.d.juju_env, services, timeout=timeout) # Check for ready messages self.d.sentry.wait_for_messages(service_messages, timeout=timeout) diff --git a/tests/charmhelpers/contrib/openstack/amulet/utils.py b/tests/charmhelpers/contrib/openstack/amulet/utils.py index b71b2b19..87f364d1 100644 --- a/tests/charmhelpers/contrib/openstack/amulet/utils.py +++ b/tests/charmhelpers/contrib/openstack/amulet/utils.py @@ -858,9 +858,12 @@ class OpenStackAmuletUtils(AmuletUtils): :returns: List of pool name, object count, kb disk space used """ df = self.get_ceph_df(sentry_unit) - pool_name = df['pools'][pool_id]['name'] - obj_count = df['pools'][pool_id]['stats']['objects'] - kb_used = df['pools'][pool_id]['stats']['kb_used'] + for pool in df['pools']: + if pool['id'] == pool_id: + pool_name = pool['name'] + obj_count = pool['stats']['objects'] + kb_used = pool['stats']['kb_used'] + self.log.debug('Ceph {} pool (ID {}): {} objects, ' '{} kb used'.format(pool_name, pool_id, obj_count, kb_used)) diff --git a/tests/charmhelpers/core/host.py b/tests/charmhelpers/core/host.py index 5cc5c86b..fd14d60f 100644 --- a/tests/charmhelpers/core/host.py +++ b/tests/charmhelpers/core/host.py @@ -549,6 +549,8 @@ def write_file(path, content, owner='root', group='root', perms=0o444): with open(path, 'wb') as target: os.fchown(target.fileno(), uid, gid) os.fchmod(target.fileno(), perms) + if six.PY3 and isinstance(content, six.string_types): + content = content.encode('UTF-8') target.write(content) return # the contents were the same, but we might still need to change the diff --git a/tests/charmhelpers/core/host_factory/ubuntu.py b/tests/charmhelpers/core/host_factory/ubuntu.py index d8dc378a..99451b59 100644 --- a/tests/charmhelpers/core/host_factory/ubuntu.py +++ b/tests/charmhelpers/core/host_factory/ubuntu.py @@ -20,6 +20,7 @@ UBUNTU_RELEASES = ( 'yakkety', 'zesty', 'artful', + 'bionic', ) diff --git a/unit_tests/test_cinder_utils.py b/unit_tests/test_cinder_utils.py index 5372741a..c7983250 100644 --- a/unit_tests/test_cinder_utils.py +++ b/unit_tests/test_cinder_utils.py @@ -446,12 +446,17 @@ class TestCinderUtils(CharmTestCase): @patch.object(cinder_utils, 'clean_storage') @patch.object(cinder_utils, 'reduce_lvm_volume_group_missing') @patch.object(cinder_utils, 'extend_lvm_volume_group') - def test_configure_lvm_storage_unused_dev(self, extend_lvm, reduce_lvm, + @patch.object(cinder_utils, 'list_thin_logical_volume_pools') + @patch.object(cinder_utils, 'extend_logical_volume_by_device') + def test_configure_lvm_storage_unused_dev(self, extend_lv_by_dev, + list_thin_pools, + extend_lvm, reduce_lvm, clean_storage, has_part): devices = ['/dev/fakevbd', '/dev/fakevdc'] self.is_lvm_physical_volume.return_value = False has_part.return_value = False self.ensure_loopback_device.side_effect = lambda x, y: x + list_thin_pools.return_value = ['vg/thinpool'] cinder_utils.configure_lvm_storage(devices, 'test', False, True) clean_storage.assert_has_calls( [call('/dev/fakevbd'), @@ -464,6 +469,8 @@ class TestCinderUtils(CharmTestCase): self.create_lvm_volume_group.assert_called_with('test', '/dev/fakevbd') reduce_lvm.assert_called_with('test') extend_lvm.assert_called_with('test', '/dev/fakevdc') + extend_lv_by_dev.assert_called_once_with('vg/thinpool', + '/dev/fakevdc') @patch('cinder_utils.log_lvm_info', Mock()) @patch.object(cinder_utils, 'has_partition_table')