Merge branch 'main' into use-saml-integrator

canonical · Mar 19, 2024 · 6b3467a · 6b3467a
2 parents 42ee333 + 599624b
commit 6b3467a
Show file tree

Hide file tree

Showing 9 changed files with 250 additions and 132 deletions.
diff --git a/docs/explanation/charm-architecture.md b/docs/explanation/charm-architecture.md
@@ -10,14 +10,13 @@ Pebble `services` are configured through [layers](https://github.com/canonical/p
 
 1. An [NGINX](https://www.nginx.com/) container, which can be used to efficiently serve static resources, as well as be the incoming point for all web traffic to the pod.
 2. The [Indico](https://getindico.io/) container itself, which has a [uWSGI](https://uwsgi-docs.readthedocs.io/en/latest/) server configured in HTTP mode.
-3. A [Celery](https://docs.celeryq.dev/en/stable/) container, running tasks generated by the web application.
 
 
 As a result, if you run a `kubectl get pods` on a namespace named for the Juju model you've deployed the Indico charm into, you'll see something like the following:
 
 ```bash
 NAME                             READY   STATUS    RESTARTS   AGE
-indico-0                         4/4     Running   0         6h4m
+indico-0                         3/3     Running   0         6h4m
 ```
 
 This shows there are 4 containers - the three named above, as well as a container for the charm code itself.
@@ -55,7 +54,7 @@ The workload that this container is running is defined in the [Indico ROCK](http
 
 The Celery is used to process tasks asynchronously created by the Indico application such as sending e-mails, survey notifications, event reminders, etc.
 
-The Celery container runs the same workload as the Indico container, as defined in the [Indico ROCK](https://github.com/canonical/indico-operator/tree/main/indico_rock).
+Celery runs in the same container as the Indico container, as defined in the [Indico ROCK](https://github.com/canonical/indico-operator/tree/main/indico_rock).
 
 ## Metrics
 Inside the above mentioned containers, additional Pebble layers are defined in order to provide metrics.
@@ -78,7 +77,7 @@ The `StatsD Prometheus Exporter` listens on ports:
 
 ### Celery Prometheus exporter
 
-Inside the Indico  container, the  [Celery Exporter](https://github.com/danihodovic/celery-exporter) runs to collect metrics from Celery.
+Inside the Indico container, the  [Celery Exporter](https://github.com/danihodovic/celery-exporter) runs to collect metrics from Celery.
 
 The `Celery Exporter` is started with:
 
@@ -137,13 +136,13 @@ Action: wait for the integrations, validate the configuration, update Ingress, a
 3. [database_relation_joined](https://github.com/canonical/ops-lib-pgsql): for when the PostgreSQL relation has been joined.
 Action: if the unit is the leader, add the extensions [`pg_trgm:public`](https://www.postgresql.org/docs/current/pgtrgm.html) and [`unaccent:public`](https://www.postgresql.org/docs/current/unaccent.html).
 4. [leader_elected](https://juju.is/docs/sdk/leader-elected-event): is emitted for a unit that is elected as leader.
-Action: guarantee that all Indico workers have the same [secret key](https://docs.getindico.io/en/latest/config/settings/?highlight=secret_key#SECRET_KEY) that is used to sign tokens in URLs.
+Action: guarantee that all Indico workers have the same [secret key](https://docs.getindico.io/en/latest/config/settings/?highlight=secret_key#SECRET_KEY) that is used to sign tokens in URLs and select a unit to run Celery.
 5. [master_changed](https://github.com/canonical/ops-lib-pgsql): PostgreSQLClient custom event for when the connection details to the master database on this relation change.
 Action: Update the database connection string configuration and emit config_changed event.
 6. [redis_relation_changed](https://github.com/canonical/redis-k8s-operator): Fired when Redis is changed (host, for example).
 Action: Same as config_changed.
 7. [refresh_external_resources_action](https://charmhub.io/indico/actions): fired when refresh-external-resources action is executed.
-Action: Pull changes from the customization repository, reload uWSGI and upgrade the external plugins.
+8. [indico_peers_relation_departed](https://juju.is/docs/sdk/relation-name-relation-departed-event): fired when a Indico unit departs. Action: elect a new unit to run Celery on if the departed unit was running Celery and replans the services accordingly.
 
 ## Charm code overview
 

diff --git a/docs/tutorial/getting-started.md b/docs/tutorial/getting-started.md
@@ -35,7 +35,7 @@ To see the pod created by the Indico charm, run `kubectl get pods` on a namespac
 
 ```bash
 NAME                             READY   STATUS    RESTARTS   AGE
-indico-0                         4/4     Running   0         6h4m
+indico-0                         3/3     Running   0         6h4m
 ```
 
 Run [`juju status`](https://juju.is/docs/olm/juju-status) to see the current status of the deployment. In the Unit list, you can see that Indico is waiting:

diff --git a/metadata.yaml b/metadata.yaml
@@ -36,8 +36,6 @@ containers:
     resource: indico-image
   indico-nginx:
     resource: indico-nginx-image
-  indico-celery:
-    resource: indico-image
 
 resources:
   indico-image:

diff --git a/src-docs/actions.py.md b/src-docs/actions.py.md
@@ -0,0 +1,45 @@
+<!-- markdownlint-disable -->
+
+<a href="../src/actions.py#L0"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+
+# <kbd>module</kbd> `actions.py`
+Indico charm actions. 
+
+**Global Variables**
+---------------
+- **EMAIL_LIST_MAX**
+- **EMAIL_LIST_SEPARATOR**
+
+
+---
+
+## <kbd>class</kbd> `Observer`
+Indico charm actions observer. 
+
+<a href="../src/actions.py#L22"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+
+### <kbd>function</kbd> `__init__`
+
+```python
+__init__(charm: CharmBase, state: State)
+```
+
+Initialize the observer and register actions handlers. 
+
+
+
+**Args:**
+
+ - <b>`charm`</b>:  The parent charm to attach the observer to. 
+ - <b>`state`</b>:  The Indico charm state. 
+
+
+---
+
+#### <kbd>property</kbd> model
+
+Shortcut for more simple access the model. 
+
+
+
+
diff --git a/src/charm.py b/src/charm.py
@@ -16,7 +16,7 @@
 from charms.nginx_ingress_integrator.v0.nginx_route import require_nginx_route
 from charms.prometheus_k8s.v0.prometheus_scrape import MetricsEndpointProvider
 from charms.redis_k8s.v0.redis import RedisRelationCharmEvents, RedisRequires
-from ops.charm import ActionEvent, CharmBase, HookEvent, PebbleReadyEvent
+from ops.charm import ActionEvent, CharmBase, HookEvent, PebbleReadyEvent, RelationDepartedEvent
 from ops.framework import StoredState
 from ops.jujuversion import JujuVersion
 from ops.main import main
@@ -75,7 +75,6 @@ def __init__(self, *args):
         self.framework.observe(self.on.config_changed, self._on_config_changed)
         self.framework.observe(self.on.leader_elected, self._on_leader_elected)
         self.framework.observe(self.on.indico_pebble_ready, self._on_pebble_ready)
-        self.framework.observe(self.on.indico_celery_pebble_ready, self._on_pebble_ready)
         self.framework.observe(self.on.indico_nginx_pebble_ready, self._on_pebble_ready)
         self.framework.observe(
             self.on.refresh_external_resources_action, self._refresh_external_resources_action
@@ -97,6 +96,9 @@ def __init__(self, *args):
         self.framework.observe(
             self.redis_cache.charm.on.redis_relation_updated, self._on_config_changed
         )
+        self.framework.observe(
+            self.on["indico-peers"].relation_departed, self._on_peer_relation_departed
+        )
         self._require_nginx_route()
 
         self._metrics_endpoint = MetricsEndpointProvider(
@@ -213,7 +215,7 @@ def _config_pebble(self, container: Container) -> None:
             container: Container to be configured by Pebble.
         """
         self.unit.status = MaintenanceStatus(f"Adding {container.name} layer to pebble")
-        if container.name in ["indico", "indico-celery"]:
+        if container.name == "indico":
             plugins = (
                 self.config["external_plugins"].split(",")
                 if self.config["external_plugins"]
@@ -222,20 +224,28 @@ def _config_pebble(self, container: Container) -> None:
             self._install_plugins(container, plugins)
         # The plugins need to be installed before adding the layer so that they are included in
         # the corresponding env vars
-        pebble_config_func = getattr(
-            self, f"_get_{container.name.replace('-', '_')}_pebble_config"
-        )
-        pebble_config = pebble_config_func(container)
-        container.add_layer(container.name, pebble_config, combine=True)
         if container.name == "indico":
-            celery_config = self._get_celery_prometheus_exporter_pebble_config(container)
+            indico_config = self._get_indico_pebble_config(container)
+            container.add_layer(container.name, indico_config, combine=True)
+            peer_relation = self.model.get_relation("indico-peers")
+            if (
+                not peer_relation
+                or peer_relation.data[self.app].get("celery-unit") == self.unit.name
+            ):
+                celery_config = self._get_celery_pebble_config(container)
+                container.add_layer("celery", celery_config, combine=True)
+                celery_exporter_config = self._get_celery_prometheus_exporter_pebble_config(
+                    container
+                )
+                container.add_layer("celery-exporter", celery_exporter_config, combine=True)
             statsd_config = self._get_statsd_prometheus_exporter_pebble_config(container)
-            container.add_layer("celery", celery_config, combine=True)
             container.add_layer("statsd", statsd_config, combine=True)
             self._download_customization_changes(container)
         if container.name == "indico-nginx":
-            pebble_config = self._get_nginx_prometheus_exporter_pebble_config(container)
-            container.add_layer("nginx", pebble_config, combine=True)
+            nginx_config = self._get_nginx_pebble_config(container)
+            container.add_layer(container.name, nginx_config, combine=True)
+            nginx_exporter_config = self._get_nginx_prometheus_exporter_pebble_config(container)
+            container.add_layer("nginx", nginx_exporter_config, combine=True)
         self.unit.status = MaintenanceStatus(f"Starting {container.name} container")
         container.pebble.replan_services()
         if self._are_pebble_instances_ready():
@@ -277,8 +287,8 @@ def _get_indico_pebble_config(self, container: Container) -> ops.pebble.LayerDic
         }
         return typing.cast(ops.pebble.LayerDict, layer)
 
-    def _get_indico_celery_pebble_config(self, container: Container) -> Dict:
-        """Generate pebble config for the indico-celery container.
+    def _get_celery_pebble_config(self, container: Container) -> ops.pebble.LayerDict:
+        """Generate pebble config for the celery container.
 
         Args:
             container: Celery container that has the target configuration.
@@ -287,11 +297,11 @@ def _get_indico_celery_pebble_config(self, container: Container) -> Dict:
             The pebble configuration for the container.
         """
         indico_env_config = self._get_indico_env_config(container)
-        return {
+        layer = {
             "summary": "Indico celery layer",
             "description": "Indico celery layer",
             "services": {
-                "indico-celery": {
+                "celery": {
                     "override": "replace",
                     "summary": "Indico celery",
                     "command": "/usr/local/bin/indico celery worker -B -E",
@@ -313,14 +323,15 @@ def _get_indico_celery_pebble_config(self, container: Container) -> Dict:
                 },
             },
         }
+        return typing.cast(ops.pebble.LayerDict, layer)
 
-    def _get_indico_nginx_pebble_config(self, _) -> Dict:
+    def _get_nginx_pebble_config(self, _) -> ops.pebble.LayerDict:
         """Generate pebble config for the indico-nginx container.
 
         Returns:
             The pebble configuration for the container.
         """
-        return {
+        layer = {
             "summary": "Indico nginx layer",
             "description": "Indico nginx layer",
             "services": {
@@ -339,6 +350,7 @@ def _get_indico_nginx_pebble_config(self, _) -> Dict:
                 },
             },
         }
+        return typing.cast(ops.pebble.LayerDict, layer)
 
     def _get_celery_prometheus_exporter_pebble_config(self, container) -> ops.pebble.LayerDict:
         """Generate pebble config for the celery-prometheus-exporter container.
@@ -459,9 +471,8 @@ def _get_indico_secret_key_from_relation(self) -> Optional[str]:
             secret_value = peer_relation.data[self.app].get("secret-key")
         else:
             secret_id = peer_relation.data[self.app].get("secret-id")
-            if secret_id:
-                secret = self.model.get_secret(id=secret_id)
-                secret_value = secret.get_content().get("secret-key")
+            secret = self.model.get_secret(id=secret_id)
+            secret_value = secret.get_content().get("secret-key")
         return secret_value
 
     def _get_indico_env_config(self, container: Container) -> Dict:
@@ -750,6 +761,30 @@ def _on_leader_elected(self, _) -> None:
         ):
             secret = self.app.add_secret({"secret-key": secret_value})
             peer_relation.data[self.app].update({"secret-id": secret.id})
+        if peer_relation and not peer_relation.data[self.app].get("celery-unit"):
+            peer_relation.data[self.app].update({"celery-unit": self.unit.name})
+
+    def _on_peer_relation_departed(self, event: RelationDepartedEvent) -> None:
+        """Handle the peer relation departed event.
+
+        Args:
+            event: the event triggering the handler.
+        """
+        peer_relation = self.model.get_relation("indico-peers")
+        if (
+            self.unit.is_leader()
+            and peer_relation
+            and event.departing_unit
+            and peer_relation.data[self.app].get("celery-unit") == event.departing_unit.name
+        ):
+            if self.unit != event.departing_unit:
+                peer_relation.data[self.app].update({"celery-unit": self.unit.name})
+                container = self.unit.get_container("indico")
+                if self._are_relations_ready(event) and container.can_connect():
+                    self._config_pebble(container)
+            else:
+                # Leadership election will select a new celery-unit
+                peer_relation.data[self.app].update({"celery-unit": ""})
 
     def _has_secrets(self) -> bool:
         """Check if current Juju version supports secrets.

diff --git a/src/grafana_dashboards/celery.json b/src/grafana_dashboards/celery.json
@@ -557,7 +557,7 @@
     ]
   },
   "timezone": "utc",
-  "title": "Indico Celery",
-  "uid": "indico-celery-32s3",
+  "title": "Celery",
+  "uid": "celery-32s3",
   "version": 0
 }
diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py
@@ -63,10 +63,11 @@ async def test_health_checks(app: Application):
 
     Assume that the charm has already been built and is running.
     """
-    container_list = ["indico-celery", "indico-nginx", "indico"]
+    container__checks_list = [("indico-nginx", 2), ("indico", 4)]
     # Application actually does have units
     indico_unit = app.units[0]  # type: ignore
-    for container in container_list:
+    for container_checks in container__checks_list:
+        container = container_checks[0]
         cmd = f"PEBBLE_SOCKET=/charm/containers/{container}/pebble.socket /charm/bin/pebble checks"
         action = await indico_unit.run(cmd, timeout=10)
         # Change this if upgrading Juju lib version to >= 3
@@ -79,7 +80,7 @@ async def test_health_checks(app: Application):
         # When executing the checks, `0/3` means there are 0 errors of 3.
         # Each check has it's own `0/3`, so we will count `n` times,
         # where `n` is the number of checks for that container.
-        assert stdout.count("0/3") == container_list.index(container) + 1
+        assert stdout.count("0/3") == container_checks[1]
 
 
 @pytest.mark.abort_on_fail

diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py
@@ -63,7 +63,6 @@ def set_relations_and_leader(self):
         self.is_ready(
             [
                 "indico",
-                "indico-celery",
                 "indico-nginx",
             ]
         )