From b9d15257ecd09bfe67e4c4242142b5160f804b93 Mon Sep 17 00:00:00 2001 From: Billy Olsen Date: Thu, 4 Jun 2015 16:14:33 -0700 Subject: [PATCH] [wolsen,r=] Add support for overriding public endpoint addresses. Adds in the config option for overriding public endpoint addresses and introduces a unit tests to ensure that the override for the public address is functioning correctly. Closes-Bug: #1398182 --- config.yaml | 14 +- .../charmhelpers/contrib/hahelpers/cluster.py | 25 +++ hooks/charmhelpers/contrib/openstack/ip.py | 93 ++++++----- .../charmhelpers/contrib/openstack/neutron.py | 15 +- hooks/charmhelpers/contrib/openstack/utils.py | 81 +++++++-- hooks/charmhelpers/contrib/python/packages.py | 33 +++- hooks/charmhelpers/core/hookenv.py | 157 ++++++++++++++++-- hooks/charmhelpers/core/host.py | 2 +- hooks/charmhelpers/core/services/base.py | 43 +++-- hooks/charmhelpers/fetch/__init__.py | 2 +- hooks/charmhelpers/fetch/giturl.py | 12 +- hooks/heat_relations.py | 36 ++-- unit_tests/test_heat_relations.py | 38 ++++- 13 files changed, 435 insertions(+), 116 deletions(-) diff --git a/config.yaml b/config.yaml index 1e39182..4302d68 100644 --- a/config.yaml +++ b/config.yaml @@ -62,4 +62,16 @@ options: default: description: | SSL CA to use with the certificate and key provided - this is only - required if you are providing a privately signed ssl_cert and ssl_key. \ No newline at end of file + required if you are providing a privately signed ssl_cert and ssl_key. + os-public-hostname: + type: string + default: + description: | + The hostname or address of the public endpoints created for heat + in the keystone identity provider. + . + This value will be used for public endpoints. For example, an + os-public-hostname set to 'heat.example.com' with ssl enabled will + create the following public endpoints for ceilometer. + . + https://ceilometer.example.com:8777/ \ No newline at end of file diff --git a/hooks/charmhelpers/contrib/hahelpers/cluster.py b/hooks/charmhelpers/contrib/hahelpers/cluster.py index 9333efc..c555d7a 100644 --- a/hooks/charmhelpers/contrib/hahelpers/cluster.py +++ b/hooks/charmhelpers/contrib/hahelpers/cluster.py @@ -52,6 +52,8 @@ from charmhelpers.core.strutils import ( bool_from_string, ) +DC_RESOURCE_NAME = 'DC' + class HAIncompleteConfig(Exception): pass @@ -95,6 +97,27 @@ def is_clustered(): return False +def is_crm_dc(): + """ + Determine leadership by querying the pacemaker Designated Controller + """ + cmd = ['crm', 'status'] + try: + status = subprocess.check_output(cmd, stderr=subprocess.STDOUT) + if not isinstance(status, six.text_type): + status = six.text_type(status, "utf-8") + except subprocess.CalledProcessError: + return False + current_dc = '' + for line in status.split('\n'): + if line.startswith('Current DC'): + # Current DC: juju-lytrusty-machine-2 (168108163) - partition with quorum + current_dc = line.split(':')[1].split()[0] + if current_dc == get_unit_hostname(): + return True + return False + + @retry_on_exception(5, base_delay=2, exc_type=CRMResourceNotFound) def is_crm_leader(resource, retry=False): """ @@ -104,6 +127,8 @@ def is_crm_leader(resource, retry=False): We allow this operation to be retried to avoid the possibility of getting a false negative. See LP #1396246 for more info. """ + if resource == DC_RESOURCE_NAME: + return is_crm_dc() cmd = ['crm', 'resource', 'show', resource] try: status = subprocess.check_output(cmd, stderr=subprocess.STDOUT) diff --git a/hooks/charmhelpers/contrib/openstack/ip.py b/hooks/charmhelpers/contrib/openstack/ip.py index 29bbddc..3dca6dc 100644 --- a/hooks/charmhelpers/contrib/openstack/ip.py +++ b/hooks/charmhelpers/contrib/openstack/ip.py @@ -17,6 +17,7 @@ from charmhelpers.core.hookenv import ( config, unit_get, + service_name, ) from charmhelpers.contrib.network.ip import ( get_address_in_network, @@ -26,8 +27,6 @@ from charmhelpers.contrib.network.ip import ( ) from charmhelpers.contrib.hahelpers.cluster import is_clustered -from functools import partial - PUBLIC = 'public' INTERNAL = 'int' ADMIN = 'admin' @@ -35,15 +34,18 @@ ADMIN = 'admin' ADDRESS_MAP = { PUBLIC: { 'config': 'os-public-network', - 'fallback': 'public-address' + 'fallback': 'public-address', + 'override': 'os-public-hostname', }, INTERNAL: { 'config': 'os-internal-network', - 'fallback': 'private-address' + 'fallback': 'private-address', + 'override': 'os-internal-hostname', }, ADMIN: { 'config': 'os-admin-network', - 'fallback': 'private-address' + 'fallback': 'private-address', + 'override': 'os-admin-hostname', } } @@ -57,15 +59,50 @@ def canonical_url(configs, endpoint_type=PUBLIC): :param endpoint_type: str endpoint type to resolve. :param returns: str base URL for services on the current service unit. """ - scheme = 'http' - if 'https' in configs.complete_contexts(): - scheme = 'https' + scheme = _get_scheme(configs) + address = resolve_address(endpoint_type) if is_ipv6(address): address = "[{}]".format(address) + return '%s://%s' % (scheme, address) +def _get_scheme(configs): + """Returns the scheme to use for the url (either http or https) + depending upon whether https is in the configs value. + + :param configs: OSTemplateRenderer config templating object to inspect + for a complete https context. + :returns: either 'http' or 'https' depending on whether https is + configured within the configs context. + """ + scheme = 'http' + if configs and 'https' in configs.complete_contexts(): + scheme = 'https' + return scheme + + +def _get_address_override(endpoint_type=PUBLIC): + """Returns any address overrides that the user has defined based on the + endpoint type. + + Note: this function allows for the service name to be inserted into the + address if the user specifies {service_name}.somehost.org. + + :param endpoint_type: the type of endpoint to retrieve the override + value for. + :returns: any endpoint address or hostname that the user has overridden + or None if an override is not present. + """ + override_key = ADDRESS_MAP[endpoint_type]['override'] + addr_override = config(override_key) + if not addr_override: + return None + else: + return addr_override.format(service_name=service_name()) + + def resolve_address(endpoint_type=PUBLIC): """Return unit address depending on net config. @@ -77,7 +114,10 @@ def resolve_address(endpoint_type=PUBLIC): :param endpoint_type: Network endpoing type """ - resolved_address = None + resolved_address = _get_address_override(endpoint_type) + if resolved_address: + return resolved_address + vips = config('vip') if vips: vips = vips.split() @@ -109,38 +149,3 @@ def resolve_address(endpoint_type=PUBLIC): "clustered=%s)" % (net_type, clustered)) return resolved_address - - -def endpoint_url(configs, url_template, port, endpoint_type=PUBLIC, - override=None): - """Returns the correct endpoint URL to advertise to Keystone. - - This method provides the correct endpoint URL which should be advertised to - the keystone charm for endpoint creation. This method allows for the url to - be overridden to force a keystone endpoint to have specific URL for any of - the defined scopes (admin, internal, public). - - :param configs: OSTemplateRenderer config templating object to inspect - for a complete https context. - :param url_template: str format string for creating the url template. Only - two values will be passed - the scheme+hostname - returned by the canonical_url and the port. - :param endpoint_type: str endpoint type to resolve. - :param override: str the name of the config option which overrides the - endpoint URL defined by the charm itself. None will - disable any overrides (default). - """ - if override: - # Return any user-defined overrides for the keystone endpoint URL. - user_value = config(override) - if user_value: - return user_value.strip() - - return url_template % (canonical_url(configs, endpoint_type), port) - - -public_endpoint = partial(endpoint_url, endpoint_type=PUBLIC) - -internal_endpoint = partial(endpoint_url, endpoint_type=INTERNAL) - -admin_endpoint = partial(endpoint_url, endpoint_type=ADMIN) diff --git a/hooks/charmhelpers/contrib/openstack/neutron.py b/hooks/charmhelpers/contrib/openstack/neutron.py index 02c92e9..b3aa3d4 100644 --- a/hooks/charmhelpers/contrib/openstack/neutron.py +++ b/hooks/charmhelpers/contrib/openstack/neutron.py @@ -256,11 +256,14 @@ def network_manager(): def parse_mappings(mappings): parsed = {} if mappings: - mappings = mappings.split(' ') + mappings = mappings.split() for m in mappings: p = m.partition(':') - if p[1] == ':': - parsed[p[0].strip()] = p[2].strip() + key = p[0].strip() + if p[1]: + parsed[key] = p[2].strip() + else: + parsed[key] = '' return parsed @@ -283,13 +286,13 @@ def parse_data_port_mappings(mappings, default_bridge='br-data'): Returns dict of the form {bridge:port}. """ _mappings = parse_mappings(mappings) - if not _mappings: + if not _mappings or list(_mappings.values()) == ['']: if not mappings: return {} # For backwards-compatibility we need to support port-only provided in # config. - _mappings = {default_bridge: mappings.split(' ')[0]} + _mappings = {default_bridge: mappings.split()[0]} bridges = _mappings.keys() ports = _mappings.values() @@ -309,6 +312,8 @@ def parse_vlan_range_mappings(mappings): Mappings must be a space-delimited list of provider:start:end mappings. + The start:end range is optional and may be omitted. + Returns dict of the form {provider: (start, end)}. """ _mappings = parse_mappings(mappings) diff --git a/hooks/charmhelpers/contrib/openstack/utils.py b/hooks/charmhelpers/contrib/openstack/utils.py index f90a028..d795a35 100644 --- a/hooks/charmhelpers/contrib/openstack/utils.py +++ b/hooks/charmhelpers/contrib/openstack/utils.py @@ -53,9 +53,13 @@ from charmhelpers.contrib.network.ip import ( get_ipv6_addr ) +from charmhelpers.contrib.python.packages import ( + pip_create_virtualenv, + pip_install, +) + from charmhelpers.core.host import lsb_release, mounts, umount from charmhelpers.fetch import apt_install, apt_cache, install_remote -from charmhelpers.contrib.python.packages import pip_install from charmhelpers.contrib.storage.linux.utils import is_block_device, zap_disk from charmhelpers.contrib.storage.linux.loopback import ensure_loopback_device @@ -497,7 +501,17 @@ def git_install_requested(): requirements_dir = None -def git_clone_and_install(projects_yaml, core_project): +def _git_yaml_load(projects_yaml): + """ + Load the specified yaml into a dictionary. + """ + if not projects_yaml: + return None + + return yaml.load(projects_yaml) + + +def git_clone_and_install(projects_yaml, core_project, depth=1): """ Clone/install all specified OpenStack repositories. @@ -510,23 +524,22 @@ def git_clone_and_install(projects_yaml, core_project): repository: 'git://git.openstack.org/openstack/requirements.git', branch: 'stable/icehouse'} directory: /mnt/openstack-git - http_proxy: http://squid.internal:3128 - https_proxy: https://squid.internal:3128 + http_proxy: squid-proxy-url + https_proxy: squid-proxy-url The directory, http_proxy, and https_proxy keys are optional. """ global requirements_dir parent_dir = '/mnt/openstack-git' + http_proxy = None - if not projects_yaml: - return - - projects = yaml.load(projects_yaml) + projects = _git_yaml_load(projects_yaml) _git_validate_projects_yaml(projects, core_project) old_environ = dict(os.environ) if 'http_proxy' in projects.keys(): + http_proxy = projects['http_proxy'] os.environ['http_proxy'] = projects['http_proxy'] if 'https_proxy' in projects.keys(): os.environ['https_proxy'] = projects['https_proxy'] @@ -534,15 +547,19 @@ def git_clone_and_install(projects_yaml, core_project): if 'directory' in projects.keys(): parent_dir = projects['directory'] + pip_create_virtualenv(os.path.join(parent_dir, 'venv')) + for p in projects['repositories']: repo = p['repository'] branch = p['branch'] if p['name'] == 'requirements': - repo_dir = _git_clone_and_install_single(repo, branch, parent_dir, + repo_dir = _git_clone_and_install_single(repo, branch, depth, + parent_dir, http_proxy, update_requirements=False) requirements_dir = repo_dir else: - repo_dir = _git_clone_and_install_single(repo, branch, parent_dir, + repo_dir = _git_clone_and_install_single(repo, branch, depth, + parent_dir, http_proxy, update_requirements=True) os.environ = old_environ @@ -574,7 +591,8 @@ def _git_ensure_key_exists(key, keys): error_out('openstack-origin-git key \'{}\' is missing'.format(key)) -def _git_clone_and_install_single(repo, branch, parent_dir, update_requirements): +def _git_clone_and_install_single(repo, branch, depth, parent_dir, http_proxy, + update_requirements): """ Clone and install a single git repository. """ @@ -587,7 +605,8 @@ def _git_clone_and_install_single(repo, branch, parent_dir, update_requirements) if not os.path.exists(dest_dir): juju_log('Cloning git repo: {}, branch: {}'.format(repo, branch)) - repo_dir = install_remote(repo, dest=parent_dir, branch=branch) + repo_dir = install_remote(repo, dest=parent_dir, branch=branch, + depth=depth) else: repo_dir = dest_dir @@ -598,7 +617,12 @@ def _git_clone_and_install_single(repo, branch, parent_dir, update_requirements) _git_update_requirements(repo_dir, requirements_dir) juju_log('Installing git repo from dir: {}'.format(repo_dir)) - pip_install(repo_dir) + if http_proxy: + pip_install(repo_dir, proxy=http_proxy, + venv=os.path.join(parent_dir, 'venv')) + else: + pip_install(repo_dir, + venv=os.path.join(parent_dir, 'venv')) return repo_dir @@ -621,16 +645,27 @@ def _git_update_requirements(package_dir, reqs_dir): os.chdir(orig_dir) +def git_pip_venv_dir(projects_yaml): + """ + Return the pip virtualenv path. + """ + parent_dir = '/mnt/openstack-git' + + projects = _git_yaml_load(projects_yaml) + + if 'directory' in projects.keys(): + parent_dir = projects['directory'] + + return os.path.join(parent_dir, 'venv') + + def git_src_dir(projects_yaml, project): """ Return the directory where the specified project's source is located. """ parent_dir = '/mnt/openstack-git' - if not projects_yaml: - return - - projects = yaml.load(projects_yaml) + projects = _git_yaml_load(projects_yaml) if 'directory' in projects.keys(): parent_dir = projects['directory'] @@ -640,3 +675,15 @@ def git_src_dir(projects_yaml, project): return os.path.join(parent_dir, os.path.basename(p['repository'])) return None + + +def git_yaml_value(projects_yaml, key): + """ + Return the value in projects_yaml for the specified key. + """ + projects = _git_yaml_load(projects_yaml) + + if key in projects.keys(): + return projects[key] + + return None diff --git a/hooks/charmhelpers/contrib/python/packages.py b/hooks/charmhelpers/contrib/python/packages.py index 8659516..07b0c1d 100644 --- a/hooks/charmhelpers/contrib/python/packages.py +++ b/hooks/charmhelpers/contrib/python/packages.py @@ -17,8 +17,11 @@ # You should have received a copy of the GNU Lesser General Public License # along with charm-helpers. If not, see . +import os +import subprocess + from charmhelpers.fetch import apt_install, apt_update -from charmhelpers.core.hookenv import log +from charmhelpers.core.hookenv import charm_dir, log try: from pip import main as pip_execute @@ -51,11 +54,15 @@ def pip_install_requirements(requirements, **options): pip_execute(command) -def pip_install(package, fatal=False, upgrade=False, **options): +def pip_install(package, fatal=False, upgrade=False, venv=None, **options): """Install a python package""" - command = ["install"] + if venv: + venv_python = os.path.join(venv, 'bin/pip') + command = [venv_python, "install"] + else: + command = ["install"] - available_options = ('proxy', 'src', 'log', "index-url", ) + available_options = ('proxy', 'src', 'log', 'index-url', ) for option in parse_options(options, available_options): command.append(option) @@ -69,7 +76,10 @@ def pip_install(package, fatal=False, upgrade=False, **options): log("Installing {} package with options: {}".format(package, command)) - pip_execute(command) + if venv: + subprocess.check_call(command) + else: + pip_execute(command) def pip_uninstall(package, **options): @@ -94,3 +104,16 @@ def pip_list(): """Returns the list of current python installed packages """ return pip_execute(["list"]) + + +def pip_create_virtualenv(path=None): + """Create an isolated Python environment.""" + apt_install('python-virtualenv') + + if path: + venv_path = path + else: + venv_path = os.path.join(charm_dir(), 'venv') + + if not os.path.exists(venv_path): + subprocess.check_call(['virtualenv', venv_path]) diff --git a/hooks/charmhelpers/core/hookenv.py b/hooks/charmhelpers/core/hookenv.py index 86f805f..117429f 100644 --- a/hooks/charmhelpers/core/hookenv.py +++ b/hooks/charmhelpers/core/hookenv.py @@ -21,12 +21,14 @@ # Charm Helpers Developers from __future__ import print_function +from functools import wraps import os import json import yaml import subprocess import sys import errno +import tempfile from subprocess import CalledProcessError import six @@ -58,15 +60,17 @@ def cached(func): will cache the result of unit_get + 'test' for future calls. """ + @wraps(func) def wrapper(*args, **kwargs): global cache key = str((func, args, kwargs)) try: return cache[key] except KeyError: - res = func(*args, **kwargs) - cache[key] = res - return res + pass # Drop out of the exception handler scope. + res = func(*args, **kwargs) + cache[key] = res + return res return wrapper @@ -178,7 +182,7 @@ def local_unit(): def remote_unit(): """The remote unit for the current relation hook""" - return os.environ['JUJU_REMOTE_UNIT'] + return os.environ.get('JUJU_REMOTE_UNIT', None) def service_name(): @@ -250,6 +254,12 @@ class Config(dict): except KeyError: return (self._prev_dict or {})[key] + def get(self, key, default=None): + try: + return self[key] + except KeyError: + return default + def keys(self): prev_keys = [] if self._prev_dict is not None: @@ -353,18 +363,49 @@ def relation_set(relation_id=None, relation_settings=None, **kwargs): """Set relation information for the current unit""" relation_settings = relation_settings if relation_settings else {} relation_cmd_line = ['relation-set'] + accepts_file = "--file" in subprocess.check_output( + relation_cmd_line + ["--help"], universal_newlines=True) if relation_id is not None: relation_cmd_line.extend(('-r', relation_id)) - for k, v in (list(relation_settings.items()) + list(kwargs.items())): - if v is None: - relation_cmd_line.append('{}='.format(k)) - else: - relation_cmd_line.append('{}={}'.format(k, v)) - subprocess.check_call(relation_cmd_line) + settings = relation_settings.copy() + settings.update(kwargs) + for key, value in settings.items(): + # Force value to be a string: it always should, but some call + # sites pass in things like dicts or numbers. + if value is not None: + settings[key] = "{}".format(value) + if accepts_file: + # --file was introduced in Juju 1.23.2. Use it by default if + # available, since otherwise we'll break if the relation data is + # too big. Ideally we should tell relation-set to read the data from + # stdin, but that feature is broken in 1.23.2: Bug #1454678. + with tempfile.NamedTemporaryFile(delete=False) as settings_file: + settings_file.write(yaml.safe_dump(settings).encode("utf-8")) + subprocess.check_call( + relation_cmd_line + ["--file", settings_file.name]) + os.remove(settings_file.name) + else: + for key, value in settings.items(): + if value is None: + relation_cmd_line.append('{}='.format(key)) + else: + relation_cmd_line.append('{}={}'.format(key, value)) + subprocess.check_call(relation_cmd_line) # Flush cache of any relation-gets for local unit flush(local_unit()) +def relation_clear(r_id=None): + ''' Clears any relation data already set on relation r_id ''' + settings = relation_get(rid=r_id, + unit=local_unit()) + for setting in settings: + if setting not in ['public-address', 'private-address']: + settings[setting] = None + relation_set(relation_id=r_id, + **settings) + + @cached def relation_ids(reltype=None): """A list of relation_ids""" @@ -509,6 +550,11 @@ def unit_get(attribute): return None +def unit_public_ip(): + """Get this unit's public IP address""" + return unit_get('public-address') + + def unit_private_ip(): """Get this unit's private IP address""" return unit_get('private-address') @@ -605,3 +651,94 @@ def action_fail(message): The results set by action_set are preserved.""" subprocess.check_call(['action-fail', message]) + + +def status_set(workload_state, message): + """Set the workload state with a message + + Use status-set to set the workload state with a message which is visible + to the user via juju status. If the status-set command is not found then + assume this is juju < 1.23 and juju-log the message unstead. + + workload_state -- valid juju workload state. + message -- status update message + """ + valid_states = ['maintenance', 'blocked', 'waiting', 'active'] + if workload_state not in valid_states: + raise ValueError( + '{!r} is not a valid workload state'.format(workload_state) + ) + cmd = ['status-set', workload_state, message] + try: + ret = subprocess.call(cmd) + if ret == 0: + return + except OSError as e: + if e.errno != errno.ENOENT: + raise + log_message = 'status-set failed: {} {}'.format(workload_state, + message) + log(log_message, level='INFO') + + +def status_get(): + """Retrieve the previously set juju workload state + + If the status-set command is not found then assume this is juju < 1.23 and + return 'unknown' + """ + cmd = ['status-get'] + try: + raw_status = subprocess.check_output(cmd, universal_newlines=True) + status = raw_status.rstrip() + return status + except OSError as e: + if e.errno == errno.ENOENT: + return 'unknown' + else: + raise + + +def translate_exc(from_exc, to_exc): + def inner_translate_exc1(f): + def inner_translate_exc2(*args, **kwargs): + try: + return f(*args, **kwargs) + except from_exc: + raise to_exc + + return inner_translate_exc2 + + return inner_translate_exc1 + + +@translate_exc(from_exc=OSError, to_exc=NotImplementedError) +def is_leader(): + """Does the current unit hold the juju leadership + + Uses juju to determine whether the current unit is the leader of its peers + """ + cmd = ['is-leader', '--format=json'] + return json.loads(subprocess.check_output(cmd).decode('UTF-8')) + + +@translate_exc(from_exc=OSError, to_exc=NotImplementedError) +def leader_get(attribute=None): + """Juju leader get value(s)""" + cmd = ['leader-get', '--format=json'] + [attribute or '-'] + return json.loads(subprocess.check_output(cmd).decode('UTF-8')) + + +@translate_exc(from_exc=OSError, to_exc=NotImplementedError) +def leader_set(settings=None, **kwargs): + """Juju leader set value(s)""" + log("Juju leader-set '%s'" % (settings), level=DEBUG) + cmd = ['leader-set'] + settings = settings or {} + settings.update(kwargs) + for k, v in settings.iteritems(): + if v is None: + cmd.append('{}='.format(k)) + else: + cmd.append('{}={}'.format(k, v)) + subprocess.check_call(cmd) diff --git a/hooks/charmhelpers/core/host.py b/hooks/charmhelpers/core/host.py index 830822a..0d2ab4b 100644 --- a/hooks/charmhelpers/core/host.py +++ b/hooks/charmhelpers/core/host.py @@ -90,7 +90,7 @@ def service_available(service_name): ['service', service_name, 'status'], stderr=subprocess.STDOUT).decode('UTF-8') except subprocess.CalledProcessError as e: - return 'unrecognized service' not in e.output + return b'unrecognized service' not in e.output else: return True diff --git a/hooks/charmhelpers/core/services/base.py b/hooks/charmhelpers/core/services/base.py index c5534e4..98d344e 100644 --- a/hooks/charmhelpers/core/services/base.py +++ b/hooks/charmhelpers/core/services/base.py @@ -15,9 +15,9 @@ # along with charm-helpers. If not, see . import os -import re import json -from collections import Iterable +from inspect import getargspec +from collections import Iterable, OrderedDict from charmhelpers.core import host from charmhelpers.core import hookenv @@ -119,7 +119,7 @@ class ServiceManager(object): """ self._ready_file = os.path.join(hookenv.charm_dir(), 'READY-SERVICES.json') self._ready = None - self.services = {} + self.services = OrderedDict() for service in services or []: service_name = service['service'] self.services[service_name] = service @@ -132,8 +132,8 @@ class ServiceManager(object): if hook_name == 'stop': self.stop_services() else: - self.provide_data() self.reconfigure_services() + self.provide_data() cfg = hookenv.config() if cfg.implicit_save: cfg.save() @@ -145,15 +145,36 @@ class ServiceManager(object): A provider must have a `name` attribute, which indicates which relation to set data on, and a `provide_data()` method, which returns a dict of data to set. + + The `provide_data()` method can optionally accept two parameters: + + * ``remote_service`` The name of the remote service that the data will + be provided to. The `provide_data()` method will be called once + for each connected service (not unit). This allows the method to + tailor its data to the given service. + * ``service_ready`` Whether or not the service definition had all of + its requirements met, and thus the ``data_ready`` callbacks run. + + Note that the ``provided_data`` methods are now called **after** the + ``data_ready`` callbacks are run. This gives the ``data_ready`` callbacks + a chance to generate any data necessary for the providing to the remote + services. """ - hook_name = hookenv.hook_name() - for service in self.services.values(): + for service_name, service in self.services.items(): + service_ready = self.is_ready(service_name) for provider in service.get('provided_data', []): - if re.match(r'{}-relation-(joined|changed)'.format(provider.name), hook_name): - data = provider.provide_data() - _ready = provider._is_ready(data) if hasattr(provider, '_is_ready') else data - if _ready: - hookenv.relation_set(None, data) + for relid in hookenv.relation_ids(provider.name): + units = hookenv.related_units(relid) + if not units: + continue + remote_service = units[0].split('/')[0] + argspec = getargspec(provider.provide_data) + if len(argspec.args) > 1: + data = provider.provide_data(remote_service, service_ready) + else: + data = provider.provide_data() + if data: + hookenv.relation_set(relid, data) def reconfigure_services(self, *service_names): """ diff --git a/hooks/charmhelpers/fetch/__init__.py b/hooks/charmhelpers/fetch/__init__.py index 792e629..9a1a251 100644 --- a/hooks/charmhelpers/fetch/__init__.py +++ b/hooks/charmhelpers/fetch/__init__.py @@ -158,7 +158,7 @@ def filter_installed_packages(packages): def apt_cache(in_memory=True): """Build and return an apt cache""" - import apt_pkg + from apt import apt_pkg apt_pkg.init() if in_memory: apt_pkg.config.set("Dir::Cache::pkgcache", "") diff --git a/hooks/charmhelpers/fetch/giturl.py b/hooks/charmhelpers/fetch/giturl.py index 93aae87..ddc25b7 100644 --- a/hooks/charmhelpers/fetch/giturl.py +++ b/hooks/charmhelpers/fetch/giturl.py @@ -45,14 +45,16 @@ class GitUrlFetchHandler(BaseFetchHandler): else: return True - def clone(self, source, dest, branch): + def clone(self, source, dest, branch, depth=None): if not self.can_handle(source): raise UnhandledSource("Cannot handle {}".format(source)) - repo = Repo.clone_from(source, dest) - repo.git.checkout(branch) + if depth: + Repo.clone_from(source, dest, branch=branch, depth=depth) + else: + Repo.clone_from(source, dest, branch=branch) - def install(self, source, branch="master", dest=None): + def install(self, source, branch="master", dest=None, depth=None): url_parts = self.parse_url(source) branch_name = url_parts.path.strip("/").split("/")[-1] if dest: @@ -63,7 +65,7 @@ class GitUrlFetchHandler(BaseFetchHandler): if not os.path.exists(dest_dir): mkdir(dest_dir, perms=0o755) try: - self.clone(source, dest_dir, branch) + self.clone(source, dest_dir, branch, depth) except GitCommandError as e: raise UnhandledSource(e.message) except OSError as e: diff --git a/hooks/heat_relations.py b/hooks/heat_relations.py index 8926312..66828cd 100755 --- a/hooks/heat_relations.py +++ b/hooks/heat_relations.py @@ -41,8 +41,11 @@ from charmhelpers.contrib.openstack.utils import ( openstack_upgrade_available ) -from charmhelpers.contrib.hahelpers.cluster import ( - canonical_url +from charmhelpers.contrib.openstack.ip import ( + canonical_url, + ADMIN, + INTERNAL, + PUBLIC, ) from heat_utils import ( @@ -144,20 +147,31 @@ def configure_https(): @hooks.hook('identity-service-relation-joined') def identity_joined(rid=None): - base_url = canonical_url(CONFIGS) - api_url = '%s:8004/v1/$(tenant_id)s' % base_url - cfn_url = '%s:8000/v1' % base_url + public_url_base = canonical_url(CONFIGS, PUBLIC) + internal_url_base = canonical_url(CONFIGS, INTERNAL) + admin_url_base = canonical_url(CONFIGS, ADMIN) + + api_url_template = '%s:8004/v1/$(tenant_id)s' + public_api_endpoint = (api_url_template % public_url_base) + internal_api_endpoint = (api_url_template % internal_url_base) + admin_api_endpoint = (api_url_template % admin_url_base) + + cfn_url_template = '%s:8000/v1' + public_cfn_endpoint = (cfn_url_template % public_url_base) + internal_cfn_endpoint = (cfn_url_template % internal_url_base) + admin_cfn_endpoint = (cfn_url_template % admin_url_base) + relation_data = { 'heat_service': 'heat', 'heat_region': config('region'), - 'heat_public_url': api_url, - 'heat_admin_url': api_url, - 'heat_internal_url': api_url, + 'heat_public_url': public_api_endpoint, + 'heat_admin_url': admin_api_endpoint, + 'heat_internal_url': internal_api_endpoint, 'heat-cfn_service': 'heat-cfn', 'heat-cfn_region': config('region'), - 'heat-cfn_public_url': cfn_url, - 'heat-cfn_admin_url': cfn_url, - 'heat-cfn_internal_url': cfn_url + 'heat-cfn_public_url': public_cfn_endpoint, + 'heat-cfn_admin_url': admin_cfn_endpoint, + 'heat-cfn_internal_url': internal_cfn_endpoint, } relation_set(relation_id=rid, **relation_data) diff --git a/unit_tests/test_heat_relations.py b/unit_tests/test_heat_relations.py index 6b266ef..85d3ca3 100644 --- a/unit_tests/test_heat_relations.py +++ b/unit_tests/test_heat_relations.py @@ -18,7 +18,6 @@ utils.restart_map = _map TO_PATCH = [ # charmhelpers.core.hookenv 'Hooks', - 'canonical_url', 'config', 'open_port', 'relation_set', @@ -141,10 +140,11 @@ class HeatRelationTests(CharmTestCase): relations.relation_broken() self.assertTrue(configs.write_all.called) - def test_identity_service_joined(self): + @patch.object(relations, 'canonical_url') + def test_identity_service_joined(self, _canonical_url): "It properly requests unclustered endpoint via identity-service" self.unit_get.return_value = 'heatnode1' - self.canonical_url.return_value = 'http://heatnode1' + _canonical_url.return_value = 'http://heatnode1' relations.identity_joined() expected = { 'heat_service': 'heat', @@ -161,8 +161,9 @@ class HeatRelationTests(CharmTestCase): } self.relation_set.assert_called_with(**expected) - def test_identity_service_joined_with_relation_id(self): - self.canonical_url.return_value = 'http://heatnode1' + @patch.object(relations, 'canonical_url') + def test_identity_service_joined_with_relation_id(self, _canonical_url): + _canonical_url.return_value = 'http://heatnode1' relations.identity_joined(rid='identity-service:0') ex = { 'heat_service': 'heat', @@ -179,6 +180,33 @@ class HeatRelationTests(CharmTestCase): } self.relation_set.assert_called_with(**ex) + @patch('charmhelpers.contrib.openstack.ip.unit_get', + lambda *args: 'heatnode1') + @patch('charmhelpers.contrib.openstack.ip.is_clustered', + lambda *args: False) + @patch('charmhelpers.contrib.openstack.ip.service_name') + @patch('charmhelpers.contrib.openstack.ip.config') + def test_identity_service_public_address_override(self, ip_config, + _service_name): + ip_config.side_effect = self.test_config.get + _service_name.return_value = 'heat' + self.test_config.set('os-public-hostname', 'heat.example.org') + relations.identity_joined(rid='identity-service:0') + exp = { + 'heat_service': 'heat', + 'heat_region': 'RegionOne', + 'heat_public_url': 'http://heat.example.org:8004/v1/$(tenant_id)s', + 'heat_admin_url': 'http://heatnode1:8004/v1/$(tenant_id)s', + 'heat_internal_url': 'http://heatnode1:8004/v1/$(tenant_id)s', + 'heat-cfn_service': 'heat-cfn', + 'heat-cfn_region': 'RegionOne', + 'heat-cfn_public_url': 'http://heat.example.org:8000/v1', + 'heat-cfn_admin_url': 'http://heatnode1:8000/v1', + 'heat-cfn_internal_url': 'http://heatnode1:8000/v1', + 'relation_id': 'identity-service:0', + } + self.relation_set.assert_called_with(**exp) + @patch.object(relations, 'configure_https') @patch.object(relations, 'CONFIGS') def test_identity_changed(self, configs, mock_configure_https):