From f36b718162c86889f07dc7e77ad610b481018f3a Mon Sep 17 00:00:00 2001 From: Ryan Beisner Date: Fri, 19 Jan 2018 12:10:37 +0000 Subject: [PATCH] Sync charm-helpers Notable issues resolved: openstack_upgrade_available() broken for swift https://bugs.launchpad.net/charm-swift-proxy/+bug/1743847 haproxy context doesn't consider bindings https://bugs.launchpad.net/charm-helpers/+bug/1735421 regression in haproxy check https://bugs.launchpad.net/charm-helpers/+bug/1743287 This change also breaks the inheritance between HorizonHAProxyContext and HAProxyContext (as this was not needed) and aligns the code in HorizonHAProxyContext to the behaviour in HAProxyContext of using get_relation_ip instead of unit_get. Change-Id: I72600b01744a07e140a29255efc2823ea39ce730 --- hooks/charmhelpers/contrib/network/ip.py | 4 + .../charmhelpers/contrib/openstack/context.py | 60 ++++-- .../contrib/openstack/files/check_haproxy.sh | 2 +- .../files/check_haproxy_queue_depth.sh | 2 +- .../contrib/openstack/ha/utils.py | 175 +++++++++++++++--- .../templates/wsgi-openstack-api.conf | 15 +- .../contrib/openstack/templating.py | 91 ++++++--- hooks/charmhelpers/contrib/openstack/utils.py | 5 - hooks/charmhelpers/core/hookenv.py | 18 +- hooks/charmhelpers/core/unitdata.py | 2 + hooks/horizon_contexts.py | 7 +- hooks/horizon_utils.py | 3 +- tests/charmhelpers/core/hookenv.py | 18 +- tests/charmhelpers/core/unitdata.py | 2 + unit_tests/test_horizon_contexts.py | 8 +- 15 files changed, 324 insertions(+), 88 deletions(-) diff --git a/hooks/charmhelpers/contrib/network/ip.py b/hooks/charmhelpers/contrib/network/ip.py index a871ce37..9483e257 100644 --- a/hooks/charmhelpers/contrib/network/ip.py +++ b/hooks/charmhelpers/contrib/network/ip.py @@ -27,6 +27,7 @@ from charmhelpers.core.hookenv import ( network_get_primary_address, unit_get, WARNING, + NoNetworkBinding, ) from charmhelpers.core.host import ( @@ -578,6 +579,9 @@ def get_relation_ip(interface, cidr_network=None): except NotImplementedError: # If network-get is not available address = get_host_ip(unit_get('private-address')) + except NoNetworkBinding: + log("No network binding for {}".format(interface), WARNING) + address = get_host_ip(unit_get('private-address')) if config('prefer-ipv6'): # Currently IPv6 has priority, eventually we want IPv6 to just be diff --git a/hooks/charmhelpers/contrib/openstack/context.py b/hooks/charmhelpers/contrib/openstack/context.py index 161be128..52d0cebf 100644 --- a/hooks/charmhelpers/contrib/openstack/context.py +++ b/hooks/charmhelpers/contrib/openstack/context.py @@ -93,10 +93,10 @@ from charmhelpers.contrib.network.ip import ( format_ipv6_addr, is_bridge_member, is_ipv6_disabled, + get_relation_ip, ) from charmhelpers.contrib.openstack.utils import ( config_flags_parser, - get_host_ip, enable_memcache, snap_install_requested, CompareOpenStackReleases, @@ -617,7 +617,9 @@ class HAProxyContext(OSContextGenerator): """ interfaces = ['cluster'] - def __init__(self, singlenode_mode=False): + def __init__(self, singlenode_mode=False, + address_types=ADDRESS_TYPES): + self.address_types = address_types self.singlenode_mode = singlenode_mode def __call__(self): @@ -626,19 +628,22 @@ class HAProxyContext(OSContextGenerator): if not relation_ids('cluster') and not self.singlenode_mode: return {} - if config('prefer-ipv6'): - addr = get_ipv6_addr(exc_list=[config('vip')])[0] - else: - addr = get_host_ip(unit_get('private-address')) - l_unit = local_unit().replace('/', '-') cluster_hosts = {} # NOTE(jamespage): build out map of configured network endpoints # and associated backends - for addr_type in ADDRESS_TYPES: + for addr_type in self.address_types: cfg_opt = 'os-{}-network'.format(addr_type) - laddr = get_address_in_network(config(cfg_opt)) + # NOTE(thedac) For some reason the ADDRESS_MAP uses 'int' rather + # than 'internal' + if addr_type == 'internal': + _addr_map_type = INTERNAL + else: + _addr_map_type = addr_type + # Network spaces aware + laddr = get_relation_ip(ADDRESS_MAP[_addr_map_type]['binding'], + config(cfg_opt)) if laddr: netmask = get_netmask_for_address(laddr) cluster_hosts[laddr] = { @@ -649,15 +654,19 @@ class HAProxyContext(OSContextGenerator): } for rid in relation_ids('cluster'): for unit in sorted(related_units(rid)): + # API Charms will need to set {addr_type}-address with + # get_relation_ip(addr_type) _laddr = relation_get('{}-address'.format(addr_type), rid=rid, unit=unit) if _laddr: _unit = unit.replace('/', '-') cluster_hosts[laddr]['backends'][_unit] = _laddr - # NOTE(jamespage) add backend based on private address - this - # with either be the only backend or the fallback if no acls + # NOTE(jamespage) add backend based on get_relation_ip - this + # will either be the only backend or the fallback if no acls # match in the frontend + # Network spaces aware + addr = get_relation_ip('cluster') cluster_hosts[addr] = {} netmask = get_netmask_for_address(addr) cluster_hosts[addr] = { @@ -667,6 +676,8 @@ class HAProxyContext(OSContextGenerator): } for rid in relation_ids('cluster'): for unit in sorted(related_units(rid)): + # API Charms will need to set their private-address with + # get_relation_ip('cluster') _laddr = relation_get('private-address', rid=rid, unit=unit) if _laddr: @@ -1775,3 +1786,30 @@ class MemcacheContext(OSContextGenerator): ctxt['memcache_server_formatted'], ctxt['memcache_port']) return ctxt + + +class EnsureDirContext(OSContextGenerator): + ''' + Serves as a generic context to create a directory as a side-effect. + + Useful for software that supports drop-in files (.d) in conjunction + with config option-based templates. Examples include: + * OpenStack oslo.policy drop-in files; + * systemd drop-in config files; + * other software that supports overriding defaults with .d files + + Another use-case is when a subordinate generates a configuration for + primary to render in a separate directory. + + Some software requires a user to create a target directory to be + scanned for drop-in files with a specific format. This is why this + context is needed to do that before rendering a template. + ''' + + def __init__(self, dirname): + '''Used merely to ensure that a given directory exists.''' + self.dirname = dirname + + def __call__(self): + mkdir(self.dirname) + return {} diff --git a/hooks/charmhelpers/contrib/openstack/files/check_haproxy.sh b/hooks/charmhelpers/contrib/openstack/files/check_haproxy.sh index 7aab129a..1df55db4 100755 --- a/hooks/charmhelpers/contrib/openstack/files/check_haproxy.sh +++ b/hooks/charmhelpers/contrib/openstack/files/check_haproxy.sh @@ -9,7 +9,7 @@ CRITICAL=0 NOTACTIVE='' LOGFILE=/var/log/nagios/check_haproxy.log -AUTH=$(grep -r "stats auth" /etc/haproxy/haproxy.cfg | awk 'NR=1{print $4}') +AUTH=$(grep -r "stats auth" /etc/haproxy/haproxy.cfg | awk 'NR=1{print $3}') typeset -i N_INSTANCES=0 for appserver in $(awk '/^\s+server/{print $2}' /etc/haproxy/haproxy.cfg) diff --git a/hooks/charmhelpers/contrib/openstack/files/check_haproxy_queue_depth.sh b/hooks/charmhelpers/contrib/openstack/files/check_haproxy_queue_depth.sh index 3ebb5329..91ce0246 100755 --- a/hooks/charmhelpers/contrib/openstack/files/check_haproxy_queue_depth.sh +++ b/hooks/charmhelpers/contrib/openstack/files/check_haproxy_queue_depth.sh @@ -10,7 +10,7 @@ CURRQthrsh=0 MAXQthrsh=100 -AUTH=$(grep -r "stats auth" /etc/haproxy | head -1 | awk '{print $4}') +AUTH=$(grep -r "stats auth" /etc/haproxy/haproxy.cfg | awk 'NR=1{print $3}') HAPROXYSTATS=$(/usr/lib/nagios/plugins/check_http -a ${AUTH} -I 127.0.0.1 -p 8888 -u '/;csv' -v) diff --git a/hooks/charmhelpers/contrib/openstack/ha/utils.py b/hooks/charmhelpers/contrib/openstack/ha/utils.py index 9a4d79c1..6060ae50 100644 --- a/hooks/charmhelpers/contrib/openstack/ha/utils.py +++ b/hooks/charmhelpers/contrib/openstack/ha/utils.py @@ -23,6 +23,8 @@ Helpers for high availability. """ +import json + import re from charmhelpers.core.hookenv import ( @@ -32,6 +34,7 @@ from charmhelpers.core.hookenv import ( config, status_set, DEBUG, + WARNING, ) from charmhelpers.core.host import ( @@ -40,6 +43,23 @@ from charmhelpers.core.host import ( from charmhelpers.contrib.openstack.ip import ( resolve_address, + is_ipv6, +) + +from charmhelpers.contrib.network.ip import ( + get_iface_for_address, + get_netmask_for_address, +) + +from charmhelpers.contrib.hahelpers.cluster import ( + get_hacluster_config +) + +JSON_ENCODE_OPTIONS = dict( + sort_keys=True, + allow_nan=False, + indent=None, + separators=(',', ':'), ) @@ -53,8 +73,8 @@ class DNSHAException(Exception): def update_dns_ha_resource_params(resources, resource_params, relation_id=None, crm_ocf='ocf:maas:dns'): - """ Check for os-*-hostname settings and update resource dictionaries for - the HA relation. + """ Configure DNS-HA resources based on provided configuration and + update resource dictionaries for the HA relation. @param resources: Pointer to dictionary of resources. Usually instantiated in ha_joined(). @@ -64,7 +84,85 @@ def update_dns_ha_resource_params(resources, resource_params, @param crm_ocf: Corosync Open Cluster Framework resource agent to use for DNS HA """ + _relation_data = {'resources': {}, 'resource_params': {}} + update_hacluster_dns_ha(charm_name(), + _relation_data, + crm_ocf) + resources.update(_relation_data['resources']) + resource_params.update(_relation_data['resource_params']) + relation_set(relation_id=relation_id, groups=_relation_data['groups']) + +def assert_charm_supports_dns_ha(): + """Validate prerequisites for DNS HA + The MAAS client is only available on Xenial or greater + + :raises DNSHAException: if release is < 16.04 + """ + if lsb_release().get('DISTRIB_RELEASE') < '16.04': + msg = ('DNS HA is only supported on 16.04 and greater ' + 'versions of Ubuntu.') + status_set('blocked', msg) + raise DNSHAException(msg) + return True + + +def expect_ha(): + """ Determine if the unit expects to be in HA + + Check for VIP or dns-ha settings which indicate the unit should expect to + be related to hacluster. + + @returns boolean + """ + return config('vip') or config('dns-ha') + + +def generate_ha_relation_data(service): + """ Generate relation data for ha relation + + Based on configuration options and unit interfaces, generate a json + encoded dict of relation data items for the hacluster relation, + providing configuration for DNS HA or VIP's + haproxy clone sets. + + @returns dict: json encoded data for use with relation_set + """ + _haproxy_res = 'res_{}_haproxy'.format(service) + _relation_data = { + 'resources': { + _haproxy_res: 'lsb:haproxy', + }, + 'resource_params': { + _haproxy_res: 'op monitor interval="5s"' + }, + 'init_services': { + _haproxy_res: 'haproxy' + }, + 'clones': { + 'cl_{}_haproxy'.format(service): _haproxy_res + }, + } + + if config('dns-ha'): + update_hacluster_dns_ha(service, _relation_data) + else: + update_hacluster_vip(service, _relation_data) + + return { + 'json_{}'.format(k): json.dumps(v, **JSON_ENCODE_OPTIONS) + for k, v in _relation_data.items() if v + } + + +def update_hacluster_dns_ha(service, relation_data, + crm_ocf='ocf:maas:dns'): + """ Configure DNS-HA resources based on provided configuration + + @param service: Name of the service being configured + @param relation_data: Pointer to dictionary of relation data. + @param crm_ocf: Corosync Open Cluster Framework resource agent to use for + DNS HA + """ # Validate the charm environment for DNS HA assert_charm_supports_dns_ha() @@ -93,7 +191,7 @@ def update_dns_ha_resource_params(resources, resource_params, status_set('blocked', msg) raise DNSHAException(msg) - hostname_key = 'res_{}_{}_hostname'.format(charm_name(), endpoint_type) + hostname_key = 'res_{}_{}_hostname'.format(service, endpoint_type) if hostname_key in hostname_group: log('DNS HA: Resource {}: {} already exists in ' 'hostname group - skipping'.format(hostname_key, hostname), @@ -101,42 +199,67 @@ def update_dns_ha_resource_params(resources, resource_params, continue hostname_group.append(hostname_key) - resources[hostname_key] = crm_ocf - resource_params[hostname_key] = ( - 'params fqdn="{}" ip_address="{}" ' - ''.format(hostname, resolve_address(endpoint_type=endpoint_type, - override=False))) + relation_data['resources'][hostname_key] = crm_ocf + relation_data['resource_params'][hostname_key] = ( + 'params fqdn="{}" ip_address="{}"' + .format(hostname, resolve_address(endpoint_type=endpoint_type, + override=False))) if len(hostname_group) >= 1: log('DNS HA: Hostname group is set with {} as members. ' 'Informing the ha relation'.format(' '.join(hostname_group)), DEBUG) - relation_set(relation_id=relation_id, groups={ - 'grp_{}_hostnames'.format(charm_name()): ' '.join(hostname_group)}) + relation_data['groups'] = { + 'grp_{}_hostnames'.format(service): ' '.join(hostname_group) + } else: msg = 'DNS HA: Hostname group has no members.' status_set('blocked', msg) raise DNSHAException(msg) -def assert_charm_supports_dns_ha(): - """Validate prerequisites for DNS HA - The MAAS client is only available on Xenial or greater +def update_hacluster_vip(service, relation_data): + """ Configure VIP resources based on provided configuration + + @param service: Name of the service being configured + @param relation_data: Pointer to dictionary of relation data. """ - if lsb_release().get('DISTRIB_RELEASE') < '16.04': - msg = ('DNS HA is only supported on 16.04 and greater ' - 'versions of Ubuntu.') - status_set('blocked', msg) - raise DNSHAException(msg) - return True + cluster_config = get_hacluster_config() + vip_group = [] + for vip in cluster_config['vip'].split(): + if is_ipv6(vip): + res_neutron_vip = 'ocf:heartbeat:IPv6addr' + vip_params = 'ipv6addr' + else: + res_neutron_vip = 'ocf:heartbeat:IPaddr2' + vip_params = 'ip' + iface = (get_iface_for_address(vip) or + config('vip_iface')) + netmask = (get_netmask_for_address(vip) or + config('vip_cidr')) -def expect_ha(): - """ Determine if the unit expects to be in HA + if iface is not None: + vip_key = 'res_{}_{}_vip'.format(service, iface) + if vip_key in vip_group: + if vip not in relation_data['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 - Check for VIP or dns-ha settings which indicate the unit should expect to - be related to hacluster. + relation_data['resources'][vip_key] = res_neutron_vip + relation_data['resource_params'][vip_key] = ( + 'params {ip}="{vip}" cidr_netmask="{netmask}" ' + 'nic="{iface}"'.format(ip=vip_params, + vip=vip, + iface=iface, + netmask=netmask) + ) + vip_group.append(vip_key) - @returns boolean - """ - return config('vip') or config('dns-ha') + if len(vip_group) >= 1: + relation_data['groups'] = { + 'grp_{}_vips'.format(service): ' '.join(vip_group) + } diff --git a/hooks/charmhelpers/contrib/openstack/templates/wsgi-openstack-api.conf b/hooks/charmhelpers/contrib/openstack/templates/wsgi-openstack-api.conf index 315b2a3f..e2e73b2c 100644 --- a/hooks/charmhelpers/contrib/openstack/templates/wsgi-openstack-api.conf +++ b/hooks/charmhelpers/contrib/openstack/templates/wsgi-openstack-api.conf @@ -15,9 +15,6 @@ Listen {{ public_port }} {% if port -%} WSGIDaemonProcess {{ service_name }} processes={{ processes }} threads={{ threads }} user={{ service_name }} group={{ service_name }} \ -{% if python_path -%} - python-path={{ python_path }} \ -{% endif -%} display-name=%{GROUP} WSGIProcessGroup {{ service_name }} WSGIScriptAlias / {{ script }} @@ -29,7 +26,7 @@ Listen {{ public_port }} ErrorLog /var/log/apache2/{{ service_name }}_error.log CustomLog /var/log/apache2/{{ service_name }}_access.log combined - + = 2.4> Require all granted @@ -44,9 +41,6 @@ Listen {{ public_port }} {% if admin_port -%} WSGIDaemonProcess {{ service_name }}-admin processes={{ admin_processes }} threads={{ threads }} user={{ service_name }} group={{ service_name }} \ -{% if python_path -%} - python-path={{ python_path }} \ -{% endif -%} display-name=%{GROUP} WSGIProcessGroup {{ service_name }}-admin WSGIScriptAlias / {{ admin_script }} @@ -58,7 +52,7 @@ Listen {{ public_port }} ErrorLog /var/log/apache2/{{ service_name }}_error.log CustomLog /var/log/apache2/{{ service_name }}_access.log combined - + = 2.4> Require all granted @@ -73,9 +67,6 @@ Listen {{ public_port }} {% if public_port -%} WSGIDaemonProcess {{ service_name }}-public processes={{ public_processes }} threads={{ threads }} user={{ service_name }} group={{ service_name }} \ -{% if python_path -%} - python-path={{ python_path }} \ -{% endif -%} display-name=%{GROUP} WSGIProcessGroup {{ service_name }}-public WSGIScriptAlias / {{ public_script }} @@ -87,7 +78,7 @@ Listen {{ public_port }} ErrorLog /var/log/apache2/{{ service_name }}_error.log CustomLog /var/log/apache2/{{ service_name }}_access.log combined - + = 2.4> Require all granted diff --git a/hooks/charmhelpers/contrib/openstack/templating.py b/hooks/charmhelpers/contrib/openstack/templating.py index 77490e4d..a623315d 100644 --- a/hooks/charmhelpers/contrib/openstack/templating.py +++ b/hooks/charmhelpers/contrib/openstack/templating.py @@ -93,7 +93,8 @@ class OSConfigTemplate(object): Associates a config file template with a list of context generators. Responsible for constructing a template context based on those generators. """ - def __init__(self, config_file, contexts): + + def __init__(self, config_file, contexts, config_template=None): self.config_file = config_file if hasattr(contexts, '__call__'): @@ -103,6 +104,8 @@ class OSConfigTemplate(object): self._complete_contexts = [] + self.config_template = config_template + def context(self): ctxt = {} for context in self.contexts: @@ -124,6 +127,11 @@ class OSConfigTemplate(object): self.context() return self._complete_contexts + @property + def is_string_template(self): + """:returns: Boolean if this instance is a template initialised with a string""" + return self.config_template is not None + class OSConfigRenderer(object): """ @@ -148,6 +156,10 @@ class OSConfigRenderer(object): contexts=[context.IdentityServiceContext()]) configs.register(config_file='/etc/haproxy/haproxy.conf', contexts=[context.HAProxyContext()]) + configs.register(config_file='/etc/keystone/policy.d/extra.cfg', + contexts=[context.ExtraPolicyContext() + context.KeystoneContext()], + config_template=hookenv.config('extra-policy')) # write out a single config configs.write('/etc/nova/nova.conf') # write out all registered configs @@ -218,14 +230,23 @@ class OSConfigRenderer(object): else: apt_install('python3-jinja2') - def register(self, config_file, contexts): + def register(self, config_file, contexts, config_template=None): """ Register a config file with a list of context generators to be called during rendering. + config_template can be used to load a template from a string instead of + using template loaders and template files. + :param config_file (str): a path where a config file will be rendered + :param contexts (list): a list of context dictionaries with kv pairs + :param config_template (str): an optional template string to use """ - self.templates[config_file] = OSConfigTemplate(config_file=config_file, - contexts=contexts) - log('Registered config file: %s' % config_file, level=INFO) + self.templates[config_file] = OSConfigTemplate( + config_file=config_file, + contexts=contexts, + config_template=config_template + ) + log('Registered config file: {}'.format(config_file), + level=INFO) def _get_tmpl_env(self): if not self._tmpl_env: @@ -235,32 +256,58 @@ class OSConfigRenderer(object): def _get_template(self, template): self._get_tmpl_env() template = self._tmpl_env.get_template(template) - log('Loaded template from %s' % template.filename, level=INFO) + log('Loaded template from {}'.format(template.filename), + level=INFO) + return template + + def _get_template_from_string(self, ostmpl): + ''' + Get a jinja2 template object from a string. + :param ostmpl: OSConfigTemplate to use as a data source. + ''' + self._get_tmpl_env() + template = self._tmpl_env.from_string(ostmpl.config_template) + log('Loaded a template from a string for {}'.format( + ostmpl.config_file), + level=INFO) return template def render(self, config_file): if config_file not in self.templates: - log('Config not registered: %s' % config_file, level=ERROR) + log('Config not registered: {}'.format(config_file), level=ERROR) raise OSConfigException - ctxt = self.templates[config_file].context() - _tmpl = os.path.basename(config_file) - try: - template = self._get_template(_tmpl) - except exceptions.TemplateNotFound: - # if no template is found with basename, try looking for it - # using a munged full path, eg: - # /etc/apache2/apache2.conf -> etc_apache2_apache2.conf - _tmpl = '_'.join(config_file.split('/')[1:]) + ostmpl = self.templates[config_file] + ctxt = ostmpl.context() + + if ostmpl.is_string_template: + template = self._get_template_from_string(ostmpl) + log('Rendering from a string template: ' + '{}'.format(config_file), + level=INFO) + else: + _tmpl = os.path.basename(config_file) try: template = self._get_template(_tmpl) - except exceptions.TemplateNotFound as e: - log('Could not load template from %s by %s or %s.' % - (self.templates_dir, os.path.basename(config_file), _tmpl), - level=ERROR) - raise e + except exceptions.TemplateNotFound: + # if no template is found with basename, try looking + # for it using a munged full path, eg: + # /etc/apache2/apache2.conf -> etc_apache2_apache2.conf + _tmpl = '_'.join(config_file.split('/')[1:]) + try: + template = self._get_template(_tmpl) + except exceptions.TemplateNotFound as e: + log('Could not load template from {} by {} or {}.' + ''.format( + self.templates_dir, + os.path.basename(config_file), + _tmpl + ), + level=ERROR) + raise e - log('Rendering from template: %s' % _tmpl, level=INFO) + log('Rendering from template: {}'.format(config_file), + level=INFO) return template.render(ctxt) def write(self, config_file): diff --git a/hooks/charmhelpers/contrib/openstack/utils.py b/hooks/charmhelpers/contrib/openstack/utils.py index 0e867310..b753275d 100644 --- a/hooks/charmhelpers/contrib/openstack/utils.py +++ b/hooks/charmhelpers/contrib/openstack/utils.py @@ -626,11 +626,6 @@ def openstack_upgrade_available(package): else: avail_vers = get_os_version_install_source(src) apt.init() - if "swift" in package: - major_cur_vers = cur_vers.split('.', 1)[0] - major_avail_vers = avail_vers.split('.', 1)[0] - major_diff = apt.version_compare(major_avail_vers, major_cur_vers) - return avail_vers > cur_vers and (major_diff == 1 or major_diff == 0) return apt.version_compare(avail_vers, cur_vers) == 1 diff --git a/hooks/charmhelpers/core/hookenv.py b/hooks/charmhelpers/core/hookenv.py index 5a88f798..7ed1cc4e 100644 --- a/hooks/charmhelpers/core/hookenv.py +++ b/hooks/charmhelpers/core/hookenv.py @@ -39,6 +39,7 @@ if not six.PY3: else: from collections import UserDict + CRITICAL = "CRITICAL" ERROR = "ERROR" WARNING = "WARNING" @@ -344,6 +345,7 @@ class Config(dict): """ with open(self.path, 'w') as f: + os.fchmod(f.fileno(), 0o600) json.dump(self, f) def _implicit_save(self): @@ -818,6 +820,10 @@ class Hooks(object): return wrapper +class NoNetworkBinding(Exception): + pass + + def charm_dir(): """Return the root directory of the current charm""" d = os.environ.get('JUJU_CHARM_DIR') @@ -1104,7 +1110,17 @@ def network_get_primary_address(binding): :raise: NotImplementedError if run on Juju < 2.0 ''' cmd = ['network-get', '--primary-address', binding] - return subprocess.check_output(cmd).decode('UTF-8').strip() + try: + response = subprocess.check_output( + cmd, + stderr=subprocess.STDOUT).decode('UTF-8').strip() + except CalledProcessError as e: + if 'no network config found for binding' in e.output.decode('UTF-8'): + raise NoNetworkBinding("No network binding for {}" + .format(binding)) + else: + raise + return response @translate_exc(from_exc=OSError, to_exc=NotImplementedError) diff --git a/hooks/charmhelpers/core/unitdata.py b/hooks/charmhelpers/core/unitdata.py index 7af875c2..6d7b4942 100644 --- a/hooks/charmhelpers/core/unitdata.py +++ b/hooks/charmhelpers/core/unitdata.py @@ -175,6 +175,8 @@ class Storage(object): else: self.db_path = os.path.join( os.environ.get('CHARM_DIR', ''), '.unit-state.db') + with open(self.db_path, 'a') as f: + os.fchmod(f.fileno(), 0o600) self.conn = sqlite3.connect('%s' % self.db_path) self.cursor = self.conn.cursor() self.revision = None diff --git a/hooks/horizon_contexts.py b/hooks/horizon_contexts.py index 7f1518ec..bb99848c 100644 --- a/hooks/horizon_contexts.py +++ b/hooks/horizon_contexts.py @@ -20,7 +20,6 @@ from charmhelpers.core.hookenv import ( related_units, relation_get, local_unit, - unit_get, log, WARNING, ERROR, @@ -28,7 +27,6 @@ from charmhelpers.core.hookenv import ( from charmhelpers.core.strutils import bool_from_string from charmhelpers.contrib.openstack.context import ( OSContextGenerator, - HAProxyContext, context_complete ) from charmhelpers.contrib.hahelpers.apache import ( @@ -39,6 +37,7 @@ from charmhelpers.contrib.hahelpers.apache import ( from charmhelpers.contrib.network.ip import ( get_ipv6_addr, format_ipv6_addr, + get_relation_ip, ) from charmhelpers.core.host import pwgen @@ -53,7 +52,7 @@ VALID_ENDPOINT_TYPES = { } -class HorizonHAProxyContext(HAProxyContext): +class HorizonHAProxyContext(OSContextGenerator): def __call__(self): ''' Horizon specific HAProxy context; haproxy is used all the time @@ -65,7 +64,7 @@ class HorizonHAProxyContext(HAProxyContext): if config('prefer-ipv6'): cluster_hosts[l_unit] = get_ipv6_addr(exc_list=[config('vip')])[0] else: - cluster_hosts[l_unit] = unit_get('private-address') + cluster_hosts[l_unit] = get_relation_ip('cluster') for rid in relation_ids('cluster'): for unit in related_units(rid): diff --git a/hooks/horizon_utils.py b/hooks/horizon_utils.py index 41c474a4..e5f62dd3 100644 --- a/hooks/horizon_utils.py +++ b/hooks/horizon_utils.py @@ -131,7 +131,8 @@ CONFIG_FILES = OrderedDict([ (HAPROXY_CONF, { 'hook_contexts': [ horizon_contexts.HorizonHAProxyContext(), - context.HAProxyContext(singlenode_mode=True), + context.HAProxyContext(singlenode_mode=True, + address_types=[]), ], 'services': ['haproxy'], }), diff --git a/tests/charmhelpers/core/hookenv.py b/tests/charmhelpers/core/hookenv.py index 5a88f798..7ed1cc4e 100644 --- a/tests/charmhelpers/core/hookenv.py +++ b/tests/charmhelpers/core/hookenv.py @@ -39,6 +39,7 @@ if not six.PY3: else: from collections import UserDict + CRITICAL = "CRITICAL" ERROR = "ERROR" WARNING = "WARNING" @@ -344,6 +345,7 @@ class Config(dict): """ with open(self.path, 'w') as f: + os.fchmod(f.fileno(), 0o600) json.dump(self, f) def _implicit_save(self): @@ -818,6 +820,10 @@ class Hooks(object): return wrapper +class NoNetworkBinding(Exception): + pass + + def charm_dir(): """Return the root directory of the current charm""" d = os.environ.get('JUJU_CHARM_DIR') @@ -1104,7 +1110,17 @@ def network_get_primary_address(binding): :raise: NotImplementedError if run on Juju < 2.0 ''' cmd = ['network-get', '--primary-address', binding] - return subprocess.check_output(cmd).decode('UTF-8').strip() + try: + response = subprocess.check_output( + cmd, + stderr=subprocess.STDOUT).decode('UTF-8').strip() + except CalledProcessError as e: + if 'no network config found for binding' in e.output.decode('UTF-8'): + raise NoNetworkBinding("No network binding for {}" + .format(binding)) + else: + raise + return response @translate_exc(from_exc=OSError, to_exc=NotImplementedError) diff --git a/tests/charmhelpers/core/unitdata.py b/tests/charmhelpers/core/unitdata.py index 7af875c2..6d7b4942 100644 --- a/tests/charmhelpers/core/unitdata.py +++ b/tests/charmhelpers/core/unitdata.py @@ -175,6 +175,8 @@ class Storage(object): else: self.db_path = os.path.join( os.environ.get('CHARM_DIR', ''), '.unit-state.db') + with open(self.db_path, 'a') as f: + os.fchmod(f.fileno(), 0o600) self.conn = sqlite3.connect('%s' % self.db_path) self.cursor = self.conn.cursor() self.revision = None diff --git a/unit_tests/test_horizon_contexts.py b/unit_tests/test_horizon_contexts.py index 030f65f8..90c2d071 100644 --- a/unit_tests/test_horizon_contexts.py +++ b/unit_tests/test_horizon_contexts.py @@ -30,7 +30,7 @@ TO_PATCH = [ 'b64decode', 'context_complete', 'local_unit', - 'unit_get', + 'get_relation_ip', 'pwgen', ] @@ -597,7 +597,7 @@ class TestHorizonContexts(CharmTestCase): def test_HorizonHAProxyContext_no_cluster(self): self.relation_ids.return_value = [] self.local_unit.return_value = 'openstack-dashboard/0' - self.unit_get.return_value = "10.5.0.1" + self.get_relation_ip.return_value = "10.5.0.1" with patch_open() as (_open, _file): self.assertEqual(horizon_contexts.HorizonHAProxyContext()(), {'units': {'openstack-dashboard-0': '10.5.0.1'}, @@ -606,6 +606,7 @@ class TestHorizonContexts(CharmTestCase): 'prefer_ipv6': False}) _open.assert_called_with('/etc/default/haproxy', 'w') self.assertTrue(_file.write.called) + self.get_relation_ip.assert_called_with('cluster') def test_HorizonHAProxyContext_clustered(self): self.relation_ids.return_value = ['cluster:0'] @@ -614,7 +615,7 @@ class TestHorizonContexts(CharmTestCase): ] self.relation_get.side_effect = ['10.5.0.2', '10.5.0.3'] self.local_unit.return_value = 'openstack-dashboard/0' - self.unit_get.return_value = "10.5.0.1" + self.get_relation_ip.return_value = "10.5.0.1" with patch_open() as (_open, _file): self.assertEqual(horizon_contexts.HorizonHAProxyContext()(), {'units': {'openstack-dashboard-0': '10.5.0.1', @@ -625,6 +626,7 @@ class TestHorizonContexts(CharmTestCase): 'prefer_ipv6': False}) _open.assert_called_with('/etc/default/haproxy', 'w') self.assertTrue(_file.write.called) + self.get_relation_ip.assert_called_with('cluster') def test_RouterSettingContext(self): self.test_config.set('profile', 'cisco')