From 7213b596e50f39492f61b4a475a4520ebed4aff5 Mon Sep 17 00:00:00 2001 From: Edward Hope-Morley Date: Wed, 13 May 2015 12:20:38 +0100 Subject: [PATCH 1/2] [hopem,r=] Fixes network-vlan-ranges parsing Partially-Closes-Bug: 1451095 --- config.yaml | 6 ++++-- hooks/charmhelpers/contrib/openstack/neutron.py | 15 ++++++++++----- unit_tests/test_neutron_utils.py | 3 ++- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/config.yaml b/config.yaml index 36ff290b..7e757614 100644 --- a/config.yaml +++ b/config.yaml @@ -153,8 +153,10 @@ options: type: string default: "physnet1:1000:2000" description: | - Space-delimited list of Neutron network-provider & vlan-id-ranges using - format ":: ...". + Space-delimited list of :: or + specifying physical_network names usable for VLAN + provider and tenant networks, as well as ranges of VLAN tags on each + available for allocation to tenant networks. # Network configuration options # by default all access is over 'private-address' os-data-network: diff --git a/hooks/charmhelpers/contrib/openstack/neutron.py b/hooks/charmhelpers/contrib/openstack/neutron.py index 02c92e9c..b3f4e2b7 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 _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/unit_tests/test_neutron_utils.py b/unit_tests/test_neutron_utils.py index befa9eb3..96254bcd 100644 --- a/unit_tests/test_neutron_utils.py +++ b/unit_tests/test_neutron_utils.py @@ -930,7 +930,8 @@ class TestQuantumAgentReallocation(CharmTestCase): 'process_name': 'neutron-nvsd-agent', 'executable_name': '/usr/local/bin/neutron-nvsd-agent', 'config_files': ['/etc/neutron/neutron.conf', - '/etc/neutron/plugins/oneconvergence/nvsdplugin.ini'], + '/etc/neutron/plugins/oneconvergence/' + 'nvsdplugin.ini'], 'log_file': '/var/log/neutron/nvsd-agent.log', } neutron_plugin_openflow_context = { From a13d92c75c16795be5080d30ee5b1e2559ef49b8 Mon Sep 17 00:00:00 2001 From: Edward Hope-Morley Date: Wed, 20 May 2015 12:44:05 -0700 Subject: [PATCH 2/2] sync --- .../charmhelpers/contrib/hahelpers/cluster.py | 25 +++++ .../charmhelpers/contrib/openstack/neutron.py | 2 +- hooks/charmhelpers/core/hookenv.py | 96 +++++++++++++++++-- hooks/charmhelpers/core/host.py | 2 +- hooks/charmhelpers/core/services/base.py | 4 +- hooks/charmhelpers/fetch/__init__.py | 2 +- 6 files changed, 116 insertions(+), 15 deletions(-) diff --git a/hooks/charmhelpers/contrib/hahelpers/cluster.py b/hooks/charmhelpers/contrib/hahelpers/cluster.py index 9333efc3..c555d7aa 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/neutron.py b/hooks/charmhelpers/contrib/openstack/neutron.py index b3f4e2b7..b3aa3d4c 100644 --- a/hooks/charmhelpers/contrib/openstack/neutron.py +++ b/hooks/charmhelpers/contrib/openstack/neutron.py @@ -286,7 +286,7 @@ def parse_data_port_mappings(mappings, default_bridge='br-data'): Returns dict of the form {bridge:port}. """ _mappings = parse_mappings(mappings) - if not _mappings or _mappings.values() == ['']: + if not _mappings or list(_mappings.values()) == ['']: if not mappings: return {} diff --git a/hooks/charmhelpers/core/hookenv.py b/hooks/charmhelpers/core/hookenv.py index 86f805f1..411d0f2d 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,14 +363,29 @@ 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"]) 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) + 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()) @@ -509,6 +534,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 +635,49 @@ 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 diff --git a/hooks/charmhelpers/core/host.py b/hooks/charmhelpers/core/host.py index 830822af..0d2ab4b4 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 c5534e4c..2ff53381 100644 --- a/hooks/charmhelpers/core/services/base.py +++ b/hooks/charmhelpers/core/services/base.py @@ -17,7 +17,7 @@ import os import re import json -from collections import Iterable +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 diff --git a/hooks/charmhelpers/fetch/__init__.py b/hooks/charmhelpers/fetch/__init__.py index 792e629a..9a1a2515 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", "")