From 536da1756a18bc555da0ad493827a828457b523a Mon Sep 17 00:00:00 2001 From: James Page Date: Mon, 11 Apr 2016 08:46:24 +0100 Subject: [PATCH] Add Juju network spaces support Add the 'data' extra-binding to support binding of Open vSwitch endpoints to a specific Juju network space. Change-Id: Iff6567c2c9b353d729cc9b73d0523d6f72946d98 --- .../contrib/network/ovs/__init__.py | 8 +- .../charmhelpers/contrib/openstack/context.py | 92 ++++++++++++++++++- hooks/charmhelpers/contrib/openstack/utils.py | 7 +- hooks/charmhelpers/core/host.py | 23 +++-- hooks/neutron_contexts.py | 21 ++++- metadata.yaml | 2 + tests/charmhelpers/contrib/amulet/utils.py | 26 ++++-- unit_tests/test_neutron_contexts.py | 60 +++++++++++- 8 files changed, 211 insertions(+), 28 deletions(-) diff --git a/hooks/charmhelpers/contrib/network/ovs/__init__.py b/hooks/charmhelpers/contrib/network/ovs/__init__.py index 77e2db7f..83d18134 100644 --- a/hooks/charmhelpers/contrib/network/ovs/__init__.py +++ b/hooks/charmhelpers/contrib/network/ovs/__init__.py @@ -25,10 +25,14 @@ from charmhelpers.core.host import ( ) -def add_bridge(name): +def add_bridge(name, datapath_type=None): ''' Add the named bridge to openvswitch ''' log('Creating bridge {}'.format(name)) - subprocess.check_call(["ovs-vsctl", "--", "--may-exist", "add-br", name]) + cmd = ["ovs-vsctl", "--", "--may-exist", "add-br", name] + if datapath_type is not None: + cmd += ['--', 'set', 'bridge', name, + 'datapath_type={}'.format(datapath_type)] + subprocess.check_call(cmd) def del_bridge(name): diff --git a/hooks/charmhelpers/contrib/openstack/context.py b/hooks/charmhelpers/contrib/openstack/context.py index d495da3f..c07b33dd 100644 --- a/hooks/charmhelpers/contrib/openstack/context.py +++ b/hooks/charmhelpers/contrib/openstack/context.py @@ -20,7 +20,7 @@ import os import re import time from base64 import b64decode -from subprocess import check_call +from subprocess import check_call, CalledProcessError import six import yaml @@ -45,6 +45,7 @@ from charmhelpers.core.hookenv import ( INFO, WARNING, ERROR, + status_set, ) from charmhelpers.core.sysctl import create as sysctl_create @@ -1491,3 +1492,92 @@ class InternalEndpointContext(OSContextGenerator): """ def __call__(self): return {'use_internal_endpoints': config('use-internal-endpoints')} + + +class AppArmorContext(OSContextGenerator): + """Base class for apparmor contexts.""" + + def __init__(self): + self._ctxt = None + self.aa_profile = None + self.aa_utils_packages = ['apparmor-utils'] + + @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): + """ + Validate aa-profile-mode settings is disable, enforce, or complain. + + :return ctxt: Dictionary of the apparmor profile or None + """ + if config('aa-profile-mode') in ['disable', 'enforce', 'complain']: + ctxt = {'aa-profile-mode': config('aa-profile-mode')} + else: + ctxt = None + return ctxt + + def __call__(self): + return self.ctxt + + def install_aa_utils(self): + """ + Install packages required for apparmor configuration. + """ + log("Installing apparmor utils.") + ensure_packages(self.aa_utils_packages) + + def manually_disable_aa_profile(self): + """ + Manually disable an apparmor profile. + + If aa-profile-mode is set to disabled (default) this is required as the + template has been written but apparmor is yet unaware of the profile + and aa-disable aa-profile fails. Without this the profile would kick + into enforce mode on the next service restart. + + """ + profile_path = '/etc/apparmor.d' + disable_path = '/etc/apparmor.d/disable' + if not os.path.lexists(os.path.join(disable_path, self.aa_profile)): + os.symlink(os.path.join(profile_path, self.aa_profile), + os.path.join(disable_path, self.aa_profile)) + + def setup_aa_profile(self): + """ + Setup an apparmor profile. + The ctxt dictionary will contain the apparmor profile mode and + the apparmor profile name. + Makes calls out to aa-disable, aa-complain, or aa-enforce to setup + the apparmor profile. + """ + self() + if not self.ctxt: + log("Not enabling apparmor Profile") + return + self.install_aa_utils() + cmd = ['aa-{}'.format(self.ctxt['aa-profile-mode'])] + cmd.append(self.ctxt['aa-profile']) + log("Setting up the apparmor profile for {} in {} mode." + "".format(self.ctxt['aa-profile'], self.ctxt['aa-profile-mode'])) + try: + check_call(cmd) + except CalledProcessError as e: + # If aa-profile-mode is set to disabled (default) manual + # disabling is required as the template has been written but + # apparmor is yet unaware of the profile and aa-disable aa-profile + # fails. If aa-disable learns to read profile files first this can + # be removed. + if self.ctxt['aa-profile-mode'] == 'disable': + log("Manually disabling the apparmor profile for {}." + "".format(self.ctxt['aa-profile'])) + self.manually_disable_aa_profile() + return + status_set('blocked', "Apparmor profile {} failed to be set to {}." + "".format(self.ctxt['aa-profile'], + self.ctxt['aa-profile-mode'])) + raise e diff --git a/hooks/charmhelpers/contrib/openstack/utils.py b/hooks/charmhelpers/contrib/openstack/utils.py index 3fb67b10..0dba98ff 100644 --- a/hooks/charmhelpers/contrib/openstack/utils.py +++ b/hooks/charmhelpers/contrib/openstack/utils.py @@ -156,6 +156,7 @@ PACKAGE_CODENAMES = { ]), 'keystone': OrderedDict([ ('8.0', 'liberty'), + ('8.1', 'liberty'), ('9.0', 'mitaka'), ]), 'horizon-common': OrderedDict([ @@ -1534,7 +1535,8 @@ def make_assess_status_func(*args, **kwargs): return _assess_status_func -def pausable_restart_on_change(restart_map, stopstart=False): +def pausable_restart_on_change(restart_map, stopstart=False, + restart_functions=None): """A restart_on_change decorator that checks to see if the unit is paused. If it is paused then the decorated function doesn't fire. @@ -1567,6 +1569,7 @@ def pausable_restart_on_change(restart_map, stopstart=False): return f(*args, **kwargs) # otherwise, normal restart_on_change functionality return restart_on_change_helper( - (lambda: f(*args, **kwargs)), restart_map, stopstart) + (lambda: f(*args, **kwargs)), restart_map, stopstart, + restart_functions) return wrapped_f return wrap diff --git a/hooks/charmhelpers/core/host.py b/hooks/charmhelpers/core/host.py index d44aecd2..bfea6a15 100644 --- a/hooks/charmhelpers/core/host.py +++ b/hooks/charmhelpers/core/host.py @@ -423,7 +423,7 @@ class ChecksumError(ValueError): pass -def restart_on_change(restart_map, stopstart=False): +def restart_on_change(restart_map, stopstart=False, restart_functions=None): """Restart services based on configuration files changing This function is used a decorator, for example:: @@ -444,18 +444,22 @@ def restart_on_change(restart_map, stopstart=False): @param restart_map: {path_file_name: [service_name, ...] @param stopstart: DEFAULT false; whether to stop, start OR restart + @param restart_functions: nonstandard functions to use to restart services + {svc: func, ...} @returns result from decorated function """ def wrap(f): @functools.wraps(f) def wrapped_f(*args, **kwargs): return restart_on_change_helper( - (lambda: f(*args, **kwargs)), restart_map, stopstart) + (lambda: f(*args, **kwargs)), restart_map, stopstart, + restart_functions) return wrapped_f return wrap -def restart_on_change_helper(lambda_f, restart_map, stopstart=False): +def restart_on_change_helper(lambda_f, restart_map, stopstart=False, + restart_functions=None): """Helper function to perform the restart_on_change function. This is provided for decorators to restart services if files described @@ -464,8 +468,12 @@ def restart_on_change_helper(lambda_f, restart_map, stopstart=False): @param lambda_f: function to call. @param restart_map: {file: [service, ...]} @param stopstart: whether to stop, start or restart a service + @param restart_functions: nonstandard functions to use to restart services + {svc: func, ...} @returns result of lambda_f() """ + if restart_functions is None: + restart_functions = {} checksums = {path: path_hash(path) for path in restart_map} r = lambda_f() # create a list of lists of the services to restart @@ -476,9 +484,12 @@ def restart_on_change_helper(lambda_f, restart_map, stopstart=False): services_list = list(OrderedDict.fromkeys(itertools.chain(*restarts))) if services_list: actions = ('stop', 'start') if stopstart else ('restart',) - for action in actions: - for service_name in services_list: - service(action, service_name) + for service_name in services_list: + if service_name in restart_functions: + restart_functions[service_name](service_name) + else: + for action in actions: + service(action, service_name) return r diff --git a/hooks/neutron_contexts.py b/hooks/neutron_contexts.py index de3f3c1d..9222c67f 100644 --- a/hooks/neutron_contexts.py +++ b/hooks/neutron_contexts.py @@ -5,7 +5,8 @@ import socket from charmhelpers.core.hookenv import ( config, unit_get, - cached + cached, + network_get_primary_address, ) from charmhelpers.fetch import ( apt_install, @@ -78,9 +79,6 @@ class NeutronGatewayContext(NeutronAPIContext): api_settings = super(NeutronGatewayContext, self).__call__() ctxt = { 'shared_secret': get_shared_secret(), - 'local_ip': - get_address_in_network(config('os-data-network'), - get_host_ip(unit_get('private-address'))), 'core_plugin': core_plugin(), 'plugin': config('plugin'), 'debug': config('debug'), @@ -93,6 +91,21 @@ class NeutronGatewayContext(NeutronAPIContext): api_settings['overlay_network_type'], } + fallback = get_host_ip(unit_get('private-address')) + if config('os-data-network'): + # NOTE: prefer any existing use of config based networking + ctxt['local_ip'] = \ + get_address_in_network(config('os-data-network'), + fallback) + else: + # NOTE: test out network-spaces support, then fallback + try: + ctxt['local_ip'] = get_host_ip( + network_get_primary_address('data') + ) + except NotImplementedError: + ctxt['local_ip'] = fallback + mappings = config('bridge-mappings') if mappings: ctxt['bridge_mappings'] = ','.join(mappings.split()) diff --git a/metadata.yaml b/metadata.yaml index 50c57c8a..d2103cc4 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -15,6 +15,8 @@ description: | of a Neutron based OpenStack deployment tags: - openstack +extra-bindings: + data: provides: nrpe-external-master: interface: nrpe-external-master diff --git a/tests/charmhelpers/contrib/amulet/utils.py b/tests/charmhelpers/contrib/amulet/utils.py index 3e159039..7e5c25a9 100644 --- a/tests/charmhelpers/contrib/amulet/utils.py +++ b/tests/charmhelpers/contrib/amulet/utils.py @@ -601,7 +601,7 @@ class AmuletUtils(object): return ('Process name count mismatch. expected, actual: {}, ' '{}'.format(len(expected), len(actual))) - for (e_proc_name, e_pids_length), (a_proc_name, a_pids) in \ + for (e_proc_name, e_pids), (a_proc_name, a_pids) in \ zip(e_proc_names.items(), a_proc_names.items()): if e_proc_name != a_proc_name: return ('Process name mismatch. expected, actual: {}, ' @@ -610,25 +610,31 @@ class AmuletUtils(object): a_pids_length = len(a_pids) fail_msg = ('PID count mismatch. {} ({}) expected, actual: ' '{}, {} ({})'.format(e_sentry_name, e_proc_name, - e_pids_length, a_pids_length, + e_pids, a_pids_length, a_pids)) - # If expected is not bool, ensure PID quantities match - if not isinstance(e_pids_length, bool) and \ - a_pids_length != e_pids_length: + # If expected is a list, ensure at least one PID quantity match + if isinstance(e_pids, list) and \ + a_pids_length not in e_pids: + return fail_msg + # If expected is not bool and not list, + # ensure PID quantities match + elif not isinstance(e_pids, bool) and \ + not isinstance(e_pids, list) and \ + a_pids_length != e_pids: return fail_msg # If expected is bool True, ensure 1 or more PIDs exist - elif isinstance(e_pids_length, bool) and \ - e_pids_length is True and a_pids_length < 1: + elif isinstance(e_pids, bool) and \ + e_pids is True and a_pids_length < 1: return fail_msg # If expected is bool False, ensure 0 PIDs exist - elif isinstance(e_pids_length, bool) and \ - e_pids_length is False and a_pids_length != 0: + elif isinstance(e_pids, bool) and \ + e_pids is False and a_pids_length != 0: return fail_msg else: self.log.debug('PID check OK: {} {} {}: ' '{}'.format(e_sentry_name, e_proc_name, - e_pids_length, a_pids)) + e_pids, a_pids)) return None def validate_list_of_identical_dicts(self, list_of_dicts): diff --git a/unit_tests/test_neutron_contexts.py b/unit_tests/test_neutron_contexts.py index 93e5fa9b..625b2f25 100644 --- a/unit_tests/test_neutron_contexts.py +++ b/unit_tests/test_neutron_contexts.py @@ -16,6 +16,7 @@ TO_PATCH = [ 'config', 'eligible_leader', 'unit_get', + 'network_get_primary_address', ] @@ -51,6 +52,7 @@ class TestL3AgentContext(CharmTestCase): def setUp(self): super(TestL3AgentContext, self).setUp(neutron_contexts, TO_PATCH) + self.network_get_primary_address.side_effect = NotImplementedError self.config.side_effect = self.test_config.get @patch('neutron_contexts.NeutronAPIContext') @@ -111,8 +113,7 @@ class TestNeutronGatewayContext(CharmTestCase): @patch('charmhelpers.contrib.openstack.context.related_units') @patch('charmhelpers.contrib.openstack.context.relation_ids') @patch.object(neutron_contexts, 'get_shared_secret') - @patch.object(neutron_contexts, 'get_host_ip') - def test_all(self, _host_ip, _secret, _rids, _runits, _rget): + def test_all(self, _secret, _rids, _runits, _rget): rdata = {'l2-population': 'True', 'enable-dvr': 'True', 'overlay-network-type': 'gre', @@ -127,11 +128,12 @@ class TestNeutronGatewayContext(CharmTestCase): self.test_config.set('vlan-ranges', 'physnet1:1000:2000 physnet2:2001:3000') self.test_config.set('flat-network-providers', 'physnet3 physnet4') + self.network_get_primary_address.side_effect = NotImplementedError + self.unit_get.return_value = '10.5.0.1' # Provided by neutron-api relation _rids.return_value = ['neutron-plugin-api:0'] _runits.return_value = ['neutron-api/0'] _rget.side_effect = lambda *args, **kwargs: rdata - _host_ip.return_value = '10.5.0.1' _secret.return_value = 'testsecret' ctxt = neutron_contexts.NeutronGatewayContext()() self.assertEquals(ctxt, { @@ -157,6 +159,56 @@ class TestNeutronGatewayContext(CharmTestCase): } }) + @patch('charmhelpers.contrib.openstack.context.relation_get') + @patch('charmhelpers.contrib.openstack.context.related_units') + @patch('charmhelpers.contrib.openstack.context.relation_ids') + @patch.object(neutron_contexts, 'get_shared_secret') + def test_all_network_spaces(self, _secret, _rids, _runits, _rget): + rdata = {'l2-population': 'True', + 'enable-dvr': 'True', + 'overlay-network-type': 'gre', + 'enable-l3ha': 'True', + 'network-device-mtu': 9000} + self.test_config.set('plugin', 'ovs') + self.test_config.set('debug', False) + self.test_config.set('verbose', True) + self.test_config.set('instance-mtu', 1420) + self.test_config.set('dnsmasq-flags', 'dhcp-userclass=set:ipxe,iPXE,' + 'dhcp-match=set:ipxe,175') + self.test_config.set('vlan-ranges', + 'physnet1:1000:2000 physnet2:2001:3000') + self.test_config.set('flat-network-providers', 'physnet3 physnet4') + self.network_get_primary_address.return_value = '192.168.20.2' + self.unit_get.return_value = '10.5.0.1' + # Provided by neutron-api relation + _rids.return_value = ['neutron-plugin-api:0'] + _runits.return_value = ['neutron-api/0'] + _rget.side_effect = lambda *args, **kwargs: rdata + _secret.return_value = 'testsecret' + ctxt = neutron_contexts.NeutronGatewayContext()() + self.assertEquals(ctxt, { + 'shared_secret': 'testsecret', + 'enable_dvr': True, + 'enable_l3ha': True, + 'local_ip': '192.168.20.2', + 'instance_mtu': 1420, + 'core_plugin': "ml2", + 'plugin': 'ovs', + 'debug': False, + 'verbose': True, + 'l2_population': True, + 'overlay_network_type': 'gre', + 'bridge_mappings': 'physnet1:br-data', + 'network_providers': 'physnet3,physnet4', + 'vlan_ranges': 'physnet1:1000:2000,physnet2:2001:3000', + 'network_device_mtu': 9000, + 'veth_mtu': 9000, + 'dnsmasq_flags': { + 'dhcp-userclass': 'set:ipxe,iPXE', + 'dhcp-match': 'set:ipxe,175' + } + }) + class TestSharedSecret(CharmTestCase): @@ -164,6 +216,7 @@ class TestSharedSecret(CharmTestCase): super(TestSharedSecret, self).setUp(neutron_contexts, TO_PATCH) self.config.side_effect = self.test_config.get + self.network_get_primary_address.side_effect = NotImplementedError @patch('os.path') @patch('uuid.uuid4') @@ -194,6 +247,7 @@ class TestHostIP(CharmTestCase): super(TestHostIP, self).setUp(neutron_contexts, TO_PATCH) self.config.side_effect = self.test_config.get + self.network_get_primary_address.side_effect = NotImplementedError # Save and inject self.mods = {'dns': None, 'dns.resolver': None} for mod in self.mods: