Skip to content

Commit

Permalink
address the comments
Browse files Browse the repository at this point in the history
  • Loading branch information
wood-push-melon committed Oct 6, 2023
1 parent 7fb1d3c commit 3e6e48d
Show file tree
Hide file tree
Showing 9 changed files with 100 additions and 130 deletions.
8 changes: 4 additions & 4 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,17 @@ log_cli_level = "INFO"

# Formatting tools configuration
[tool.black]
line-length = 80
line-length = 99
target-version = ["py38"]

[tool.isort]
line_length = 80
line_length = 99
profile = "black"

# Linting tools configuration
[tool.flake8]
max-line-length = 80
max-doc-length = 80
max-line-length = 99
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"]
Expand Down
72 changes: 24 additions & 48 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,8 @@
DatabaseRequires,
)
from charms.grafana_k8s.v0.grafana_dashboard import GrafanaDashboardProvider
from charms.loki_k8s.v0.loki_push_api import (
LogProxyConsumer,
PromtailDigestError,
)
from charms.observability_libs.v0.kubernetes_service_patch import (
KubernetesServicePatch,
)
from charms.loki_k8s.v0.loki_push_api import LogProxyConsumer, PromtailDigestError
from charms.observability_libs.v0.kubernetes_service_patch import KubernetesServicePatch
from charms.prometheus_k8s.v0.prometheus_scrape import MetricsEndpointProvider
from jinja2 import Template
from lightkube import Client
Expand All @@ -37,23 +32,24 @@
from ops.pebble import ChangeError, Layer

from constants import (
DATABASE_RELATION_NAME,
DATABASE_INTEGRATION_NAME,
GLAUTH_COMMANDS,
GLAUTH_CONFIG_DIR,
GLAUTH_LDAP_PORT,
GRAFANA_DASHBOARD_RELATION_NAME,
GRAFANA_DASHBOARD_INTEGRATION_NAME,
LOG_DIR,
LOG_FILE,
LOKI_API_PUSH_RELATION_NAME,
PROMETHEUS_SCRAPE_RELATION_NAME,
LOKI_API_PUSH_INTEGRATION_NAME,
PROMETHEUS_SCRAPE_INTEGRATION_NAME,
WORKLOAD_CONTAINER,
WORKLOAD_SERVICE,
)
from kubernetes_resource import ConfigMapResource, StatefulSetResource
from validators import (
leader_unit,
validate_container_connectivity,
validate_database_relation,
validate_database_resource,
validate_integration_exists,
)

logger = logging.getLogger(__name__)
Expand All @@ -66,53 +62,39 @@ def __init__(self, *args):
super().__init__(*args)
self._container = self.unit.get_container(WORKLOAD_CONTAINER)

self._k8s_client = Client(
field_manager=self.app.name, namespace=self.model.name
)
self._configmap = ConfigMapResource(
client=self._k8s_client, name=self.app.name
)
self._statefulset = StatefulSetResource(
client=self._k8s_client, name=self.app.name
)
self._k8s_client = Client(field_manager=self.app.name, namespace=self.model.name)
self._configmap = ConfigMapResource(client=self._k8s_client, name=self.app.name)
self._statefulset = StatefulSetResource(client=self._k8s_client, name=self.app.name)

self._db_name = f"{self.model.name}_{self.app.name}"
self.database = DatabaseRequires(
self,
relation_name=DATABASE_RELATION_NAME,
relation_name=DATABASE_INTEGRATION_NAME,
database_name=self._db_name,
extra_user_roles="SUPERUSER",
)

self.service_patcher = KubernetesServicePatch(
self, [("ldap", GLAUTH_LDAP_PORT)]
)
self.service_patcher = KubernetesServicePatch(self, [("ldap", GLAUTH_LDAP_PORT)])

self.loki_consumer = LogProxyConsumer(
self,
log_files=[str(LOG_FILE)],
relation_name=LOKI_API_PUSH_RELATION_NAME,
relation_name=LOKI_API_PUSH_INTEGRATION_NAME,
container_name=WORKLOAD_CONTAINER,
)
self.metrics_endpoint = MetricsEndpointProvider(
self, relation_name=PROMETHEUS_SCRAPE_RELATION_NAME
self, relation_name=PROMETHEUS_SCRAPE_INTEGRATION_NAME
)
self._grafana_dashboards = GrafanaDashboardProvider(
self, relation_name=GRAFANA_DASHBOARD_RELATION_NAME
self, relation_name=GRAFANA_DASHBOARD_INTEGRATION_NAME
)

self.framework.observe(self.on.install, self._on_install)
self.framework.observe(self.on.config_changed, self._on_config_changed)
self.framework.observe(self.on.remove, self._on_remove)
self.framework.observe(
self.on.glauth_pebble_ready, self._on_pebble_ready
)
self.framework.observe(
self.database.on.database_created, self._on_database_created
)
self.framework.observe(
self.database.on.endpoints_changed, self._on_database_changed
)
self.framework.observe(self.on.glauth_pebble_ready, self._on_pebble_ready)
self.framework.observe(self.database.on.database_created, self._on_database_created)
self.framework.observe(self.database.on.endpoints_changed, self._on_database_changed)
self.framework.observe(
self.loki_consumer.on.promtail_digest_error,
self._on_promtail_error,
Expand All @@ -124,7 +106,7 @@ def _pebble_layer(self) -> Layer:
"summary": "GLAuth layer",
"description": "pebble layer for GLAuth service",
"services": {
WORKLOAD_CONTAINER: {
WORKLOAD_SERVICE: {
"override": "replace",
"summary": "GLAuth Operator layer",
"startup": "disabled",
Expand All @@ -147,13 +129,11 @@ def _restart_glauth_service(self):
)

@validate_container_connectivity
@validate_database_relation
@validate_integration_exists(DATABASE_INTEGRATION_NAME)
@validate_database_resource
def _handle_event_update(self, event: HookEvent) -> None:
self._update_glauth_config()
self._container.add_layer(
WORKLOAD_CONTAINER, self._pebble_layer, combine=True
)
self._container.add_layer(WORKLOAD_CONTAINER, self._pebble_layer, combine=True)

self._restart_glauth_service()
self.unit.status = ActiveStatus()
Expand Down Expand Up @@ -221,15 +201,11 @@ def _on_remove(self, event: RemoveEvent):
def _on_database_created(self, event: DatabaseCreatedEvent):
self._update_glauth_config()
self._mount_glauth_config()
self._container.add_layer(
WORKLOAD_CONTAINER, self._pebble_layer, combine=True
)
self._container.add_layer(WORKLOAD_CONTAINER, self._pebble_layer, combine=True)
self._restart_glauth_service()
self.unit.status = ActiveStatus()

def _on_database_changed(
self, event: DatabaseEndpointsChangedEvent
) -> None:
def _on_database_changed(self, event: DatabaseEndpointsChangedEvent) -> None:
self._handle_event_update(event)

def _on_config_changed(self, event: ConfigChangedEvent):
Expand Down
9 changes: 5 additions & 4 deletions src/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@

from pathlib import PurePath

DATABASE_RELATION_NAME = "pg-database"
LOKI_API_PUSH_RELATION_NAME = "logging"
PROMETHEUS_SCRAPE_RELATION_NAME = "metrics-endpoint"
GRAFANA_DASHBOARD_RELATION_NAME = "grafana-dashboard"
DATABASE_INTEGRATION_NAME = "pg-database"
LOKI_API_PUSH_INTEGRATION_NAME = "logging"
PROMETHEUS_SCRAPE_INTEGRATION_NAME = "metrics-endpoint"
GRAFANA_DASHBOARD_INTEGRATION_NAME = "grafana-dashboard"

GLAUTH_CONFIG_DIR = PurePath("/etc/config")
GLAUTH_CONFIG_FILE = GLAUTH_CONFIG_DIR / "glauth.cfg"
Expand All @@ -17,3 +17,4 @@
LOG_FILE = LOG_DIR / "glauth.log"

WORKLOAD_CONTAINER = "glauth"
WORKLOAD_SERVICE = "glauth"
2 changes: 1 addition & 1 deletion src/grafana_dashboards/glauth.json.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
},
"id": 27,
"panels": [],
"title": "Avaliability",
"title": "Availability",
"type": "row"
},
{
Expand Down
23 changes: 10 additions & 13 deletions src/kubernetes_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
logger = logging.getLogger(__name__)


class KubernetesResourceError(Exception):
def __init__(self, message: str):
self.message = message


def watch_for_update(func: Callable):
def wrapper(self, *args, **kwargs):
resource = self.get()
Expand All @@ -36,10 +41,7 @@ def wrapper(self, *args, **kwargs):
):
with attempt:
resource = self.get()
if (
not resource
or resource.metadata.resourceVersion == version
):
if not resource or resource.metadata.resourceVersion == version:
raise TryAgain
except RetryError:
logger.debug("No changes in the watched k8s resource")
Expand All @@ -58,9 +60,7 @@ def name(self):

def get(self):
try:
cm = self._client.get(
ConfigMap, self._name, namespace=self._client.namespace
)
cm = self._client.get(ConfigMap, self._name, namespace=self._client.namespace)
return cm
except ApiError as e:
logging.error(f"Error fetching ConfigMap: {e}")
Expand All @@ -81,6 +81,7 @@ def create(self):
self._client.create(cm)
except ApiError as e:
logging.error(f"Error creating ConfigMap: {e}")
raise KubernetesResourceError(f"Failed to create ConfigMap {self._name}")

@watch_for_update
def patch(self, data: dict):
Expand All @@ -98,9 +99,7 @@ def patch(self, data: dict):

def delete(self):
try:
self._client.delete(
ConfigMap, self._name, namespace=self._client.namespace
)
self._client.delete(ConfigMap, self._name, namespace=self._client.namespace)
except ApiError as e:
logging.error(f"Error deleting ConfigMap: {e}")

Expand All @@ -116,9 +115,7 @@ def name(self):

def get(self):
try:
ss = self._client.get(
StatefulSet, self._name, namespace=self._client.namespace
)
ss = self._client.get(StatefulSet, self._name, namespace=self._client.namespace)
return ss
except ApiError as e:
logging.error(f"Error fetching ConfigMap: {e}")
Expand Down
51 changes: 24 additions & 27 deletions src/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,15 @@
from functools import wraps
from typing import Any, Callable

from ops.charm import EventBase
from ops.charm import CharmBase, EventBase
from ops.model import BlockedStatus, MaintenanceStatus, WaitingStatus

from constants import DATABASE_RELATION_NAME

logger = logging.getLogger(__name__)


def leader_unit(func: Callable):
@wraps(func)
def wrapper(self, *args: EventBase, **kwargs: Any):
def wrapper(self: CharmBase, *args: EventBase, **kwargs: Any):
if not self.unit.is_leader():
return

Expand All @@ -26,55 +24,54 @@ def wrapper(self, *args: EventBase, **kwargs: Any):

def validate_container_connectivity(func: Callable):
@wraps(func)
def wrapper(self, *args: EventBase, **kwargs: Any):
def wrapper(self: CharmBase, *args: EventBase, **kwargs: Any):
event, *_ = args
logger.debug(f"Handling event: {event}")
if not self._container.can_connect():
logger.debug(f"Cannot connect to container, defer event {event}.")
event.defer()

self.unit.status = WaitingStatus(
"Waiting to connect to " "container."
)
self.unit.status = WaitingStatus("Waiting to connect to container.")
return

return func(self, *args, **kwargs)

return wrapper


def validate_database_relation(func: Callable):
@wraps(func)
def wrapper(self, *args: EventBase, **kwargs: Any):
event, *_ = args
logger.debug(f"Handling event: {event}")
def validate_integration_exists(integration_name: str):
def decorator(func: Callable):
@wraps(func)
def wrapper(self: CharmBase, *args: EventBase, **kwargs: Any):
event, *_ = args
logger.debug(f"Handling event: {event}")

self.unit.status = MaintenanceStatus("Configuring resources")
if not self.model.relations[DATABASE_RELATION_NAME]:
logger.debug(f"Database relation is missing, defer event {event}.")
event.defer()
self.unit.status = MaintenanceStatus("Configuring resources")
if not self.model.relations[integration_name]:
logger.debug(f"Integration {integration_name} is missing, defer event {event}.")
event.defer()

self.unit.status = BlockedStatus(
"Missing required relation with " "database"
)
return
self.unit.status = BlockedStatus(
f"Missing required integration {integration_name}"
)
return

return func(self, *args, **kwargs)
return func(self, *args, **kwargs)

return wrapper
return wrapper

return decorator


def validate_database_resource(func: Callable):
@wraps(func)
def wrapper(self, *args: EventBase, **kwargs: Any):
def wrapper(self: CharmBase, *args: EventBase, **kwargs: Any):
event, *_ = args
logger.debug(f"Handling event: {event}")

self.unit.status = MaintenanceStatus("Configuring resources")
if not self.database.is_resource_created():
logger.debug(
f"Database has not been created yet, defer event {event}"
)
logger.debug(f"Database has not been created yet, defer event {event}")
event.defer()

self.unit.status = WaitingStatus("Waiting for database creation")
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from pytest_mock import MockerFixture

from charm import GLAuthCharm
from constants import DATABASE_RELATION_NAME
from constants import DATABASE_INTEGRATION_NAME

DB_APP = "postgresql-k8s"
DB_USERNAME = "relation_id"
Expand Down Expand Up @@ -58,7 +58,7 @@ def mocked_statefulset(mocker: MockerFixture) -> MagicMock:

@pytest.fixture()
def database_relation(harness: Harness) -> int:
relation_id = harness.add_relation(DATABASE_RELATION_NAME, DB_APP)
relation_id = harness.add_relation(DATABASE_INTEGRATION_NAME, DB_APP)
harness.add_relation_unit(relation_id, "postgresql-k8s/0")
return relation_id

Expand Down
Loading

0 comments on commit 3e6e48d

Please sign in to comment.