openstack/sunbeam-charms
Code Issues Proposed changes

General tidy for charm

Browse Source 
Update allowlist_externals with full path to commands for tox4
compatibility.

Resync charm scaffolding with other Charmed K8S operators.

Fixup numerous lint warnings across codebase.

Change-Id: Ic39ad1351265cb5df9813cee5d27ee0d1392c632
This commit is contained in:
James Page 2023-01-18 13:27:31 +00:00
parent ab36a87baa
commit a0ebfbb51e
8 changed files with 211 additions and 147 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
charms/cinder-k8s/pyproject.toml Normal file
View File

@ -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"

132
charms/cinder-k8s/src/charm.py
View File

@ -1,23 +1,44 @@
#!/usr/bin/env python3
#
# Copyright 2021 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.
"""Cinder Operator Charm.
This charm provide Cinder services as part of an OpenStack deployment
"""
import logging
from typing import List
import ops.pebble
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.container_handlers as sunbeam_chandlers
import ops_sunbeam.relation_handlers as sunbeam_rhandlers
from typing import (
Dict,
List,
)
import charms.cinder_k8s.v0.storage_backend as sunbeam_storage_backend # noqa
import ops.pebble
import ops_sunbeam.charm as sunbeam_charm
import ops_sunbeam.container_handlers as sunbeam_chandlers
import ops_sunbeam.core as sunbeam_core
import ops_sunbeam.relation_handlers as sunbeam_rhandlers
from ops.framework import (
StoredState,
)
from ops.main import (
main,
)
logger = logging.getLogger(__name__)
@ -27,26 +48,19 @@ CINDER_SCHEDULER_CONTAINER = "cinder-scheduler"
class CinderWSGIPebbleHandler(sunbeam_chandlers.WSGIPebbleHandler):
"""Pebble handler for Cinder WSGI services."""
def start_service(self):
"""Start services in container."""
pass
def init_service(self, context) -> None:
"""Enable and start WSGI service"""
"""Enable and start WSGI service."""
self.write_config(context)
try:
self.execute(["a2disconf", "cinder-wsgi"], exception_on_error=True)
self.execute(
[
"a2disconf",
"cinder-wsgi"
],
exception_on_error=True
)
self.execute(
[
"a2ensite",
self.wsgi_service_name
],
exception_on_error=True
["a2ensite", self.wsgi_service_name], exception_on_error=True
)
except ops.pebble.ExecError:
logger.exception(
@ -69,16 +83,17 @@ class CinderWSGIPebbleHandler(sunbeam_chandlers.WSGIPebbleHandler):
"online": {
"override": "replace",
"level": "ready",
"exec": {
"command": "service apache2 status"
}
"exec": {"command": "service apache2 status"},
},
}
}
class CinderSchedulerPebbleHandler(sunbeam_chandlers.PebbleHandler):
"""Pebble handler for Cinder Scheduler services."""
def start_service(self):
"""Start services in container."""
container = self.charm.unit.get_container(self.container_name)
if not container:
logger.debug(
@ -93,7 +108,7 @@ class CinderSchedulerPebbleHandler(sunbeam_chandlers.PebbleHandler):
container.start(self.service_name)
def get_layer(self) -> dict:
"""Cinder Scheduler service
"""Cinder Scheduler service.
:returns: pebble layer configuration for wsgi services
:rtype: dict
@ -122,19 +137,18 @@ class CinderSchedulerPebbleHandler(sunbeam_chandlers.PebbleHandler):
"online": {
"override": "replace",
"level": "ready",
"exec": {
"command": "service cinder-scheduler status"
}
"exec": {"command": "service cinder-scheduler status"},
},
}
}
def init_service(self, context):
def init_service(self, context) -> None:
"""Initialize services and write configuration."""
self.write_config(context)
self.start_service()
self._state.service_ready = True
def default_container_configs(self):
def default_container_configs(self) -> List[Dict]:
"""Generate default configuration files for container."""
return [
sunbeam_core.ContainerConfigFile(
"/etc/cinder/cinder.conf",
@ -145,6 +159,8 @@ class CinderSchedulerPebbleHandler(sunbeam_chandlers.PebbleHandler):
class StorageBackendRequiresHandler(sunbeam_rhandlers.RelationHandler):
"""Relation handler for cinder storage backends."""
def setup_event_handler(self):
"""Configure event handlers for an Identity service relation."""
logger.debug("Setting up Identity Service event handler")
@ -162,10 +178,12 @@ class StorageBackendRequiresHandler(sunbeam_rhandlers.RelationHandler):
self.callback_f(event)
def set_ready(self) -> None:
"""Flag that all services are running and ready for use."""
return self.interface.set_ready()
@property
def ready(self) -> bool:
"""Determine whether interface is ready for use."""
return True
@ -179,11 +197,11 @@ class CinderOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
wsgi_public_script = "/usr/bin/cinder-wsgi"
mandatory_relations = {
'database',
'amqp',
'storage-backend',
'identity-service',
'ingress-public',
"database",
"amqp",
"storage-backend",
"identity-service",
"ingress-public",
}
def __init__(self, framework):
@ -210,7 +228,8 @@ class CinderOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
return handlers
@property
def service_endpoints(self):
def service_endpoints(self) -> List[Dict]:
"""Service endpoints for the Cinder API services."""
return [
{
"service_name": "cinderv2",
@ -237,7 +256,7 @@ class CinderOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
_cconfigs.extend(
[
sunbeam_core.ContainerConfigFile(
'/etc/cinder/api-paste.ini',
"/etc/cinder/api-paste.ini",
self.service_user,
self.service_group,
)
@ -245,7 +264,8 @@ class CinderOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
)
return _cconfigs
def get_pebble_handlers(self):
def get_pebble_handlers(self) -> List[sunbeam_chandlers.PebbleHandler]:
"""Pebble handlers for the charm."""
pebble_handlers = [
CinderWSGIPebbleHandler(
self,
@ -269,14 +289,17 @@ class CinderOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
@property
def default_public_ingress_port(self):
"""Public ingress port for service."""
return 8776
@property
def wsgi_container_name(self):
"""WSGI API service container name."""
return CINDER_API_CONTAINER
def _do_bootstrap(self):
"""
"""Bootstrap the service ready for use.
Starts the appropriate services in the order they are needed.
If the service has not yet been bootstrapped, then this will
1. Create the database
@ -287,16 +310,18 @@ class CinderOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
pebble_handler = self.get_named_pebble_handler(
CINDER_SCHEDULER_CONTAINER
)
pebble_handler.execute([
"sudo",
"-u",
"cinder",
"cinder-manage",
"--config-dir",
"/etc/cinder",
"db",
"sync"],
exception_on_error=True
pebble_handler.execute(
[
"sudo",
"-u",
"cinder",
"cinder-manage",
"--config-dir",
"/etc/cinder",
"db",
"sync",
],
exception_on_error=True,
)
except ops.pebble.ExecError:
logger.exception("Failed to bootstrap")
@ -304,6 +329,7 @@ class CinderOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
return
def configure_charm(self, event) -> None:
"""Configure the charmed services."""
super().configure_charm(event)
# Restarting services after bootstrap should be in aso
if self._state.bootstrapped:

16
charms/cinder-k8s/tests/__init__.py
View File

@ -0,0 +1,16 @@
#
# Copyright 2021 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.
"""Tests for charmed cinder."""

68
charms/cinder-k8s/tests/test_charm.py
View File

@ -1,68 +0,0 @@
# Copyright 2021 Canonical Ltd.
# See LICENSE file for licensing details.
#
# Learn more about testing at: https://juju.is/docs/sdk/testing
import unittest
from unittest.mock import Mock
from charm import CharmCinderOperatorCharm
from ops.model import ActiveStatus
from ops.testing import Harness
class TestCharm(unittest.TestCase):
def setUp(self):
self.harness = Harness(CharmCinderOperatorCharm)
self.addCleanup(self.harness.cleanup)
self.harness.begin()
def test_config_changed(self):
self.assertEqual(list(self.harness.charm._stored.things), [])
self.harness.update_config({"thing": "foo"})
self.assertEqual(list(self.harness.charm._stored.things), ["foo"])
def test_action(self):
# the harness doesn't (yet!) help much with actions themselves
action_event = Mock(params={"fail": ""})
self.harness.charm._on_fortune_action(action_event)
self.assertTrue(action_event.set_results.called)
def test_action_fail(self):
action_event = Mock(params={"fail": "fail this"})
self.harness.charm._on_fortune_action(action_event)
self.assertEqual(action_event.fail.call_args, [("fail this",)])
def test_httpbin_pebble_ready(self):
# Check the initial Pebble plan is empty
initial_plan = self.harness.get_container_pebble_plan("httpbin")
self.assertEqual(initial_plan.to_yaml(), "{}\n")
# Expected plan after Pebble ready with default config
expected_plan = {
"services": {
"httpbin": {
"override": "replace",
"summary": "httpbin",
"command": "gunicorn -b 0.0.0.0:80 httpbin:app -k gevent",
"startup": "enabled",
"environment": {"thing": "🎁"},
}
},
}
# Get the httpbin container from the model
container = self.harness.model.unit.get_container("httpbin")
# Emit the PebbleReadyEvent carrying the httpbin container
self.harness.charm.on.httpbin_pebble_ready.emit(container)
# Get the plan now we've run PebbleReady
updated_plan = self.harness.get_container_pebble_plan(
"httpbin").to_dict()
# Check we've got the plan we expected
self.assertEqual(expected_plan, updated_plan)
# Check the service was started
service = self.harness.model.unit.get_container(
"httpbin").get_service("httpbin")
self.assertTrue(service.is_running())
# Ensure we set an ActiveStatus with no message
self.assertEqual(self.harness.model.unit.status, ActiveStatus())

2
charms/cinder-k8s/tests/tests.yaml
View File

@ -28,7 +28,7 @@ target_deploy_status:
workload-status-message-regex: '^$'
mysql:
workload-status: active
workload-status-message-regex: '^$'
workload-status-message-regex: '^.*$'
cinder:
workload-status: blocked
workload-status-message-regex: '\(storage-backend\) integration missing'

16
charms/cinder-k8s/tests/unit/__init__.py
View File

@ -0,0 +1,16 @@
#
# Copyright 2021 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 Cinder K8S Operator."""

38
charms/cinder-k8s/tests/unit/test_cinder_charm.py
View File

@ -14,13 +14,16 @@
# See the License for the specific language governing permissions and
# limitations under the License.
"""Unit tests for core Cinder charm class."""
import mock
import ops_sunbeam.test_utils as test_utils
import charm
import ops_sunbeam.test_utils as test_utils
class _CinderOperatorCharm(charm.CinderOperatorCharm):
"""Test implementation of Cinder operator."""
def __init__(self, framework):
self.seen_events = []
@ -30,40 +33,39 @@ class _CinderOperatorCharm(charm.CinderOperatorCharm):
def _log_event(self, event):
self.seen_events.append(type(event).__name__)
def renderer(self, containers, container_configs, template_dir,
adapters):
def renderer(self, containers, container_configs, template_dir, adapters):
"""Intercept and record all calls to render config files."""
self.render_calls.append(
(
containers,
container_configs,
template_dir,
adapters))
(containers, container_configs, template_dir, adapters)
)
def configure_charm(self, event):
"""Intercept and record full charm configuration events."""
super().configure_charm(event)
self._log_event(event)
class TestCinderOperatorCharm(test_utils.CharmTestCase):
"""Unit tests for Cinder Operator."""
PATCHES = []
@mock.patch(
'charms.observability_libs.v0.kubernetes_service_patch.'
'KubernetesServicePatch')
"charms.observability_libs.v0.kubernetes_service_patch."
"KubernetesServicePatch"
)
def setUp(self, mock_patch):
self.container_calls = {
'push': {},
'pull': [],
'remove_path': []}
"""Setup test fixtures for test."""
self.container_calls = {"push": {}, "pull": [], "remove_path": []}
super().setUp(charm, self.PATCHES)
self.harness = test_utils.get_harness(
_CinderOperatorCharm,
container_calls=self.container_calls)
_CinderOperatorCharm, container_calls=self.container_calls
)
self.addCleanup(self.harness.cleanup)
self.harness.begin()
def test_pebble_ready_handler(self):
"""Test pebble ready event handling."""
self.assertEqual(self.harness.charm.seen_events, [])
self.harness.container_pebble_ready('cinder-api')
self.assertEqual(self.harness.charm.seen_events, ['PebbleReadyEvent'])
self.harness.container_pebble_ready("cinder-api")
self.assertEqual(self.harness.charm.seen_events, ["PebbleReadyEvent"])

47
charms/cinder-k8s/tox.ini
View File

@ -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
@ -28,11 +30,20 @@ commands = stestr run --slowest {posargs}
allowlist_externals =
git
charmcraft
fetch-libs.sh
rename.sh
{toxinidir}/fetch-libs.sh
{toxinidir}/rename.sh
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,30 @@ 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<6
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 =
@ -95,6 +125,9 @@ commands =
[testenv:func-smoke]
basepython = python3
setenv =
TEST_MODEL_SETTINGS = automatically-retry-hooks=true
TEST_MAX_RESOLVE_COUNT = 5
commands =
functest-run-suite --keep-model --smoke