openstack/sunbeam-charms
Code Issues Proposed changes

General tidy for module ready for release.

Browse Source 
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: I8ca548b84ddb7abb9f6c9ada6468ad78b0031160
This commit is contained in:
Liam Young 2022-11-04 06:29:54 +00:00
parent 37f277ef63
commit ea4a84b9ca
5 changed files with 262 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
charms/glance-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"

211
charms/glance-k8s/src/charm.py
View File

@ -22,22 +22,32 @@ This charm provide Glance services as part of an OpenStack deployment
"""
import logging
from typing import Callable
from typing import List
from ops.framework import StoredState, EventBase
from ops.main import main
from ops.model import (
ActiveStatus, BlockedStatus, WaitingStatus,
from typing import (
Callable,
List,
)
from ops.charm import CharmBase
import ops_sunbeam.charm as sunbeam_charm
import ops_sunbeam.compound_status as compound_status
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.container_handlers as sunbeam_chandlers
import ops_sunbeam.core as sunbeam_core
import ops_sunbeam.relation_handlers as sunbeam_rhandlers
from ops.charm import (
CharmBase,
)
from ops.framework import (
EventBase,
StoredState,
)
from ops.main import (
main,
)
from ops.model import (
ActiveStatus,
BlockedStatus,
WaitingStatus,
)
logger = logging.getLogger(__name__)
@ -47,6 +57,7 @@ logger = logging.getLogger(__name__)
class GlanceAPIPebbleHandler(sunbeam_chandlers.ServicePebbleHandler):
"""Handler for glance api container."""
def get_layer(self) -> dict:
"""Glance API service pebble layer.
@ -60,8 +71,10 @@ class GlanceAPIPebbleHandler(sunbeam_chandlers.ServicePebbleHandler):
f"{self.service_name}": {
"override": "replace",
"summary": f"{self.service_name} standalone",
"command": ("/usr/bin/glance-api "
"--config-file /etc/glance/glance-api.conf"),
"command": (
"/usr/bin/glance-api "
"--config-file /etc/glance/glance-api.conf"
),
"startup": "disabled",
},
"apache forwarder": {
@ -94,23 +107,30 @@ class GlanceStorageRelationHandler(sunbeam_rhandlers.CephClientHandler):
) -> None:
"""Run constructor."""
self.juju_storage_name = juju_storage_name
super().__init__(charm, relation_name, callback_f, allow_ec_overwrites,
app_name, mandatory=True)
super().__init__(
charm,
relation_name,
callback_f,
allow_ec_overwrites,
app_name,
mandatory=True,
)
def set_status(self, status: compound_status.Status) -> None:
"""
Override the base set_status.
"""Override the base set_status.
Custom logic is required here since this relation handler
falls back to local storage if the ceph relation isn't found.
"""
if (
not self.charm.has_ceph_relation() and
not self.charm.has_local_storage()
not self.charm.has_ceph_relation()
and not self.charm.has_local_storage()
):
status.set(BlockedStatus(
"ceph integration and local storage are not available"
))
status.set(
BlockedStatus(
"ceph integration and local storage are not available"
)
)
elif self.ready:
status.set(ActiveStatus(""))
else:
@ -132,20 +152,22 @@ class GlanceStorageRelationHandler(sunbeam_rhandlers.CephClientHandler):
:return: True if the storage is ready, False otherwise.
"""
if self.charm.has_ceph_relation():
logger.debug('ceph relation is connected, deferring to parent')
logger.debug("ceph relation is connected, deferring to parent")
return super().ready
# Check to see if the storage is satisfied
if self.charm.has_local_storage():
logger.debug(f'Storage {self.juju_storage_name} is attached')
logger.debug(f"Storage {self.juju_storage_name} is attached")
return True
logger.debug('Ceph relation does not exist and no local storage is '
'available.')
logger.debug(
"Ceph relation does not exist and no local storage is "
"available."
)
return False
def context(self) -> dict:
"""
"""Context for the ceph relation.
:return:
"""
@ -162,28 +184,36 @@ class GlanceOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
_state = StoredState()
_authed = False
service_name = "glance-api"
wsgi_admin_script = '/usr/bin/glance-wsgi-api'
wsgi_public_script = '/usr/bin/glance-wsgi-api'
wsgi_admin_script = "/usr/bin/glance-wsgi-api"
wsgi_public_script = "/usr/bin/glance-wsgi-api"
db_sync_cmds = [
['sudo', '-u', 'glance', 'glance-manage', '--config-dir',
'/etc/glance', 'db', 'sync']]
[
"sudo",
"-u",
"glance",
"glance-manage",
"--config-dir",
"/etc/glance",
"db",
"sync",
]
]
# ceph is included in the mandatory list as the GlanceStorage
# relation handler falls back to juju storage if ceph relation
# is not connected.
mandatory_relations = {
'database',
'identity-service',
'ingress-public',
'ceph',
"database",
"identity-service",
"ingress-public",
"ceph",
}
def __init__(self, *args, **kwargs) -> None:
super().__init__(*args, **kwargs)
self.framework.observe(
self.on.describe_status_action,
self._describe_status_action
self.on.describe_status_action, self._describe_status_action
)
def _describe_status_action(self, event: EventBase) -> None:
@ -194,12 +224,15 @@ class GlanceOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
"""Configuration contexts for the operator."""
contexts = super().config_contexts
if self.has_ceph_relation():
logger.debug('Application has ceph relation')
logger.debug("Application has ceph relation")
contexts.append(
sunbeam_ctxts.CephConfigurationContext(self, "ceph_config"))
sunbeam_ctxts.CephConfigurationContext(self, "ceph_config")
)
contexts.append(
sunbeam_ctxts.CinderCephConfigurationContext(self,
"cinder_ceph"))
sunbeam_ctxts.CinderCephConfigurationContext(
self, "cinder_ceph"
)
)
return contexts
@property
@ -209,7 +242,7 @@ class GlanceOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
_cconfigs.extend(
[
sunbeam_core.ContainerConfigFile(
'/etc/apache2/sites-enabled/glance-forwarding.conf',
"/etc/apache2/sites-enabled/glance-forwarding.conf",
self.service_user,
self.service_group,
),
@ -235,8 +268,8 @@ class GlanceOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
"ceph",
self.configure_charm,
allow_ec_overwrites=True,
app_name='rbd',
juju_storage_name='local-repository',
app_name="rbd",
juju_storage_name="local-repository",
)
handlers.append(self.ceph)
return handlers
@ -249,41 +282,44 @@ class GlanceOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
@property
def service_user(self) -> str:
"""Service user file and directory ownership."""
return 'glance'
return "glance"
@property
def service_group(self) -> str:
"""Service group file and directory ownership."""
return 'glance'
return "glance"
@property
def service_endpoints(self):
"""Describe the glance service endpoint."""
return [
{
'service_name': 'glance',
'type': 'image',
'description': "OpenStack Image",
'internal_url': f'{self.internal_url}',
'public_url': f'{self.public_url}',
'admin_url': f'{self.admin_url}'}]
"service_name": "glance",
"type": "image",
"description": "OpenStack Image",
"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):
def default_public_ingress_port(self) -> int:
"""Default ingress port."""
return 9292
@property
def healthcheck_http_url(self) -> str:
"""Healthcheck HTTP URL for the service."""
# / returns 300 and /versions return 200
return f'http://localhost:{self.default_public_ingress_port}/versions'
return f"http://localhost:{self.default_public_ingress_port}/versions"
def has_local_storage(self) -> bool:
"""Returns whether the application has been deployed with local
storage or not.
"""Whether the application has been deployed with local storage or not.
:return: True if local storage is present, False otherwise
"""
storages = self.model.storages['local-repository']
storages = self.model.storages["local-repository"]
return len(storages) > 0
def has_ceph_relation(self) -> bool:
@ -291,7 +327,7 @@ class GlanceOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
:return: True if the ceph relation has been made, False otherwise.
"""
return self.model.get_relation('ceph') is not None
return self.model.get_relation("ceph") is not None
def configure_charm(self, event) -> None:
"""Catchall handler to configure charm services."""
@ -301,44 +337,43 @@ class GlanceOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
if self.has_ceph_relation():
if not self.ceph.key:
logger.debug('Ceph key is not yet present, waiting.')
self.status.set(
WaitingStatus("ceph key not present yet")
)
logger.debug("Ceph key is not yet present, waiting.")
self.status.set(WaitingStatus("ceph key not present yet"))
return
elif self.has_local_storage():
logger.debug('Local storage is configured, using that.')
logger.debug("Local storage is configured, using that.")
else:
logger.debug('Neither local storage nor ceph relation exists.')
self.status.set(BlockedStatus(
'Missing storage. Relate to Ceph '
'or add local storage to continue.'
))
logger.debug("Neither local storage nor ceph relation exists.")
self.status.set(
BlockedStatus(
"Missing storage. Relate to Ceph "
"or add local storage to continue."
)
)
return
ph = self.get_named_pebble_handler("glance-api")
ph.execute(
['a2enmod', 'proxy_http'],
exception_on_error=True)
ph.execute(["a2enmod", "proxy_http"], exception_on_error=True)
if ph.pebble_ready:
if self.has_ceph_relation() and self.ceph.key:
logger.debug('Setting up Ceph packages in images.')
logger.debug("Setting up Ceph packages in images.")
ph.execute(["apt", "update"], exception_on_error=True)
ph.execute(
['apt', 'update'],
exception_on_error=True)
ph.execute(
['apt', 'install', '-y', 'ceph-common'],
exception_on_error=True)
["apt", "install", "-y", "ceph-common"],
exception_on_error=True,
)
ph.execute(
[
'ceph-authtool',
f'/etc/ceph/ceph.client.{self.app.name}.keyring',
'--create-keyring',
f'--name=client.{self.app.name}',
f'--add-key={self.ceph.key}'],
exception_on_error=True)
"ceph-authtool",
f"/etc/ceph/ceph.client.{self.app.name}.keyring",
"--create-keyring",
f"--name=client.{self.app.name}",
f"--add-key={self.ceph.key}",
],
exception_on_error=True,
)
else:
logger.debug('Using local storage')
logger.debug("Using local storage")
ph.init_service(self.contexts())
super().configure_charm(event)
@ -356,18 +391,12 @@ class GlanceOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
self.service_name,
self.container_configs,
self.template_dir,
self.openstack_release,
self.configure_charm
self.configure_charm,
)
]
class GlanceXenaOperatorCharm(GlanceOperatorCharm):
openstack_release = 'xena'
if __name__ == "__main__":
# Note: use_juju_for_storage=True required per
# https://github.com/canonical/operator/issues/506
main(GlanceXenaOperatorCharm, use_juju_for_storage=True)
main(GlanceOperatorCharm, use_juju_for_storage=True)

15
charms/glance-k8s/tests/unit/__init__.py
View File

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

80
charms/glance-k8s/tests/unit/test_glance_charm.py
View File

@ -14,14 +14,15 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import mock
"""Tests for glance charm."""
import charm
import mock
import ops_sunbeam.test_utils as test_utils
import charm
class _GlanceXenaOperatorCharm(charm.GlanceXenaOperatorCharm):
class _GlanceOperatorCharm(charm.GlanceOperatorCharm):
def __init__(self, framework):
self.seen_events = []
super().__init__(framework)
@ -39,21 +40,24 @@ class _GlanceXenaOperatorCharm(charm.GlanceXenaOperatorCharm):
class TestGlanceOperatorCharm(test_utils.CharmTestCase):
"""Class for testing glance charm."""
PATCHES = []
def setUp(self):
"""Setup Glance tests."""
super().setUp(charm, self.PATCHES)
self.harness = test_utils.get_harness(
_GlanceXenaOperatorCharm,
container_calls=self.container_calls)
_GlanceOperatorCharm, 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",
@ -68,47 +72,65 @@ class TestGlanceOperatorCharm(test_utils.CharmTestCase):
test_utils.add_complete_ingress_relation(self.harness)
@mock.patch(
'charms.observability_libs.v0.kubernetes_service_patch.'
'KubernetesServicePatch')
"charms.observability_libs.v0.kubernetes_service_patch."
"KubernetesServicePatch"
)
def test_pebble_ready_handler(self, svc_patch):
"""Test Pebble ready event is captured."""
self.harness.begin()
self.assertEqual(self.harness.charm.seen_events, [])
test_utils.set_all_pebbles_ready(self.harness)
self.assertEqual(self.harness.charm.seen_events, ['PebbleReadyEvent'])
self.assertEqual(self.harness.charm.seen_events, ["PebbleReadyEvent"])
@mock.patch(
'charms.observability_libs.v0.kubernetes_service_patch.'
'KubernetesServicePatch')
"charms.observability_libs.v0.kubernetes_service_patch."
"KubernetesServicePatch"
)
def test_all_relations(self, svc_patch):
"""Test all the charms relations."""
ceph_rel_id = self.harness.add_relation("ceph", "ceph-mon")
self.harness.begin_with_initial_hooks()
self.harness.add_relation_unit(ceph_rel_id, "ceph-mon/0")
self.harness.update_relation_data(
ceph_rel_id,
"ceph-mon/0",
{"ingress-address": "10.0.0.33"})
ceph_rel_id, "ceph-mon/0", {"ingress-address": "10.0.0.33"}
)
test_utils.add_ceph_relation_credentials(self.harness, ceph_rel_id)
test_utils.add_api_relations(self.harness)
self.harness.set_leader()
test_utils.set_all_pebbles_ready(self.harness)
ceph_install_cmds = [
['apt', 'update'],
['apt', 'install', '-y', 'ceph-common'],
['ceph-authtool',
'/etc/ceph/ceph.client.glance-k8s.keyring',
'--create-keyring',
'--name=client.glance-k8s',
'--add-key=AQBUfpVeNl7CHxAA8/f6WTcYFxW2dJ5VyvWmJg==']]
["apt", "update"],
["apt", "install", "-y", "ceph-common"],
[
"ceph-authtool",
"/etc/ceph/ceph.client.glance-k8s.keyring",
"--create-keyring",
"--name=client.glance-k8s",
"--add-key=AQBUfpVeNl7CHxAA8/f6WTcYFxW2dJ5VyvWmJg==",
],
]
for cmd in ceph_install_cmds:
self.assertIn(cmd, self.container_calls.execute['glance-api'])
self.assertIn(cmd, self.container_calls.execute["glance-api"])
app_setup_cmds = [
['a2enmod', 'proxy_http'],
['sudo', '-u', 'glance', 'glance-manage', '--config-dir',
'/etc/glance', 'db', 'sync']]
["a2enmod", "proxy_http"],
[
"sudo",
"-u",
"glance",
"glance-manage",
"--config-dir",
"/etc/glance",
"db",
"sync",
],
]
for cmd in app_setup_cmds:
self.assertIn(cmd, self.container_calls.execute['glance-api'])
self.assertIn(cmd, self.container_calls.execute["glance-api"])
for f in ['/etc/apache2/sites-enabled/glance-forwarding.conf',
'/etc/glance/glance-api.conf', '/etc/ceph/ceph.conf']:
self.check_file('glance-api', f)
for f in [
"/etc/apache2/sites-enabled/glance-forwarding.conf",
"/etc/glance/glance-api.conf",
"/etc/ceph/ceph.conf",
]:
self.check_file("glance-api", f)

43
charms/glance-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
@ -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 =
@ -96,8 +127,8 @@ commands =
[testenv:func-smoke]
basepython = python3
setenv =
TEST_MAX_RESOLVE_COUNT = 5
TEST_MODEL_SETTINGS = automatically-retry-hooks=true
TEST_MAX_RESOLVE_COUNT = 5
commands =
functest-run-suite --keep-model --smoke