Move _ovs_additional_ids() and sequence_functions() to charmhelpers

Sync charmhelpers.

Replace _ovs_additional_ids() calls with generate_external_ids() from
charmhelpers.

Replace sequence_functions() with sequence_status_check_functions()
from charmhelpers.

This allows to share helper functions between charm-neutron-gateway
and charm-neutron-openvswitch.

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

hooks/charmhelpers/contrib/charmsupport/nrpe.py

@@ -337,10 +337,8 @@ class NRPE(object):
                 "command": nrpecheck.command,
             }
             # If we were passed max_check_attempts, add that to the relation data
-            try:
+            if nrpecheck.max_check_attempts is not None:
                 nrpe_monitors[nrpecheck.shortname]['max_check_attempts'] = nrpecheck.max_check_attempts
-            except AttributeError:
-                pass
 
         # update-status hooks are configured to firing every 5 minutes by
         # default. When nagios-nrpe-server is restarted, the nagios server

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/utils.py

@@ -483,9 +483,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 +596,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')
 
 
@@ -1717,7 +1748,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 +1777,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 +1814,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
 
@@ -2418,3 +2474,26 @@ 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

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

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
@@ -730,37 +730,84 @@ def restart_on_change(restart_map, stopstart=False, restart_functions=None):
 
 
 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 returna 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
     """
     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, 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)
+            if post_svc_restart_f:
+                post_svc_restart_f(service_name)
     return r
 
 
@@ -1068,6 +1115,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 +1141,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
     """
+    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
+    # 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.
-    _run_with_retries(['add-apt-repository', '--yes', spec],
+    if series in ('trusty', 'xenial'):
-                      cmd_env=env_proxy_settings(['https', 'http']))
+        _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_utils.py

@@ -41,6 +41,7 @@ from charmhelpers.contrib.network.ovs import (
     full_restart,
     get_bridges_and_ports_map,
     is_linuxbridge_interface,
+    generate_external_ids,
 )
 from charmhelpers.contrib.hahelpers.cluster import (
     get_hacluster_config,
@@ -55,7 +56,7 @@ from charmhelpers.contrib.openstack.utils import (
     pause_unit,
     reset_os_release,
     resume_unit,
-    workload_state_compare,
+    sequence_status_check_functions,
 )
 
 from charmhelpers.contrib.openstack.neutron import (
@@ -867,8 +868,8 @@ def configure_ovs():
             .format(", ".join("{}: {}".format(b, ",".join(v))
                               for b, v in current_bridges_and_ports.items())))
 
-        add_bridge(INT_BRIDGE, brdata=_ovs_additional_data())
+        add_bridge(INT_BRIDGE, brdata=generate_external_ids())
-        add_bridge(EXT_BRIDGE, brdata=_ovs_additional_data())
+        add_bridge(EXT_BRIDGE, brdata=generate_external_ids())
 
         ext_port_ctx = ExternalPortContext()()
         portmaps = DataPortContext()()
@@ -884,15 +885,15 @@ def configure_ovs():
         if not portmaps and ext_port_ctx and ext_port_ctx['ext_port']:
             _port = ext_port_ctx['ext_port']
             add_bridge_port(EXT_BRIDGE, _port,
-                            ifdata=_ovs_additional_data(EXT_BRIDGE),
+                            ifdata=generate_external_ids(EXT_BRIDGE),
-                            portdata=_ovs_additional_data(EXT_BRIDGE))
+                            portdata=generate_external_ids(EXT_BRIDGE))
             log("DEPRECATION: using ext-port to set the port {} on the "
                 "EXT_BRIDGE ({}) is deprecated.  Please use data-port instead."
                 .format(_port, EXT_BRIDGE),
                 level=WARNING)
 
         for br in bridgemaps.values():
-            add_bridge(br, brdata=_ovs_additional_data())
+            add_bridge(br, brdata=generate_external_ids())
             if not portmaps:
                 continue
 
@@ -900,14 +901,14 @@ def configure_ovs():
                 if _br == br:
                     if not is_linuxbridge_interface(port):
                         add_bridge_port(br, port, promisc=True,
-                                        ifdata=_ovs_additional_data(br),
+                                        ifdata=generate_external_ids(br),
-                                        portdata=_ovs_additional_data(br))
+                                        portdata=generate_external_ids(br))
                     else:
                         # NOTE(lourot): this will raise on focal+ and/or if the
                         # system has no `ifup`. See lp:1877594
                         add_ovsbridge_linuxbridge(
-                            br, port, ifdata=_ovs_additional_data(br),
+                            br, port, ifdata=generate_external_ids(br),
-                            portdata=_ovs_additional_data(br))
+                            portdata=generate_external_ids(br))
 
         target = config('ipfix-target')
         bridges = [INT_BRIDGE, EXT_BRIDGE]
@@ -934,31 +935,6 @@ def configure_ovs():
         service_restart('os-charm-phy-nic-mtu')
 
 
-def _ovs_additional_data(external_id_value=None):
-    """Returns OVS additional data from the given external-id value.
-
-    The returned value is in the input format expected by the
-    charmhelpers.contrib.network.ovs functions.
-
-    We set an external-id as OVS additional data on all the bridges and ports
-    that we create in order to mark them as managed by us. See
-    http://docs.openvswitch.org/en/latest/topics/integration/
-
-    :param external_id_value: Value to be set as external ID. For a bridge,
-        typically 'managed' (which is also the default if nothing is passed).
-        For a port, typically the name of the corresponding bridge.
-    :type external_id_value: str
-    :rtype: Dict[str,Dict[str,str]]
-    """
-    external_id_value = ('managed' if external_id_value is None
-                         else external_id_value)
-    return {
-        'external-ids': {
-            'charm-neutron-gateway': external_id_value
-        }
-    }
-
-
 def copy_file(src, dst, perms=None, force=False):
     """Copy file to destination and optionally set permissionss.
 
@@ -1158,29 +1134,6 @@ def check_ext_port_data_port_config(configs):
     return 'unknown', ''
 
 
-def sequence_functions(*functions):
-    """Sequence the functions passed so that they all get a chance to run as
-    the check_charm_func for the assess_status.
-
-    :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
-
-
 def assess_status(configs):
     """Assess status of current unit
     Decides what the state of the unit should be based on the current
@@ -1216,8 +1169,8 @@ def assess_status_func(configs):
     required_interfaces = REQUIRED_INTERFACES.copy()
     required_interfaces.update(get_optional_interfaces())
     active_services = [s for s in services() if s not in STOPPED_SERVICES]
-    charm_func = sequence_functions(check_optional_relations,
+    charm_func = sequence_status_check_functions(
-                                    check_ext_port_data_port_config)
+        check_optional_relations, check_ext_port_data_port_config)
     return make_assess_status_func(
         configs, required_interfaces,
         charm_func=charm_func,

unit_tests/test_neutron_utils.py

@@ -275,8 +275,11 @@ class TestNeutronUtils(CharmTestCase):
         self.os_release.return_value = 'juno'
         self.assertTrue('keepalived' in neutron_utils.get_packages())
 
+    @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(
+            self, mock_config, charm_name):
+        charm_name.return_value = "neutron-gateway"
         mock_config.side_effect = self.test_config.get
         self.config.return_value = 'ovs'
         self.service_running.return_value = False
@@ -288,8 +291,10 @@ class TestNeutronUtils(CharmTestCase):
         neutron_utils.configure_ovs()
         self.assertFalse(self.full_restart.called)
 
+    @patch('charmhelpers.contrib.network.ovs.charm_name')
     @patch('charmhelpers.contrib.openstack.context.config')
-    def test_configure_ovs_ovs_ext_port(self, mock_config):
+    def test_configure_ovs_ovs_ext_port(self, mock_config, charm_name):
+        charm_name.return_value = "neutron-gateway"
         mock_config.side_effect = self.test_config.get
         self.config.side_effect = self.test_config.get
         self.test_config.set('plugin', 'ovs')
@@ -308,8 +313,10 @@ class TestNeutronUtils(CharmTestCase):
 
     @patch('charmhelpers.contrib.openstack.context.list_nics',
            return_value=['eth0', 'eth0.100', 'eth0.200'])
+    @patch('charmhelpers.contrib.network.ovs.charm_name')
     @patch('charmhelpers.contrib.openstack.context.config')
-    def test_configure_ovs_ovs_data_port(self, mock_config, _nics):
+    def test_configure_ovs_ovs_data_port(self, mock_config, charm_name, _nics):
+        charm_name.return_value = "neutron-gateway"
         self.is_linuxbridge_interface.return_value = False
         mock_config.side_effect = self.test_config.get
         self.config.side_effect = self.test_config.get
@@ -360,8 +367,11 @@ class TestNeutronUtils(CharmTestCase):
 
     @patch('charmhelpers.contrib.openstack.context.list_nics',
            return_value=['br-eth0'])
+    @patch('charmhelpers.contrib.network.ovs.charm_name')
     @patch('charmhelpers.contrib.openstack.context.config')
-    def test_configure_ovs_ovs_data_port_bridge(self, mock_config, _nics):
+    def test_configure_ovs_ovs_data_port_bridge(
+            self, mock_config, charm_name, _nics):
+        charm_name.return_value = "neutron-gateway"
         self.is_linuxbridge_interface.return_value = True
         mock_config.side_effect = self.test_config.get
         self.config.side_effect = self.test_config.get
@@ -391,8 +401,10 @@ class TestNeutronUtils(CharmTestCase):
                       portdata=expected_portdata)]
         self.add_ovsbridge_linuxbridge.assert_has_calls(calls)
 
+    @patch('charmhelpers.contrib.network.ovs.charm_name')
     @patch('charmhelpers.contrib.openstack.context.config')
-    def test_configure_ovs_enable_ipfix(self, mock_config):
+    def test_configure_ovs_enable_ipfix(self, mock_config, charm_name):
+        charm_name.return_value = "neutron-gateway"
         mock_config.side_effect = self.test_config.get
         self.config.side_effect = self.test_config.get
         self.test_config.set('plugin', 'ovs')
@@ -405,9 +417,11 @@ class TestNeutronUtils(CharmTestCase):
         ])
 
     @patch.object(neutron_utils, 'DataPortContext')
+    @patch('charmhelpers.contrib.network.ovs.charm_name')
     @patch('charmhelpers.contrib.openstack.context.config')
     def test_configure_ovs_ensure_ext_port_overriden(
-            self, mock_config, mock_data_port_context):
+            self, mock_config, charm_name, mock_data_port_context):
+        charm_name.return_value = "neutron-gateway"
         mock_config.side_effect = self.test_config.get
         self.config.side_effect = self.test_config.get
         self.test_config.set('plugin', 'ovs')
@@ -1090,7 +1104,7 @@ class TestNeutronAgentReallocation(CharmTestCase):
             )
 
     @patch.object(neutron_utils, 'get_optional_interfaces')
-    @patch.object(neutron_utils, 'sequence_functions')
+    @patch.object(neutron_utils, 'sequence_status_check_functions')
     @patch.object(neutron_utils, 'REQUIRED_INTERFACES')
     @patch.object(neutron_utils, 'services')
     @patch.object(neutron_utils, 'make_assess_status_func')