diff --git a/charms/ovn-central-k8s/osci.yaml b/charms/ovn-central-k8s/osci.yaml index 6d7642cc..f2674018 100644 --- a/charms/ovn-central-k8s/osci.yaml +++ b/charms/ovn-central-k8s/osci.yaml @@ -7,4 +7,4 @@ build_type: charmcraft publish_charm: true charmcraft_channel: 2.0/stable - publish_channel: latest/edge + publish_channel: 21.09/edge diff --git a/charms/ovn-central-k8s/pyproject.toml b/charms/ovn-central-k8s/pyproject.toml new file mode 100644 index 00000000..2896bc05 --- /dev/null +++ b/charms/ovn-central-k8s/pyproject.toml @@ -0,0 +1,39 @@ +# Copyright 2022 Canonical Ltd. +# See LICENSE file for licensing details. + +# Testing tools configuration +[tool.coverage.run] +branch = true + +[tool.coverage.report] +show_missing = true + +[tool.pytest.ini_options] +minversion = "6.0" +log_cli_level = "INFO" + +# Formatting tools configuration +[tool.black] +line-length = 79 + +[tool.isort] +profile = "black" +multi_line_output = 3 +force_grid_wrap = true + +# Linting tools configuration +[tool.flake8] +max-line-length = 79 +max-doc-length = 99 +max-complexity = 10 +exclude = [".git", "__pycache__", ".tox", "build", "dist", "*.egg_info", "venv"] +select = ["E", "W", "F", "C", "N", "R", "D", "H"] +# Ignore W503, E501 because using black creates errors with this +# Ignore D107 Missing docstring in __init__ +ignore = ["W503", "E501", "D107", "E402"] +per-file-ignores = [] +docstring-convention = "google" +# Check for properly formatted copyright header in each file +copyright-check = "True" +copyright-author = "Canonical Ltd." +copyright-regexp = "Copyright\\s\\d{4}([-,]\\d{4})*\\s+%(author)s" diff --git a/charms/ovn-central-k8s/src/charm.py b/charms/ovn-central-k8s/src/charm.py index bda7b945..9ce5eb11 100755 --- a/charms/ovn-central-k8s/src/charm.py +++ b/charms/ovn-central-k8s/src/charm.py @@ -1,30 +1,50 @@ #!/usr/bin/env python3 + +# Copyright 2022 Canonical Ltd. +# +# 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 + """OVN Central Operator Charm. This charm provide Glance services as part of an OpenStack deployment """ -import ovn -import ovsdb as ch_ovsdb import logging -from typing import List, Mapping - -import ops.charm -from ops.framework import StoredState -from ops.main import main - -import ops_sunbeam.charm as sunbeam_charm -import ops_sunbeam.core as sunbeam_core -import ops_sunbeam.relation_handlers as sunbeam_rhandlers -import ops_sunbeam.config_contexts as sunbeam_ctxts -import ops_sunbeam.ovn.container_handlers as ovn_chandlers -import ops_sunbeam.ovn.config_contexts as ovn_ctxts -import ops_sunbeam.ovn.relation_handlers as ovn_rhandlers +from typing import ( + List, + Mapping, +) import charms.ovn_central_k8s.v0.ovsdb as ovsdb +import ops.charm +import ops_sunbeam.charm as sunbeam_charm +import ops_sunbeam.config_contexts as sunbeam_ctxts +import ops_sunbeam.core as sunbeam_core +import ops_sunbeam.ovn.config_contexts as ovn_ctxts +import ops_sunbeam.ovn.container_handlers as ovn_chandlers +import ops_sunbeam.ovn.relation_handlers as ovn_rhandlers +import ops_sunbeam.relation_handlers as sunbeam_rhandlers +from charms.observability_libs.v0.kubernetes_service_patch import ( + KubernetesServicePatch, +) +from ops.framework import ( + StoredState, +) +from ops.main import ( + main, +) -from charms.observability_libs.v0.kubernetes_service_patch \ - import KubernetesServicePatch +import ovn +import ovsdb as ch_ovsdb logger = logging.getLogger(__name__) @@ -35,52 +55,62 @@ OVN_DB_CONTAINERS = [OVN_SB_DB_CONTAINER, OVN_NB_DB_CONTAINER] class OVNNorthBPebbleHandler(ovn_chandlers.OVNPebbleHandler): + """Handler for North OVN DB.""" @property def wrapper_script(self): - return '/root/ovn-northd-wrapper.sh' + """Wrapper script for managing OVN service.""" + return "/root/ovn-northd-wrapper.sh" @property def status_command(self): - return '/usr/share/ovn/scripts/ovn-ctl status_northd' + """Status command for container.""" + return "/usr/share/ovn/scripts/ovn-ctl status_northd" @property def service_description(self): - return 'OVN Northd' + """Description of service.""" + return "OVN Northd" def default_container_configs(self): + """Config files for container.""" _cc = super().default_container_configs() _cc.append( sunbeam_core.ContainerConfigFile( - '/etc/ovn/ovn-northd-db-params.conf', - 'root', - 'root')) + "/etc/ovn/ovn-northd-db-params.conf", "root", "root" + ) + ) return _cc class OVNNorthBDBPebbleHandler(ovn_chandlers.OVNPebbleHandler): + """Handler for North-bound OVN DB.""" @property def wrapper_script(self): - return '/root/ovn-nb-db-server-wrapper.sh' + """Wrapper script for managing OVN service.""" + return "/root/ovn-nb-db-server-wrapper.sh" @property def status_command(self): + """Status command for container.""" # This command always return 0 even if the DB service # is not running, so adding healthcheck with tcp check - return '/usr/share/ovn/scripts/ovn-ctl status_ovsdb' + return "/usr/share/ovn/scripts/ovn-ctl status_ovsdb" @property def service_description(self): - return 'OVN North Bound DB' + """Description of service.""" + return "OVN North Bound DB" def default_container_configs(self): + """Config files for container.""" _cc = super().default_container_configs() _cc.append( sunbeam_core.ContainerConfigFile( - '/root/ovn-nb-cluster-join.sh', - 'root', - 'root')) + "/root/ovn-nb-cluster-join.sh", "root", "root" + ) + ) return _cc def get_healthcheck_layer(self) -> dict: @@ -94,37 +124,40 @@ class OVNNorthBDBPebbleHandler(ovn_chandlers.OVNPebbleHandler): "online": { "override": "replace", "level": "ready", - "tcp": { - "port": 6641 - } + "tcp": {"port": 6641}, }, } } class OVNSouthBDBPebbleHandler(ovn_chandlers.OVNPebbleHandler): + """Handler for South-bound OVN DB.""" @property def wrapper_script(self): - return '/root/ovn-sb-db-server-wrapper.sh' + """Wrapper script for managing OVN service.""" + return "/root/ovn-sb-db-server-wrapper.sh" @property def status_command(self): + """Status command for container.""" # This command always return 0 even if the DB service # is not running, so adding healthcheck with tcp check - return '/usr/share/ovn/scripts/ovn-ctl status_ovsdb' + return "/usr/share/ovn/scripts/ovn-ctl status_ovsdb" @property def service_description(self): - return 'OVN South Bound DB' + """Description of service.""" + return "OVN South Bound DB" def default_container_configs(self): + """Config files for container.""" _cc = super().default_container_configs() _cc.append( sunbeam_core.ContainerConfigFile( - '/root/ovn-sb-cluster-join.sh', - 'root', - 'root')) + "/root/ovn-sb-cluster-join.sh", "root", "root" + ) + ) return _cc def get_healthcheck_layer(self) -> dict: @@ -138,9 +171,7 @@ class OVNSouthBDBPebbleHandler(ovn_chandlers.OVNPebbleHandler): "online": { "override": "replace", "level": "ready", - "tcp": { - "port": 6642 - } + "tcp": {"port": 6642}, }, } } @@ -150,66 +181,69 @@ class OVNCentralOperatorCharm(sunbeam_charm.OSBaseOperatorCharm): """Charm the service.""" _state = StoredState() - mandatory_relations = { - 'certificates', - 'peers' - } + mandatory_relations = {"certificates", "peers"} def __init__(self, framework): + """Setup OVN central charm class.""" super().__init__(framework) self.service_patcher = KubernetesServicePatch( self, [ - ('northbound', 6641), - ('southbound', 6642), - ] + ("northbound", 6641), + ("southbound", 6642), + ], ) def get_pebble_handlers(self): + """Pebble handlers for all OVN containers.""" pebble_handlers = [ OVNNorthBPebbleHandler( self, OVN_NORTHD_CONTAINER, - 'ovn-northd', + "ovn-northd", self.container_configs, self.template_dir, - self.openstack_release, - self.configure_charm), + self.configure_charm, + ), OVNSouthBDBPebbleHandler( self, OVN_SB_DB_CONTAINER, - 'ovn-sb-db-server', + "ovn-sb-db-server", self.container_configs, self.template_dir, - self.openstack_release, - self.configure_charm), + self.configure_charm, + ), OVNNorthBDBPebbleHandler( self, OVN_NB_DB_CONTAINER, - 'ovn-nb-db-server', + "ovn-nb-db-server", self.container_configs, self.template_dir, - self.openstack_release, - self.configure_charm)] + self.configure_charm, + ), + ] return pebble_handlers - def get_relation_handlers(self, handlers=None) -> List[ - sunbeam_rhandlers.RelationHandler]: + def get_relation_handlers( + self, handlers=None + ) -> List[sunbeam_rhandlers.RelationHandler]: """Relation handlers for the service.""" handlers = handlers or [] - if self.can_add_handler('peers', handlers): + if self.can_add_handler("peers", handlers): self.peers = ovn_rhandlers.OVNDBClusterPeerHandler( self, - 'peers', + "peers", self.configure_charm, - 'peers' in self.mandatory_relations) + "peers" in self.mandatory_relations, + ) handlers.append(self.peers) - if self.can_add_handler('ovsdb-cms', handlers): + if self.can_add_handler("ovsdb-cms", handlers): self.ovsdb_cms = ovn_rhandlers.OVSDBCMSProvidesHandler( self, - 'ovsdb-cms', + "ovsdb-cms", self.configure_charm, - 'ovsdb-cms' in self.mandatory_relations) + "ovsdb-cms" in self.mandatory_relations, + ) handlers.append(self.ovsdb_cms) handlers = super().get_relation_handlers(handlers) return handlers @@ -218,8 +252,7 @@ class OVNCentralOperatorCharm(sunbeam_charm.OSBaseOperatorCharm): def config_contexts(self) -> List[sunbeam_ctxts.ConfigContext]: """Configuration contexts for the operator.""" contexts = super().config_contexts - contexts.append( - ovn_ctxts.OVNDBConfigContext(self, "ovs_db")) + contexts.append(ovn_ctxts.OVNDBConfigContext(self, "ovs_db")) return contexts @property @@ -232,18 +265,19 @@ class OVNCentralOperatorCharm(sunbeam_charm.OSBaseOperatorCharm): return {} def ovn_rundir(self): - return '/var/run/ovn' + """OVN run dir.""" + return "/var/run/ovn" def get_pebble_executor(self, container_name): - container = self.unit.get_container( - container_name) + """Execute command in pebble.""" + container = self.unit.get_container(container_name) def _run_via_pebble(*args): - process = container.exec(list(args), timeout=5*60) + process = container.exec(list(args), timeout=5 * 60) out, warnings = process.wait_output() if warnings: for line in warnings.splitlines(): - logger.warning('CMD Out: %s', line.strip()) + logger.warning("CMD Out: %s", line.strip()) return out return _run_via_pebble @@ -261,12 +295,13 @@ class OVNCentralOperatorCharm(sunbeam_charm.OSBaseOperatorCharm): # is clustered and while units are paused, so we need to handle # errors from this call gracefully. return ovn.cluster_status( - db, - rundir=self.ovn_rundir(), - cmd_executor=cmd_executor) + db, rundir=self.ovn_rundir(), cmd_executor=cmd_executor + ) except (ValueError) as e: - logging.error('Unable to get cluster status, ovsdb-server ' - 'not ready yet?: {}'.format(e)) + logging.error( + "Unable to get cluster status, ovsdb-server " + "not ready yet?: {}".format(e) + ) return def configure_ovn_listener(self, db, port_map): @@ -278,65 +313,79 @@ class OVNCentralOperatorCharm(sunbeam_charm.OSBaseOperatorCharm): :type port_map: Dict[int,Dict[str,str]] :raises: ValueError """ - if db == 'nb': + if db == "nb": executor = self.get_pebble_executor(OVN_NB_DB_CONTAINER) - elif db == 'sb': + elif db == "sb": executor = self.get_pebble_executor(OVN_SB_DB_CONTAINER) status = self.cluster_status( - 'ovn{}_db'.format(db), - cmd_executor=executor) + "ovn{}_db".format(db), cmd_executor=executor + ) if status and status.is_cluster_leader: logging.debug( - 'configure_ovn_listener is_cluster_leader {}'.format(db)) + "configure_ovn_listener is_cluster_leader {}".format(db) + ) connections = ch_ovsdb.SimpleOVSDB( - 'ovn-{}ctl'.format(db), - cmd_executor=executor).connection + "ovn-{}ctl".format(db), cmd_executor=executor + ).connection for port, settings in port_map.items(): - logging.debug('port {} {}'.format(port, settings)) + logging.debug("port {} {}".format(port, settings)) # discover and create any non-existing listeners first for connection in connections.find( - 'target="pssl:{}"'.format(port)): - logging.debug('Found port {}'.format(port)) + 'target="pssl:{}"'.format(port) + ): + logging.debug("Found port {}".format(port)) break else: - logging.debug('Create port {}'.format(port)) + logging.debug("Create port {}".format(port)) executor( - 'ovn-{}ctl'.format(db), - '--', - '--id=@connection', - 'create', 'connection', + "ovn-{}ctl".format(db), + "--", + "--id=@connection", + "create", + "connection", 'target="pssl:{}"'.format(port), - '--', - 'add', '{}_Global'.format(db.upper()), - '.', 'connections', '@connection') + "--", + "add", + "{}_Global".format(db.upper()), + ".", + "connections", + "@connection", + ) # set/update connection settings for connection in connections.find( - 'target="pssl:{}"'.format(port)): + 'target="pssl:{}"'.format(port) + ): for k, v in settings.items(): logging.debug( - 'set {} {} {}' - .format(str(connection['_uuid']), k, v)) - connections.set(str(connection['_uuid']), k, v) + "set {} {} {}".format( + str(connection["_uuid"]), k, v + ) + ) + connections.set(str(connection["_uuid"]), k, v) - def configure_charm(self, event: ops.framework.EventBase) -> None: - """Catchall handler to configure charm services. - - """ + # Refactor this method to simplify it. + def configure_charm( # noqa: C901 + self, event: ops.framework.EventBase + ) -> None: + """Catchall handler to configure charm services.""" if not self.unit.is_leader(): if not self.is_leader_ready(): self.unit.status = ops.model.WaitingStatus( - "Waiting for leader to be ready") + "Waiting for leader to be ready" + ) return missing_leader_data = [ - k for k in ['nb_cid', 'sb_cid'] - if not self.leader_get(k)] + k for k in ["nb_cid", "sb_cid"] if not self.leader_get(k) + ] if missing_leader_data: logging.debug(f"missing {missing_leader_data} from leader") self.unit.status = ops.model.WaitingStatus( - "Waiting for data from leader") + "Waiting for data from leader" + ) return logging.debug( - "Remote leader is ready and has supplied all data needed") + "Remote leader is ready and has supplied all data needed" + ) if not self.relation_handlers_ready(): logging.debug("Aborting charm relations not ready") @@ -344,7 +393,8 @@ class OVNCentralOperatorCharm(sunbeam_charm.OSBaseOperatorCharm): if not all([ph.pebble_ready for ph in self.pebble_handlers]): logging.debug( - "Aborting configuration, not all pebble handlers are ready") + "Aborting configuration, not all pebble handlers are ready" + ) return # Render Config in all containers but init should *NOT* start @@ -356,7 +406,8 @@ class OVNCentralOperatorCharm(sunbeam_charm.OSBaseOperatorCharm): else: logging.debug( f"Not running init for {ph.service_name}," - " container not ready") + " container not ready" + ) if self.unit.is_leader(): # Start services in North/South containers on lead unit @@ -366,35 +417,39 @@ class OVNCentralOperatorCharm(sunbeam_charm.OSBaseOperatorCharm): # Attempt to setup listers etc self.configure_ovn() nb_status = self.cluster_status( - 'ovnnb_db', - self.get_pebble_executor(OVN_NB_DB_CONTAINER)) + "ovnnb_db", self.get_pebble_executor(OVN_NB_DB_CONTAINER) + ) sb_status = self.cluster_status( - 'ovnsb_db', - self.get_pebble_executor(OVN_SB_DB_CONTAINER)) + "ovnsb_db", self.get_pebble_executor(OVN_SB_DB_CONTAINER) + ) logging.debug("Telling peers leader is ready and cluster ids") self.set_leader_ready() - self.leader_set({ - 'nb_cid': str(nb_status.cluster_id), - 'sb_cid': str(sb_status.cluster_id), - }) + self.leader_set( + { + "nb_cid": str(nb_status.cluster_id), + "sb_cid": str(sb_status.cluster_id), + } + ) else: logging.debug("Attempting to join OVN_Northbound cluster") container = self.unit.get_container(OVN_NB_DB_CONTAINER) process = container.exec( - ['bash', '/root/ovn-nb-cluster-join.sh'], timeout=5*60) + ["bash", "/root/ovn-nb-cluster-join.sh"], timeout=5 * 60 + ) out, warnings = process.wait_output() if warnings: for line in warnings.splitlines(): - logger.warning('CMD Out: %s', line.strip()) + logger.warning("CMD Out: %s", line.strip()) logging.debug("Attempting to join OVN_Southbound cluster") container = self.unit.get_container(OVN_SB_DB_CONTAINER) process = container.exec( - ['bash', '/root/ovn-sb-cluster-join.sh'], timeout=5*60) + ["bash", "/root/ovn-sb-cluster-join.sh"], timeout=5 * 60 + ) out, warnings = process.wait_output() if warnings: for line in warnings.splitlines(): - logger.warning('CMD Out: %s', line.strip()) + logger.warning("CMD Out: %s", line.strip()) logging.debug("Starting services in DB containers") for ph in self.get_named_pebble_handlers(OVN_DB_CONTAINERS): ph.start_service() @@ -414,34 +469,37 @@ class OVNCentralOperatorCharm(sunbeam_charm.OSBaseOperatorCharm): self.unit.status = ops.model.ActiveStatus() def configure_ovn(self): - inactivity_probe = int( - self.config['ovsdb-server-inactivity-probe']) * 1000 + """Configure ovn listener.""" + inactivity_probe = ( + int(self.config["ovsdb-server-inactivity-probe"]) * 1000 + ) self.configure_ovn_listener( - 'nb', { + "nb", + { self.ovsdb_cms.db_nb_port: { - 'inactivity_probe': inactivity_probe, + "inactivity_probe": inactivity_probe, }, - }) + }, + ) self.configure_ovn_listener( - 'sb', { + "sb", + { self.ovsdb_cms.db_sb_port: { - 'inactivity_probe': inactivity_probe, + "inactivity_probe": inactivity_probe, }, - }) + }, + ) self.configure_ovn_listener( - 'sb', { + "sb", + { self.ovsdb_cms.db_sb_admin_port: { - 'inactivity_probe': inactivity_probe, + "inactivity_probe": inactivity_probe, }, - }) - - -class OVNCentralXenaOperatorCharm(OVNCentralOperatorCharm): - - openstack_release = 'xena' + }, + ) if __name__ == "__main__": # Note: use_juju_for_storage=True required per # https://github.com/canonical/operator/issues/506 - main(OVNCentralXenaOperatorCharm, use_juju_for_storage=True) + main(OVNCentralOperatorCharm, use_juju_for_storage=True) diff --git a/charms/ovn-central-k8s/src/ovn.py b/charms/ovn-central-k8s/src/ovn.py index 4aed794f..153c6185 100644 --- a/charms/ovn-central-k8s/src/ovn.py +++ b/charms/ovn-central-k8s/src/ovn.py @@ -1,4 +1,4 @@ -# Copyright 2019 Canonical Ltd +# Copyright 2022 Canonical Ltd. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -11,19 +11,22 @@ # 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. + +"""OVN utilities.""" + import os import subprocess import uuid import utils - -OVN_RUNDIR = '/var/run/ovn' -OVN_SYSCONFDIR = '/etc/ovn' +OVN_RUNDIR = "/var/run/ovn" +OVN_SYSCONFDIR = "/etc/ovn" -def ovn_appctl(target, args, rundir=None, use_ovs_appctl=False, - cmd_executor=None): +def ovn_appctl( + target, args, rundir=None, use_ovs_appctl=False, cmd_executor=None +): """Run ovn/ovs-appctl for target with args and return output. :param target: Name of daemon to contact. Unless target begins with '/', @@ -44,25 +47,43 @@ def ovn_appctl(target, args, rundir=None, use_ovs_appctl=False, # NOTE(fnordahl): The ovsdb-server processes for the OVN databases use a # non-standard naming scheme for their daemon control socket and we need # to pass the full path to the socket. - if target in ('ovnnb_db', 'ovnsb_db',): - target = os.path.join(rundir or OVN_RUNDIR, target + '.ctl') + if target in ( + "ovnnb_db", + "ovnsb_db", + ): + target = os.path.join(rundir or OVN_RUNDIR, target + ".ctl") if use_ovs_appctl: - tool = 'ovs-appctl' + tool = "ovs-appctl" else: - tool = 'ovn-appctl' + tool = "ovn-appctl" if not cmd_executor: cmd_executor = utils._run - return cmd_executor(tool, '-t', target, *args) + return cmd_executor(tool, "-t", target, *args) class OVNClusterStatus(object): + """Class for examining cluster status.""" - def __init__(self, name, cluster_id, server_id, address, status, role, - term, leader, vote, election_timer, log, - entries_not_yet_committed, entries_not_yet_applied, - connections, servers): + def __init__( + self, + name, + cluster_id, + server_id, + address, + status, + role, + term, + leader, + vote, + election_timer, + log, + entries_not_yet_committed, + entries_not_yet_applied, + connections, + servers, + ): """Initialize and populate OVNClusterStatus object. Use class initializer so we can define types in a compatible manner. @@ -116,23 +137,25 @@ class OVNClusterStatus(object): self.servers = servers def __eq__(self, other): + """Whether statuses are equal.""" return ( - self.name == other.name and - self.cluster_id == other.cluster_id and - self.server_id == other.server_id and - self.address == other.address and - self.status == other.status and - self.role == other.role and - self.term == other.term and - self.leader == other.leader and - self.vote == other.vote and - self.election_timer == other.election_timer and - self.log == other.log and - self.entries_not_yet_committed == - other.entries_not_yet_committed and - self.entries_not_yet_applied == other.entries_not_yet_applied and - self.connections == other.connections and - self.servers == other.servers) + self.name == other.name + and self.cluster_id == other.cluster_id + and self.server_id == other.server_id + and self.address == other.address + and self.status == other.status + and self.role == other.role + and self.term == other.term + and self.leader == other.leader + and self.vote == other.vote + and self.election_timer == other.election_timer + and self.log == other.log + and self.entries_not_yet_committed + == other.entries_not_yet_committed + and self.entries_not_yet_applied == other.entries_not_yet_applied + and self.connections == other.connections + and self.servers == other.servers + ) @property def is_cluster_leader(self): @@ -141,11 +164,12 @@ class OVNClusterStatus(object): :returns: Whether target is cluster leader :rtype: bool """ - return self.leader == 'self' + return self.leader == "self" -def cluster_status(target, schema=None, use_ovs_appctl=False, rundir=None, - cmd_executor=None): +def cluster_status( + target, schema=None, use_ovs_appctl=False, rundir=None, cmd_executor=None +): """Retrieve status information from clustered OVSDB. :param target: Usually one of 'ovsdb-server', 'ovnnb_db', 'ovnsb_db', can @@ -163,38 +187,44 @@ def cluster_status(target, schema=None, use_ovs_appctl=False, rundir=None, :raises: subprocess.CalledProcessError, KeyError, RuntimeError """ schema_map = { - 'ovnnb_db': 'OVN_Northbound', - 'ovnsb_db': 'OVN_Southbound', + "ovnnb_db": "OVN_Northbound", + "ovnsb_db": "OVN_Southbound", } if schema and schema not in schema_map.keys(): raise RuntimeError('Unknown schema provided: "{}"'.format(schema)) status = {} - k = '' - for line in ovn_appctl(target, - ('cluster/status', schema or schema_map[target]), - rundir=rundir, - use_ovs_appctl=use_ovs_appctl, - cmd_executor=cmd_executor).splitlines(): - if k and line.startswith(' '): + k = "" + for line in ovn_appctl( + target, + ("cluster/status", schema or schema_map[target]), + rundir=rundir, + use_ovs_appctl=use_ovs_appctl, + cmd_executor=cmd_executor, + ).splitlines(): + if k and line.startswith(" "): # there is no key which means this is a instance of a multi-line/ # multi-value item, populate the List which is already stored under # the key. - if k == 'servers': + if k == "servers": status[k].append( - tuple(line.replace(')', '').lstrip().split()[0:4:3])) + tuple(line.replace(")", "").lstrip().split()[0:4:3]) + ) else: status[k].append(line.lstrip()) - elif ':' in line: + elif ":" in line: # this is a line with a key - k, v = line.split(':', 1) + k, v = line.split(":", 1) k = k.lower() - k = k.replace(' ', '_') + k = k.replace(" ", "_") if v: # this is a line with both key and value - if k in ('cluster_id', 'server_id',): - v = v.replace('(', '') - v = v.replace(')', '') + if k in ( + "cluster_id", + "server_id", + ): + v = v.replace("(", "") + v = v.replace(")", "") status[k] = tuple(v.split()) else: status[k] = v.lstrip() @@ -204,21 +234,22 @@ def cluster_status(target, schema=None, use_ovs_appctl=False, rundir=None, # populated on subsequent iterations. status[k] = [] return OVNClusterStatus( - status['name'], - uuid.UUID(status['cluster_id'][1]), - uuid.UUID(status['server_id'][1]), - status['address'], - status['status'], - status['role'], - int(status['term']), - status['leader'], - status['vote'], - int(status['election_timer']), - status['log'], - int(status['entries_not_yet_committed']), - int(status['entries_not_yet_applied']), - status['connections'], - status['servers']) + status["name"], + uuid.UUID(status["cluster_id"][1]), + uuid.UUID(status["server_id"][1]), + status["address"], + status["status"], + status["role"], + int(status["term"]), + status["leader"], + status["vote"], + int(status["election_timer"]), + status["log"], + int(status["entries_not_yet_committed"]), + int(status["entries_not_yet_applied"]), + status["connections"], + status["servers"], + ) def is_northd_active(cmd_executor=None): @@ -231,9 +262,10 @@ def is_northd_active(cmd_executor=None): :rtype: bool """ try: - for line in ovn_appctl('ovn-northd', ('status',), - cmd_executor=cmd_executor).splitlines(): - if line.startswith('Status:') and 'active' in line: + for line in ovn_appctl( + "ovn-northd", ("status",), cmd_executor=cmd_executor + ).splitlines(): + if line.startswith("Status:") and "active" in line: return True except subprocess.CalledProcessError: pass diff --git a/charms/ovn-central-k8s/src/ovsdb.py b/charms/ovn-central-k8s/src/ovsdb.py index 780e51db..e12d952c 100644 --- a/charms/ovn-central-k8s/src/ovsdb.py +++ b/charms/ovn-central-k8s/src/ovsdb.py @@ -1,4 +1,4 @@ -# Copyright 2019 Canonical Ltd +# Copyright 2022 Canonical Ltd. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -11,6 +11,9 @@ # 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. + +"""Interface for interacting with OVSDB.""" + import json import uuid @@ -49,85 +52,85 @@ class SimpleOVSDB(object): # that will most likely be lower then the cost of finding the needle in # the haystack whenever downstream code misspells something. _tool_table_map = { - 'ovs-vsctl': ( - 'autoattach', - 'bridge', - 'ct_timeout_policy', - 'ct_zone', - 'controller', - 'datapath', - 'flow_sample_collector_set', - 'flow_table', - 'ipfix', - 'interface', - 'manager', - 'mirror', - 'netflow', - 'open_vswitch', - 'port', - 'qos', - 'queue', - 'ssl', - 'sflow', + "ovs-vsctl": ( + "autoattach", + "bridge", + "ct_timeout_policy", + "ct_zone", + "controller", + "datapath", + "flow_sample_collector_set", + "flow_table", + "ipfix", + "interface", + "manager", + "mirror", + "netflow", + "open_vswitch", + "port", + "qos", + "queue", + "ssl", + "sflow", ), - 'ovn-nbctl': ( - 'acl', - 'address_set', - 'connection', - 'dhcp_options', - 'dns', - 'forwarding_group', - 'gateway_chassis', - 'ha_chassis', - 'ha_chassis_group', - 'load_balancer', - 'load_balancer_health_check', - 'logical_router', - 'logical_router_policy', - 'logical_router_port', - 'logical_router_static_route', - 'logical_switch', - 'logical_switch_port', - 'meter', - 'meter_band', - 'nat', - 'nb_global', - 'port_group', - 'qos', - 'ssl', + "ovn-nbctl": ( + "acl", + "address_set", + "connection", + "dhcp_options", + "dns", + "forwarding_group", + "gateway_chassis", + "ha_chassis", + "ha_chassis_group", + "load_balancer", + "load_balancer_health_check", + "logical_router", + "logical_router_policy", + "logical_router_port", + "logical_router_static_route", + "logical_switch", + "logical_switch_port", + "meter", + "meter_band", + "nat", + "nb_global", + "port_group", + "qos", + "ssl", ), - 'ovn-sbctl': ( - 'address_set', - 'chassis', - 'connection', - 'controller_event', - 'dhcp_options', - 'dhcpv6_options', - 'dns', - 'datapath_binding', - 'encap', - 'gateway_chassis', - 'ha_chassis', - 'ha_chassis_group', - 'igmp_group', - 'ip_multicast', - 'logical_flow', - 'mac_binding', - 'meter', - 'meter_band', - 'multicast_group', - 'port_binding', - 'port_group', - 'rbac_permission', - 'rbac_role', - 'sb_global', - 'ssl', - 'service_monitor', + "ovn-sbctl": ( + "address_set", + "chassis", + "connection", + "controller_event", + "dhcp_options", + "dhcpv6_options", + "dns", + "datapath_binding", + "encap", + "gateway_chassis", + "ha_chassis", + "ha_chassis_group", + "igmp_group", + "ip_multicast", + "logical_flow", + "mac_binding", + "meter", + "meter_band", + "multicast_group", + "port_binding", + "port_group", + "rbac_permission", + "rbac_role", + "sb_global", + "ssl", + "service_monitor", ), } def __init__(self, tool, args=None, cmd_executor=None): - """SimpleOVSDB constructor. + """The SimpleOVSDB constructor. :param tool: Which tool with database commands to operate on. Usually one of `ovs-vsctl`, `ovn-nbctl`, `ovn-sbctl` @@ -137,18 +140,23 @@ class SimpleOVSDB(object): """ if tool not in self._tool_table_map: raise RuntimeError( - 'tool must be one of "{}"'.format(self._tool_table_map.keys())) + 'tool must be one of "{}"'.format(self._tool_table_map.keys()) + ) self._tool = tool self._args = args self.cmd_executor = cmd_executor or utils._run def __getattr__(self, table): + """Get table for tool.""" if table not in self._tool_table_map[self._tool]: raise AttributeError( - 'table "{}" not known for use with "{}"' - .format(table, self._tool)) + 'table "{}" not known for use with "{}"'.format( + table, self._tool + ) + ) return self.Table( - self._tool, table, args=self._args, cmd_executor=self.cmd_executor) + self._tool, table, args=self._args, cmd_executor=self.cmd_executor + ) class Table(object): """Methods to interact with contents of OVSDB tables. @@ -159,7 +167,7 @@ class SimpleOVSDB(object): """ def __init__(self, tool, table, args=None, cmd_executor=None): - """SimpleOVSDBTable constructor. + """Run SimpleOVSDBTable constructor. :param table: Which table to operate on :type table: str @@ -184,15 +192,17 @@ class SimpleOVSDB(object): # notation may occur that require further deserializing. # Reference: https://tools.ietf.org/html/rfc7047#section-5.1 ovs_type_cb_map = { - 'uuid': uuid.UUID, + "uuid": uuid.UUID, # NOTE: OVSDB sets have overloaded type # see special handling below - 'set': list, - 'map': dict, + "set": list, + "map": dict, } - assert len(data) > 1, ('Invalid data provided, expecting list ' - 'with at least two elements.') - if data[0] == 'set': + assert len(data) > 1, ( + "Invalid data provided, expecting list " + "with at least two elements." + ) + if data[0] == "set": # special handling for set # # it is either a list of strings or a list of typed lists. @@ -203,8 +213,7 @@ class SimpleOVSDB(object): # We could potentially just handle this generally based on # the types listed in `ovs_type_cb_map` but let's open for # that as soon as we have a concrete example to validate on - if isinstance( - el, list) and len(el) and el[0] == 'uuid': + if isinstance(el, list) and len(el) and el[0] == "uuid": decoded_set = [] for el in data[1]: decoded_set.append(self._deserialize_ovsdb(el)) @@ -227,33 +236,40 @@ class SimpleOVSDB(object): cmd = [self._tool] if self._args: cmd.extend(self._args) - cmd.extend(['-f', 'json', 'find', self._table]) + cmd.extend(["-f", "json", "find", self._table]) if condition: cmd.append(condition) output = self.cmd_executor(*cmd) data = json.loads(output) - for row in data['data']: + for row in data["data"]: values = [] for col in row: if isinstance(col, list) and len(col) > 1: values.append(self._deserialize_ovsdb(col)) else: values.append(col) - yield dict(zip(data['headings'], values)) + yield dict(zip(data["headings"], values)) def __iter__(self): + """Iterate over values in OVSDB table.""" return self._find_tbl() def clear(self, rec, col): - self.cmd_executor(self._tool, 'clear', self._table, rec, col) + """Clear value from OVSDB table.""" + self.cmd_executor(self._tool, "clear", self._table, rec, col) def find(self, condition): + """Find value in OVSDB table.""" return self._find_tbl(condition=condition) def remove(self, rec, col, value): + """Remove value from OVSDB table.""" self.cmd_executor( - self._tool, 'remove', self._table, rec, col, value) + self._tool, "remove", self._table, rec, col, value + ) def set(self, rec, col, value): - self.cmd_executor(self._tool, 'set', self._table, rec, - '{}={}'.format(col, value)) + """Set value in OVSDB table.""" + self.cmd_executor( + self._tool, "set", self._table, rec, "{}={}".format(col, value) + ) diff --git a/charms/ovn-central-k8s/src/utils.py b/charms/ovn-central-k8s/src/utils.py index 53c9b4dd..ca522888 100644 --- a/charms/ovn-central-k8s/src/utils.py +++ b/charms/ovn-central-k8s/src/utils.py @@ -1,4 +1,4 @@ -# Copyright 2019 Canonical Ltd +# Copyright 2019 Canonical Ltd. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -11,6 +11,9 @@ # 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. + +"""Patch utilities.""" + import subprocess diff --git a/charms/ovn-central-k8s/tests/unit/__init__.py b/charms/ovn-central-k8s/tests/unit/__init__.py index e69de29b..304f420a 100644 --- a/charms/ovn-central-k8s/tests/unit/__init__.py +++ b/charms/ovn-central-k8s/tests/unit/__init__.py @@ -0,0 +1,15 @@ +# Copyright 2022 Canonical Ltd. +# +# 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. + +"""Unit tests for charm.""" diff --git a/charms/ovn-central-k8s/tests/unit/test_ovn_central_charm.py b/charms/ovn-central-k8s/tests/unit/test_ovn_central_charm.py index afdb06c2..ea946593 100644 --- a/charms/ovn-central-k8s/tests/unit/test_ovn_central_charm.py +++ b/charms/ovn-central-k8s/tests/unit/test_ovn_central_charm.py @@ -14,14 +14,15 @@ # See the License for the specific language governing permissions and # limitations under the License. -import mock +"""Tests for OVN central charm.""" -import charm +import mock import ops_sunbeam.test_utils as test_utils +import charm -class _OVNCentralXenaOperatorCharm(charm.OVNCentralXenaOperatorCharm): +class _OVNCentralOperatorCharm(charm.OVNCentralOperatorCharm): def __init__(self, framework): self.seen_events = [] super().__init__(framework) @@ -37,64 +38,72 @@ class _OVNCentralXenaOperatorCharm(charm.OVNCentralXenaOperatorCharm): pass def cluster_status(self, db, cmd_executor): - if db == 'ovnnb_db': + if db == "ovnnb_db": nb_mock = mock.MagicMock() - nb_mock.cluster_id = 'nb_id' + nb_mock.cluster_id = "nb_id" return nb_mock - if db == 'ovnsb_db': + if db == "ovnsb_db": sb_mock = mock.MagicMock() - sb_mock.cluster_id = 'sb_id' + sb_mock.cluster_id = "sb_id" return sb_mock -class TestOVNCentralXenaOperatorCharm(test_utils.CharmTestCase): +class TestOVNCentralOperatorCharm(test_utils.CharmTestCase): + """Class for testing OVN central charm.""" PATCHES = [ - 'KubernetesServicePatch', + "KubernetesServicePatch", ] def setUp(self): + """Setup Glance tests.""" super().setUp(charm, self.PATCHES) self.harness = test_utils.get_harness( - _OVNCentralXenaOperatorCharm, - container_calls=self.container_calls) + _OVNCentralOperatorCharm, container_calls=self.container_calls + ) self.addCleanup(self.harness.cleanup) self.harness.begin() def test_pebble_ready_handler(self): + """Test Pebble ready event is captured.""" self.assertEqual(self.harness.charm.seen_events, []) test_utils.set_all_pebbles_ready(self.harness) self.assertEqual(len(self.harness.charm.seen_events), 3) def check_rendered_files(self): + """Check all files are rendered.""" sb_config_files = [ - '/etc/ovn/cert_host', - '/etc/ovn/key_host', - '/etc/ovn/ovn-central.crt', - '/root/ovn-sb-cluster-join.sh', - '/root/ovn-sb-db-server-wrapper.sh'] + "/etc/ovn/cert_host", + "/etc/ovn/key_host", + "/etc/ovn/ovn-central.crt", + "/root/ovn-sb-cluster-join.sh", + "/root/ovn-sb-db-server-wrapper.sh", + ] for f in sb_config_files: - self.check_file('ovn-sb-db-server', f) + self.check_file("ovn-sb-db-server", f) nb_config_files = [ - '/etc/ovn/cert_host', - '/etc/ovn/key_host', - '/etc/ovn/ovn-central.crt', - '/root/ovn-nb-cluster-join.sh', - '/root/ovn-nb-db-server-wrapper.sh'] + "/etc/ovn/cert_host", + "/etc/ovn/key_host", + "/etc/ovn/ovn-central.crt", + "/root/ovn-nb-cluster-join.sh", + "/root/ovn-nb-db-server-wrapper.sh", + ] for f in nb_config_files: - self.check_file('ovn-nb-db-server', f) + self.check_file("ovn-nb-db-server", f) northd_config_files = [ - '/etc/ovn/cert_host', - '/etc/ovn/key_host', - '/etc/ovn/ovn-central.crt', - '/etc/ovn/ovn-northd-db-params.conf', - '/root/ovn-northd-wrapper.sh'] + "/etc/ovn/cert_host", + "/etc/ovn/key_host", + "/etc/ovn/ovn-central.crt", + "/etc/ovn/ovn-northd-db-params.conf", + "/root/ovn-northd-wrapper.sh", + ] for f in northd_config_files: - self.check_file('ovn-northd', f) + self.check_file("ovn-northd", f) def test_all_relations_leader(self): + """Test all the charms relations.""" self.harness.set_leader() self.assertEqual(self.harness.charm.seen_events, []) test_utils.set_all_pebbles_ready(self.harness) @@ -102,28 +111,23 @@ class TestOVNCentralXenaOperatorCharm(test_utils.CharmTestCase): self.check_rendered_files() def test_all_relations_non_leader(self): + """Test all the charms relations on non-leader.""" self.harness.set_leader(False) self.assertEqual(self.harness.charm.seen_events, []) test_utils.set_all_pebbles_ready(self.harness) rel_ids = test_utils.add_all_relations(self.harness) - test_utils.set_remote_leader_ready( - self.harness, - rel_ids['peers']) + test_utils.set_remote_leader_ready(self.harness, rel_ids["peers"]) self.harness.update_relation_data( - rel_ids['peers'], + rel_ids["peers"], self.harness.charm.app.name, - { - 'nb_cid': 'nbcid', - 'sb_cid': 'sbcid'} + {"nb_cid": "nbcid", "sb_cid": "sbcid"}, ) self.check_rendered_files() self.assertEqual( - self.container_calls.execute['ovn-sb-db-server'], - [ - ['bash', '/root/ovn-sb-cluster-join.sh'] - ]) + self.container_calls.execute["ovn-sb-db-server"], + [["bash", "/root/ovn-sb-cluster-join.sh"]], + ) self.assertEqual( - self.container_calls.execute['ovn-nb-db-server'], - [ - ['bash', '/root/ovn-nb-cluster-join.sh'] - ]) + self.container_calls.execute["ovn-nb-db-server"], + [["bash", "/root/ovn-nb-cluster-join.sh"]], + ) diff --git a/charms/ovn-central-k8s/tox.ini b/charms/ovn-central-k8s/tox.ini index cfa25bf3..ea682f9a 100644 --- a/charms/ovn-central-k8s/tox.ini +++ b/charms/ovn-central-k8s/tox.ini @@ -15,6 +15,8 @@ minversion = 3.18.0 src_path = {toxinidir}/src/ tst_path = {toxinidir}/tests/ lib_path = {toxinidir}/lib/ +pyproject_toml = {toxinidir}/pyproject.toml +all_path = {[vars]src_path} {[vars]tst_path} [testenv] basepython = python3 @@ -33,6 +35,15 @@ allowlist_externals = deps = -r{toxinidir}/test-requirements.txt +[testenv:fmt] +description = Apply coding style standards to code +deps = + black + isort +commands = + isort {[vars]all_path} --skip-glob {[vars]lib_path} --skip {toxinidir}/.tox + black --config {[vars]pyproject_toml} {[vars]all_path} --exclude {[vars]lib_path} + [testenv:build] basepython = python3 deps = @@ -64,11 +75,6 @@ deps = {[testenv:py3]deps} basepython = python3.10 deps = {[testenv:py3]deps} -[testenv:pep8] -basepython = python3 -deps = {[testenv]deps} -commands = flake8 {posargs} {[vars]src_path} {[vars]tst_path} - [testenv:cover] basepython = python3 deps = {[testenv:py3]deps} @@ -83,6 +89,31 @@ commands = coverage xml -o cover/coverage.xml coverage report +[testenv:pep8] +description = Alias for lint +deps = {[testenv:lint]deps} +commands = {[testenv:lint]commands} + +[testenv:lint] +description = Check code against coding style standards +deps = + black + # flake8==4.0.1 # Pin version until https://github.com/csachs/pyproject-flake8/pull/14 is merged + flake8 + flake8-docstrings + flake8-copyright + flake8-builtins + pyproject-flake8 + pep8-naming + isort + codespell +commands = + codespell {[vars]all_path} + # pflake8 wrapper supports config from pyproject.toml + pflake8 --exclude {[vars]lib_path} --config {toxinidir}/pyproject.toml {[vars]all_path} + isort --check-only --diff {[vars]all_path} --skip-glob {[vars]lib_path} + black --config {[vars]pyproject_toml} --check --diff {[vars]all_path} --exclude {[vars]lib_path} + [testenv:func-noop] basepython = python3 commands =