From 0e1a100fca90d3f4e6d5d5c39312d299a6d53bf2 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Thu, 3 Nov 2022 15:30:02 +0000 Subject: [PATCH] General tidy for module ready for release. Refresh charm to drop release usage in ops-sunbeam. Drop surplus template fragments. Refresh unit tests. Tidy requirements.txt. Switch to black + other linters. Tidy docstrings across operator. Change-Id: I83fd4e5f33574d3564b7b097586928f035bafcdd --- charms/neutron-k8s/pyproject.toml | 39 ++++ charms/neutron-k8s/requirements.txt | 1 - charms/neutron-k8s/src/charm.py | 209 ++++++++++-------- charms/neutron-k8s/tests/unit/__init__.py | 15 ++ .../tests/unit/test_neutron_charm.py | 63 ++++-- charms/neutron-k8s/tox.ini | 41 +++- 6 files changed, 250 insertions(+), 118 deletions(-) create mode 100644 charms/neutron-k8s/pyproject.toml diff --git a/charms/neutron-k8s/pyproject.toml b/charms/neutron-k8s/pyproject.toml new file mode 100644 index 00000000..2896bc05 --- /dev/null +++ b/charms/neutron-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/neutron-k8s/requirements.txt b/charms/neutron-k8s/requirements.txt index b9b8fb9b..a1f17751 100644 --- a/charms/neutron-k8s/requirements.txt +++ b/charms/neutron-k8s/requirements.txt @@ -24,4 +24,3 @@ git+https://opendev.org/openstack/charm-ops-interface-tls-certificates#egg=inter # Note: Required for cinder-k8s, cinder-ceph-k8s, glance-k8s, nova-k8s git+https://opendev.org/openstack/charm-ops-interface-ceph-client#egg=interface_ceph_client # Charmhelpers is only present as interface_ceph_client uses it. -git+https://github.com/juju/charm-helpers.git#egg=charmhelpers diff --git a/charms/neutron-k8s/src/charm.py b/charms/neutron-k8s/src/charm.py index 1939e6d7..d04decda 100755 --- a/charms/neutron-k8s/src/charm.py +++ b/charms/neutron-k8s/src/charm.py @@ -1,29 +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 +# limitations under the License. + """Neutron Operator Charm. This charm provide Neutron services as part of an OpenStack deployment """ import logging -from typing import List - -from ops.framework import StoredState -from ops.main import main +from typing import ( + List, +) import ops_sunbeam.charm as sunbeam_charm -import ops_sunbeam.core as sunbeam_core -import ops_sunbeam.container_handlers as sunbeam_chandlers import ops_sunbeam.config_contexts as sunbeam_ctxts -import ops_sunbeam.relation_handlers as sunbeam_rhandlers +import ops_sunbeam.container_handlers as sunbeam_chandlers +import ops_sunbeam.core as sunbeam_core import ops_sunbeam.ovn.relation_handlers as ovn_rhandlers +import ops_sunbeam.relation_handlers as sunbeam_rhandlers +from ops.framework import ( + StoredState, +) +from ops.main import ( + main, +) logger = logging.getLogger(__name__) class NeutronServerPebbleHandler(sunbeam_chandlers.ServicePebbleHandler): + """Handler for interacting with pebble data.""" def get_layer(self): - """Neutron server service + """Neutron server service. :returns: pebble service layer configuration for neutron server service :rtype: dict @@ -36,9 +57,9 @@ class NeutronServerPebbleHandler(sunbeam_chandlers.ServicePebbleHandler): "override": "replace", "summary": "Neutron Server", "command": "neutron-server", - "startup": "enabled" + "startup": "enabled", } - } + }, } def get_healthcheck_layer(self) -> dict: @@ -53,23 +74,20 @@ class NeutronServerPebbleHandler(sunbeam_chandlers.ServicePebbleHandler): "online": { "override": "replace", "level": "ready", - "http": { - "url": self.charm.healthcheck_http_url - } + "http": {"url": self.charm.healthcheck_http_url}, }, } } def default_container_configs(self): + """Base container configs.""" return [ sunbeam_core.ContainerConfigFile( - '/etc/neutron/neutron.conf', - 'neutron', - 'neutron'), + "/etc/neutron/neutron.conf", "neutron", "neutron" + ), sunbeam_core.ContainerConfigFile( - '/etc/neutron/api-paste.ini', - 'neutron', - 'neutron'), + "/etc/neutron/api-paste.ini", "neutron", "neutron" + ), ] @@ -83,48 +101,61 @@ class NeutronOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm): wsgi_public_script = "" db_sync_cmds = [ - ['sudo', '-u', 'neutron', 'neutron-db-manage', '--config-file', - '/etc/neutron/neutron.conf', '--config-file', - '/etc/neutron/plugins/ml2/ml2_conf.ini', 'upgrade', 'head']] + [ + "sudo", + "-u", + "neutron", + "neutron-db-manage", + "--config-file", + "/etc/neutron/neutron.conf", + "--config-file", + "/etc/neutron/plugins/ml2/ml2_conf.ini", + "upgrade", + "head", + ] + ] def get_pebble_handlers(self) -> List[sunbeam_chandlers.PebbleHandler]: """Pebble handlers for the service.""" return [ NeutronServerPebbleHandler( self, - 'neutron-server', + "neutron-server", self.service_name, self.container_configs, self.template_dir, - self.openstack_release, self.configure_charm, ) ] @property def service_endpoints(self): + """Neutron service endpoint description.""" return [ { - 'service_name': 'neutron', - 'type': 'network', - 'description': "OpenStack Networking", - 'internal_url': f'{self.internal_url}', - 'public_url': f'{self.public_url}', - 'admin_url': f'{self.admin_url}'}] + "service_name": "neutron", + "type": "network", + "description": "OpenStack Networking", + "internal_url": f"{self.internal_url}", + "public_url": f"{self.public_url}", + "admin_url": f"{self.admin_url}", + } + ] @property def default_public_ingress_port(self): + """Public ingress port.""" return 9696 @property def service_user(self) -> str: """Service user file and directory ownership.""" - return 'neutron' + return "neutron" @property def service_group(self) -> str: """Service group file and directory ownership.""" - return 'neutron' + return "neutron" @property def service_conf(self) -> str: @@ -134,86 +165,86 @@ class NeutronOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm): # Neutron OVN Specific Code + class OVNContext(sunbeam_ctxts.ConfigContext): + """OVN configuration.""" def context(self) -> dict: + """Configuration context.""" return { - 'extension_drivers': 'port_security', - 'type_drivers': 'geneve,gre,vlan,flat,local', - 'tenant_network_types': 'geneve,gre,vlan,flat,local', - 'mechanism_drivers': 'ovn', - 'path_mtu': '1500', - 'tunnel_id_ranges': '1:1000', - 'vni_ranges': '1001:2000', - 'network_vlan_ranges': 'physnet1:1000:2000', - 'flat_networks': 'physnet1', - 'enable_tunneling': 'True', - 'local_ip': '127.0.0.1', - 'tunnel_types': 'gre', - 'enable_security_group': 'True', - 'vni_ranges': '1001:2000', - 'max_header_size': '38', - 'ovn_l3_scheduler': 'leastloaded', - 'ovn_metadata_enabled': 'True', - 'enable_distributed_floating_ip': 'False', - 'dns_servers': '', - 'dhcp_default_lease_time': '43200', - 'dns_servers': '', - 'ovn_dhcp4_global_options': '', - 'ovn_dhcp6_global_options': '', - 'vhost_sock_dir': '/run/libvirt-vhost-user', - 'ovn_key': '/etc/neutron/plugins/ml2/key_host', - 'ovn_cert': '/etc/neutron/plugins/ml2/cert_host', - 'ovn_ca_cert': '/etc/neutron/plugins/ml2/neutron-ovn.crt'} + "extension_drivers": "port_security", + "type_drivers": "geneve,gre,vlan,flat,local", + "tenant_network_types": "geneve,gre,vlan,flat,local", + "mechanism_drivers": "ovn", + "path_mtu": "1500", + "tunnel_id_ranges": "1:1000", + "vni_ranges": "1001:2000", + "network_vlan_ranges": "physnet1:1000:2000", + "flat_networks": "physnet1", + "enable_tunneling": "True", + "local_ip": "127.0.0.1", + "tunnel_types": "gre", + "enable_security_group": "True", + "vni_ranges": "1001:2000", + "max_header_size": "38", + "ovn_l3_scheduler": "leastloaded", + "ovn_metadata_enabled": "True", + "enable_distributed_floating_ip": "False", + "dns_servers": "", + "dhcp_default_lease_time": "43200", + "dns_servers": "", + "ovn_dhcp4_global_options": "", + "ovn_dhcp6_global_options": "", + "vhost_sock_dir": "/run/libvirt-vhost-user", + "ovn_key": "/etc/neutron/plugins/ml2/key_host", + "ovn_cert": "/etc/neutron/plugins/ml2/cert_host", + "ovn_ca_cert": "/etc/neutron/plugins/ml2/neutron-ovn.crt", + } class NeutronServerOVNPebbleHandler(NeutronServerPebbleHandler): + """Handler for interacting with neutron container.""" def default_container_configs(self): + """Neutron container configs.""" return [ sunbeam_core.ContainerConfigFile( - '/etc/neutron/neutron.conf', - 'neutron', - 'neutron'), + "/etc/neutron/neutron.conf", "neutron", "neutron" + ), sunbeam_core.ContainerConfigFile( - '/etc/neutron/plugins/ml2/key_host', - 'root', - 'root'), + "/etc/neutron/plugins/ml2/key_host", "root", "root" + ), sunbeam_core.ContainerConfigFile( - '/etc/neutron/plugins/ml2/cert_host', - 'root', - 'root'), + "/etc/neutron/plugins/ml2/cert_host", "root", "root" + ), sunbeam_core.ContainerConfigFile( - '/etc/neutron/plugins/ml2/neutron-ovn.crt', - 'root', - 'root'), + "/etc/neutron/plugins/ml2/neutron-ovn.crt", "root", "root" + ), sunbeam_core.ContainerConfigFile( - '/etc/neutron/plugins/ml2/ml2_conf.ini', - 'root', - 'root'), + "/etc/neutron/plugins/ml2/ml2_conf.ini", "root", "root" + ), sunbeam_core.ContainerConfigFile( - '/etc/neutron/api-paste.ini', - 'neutron', - 'neutron'), + "/etc/neutron/api-paste.ini", "neutron", "neutron" + ), ] class NeutronOVNOperatorCharm(NeutronOperatorCharm): + """Neutron charm class for OVN.""" mandatory_relations = { - 'amqp', - 'database', - 'ovsdb-cms', - 'identity-service', - 'ingress-public', + "amqp", + "database", + "ovsdb-cms", + "identity-service", + "ingress-public", } @property def config_contexts(self) -> List[sunbeam_ctxts.ConfigContext]: """Configuration contexts for the operator.""" contexts = super().config_contexts - contexts.append( - OVNContext(self, "ovn")) + contexts.append(OVNContext(self, "ovn")) return contexts def get_pebble_handlers(self) -> List[sunbeam_chandlers.PebbleHandler]: @@ -221,11 +252,10 @@ class NeutronOVNOperatorCharm(NeutronOperatorCharm): return [ NeutronServerOVNPebbleHandler( self, - 'neutron-server', + "neutron-server", self.service_name, self.container_configs, self.template_dir, - self.openstack_release, self.configure_charm, ) ] @@ -247,12 +277,7 @@ class NeutronOVNOperatorCharm(NeutronOperatorCharm): return handlers -class NeutronOVNXenaOperatorCharm(NeutronOVNOperatorCharm): - - openstack_release = 'xena' - - if __name__ == "__main__": # Note: use_juju_for_storage=True required per # https://github.com/canonical/operator/issues/506 - main(NeutronOVNXenaOperatorCharm, use_juju_for_storage=True) + main(NeutronOVNOperatorCharm, use_juju_for_storage=True) diff --git a/charms/neutron-k8s/tests/unit/__init__.py b/charms/neutron-k8s/tests/unit/__init__.py index e69de29b..304f420a 100644 --- a/charms/neutron-k8s/tests/unit/__init__.py +++ b/charms/neutron-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/neutron-k8s/tests/unit/test_neutron_charm.py b/charms/neutron-k8s/tests/unit/test_neutron_charm.py index ca996559..3b208a68 100644 --- a/charms/neutron-k8s/tests/unit/test_neutron_charm.py +++ b/charms/neutron-k8s/tests/unit/test_neutron_charm.py @@ -14,49 +14,60 @@ # See the License for the specific language governing permissions and # limitations under the License. -import mock +"""Tests for neutron charm.""" -import charm +import mock import ops_sunbeam.test_utils as test_utils +import charm -class _NeutronOVNXenaOperatorCharm(charm.NeutronOVNXenaOperatorCharm): + +class _NeutronOVNOperatorCharm(charm.NeutronOVNOperatorCharm): + """Neutron test charm.""" def __init__(self, framework): + """Setup event logging.""" self.seen_events = [] super().__init__(framework) def _log_event(self, event): + """Log events.""" self.seen_events.append(type(event).__name__) def configure_charm(self, event): + """Log configure charm call.""" super().configure_charm(event) self._log_event(event) @property def public_ingress_address(self): + """Ingress address for charm.""" return "neutron.juju" class TestNeutronOperatorCharm(test_utils.CharmTestCase): + """Classes for testing neutron charms.""" PATCHES = [] @mock.patch( - 'charms.observability_libs.v0.kubernetes_service_patch.' - 'KubernetesServicePatch') + "charms.observability_libs.v0.kubernetes_service_patch." + "KubernetesServicePatch" + ) def setUp(self, mock_patch): + """Setup Neutron tests.""" super().setUp(charm, self.PATCHES) self.harness = test_utils.get_harness( - _NeutronOVNXenaOperatorCharm, - container_calls=self.container_calls) + _NeutronOVNOperatorCharm, container_calls=self.container_calls + ) # clean up events that were dynamically defined, # otherwise we get issues because they'll be redefined, # which is not allowed. from charms.data_platform_libs.v0.database_requires import ( - DatabaseEvents + DatabaseEvents, ) + for attr in ( "database_database_created", "database_endpoints_changed", @@ -71,11 +82,13 @@ class TestNeutronOperatorCharm(test_utils.CharmTestCase): self.harness.begin() def test_pebble_ready_handler(self): + """Test Pebble ready event is captured.""" self.assertEqual(self.harness.charm.seen_events, []) - self.harness.container_pebble_ready('neutron-server') + self.harness.container_pebble_ready("neutron-server") self.assertEqual(len(self.harness.charm.seen_events), 1) def test_all_relations(self): + """Test all the charms relations.""" self.harness.set_leader() test_utils.set_all_pebbles_ready(self.harness) test_utils.add_all_relations(self.harness) @@ -83,19 +96,29 @@ class TestNeutronOperatorCharm(test_utils.CharmTestCase): setup_cmds = [ [ - 'sudo', '-u', 'neutron', 'neutron-db-manage', '--config-file', - '/etc/neutron/neutron.conf', '--config-file', - '/etc/neutron/plugins/ml2/ml2_conf.ini', 'upgrade', 'head']] + "sudo", + "-u", + "neutron", + "neutron-db-manage", + "--config-file", + "/etc/neutron/neutron.conf", + "--config-file", + "/etc/neutron/plugins/ml2/ml2_conf.ini", + "upgrade", + "head", + ] + ] for cmd in setup_cmds: - self.assertIn(cmd, self.container_calls.execute['neutron-server']) + self.assertIn(cmd, self.container_calls.execute["neutron-server"]) config_files = [ - '/etc/neutron/neutron.conf', - '/etc/neutron/api-paste.ini', - '/etc/neutron/plugins/ml2/cert_host', - '/etc/neutron/plugins/ml2/key_host', - '/etc/neutron/plugins/ml2/ml2_conf.ini', - '/etc/neutron/plugins/ml2/neutron-ovn.crt'] + "/etc/neutron/neutron.conf", + "/etc/neutron/api-paste.ini", + "/etc/neutron/plugins/ml2/cert_host", + "/etc/neutron/plugins/ml2/key_host", + "/etc/neutron/plugins/ml2/ml2_conf.ini", + "/etc/neutron/plugins/ml2/neutron-ovn.crt", + ] for f in config_files: - self.check_file('neutron-server', f) + self.check_file("neutron-server", f) diff --git a/charms/neutron-k8s/tox.ini b/charms/neutron-k8s/tox.ini index cfa25bf3..ea682f9a 100644 --- a/charms/neutron-k8s/tox.ini +++ b/charms/neutron-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 =