From d8d180aaaf22fcf355aa6fa62d5fb65de77255ae Mon Sep 17 00:00:00 2001 From: Liam Young Date: Wed, 15 Nov 2017 11:39:07 +0000 Subject: [PATCH] Fix _parse_block_device to return (str, int) _parse_block_device was returning inconsistent types. This seems to have been working by accident under py2 but fails for py3. This change fixes _parse_block_device to always returns (str, int) Closes-Bug: #1732413 Change-Id: If0bfb4030ab9acfdf599ef8e70fed14816df3e26 --- .gitignore | 1 + .../charmhelpers/contrib/charmsupport/nrpe.py | 8 +++- .../contrib/openstack/amulet/utils.py | 21 +++++++++- .../charmhelpers/contrib/openstack/context.py | 10 ----- hooks/charmhelpers/contrib/openstack/utils.py | 2 +- hooks/charmhelpers/core/hookenv.py | 40 +++++++++++++++++++ hooks/charmhelpers/core/strutils.py | 16 +++++--- hooks/cinder_utils.py | 8 +++- .../contrib/openstack/amulet/utils.py | 21 +++++++++- tests/charmhelpers/core/hookenv.py | 40 +++++++++++++++++++ tests/charmhelpers/core/strutils.py | 16 +++++--- unit_tests/test_cinder_utils.py | 5 +++ 12 files changed, 162 insertions(+), 26 deletions(-) diff --git a/.gitignore b/.gitignore index 45f9daaf..9e552b19 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ bin *.pyc .unit-state.db .stestr +__pycache__ diff --git a/hooks/charmhelpers/contrib/charmsupport/nrpe.py b/hooks/charmhelpers/contrib/charmsupport/nrpe.py index 455fe60d..1c55b30f 100644 --- a/hooks/charmhelpers/contrib/charmsupport/nrpe.py +++ b/hooks/charmhelpers/contrib/charmsupport/nrpe.py @@ -30,6 +30,7 @@ import yaml from charmhelpers.core.hookenv import ( config, + hook_name, local_unit, log, relation_ids, @@ -302,7 +303,12 @@ class NRPE(object): "command": nrpecheck.command, } - service('restart', 'nagios-nrpe-server') + # update-status hooks are configured to firing every 5 minutes by + # default. When nagios-nrpe-server is restarted, the nagios server + # reports checks failing causing unneccessary alerts. Let's not restart + # on update-status hooks. + if not hook_name() == 'update-status': + service('restart', 'nagios-nrpe-server') monitor_ids = relation_ids("local-monitors") + \ relation_ids("nrpe-external-master") diff --git a/hooks/charmhelpers/contrib/openstack/amulet/utils.py b/hooks/charmhelpers/contrib/openstack/amulet/utils.py index 4fda829f..c24be8f1 100644 --- a/hooks/charmhelpers/contrib/openstack/amulet/utils.py +++ b/hooks/charmhelpers/contrib/openstack/amulet/utils.py @@ -628,6 +628,18 @@ class OpenStackAmuletUtils(AmuletUtils): _keypair = nova.keypairs.create(name=keypair_name) return _keypair + def _get_cinder_obj_name(self, cinder_object): + """Retrieve name of cinder object. + + :param cinder_object: cinder snapshot or volume object + :returns: str cinder object name + """ + # v1 objects store name in 'display_name' attr but v2+ use 'name' + try: + return cinder_object.display_name + except AttributeError: + return cinder_object.name + def create_cinder_volume(self, cinder, vol_name="demo-vol", vol_size=1, img_id=None, src_vol_id=None, snap_id=None): """Create cinder volume, optionally from a glance image, OR @@ -678,6 +690,13 @@ class OpenStackAmuletUtils(AmuletUtils): source_volid=src_vol_id, snapshot_id=snap_id) vol_id = vol_new.id + except TypeError: + vol_new = cinder.volumes.create(name=vol_name, + imageRef=img_id, + size=vol_size, + source_volid=src_vol_id, + snapshot_id=snap_id) + vol_id = vol_new.id except Exception as e: msg = 'Failed to create volume: {}'.format(e) amulet.raise_status(amulet.FAIL, msg=msg) @@ -692,7 +711,7 @@ class OpenStackAmuletUtils(AmuletUtils): # Re-validate new volume self.log.debug('Validating volume attributes...') - val_vol_name = cinder.volumes.get(vol_id).display_name + val_vol_name = self._get_cinder_obj_name(cinder.volumes.get(vol_id)) val_vol_boot = cinder.volumes.get(vol_id).bootable val_vol_stat = cinder.volumes.get(vol_id).status val_vol_size = cinder.volumes.get(vol_id).size diff --git a/hooks/charmhelpers/contrib/openstack/context.py b/hooks/charmhelpers/contrib/openstack/context.py index 86f2dac4..ece75df8 100644 --- a/hooks/charmhelpers/contrib/openstack/context.py +++ b/hooks/charmhelpers/contrib/openstack/context.py @@ -853,15 +853,6 @@ class NeutronContext(OSContextGenerator): for pkgs in self.packages: ensure_packages(pkgs) - def _save_flag_file(self): - if self.network_manager == 'quantum': - _file = '/etc/nova/quantum_plugin.conf' - else: - _file = '/etc/nova/neutron_plugin.conf' - - with open(_file, 'wb') as out: - out.write(self.plugin + '\n') - def ovs_ctxt(self): driver = neutron_plugin_attribute(self.plugin, 'driver', self.network_manager) @@ -1006,7 +997,6 @@ class NeutronContext(OSContextGenerator): flags = config_flags_parser(alchemy_flags) ctxt['neutron_alchemy_flags'] = flags - self._save_flag_file() return ctxt diff --git a/hooks/charmhelpers/contrib/openstack/utils.py b/hooks/charmhelpers/contrib/openstack/utils.py index cb61af38..b073c77b 100644 --- a/hooks/charmhelpers/contrib/openstack/utils.py +++ b/hooks/charmhelpers/contrib/openstack/utils.py @@ -618,7 +618,7 @@ def save_script_rc(script_path="scripts/scriptrc", **env_vars): juju_rc_path = "%s/%s" % (charm_dir(), script_path) if not os.path.exists(os.path.dirname(juju_rc_path)): os.mkdir(os.path.dirname(juju_rc_path)) - with open(juju_rc_path, 'wb') as rc_script: + with open(juju_rc_path, 'wt') as rc_script: rc_script.write( "#!/bin/bash\n") [rc_script.write('export %s=%s\n' % (u, p)) diff --git a/hooks/charmhelpers/core/hookenv.py b/hooks/charmhelpers/core/hookenv.py index 70085a40..5a88f798 100644 --- a/hooks/charmhelpers/core/hookenv.py +++ b/hooks/charmhelpers/core/hookenv.py @@ -22,6 +22,7 @@ from __future__ import print_function import copy from distutils.version import LooseVersion from functools import wraps +from collections import namedtuple import glob import os import json @@ -1164,3 +1165,42 @@ def meter_info(): """Get the meter status information, if running in the meter-status-changed hook.""" return os.environ.get('JUJU_METER_INFO') + + +def iter_units_for_relation_name(relation_name): + """Iterate through all units in a relation + + Generator that iterates through all the units in a relation and yields + a named tuple with rid and unit field names. + + Usage: + data = [(u.rid, u.unit) + for u in iter_units_for_relation_name(relation_name)] + + :param relation_name: string relation name + :yield: Named Tuple with rid and unit field names + """ + RelatedUnit = namedtuple('RelatedUnit', 'rid, unit') + for rid in relation_ids(relation_name): + for unit in related_units(rid): + yield RelatedUnit(rid, unit) + + +def ingress_address(rid=None, unit=None): + """ + Retrieve the ingress-address from a relation when available. Otherwise, + return the private-address. This function is to be used on the consuming + side of the relation. + + Usage: + addresses = [ingress_address(rid=u.rid, unit=u.unit) + for u in iter_units_for_relation_name(relation_name)] + + :param rid: string relation id + :param unit: string unit name + :side effect: calls relation_get + :return: string IP address + """ + settings = relation_get(rid=rid, unit=unit) + return (settings.get('ingress-address') or + settings.get('private-address')) diff --git a/hooks/charmhelpers/core/strutils.py b/hooks/charmhelpers/core/strutils.py index 685dabde..e8df0452 100644 --- a/hooks/charmhelpers/core/strutils.py +++ b/hooks/charmhelpers/core/strutils.py @@ -61,13 +61,19 @@ def bytes_from_string(value): if isinstance(value, six.string_types): value = six.text_type(value) else: - msg = "Unable to interpret non-string value '%s' as boolean" % (value) + msg = "Unable to interpret non-string value '%s' as bytes" % (value) raise ValueError(msg) matches = re.match("([0-9]+)([a-zA-Z]+)", value) - if not matches: - msg = "Unable to interpret string value '%s' as bytes" % (value) - raise ValueError(msg) - return int(matches.group(1)) * (1024 ** BYTE_POWER[matches.group(2)]) + if matches: + size = int(matches.group(1)) * (1024 ** BYTE_POWER[matches.group(2)]) + else: + # Assume that value passed in is bytes + try: + size = int(value) + except ValueError: + msg = "Unable to interpret string value '%s' as bytes" % (value) + raise ValueError(msg) + return size class BasicStringComparator(object): diff --git a/hooks/cinder_utils.py b/hooks/cinder_utils.py index 7920c3c8..963a8a1a 100644 --- a/hooks/cinder_utils.py +++ b/hooks/cinder_utils.py @@ -25,6 +25,10 @@ from charmhelpers.contrib.python.packages import ( pip_install, ) +from charmhelpers.core.strutils import ( + bytes_from_string +) + from charmhelpers.core.hookenv import ( charm_dir, config, @@ -517,7 +521,7 @@ def configure_lvm_storage(block_devices, volume_group, overwrite=False, if size == 0 and is_block_device(block_device): devices.append(block_device) elif size > 0: - devices.append(ensure_loopback_device(block_device, size)) + devices.append(ensure_loopback_device(block_device, str(size))) # NOTE(jamespage) # might need todo an initial one-time scrub on install if need be @@ -618,7 +622,7 @@ def _parse_block_device(block_device): else: bdev = block_device size = DEFAULT_LOOPBACK_SIZE - return (bdev, size) + return (bdev, bytes_from_string(str(size))) else: return ('/dev/{}'.format(block_device), 0) diff --git a/tests/charmhelpers/contrib/openstack/amulet/utils.py b/tests/charmhelpers/contrib/openstack/amulet/utils.py index 4fda829f..c24be8f1 100644 --- a/tests/charmhelpers/contrib/openstack/amulet/utils.py +++ b/tests/charmhelpers/contrib/openstack/amulet/utils.py @@ -628,6 +628,18 @@ class OpenStackAmuletUtils(AmuletUtils): _keypair = nova.keypairs.create(name=keypair_name) return _keypair + def _get_cinder_obj_name(self, cinder_object): + """Retrieve name of cinder object. + + :param cinder_object: cinder snapshot or volume object + :returns: str cinder object name + """ + # v1 objects store name in 'display_name' attr but v2+ use 'name' + try: + return cinder_object.display_name + except AttributeError: + return cinder_object.name + def create_cinder_volume(self, cinder, vol_name="demo-vol", vol_size=1, img_id=None, src_vol_id=None, snap_id=None): """Create cinder volume, optionally from a glance image, OR @@ -678,6 +690,13 @@ class OpenStackAmuletUtils(AmuletUtils): source_volid=src_vol_id, snapshot_id=snap_id) vol_id = vol_new.id + except TypeError: + vol_new = cinder.volumes.create(name=vol_name, + imageRef=img_id, + size=vol_size, + source_volid=src_vol_id, + snapshot_id=snap_id) + vol_id = vol_new.id except Exception as e: msg = 'Failed to create volume: {}'.format(e) amulet.raise_status(amulet.FAIL, msg=msg) @@ -692,7 +711,7 @@ class OpenStackAmuletUtils(AmuletUtils): # Re-validate new volume self.log.debug('Validating volume attributes...') - val_vol_name = cinder.volumes.get(vol_id).display_name + val_vol_name = self._get_cinder_obj_name(cinder.volumes.get(vol_id)) val_vol_boot = cinder.volumes.get(vol_id).bootable val_vol_stat = cinder.volumes.get(vol_id).status val_vol_size = cinder.volumes.get(vol_id).size diff --git a/tests/charmhelpers/core/hookenv.py b/tests/charmhelpers/core/hookenv.py index 70085a40..5a88f798 100644 --- a/tests/charmhelpers/core/hookenv.py +++ b/tests/charmhelpers/core/hookenv.py @@ -22,6 +22,7 @@ from __future__ import print_function import copy from distutils.version import LooseVersion from functools import wraps +from collections import namedtuple import glob import os import json @@ -1164,3 +1165,42 @@ def meter_info(): """Get the meter status information, if running in the meter-status-changed hook.""" return os.environ.get('JUJU_METER_INFO') + + +def iter_units_for_relation_name(relation_name): + """Iterate through all units in a relation + + Generator that iterates through all the units in a relation and yields + a named tuple with rid and unit field names. + + Usage: + data = [(u.rid, u.unit) + for u in iter_units_for_relation_name(relation_name)] + + :param relation_name: string relation name + :yield: Named Tuple with rid and unit field names + """ + RelatedUnit = namedtuple('RelatedUnit', 'rid, unit') + for rid in relation_ids(relation_name): + for unit in related_units(rid): + yield RelatedUnit(rid, unit) + + +def ingress_address(rid=None, unit=None): + """ + Retrieve the ingress-address from a relation when available. Otherwise, + return the private-address. This function is to be used on the consuming + side of the relation. + + Usage: + addresses = [ingress_address(rid=u.rid, unit=u.unit) + for u in iter_units_for_relation_name(relation_name)] + + :param rid: string relation id + :param unit: string unit name + :side effect: calls relation_get + :return: string IP address + """ + settings = relation_get(rid=rid, unit=unit) + return (settings.get('ingress-address') or + settings.get('private-address')) diff --git a/tests/charmhelpers/core/strutils.py b/tests/charmhelpers/core/strutils.py index 685dabde..e8df0452 100644 --- a/tests/charmhelpers/core/strutils.py +++ b/tests/charmhelpers/core/strutils.py @@ -61,13 +61,19 @@ def bytes_from_string(value): if isinstance(value, six.string_types): value = six.text_type(value) else: - msg = "Unable to interpret non-string value '%s' as boolean" % (value) + msg = "Unable to interpret non-string value '%s' as bytes" % (value) raise ValueError(msg) matches = re.match("([0-9]+)([a-zA-Z]+)", value) - if not matches: - msg = "Unable to interpret string value '%s' as bytes" % (value) - raise ValueError(msg) - return int(matches.group(1)) * (1024 ** BYTE_POWER[matches.group(2)]) + if matches: + size = int(matches.group(1)) * (1024 ** BYTE_POWER[matches.group(2)]) + else: + # Assume that value passed in is bytes + try: + size = int(value) + except ValueError: + msg = "Unable to interpret string value '%s' as bytes" % (value) + raise ValueError(msg) + return size class BasicStringComparator(object): diff --git a/unit_tests/test_cinder_utils.py b/unit_tests/test_cinder_utils.py index 9c5b56d4..5372741a 100644 --- a/unit_tests/test_cinder_utils.py +++ b/unit_tests/test_cinder_utils.py @@ -426,6 +426,7 @@ class TestCinderUtils(CharmTestCase): clean_storage, ensure_non_existent): devices = ['/dev/fakevbd', '/dev/fakevdc'] self.is_lvm_physical_volume.return_value = False + self.ensure_loopback_device.side_effect = lambda x, y: x cinder_utils.configure_lvm_storage(devices, 'test', True, True) clean_storage.assert_has_calls( [call('/dev/fakevbd'), @@ -450,6 +451,7 @@ class TestCinderUtils(CharmTestCase): 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 cinder_utils.configure_lvm_storage(devices, 'test', False, True) clean_storage.assert_has_calls( [call('/dev/fakevbd'), @@ -517,6 +519,7 @@ class TestCinderUtils(CharmTestCase): lvm_exists.return_value = False self.is_lvm_physical_volume.side_effect = pv_lookup self.list_lvm_volume_group.side_effect = vg_lookup + self.ensure_loopback_device.side_effect = lambda x, y: x cinder_utils.configure_lvm_storage(devices, 'test', True, True) clean_storage.assert_has_calls( [call('/dev/fakevdc')] @@ -552,6 +555,7 @@ class TestCinderUtils(CharmTestCase): self.is_lvm_physical_volume.side_effect = pv_lookup self.list_lvm_volume_group.side_effect = vg_lookup lvm_exists.return_value = False + self.ensure_loopback_device.side_effect = lambda x, y: x cinder_utils.configure_lvm_storage(devices, 'test', True, True) clean_storage.assert_called_with('/dev/fakevdc') self.create_lvm_physical_volume.assert_called_with('/dev/fakevdc') @@ -582,6 +586,7 @@ class TestCinderUtils(CharmTestCase): devices = ['/dev/fakevbd', '/dev/fakevdc'] self.is_lvm_physical_volume.side_effect = pv_lookup self.list_lvm_volume_group.side_effect = vg_lookup + self.ensure_loopback_device.side_effect = lambda x, y: x cinder_utils.configure_lvm_storage(devices, 'test', False, False) self.assertFalse(clean_storage.called) self.assertFalse(self.create_lvm_physical_volume.called)