Set external-ids for managed OVS bridges and ports

Enable marking of OVS bridges and ports as managed by
charm-neutron-openvswitch. This enables more advanced use cases like
implmentation of idempotent config changes or cleanup of stale OVS
configuration.

Include functional test case that verifies whether external-ids are
properly set up on ports and bridges.

Sync charmhelpers.

Closes-Bug: #1917025
Partial-Bug: #1915967

Signed-off-by: Przemysław Lal <przemyslaw.lal@canonical.com>
Change-Id: I343f2c8258b7b8b91417dc6efc5bfe95351392a9

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.
-

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
+        }
+    }

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
 )

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(
+        name=get_cert_relation_ca_name(cert_relation_id))
-            remote_service_name(relid=cert_relation_id)))
 
 
 def process_certificates(service_name, relation_id, unit,

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
+        #                  worker configuration on large servers
-        #                  on large servers
         # Reference: https://pad.lv/1665270
         count = min(count, MAX_DEFAULT_WORKERS)
 

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])

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

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)

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] <initscript ID> <actions>
+
+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)

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)
+    """Determine the OpenStack release code name from openstack-release pkg.
-    print("OpenStack Release: {}".format(openstack_release()))
+
+    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 f: function to decorate.
-    @param restart_map: (optionally callable, which then returns the
+    :type f: Callable
-        restart_map) the restart map {conf_file: [services]}
+    :param restart_map: Optionally callable, which then returns the restart_map or
-    @param stopstart: DEFAULT false; whether to stop, start or just restart
+                        the restart map {conf_file: [services]}
-    @returns decorator to use a restart_on_change with pausability
+    :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'],
+                (lambda: f(*args, **kwargs)),
-                stopstart, restart_functions)
+                __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))})

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

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):
+class restart_on_change(object):
-    """Restart services based on configuration files changing
+    """Decorator and context manager to handle restarts.
 
-    This function is used a decorator, for example::
+    Usage:
 
-        @restart_on_change({
+       @restart_on_change(restart_map, ...)
-            '/etc/ceph/ceph.conf': [ 'cinder-api', 'cinder-volume' ]
+       def function_that_might_trigger_a_restart(...)
-            '/etc/apache/sites-enabled/*': [ 'apache2' ]
+           ...
-            })
-        def config_changed():
-            pass  # your code here
 
-    In this example, the cinder-api and cinder-volume services
+    Or:
-    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.
 
-    @param restart_map: {path_file_name: [service_name, ...]
+       with restart_on_change(restart_map, ...):
-    @param stopstart: DEFAULT false; whether to stop, start OR restart
+           do_stuff_that_might_trigger_a_restart()
-    @param restart_functions: nonstandard functions to use to restart services
+           ...
-                              {svc: func, ...}
+    """
-    @returns result from decorated function
+
+    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]
         """
-    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_functions)
+                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.
+    This functions allows for a number of helper functions to be passed.
-    @param restart_map: {file: [service, ...]}
+
-    @param stopstart: whether to stop, start or restart a service
+    `restart_functions` is a map with a service as the key and the
-    @param restart_functions: nonstandard functions to use to restart services
+    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}
+    changed_files = defaultdict(list)
-    r = lambda_f()
+    restarts = []
     # create a list of lists of the services to restart
-    restarts = [restart_map[path]
+    for path, services in restart_map.items():
-                for path in restart_map
+        if path_hash(path) != checksums[path]:
-                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

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'
+        @deprecate(
-            'with charm-helpers is deprecated since 2019-06-25', level=WARNING)
+            '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,
+    if not quiet:
-                                                options))
+        log("Installing {} with options: {}"
-    _run_apt_command(cmd, fatal)
+            .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
     """
-    if '{series}' in spec:
     series = get_distrib_codename()
+    if '{series}' in spec:
         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
+    # set via apt.conf (see lp:1433761), however this is not the case for LTS
-    # LTS and non-LTS releases below bionic.
+    # 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):

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),
+                            portdata=portdata, ifdatamap=ifdatamap)
-                            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,
+                        mac.entity, ifname, ifdata=ifdata,
-                        promisc=None)
+                        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),
+      services=services(exclude_services), ports=None)
-      ports=None)

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:

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,
+    def test_configure_ovs_starts_service_if_required(
-                                                      _use_dvr):
+            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')