diff --git a/README.md b/README.md index 0f33557b..2b67e940 100644 --- a/README.md +++ b/README.md @@ -170,4 +170,3 @@ configured later using neutron net-create). This replaces the previous system of using ext-port, which always created a bridge called br-ex for external networks which was used implicitly by external router interfaces. - diff --git a/hooks/charmhelpers/contrib/network/ovs/__init__.py b/hooks/charmhelpers/contrib/network/ovs/__init__.py index 3004125a..e1a3cc57 100644 --- a/hooks/charmhelpers/contrib/network/ovs/__init__.py +++ b/hooks/charmhelpers/contrib/network/ovs/__init__.py @@ -25,7 +25,7 @@ from charmhelpers.contrib.network.ovs import ovsdb as ch_ovsdb from charmhelpers.fetch import apt_install from charmhelpers.core.hookenv import ( - log, WARNING, INFO, DEBUG + log, WARNING, INFO, DEBUG, charm_name ) from charmhelpers.core.host import ( CompareHostReleases, @@ -666,3 +666,28 @@ def patch_ports_on_bridge(bridge): # reference to PEP479 just doing a return will provide a emtpy iterator # and not None. return + + +def generate_external_ids(external_id_value=None): + """Generate external-ids dictionary that can be used to mark OVS bridges + and ports as managed by the charm. + + :param external_id_value: Value of the external-ids entry. + Note: 'managed' will be used if not specified. + :type external_id_value: Optional[str] + :returns: Dict with a single external-ids entry. + { + 'external-ids': { + charm-``charm_name``: ``external_id_value`` + } + } + :rtype: Dict[str, Dict[str]] + """ + external_id_key = "charm-{}".format(charm_name()) + external_id_value = ('managed' if external_id_value is None + else external_id_value) + return { + 'external-ids': { + external_id_key: external_id_value + } + } diff --git a/hooks/charmhelpers/contrib/openstack/amulet/utils.py b/hooks/charmhelpers/contrib/openstack/amulet/utils.py index 63aea1e3..0a14af7e 100644 --- a/hooks/charmhelpers/contrib/openstack/amulet/utils.py +++ b/hooks/charmhelpers/contrib/openstack/amulet/utils.py @@ -42,6 +42,7 @@ import pika import swiftclient from charmhelpers.core.decorators import retry_on_exception + from charmhelpers.contrib.amulet.utils import ( AmuletUtils ) diff --git a/hooks/charmhelpers/contrib/openstack/cert_utils.py b/hooks/charmhelpers/contrib/openstack/cert_utils.py index 24867497..703fc6ef 100644 --- a/hooks/charmhelpers/contrib/openstack/cert_utils.py +++ b/hooks/charmhelpers/contrib/openstack/cert_utils.py @@ -47,7 +47,7 @@ from charmhelpers.contrib.network.ip import ( ) from charmhelpers.core.host import ( - CA_CERT_DIR, + ca_cert_absolute_path, install_ca_cert, mkdir, write_file, @@ -307,6 +307,26 @@ def install_certs(ssl_dir, certs, chain=None, user='root', group='root'): content=bundle['key'], perms=0o640) +def get_cert_relation_ca_name(cert_relation_id=None): + """Determine CA certificate name as provided by relation. + + The filename on disk depends on the name chosen for the application on the + providing end of the certificates relation. + + :param cert_relation_id: (Optional) Relation id providing the certs + :type cert_relation_id: str + :returns: CA certificate filename without path nor extension + :rtype: str + """ + if cert_relation_id is None: + try: + cert_relation_id = relation_ids('certificates')[0] + except IndexError: + return '' + return '{}_juju_ca_cert'.format( + remote_service_name(relid=cert_relation_id)) + + def _manage_ca_certs(ca, cert_relation_id): """Manage CA certs. @@ -316,7 +336,7 @@ def _manage_ca_certs(ca, cert_relation_id): :type cert_relation_id: str """ config_ssl_ca = config('ssl_ca') - config_cert_file = '{}/{}.crt'.format(CA_CERT_DIR, CONFIG_CA_CERT_FILE) + config_cert_file = ca_cert_absolute_path(CONFIG_CA_CERT_FILE) if config_ssl_ca: log("Installing CA certificate from charm ssl_ca config to {}".format( config_cert_file), INFO) @@ -329,8 +349,7 @@ def _manage_ca_certs(ca, cert_relation_id): log("Installing CA certificate from certificate relation", INFO) install_ca_cert( ca.encode(), - name='{}_juju_ca_cert'.format( - remote_service_name(relid=cert_relation_id))) + name=get_cert_relation_ca_name(cert_relation_id)) def process_certificates(service_name, relation_id, unit, diff --git a/hooks/charmhelpers/contrib/openstack/context.py b/hooks/charmhelpers/contrib/openstack/context.py index c242d18d..b67dafda 100644 --- a/hooks/charmhelpers/contrib/openstack/context.py +++ b/hooks/charmhelpers/contrib/openstack/context.py @@ -74,7 +74,6 @@ from charmhelpers.core.host import ( pwgen, lsb_release, CompareHostReleases, - is_container, ) from charmhelpers.contrib.hahelpers.cluster import ( determine_apache_port, @@ -1596,16 +1595,21 @@ def _calculate_workers(): @returns int: number of worker processes to use ''' - multiplier = config('worker-multiplier') or DEFAULT_MULTIPLIER + multiplier = config('worker-multiplier') + + # distinguish an empty config and an explicit config as 0.0 + if multiplier is None: + multiplier = DEFAULT_MULTIPLIER + count = int(_num_cpus() * multiplier) - if multiplier > 0 and count == 0: + if count <= 0: + # assign at least one worker count = 1 - if config('worker-multiplier') is None and is_container(): + if config('worker-multiplier') is None: # NOTE(jamespage): Limit unconfigured worker-multiplier # to MAX_DEFAULT_WORKERS to avoid insane - # worker configuration in LXD containers - # on large servers + # worker configuration on large servers # Reference: https://pad.lv/1665270 count = min(count, MAX_DEFAULT_WORKERS) diff --git a/hooks/charmhelpers/contrib/openstack/deferred_events.py b/hooks/charmhelpers/contrib/openstack/deferred_events.py new file mode 100644 index 00000000..433eb936 --- /dev/null +++ b/hooks/charmhelpers/contrib/openstack/deferred_events.py @@ -0,0 +1,363 @@ +# Copyright 2021 Canonical Limited. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Module for managing deferred service events. + +This module is used to manage deferred service events from both charm actions +and package actions. +""" + +import datetime +import glob +import yaml +import os +import time +import uuid + +import charmhelpers.contrib.openstack.policy_rcd as policy_rcd +import charmhelpers.core.hookenv as hookenv +import charmhelpers.core.host as host + +import subprocess + + +# Deferred events generated from the charm are stored along side those +# generated from packaging. +DEFERRED_EVENTS_DIR = policy_rcd.POLICY_DEFERRED_EVENTS_DIR + + +class ServiceEvent(): + + def __init__(self, timestamp, service, reason, action, + policy_requestor_name=None, policy_requestor_type=None): + self.timestamp = timestamp + self.service = service + self.reason = reason + self.action = action + if not policy_requestor_name: + self.policy_requestor_name = hookenv.service_name() + if not policy_requestor_type: + self.policy_requestor_type = 'charm' + + def __eq__(self, other): + for attr in vars(self): + if getattr(self, attr) != getattr(other, attr): + return False + return True + + def matching_request(self, other): + for attr in ['service', 'action', 'reason']: + if getattr(self, attr) != getattr(other, attr): + return False + return True + + @classmethod + def from_dict(cls, data): + return cls( + data['timestamp'], + data['service'], + data['reason'], + data['action'], + data.get('policy_requestor_name'), + data.get('policy_requestor_type')) + + +def deferred_events_files(): + """Deferred event files + + Deferred event files that were generated by service_name() policy. + + :returns: Deferred event files + :rtype: List[str] + """ + return glob.glob('{}/*.deferred'.format(DEFERRED_EVENTS_DIR)) + + +def read_event_file(file_name): + """Read a file and return the corresponding objects. + + :param file_name: Name of file to read. + :type file_name: str + :returns: ServiceEvent from file. + :rtype: ServiceEvent + """ + with open(file_name, 'r') as f: + contents = yaml.safe_load(f) + event = ServiceEvent( + contents['timestamp'], + contents['service'], + contents['reason'], + contents['action']) + return event + + +def deferred_events(): + """Get list of deferred events. + + List of deferred events. Events are represented by dicts of the form: + + { + action: restart, + policy_requestor_name: neutron-openvswitch, + policy_requestor_type: charm, + reason: 'Pkg update', + service: openvswitch-switch, + time: 1614328743} + + :returns: List of deferred events. + :rtype: List[ServiceEvent] + """ + events = [] + for defer_file in deferred_events_files(): + events.append((defer_file, read_event_file(defer_file))) + return events + + +def duplicate_event_files(event): + """Get list of event files that have equivalent deferred events. + + :param event: Event to compare + :type event: ServiceEvent + :returns: List of event files + :rtype: List[str] + """ + duplicates = [] + for event_file, existing_event in deferred_events(): + if event.matching_request(existing_event): + duplicates.append(event_file) + return duplicates + + +def get_event_record_file(policy_requestor_type, policy_requestor_name): + """Generate filename for storing a new event. + + :param policy_requestor_type: System that blocked event + :type policy_requestor_type: str + :param policy_requestor_name: Name of application that blocked event + :type policy_requestor_name: str + :returns: File name + :rtype: str + """ + file_name = '{}/{}-{}-{}.deferred'.format( + DEFERRED_EVENTS_DIR, + policy_requestor_type, + policy_requestor_name, + uuid.uuid1()) + return file_name + + +def save_event(event): + """Write deferred events to backend. + + :param event: Event to save + :type event: ServiceEvent + """ + requestor_name = hookenv.service_name() + requestor_type = 'charm' + init_policy_log_dir() + if duplicate_event_files(event): + hookenv.log( + "Not writing new event, existing event found. {} {} {}".format( + event.service, + event.action, + event.reason), + level="DEBUG") + else: + record_file = get_event_record_file( + policy_requestor_type=requestor_type, + policy_requestor_name=requestor_name) + + with open(record_file, 'w') as f: + data = { + 'timestamp': event.timestamp, + 'service': event.service, + 'action': event.action, + 'reason': event.reason, + 'policy_requestor_type': requestor_type, + 'policy_requestor_name': requestor_name} + yaml.dump(data, f) + + +def clear_deferred_events(svcs, action): + """Remove any outstanding deferred events. + + Remove a deferred event if its service is in the services list and its + action matches. + + :param svcs: List of services to remove. + :type svcs: List[str] + :param action: Action to remove + :type action: str + """ + # XXX This function is not currently processing the action. It needs to + # match the action and also take account of try-restart and the + # equivalnce of stop-start and restart. + for defer_file in deferred_events_files(): + deferred_event = read_event_file(defer_file) + if deferred_event.service in svcs: + os.remove(defer_file) + + +def init_policy_log_dir(): + """Ensure directory to store events exists.""" + if not os.path.exists(DEFERRED_EVENTS_DIR): + os.mkdir(DEFERRED_EVENTS_DIR) + + +def get_deferred_events(): + """Return a list of deferred events requested by the charm and packages. + + :returns: List of deferred events + :rtype: List[ServiceEvent] + """ + events = [] + for _, event in deferred_events(): + events.append(event) + return events + + +def get_deferred_restarts(): + """List of deferred restart events requested by the charm and packages. + + :returns: List of deferred restarts + :rtype: List[ServiceEvent] + """ + return [e for e in get_deferred_events() if e.action == 'restart'] + + +def clear_deferred_restarts(services): + """Clear deferred restart events targetted at `services`. + + :param services: Services with deferred actions to clear. + :type services: List[str] + """ + clear_deferred_events(services, 'restart') + + +def process_svc_restart(service): + """Respond to a service restart having occured. + + :param service: Services that the action was performed against. + :type service: str + """ + clear_deferred_restarts([service]) + + +def is_restart_permitted(): + """Check whether restarts are permitted. + + :returns: Whether restarts are permitted + :rtype: bool + """ + if hookenv.config('enable-auto-restarts') is None: + return True + return hookenv.config('enable-auto-restarts') + + +def check_and_record_restart_request(service, changed_files): + """Check if restarts are permitted, if they are not log the request. + + :param service: Service to be restarted + :type service: str + :param changed_files: Files that have changed to trigger restarts. + :type changed_files: List[str] + :returns: Whether restarts are permitted + :rtype: bool + """ + changed_files = sorted(list(set(changed_files))) + permitted = is_restart_permitted() + if not permitted: + save_event(ServiceEvent( + timestamp=round(time.time()), + service=service, + reason='File(s) changed: {}'.format( + ', '.join(changed_files)), + action='restart')) + return permitted + + +def deferrable_svc_restart(service, reason=None): + """Restarts service if permitted, if not defer it. + + :param service: Service to be restarted + :type service: str + :param reason: Reason for restart + :type reason: Union[str, None] + """ + if is_restart_permitted(): + host.service_restart(service) + else: + save_event(ServiceEvent( + timestamp=round(time.time()), + service=service, + reason=reason, + action='restart')) + + +def configure_deferred_restarts(services): + """Setup deferred restarts. + + :param services: Services to block restarts of. + :type services: List[str] + """ + policy_rcd.install_policy_rcd() + if is_restart_permitted(): + policy_rcd.remove_policy_file() + else: + blocked_actions = ['stop', 'restart', 'try-restart'] + for svc in services: + policy_rcd.add_policy_block(svc, blocked_actions) + + +def get_service_start_time(service): + """Find point in time when the systemd unit transitioned to active state. + + :param service: Services to check timetsamp of. + :type service: str + """ + start_time = None + out = subprocess.check_output( + [ + 'systemctl', + 'show', + service, + '--property=ActiveEnterTimestamp']) + str_time = out.decode().rstrip().replace('ActiveEnterTimestamp=', '') + if str_time: + start_time = datetime.datetime.strptime( + str_time, + '%a %Y-%m-%d %H:%M:%S %Z') + return start_time + + +def check_restart_timestamps(): + """Check deferred restarts against systemd units start time. + + Check if a service has a deferred event and clear it if it has been + subsequently restarted. + """ + for event in get_deferred_restarts(): + start_time = get_service_start_time(event.service) + deferred_restart_time = datetime.datetime.fromtimestamp( + event.timestamp) + if start_time and start_time < deferred_restart_time: + hookenv.log( + ("Restart still required, {} was started at {}, restart was " + "requested after that at {}").format( + event.service, + start_time, + deferred_restart_time), + level='DEBUG') + else: + clear_deferred_restarts([event.service]) diff --git a/hooks/charmhelpers/contrib/openstack/exceptions.py b/hooks/charmhelpers/contrib/openstack/exceptions.py index f85ae4f4..b2330637 100644 --- a/hooks/charmhelpers/contrib/openstack/exceptions.py +++ b/hooks/charmhelpers/contrib/openstack/exceptions.py @@ -19,3 +19,8 @@ class OSContextError(Exception): This exception is principally used in contrib.openstack.context """ pass + + +class ServiceActionError(Exception): + """Raised when a service action (stop/start/ etc) failed.""" + pass diff --git a/hooks/charmhelpers/contrib/openstack/files/policy_rc_d_script.py b/hooks/charmhelpers/contrib/openstack/files/policy_rc_d_script.py new file mode 100755 index 00000000..344a7662 --- /dev/null +++ b/hooks/charmhelpers/contrib/openstack/files/policy_rc_d_script.py @@ -0,0 +1,196 @@ +#!/usr/bin/env python3 + +"""This script is an implemenation of policy-rc.d + +For further information on policy-rc.d see *1 + +*1 https://people.debian.org/~hmh/invokerc.d-policyrc.d-specification.txt +""" +import collections +import glob +import os +import logging +import sys +import time +import uuid +import yaml + + +SystemPolicy = collections.namedtuple( + 'SystemPolicy', + [ + 'policy_requestor_name', + 'policy_requestor_type', + 'service', + 'blocked_actions']) + +DEFAULT_POLICY_CONFIG_DIR = '/etc/policy-rc.d' +DEFAULT_POLICY_LOG_DIR = '/var/lib/policy-rc.d' + + +def read_policy_file(policy_file): + """Return system policies from given file. + + :param file_name: Name of file to read. + :type file_name: str + :returns: Policy + :rtype: List[SystemPolicy] + """ + policies = [] + if os.path.exists(policy_file): + with open(policy_file, 'r') as f: + policy = yaml.safe_load(f) + for service, actions in policy['blocked_actions'].items(): + service = service.replace('.service', '') + policies.append(SystemPolicy( + policy_requestor_name=policy['policy_requestor_name'], + policy_requestor_type=policy['policy_requestor_type'], + service=service, + blocked_actions=actions)) + return policies + + +def get_policies(policy_config_dir): + """Return all system policies in policy_config_dir. + + :param policy_config_dir: Name of file to read. + :type policy_config_dir: str + :returns: Policy + :rtype: List[SystemPolicy] + """ + _policy = [] + for f in glob.glob('{}/*.policy'.format(policy_config_dir)): + _policy.extend(read_policy_file(f)) + return _policy + + +def record_blocked_action(service, action, blocking_policies, policy_log_dir): + """Record that an action was requested but deniedl + + :param service: Service that was blocked + :type service: str + :param action: Action that was blocked. + :type action: str + :param blocking_policies: Policies that blocked the action on the service. + :type blocking_policies: List[SystemPolicy] + :param policy_log_dir: Directory to place the blocking action record. + :type policy_log_dir: str + """ + if not os.path.exists(policy_log_dir): + os.mkdir(policy_log_dir) + seconds = round(time.time()) + for policy in blocking_policies: + if not os.path.exists(policy_log_dir): + os.mkdir(policy_log_dir) + file_name = '{}/{}-{}-{}.deferred'.format( + policy_log_dir, + policy.policy_requestor_type, + policy.policy_requestor_name, + uuid.uuid1()) + with open(file_name, 'w') as f: + data = { + 'timestamp': seconds, + 'service': service, + 'action': action, + 'reason': 'Package update', + 'policy_requestor_type': policy.policy_requestor_type, + 'policy_requestor_name': policy.policy_requestor_name} + yaml.dump(data, f) + + +def get_blocking_policies(service, action, policy_config_dir): + """Record that an action was requested but deniedl + + :param service: Service that action is requested against. + :type service: str + :param action: Action that is requested. + :type action: str + :param policy_config_dir: Directory that stores policy files. + :type policy_config_dir: str + :returns: Policies + :rtype: List[SystemPolicy] + """ + service = service.replace('.service', '') + blocking_policies = [ + policy + for policy in get_policies(policy_config_dir) + if policy.service == service and action in policy.blocked_actions] + return blocking_policies + + +def process_action_request(service, action, policy_config_dir, policy_log_dir): + """Take the requested action against service and check if it is permitted. + + :param service: Service that action is requested against. + :type service: str + :param action: Action that is requested. + :type action: str + :param policy_config_dir: Directory that stores policy files. + :type policy_config_dir: str + :param policy_log_dir: Directory that stores policy files. + :type policy_log_dir: str + :returns: Tuple of whether the action is permitted and explanation. + :rtype: (boolean, str) + """ + blocking_policies = get_blocking_policies( + service, + action, + policy_config_dir) + if blocking_policies: + policy_msg = [ + '{} {}'.format(p.policy_requestor_type, p.policy_requestor_name) + for p in sorted(blocking_policies)] + message = '{} of {} blocked by {}'.format( + action, + service, + ', '.join(policy_msg)) + record_blocked_action( + service, + action, + blocking_policies, + policy_log_dir) + action_permitted = False + else: + message = "Permitting {} {}".format(service, action) + action_permitted = True + return action_permitted, message + + +def main(): + logging.basicConfig( + filename='/var/log/policy-rc.d.log', + level=logging.DEBUG, + format='%(asctime)s %(message)s') + + service = sys.argv[1] + action = sys.argv[2] + + permitted, message = process_action_request( + service, + action, + DEFAULT_POLICY_CONFIG_DIR, + DEFAULT_POLICY_LOG_DIR) + logging.info(message) + + # https://people.debian.org/~hmh/invokerc.d-policyrc.d-specification.txt + # Exit status codes: + # 0 - action allowed + # 1 - unknown action (therefore, undefined policy) + # 100 - unknown initscript id + # 101 - action forbidden by policy + # 102 - subsystem error + # 103 - syntax error + # 104 - [reserved] + # 105 - behaviour uncertain, policy undefined. + # 106 - action not allowed. Use the returned fallback actions + # (which are implied to be "allowed") instead. + + if permitted: + return 0 + else: + return 101 + + +if __name__ == "__main__": + rc = main() + sys.exit(rc) diff --git a/hooks/charmhelpers/contrib/openstack/policy_rcd.py b/hooks/charmhelpers/contrib/openstack/policy_rcd.py new file mode 100644 index 00000000..ecffbc68 --- /dev/null +++ b/hooks/charmhelpers/contrib/openstack/policy_rcd.py @@ -0,0 +1,173 @@ +# Copyright 2021 Canonical Limited. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Module for managing policy-rc.d script and associated files. + +This module manages the installation of /usr/sbin/policy-rc.d, the +policy files and the event files. When a package update occurs the +packaging system calls: + +policy-rc.d [options] + +The return code of the script determines if the packaging system +will perform that action on the given service. The policy-rc.d +implementation installed by this module checks if an action is +permitted by checking policy files placed in /etc/policy-rc.d. +If a policy file exists which denies the requested action then +this is recorded in an event file which is placed in +/var/lib/policy-rc.d. +""" + +import os +import shutil +import tempfile +import yaml + +import charmhelpers.contrib.openstack.files as os_files +import charmhelpers.contrib.openstack.alternatives as alternatives +import charmhelpers.core.hookenv as hookenv +import charmhelpers.core.host as host + +POLICY_HEADER = """# Managed by juju\n""" +POLICY_DEFERRED_EVENTS_DIR = '/var/lib/policy-rc.d' +POLICY_CONFIG_DIR = '/etc/policy-rc.d' + + +def get_policy_file_name(): + """Get the name of the policy file for this application. + + :returns: Policy file name + :rtype: str + """ + application_name = hookenv.service_name() + return '{}/charm-{}.policy'.format(POLICY_CONFIG_DIR, application_name) + + +def read_default_policy_file(): + """Return the policy file. + + A policy is in the form: + blocked_actions: + neutron-dhcp-agent: [restart, stop, try-restart] + neutron-l3-agent: [restart, stop, try-restart] + neutron-metadata-agent: [restart, stop, try-restart] + neutron-openvswitch-agent: [restart, stop, try-restart] + openvswitch-switch: [restart, stop, try-restart] + ovs-vswitchd: [restart, stop, try-restart] + ovs-vswitchd-dpdk: [restart, stop, try-restart] + ovsdb-server: [restart, stop, try-restart] + policy_requestor_name: neutron-openvswitch + policy_requestor_type: charm + + :returns: Policy + :rtype: Dict[str, Union[str, Dict[str, List[str]]] + """ + policy = {} + policy_file = get_policy_file_name() + if os.path.exists(policy_file): + with open(policy_file, 'r') as f: + policy = yaml.safe_load(f) + return policy + + +def write_policy_file(policy_file, policy): + """Write policy to disk. + + :param policy_file: Name of policy file + :type policy_file: str + :param policy: Policy + :type policy: Dict[str, Union[str, Dict[str, List[str]]]] + """ + with tempfile.NamedTemporaryFile('w', delete=False) as f: + f.write(POLICY_HEADER) + yaml.dump(policy, f) + tmp_file_name = f.name + shutil.move(tmp_file_name, policy_file) + + +def remove_policy_file(): + """Remove policy file.""" + try: + os.remove(get_policy_file_name()) + except FileNotFoundError: + pass + + +def install_policy_rcd(): + """Install policy-rc.d components.""" + source_file_dir = os.path.dirname(os.path.abspath(os_files.__file__)) + policy_rcd_exec = "/var/lib/charm/{}/policy-rc.d".format( + hookenv.service_name()) + host.mkdir(os.path.dirname(policy_rcd_exec)) + shutil.copy2( + '{}/policy_rc_d_script.py'.format(source_file_dir), + policy_rcd_exec) + # policy-rc.d must be installed via the alternatives system: + # https://people.debian.org/~hmh/invokerc.d-policyrc.d-specification.txt + if not os.path.exists('/usr/sbin/policy-rc.d'): + alternatives.install_alternative( + 'policy-rc.d', + '/usr/sbin/policy-rc.d', + policy_rcd_exec) + host.mkdir(POLICY_CONFIG_DIR) + + +def get_default_policy(): + """Return the default policy structure. + + :returns: Policy + :rtype: Dict[str, Union[str, Dict[str, List[str]]] + """ + policy = { + 'policy_requestor_name': hookenv.service_name(), + 'policy_requestor_type': 'charm', + 'blocked_actions': {}} + return policy + + +def add_policy_block(service, blocked_actions): + """Update a policy file with new list of actions. + + :param service: Service name + :type service: str + :param blocked_actions: Action to block + :type blocked_actions: List[str] + """ + policy = read_default_policy_file() or get_default_policy() + policy_file = get_policy_file_name() + if policy['blocked_actions'].get(service): + policy['blocked_actions'][service].extend(blocked_actions) + else: + policy['blocked_actions'][service] = blocked_actions + policy['blocked_actions'][service] = sorted( + list(set(policy['blocked_actions'][service]))) + write_policy_file(policy_file, policy) + + +def remove_policy_block(service, unblocked_actions): + """Remove list of actions from policy file. + + :param service: Service name + :type service: str + :param unblocked_actions: Action to unblock + :type unblocked_actions: List[str] + """ + policy_file = get_policy_file_name() + policy = read_default_policy_file() + for action in unblocked_actions: + try: + policy['blocked_actions'][service].remove(action) + except (KeyError, ValueError): + continue + write_policy_file(policy_file, policy) diff --git a/hooks/charmhelpers/contrib/openstack/utils.py b/hooks/charmhelpers/contrib/openstack/utils.py index f27aa6c9..7adf2f9c 100644 --- a/hooks/charmhelpers/contrib/openstack/utils.py +++ b/hooks/charmhelpers/contrib/openstack/utils.py @@ -14,7 +14,7 @@ # Common python helper functions used for OpenStack charms. from collections import OrderedDict, namedtuple -from functools import wraps +from functools import partial, wraps import subprocess import json @@ -36,9 +36,12 @@ from charmhelpers.contrib.network import ip from charmhelpers.core import decorators, unitdata +import charmhelpers.contrib.openstack.deferred_events as deferred_events + from charmhelpers.core.hookenv import ( WORKLOAD_STATES, action_fail, + action_get, action_set, config, expected_peer_units, @@ -112,7 +115,7 @@ from charmhelpers.fetch.snap import ( from charmhelpers.contrib.storage.linux.utils import is_block_device, zap_disk from charmhelpers.contrib.storage.linux.loopback import ensure_loopback_device -from charmhelpers.contrib.openstack.exceptions import OSContextError +from charmhelpers.contrib.openstack.exceptions import OSContextError, ServiceActionError from charmhelpers.contrib.openstack.policyd import ( policyd_status_message_prefix, POLICYD_CONFIG_NAME, @@ -483,9 +486,26 @@ def get_swift_codename(version): return None -@deprecate("moved to charmhelpers.contrib.openstack.utils.get_installed_os_version()", "2021-01", log=juju_log) def get_os_codename_package(package, fatal=True): - '''Derive OpenStack release codename from an installed package.''' + """Derive OpenStack release codename from an installed package. + + Initially, see if the openstack-release pkg is available (by trying to + install it) and use it instead. + + If it isn't then it falls back to the existing method of checking the + version of the package passed and then resolving the version from that + using lookup tables. + + Note: if possible, charms should use get_installed_os_version() to + determine the version of the "openstack-release" pkg. + + :param package: the package to test for version information. + :type package: str + :param fatal: If True (default), then die via error_out() + :type fatal: bool + :returns: the OpenStack release codename (e.g. ussuri) + :rtype: str + """ codename = get_installed_os_version() if codename: @@ -579,8 +599,22 @@ def get_os_version_package(pkg, fatal=True): def get_installed_os_version(): - apt_install(filter_installed_packages(['openstack-release']), fatal=False) - print("OpenStack Release: {}".format(openstack_release())) + """Determine the OpenStack release code name from openstack-release pkg. + + This uses the "openstack-release" pkg (if it exists) to return the + OpenStack release codename (e.g. usurri, mitaka, ocata, etc.) + + Note, it caches the result so that it is only done once per hook. + + :returns: the OpenStack release codename, if available + :rtype: Optional[str] + """ + @cached + def _do_install(): + apt_install(filter_installed_packages(['openstack-release']), + fatal=False, quiet=True) + + _do_install() return openstack_release().get('OPENSTACK_CODENAME') @@ -1052,6 +1086,12 @@ def _determine_os_workload_status( try: if config(POLICYD_CONFIG_NAME): message = "{} {}".format(policyd_status_message_prefix(), message) + deferred_restarts = list(set( + [e.service for e in deferred_events.get_deferred_restarts()])) + if deferred_restarts: + svc_msg = "Services queued for restart: {}".format( + ', '.join(sorted(deferred_restarts))) + message = "{}. {}".format(message, svc_msg) except Exception: pass @@ -1696,6 +1736,43 @@ def resume_unit(assess_status_func, services=None, ports=None, raise Exception("Couldn't resume: {}".format("; ".join(messages))) +def restart_services_action(services=None, when_all_stopped_func=None, + deferred_only=None): + """Manage a service restart request via charm action. + + :param services: Services to be restarted + :type model_name: List[str] + :param when_all_stopped_func: Function to call when all services are + stopped. + :type when_all_stopped_func: Callable[] + :param model_name: Only restart services which have a deferred restart + event. + :type model_name: bool + """ + if services and deferred_only: + raise ValueError( + "services and deferred_only are mutually exclusive") + if deferred_only: + services = list(set( + [a.service for a in deferred_events.get_deferred_restarts()])) + _, messages = manage_payload_services( + 'stop', + services=services, + charm_func=when_all_stopped_func) + if messages: + raise ServiceActionError( + "Error processing service stop request: {}".format( + "; ".join(messages))) + _, messages = manage_payload_services( + 'start', + services=services) + if messages: + raise ServiceActionError( + "Error processing service start request: {}".format( + "; ".join(messages))) + deferred_events.clear_deferred_restarts(services) + + def make_assess_status_func(*args, **kwargs): """Creates an assess_status_func() suitable for handing to pause_unit() and resume_unit(). @@ -1717,7 +1794,10 @@ def make_assess_status_func(*args, **kwargs): def pausable_restart_on_change(restart_map, stopstart=False, - restart_functions=None): + restart_functions=None, + can_restart_now_f=None, + post_svc_restart_f=None, + pre_restarts_wait_f=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. @@ -1743,11 +1823,28 @@ def pausable_restart_on_change(restart_map, stopstart=False, function won't be called if the decorated function is never called. Note, retains backwards compatibility for passing a non-callable dictionary. - @param f: the function to decorate - @param restart_map: (optionally callable, which then returns the - restart_map) the restart map {conf_file: [services]} - @param stopstart: DEFAULT false; whether to stop, start or just restart - @returns decorator to use a restart_on_change with pausability + :param f: function to decorate. + :type f: Callable + :param restart_map: Optionally callable, which then returns the restart_map or + the restart map {conf_file: [services]} + :type restart_map: Union[Callable[[],], Dict[str, List[str,]] + :param stopstart: whether to stop, start or restart a service + :type stopstart: booleean + :param restart_functions: nonstandard functions to use to restart services + {svc: func, ...} + :type restart_functions: Dict[str, Callable[[str], None]] + :param can_restart_now_f: A function used to check if the restart is + permitted. + :type can_restart_now_f: Callable[[str, List[str]], boolean] + :param post_svc_restart_f: A function run after a service has + restarted. + :type post_svc_restart_f: Callable[[str], None] + :param pre_restarts_wait_f: A function callled before any restarts. + :type pre_restarts_wait_f: Callable[None, None] + :returns: decorator to use a restart_on_change with pausability + :rtype: decorator + + """ def wrap(f): # py27 compatible nonlocal variable. When py3 only, replace with @@ -1763,8 +1860,13 @@ def pausable_restart_on_change(restart_map, stopstart=False, if callable(restart_map) else restart_map # otherwise, normal restart_on_change functionality return restart_on_change_helper( - (lambda: f(*args, **kwargs)), __restart_map_cache['cache'], - stopstart, restart_functions) + (lambda: f(*args, **kwargs)), + __restart_map_cache['cache'], + stopstart, + restart_functions, + can_restart_now_f, + post_svc_restart_f, + pre_restarts_wait_f) return wrapped_f return wrap @@ -2145,6 +2247,23 @@ def container_scoped_relations(): return relations +def container_scoped_relation_get(attribute=None): + """Get relation data from all container scoped relations. + + :param attribute: Name of attribute to get + :type attribute: Optional[str] + :returns: Iterator with relation data + :rtype: Iterator[Optional[any]] + """ + for endpoint_name in container_scoped_relations(): + for rid in relation_ids(endpoint_name): + for unit in related_units(rid): + yield relation_get( + attribute=attribute, + unit=unit, + rid=rid) + + def is_db_ready(use_current_context=False, rel_name=None): """Check remote database is ready to be used. @@ -2418,3 +2537,104 @@ def get_api_application_status(): msg = 'Some units are not ready' juju_log(msg, 'DEBUG') return app_state, msg + + +def sequence_status_check_functions(*functions): + """Sequence the functions passed so that they all get a chance to run as + the charm status check functions. + + :param *functions: a list of functions that return (state, message) + :type *functions: List[Callable[[OSConfigRender], (str, str)]] + :returns: the Callable that takes configs and returns (state, message) + :rtype: Callable[[OSConfigRender], (str, str)] + """ + def _inner_sequenced_functions(configs): + state, message = 'unknown', '' + for f in functions: + new_state, new_message = f(configs) + state = workload_state_compare(state, new_state) + if message: + message = "{}, {}".format(message, new_message) + else: + message = new_message + return state, message + + return _inner_sequenced_functions + + +SubordinatePackages = namedtuple('SubordinatePackages', ['install', 'purge']) + + +def get_subordinate_release_packages(os_release, package_type='deb'): + """Iterate over subordinate relations and get package information. + + :param os_release: OpenStack release to look for + :type os_release: str + :param package_type: Package type (one of 'deb' or 'snap') + :type package_type: str + :returns: Packages to install and packages to purge or None + :rtype: SubordinatePackages[set,set] + """ + install = set() + purge = set() + + for rdata in container_scoped_relation_get('releases-packages-map'): + rp_map = json.loads(rdata or '{}') + # The map provided by subordinate has OpenStack release name as key. + # Find package information from subordinate matching requested release + # or the most recent release prior to requested release by sorting the + # keys in reverse order. This follows established patterns in our + # charms for templates and reactive charm implementations, i.e. as long + # as nothing has changed the definitions for the prior OpenStack + # release is still valid. + for release in sorted(rp_map.keys(), reverse=True): + if (CompareOpenStackReleases(release) <= os_release and + package_type in rp_map[release]): + for name, container in ( + ('install', install), + ('purge', purge)): + for pkg in rp_map[release][package_type].get(name, []): + container.add(pkg) + break + return SubordinatePackages(install, purge) + + +os_restart_on_change = partial( + pausable_restart_on_change, + can_restart_now_f=deferred_events.check_and_record_restart_request, + post_svc_restart_f=deferred_events.process_svc_restart) + + +def restart_services_action_helper(all_services): + """Helper to run the restart-services action. + + NOTE: all_services is all services that could be restarted but + depending on the action arguments it may be a subset of + these that are actually restarted. + + :param all_services: All services that could be restarted + :type all_services: List[str] + """ + deferred_only = action_get("deferred-only") + services = action_get("services") + if services: + services = services.split() + else: + services = all_services + if deferred_only: + restart_services_action(deferred_only=True) + else: + restart_services_action(services=services) + + +def show_deferred_restarts_action_helper(): + """Helper to run the show-deferred-restarts action.""" + output = [] + for event in deferred_events.get_deferred_events(): + output.append('{} {} {}'.format( + str(event.timestamp), + event.service.ljust(40), + event.reason)) + output.sort() + action_set({'output': "{}".format( + yaml.dump(output, default_flow_style=False))}) diff --git a/hooks/charmhelpers/core/hookenv.py b/hooks/charmhelpers/core/hookenv.py index db7ce728..778aa4b6 100644 --- a/hooks/charmhelpers/core/hookenv.py +++ b/hooks/charmhelpers/core/hookenv.py @@ -226,6 +226,17 @@ def relation_id(relation_name=None, service_or_unit=None): raise ValueError('Must specify neither or both of relation_name and service_or_unit') +def departing_unit(): + """The departing unit for the current relation hook. + + Available since juju 2.8. + + :returns: the departing unit, or None if the information isn't available. + :rtype: Optional[str] + """ + return os.environ.get('JUJU_DEPARTING_UNIT', None) + + def local_unit(): """Local unit ID""" return os.environ['JUJU_UNIT_NAME'] @@ -1611,3 +1622,12 @@ def _contains_range(addresses): addresses.startswith(".") or ",." in addresses or " ." in addresses) + + +def is_subordinate(): + """Check whether charm is subordinate in unit metadata. + + :returns: True if unit is subordniate, False otherwise. + :rtype: bool + """ + return metadata().get('subordinate') is True diff --git a/hooks/charmhelpers/core/host.py b/hooks/charmhelpers/core/host.py index f826f6fe..d25e6c59 100644 --- a/hooks/charmhelpers/core/host.py +++ b/hooks/charmhelpers/core/host.py @@ -34,7 +34,7 @@ import itertools import six from contextlib import contextmanager -from collections import OrderedDict +from collections import OrderedDict, defaultdict from .hookenv import log, INFO, DEBUG, local_unit, charm_name from .fstab import Fstab from charmhelpers.osplatform import get_platform @@ -694,74 +694,223 @@ class ChecksumError(ValueError): pass -def restart_on_change(restart_map, stopstart=False, restart_functions=None): - """Restart services based on configuration files changing +class restart_on_change(object): + """Decorator and context manager to handle restarts. - This function is used a decorator, for example:: + Usage: - @restart_on_change({ - '/etc/ceph/ceph.conf': [ 'cinder-api', 'cinder-volume' ] - '/etc/apache/sites-enabled/*': [ 'apache2' ] - }) - def config_changed(): - pass # your code here + @restart_on_change(restart_map, ...) + def function_that_might_trigger_a_restart(...) + ... - In this example, the cinder-api and cinder-volume services - would be restarted if /etc/ceph/ceph.conf is changed by the - ceph_client_changed function. The apache2 service would be - restarted if any file matching the pattern got changed, created - or removed. Standard wildcards are supported, see documentation - for the 'glob' module for more information. + Or: - @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 + with restart_on_change(restart_map, ...): + do_stuff_that_might_trigger_a_restart() + ... """ - def wrap(f): + + def __init__(self, restart_map, stopstart=False, restart_functions=None, + can_restart_now_f=None, post_svc_restart_f=None, + pre_restarts_wait_f=None): + """ + :param restart_map: {file: [service, ...]} + :type restart_map: Dict[str, List[str,]] + :param stopstart: whether to stop, start or restart a service + :type stopstart: booleean + :param restart_functions: nonstandard functions to use to restart + services {svc: func, ...} + :type restart_functions: Dict[str, Callable[[str], None]] + :param can_restart_now_f: A function used to check if the restart is + permitted. + :type can_restart_now_f: Callable[[str, List[str]], boolean] + :param post_svc_restart_f: A function run after a service has + restarted. + :type post_svc_restart_f: Callable[[str], None] + :param pre_restarts_wait_f: A function callled before any restarts. + :type pre_restarts_wait_f: Callable[None, None] + """ + self.restart_map = restart_map + self.stopstart = stopstart + self.restart_functions = restart_functions + self.can_restart_now_f = can_restart_now_f + self.post_svc_restart_f = post_svc_restart_f + self.pre_restarts_wait_f = pre_restarts_wait_f + + def __call__(self, f): + """Work like a decorator. + + Returns a wrapped function that performs the restart if triggered. + + :param f: The function that is being wrapped. + :type f: Callable[[Any], Any] + :returns: the wrapped function + :rtype: Callable[[Any], Any] + """ @functools.wraps(f) def wrapped_f(*args, **kwargs): return restart_on_change_helper( - (lambda: f(*args, **kwargs)), restart_map, stopstart, - restart_functions) + (lambda: f(*args, **kwargs)), + self.restart_map, + stopstart=self.stopstart, + restart_functions=self.restart_functions, + can_restart_now_f=self.can_restart_now_f, + post_svc_restart_f=self.post_svc_restart_f, + pre_restarts_wait_f=self.pre_restarts_wait_f) return wrapped_f - return wrap + + def __enter__(self): + """Enter the runtime context related to this object. """ + self.checksums = _pre_restart_on_change_helper(self.restart_map) + + def __exit__(self, exc_type, exc_val, exc_tb): + """Exit the runtime context related to this object. + + The parameters describe the exception that caused the context to be + exited. If the context was exited without an exception, all three + arguments will be None. + """ + if exc_type is None: + _post_restart_on_change_helper( + self.checksums, + self.restart_map, + stopstart=self.stopstart, + restart_functions=self.restart_functions, + can_restart_now_f=self.can_restart_now_f, + post_svc_restart_f=self.post_svc_restart_f, + pre_restarts_wait_f=self.pre_restarts_wait_f) + # All is good, so return False; any exceptions will propagate. + return False def restart_on_change_helper(lambda_f, restart_map, stopstart=False, - restart_functions=None): + restart_functions=None, + can_restart_now_f=None, + post_svc_restart_f=None, + pre_restarts_wait_f=None): """Helper function to perform the restart_on_change function. This is provided for decorators to restart services if files described in the restart_map have changed after an invocation of lambda_f(). - @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 + This functions allows for a number of helper functions to be passed. + + `restart_functions` is a map with a service as the key and the + corresponding value being the function to call to restart the service. For + example if `restart_functions={'some-service': my_restart_func}` then + `my_restart_func` should a function which takes one argument which is the + service name to be retstarted. + + `can_restart_now_f` is a function which checks that a restart is permitted. + It should return a bool which indicates if a restart is allowed and should + take a service name (str) and a list of changed files (List[str]) as + arguments. + + `post_svc_restart_f` is a function which runs after a service has been + restarted. It takes the service name that was restarted as an argument. + + `pre_restarts_wait_f` is a function which is called before any restarts + occur. The use case for this is an application which wants to try and + stagger restarts between units. + + :param lambda_f: function to call. + :type lambda_f: Callable[[], ANY] + :param restart_map: {file: [service, ...]} + :type restart_map: Dict[str, List[str,]] + :param stopstart: whether to stop, start or restart a service + :type stopstart: booleean + :param restart_functions: nonstandard functions to use to restart services {svc: func, ...} - @returns result of lambda_f() + :type restart_functions: Dict[str, Callable[[str], None]] + :param can_restart_now_f: A function used to check if the restart is + permitted. + :type can_restart_now_f: Callable[[str, List[str]], boolean] + :param post_svc_restart_f: A function run after a service has + restarted. + :type post_svc_restart_f: Callable[[str], None] + :param pre_restarts_wait_f: A function callled before any restarts. + :type pre_restarts_wait_f: Callable[None, None] + :returns: result of lambda_f() + :rtype: ANY + """ + checksums = _pre_restart_on_change_helper(restart_map) + r = lambda_f() + _post_restart_on_change_helper(checksums, + restart_map, + stopstart, + restart_functions, + can_restart_now_f, + post_svc_restart_f, + pre_restarts_wait_f) + return r + + +def _pre_restart_on_change_helper(restart_map): + """Take a snapshot of file hashes. + + :param restart_map: {file: [service, ...]} + :type restart_map: Dict[str, List[str,]] + :returns: Dictionary of file paths and the files checksum. + :rtype: Dict[str, str] + """ + return {path: path_hash(path) for path in restart_map} + + +def _post_restart_on_change_helper(checksums, + restart_map, + stopstart=False, + restart_functions=None, + can_restart_now_f=None, + post_svc_restart_f=None, + pre_restarts_wait_f=None): + """Check whether files have changed. + + :param checksums: Dictionary of file paths and the files checksum. + :type checksums: Dict[str, str] + :param restart_map: {file: [service, ...]} + :type restart_map: Dict[str, List[str,]] + :param stopstart: whether to stop, start or restart a service + :type stopstart: booleean + :param restart_functions: nonstandard functions to use to restart services + {svc: func, ...} + :type restart_functions: Dict[str, Callable[[str], None]] + :param can_restart_now_f: A function used to check if the restart is + permitted. + :type can_restart_now_f: Callable[[str, List[str]], boolean] + :param post_svc_restart_f: A function run after a service has + restarted. + :type post_svc_restart_f: Callable[[str], None] + :param pre_restarts_wait_f: A function callled before any restarts. + :type pre_restarts_wait_f: Callable[None, None] """ if restart_functions is None: restart_functions = {} - checksums = {path: path_hash(path) for path in restart_map} - r = lambda_f() + changed_files = defaultdict(list) + restarts = [] # create a list of lists of the services to restart - restarts = [restart_map[path] - for path in restart_map - if path_hash(path) != checksums[path]] + for path, services in restart_map.items(): + if path_hash(path) != checksums[path]: + restarts.append(services) + for svc in services: + changed_files[svc].append(path) # create a flat list of ordered services without duplicates from lists services_list = list(OrderedDict.fromkeys(itertools.chain(*restarts))) if services_list: + if pre_restarts_wait_f: + pre_restarts_wait_f() actions = ('stop', 'start') if stopstart else ('restart',) for service_name in services_list: + if can_restart_now_f: + if not can_restart_now_f(service_name, + changed_files[service_name]): + continue if service_name in restart_functions: restart_functions[service_name](service_name) else: for action in actions: service(action, service_name) - return r + if post_svc_restart_f: + post_svc_restart_f(service_name) def pwgen(length=None): @@ -1068,6 +1217,17 @@ def modulo_distribution(modulo=3, wait=30, non_zero_wait=False): return calculated_wait_time +def ca_cert_absolute_path(basename_without_extension): + """Returns absolute path to CA certificate. + + :param basename_without_extension: Filename without extension + :type basename_without_extension: str + :returns: Absolute full path + :rtype: str + """ + return '{}/{}.crt'.format(CA_CERT_DIR, basename_without_extension) + + def install_ca_cert(ca_cert, name=None): """ Install the given cert as a trusted CA. @@ -1083,7 +1243,7 @@ def install_ca_cert(ca_cert, name=None): ca_cert = ca_cert.encode('utf8') if not name: name = 'juju-{}'.format(charm_name()) - cert_file = '{}/{}.crt'.format(CA_CERT_DIR, name) + cert_file = ca_cert_absolute_path(name) new_hash = hashlib.md5(ca_cert).hexdigest() if file_hash(cert_file) == new_hash: return diff --git a/hooks/charmhelpers/fetch/ubuntu.py b/hooks/charmhelpers/fetch/ubuntu.py index b5953019..1de9cd52 100644 --- a/hooks/charmhelpers/fetch/ubuntu.py +++ b/hooks/charmhelpers/fetch/ubuntu.py @@ -13,6 +13,7 @@ # limitations under the License. from collections import OrderedDict +import os import platform import re import six @@ -20,6 +21,7 @@ import subprocess import sys import time +from charmhelpers import deprecate from charmhelpers.core.host import get_distrib_codename, get_system_env from charmhelpers.core.hookenv import ( @@ -251,13 +253,19 @@ def apt_cache(*_, **__): # Detect this situation, log a warning and make the call to # ``apt_pkg.init()`` to avoid the consumer Python interpreter from # crashing with a segmentation fault. - log('Support for use of upstream ``apt_pkg`` module in conjunction' - 'with charm-helpers is deprecated since 2019-06-25', level=WARNING) + @deprecate( + 'Support for use of upstream ``apt_pkg`` module in conjunction' + 'with charm-helpers is deprecated since 2019-06-25', + date=None, log=lambda x: log(x, level=WARNING)) + def one_shot_log(): + pass + + one_shot_log() sys.modules['apt_pkg'].init() return ubuntu_apt_pkg.Cache() -def apt_install(packages, options=None, fatal=False): +def apt_install(packages, options=None, fatal=False, quiet=False): """Install one or more packages. :param packages: Package(s) to install @@ -267,6 +275,8 @@ def apt_install(packages, options=None, fatal=False): :param fatal: Whether the command's output should be checked and retried. :type fatal: bool + :param quiet: if True (default), supress log message to stdout/stderr + :type quiet: bool :raises: subprocess.CalledProcessError """ if options is None: @@ -279,9 +289,10 @@ def apt_install(packages, options=None, fatal=False): cmd.append(packages) else: cmd.extend(packages) - log("Installing {} with options: {}".format(packages, - options)) - _run_apt_command(cmd, fatal) + if not quiet: + log("Installing {} with options: {}" + .format(packages, options)) + _run_apt_command(cmd, fatal, quiet=quiet) def apt_upgrade(options=None, fatal=False, dist=False): @@ -639,14 +650,17 @@ def _add_apt_repository(spec): :param spec: the parameter to pass to add_apt_repository :type spec: str """ + series = get_distrib_codename() if '{series}' in spec: - series = get_distrib_codename() spec = spec.replace('{series}', series) # software-properties package for bionic properly reacts to proxy settings - # passed as environment variables (See lp:1433761). This is not the case - # LTS and non-LTS releases below bionic. - _run_with_retries(['add-apt-repository', '--yes', spec], - cmd_env=env_proxy_settings(['https', 'http'])) + # set via apt.conf (see lp:1433761), however this is not the case for LTS + # and non-LTS releases before bionic. + if series in ('trusty', 'xenial'): + _run_with_retries(['add-apt-repository', '--yes', spec], + cmd_env=env_proxy_settings(['https', 'http'])) + else: + _run_with_retries(['add-apt-repository', '--yes', spec]) def _add_cloud_pocket(pocket): @@ -723,7 +737,7 @@ def _verify_is_ubuntu_rel(release, os_release): def _run_with_retries(cmd, max_retries=CMD_RETRY_COUNT, retry_exitcodes=(1,), - retry_message="", cmd_env=None): + retry_message="", cmd_env=None, quiet=False): """Run a command and retry until success or max_retries is reached. :param cmd: The apt command to run. @@ -738,11 +752,20 @@ def _run_with_retries(cmd, max_retries=CMD_RETRY_COUNT, retry_exitcodes=(1,), :type retry_message: str :param: cmd_env: Environment variables to add to the command run. :type cmd_env: Option[None, Dict[str, str]] + :param quiet: if True, silence the output of the command from stdout and + stderr + :type quiet: bool """ env = get_apt_dpkg_env() if cmd_env: env.update(cmd_env) + kwargs = {} + if quiet: + devnull = os.devnull if six.PY2 else subprocess.DEVNULL + kwargs['stdout'] = devnull + kwargs['stderr'] = devnull + if not retry_message: retry_message = "Failed executing '{}'".format(" ".join(cmd)) retry_message += ". Will retry in {} seconds".format(CMD_RETRY_DELAY) @@ -753,7 +776,7 @@ def _run_with_retries(cmd, max_retries=CMD_RETRY_COUNT, retry_exitcodes=(1,), retry_results = (None,) + retry_exitcodes while result in retry_results: try: - result = subprocess.check_call(cmd, env=env) + result = subprocess.check_call(cmd, env=env, **kwargs) except subprocess.CalledProcessError as e: retry_count = retry_count + 1 if retry_count > max_retries: @@ -763,7 +786,7 @@ def _run_with_retries(cmd, max_retries=CMD_RETRY_COUNT, retry_exitcodes=(1,), time.sleep(CMD_RETRY_DELAY) -def _run_apt_command(cmd, fatal=False): +def _run_apt_command(cmd, fatal=False, quiet=False): """Run an apt command with optional retries. :param cmd: The apt command to run. @@ -771,13 +794,22 @@ def _run_apt_command(cmd, fatal=False): :param fatal: Whether the command's output should be checked and retried. :type fatal: bool + :param quiet: if True, silence the output of the command from stdout and + stderr + :type quiet: bool """ if fatal: _run_with_retries( cmd, retry_exitcodes=(1, APT_NO_LOCK,), - retry_message="Couldn't acquire DPKG lock") + retry_message="Couldn't acquire DPKG lock", + quiet=quiet) else: - subprocess.call(cmd, env=get_apt_dpkg_env()) + kwargs = {} + if quiet: + devnull = os.devnull if six.PY2 else subprocess.DEVNULL + kwargs['stdout'] = devnull + kwargs['stderr'] = devnull + subprocess.call(cmd, env=get_apt_dpkg_env(), **kwargs) def get_upstream_version(package): diff --git a/hooks/neutron_ovs_utils.py b/hooks/neutron_ovs_utils.py index ab37fe4a..e368cd0a 100644 --- a/hooks/neutron_ovs_utils.py +++ b/hooks/neutron_ovs_utils.py @@ -36,13 +36,14 @@ from collections import OrderedDict import neutron_ovs_context from charmhelpers.contrib.network.ovs import ( add_bridge, - add_bridge_port, add_bridge_bond, + add_bridge_port, is_linuxbridge_interface, add_ovsbridge_linuxbridge, full_restart, enable_ipfix, disable_ipfix, + generate_external_ids, ) from charmhelpers.core.hookenv import ( config, @@ -596,6 +597,15 @@ def purge_sriov_systemd_files(): def configure_ovs(): + """Configure the OVS plugin. + + This function uses the config.yaml parameters ext-port, data-port and + bridge-mappings to configure the bridges and ports on the ovs on the + unit. + + Note that the ext-port is deprecated and data-port/bridge-mappings are + preferred. + """ status_set('maintenance', 'Configuring ovs') if not service_running('openvswitch-switch'): full_restart() @@ -604,6 +614,7 @@ def configure_ovs(): brdata = { 'datapath-type': determine_datapath_type(), } + brdata.update(generate_external_ids()) add_bridge(INT_BRIDGE, brdata=brdata) add_bridge(EXT_BRIDGE, brdata=brdata) @@ -612,7 +623,9 @@ def configure_ovs(): if use_dvr(): ext_port_ctx = ExternalPortContext()() if ext_port_ctx and ext_port_ctx['ext_port']: - add_bridge_port(EXT_BRIDGE, ext_port_ctx['ext_port']) + add_bridge_port(EXT_BRIDGE, ext_port_ctx['ext_port'], + ifdata=generate_external_ids(EXT_BRIDGE), + portdata=generate_external_ids(EXT_BRIDGE)) modern_ovs = ovs_has_late_dpdk_init() @@ -634,9 +647,14 @@ def configure_ovs(): for port, _br in portmaps.items(): if _br == br: if not is_linuxbridge_interface(port): - add_bridge_port(br, port, promisc=True) + add_bridge_port( + br, port, promisc=True, + ifdata=generate_external_ids(br), + portdata=generate_external_ids(br)) else: - add_ovsbridge_linuxbridge(br, port) + add_ovsbridge_linuxbridge( + br, port, ifdata=generate_external_ids(br), + portdata=generate_external_ids(br)) # NOTE(jamespage): # hw-offload and dpdk are mutually exclusive so log and error @@ -671,22 +689,27 @@ def configure_ovs(): for port in port_iface_map.keys(): ifdatamap = bridge_port_interface_map.get_ifdatamap( br, port) + # set external-ids for all interfaces + for iface in ifdatamap: + ifdatamap[iface].update(generate_external_ids(br)) # NOTE: DPDK bonds are referenced by name and can be found # in the data-port config, regular DPDK ports are # referenced by MAC addresses and their names should # never be found in data-port if port in portmaps.keys(): + portdata = bond_config.get_ovs_portdata(port) + portdata.update(generate_external_ids(br)) log('Adding DPDK bond: {}({}) to bridge: {}'.format( port, list(ifdatamap.keys()), br), level=DEBUG) add_bridge_bond( br, port, list(ifdatamap.keys()), - portdata=bond_config.get_ovs_portdata(port), - ifdatamap=ifdatamap) + portdata=portdata, ifdatamap=ifdatamap) else: log('Adding DPDK port: {} to bridge: {}'.format( port, br), level=DEBUG) ifdata = ifdatamap[port] add_bridge_port(br, port, ifdata=ifdata, + portdata=generate_external_ids(br), linkup=False, promisc=None) if not modern_ovs: # port enumeration in legacy OVS-DPDK must follow alphabetic order @@ -701,12 +724,14 @@ def configure_ovs(): 'type': 'dpdk', 'mtu-request': global_mtu } + ifdata.update(generate_external_ids(mac.entity)) ifname = 'dpdk{}'.format(dev_idx) log('Adding DPDK port {}:{} to bridge {}'.format( ifname, ifdata, mac.entity), level=DEBUG) add_bridge_port( - mac.entity, ifname, ifdata=ifdata, linkup=False, - promisc=None) + mac.entity, ifname, ifdata=ifdata, + portdata=generate_external_ids(mac.entity), + linkup=False, promisc=None) else: log('DPDK device {} skipped, {} is not a bridge'.format( pci, mac.entity), level=WARNING) @@ -885,7 +910,8 @@ def assess_status_func(configs, exclude_services=None): return make_assess_status_func( configs, required_interfaces, charm_func=validate_ovs_use_veth, - services=services(exclude_services), ports=None) + services=services(exclude_services), + ports=None) def pause_unit_helper(configs, exclude_services=None): @@ -924,5 +950,4 @@ def _pause_resume_helper(f, configs, exclude_services=None): if exclude_services is None: exclude_services = [] f(assess_status_func(configs, exclude_services), - services=services(exclude_services), - ports=None) + services=services(exclude_services), ports=None) diff --git a/tests/tests.yaml b/tests/tests.yaml index cb21f495..f752e070 100644 --- a/tests/tests.yaml +++ b/tests/tests.yaml @@ -57,8 +57,10 @@ target_deploy_status: tests: - zaza.openstack.charm_tests.neutron.tests.NeutronNetworkingTest - zaza.openstack.charm_tests.neutron.tests.NeutronOpenvSwitchTest + - zaza.openstack.charm_tests.neutron.tests.NeutronOvsVsctlTest - migrate-ovn: - zaza.openstack.charm_tests.neutron.tests.NeutronNetworkingTest + - zaza.openstack.charm_tests.neutron.tests.NeutronOvsVsctlTest - zaza.openstack.charm_tests.ovn.tests.OVSOVNMigrationTest - zaza.openstack.charm_tests.neutron.tests.NeutronNetworkingTest tests_options: diff --git a/unit_tests/test_neutron_ovs_utils.py b/unit_tests/test_neutron_ovs_utils.py index d636b88c..38dd6253 100644 --- a/unit_tests/test_neutron_ovs_utils.py +++ b/unit_tests/test_neutron_ovs_utils.py @@ -15,7 +15,7 @@ import hashlib import subprocess -from mock import MagicMock, patch, call +from mock import MagicMock, patch, call, ANY from collections import OrderedDict import charmhelpers.contrib.openstack.templating as templating @@ -588,9 +588,12 @@ class TestNeutronOVSUtils(CharmTestCase): @patch('charmhelpers.contrib.openstack.context.list_nics', return_value=['eth0']) @patch.object(nutils, 'use_dvr') + @patch('charmhelpers.contrib.network.ovs.charm_name') @patch('charmhelpers.contrib.openstack.context.config') - def test_configure_ovs_ovs_data_port(self, mock_config, _use_dvr, _nics): + def test_configure_ovs_ovs_data_port( + self, mock_config, _charm_name, _use_dvr, _nics): _use_dvr.return_value = False + _charm_name.return_value = "neutron-openvswitch" self.is_linuxbridge_interface.return_value = False mock_config.side_effect = self.test_config.get self.config.side_effect = self.test_config.get @@ -600,15 +603,28 @@ class TestNeutronOVSUtils(CharmTestCase): # Test back-compatibility i.e. port but no bridge (so br-data is # assumed) self.test_config.set('data-port', 'eth0') + # Ensure that bridges are marked as managed + expected_brdata = { + 'datapath-type': 'system', + 'external-ids': { + 'charm-neutron-openvswitch': 'managed' + } + } + expected_ifdata = { + 'external-ids': { + 'charm-neutron-openvswitch': 'br-data' + } + } nutils.configure_ovs() - expected_brdata = {'datapath-type': 'system'} self.add_bridge.assert_has_calls([ call('br-int', brdata=expected_brdata), call('br-ex', brdata=expected_brdata), call('br-data', brdata=expected_brdata) ]) - self.assertTrue(self.add_bridge_port.called) - + self.add_bridge_port.assert_called_with('br-data', 'eth0', + ifdata=expected_ifdata, + portdata=expected_ifdata, + promisc=ANY) # Now test with bridge:port format self.test_config.set('data-port', 'br-foo:eth0') self.add_bridge.reset_mock() @@ -624,11 +640,13 @@ class TestNeutronOVSUtils(CharmTestCase): @patch('charmhelpers.contrib.openstack.context.list_nics', return_value=['eth0', 'br-juju']) + @patch('charmhelpers.contrib.network.ovs.charm_name') @patch.object(nutils, 'use_dvr') @patch('charmhelpers.contrib.openstack.context.config') def test_configure_ovs_data_port_with_bridge( - self, mock_config, _use_dvr, _nics): + self, mock_config, _use_dvr, _charm_name, _nics): _use_dvr.return_value = False + _charm_name.return_value = "neutron-openvswitch" self.is_linuxbridge_interface.return_value = True mock_config.side_effect = self.test_config.get self.config.side_effect = self.test_config.get @@ -645,10 +663,12 @@ class TestNeutronOVSUtils(CharmTestCase): self.assertTrue(self.add_ovsbridge_linuxbridge.called) @patch.object(nutils, 'use_dvr') + @patch('charmhelpers.contrib.network.ovs.charm_name') @patch('charmhelpers.contrib.openstack.context.config') - def test_configure_ovs_starts_service_if_required(self, mock_config, - _use_dvr): + def test_configure_ovs_starts_service_if_required( + self, mock_config, _charm_name, _use_dvr): _use_dvr.return_value = False + _charm_name.return_value = "neutron-openvswitch" mock_config.side_effect = self.test_config.get self.config.return_value = 'ovs' self.service_running.return_value = False @@ -656,9 +676,12 @@ class TestNeutronOVSUtils(CharmTestCase): self.assertTrue(self.full_restart.called) @patch.object(nutils, 'use_dvr') + @patch('charmhelpers.contrib.network.ovs.charm_name') @patch('charmhelpers.contrib.openstack.context.config') - def test_configure_ovs_doesnt_restart_service(self, mock_config, _use_dvr): + def test_configure_ovs_doesnt_restart_service( + self, mock_config, _charm_name, _use_dvr): _use_dvr.return_value = False + _charm_name.return_value = "neutron-openvswitch" mock_config.side_effect = self.test_config.get self.config.side_effect = self.test_config.get self.service_running.return_value = True @@ -666,24 +689,40 @@ class TestNeutronOVSUtils(CharmTestCase): self.assertFalse(self.full_restart.called) @patch.object(nutils, 'use_dvr') + @patch('charmhelpers.contrib.network.ovs.charm_name') @patch('charmhelpers.contrib.openstack.context.config') - def test_configure_ovs_ovs_ext_port(self, mock_config, _use_dvr): + def test_configure_ovs_ovs_ext_port( + self, mock_config, _charm_name, _use_dvr): _use_dvr.return_value = True + _charm_name.return_value = "neutron-openvswitch" mock_config.side_effect = self.test_config.get self.config.side_effect = self.test_config.get self.test_config.set('ext-port', 'eth0') self.ExternalPortContext.return_value = \ DummyContext(return_value={'ext_port': 'eth0'}) + # Ensure that bridges are marked as managed + expected_brdata = { + 'datapath-type': 'system', + 'external-ids': { + 'charm-neutron-openvswitch': 'managed' + } + } + expected_ifdata = { + 'external-ids': { + 'charm-neutron-openvswitch': 'br-ex' + } + } nutils.configure_ovs() - expected_brdata = {'datapath-type': 'system'} self.add_bridge.assert_has_calls([ call('br-int', brdata=expected_brdata), call('br-ex', brdata=expected_brdata), call('br-data', brdata=expected_brdata) ]) - self.add_bridge_port.assert_called_with('br-ex', 'eth0') + self.add_bridge_port.assert_called_with('br-ex', 'eth0', + ifdata=expected_ifdata, + portdata=expected_ifdata) - def _run_configure_ovs_dpdk(self, mock_config, _use_dvr, + def _run_configure_ovs_dpdk(self, mock_config, _use_dvr, _charm_name, _OVSDPDKDeviceContext, _BridgePortInterfaceMap, _parse_data_port_mappings, @@ -761,6 +800,10 @@ class TestNeutronOVSUtils(CharmTestCase): _parse_bridge_mappings.return_value = { 'phynet1': br[0], 'phynet2': br[1], 'phynet3': br[2]} _use_dvr.return_value = True + _charm_name.return_value = "neutron-openvswitch" + self.use_dpdk.return_value = True + self.ovs_has_late_dpdk_init.return_value = _late_init + self.ovs_vhostuser_client.return_value = _ovs_vhostuser_client mock_config.side_effect = self.test_config.get self.config.side_effect = self.test_config.get self.test_config.set('enable-dpdk', True) @@ -768,7 +811,12 @@ class TestNeutronOVSUtils(CharmTestCase): self.ovs_has_late_dpdk_init.return_value = _late_init self.ovs_vhostuser_client.return_value = _ovs_vhostuser_client nutils.configure_ovs() - expetected_brdata = {'datapath-type': 'netdev'} + expetected_brdata = { + 'datapath-type': 'netdev', + 'external-ids': { + 'charm-neutron-openvswitch': 'managed' + } + } self.add_bridge.assert_has_calls([ call('br-int', brdata=expetected_brdata), call('br-ex', brdata=expetected_brdata), @@ -783,32 +831,44 @@ class TestNeutronOVSUtils(CharmTestCase): [_resolve_port_name(pci[0], 0, _late_init)], portdata={'bond_mode': 'balance-tcp', 'lacp': 'active', - 'other_config': {'lacp-time': 'fast'}}, + 'other_config': {'lacp-time': 'fast'}, + 'external-ids': { + 'charm-neutron-openvswitch': br[0]}}, ifdatamap={ _resolve_port_name(pci[0], 0, _late_init): { 'type': 'dpdk', 'mtu-request': 1500, - 'options': {'dpdk-devargs': pci[0]}}}), + 'options': {'dpdk-devargs': pci[0]}, + 'external-ids':{ + 'charm-neutron-openvswitch': br[0]}}}), call(br[1], 'bond1', [_resolve_port_name(pci[1], 1, _late_init)], portdata={'bond_mode': 'balance-tcp', 'lacp': 'active', - 'other_config': {'lacp-time': 'fast'}}, + 'other_config': {'lacp-time': 'fast'}, + 'external-ids': { + 'charm-neutron-openvswitch': br[1]}}, ifdatamap={ _resolve_port_name(pci[1], 1, _late_init):{ 'type': 'dpdk', 'mtu-request': 1500, - 'options': {'dpdk-devargs': pci[1]}}}), + 'options': {'dpdk-devargs': pci[1]}, + 'external-ids':{ + 'charm-neutron-openvswitch': br[1]}}}), call(br[2], 'bond2', [_resolve_port_name(pci[2], 2, _late_init)], portdata={'bond_mode': 'balance-tcp', 'lacp': 'active', - 'other_config': {'lacp-time': 'fast'}}, + 'other_config': {'lacp-time': 'fast'}, + 'external-ids': { + 'charm-neutron-openvswitch': br[2]}}, ifdatamap={ _resolve_port_name(pci[2], 2, _late_init): { 'type': 'dpdk', 'mtu-request': 1500, - 'options': {'dpdk-devargs': pci[2]}}})], + 'options': {'dpdk-devargs': pci[2]}, + 'external-ids':{ + 'charm-neutron-openvswitch': br[2]}}})], any_order=True) else: if _late_init: @@ -816,28 +876,49 @@ class TestNeutronOVSUtils(CharmTestCase): call(br[0], _resolve_port_name(pci[0], 0, _late_init), ifdata={'type': 'dpdk', 'mtu-request': 1500, - 'options': {'dpdk-devargs': pci[0]}}, + 'options': {'dpdk-devargs': pci[0]}, + 'external-ids':{ + 'charm-neutron-openvswitch': br[0]}}, + portdata={'external-ids': { + 'charm-neutron-openvswitch': br[0]}}, linkup=False, promisc=None), call(br[1], _resolve_port_name(pci[1], 1, _late_init), ifdata={'type': 'dpdk', 'mtu-request': 1500, - 'options': {'dpdk-devargs': pci[1]}}, + 'options': {'dpdk-devargs': pci[1]}, + 'external-ids':{ + 'charm-neutron-openvswitch': br[1]}}, + portdata={'external-ids': { + 'charm-neutron-openvswitch': br[1]}}, linkup=False, promisc=None), call(br[2], _resolve_port_name(pci[2], 2, _late_init), ifdata={'type': 'dpdk', 'mtu-request': 1500, - 'options': {'dpdk-devargs': pci[2]}}, + 'options': {'dpdk-devargs': pci[2]}, + 'external-ids':{ + 'charm-neutron-openvswitch': br[2]}}, + portdata={'external-ids': { + 'charm-neutron-openvswitch': br[2]}}, linkup=False, promisc=None)], any_order=True) else: self.add_bridge_port.assert_has_calls([ call(br[0], _resolve_port_name(pci[0], 0, _late_init), - ifdata={'type': 'dpdk', 'mtu-request': 1500}, + ifdata={'type': 'dpdk', 'mtu-request': 1500, + 'external-ids': {'charm-neutron-openvswitch': br[0]}}, + portdata={'external-ids': { + 'charm-neutron-openvswitch': br[0]}}, linkup=False, promisc=None), call(br[1], _resolve_port_name(pci[1], 1, _late_init), - ifdata={'type': 'dpdk', 'mtu-request': 1500}, + ifdata={'type': 'dpdk', 'mtu-request': 1500, + 'external-ids': {'charm-neutron-openvswitch': br[1]}}, + portdata={'external-ids': { + 'charm-neutron-openvswitch': br[1]}}, linkup=False, promisc=None), call(br[2], _resolve_port_name(pci[2], 2, _late_init), - ifdata={'type': 'dpdk', 'mtu-request': 1500}, + ifdata={'type': 'dpdk', 'mtu-request': 1500, + 'external-ids': {'charm-neutron-openvswitch': br[2]}}, + portdata={'external-ids': { + 'charm-neutron-openvswitch': br[2]}}, linkup=False, promisc=None)], any_order=True) @patch.object(nutils, 'use_hw_offload', return_value=False) @@ -846,9 +927,10 @@ class TestNeutronOVSUtils(CharmTestCase): @patch.object(neutron_ovs_context, 'NeutronAPIContext') @patch.object(nutils, 'BridgePortInterfaceMap') @patch.object(nutils, 'OVSDPDKDeviceContext') + @patch('charmhelpers.contrib.network.ovs.charm_name') @patch.object(nutils, 'use_dvr') @patch('charmhelpers.contrib.openstack.context.config') - def test_configure_ovs_dpdk(self, mock_config, _use_dvr, + def test_configure_ovs_dpdk(self, mock_config, _use_dvr, _charm_name, _OVSDPDKDeviceContext, _BridgePortInterfaceMap, _NeutronAPIContext, @@ -857,7 +939,7 @@ class TestNeutronOVSUtils(CharmTestCase): _use_hw_offload): _NeutronAPIContext.return_value = DummyContext( return_value={'global_physnet_mtu': 1500}) - return self._run_configure_ovs_dpdk(mock_config, _use_dvr, + return self._run_configure_ovs_dpdk(mock_config, _use_dvr, _charm_name, _OVSDPDKDeviceContext, _BridgePortInterfaceMap, _parse_data_port_mappings, @@ -871,9 +953,11 @@ class TestNeutronOVSUtils(CharmTestCase): @patch.object(neutron_ovs_context, 'NeutronAPIContext') @patch.object(nutils, 'BridgePortInterfaceMap') @patch.object(nutils, 'OVSDPDKDeviceContext') + @patch('charmhelpers.contrib.network.ovs.charm_name') @patch.object(nutils, 'use_dvr') @patch('charmhelpers.contrib.openstack.context.config') def test_configure_ovs_dpdk_late_init(self, mock_config, _use_dvr, + _charm_name, _OVSDPDKDeviceContext, _BridgePortInterfaceMap, _NeutronAPIContext, @@ -882,7 +966,7 @@ class TestNeutronOVSUtils(CharmTestCase): _use_hw_offload): _NeutronAPIContext.return_value = DummyContext( return_value={'global_physnet_mtu': 1500}) - return self._run_configure_ovs_dpdk(mock_config, _use_dvr, + return self._run_configure_ovs_dpdk(mock_config, _use_dvr, _charm_name, _OVSDPDKDeviceContext, _BridgePortInterfaceMap, _parse_data_port_mappings, @@ -896,9 +980,11 @@ class TestNeutronOVSUtils(CharmTestCase): @patch.object(neutron_ovs_context, 'NeutronAPIContext') @patch.object(nutils, 'BridgePortInterfaceMap') @patch.object(nutils, 'OVSDPDKDeviceContext') + @patch('charmhelpers.contrib.network.ovs.charm_name') @patch.object(nutils, 'use_dvr') @patch('charmhelpers.contrib.openstack.context.config') def test_configure_ovs_dpdk_late_init_bonds(self, mock_config, _use_dvr, + _charm_name, _OVSDPDKDeviceContext, _BridgePortInterfaceMap, _NeutronAPIContext, @@ -907,7 +993,7 @@ class TestNeutronOVSUtils(CharmTestCase): _use_hw_offload): _NeutronAPIContext.return_value = DummyContext( return_value={'global_physnet_mtu': 1500}) - return self._run_configure_ovs_dpdk(mock_config, _use_dvr, + return self._run_configure_ovs_dpdk(mock_config, _use_dvr, _charm_name, _OVSDPDKDeviceContext, _BridgePortInterfaceMap, _parse_data_port_mappings, @@ -916,9 +1002,12 @@ class TestNeutronOVSUtils(CharmTestCase): _test_bonds=True) @patch.object(nutils, 'use_dvr') + @patch('charmhelpers.contrib.network.ovs.charm_name') @patch('charmhelpers.contrib.openstack.context.config') - def test_configure_ovs_enable_ipfix(self, mock_config, mock_use_dvr): + def test_configure_ovs_enable_ipfix(self, mock_config, mock_charm_name, + mock_use_dvr): mock_use_dvr.return_value = False + mock_charm_name.return_value = "neutron-openvswitch" mock_config.side_effect = self.test_config.get self.config.side_effect = self.test_config.get self.test_config.set('ipfix-target', '127.0.0.1:80')