From 62860678f48b306cff36fd327cbfe5fd32136b3c Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Wed, 29 Mar 2017 20:16:59 +0100 Subject: [PATCH] Fix alphanumeric comparisons for openstack and ubuntu releases - sync charmhelpers with fix-alpha helpers - fix up code where the alpha comparisons are done Change-Id: I9176ea3be01146b18888266ba53a9d37439ea492 Related-Bug: #1659575 --- charm-helpers-tests.yaml | 1 + .../charmhelpers/contrib/charmsupport/nrpe.py | 2 +- hooks/charmhelpers/contrib/network/ip.py | 111 ++++++++++----- .../contrib/openstack/amulet/utils.py | 5 +- .../charmhelpers/contrib/openstack/context.py | 132 ++++++++++++------ .../charmhelpers/contrib/openstack/neutron.py | 19 ++- .../contrib/openstack/templates/haproxy.cfg | 11 ++ hooks/charmhelpers/contrib/openstack/utils.py | 40 +++++- .../contrib/storage/linux/ceph.py | 18 +-- hooks/charmhelpers/core/host.py | 6 +- .../charmhelpers/core/host_factory/centos.py | 16 +++ .../charmhelpers/core/host_factory/ubuntu.py | 32 +++++ hooks/charmhelpers/core/strutils.py | 53 +++++++ hooks/lxd_utils.py | 10 +- test-requirements.txt | 16 +-- .../contrib/openstack/amulet/utils.py | 5 +- tests/charmhelpers/core/host.py | 6 +- .../charmhelpers/core/host_factory/centos.py | 16 +++ .../charmhelpers/core/host_factory/ubuntu.py | 32 +++++ tests/charmhelpers/core/strutils.py | 53 +++++++ tests/charmhelpers/osplatform.py | 25 ++++ tox.ini | 2 +- 22 files changed, 489 insertions(+), 122 deletions(-) create mode 100644 tests/charmhelpers/osplatform.py diff --git a/charm-helpers-tests.yaml b/charm-helpers-tests.yaml index e506325..b0de9df 100644 --- a/charm-helpers-tests.yaml +++ b/charm-helpers-tests.yaml @@ -4,3 +4,4 @@ include: - contrib.amulet - contrib.openstack.amulet - core + - osplatform diff --git a/hooks/charmhelpers/contrib/charmsupport/nrpe.py b/hooks/charmhelpers/contrib/charmsupport/nrpe.py index 9646b83..8240249 100644 --- a/hooks/charmhelpers/contrib/charmsupport/nrpe.py +++ b/hooks/charmhelpers/contrib/charmsupport/nrpe.py @@ -373,7 +373,7 @@ def add_init_service_checks(nrpe, services, unit_name, immediate_check=True): checkpath = '%s/service-check-%s.txt' % (nrpe.homedir, svc) croncmd = ( '/usr/local/lib/nagios/plugins/check_exit_status.pl ' - '-s /etc/init.d/%s status' % svc + '-e -s /etc/init.d/%s status' % svc ) cron_file = '*/5 * * * * root %s > %s\n' % (croncmd, checkpath) f = open(cronpath, 'w') diff --git a/hooks/charmhelpers/contrib/network/ip.py b/hooks/charmhelpers/contrib/network/ip.py index 54c76a7..fc3f5e3 100644 --- a/hooks/charmhelpers/contrib/network/ip.py +++ b/hooks/charmhelpers/contrib/network/ip.py @@ -31,6 +31,7 @@ from charmhelpers.core.hookenv import ( from charmhelpers.core.host import ( lsb_release, + CompareHostReleases, ) try: @@ -67,6 +68,24 @@ def no_ip_found_error_out(network): raise ValueError(errmsg) +def _get_ipv6_network_from_address(address): + """Get an netaddr.IPNetwork for the given IPv6 address + :param address: a dict as returned by netifaces.ifaddresses + :returns netaddr.IPNetwork: None if the address is a link local or loopback + address + """ + if address['addr'].startswith('fe80') or address['addr'] == "::1": + return None + + prefix = address['netmask'].split("/") + if len(prefix) > 1: + netmask = prefix[1] + else: + netmask = address['netmask'] + return netaddr.IPNetwork("%s/%s" % (address['addr'], + netmask)) + + def get_address_in_network(network, fallback=None, fatal=False): """Get an IPv4 or IPv6 address within the network from the host. @@ -92,19 +111,17 @@ def get_address_in_network(network, fallback=None, fatal=False): for iface in netifaces.interfaces(): addresses = netifaces.ifaddresses(iface) if network.version == 4 and netifaces.AF_INET in addresses: - addr = addresses[netifaces.AF_INET][0]['addr'] - netmask = addresses[netifaces.AF_INET][0]['netmask'] - cidr = netaddr.IPNetwork("%s/%s" % (addr, netmask)) - if cidr in network: - return str(cidr.ip) + for addr in addresses[netifaces.AF_INET]: + cidr = netaddr.IPNetwork("%s/%s" % (addr['addr'], + addr['netmask'])) + if cidr in network: + return str(cidr.ip) if network.version == 6 and netifaces.AF_INET6 in addresses: for addr in addresses[netifaces.AF_INET6]: - if not addr['addr'].startswith('fe80'): - cidr = netaddr.IPNetwork("%s/%s" % (addr['addr'], - addr['netmask'])) - if cidr in network: - return str(cidr.ip) + cidr = _get_ipv6_network_from_address(addr) + if cidr and cidr in network: + return str(cidr.ip) if fallback is not None: return fallback @@ -180,18 +197,18 @@ def _get_for_address(address, key): if address.version == 6 and netifaces.AF_INET6 in addresses: for addr in addresses[netifaces.AF_INET6]: - if not addr['addr'].startswith('fe80'): - network = netaddr.IPNetwork("%s/%s" % (addr['addr'], - addr['netmask'])) - cidr = network.cidr - if address in cidr: - if key == 'iface': - return iface - elif key == 'netmask' and cidr: - return str(cidr).split('/')[1] - else: - return addr[key] + network = _get_ipv6_network_from_address(addr) + if not network: + continue + cidr = network.cidr + if address in cidr: + if key == 'iface': + return iface + elif key == 'netmask' and cidr: + return str(cidr).split('/')[1] + else: + return addr[key] return None @@ -222,6 +239,16 @@ def format_ipv6_addr(address): return None +def is_ipv6_disabled(): + try: + result = subprocess.check_output( + ['sysctl', 'net.ipv6.conf.all.disable_ipv6'], + stderr=subprocess.STDOUT) + return "net.ipv6.conf.all.disable_ipv6 = 1" in result + except subprocess.CalledProcessError: + return True + + def get_iface_addr(iface='eth0', inet_type='AF_INET', inc_aliases=False, fatal=True, exc_list=None): """Return the assigned IP address for a given interface, if any. @@ -521,36 +548,44 @@ def port_has_listener(address, port): def assert_charm_supports_ipv6(): """Check whether we are able to support charms ipv6.""" - if lsb_release()['DISTRIB_CODENAME'].lower() < "trusty": + release = lsb_release()['DISTRIB_CODENAME'].lower() + if CompareHostReleases(release) < "trusty": raise Exception("IPv6 is not supported in the charms for Ubuntu " "versions less than Trusty 14.04") -def get_relation_ip(interface, config_override=None): - """Return this unit's IP for the given relation. +def get_relation_ip(interface, cidr_network=None): + """Return this unit's IP for the given interface. Allow for an arbitrary interface to use with network-get to select an IP. - Handle all address selection options including configuration parameter - override and IPv6. + Handle all address selection options including passed cidr network and + IPv6. - Usage: get_relation_ip('amqp', config_override='access-network') + Usage: get_relation_ip('amqp', cidr_network='10.0.0.0/8') @param interface: string name of the relation. - @param config_override: string name of the config option for network - override. Supports legacy network override configuration parameters. + @param cidr_network: string CIDR Network to select an address from. @raises Exception if prefer-ipv6 is configured but IPv6 unsupported. @returns IPv6 or IPv4 address """ + # Select the interface address first + # For possible use as a fallback bellow with get_address_in_network + try: + # Get the interface specific IP + address = network_get_primary_address(interface) + except NotImplementedError: + # If network-get is not available + address = get_host_ip(unit_get('private-address')) - fallback = get_host_ip(unit_get('private-address')) if config('prefer-ipv6'): + # Currently IPv6 has priority, eventually we want IPv6 to just be + # another network space. assert_charm_supports_ipv6() return get_ipv6_addr()[0] - elif config_override and config(config_override): - return get_address_in_network(config(config_override), - fallback) - else: - try: - return network_get_primary_address(interface) - except NotImplementedError: - return fallback + elif cidr_network: + # If a specific CIDR network is passed get the address from that + # network. + return get_address_in_network(cidr_network, address) + + # Return the interface address + return address diff --git a/hooks/charmhelpers/contrib/openstack/amulet/utils.py b/hooks/charmhelpers/contrib/openstack/amulet/utils.py index 1f4cf42..bcef4cd 100644 --- a/hooks/charmhelpers/contrib/openstack/amulet/utils.py +++ b/hooks/charmhelpers/contrib/openstack/amulet/utils.py @@ -40,6 +40,7 @@ from charmhelpers.contrib.amulet.utils import ( AmuletUtils ) from charmhelpers.core.decorators import retry_on_exception +from charmhelpers.core.host import CompareHostReleases DEBUG = logging.DEBUG ERROR = logging.ERROR @@ -546,7 +547,7 @@ class OpenStackAmuletUtils(AmuletUtils): """Create the specified instance.""" self.log.debug('Creating instance ' '({}|{}|{})'.format(instance_name, image_name, flavor)) - image = nova.images.find(name=image_name) + image = nova.glance.find_image(image_name) flavor = nova.flavors.find(name=flavor) instance = nova.servers.create(name=instance_name, image=image, flavor=flavor) @@ -1255,7 +1256,7 @@ class OpenStackAmuletUtils(AmuletUtils): contents = self.file_contents_safe(sentry_unit, '/etc/memcached.conf', fatal=True) ubuntu_release, _ = self.run_cmd_unit(sentry_unit, 'lsb_release -cs') - if ubuntu_release <= 'trusty': + if CompareHostReleases(ubuntu_release) <= 'trusty': memcache_listen_addr = 'ip6-localhost' else: memcache_listen_addr = '::1' diff --git a/hooks/charmhelpers/contrib/openstack/context.py b/hooks/charmhelpers/contrib/openstack/context.py index 6cdbbbb..2adf2cb 100644 --- a/hooks/charmhelpers/contrib/openstack/context.py +++ b/hooks/charmhelpers/contrib/openstack/context.py @@ -59,6 +59,8 @@ from charmhelpers.core.host import ( write_file, pwgen, lsb_release, + CompareHostReleases, + is_container, ) from charmhelpers.contrib.hahelpers.cluster import ( determine_apache_port, @@ -87,6 +89,7 @@ from charmhelpers.contrib.network.ip import ( format_ipv6_addr, is_address_in_network, is_bridge_member, + is_ipv6_disabled, ) from charmhelpers.contrib.openstack.utils import ( config_flags_parser, @@ -108,6 +111,7 @@ except ImportError: CA_CERT_PATH = '/usr/local/share/ca-certificates/keystone_juju_ca_cert.crt' ADDRESS_TYPES = ['admin', 'internal', 'public'] +HAPROXY_RUN_DIR = '/var/run/haproxy/' def ensure_packages(packages): @@ -155,7 +159,8 @@ class OSContextGenerator(object): if self.missing_data: self.complete = False - log('Missing required data: %s' % ' '.join(self.missing_data), level=INFO) + log('Missing required data: %s' % ' '.join(self.missing_data), + level=INFO) else: self.complete = True return self.complete @@ -213,8 +218,9 @@ class SharedDBContext(OSContextGenerator): hostname_key = "{}_hostname".format(self.relation_prefix) else: hostname_key = "hostname" - access_hostname = get_address_in_network(access_network, - unit_get('private-address')) + access_hostname = get_address_in_network( + access_network, + unit_get('private-address')) set_hostname = relation_get(attribute=hostname_key, unit=local_unit()) if set_hostname != access_hostname: @@ -308,7 +314,10 @@ def db_ssl(rdata, ctxt, ssl_dir): class IdentityServiceContext(OSContextGenerator): - def __init__(self, service=None, service_user=None, rel_name='identity-service'): + def __init__(self, + service=None, + service_user=None, + rel_name='identity-service'): self.service = service self.service_user = service_user self.rel_name = rel_name @@ -457,19 +466,17 @@ class AMQPContext(OSContextGenerator): host = format_ipv6_addr(host) or host rabbitmq_hosts.append(host) - ctxt['rabbitmq_hosts'] = ','.join(sorted(rabbitmq_hosts)) + rabbitmq_hosts = sorted(rabbitmq_hosts) + ctxt['rabbitmq_hosts'] = ','.join(rabbitmq_hosts) transport_hosts = rabbitmq_hosts if transport_hosts: - transport_url_hosts = '' - for host in transport_hosts: - if transport_url_hosts: - format_string = ",{}:{}@{}:{}" - else: - format_string = "{}:{}@{}:{}" - transport_url_hosts += format_string.format( - ctxt['rabbitmq_user'], ctxt['rabbitmq_password'], - host, rabbitmq_port) + transport_url_hosts = ','.join([ + "{}:{}@{}:{}".format(ctxt['rabbitmq_user'], + ctxt['rabbitmq_password'], + host_, + rabbitmq_port) + for host_ in transport_hosts]) ctxt['transport_url'] = "rabbit://{}/{}".format( transport_url_hosts, vhost) @@ -530,6 +537,8 @@ class HAProxyContext(OSContextGenerator): """Provides half a context for the haproxy template, which describes all peers to be included in the cluster. Each charm needs to include its own context generator that describes the port mapping. + + :side effect: mkdir is called on HAPROXY_RUN_DIR """ interfaces = ['cluster'] @@ -537,6 +546,8 @@ class HAProxyContext(OSContextGenerator): self.singlenode_mode = singlenode_mode def __call__(self): + if not os.path.isdir(HAPROXY_RUN_DIR): + mkdir(path=HAPROXY_RUN_DIR) if not relation_ids('cluster') and not self.singlenode_mode: return {} @@ -1217,22 +1228,54 @@ class BindHostContext(OSContextGenerator): return {'bind_host': '0.0.0.0'} +MAX_DEFAULT_WORKERS = 4 +DEFAULT_MULTIPLIER = 2 + + +def _calculate_workers(): + ''' + Determine the number of worker processes based on the CPU + count of the unit containing the application. + + Workers will be limited to MAX_DEFAULT_WORKERS in + container environments where no worker-multipler configuration + option been set. + + @returns int: number of worker processes to use + ''' + multiplier = config('worker-multiplier') or DEFAULT_MULTIPLIER + count = int(_num_cpus() * multiplier) + if multiplier > 0 and count == 0: + count = 1 + + if config('worker-multiplier') is None and is_container(): + # NOTE(jamespage): Limit unconfigured worker-multiplier + # to MAX_DEFAULT_WORKERS to avoid insane + # worker configuration in LXD containers + # on large servers + # Reference: https://pad.lv/1665270 + count = min(count, MAX_DEFAULT_WORKERS) + + return count + + +def _num_cpus(): + ''' + Compatibility wrapper for calculating the number of CPU's + a unit has. + + @returns: int: number of CPU cores detected + ''' + try: + return psutil.cpu_count() + except AttributeError: + return psutil.NUM_CPUS + + class WorkerConfigContext(OSContextGenerator): - @property - def num_cpus(self): - # NOTE: use cpu_count if present (16.04 support) - if hasattr(psutil, 'cpu_count'): - return psutil.cpu_count() - else: - return psutil.NUM_CPUS - def __call__(self): - multiplier = config('worker-multiplier') or 0 - count = int(self.num_cpus * multiplier) - if multiplier > 0 and count == 0: - count = 1 - ctxt = {"workers": count} + ctxt = {"workers": _calculate_workers()} return ctxt @@ -1240,7 +1283,7 @@ class WSGIWorkerConfigContext(WorkerConfigContext): def __init__(self, name=None, script=None, admin_script=None, public_script=None, process_weight=1.00, - admin_process_weight=0.75, public_process_weight=0.25): + admin_process_weight=0.25, public_process_weight=0.75): self.service_name = name self.user = name self.group = name @@ -1252,8 +1295,7 @@ class WSGIWorkerConfigContext(WorkerConfigContext): self.public_process_weight = public_process_weight def __call__(self): - multiplier = config('worker-multiplier') or 1 - total_processes = self.num_cpus * multiplier + total_processes = _calculate_workers() ctxt = { "service_name": self.service_name, "user": self.user, @@ -1584,7 +1626,7 @@ class MemcacheContext(OSContextGenerator): """Memcache context This context provides options for configuring a local memcache client and - server + server for both IPv4 and IPv6 """ def __init__(self, package=None): @@ -1601,13 +1643,25 @@ class MemcacheContext(OSContextGenerator): if ctxt['use_memcache']: # Trusty version of memcached does not support ::1 as a listen # address so use host file entry instead - if lsb_release()['DISTRIB_CODENAME'].lower() > 'trusty': - ctxt['memcache_server'] = '::1' + release = lsb_release()['DISTRIB_CODENAME'].lower() + if is_ipv6_disabled(): + if CompareHostReleases(release) > 'trusty': + ctxt['memcache_server'] = '127.0.0.1' + else: + ctxt['memcache_server'] = 'localhost' + ctxt['memcache_server_formatted'] = '127.0.0.1' + ctxt['memcache_port'] = '11211' + ctxt['memcache_url'] = '{}:{}'.format( + ctxt['memcache_server_formatted'], + ctxt['memcache_port']) else: - ctxt['memcache_server'] = 'ip6-localhost' - ctxt['memcache_server_formatted'] = '[::1]' - ctxt['memcache_port'] = '11211' - ctxt['memcache_url'] = 'inet6:{}:{}'.format( - ctxt['memcache_server_formatted'], - ctxt['memcache_port']) + if CompareHostReleases(release) > 'trusty': + ctxt['memcache_server'] = '::1' + else: + ctxt['memcache_server'] = 'ip6-localhost' + ctxt['memcache_server_formatted'] = '[::1]' + ctxt['memcache_port'] = '11211' + ctxt['memcache_url'] = 'inet6:{}:{}'.format( + ctxt['memcache_server_formatted'], + ctxt['memcache_port']) return ctxt diff --git a/hooks/charmhelpers/contrib/openstack/neutron.py b/hooks/charmhelpers/contrib/openstack/neutron.py index a8f1ed7..37fa0eb 100644 --- a/hooks/charmhelpers/contrib/openstack/neutron.py +++ b/hooks/charmhelpers/contrib/openstack/neutron.py @@ -23,7 +23,10 @@ from charmhelpers.core.hookenv import ( ERROR, ) -from charmhelpers.contrib.openstack.utils import os_release +from charmhelpers.contrib.openstack.utils import ( + os_release, + CompareOpenStackReleases, +) def headers_package(): @@ -198,7 +201,8 @@ def neutron_plugins(): }, 'plumgrid': { 'config': '/etc/neutron/plugins/plumgrid/plumgrid.ini', - 'driver': 'neutron.plugins.plumgrid.plumgrid_plugin.plumgrid_plugin.NeutronPluginPLUMgridV2', + 'driver': ('neutron.plugins.plumgrid.plumgrid_plugin' + '.plumgrid_plugin.NeutronPluginPLUMgridV2'), 'contexts': [ context.SharedDBContext(user=config('database-user'), database=config('database'), @@ -225,7 +229,7 @@ def neutron_plugins(): 'server_services': ['neutron-server'] } } - if release >= 'icehouse': + if CompareOpenStackReleases(release) >= 'icehouse': # NOTE: patch in ml2 plugin for icehouse onwards plugins['ovs']['config'] = '/etc/neutron/plugins/ml2/ml2_conf.ini' plugins['ovs']['driver'] = 'neutron.plugins.ml2.plugin.Ml2Plugin' @@ -233,10 +237,10 @@ def neutron_plugins(): 'neutron-plugin-ml2'] # NOTE: patch in vmware renames nvp->nsx for icehouse onwards plugins['nvp'] = plugins['nsx'] - if release >= 'kilo': + if CompareOpenStackReleases(release) >= 'kilo': plugins['midonet']['driver'] = ( 'neutron.plugins.midonet.plugin.MidonetPluginV2') - if release >= 'liberty': + if CompareOpenStackReleases(release) >= 'liberty': plugins['midonet']['driver'] = ( 'midonet.neutron.plugin_v1.MidonetPluginV2') plugins['midonet']['server_packages'].remove( @@ -244,10 +248,11 @@ def neutron_plugins(): plugins['midonet']['server_packages'].append( 'python-networking-midonet') plugins['plumgrid']['driver'] = ( - 'networking_plumgrid.neutron.plugins.plugin.NeutronPluginPLUMgridV2') + 'networking_plumgrid.neutron.plugins' + '.plugin.NeutronPluginPLUMgridV2') plugins['plumgrid']['server_packages'].remove( 'neutron-plugin-plumgrid') - if release >= 'mitaka': + if CompareOpenStackReleases(release) >= 'mitaka': plugins['nsx']['server_packages'].remove('neutron-plugin-vmware') plugins['nsx']['server_packages'].append('python-vmware-nsx') plugins['nsx']['config'] = '/etc/neutron/nsx.ini' diff --git a/hooks/charmhelpers/contrib/openstack/templates/haproxy.cfg b/hooks/charmhelpers/contrib/openstack/templates/haproxy.cfg index 32b6276..54fba39 100644 --- a/hooks/charmhelpers/contrib/openstack/templates/haproxy.cfg +++ b/hooks/charmhelpers/contrib/openstack/templates/haproxy.cfg @@ -5,6 +5,8 @@ global user haproxy group haproxy spread-checks 0 + stats socket /var/run/haproxy/admin.sock mode 600 level admin + stats timeout 2m defaults log global @@ -58,6 +60,15 @@ frontend tcp-in_{{ service }} {% for frontend in frontends -%} backend {{ service }}_{{ frontend }} balance leastconn + {% if backend_options -%} + {% if backend_options[service] -%} + {% for option in backend_options[service] -%} + {% for key, value in option.items() -%} + {{ key }} {{ value }} + {% endfor -%} + {% endfor -%} + {% endif -%} + {% endif -%} {% for unit, address in frontends[frontend]['backends'].items() -%} server {{ unit }} {{ address }}:{{ ports[1] }} check {% endfor %} diff --git a/hooks/charmhelpers/contrib/openstack/utils.py b/hooks/charmhelpers/contrib/openstack/utils.py index 7e8ecff..e13450c 100644 --- a/hooks/charmhelpers/contrib/openstack/utils.py +++ b/hooks/charmhelpers/contrib/openstack/utils.py @@ -33,9 +33,7 @@ import yaml from charmhelpers.contrib.network import ip -from charmhelpers.core import ( - unitdata, -) +from charmhelpers.core import unitdata from charmhelpers.core.hookenv import ( action_fail, @@ -55,6 +53,8 @@ from charmhelpers.core.hookenv import ( application_version_set, ) +from charmhelpers.core.strutils import BasicStringComparator + from charmhelpers.contrib.storage.linux.lvm import ( deactivate_lvm_volume_group, is_lvm_physical_volume, @@ -97,6 +97,22 @@ CLOUD_ARCHIVE_KEY_ID = '5EDB1B62EC4926EA' DISTRO_PROPOSED = ('deb http://archive.ubuntu.com/ubuntu/ %s-proposed ' 'restricted main multiverse universe') +OPENSTACK_RELEASES = ( + 'diablo', + 'essex', + 'folsom', + 'grizzly', + 'havana', + 'icehouse', + 'juno', + 'kilo', + 'liberty', + 'mitaka', + 'newton', + 'ocata', + 'pike', +) + UBUNTU_OPENSTACK_RELEASE = OrderedDict([ ('oneiric', 'diablo'), ('precise', 'essex'), @@ -238,6 +254,17 @@ GIT_DEFAULT_BRANCHES = { DEFAULT_LOOPBACK_SIZE = '5G' +class CompareOpenStackReleases(BasicStringComparator): + """Provide comparisons of OpenStack releases. + + Use in the form of + + if CompareOpenStackReleases(release) > 'mitaka': + # do something with mitaka + """ + _list = OPENSTACK_RELEASES + + def error_out(msg): juju_log("FATAL ERROR: %s" % msg, level='ERROR') sys.exit(1) @@ -1066,7 +1093,8 @@ def git_generate_systemd_init_files(templates_dir): shutil.copyfile(init_in_source, init_source) with open(init_source, 'a') as outfile: - template = '/usr/share/openstack-pkg-tools/init-script-template' + template = ('/usr/share/openstack-pkg-tools/' + 'init-script-template') with open(template) as infile: outfile.write('\n\n{}'.format(infile.read())) @@ -1971,9 +1999,7 @@ def enable_memcache(source=None, release=None, package=None): if not _release: _release = get_os_codename_install_source(source) - # TODO: this should be changed to a numeric comparison using a known list - # of releases and comparing by index. - return _release >= 'mitaka' + return CompareOpenStackReleases(_release) >= 'mitaka' def token_cache_pkgs(source=None, release=None): diff --git a/hooks/charmhelpers/contrib/storage/linux/ceph.py b/hooks/charmhelpers/contrib/storage/linux/ceph.py index ae7f3f9..9417d68 100644 --- a/hooks/charmhelpers/contrib/storage/linux/ceph.py +++ b/hooks/charmhelpers/contrib/storage/linux/ceph.py @@ -987,18 +987,20 @@ def ensure_ceph_storage(service, pool, rbd_img, sizemb, mount_point, service_start(svc) -def ensure_ceph_keyring(service, user=None, group=None, relation='ceph'): +def ensure_ceph_keyring(service, user=None, group=None, + relation='ceph', key=None): """Ensures a ceph keyring is created for a named service and optionally ensures user and group ownership. - Returns False if no ceph key is available in relation state. + @returns boolean: Flag to indicate whether a key was successfully written + to disk based on either relation data or a supplied key """ - key = None - for rid in relation_ids(relation): - for unit in related_units(rid): - key = relation_get('key', rid=rid, unit=unit) - if key: - break + if not key: + for rid in relation_ids(relation): + for unit in related_units(rid): + key = relation_get('key', rid=rid, unit=unit) + if key: + break if not key: return False diff --git a/hooks/charmhelpers/core/host.py b/hooks/charmhelpers/core/host.py index 05edfa5..88e80a4 100644 --- a/hooks/charmhelpers/core/host.py +++ b/hooks/charmhelpers/core/host.py @@ -45,6 +45,7 @@ if __platform__ == "ubuntu": add_new_group, lsb_release, cmp_pkgrevno, + CompareHostReleases, ) # flake8: noqa -- ignore F401 for this import elif __platform__ == "centos": from charmhelpers.core.host_factory.centos import ( @@ -52,6 +53,7 @@ elif __platform__ == "centos": add_new_group, lsb_release, cmp_pkgrevno, + CompareHostReleases, ) # flake8: noqa -- ignore F401 for this import UPDATEDB_PATH = '/etc/updatedb.conf' @@ -189,7 +191,7 @@ def service_pause(service_name, init_dir="/etc/init", initd_dir="/etc/init.d", upstart_file = os.path.join(init_dir, "{}.conf".format(service_name)) sysv_file = os.path.join(initd_dir, service_name) if init_is_systemd(): - service('disable', service_name) + service('mask', service_name) elif os.path.exists(upstart_file): override_path = os.path.join( init_dir, '{}.override'.format(service_name)) @@ -222,7 +224,7 @@ def service_resume(service_name, init_dir="/etc/init", upstart_file = os.path.join(init_dir, "{}.conf".format(service_name)) sysv_file = os.path.join(initd_dir, service_name) if init_is_systemd(): - service('enable', service_name) + service('unmask', service_name) elif os.path.exists(upstart_file): override_path = os.path.join( init_dir, '{}.override'.format(service_name)) diff --git a/hooks/charmhelpers/core/host_factory/centos.py b/hooks/charmhelpers/core/host_factory/centos.py index 902d469..7781a39 100644 --- a/hooks/charmhelpers/core/host_factory/centos.py +++ b/hooks/charmhelpers/core/host_factory/centos.py @@ -2,6 +2,22 @@ import subprocess import yum import os +from charmhelpers.core.strutils import BasicStringComparator + + +class CompareHostReleases(BasicStringComparator): + """Provide comparisons of Host releases. + + Use in the form of + + if CompareHostReleases(release) > 'trusty': + # do something with mitaka + """ + + def __init__(self, item): + raise NotImplementedError( + "CompareHostReleases() is not implemented for CentOS") + def service_available(service_name): # """Determine whether a system service is available.""" diff --git a/hooks/charmhelpers/core/host_factory/ubuntu.py b/hooks/charmhelpers/core/host_factory/ubuntu.py index 8c66af5..0448288 100644 --- a/hooks/charmhelpers/core/host_factory/ubuntu.py +++ b/hooks/charmhelpers/core/host_factory/ubuntu.py @@ -1,5 +1,37 @@ import subprocess +from charmhelpers.core.strutils import BasicStringComparator + + +UBUNTU_RELEASES = ( + 'lucid', + 'maverick', + 'natty', + 'oneiric', + 'precise', + 'quantal', + 'raring', + 'saucy', + 'trusty', + 'utopic', + 'vivid', + 'wily', + 'xenial', + 'yakkety', + 'zesty', +) + + +class CompareHostReleases(BasicStringComparator): + """Provide comparisons of Ubuntu releases. + + Use in the form of + + if CompareHostReleases(release) > 'trusty': + # do something with mitaka + """ + _list = UBUNTU_RELEASES + def service_available(service_name): """Determine whether a system service is available""" diff --git a/hooks/charmhelpers/core/strutils.py b/hooks/charmhelpers/core/strutils.py index dd9b971..685dabd 100644 --- a/hooks/charmhelpers/core/strutils.py +++ b/hooks/charmhelpers/core/strutils.py @@ -68,3 +68,56 @@ def bytes_from_string(value): msg = "Unable to interpret string value '%s' as bytes" % (value) raise ValueError(msg) return int(matches.group(1)) * (1024 ** BYTE_POWER[matches.group(2)]) + + +class BasicStringComparator(object): + """Provides a class that will compare strings from an iterator type object. + Used to provide > and < comparisons on strings that may not necessarily be + alphanumerically ordered. e.g. OpenStack or Ubuntu releases AFTER the + z-wrap. + """ + + _list = None + + def __init__(self, item): + if self._list is None: + raise Exception("Must define the _list in the class definition!") + try: + self.index = self._list.index(item) + except Exception: + raise KeyError("Item '{}' is not in list '{}'" + .format(item, self._list)) + + def __eq__(self, other): + assert isinstance(other, str) or isinstance(other, self.__class__) + return self.index == self._list.index(other) + + def __ne__(self, other): + return not self.__eq__(other) + + def __lt__(self, other): + assert isinstance(other, str) or isinstance(other, self.__class__) + return self.index < self._list.index(other) + + def __ge__(self, other): + return not self.__lt__(other) + + def __gt__(self, other): + assert isinstance(other, str) or isinstance(other, self.__class__) + return self.index > self._list.index(other) + + def __le__(self, other): + return not self.__gt__(other) + + def __str__(self): + """Always give back the item at the index so it can be used in + comparisons like: + + s_mitaka = CompareOpenStack('mitaka') + s_newton = CompareOpenstack('newton') + + assert s_newton > s_mitaka + + @returns: + """ + return self._list[self.index] diff --git a/hooks/lxd_utils.py b/hooks/lxd_utils.py index 4b01891..5349b7f 100644 --- a/hooks/lxd_utils.py +++ b/hooks/lxd_utils.py @@ -47,6 +47,7 @@ from charmhelpers.core.host import ( pwgen, lsb_release, is_container, + CompareHostReleases, ) from charmhelpers.contrib.storage.linux.utils import ( is_block_device, @@ -410,7 +411,8 @@ def configure_lxd_remote(settings, user='root'): @retry_on_exception(5, base_delay=2, exc_type=CalledProcessError) def configure_lxd_host(): ubuntu_release = lsb_release()['DISTRIB_CODENAME'].lower() - if ubuntu_release > "vivid": + cmp_ubuntu_release = CompareHostReleases(ubuntu_release) + if cmp_ubuntu_release > "vivid": log('>= Wily deployment - configuring LXD trust password and address', level=INFO) cmd = ['lxc', 'config', 'set', @@ -425,11 +427,11 @@ def configure_lxd_host(): # within a container on an all-in-one install # Configure live migration - if ubuntu_release == 'xenial': + if cmp_ubuntu_release == 'xenial': apt_install('linux-image-extra-%s' % os.uname()[2], fatal=True) - if ubuntu_release >= 'xenial': + if cmp_ubuntu_release >= 'xenial': modprobe('netlink_diag') # Enable/disable use of ext4 within nova-lxd containers @@ -440,7 +442,7 @@ def configure_lxd_host(): ) configure_uid_mapping() - elif ubuntu_release == "vivid": + elif cmp_ubuntu_release == "vivid": log('Vivid deployment - loading overlay kernel module', level=INFO) cmd = ['modprobe', 'overlay'] check_call(cmd) diff --git a/test-requirements.txt b/test-requirements.txt index 74baa12..04ca623 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -11,15 +11,13 @@ requests==2.6.0 # Liberty client lower constraints amulet>=1.14.3,<2.0 bundletester>=0.6.1,<1.0 -python-ceilometerclient>=1.5.0,<2.0 -python-cinderclient>=1.4.0,<2.0 -python-glanceclient>=1.1.0,<2.0 -python-heatclient>=0.8.0,<1.0 -python-keystoneclient>=1.7.1,<2.0 -python-neutronclient>=3.1.0,<4.0 -python-novaclient>=2.30.1,<3.0 -python-openstackclient>=1.7.0,<2.0 -python-swiftclient>=2.6.0,<3.0 +python-ceilometerclient>=1.5.0 +python-cinderclient>=1.4.0 +python-glanceclient>=1.1.0 +python-heatclient>=0.8.0 +python-novaclient>=2.30.1 +python-openstackclient>=1.7.0 +python-swiftclient>=2.6.0 pika>=0.10.0,<1.0 distro-info # END: Amulet OpenStack Charm Helper Requirements diff --git a/tests/charmhelpers/contrib/openstack/amulet/utils.py b/tests/charmhelpers/contrib/openstack/amulet/utils.py index 1f4cf42..bcef4cd 100644 --- a/tests/charmhelpers/contrib/openstack/amulet/utils.py +++ b/tests/charmhelpers/contrib/openstack/amulet/utils.py @@ -40,6 +40,7 @@ from charmhelpers.contrib.amulet.utils import ( AmuletUtils ) from charmhelpers.core.decorators import retry_on_exception +from charmhelpers.core.host import CompareHostReleases DEBUG = logging.DEBUG ERROR = logging.ERROR @@ -546,7 +547,7 @@ class OpenStackAmuletUtils(AmuletUtils): """Create the specified instance.""" self.log.debug('Creating instance ' '({}|{}|{})'.format(instance_name, image_name, flavor)) - image = nova.images.find(name=image_name) + image = nova.glance.find_image(image_name) flavor = nova.flavors.find(name=flavor) instance = nova.servers.create(name=instance_name, image=image, flavor=flavor) @@ -1255,7 +1256,7 @@ class OpenStackAmuletUtils(AmuletUtils): contents = self.file_contents_safe(sentry_unit, '/etc/memcached.conf', fatal=True) ubuntu_release, _ = self.run_cmd_unit(sentry_unit, 'lsb_release -cs') - if ubuntu_release <= 'trusty': + if CompareHostReleases(ubuntu_release) <= 'trusty': memcache_listen_addr = 'ip6-localhost' else: memcache_listen_addr = '::1' diff --git a/tests/charmhelpers/core/host.py b/tests/charmhelpers/core/host.py index 05edfa5..88e80a4 100644 --- a/tests/charmhelpers/core/host.py +++ b/tests/charmhelpers/core/host.py @@ -45,6 +45,7 @@ if __platform__ == "ubuntu": add_new_group, lsb_release, cmp_pkgrevno, + CompareHostReleases, ) # flake8: noqa -- ignore F401 for this import elif __platform__ == "centos": from charmhelpers.core.host_factory.centos import ( @@ -52,6 +53,7 @@ elif __platform__ == "centos": add_new_group, lsb_release, cmp_pkgrevno, + CompareHostReleases, ) # flake8: noqa -- ignore F401 for this import UPDATEDB_PATH = '/etc/updatedb.conf' @@ -189,7 +191,7 @@ def service_pause(service_name, init_dir="/etc/init", initd_dir="/etc/init.d", upstart_file = os.path.join(init_dir, "{}.conf".format(service_name)) sysv_file = os.path.join(initd_dir, service_name) if init_is_systemd(): - service('disable', service_name) + service('mask', service_name) elif os.path.exists(upstart_file): override_path = os.path.join( init_dir, '{}.override'.format(service_name)) @@ -222,7 +224,7 @@ def service_resume(service_name, init_dir="/etc/init", upstart_file = os.path.join(init_dir, "{}.conf".format(service_name)) sysv_file = os.path.join(initd_dir, service_name) if init_is_systemd(): - service('enable', service_name) + service('unmask', service_name) elif os.path.exists(upstart_file): override_path = os.path.join( init_dir, '{}.override'.format(service_name)) diff --git a/tests/charmhelpers/core/host_factory/centos.py b/tests/charmhelpers/core/host_factory/centos.py index 902d469..7781a39 100644 --- a/tests/charmhelpers/core/host_factory/centos.py +++ b/tests/charmhelpers/core/host_factory/centos.py @@ -2,6 +2,22 @@ import subprocess import yum import os +from charmhelpers.core.strutils import BasicStringComparator + + +class CompareHostReleases(BasicStringComparator): + """Provide comparisons of Host releases. + + Use in the form of + + if CompareHostReleases(release) > 'trusty': + # do something with mitaka + """ + + def __init__(self, item): + raise NotImplementedError( + "CompareHostReleases() is not implemented for CentOS") + def service_available(service_name): # """Determine whether a system service is available.""" diff --git a/tests/charmhelpers/core/host_factory/ubuntu.py b/tests/charmhelpers/core/host_factory/ubuntu.py index 8c66af5..0448288 100644 --- a/tests/charmhelpers/core/host_factory/ubuntu.py +++ b/tests/charmhelpers/core/host_factory/ubuntu.py @@ -1,5 +1,37 @@ import subprocess +from charmhelpers.core.strutils import BasicStringComparator + + +UBUNTU_RELEASES = ( + 'lucid', + 'maverick', + 'natty', + 'oneiric', + 'precise', + 'quantal', + 'raring', + 'saucy', + 'trusty', + 'utopic', + 'vivid', + 'wily', + 'xenial', + 'yakkety', + 'zesty', +) + + +class CompareHostReleases(BasicStringComparator): + """Provide comparisons of Ubuntu releases. + + Use in the form of + + if CompareHostReleases(release) > 'trusty': + # do something with mitaka + """ + _list = UBUNTU_RELEASES + def service_available(service_name): """Determine whether a system service is available""" diff --git a/tests/charmhelpers/core/strutils.py b/tests/charmhelpers/core/strutils.py index dd9b971..685dabd 100644 --- a/tests/charmhelpers/core/strutils.py +++ b/tests/charmhelpers/core/strutils.py @@ -68,3 +68,56 @@ def bytes_from_string(value): msg = "Unable to interpret string value '%s' as bytes" % (value) raise ValueError(msg) return int(matches.group(1)) * (1024 ** BYTE_POWER[matches.group(2)]) + + +class BasicStringComparator(object): + """Provides a class that will compare strings from an iterator type object. + Used to provide > and < comparisons on strings that may not necessarily be + alphanumerically ordered. e.g. OpenStack or Ubuntu releases AFTER the + z-wrap. + """ + + _list = None + + def __init__(self, item): + if self._list is None: + raise Exception("Must define the _list in the class definition!") + try: + self.index = self._list.index(item) + except Exception: + raise KeyError("Item '{}' is not in list '{}'" + .format(item, self._list)) + + def __eq__(self, other): + assert isinstance(other, str) or isinstance(other, self.__class__) + return self.index == self._list.index(other) + + def __ne__(self, other): + return not self.__eq__(other) + + def __lt__(self, other): + assert isinstance(other, str) or isinstance(other, self.__class__) + return self.index < self._list.index(other) + + def __ge__(self, other): + return not self.__lt__(other) + + def __gt__(self, other): + assert isinstance(other, str) or isinstance(other, self.__class__) + return self.index > self._list.index(other) + + def __le__(self, other): + return not self.__gt__(other) + + def __str__(self): + """Always give back the item at the index so it can be used in + comparisons like: + + s_mitaka = CompareOpenStack('mitaka') + s_newton = CompareOpenstack('newton') + + assert s_newton > s_mitaka + + @returns: + """ + return self._list[self.index] diff --git a/tests/charmhelpers/osplatform.py b/tests/charmhelpers/osplatform.py new file mode 100644 index 0000000..d9a4d5c --- /dev/null +++ b/tests/charmhelpers/osplatform.py @@ -0,0 +1,25 @@ +import platform + + +def get_platform(): + """Return the current OS platform. + + For example: if current os platform is Ubuntu then a string "ubuntu" + will be returned (which is the name of the module). + This string is used to decide which platform module should be imported. + """ + # linux_distribution is deprecated and will be removed in Python 3.7 + # Warings *not* disabled, as we certainly need to fix this. + tuple_platform = platform.linux_distribution() + current_platform = tuple_platform[0] + if "Ubuntu" in current_platform: + return "ubuntu" + elif "CentOS" in current_platform: + return "centos" + elif "debian" in current_platform: + # Stock Python does not detect Ubuntu and instead returns debian. + # Or at least it does in some build environments like Travis CI + return "ubuntu" + else: + raise RuntimeError("This module is not supported on {}." + .format(current_platform)) diff --git a/tox.ini b/tox.ini index 6f1aeac..7c2936e 100644 --- a/tox.ini +++ b/tox.ini @@ -14,7 +14,7 @@ install_command = pip install --allow-unverified python-apt {opts} {packages} commands = ostestr {posargs} whitelist_externals = juju -passenv = HOME TERM AMULET_* CS_API_URL +passenv = HOME TERM AMULET_* CS_API_* [testenv:py27] basepython = python2.7