diff --git a/charmhelpers/contrib/charmsupport/nrpe.py b/charmhelpers/contrib/charmsupport/nrpe.py index 424b7f7..80d574d 100644 --- a/charmhelpers/contrib/charmsupport/nrpe.py +++ b/charmhelpers/contrib/charmsupport/nrpe.py @@ -125,7 +125,7 @@ class CheckException(Exception): class Check(object): - shortname_re = '[A-Za-z0-9-_]+$' + shortname_re = '[A-Za-z0-9-_.]+$' service_template = (""" #--------------------------------------------------- # This file is Juju managed diff --git a/charmhelpers/contrib/openstack/context.py b/charmhelpers/contrib/openstack/context.py index 74ceb62..f67f326 100644 --- a/charmhelpers/contrib/openstack/context.py +++ b/charmhelpers/contrib/openstack/context.py @@ -41,9 +41,9 @@ from charmhelpers.core.hookenv import ( charm_name, DEBUG, INFO, - WARNING, ERROR, status_set, + network_get_primary_address ) from charmhelpers.core.sysctl import create as sysctl_create @@ -80,6 +80,9 @@ from charmhelpers.contrib.openstack.neutron import ( from charmhelpers.contrib.openstack.ip import ( resolve_address, INTERNAL, + ADMIN, + PUBLIC, + ADDRESS_MAP, ) from charmhelpers.contrib.network.ip import ( get_address_in_network, @@ -87,7 +90,6 @@ from charmhelpers.contrib.network.ip import ( get_ipv6_addr, get_netmask_for_address, format_ipv6_addr, - is_address_in_network, is_bridge_member, is_ipv6_disabled, ) @@ -517,7 +519,9 @@ class CephContext(OSContextGenerator): if not ctxt.get('key'): ctxt['key'] = relation_get('key', rid=rid, unit=unit) if not ctxt.get('rbd_features'): - ctxt['rbd_features'] = relation_get('rbd-features', rid=rid, unit=unit) + default_features = relation_get('rbd-features', rid=rid, unit=unit) + if default_features is not None: + ctxt['rbd_features'] = default_features ceph_addrs = relation_get('ceph-public-address', rid=rid, unit=unit) @@ -618,7 +622,6 @@ class HAProxyContext(OSContextGenerator): ctxt['haproxy_connect_timeout'] = config('haproxy-connect-timeout') if config('prefer-ipv6'): - ctxt['ipv6'] = True ctxt['local_host'] = 'ip6-localhost' ctxt['haproxy_host'] = '::' else: @@ -734,11 +737,17 @@ class ApacheSSLContext(OSContextGenerator): return sorted(list(set(cns))) def get_network_addresses(self): - """For each network configured, return corresponding address and vip - (if available). + """For each network configured, return corresponding address and + hostnamr or vip (if available). Returns a list of tuples of the form: + [(address_in_net_a, hostname_in_net_a), + (address_in_net_b, hostname_in_net_b), + ...] + + or, if no hostnames(s) available: + [(address_in_net_a, vip_in_net_a), (address_in_net_b, vip_in_net_b), ...] @@ -750,32 +759,27 @@ class ApacheSSLContext(OSContextGenerator): ...] """ addresses = [] - if config('vip'): - vips = config('vip').split() - else: - vips = [] - - for net_type in ['os-internal-network', 'os-admin-network', - 'os-public-network']: - addr = get_address_in_network(config(net_type), - unit_get('private-address')) - if len(vips) > 1 and is_clustered(): - if not config(net_type): - log("Multiple networks configured but net_type " - "is None (%s)." % net_type, level=WARNING) - continue - - for vip in vips: - if is_address_in_network(config(net_type), vip): - addresses.append((addr, vip)) - break - - elif is_clustered() and config('vip'): - addresses.append((addr, config('vip'))) + for net_type in [INTERNAL, ADMIN, PUBLIC]: + net_config = config(ADDRESS_MAP[net_type]['config']) + # NOTE(jamespage): Fallback must always be private address + # as this is used to bind services on the + # local unit. + fallback = unit_get("private-address") + if net_config: + addr = get_address_in_network(net_config, + fallback) else: - addresses.append((addr, addr)) + try: + addr = network_get_primary_address( + ADDRESS_MAP[net_type]['binding'] + ) + except NotImplementedError: + addr = fallback - return sorted(addresses) + endpoint = resolve_address(net_type) + addresses.append((addr, endpoint)) + + return sorted(set(addresses)) def __call__(self): if isinstance(self.external_ports, six.string_types): @@ -802,7 +806,7 @@ class ApacheSSLContext(OSContextGenerator): self.configure_cert(cn) addresses = self.get_network_addresses() - for address, endpoint in sorted(set(addresses)): + for address, endpoint in addresses: for api_port in self.external_ports: ext_port = determine_apache_port(api_port, singlenode_mode=True) @@ -1417,14 +1421,26 @@ class NeutronAPIContext(OSContextGenerator): 'rel_key': 'report-interval', 'default': 30, }, + 'enable_qos': { + 'rel_key': 'enable-qos', + 'default': False, + }, } ctxt = self.get_neutron_options({}) for rid in relation_ids('neutron-plugin-api'): for unit in related_units(rid): rdata = relation_get(rid=rid, unit=unit) + # The l2-population key is used by the context as a way of + # checking if the api service on the other end is sending data + # in a recent format. if 'l2-population' in rdata: ctxt.update(self.get_neutron_options(rdata)) + if ctxt['enable_qos']: + ctxt['extension_drivers'] = 'qos' + else: + ctxt['extension_drivers'] = '' + return ctxt def get_neutron_options(self, rdata): diff --git a/charmhelpers/contrib/openstack/templates/haproxy.cfg b/charmhelpers/contrib/openstack/templates/haproxy.cfg index edae7a0..2e66045 100644 --- a/charmhelpers/contrib/openstack/templates/haproxy.cfg +++ b/charmhelpers/contrib/openstack/templates/haproxy.cfg @@ -48,9 +48,7 @@ listen stats {% for service, ports in service_ports.items() -%} frontend tcp-in_{{ service }} bind *:{{ ports[0] }} - {% if ipv6 -%} bind :::{{ ports[0] }} - {% endif -%} {% for frontend in frontends -%} acl net_{{ frontend }} dst {{ frontends[frontend]['network'] }} use_backend {{ service }}_{{ frontend }} if net_{{ frontend }} diff --git a/charmhelpers/contrib/openstack/templates/section-oslo-notifications b/charmhelpers/contrib/openstack/templates/section-oslo-notifications new file mode 100644 index 0000000..5dccd4b --- /dev/null +++ b/charmhelpers/contrib/openstack/templates/section-oslo-notifications @@ -0,0 +1,8 @@ +{% if transport_url -%} +[oslo_messaging_notifications] +driver = messagingv2 +transport_url = {{ transport_url }} +{% if notification_topics -%} +topics = {{ notification_topics }} +{% endif -%} +{% endif -%} diff --git a/charmhelpers/contrib/openstack/templating.py b/charmhelpers/contrib/openstack/templating.py index 934baf5..d8c1fc7 100644 --- a/charmhelpers/contrib/openstack/templating.py +++ b/charmhelpers/contrib/openstack/templating.py @@ -20,7 +20,8 @@ from charmhelpers.fetch import apt_install, apt_update from charmhelpers.core.hookenv import ( log, ERROR, - INFO + INFO, + TRACE ) from charmhelpers.contrib.openstack.utils import OPENSTACK_CODENAMES @@ -80,8 +81,10 @@ def get_loader(templates_dir, os_release): loaders.insert(0, FileSystemLoader(tmpl_dir)) if rel == os_release: break + # demote this log to the lowest level; we don't really need to see these + # lots in production even when debugging. log('Creating choice loader with dirs: %s' % - [l.searchpath for l in loaders], level=INFO) + [l.searchpath for l in loaders], level=TRACE) return ChoiceLoader(loaders) diff --git a/charmhelpers/contrib/openstack/utils.py b/charmhelpers/contrib/openstack/utils.py index d15a631..837a167 100644 --- a/charmhelpers/contrib/openstack/utils.py +++ b/charmhelpers/contrib/openstack/utils.py @@ -186,7 +186,7 @@ SWIFT_CODENAMES = OrderedDict([ ('ocata', ['2.11.0', '2.12.0', '2.13.0']), ('pike', - ['2.13.0']), + ['2.13.0', '2.15.0']), ]) # >= Liberty version->codename mapping @@ -2051,7 +2051,7 @@ def snap_install_requested(): If openstack-origin is of the form snap:channel-series-release and channel is in SNAPS_CHANNELS return True. """ - origin = config('openstack-origin') + origin = config('openstack-origin') or "" if not origin.startswith('snap:'): return False diff --git a/charmhelpers/core/hookenv.py b/charmhelpers/core/hookenv.py index e44e22b..12f37b2 100644 --- a/charmhelpers/core/hookenv.py +++ b/charmhelpers/core/hookenv.py @@ -43,6 +43,7 @@ ERROR = "ERROR" WARNING = "WARNING" INFO = "INFO" DEBUG = "DEBUG" +TRACE = "TRACE" MARKER = object() cache = {} @@ -202,6 +203,27 @@ def service_name(): return local_unit().split('/')[0] +def principal_unit(): + """Returns the principal unit of this unit, otherwise None""" + # Juju 2.2 and above provides JUJU_PRINCIPAL_UNIT + principal_unit = os.environ.get('JUJU_PRINCIPAL_UNIT', None) + # If it's empty, then this unit is the principal + if principal_unit == '': + return os.environ['JUJU_UNIT_NAME'] + elif principal_unit is not None: + return principal_unit + # For Juju 2.1 and below, let's try work out the principle unit by + # the various charms' metadata.yaml. + for reltype in relation_types(): + for rid in relation_ids(reltype): + for unit in related_units(rid): + md = _metadata_unit(unit) + subordinate = md.pop('subordinate', None) + if not subordinate: + return unit + return None + + @cached def remote_service_name(relid=None): """The remote service name for a given relation-id (or the current relation)""" @@ -478,6 +500,21 @@ def metadata(): return yaml.safe_load(md) +def _metadata_unit(unit): + """Given the name of a unit (e.g. apache2/0), get the unit charm's + metadata.yaml. Very similar to metadata() but allows us to inspect + other units. Unit needs to be co-located, such as a subordinate or + principal/primary. + + :returns: metadata.yaml as a python object. + + """ + basedir = os.sep.join(charm_dir().split(os.sep)[:-2]) + unitdir = 'unit-{}'.format(unit.replace(os.sep, '-')) + with open(os.path.join(basedir, unitdir, 'charm', 'metadata.yaml')) as md: + return yaml.safe_load(md) + + @cached def relation_types(): """Get a list of relation types supported by this charm""" @@ -753,6 +790,9 @@ class Hooks(object): def charm_dir(): """Return the root directory of the current charm""" + d = os.environ.get('JUJU_CHARM_DIR') + if d is not None: + return d return os.environ.get('CHARM_DIR') diff --git a/charmhelpers/core/host.py b/charmhelpers/core/host.py index b0043cb..5656e2f 100644 --- a/charmhelpers/core/host.py +++ b/charmhelpers/core/host.py @@ -34,7 +34,7 @@ import six from contextlib import contextmanager from collections import OrderedDict -from .hookenv import log +from .hookenv import log, DEBUG from .fstab import Fstab from charmhelpers.osplatform import get_platform @@ -487,13 +487,37 @@ def mkdir(path, owner='root', group='root', perms=0o555, force=False): def write_file(path, content, owner='root', group='root', perms=0o444): """Create or overwrite a file with the contents of a byte string.""" - log("Writing file {} {}:{} {:o}".format(path, owner, group, perms)) uid = pwd.getpwnam(owner).pw_uid gid = grp.getgrnam(group).gr_gid - with open(path, 'wb') as target: - os.fchown(target.fileno(), uid, gid) - os.fchmod(target.fileno(), perms) - target.write(content) + # lets see if we can grab the file and compare the context, to avoid doing + # a write. + existing_content = None + existing_uid, existing_gid = None, None + try: + with open(path, 'rb') as target: + existing_content = target.read() + stat = os.stat(path) + existing_uid, existing_gid = stat.st_uid, stat.st_gid + except: + pass + if content != existing_content: + log("Writing file {} {}:{} {:o}".format(path, owner, group, perms), + level=DEBUG) + with open(path, 'wb') as target: + os.fchown(target.fileno(), uid, gid) + os.fchmod(target.fileno(), perms) + target.write(content) + return + # the contents were the same, but we might still need to change the + # ownership. + if existing_uid != uid: + log("Changing uid on already existing content: {} -> {}" + .format(existing_uid, uid), level=DEBUG) + os.chown(path, uid, -1) + if existing_gid != gid: + log("Changing gid on already existing content: {} -> {}" + .format(existing_gid, gid), level=DEBUG) + os.chown(path, -1, gid) def fstab_remove(mp): diff --git a/charmhelpers/fetch/ubuntu.py b/charmhelpers/fetch/ubuntu.py index 545348f..40e1cb5 100644 --- a/charmhelpers/fetch/ubuntu.py +++ b/charmhelpers/fetch/ubuntu.py @@ -27,6 +27,7 @@ from charmhelpers.core.host import ( from charmhelpers.core.hookenv import ( log, DEBUG, + WARNING, ) from charmhelpers.fetch import SourceConfigError, GPGKeyError @@ -261,34 +262,47 @@ def apt_unhold(packages, fatal=False): return apt_mark(packages, 'unhold', fatal=fatal) -def import_key(keyid): - """Import a key in either ASCII Armor or Radix64 format. +def import_key(key): + """Import an ASCII Armor key. - `keyid` is either the keyid to fetch from a PGP server, or - the key in ASCII armor foramt. + /!\ A Radix64 format keyid is also supported for backwards + compatibility, but should never be used; the key retrieval + mechanism is insecure and subject to man-in-the-middle attacks + voiding all signature checks using that key. - :param keyid: String of key (or key id). + :param keyid: The key in ASCII armor format, + including BEGIN and END markers. :raises: GPGKeyError if the key could not be imported """ - key = keyid.strip() - if (key.startswith('-----BEGIN PGP PUBLIC KEY BLOCK-----') and - key.endswith('-----END PGP PUBLIC KEY BLOCK-----')): + key = key.strip() + if '-' in key or '\n' in key: + # Send everything not obviously a keyid to GPG to import, as + # we trust its validation better than our own. eg. handling + # comments before the key. log("PGP key found (looks like ASCII Armor format)", level=DEBUG) - log("Importing ASCII Armor PGP key", level=DEBUG) - with NamedTemporaryFile() as keyfile: - with open(keyfile.name, 'w') as fd: - fd.write(key) - fd.write("\n") - cmd = ['apt-key', 'add', keyfile.name] - try: - subprocess.check_call(cmd) - except subprocess.CalledProcessError: - error = "Error importing PGP key '{}'".format(key) - log(error) - raise GPGKeyError(error) + if ('-----BEGIN PGP PUBLIC KEY BLOCK-----' in key and + '-----END PGP PUBLIC KEY BLOCK-----' in key): + log("Importing ASCII Armor PGP key", level=DEBUG) + with NamedTemporaryFile() as keyfile: + with open(keyfile.name, 'w') as fd: + fd.write(key) + fd.write("\n") + cmd = ['apt-key', 'add', keyfile.name] + try: + subprocess.check_call(cmd) + except subprocess.CalledProcessError: + error = "Error importing PGP key '{}'".format(key) + log(error) + raise GPGKeyError(error) + else: + raise GPGKeyError("ASCII armor markers missing from GPG key") else: - log("PGP key found (looks like Radix64 format)", level=DEBUG) - log("Importing PGP key from keyserver", level=DEBUG) + # We should only send things obviously not a keyid offsite + # via this unsecured protocol, as it may be a secret or part + # of one. + log("PGP key found (looks like Radix64 format)", level=WARNING) + log("INSECURLY importing PGP key from keyserver; " + "full key not provided.", level=WARNING) cmd = ['apt-key', 'adv', '--keyserver', 'hkp://keyserver.ubuntu.com:80', '--recv-keys', key] try: diff --git a/hooks/ceilometer_hooks.py b/hooks/ceilometer_hooks.py index 2e259bf..5236c12 100755 --- a/hooks/ceilometer_hooks.py +++ b/hooks/ceilometer_hooks.py @@ -34,6 +34,7 @@ from charmhelpers.core.hookenv import ( Hooks, UnregisteredHookError, log, status_set, + WARNING, ) from charmhelpers.core.host import ( service_restart, @@ -82,6 +83,7 @@ from charmhelpers.contrib.network.ip import ( get_iface_for_address, get_netmask_for_address, get_relation_ip, + is_ipv6, ) from charmhelpers.contrib.hahelpers.cluster import ( get_hacluster_config, @@ -331,12 +333,24 @@ def ha_joined(relation_id=None): else: vip_group = [] for vip in cluster_config['vip'].split(): - res_ceilometer_vip = 'ocf:heartbeat:IPaddr2' - vip_params = 'ip' + if is_ipv6(vip): + res_ceilometer_vip = 'ocf:heartbeat:IPv6addr' + vip_params = 'ipv6addr' + else: + res_ceilometer_vip = 'ocf:heartbeat:IPaddr2' + vip_params = 'ip' iface = get_iface_for_address(vip) if iface is not None: vip_key = 'res_ceilometer_{}_vip'.format(iface) + if vip_key in vip_group: + if vip not in resource_params[vip_key]: + vip_key = '{}_{}'.format(vip_key, vip_params) + else: + log("Resource '%s' (vip='%s') already exists in " + "vip group - skipping" % (vip_key, vip), WARNING) + continue + resources[vip_key] = res_ceilometer_vip resource_params[vip_key] = ( 'params {ip}="{vip}" cidr_netmask="{netmask}"' diff --git a/tests/charmhelpers/core/hookenv.py b/tests/charmhelpers/core/hookenv.py index e44e22b..12f37b2 100644 --- a/tests/charmhelpers/core/hookenv.py +++ b/tests/charmhelpers/core/hookenv.py @@ -43,6 +43,7 @@ ERROR = "ERROR" WARNING = "WARNING" INFO = "INFO" DEBUG = "DEBUG" +TRACE = "TRACE" MARKER = object() cache = {} @@ -202,6 +203,27 @@ def service_name(): return local_unit().split('/')[0] +def principal_unit(): + """Returns the principal unit of this unit, otherwise None""" + # Juju 2.2 and above provides JUJU_PRINCIPAL_UNIT + principal_unit = os.environ.get('JUJU_PRINCIPAL_UNIT', None) + # If it's empty, then this unit is the principal + if principal_unit == '': + return os.environ['JUJU_UNIT_NAME'] + elif principal_unit is not None: + return principal_unit + # For Juju 2.1 and below, let's try work out the principle unit by + # the various charms' metadata.yaml. + for reltype in relation_types(): + for rid in relation_ids(reltype): + for unit in related_units(rid): + md = _metadata_unit(unit) + subordinate = md.pop('subordinate', None) + if not subordinate: + return unit + return None + + @cached def remote_service_name(relid=None): """The remote service name for a given relation-id (or the current relation)""" @@ -478,6 +500,21 @@ def metadata(): return yaml.safe_load(md) +def _metadata_unit(unit): + """Given the name of a unit (e.g. apache2/0), get the unit charm's + metadata.yaml. Very similar to metadata() but allows us to inspect + other units. Unit needs to be co-located, such as a subordinate or + principal/primary. + + :returns: metadata.yaml as a python object. + + """ + basedir = os.sep.join(charm_dir().split(os.sep)[:-2]) + unitdir = 'unit-{}'.format(unit.replace(os.sep, '-')) + with open(os.path.join(basedir, unitdir, 'charm', 'metadata.yaml')) as md: + return yaml.safe_load(md) + + @cached def relation_types(): """Get a list of relation types supported by this charm""" @@ -753,6 +790,9 @@ class Hooks(object): def charm_dir(): """Return the root directory of the current charm""" + d = os.environ.get('JUJU_CHARM_DIR') + if d is not None: + return d return os.environ.get('CHARM_DIR') diff --git a/tests/charmhelpers/core/host.py b/tests/charmhelpers/core/host.py index b0043cb..5656e2f 100644 --- a/tests/charmhelpers/core/host.py +++ b/tests/charmhelpers/core/host.py @@ -34,7 +34,7 @@ import six from contextlib import contextmanager from collections import OrderedDict -from .hookenv import log +from .hookenv import log, DEBUG from .fstab import Fstab from charmhelpers.osplatform import get_platform @@ -487,13 +487,37 @@ def mkdir(path, owner='root', group='root', perms=0o555, force=False): def write_file(path, content, owner='root', group='root', perms=0o444): """Create or overwrite a file with the contents of a byte string.""" - log("Writing file {} {}:{} {:o}".format(path, owner, group, perms)) uid = pwd.getpwnam(owner).pw_uid gid = grp.getgrnam(group).gr_gid - with open(path, 'wb') as target: - os.fchown(target.fileno(), uid, gid) - os.fchmod(target.fileno(), perms) - target.write(content) + # lets see if we can grab the file and compare the context, to avoid doing + # a write. + existing_content = None + existing_uid, existing_gid = None, None + try: + with open(path, 'rb') as target: + existing_content = target.read() + stat = os.stat(path) + existing_uid, existing_gid = stat.st_uid, stat.st_gid + except: + pass + if content != existing_content: + log("Writing file {} {}:{} {:o}".format(path, owner, group, perms), + level=DEBUG) + with open(path, 'wb') as target: + os.fchown(target.fileno(), uid, gid) + os.fchmod(target.fileno(), perms) + target.write(content) + return + # the contents were the same, but we might still need to change the + # ownership. + if existing_uid != uid: + log("Changing uid on already existing content: {} -> {}" + .format(existing_uid, uid), level=DEBUG) + os.chown(path, uid, -1) + if existing_gid != gid: + log("Changing gid on already existing content: {} -> {}" + .format(existing_gid, gid), level=DEBUG) + os.chown(path, -1, gid) def fstab_remove(mp):