From 465617c2637d8d5068badb815afe6f1966ec9832 Mon Sep 17 00:00:00 2001 From: Billy Olsen Date: Thu, 8 Feb 2018 19:07:34 -0700 Subject: [PATCH] Sync charm-helpers and use VolumeAPIContext Sync in the charm-helpers to use the new VolumeAPIContext object in order to determine the volume catalog info to use in the configuration file. This is simply an alternative implementation for commit 5d92bc9f. This will separate concerns for determining internal endpoints and determining volume api versions. Change-Id: I91009e1f9643f818b6f97898aa5d7c43e84684ed Related-Bug: #1733566 --- hooks/charmhelpers/contrib/network/ip.py | 11 +- .../charmhelpers/contrib/openstack/context.py | 119 ++++++++++++++++-- .../contrib/openstack/templating.py | 91 ++++++++++---- hooks/charmhelpers/core/hookenv.py | 16 ++- hooks/charmhelpers/core/templating.py | 27 ++-- hooks/nova_compute_utils.py | 3 +- templates/parts/section-cinder | 2 +- tests/charmhelpers/contrib/network/ip.py | 11 +- tests/charmhelpers/core/hookenv.py | 16 ++- tests/charmhelpers/core/templating.py | 27 ++-- 10 files changed, 265 insertions(+), 58 deletions(-) diff --git a/hooks/charmhelpers/contrib/network/ip.py b/hooks/charmhelpers/contrib/network/ip.py index a871ce37..b13277bb 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 ( @@ -109,7 +110,12 @@ def get_address_in_network(network, fallback=None, fatal=False): _validate_cidr(network) network = netaddr.IPNetwork(network) for iface in netifaces.interfaces(): - addresses = netifaces.ifaddresses(iface) + try: + addresses = netifaces.ifaddresses(iface) + except ValueError: + # If an instance was deleted between + # netifaces.interfaces() run and now, its interfaces are gone + continue if network.version == 4 and netifaces.AF_INET in addresses: for addr in addresses[netifaces.AF_INET]: cidr = netaddr.IPNetwork("%s/%s" % (addr['addr'], @@ -578,6 +584,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 7ada2760..36cf32fc 100644 --- a/hooks/charmhelpers/contrib/openstack/context.py +++ b/hooks/charmhelpers/contrib/openstack/context.py @@ -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): @@ -631,7 +633,7 @@ class HAProxyContext(OSContextGenerator): # 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) # NOTE(thedac) For some reason the ADDRESS_MAP uses 'int' rather # than 'internal' @@ -1635,18 +1637,84 @@ class InternalEndpointContext(OSContextGenerator): endpoints by default so this allows admins to optionally use internal endpoints. """ - def __init__(self, ost_rel_check_pkg_name): - self.ost_rel_check_pkg_name = ost_rel_check_pkg_name + def __call__(self): + return {'use_internal_endpoints': config('use-internal-endpoints')} + + +class VolumeAPIContext(InternalEndpointContext): + """Volume API context. + + This context provides information regarding the volume endpoint to use + when communicating between services. It determines which version of the + API is appropriate for use. + + This value will be determined in the resulting context dictionary + returned from calling the VolumeAPIContext object. Information provided + by this context is as follows: + + volume_api_version: the volume api version to use, currently + 'v2' or 'v3' + volume_catalog_info: the information to use for a cinder client + configuration that consumes API endpoints from the keystone + catalog. This is defined as the type:name:endpoint_type string. + """ + # FIXME(wolsen) This implementation is based on the provider being able + # to specify the package version to check but does not guarantee that the + # volume service api version selected is available. In practice, it is + # quite likely the volume service *is* providing the v3 volume service. + # This should be resolved when the service-discovery spec is implemented. + def __init__(self, pkg): + """ + Creates a new VolumeAPIContext for use in determining which version + of the Volume API should be used for communication. A package codename + should be supplied for determining the currently installed OpenStack + version. + + :param pkg: the package codename to use in order to determine the + component version (e.g. nova-common). See + charmhelpers.contrib.openstack.utils.PACKAGE_CODENAMES for more. + """ + super(VolumeAPIContext, self).__init__() + self._ctxt = None + if not pkg: + raise ValueError('package name must be provided in order to ' + 'determine current OpenStack version.') + self.pkg = pkg + + @property + def ctxt(self): + if self._ctxt is not None: + return self._ctxt + self._ctxt = self._determine_ctxt() + return self._ctxt + + def _determine_ctxt(self): + """Determines the Volume API endpoint information. + + Determines the appropriate version of the API that should be used + as well as the catalog_info string that would be supplied. Returns + a dict containing the volume_api_version and the volume_catalog_info. + """ + rel = os_release(self.pkg, base='icehouse') + version = '2' + if CompareOpenStackReleases(rel) >= 'pike': + version = '3' + + service_type = 'volumev{version}'.format(version=version) + service_name = 'cinderv{version}'.format(version=version) + endpoint_type = 'publicURL' + if config('use-internal-endpoints'): + endpoint_type = 'internalURL' + catalog_info = '{type}:{name}:{endpoint}'.format( + type=service_type, name=service_name, endpoint=endpoint_type) + + return { + 'volume_api_version': version, + 'volume_catalog_info': catalog_info, + } def __call__(self): - ctxt = {'use_internal_endpoints': config('use-internal-endpoints')} - rel = os_release(self.ost_rel_check_pkg_name, base='icehouse') - if CompareOpenStackReleases(rel) >= 'pike': - ctxt['volume_api_version'] = '3' - else: - ctxt['volume_api_version'] = '2' - - return ctxt + return self.ctxt class AppArmorContext(OSContextGenerator): @@ -1784,3 +1852,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/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/core/hookenv.py b/hooks/charmhelpers/core/hookenv.py index 211ae87d..7ed1cc4e 100644 --- a/hooks/charmhelpers/core/hookenv.py +++ b/hooks/charmhelpers/core/hookenv.py @@ -820,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') @@ -1106,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/templating.py b/hooks/charmhelpers/core/templating.py index 7b801a34..9014015c 100644 --- a/hooks/charmhelpers/core/templating.py +++ b/hooks/charmhelpers/core/templating.py @@ -20,7 +20,8 @@ from charmhelpers.core import hookenv def render(source, target, context, owner='root', group='root', - perms=0o444, templates_dir=None, encoding='UTF-8', template_loader=None): + perms=0o444, templates_dir=None, encoding='UTF-8', + template_loader=None, config_template=None): """ Render a template. @@ -32,6 +33,9 @@ def render(source, target, context, owner='root', group='root', The context should be a dict containing the values to be replaced in the template. + config_template may be provided to render from a provided template instead + of loading from a file. + The `owner`, `group`, and `perms` options will be passed to `write_file`. If omitted, `templates_dir` defaults to the `templates` folder in the charm. @@ -65,14 +69,19 @@ def render(source, target, context, owner='root', group='root', if templates_dir is None: templates_dir = os.path.join(hookenv.charm_dir(), 'templates') template_env = Environment(loader=FileSystemLoader(templates_dir)) - try: - source = source - template = template_env.get_template(source) - except exceptions.TemplateNotFound as e: - hookenv.log('Could not load template %s from %s.' % - (source, templates_dir), - level=hookenv.ERROR) - raise e + + # load from a string if provided explicitly + if config_template is not None: + template = template_env.from_string(config_template) + else: + try: + source = source + template = template_env.get_template(source) + except exceptions.TemplateNotFound as e: + hookenv.log('Could not load template %s from %s.' % + (source, templates_dir), + level=hookenv.ERROR) + raise e content = template.render(context) if target is not None: target_dir = os.path.dirname(target) diff --git a/hooks/nova_compute_utils.py b/hooks/nova_compute_utils.py index 28f29022..66eb44b4 100644 --- a/hooks/nova_compute_utils.py +++ b/hooks/nova_compute_utils.py @@ -155,7 +155,8 @@ BASE_RESOURCE_MAP = { DesignateContext(), NovaComputeVirtContext(), context.LogLevelContext(), - context.InternalEndpointContext('nova-common'), + context.InternalEndpointContext(), + context.VolumeAPIContext('nova-common'), SerialConsoleContext(), NovaComputeAvailabilityZoneContext(), context.WorkerConfigContext()], diff --git a/templates/parts/section-cinder b/templates/parts/section-cinder index 3b3a4ad7..dabc3f1e 100644 --- a/templates/parts/section-cinder +++ b/templates/parts/section-cinder @@ -1,7 +1,7 @@ {% if volume_service and volume_service == 'cinder' -%} [cinder] {% if use_internal_endpoints -%} -catalog_info = volumev{{volume_api_version}}:cinderv{{volume_api_version}}:internalURL +catalog_info = {{ volume_catalog_info }} {% endif %} {% if region -%} os_region_name = {{ region }} diff --git a/tests/charmhelpers/contrib/network/ip.py b/tests/charmhelpers/contrib/network/ip.py index a871ce37..b13277bb 100644 --- a/tests/charmhelpers/contrib/network/ip.py +++ b/tests/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 ( @@ -109,7 +110,12 @@ def get_address_in_network(network, fallback=None, fatal=False): _validate_cidr(network) network = netaddr.IPNetwork(network) for iface in netifaces.interfaces(): - addresses = netifaces.ifaddresses(iface) + try: + addresses = netifaces.ifaddresses(iface) + except ValueError: + # If an instance was deleted between + # netifaces.interfaces() run and now, its interfaces are gone + continue if network.version == 4 and netifaces.AF_INET in addresses: for addr in addresses[netifaces.AF_INET]: cidr = netaddr.IPNetwork("%s/%s" % (addr['addr'], @@ -578,6 +584,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/tests/charmhelpers/core/hookenv.py b/tests/charmhelpers/core/hookenv.py index 211ae87d..7ed1cc4e 100644 --- a/tests/charmhelpers/core/hookenv.py +++ b/tests/charmhelpers/core/hookenv.py @@ -820,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') @@ -1106,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/templating.py b/tests/charmhelpers/core/templating.py index 7b801a34..9014015c 100644 --- a/tests/charmhelpers/core/templating.py +++ b/tests/charmhelpers/core/templating.py @@ -20,7 +20,8 @@ from charmhelpers.core import hookenv def render(source, target, context, owner='root', group='root', - perms=0o444, templates_dir=None, encoding='UTF-8', template_loader=None): + perms=0o444, templates_dir=None, encoding='UTF-8', + template_loader=None, config_template=None): """ Render a template. @@ -32,6 +33,9 @@ def render(source, target, context, owner='root', group='root', The context should be a dict containing the values to be replaced in the template. + config_template may be provided to render from a provided template instead + of loading from a file. + The `owner`, `group`, and `perms` options will be passed to `write_file`. If omitted, `templates_dir` defaults to the `templates` folder in the charm. @@ -65,14 +69,19 @@ def render(source, target, context, owner='root', group='root', if templates_dir is None: templates_dir = os.path.join(hookenv.charm_dir(), 'templates') template_env = Environment(loader=FileSystemLoader(templates_dir)) - try: - source = source - template = template_env.get_template(source) - except exceptions.TemplateNotFound as e: - hookenv.log('Could not load template %s from %s.' % - (source, templates_dir), - level=hookenv.ERROR) - raise e + + # load from a string if provided explicitly + if config_template is not None: + template = template_env.from_string(config_template) + else: + try: + source = source + template = template_env.get_template(source) + except exceptions.TemplateNotFound as e: + hookenv.log('Could not load template %s from %s.' % + (source, templates_dir), + level=hookenv.ERROR) + raise e content = template.render(context) if target is not None: target_dir = os.path.dirname(target)