Merge "stop services when mandatory integrations are removed" into main

ops-sunbeam/ops_sunbeam/charm.py

@@ -35,6 +35,8 @@ import urllib
 from typing import (
     List,
     Mapping,
+    Optional,
+    Set,
 )
 
 import ops.charm
@@ -234,9 +236,12 @@ class OSBaseOperatorCharm(ops.charm.CharmBase):
         ):
             raise sunbeam_guard.WaitingExceptionError("Leader not ready")
 
-    def check_relation_handlers_ready(self):
+    def check_relation_handlers_ready(self, event: ops.framework.EventBase):
         """Check all relation handlers are ready."""
-        if not self.relation_handlers_ready():
+        not_ready_relations = self.get_mandatory_relations_not_ready(event)
+        if not_ready_relations:
+            logger.info(f"Relations {not_ready_relations} incomplete")
+            self.stop_services(not_ready_relations)
             raise sunbeam_guard.WaitingExceptionError(
                 "Not all relations are ready"
             )
@@ -252,7 +257,7 @@ class OSBaseOperatorCharm(ops.charm.CharmBase):
     def configure_unit(self, event: ops.framework.EventBase) -> None:
         """Run configuration on this unit."""
         self.check_leader_ready()
-        self.check_relation_handlers_ready()
+        self.check_relation_handlers_ready(event)
         self._state.unit_bootstrapped = True
 
     def configure_app_leader(self, event):
@@ -293,6 +298,10 @@ class OSBaseOperatorCharm(ops.charm.CharmBase):
             self.bootstrap_status.set(ActiveStatus())
             self.post_config_setup()
 
+    def stop_services(self, relation: Optional[Set[str]] = None) -> None:
+        """Stop all running services."""
+        # Machine charms should implement this function if required.
+
     @property
     def supports_peer_relation(self) -> bool:
         """Whether the charm support the peers relation."""
@@ -354,22 +363,125 @@ class OSBaseOperatorCharm(ops.charm.CharmBase):
         # charms should handle the event if required
         pass
 
-    def relation_handlers_ready(self) -> bool:
+    def check_broken_relations(
-        """Determine whether all relations are ready for use."""
+        self, relations: set, event: ops.framework.EventBase
+    ) -> set:
+        """Return all broken relations on given set of relations."""
+        broken_relations = set()
+
+        # Check for each relation if the event is gone away event.
+        # lazy import the events
+        # Note: Ceph relation not handled as there is no gone away event.
+        for relation in relations:
+            _is_broken = False
+            match relation:
+                case "database" | "api-database" | "cell-database":
+                    from ops.charm import (
+                        RelationBrokenEvent,
+                    )
+
+                    if isinstance(event, RelationBrokenEvent):
+                        _is_broken = True
+                case "ingress-public" | "ingress-internal":
+                    from charms.traefik_k8s.v2.ingress import (
+                        IngressPerAppRevokedEvent,
+                    )
+
+                    if isinstance(event, IngressPerAppRevokedEvent):
+                        _is_broken = True
+                case "identity-service":
+                    from charms.keystone_k8s.v1.identity_service import (
+                        IdentityServiceGoneAwayEvent,
+                    )
+
+                    if isinstance(event, IdentityServiceGoneAwayEvent):
+                        _is_broken = True
+                case "amqp":
+                    from charms.rabbitmq_k8s.v0.rabbitmq import (
+                        RabbitMQGoneAwayEvent,
+                    )
+
+                    if isinstance(event, RabbitMQGoneAwayEvent):
+                        _is_broken = True
+                case "certificates":
+                    from charms.tls_certificates_interface.v1.tls_certificates import (
+                        CertificateExpiredEvent,
+                    )
+
+                    if isinstance(event, CertificateExpiredEvent):
+                        _is_broken = True
+                case "ovsdb-cms":
+                    from charms.ovn_central_k8s.v0.ovsdb import (
+                        OVSDBCMSGoneAwayEvent,
+                    )
+
+                    if isinstance(event, OVSDBCMSGoneAwayEvent):
+                        _is_broken = True
+                case "identity-credentials":
+                    from charms.keystone_k8s.v0.identity_credentials import (
+                        IdentityCredentialsGoneAwayEvent,
+                    )
+
+                    if isinstance(event, IdentityCredentialsGoneAwayEvent):
+                        _is_broken = True
+                case "identity-ops":
+                    from charms.keystone_k8s.v0.identity_resource import (
+                        IdentityOpsProviderGoneAwayEvent,
+                    )
+
+                    if isinstance(event, IdentityOpsProviderGoneAwayEvent):
+                        _is_broken = True
+                case "gnocchi-db":
+                    from charms.gnocchi_k8s.v0.gnocchi_service import (
+                        GnocchiServiceGoneAwayEvent,
+                    )
+
+                    if isinstance(event, GnocchiServiceGoneAwayEvent):
+                        _is_broken = True
+                case "ceph-access":
+                    from charms.cinder_ceph_k8s.v0.ceph_access import (
+                        CephAccessGoneAwayEvent,
+                    )
+
+                    if isinstance(event, CephAccessGoneAwayEvent):
+                        _is_broken = True
+                case "dns-backend":
+                    from charms.designate_bind_k8s.v0.bind_rndc import (
+                        BindRndcGoneAwayEvent,
+                    )
+
+                    if isinstance(event, BindRndcGoneAwayEvent):
+                        _is_broken = True
+
+            if _is_broken:
+                broken_relations.add(relation)
+
+        return broken_relations
+
+    def get_mandatory_relations_not_ready(
+        self, event: ops.framework.EventBase
+    ) -> Set[str]:
+        """Get mandatory relations that are not ready for use."""
         ready_relations = {
             handler.relation_name
             for handler in self.relation_handlers
             if handler.mandatory and handler.ready
         }
+
+        # The relation data for broken relations are not cleared during
+        # processing of gone away event. This is a temporary workaround
+        # to mark broken relations as not ready.
+        # The workaround can be removed once the below bug is resolved
+        # https://bugs.launchpad.net/juju/+bug/2024583
+        # https://github.com/canonical/operator/issues/940
+        broken_relations = self.check_broken_relations(ready_relations, event)
+        ready_relations = ready_relations.difference(broken_relations)
+
         not_ready_relations = self.mandatory_relations.difference(
             ready_relations
         )
 
-        if len(not_ready_relations) != 0:
+        return not_ready_relations
-            logger.info(f"Relations {not_ready_relations} incomplete")
-            return False
-
-        return True
 
     def contexts(self) -> sunbeam_core.OPSCharmContexts:
         """Construct context for rendering templates."""
@@ -495,10 +607,19 @@ class OSBaseOperatorCharmK8S(OSBaseOperatorCharm):
                     "Container service not ready"
                 )
 
+    def stop_services(self, relation: Optional[Set[str]] = None) -> None:
+        """Stop all running services."""
+        for ph in self.pebble_handlers:
+            if ph.pebble_ready:
+                logging.debug(
+                    f"Stopping all services in container {ph.container_name}"
+                )
+                ph.stop_all()
+
     def configure_unit(self, event: ops.framework.EventBase) -> None:
         """Run configuration on this unit."""
         self.check_leader_ready()
-        self.check_relation_handlers_ready()
+        self.check_relation_handlers_ready(event)
         self.open_ports()
         self.init_container_services()
         self.check_pebble_handlers_ready()

ops-sunbeam/ops_sunbeam/container_handlers.py

@@ -302,6 +302,20 @@ class PebbleHandler(ops.charm.Object):
                 )
                 container.restart(service_name)
 
+    def stop_all(self) -> None:
+        """Stop services in container."""
+        container = self.charm.unit.get_container(self.container_name)
+        if not container.can_connect():
+            logger.debug(
+                f"Container {self.container_name} not ready, no need to stop"
+            )
+            return
+
+        services = container.get_services()
+        if services:
+            logger.debug("Stopping all services")
+            container.stop(*services.keys())
+
 
 class ServicePebbleHandler(PebbleHandler):
     """Container handler for containers which manage a service."""

ops-sunbeam/ops_sunbeam/ovn/relation_handlers.py

@@ -554,6 +554,8 @@ class OVSDBCMSRequiresHandler(
         """Handle OVSDB CMS change events."""
         self.callback_f(event)
         if self.mandatory:
+            logger.debug("ovsdb-cms integration removed, stop services")
+            self.charm.stop_services({self.relation_name})
             self.status.set(BlockedStatus("integration missing"))
 
     @property

ops-sunbeam/ops_sunbeam/relation_handlers.py

@@ -300,6 +300,14 @@ class DBHandler(RelationHandler):
             getattr(db.on, f"{alias}_endpoints_changed"),
             self._on_database_updated,
         )
+
+        # Gone away events are not handled by the database interface library.
+        # So handle them in this class
+        self.framework.observe(
+            self.charm.on[self.relation_name].relation_broken,
+            self._on_database_relation_broken,
+        )
+
         # this will be set to self.interface in parent class
         return db
 
@@ -315,6 +323,13 @@ class DBHandler(RelationHandler):
         logger.info(f"Received data: {display_data}")
         self.callback_f(event)
 
+    def _on_database_relation_broken(
+        self, event: ops.framework.EventBase
+    ) -> None:
+        """Handle database gone away event."""
+        if self.mandatory:
+            self.status.set(BlockedStatus("integration missing"))
+
     def get_relation_data(self) -> dict:
         """Load the data from the relation for consumption in the handler."""
         if len(self.interface.relations) > 0:
@@ -1182,6 +1197,8 @@ class IdentityResourceRequiresHandler(RelationHandler):
         """Handles provider_goneaway  event."""
         logger.info("Keystone provider not available process any requests")
         self.callback_f(event)
+        if self.mandatory:
+            self.status.set(BlockedStatus("integration missing"))
 
     def _on_response_available(self, event) -> None:
         """Handles response available  events."""
@@ -1252,6 +1269,8 @@ class CeilometerServiceRequiresHandler(RelationHandler):
         """Handle gone_away  event."""
         logger.debug("Ceilometer service relation is departed/broken")
         self.callback_f(event)
+        if self.mandatory:
+            self.status.set(BlockedStatus("integration missing"))
 
     @property
     def ready(self) -> bool:

ops-sunbeam/tests/unit_tests/test_core.py

@@ -41,6 +41,7 @@ class TestOSBaseOperatorCharm(test_utils.CharmTestCase):
         """Charm test class setup."""
         self.container_calls = test_utils.ContainerCalls()
         super().setUp(sunbeam_charm, self.PATCHES)
+        self.mock_event = mock.MagicMock()
         self.harness = test_utils.get_harness(
             test_charms.MyCharm,
             test_charms.CHARM_METADATA,
@@ -57,7 +58,12 @@ class TestOSBaseOperatorCharm(test_utils.CharmTestCase):
 
     def test_relation_handlers_ready(self) -> None:
         """Test relation handlers are ready."""
-        self.assertTrue(self.harness.charm.relation_handlers_ready())
+        self.assertSetEqual(
+            self.harness.charm.get_mandatory_relations_not_ready(
+                self.mock_event
+            ),
+            set(),
+        )
 
 
 class TestOSBaseOperatorCharmK8S(test_utils.CharmTestCase):
@@ -76,6 +82,7 @@ class TestOSBaseOperatorCharmK8S(test_utils.CharmTestCase):
             charm_config=test_charms.CHARM_CONFIG,
             initial_charm_config=test_charms.INITIAL_CHARM_CONFIG,
         )
+        self.mock_event = mock.MagicMock()
         self.harness.begin()
         self.addCleanup(self.harness.cleanup)
 
@@ -100,7 +107,12 @@ class TestOSBaseOperatorCharmK8S(test_utils.CharmTestCase):
 
     def test_relation_handlers_ready(self) -> None:
         """Test relation handlers are ready."""
-        self.assertTrue(self.harness.charm.relation_handlers_ready())
+        self.assertSetEqual(
+            self.harness.charm.get_mandatory_relations_not_ready(
+                self.mock_event
+            ),
+            set(),
+        )
 
 
 class _TestOSBaseOperatorAPICharm(test_utils.CharmTestCase):
@@ -113,6 +125,7 @@ class _TestOSBaseOperatorAPICharm(test_utils.CharmTestCase):
         self.container_calls = test_utils.ContainerCalls()
 
         super().setUp(sunbeam_charm, self.PATCHES)
+        self.mock_event = mock.MagicMock()
         self.harness = test_utils.get_harness(
             charm_to_test,
             test_charms.API_CHARM_METADATA,
@@ -345,11 +358,21 @@ class TestOSBaseOperatorAPICharm(_TestOSBaseOperatorAPICharm):
         # Add all mandatory relations and test relation_handlers_ready
         db_rel_id = test_utils.add_base_db_relation(self.harness)
         test_utils.add_db_relation_credentials(self.harness, db_rel_id)
-        self.assertFalse(self.harness.charm.relation_handlers_ready())
+        self.assertSetEqual(
+            self.harness.charm.get_mandatory_relations_not_ready(
+                self.mock_event
+            ),
+            {"identity-service", "ingress-public", "amqp"},
+        )
 
         amqp_rel_id = test_utils.add_base_amqp_relation(self.harness)
         test_utils.add_amqp_relation_credentials(self.harness, amqp_rel_id)
-        self.assertFalse(self.harness.charm.relation_handlers_ready())
+        self.assertSetEqual(
+            self.harness.charm.get_mandatory_relations_not_ready(
+                self.mock_event
+            ),
+            {"ingress-public", "identity-service"},
+        )
 
         identity_rel_id = test_utils.add_base_identity_service_relation(
             self.harness
@@ -357,7 +380,12 @@ class TestOSBaseOperatorAPICharm(_TestOSBaseOperatorAPICharm):
         test_utils.add_identity_service_relation_response(
             self.harness, identity_rel_id
         )
-        self.assertFalse(self.harness.charm.relation_handlers_ready())
+        self.assertSetEqual(
+            self.harness.charm.get_mandatory_relations_not_ready(
+                self.mock_event
+            ),
+            {"ingress-public"},
+        )
 
         ingress_rel_id = test_utils.add_ingress_relation(
             self.harness, "public"
@@ -372,7 +400,12 @@ class TestOSBaseOperatorAPICharm(_TestOSBaseOperatorAPICharm):
         test_utils.add_ceph_access_relation_response(
             self.harness, ceph_access_rel_id
         )
-        self.assertTrue(self.harness.charm.relation_handlers_ready())
+        self.assertSetEqual(
+            self.harness.charm.get_mandatory_relations_not_ready(
+                self.mock_event
+            ),
+            set(),
+        )
 
         # Add an optional relation and test if relation_handlers_ready
         # returns True
@@ -382,12 +415,22 @@ class TestOSBaseOperatorAPICharm(_TestOSBaseOperatorAPICharm):
         test_utils.add_ingress_relation_data(
             self.harness, optional_rel_id, "internal"
         )
-        self.assertTrue(self.harness.charm.relation_handlers_ready())
+        self.assertSetEqual(
+            self.harness.charm.get_mandatory_relations_not_ready(
+                self.mock_event
+            ),
+            set(),
+        )
 
         # Remove a mandatory relation and test if relation_handlers_ready
         # returns False
         self.harness.remove_relation(ingress_rel_id)
-        self.assertFalse(self.harness.charm.relation_handlers_ready())
+        self.assertSetEqual(
+            self.harness.charm.get_mandatory_relations_not_ready(
+                self.mock_event
+            ),
+            {"ingress-public"},
+        )
 
         # Add the mandatory relation back and retest relation_handlers_ready
         ingress_rel_id = test_utils.add_ingress_relation(
@@ -396,7 +439,12 @@ class TestOSBaseOperatorAPICharm(_TestOSBaseOperatorAPICharm):
         test_utils.add_ingress_relation_data(
             self.harness, ingress_rel_id, "public"
         )
-        self.assertTrue(self.harness.charm.relation_handlers_ready())
+        self.assertSetEqual(
+            self.harness.charm.get_mandatory_relations_not_ready(
+                self.mock_event
+            ),
+            set(),
+        )
 
     def test_add_explicit_port(self):
         """Test add_explicit_port method."""