Add mandatory flag to relation handlers

Following changes are done in this patch:
* Add mandatory flag to relations handler classes default
to False.
* Add mandatory_relations to charm base classes to list
all the mandatory relations.
* update relation_handlers_ready based on mandatory_relations

Change-Id: Ibc846461cf92a0a6501a15d03907c93ecdf90063

ops-sunbeam/ops_sunbeam/charm.py

@@ -56,6 +56,9 @@ class OSBaseOperatorCharm(ops.charm.CharmBase):
 
     _state = ops.framework.StoredState()
 
+    # Holds set of mandatory relations
+    mandatory_relations = set()
+
     def __init__(self, framework: ops.framework.Framework) -> None:
         """Run constructor."""
         super().__init__(framework)
@@ -96,6 +99,7 @@ class OSBaseOperatorCharm(ops.charm.CharmBase):
                 self.configure_charm,
                 self.config.get("rabbit-user") or self.service_name,
                 self.config.get("rabbit-vhost") or "openstack",
+                "amqp" in self.mandatory_relations,
             )
             handlers.append(self.amqp)
 
@@ -103,18 +107,26 @@ class OSBaseOperatorCharm(ops.charm.CharmBase):
         for relation_name, database_name in self.databases.items():
             if self.can_add_handler(relation_name, handlers):
                 db = sunbeam_rhandlers.DBHandler(
-                    self, relation_name, self.configure_charm, database_name,
+                    self,
+                    relation_name,
+                    self.configure_charm,
+                    database_name,
+                    relation_name in self.mandatory_relations,
                 )
                 self.dbs[relation_name] = db
                 handlers.append(db)
         if self.can_add_handler("peers", handlers):
             self.peers = sunbeam_rhandlers.BasePeerHandler(
-                self, "peers", self.configure_charm
+                self, "peers", self.configure_charm, False
             )
             handlers.append(self.peers)
         if self.can_add_handler("certificates", handlers):
             self.certs = sunbeam_rhandlers.CertificatesHandler(
-                self, "certificates", self.configure_charm, self.get_sans(),
+                self,
+                "certificates",
+                self.configure_charm,
+                self.get_sans(),
+                "certificates" in self.mandatory_relations,
             )
             handlers.append(self.certs)
         if self.can_add_handler("cloud-credentials", handlers):
@@ -122,6 +134,7 @@ class OSBaseOperatorCharm(ops.charm.CharmBase):
                 self,
                 'cloud-credentials',
                 self.configure_charm,
+                'cloud-credentials' in self.mandatory_relations,
             )
             handlers.append(self.ccreds)
         return handlers
@@ -292,10 +305,18 @@ class OSBaseOperatorCharm(ops.charm.CharmBase):
 
     def relation_handlers_ready(self) -> bool:
         """Determine whether all relations are ready for use."""
-        for handler in self.relation_handlers:
+        ready_relations = {
-            if not handler.ready:
+            handler.relation_name
-                logger.info(f"Relation {handler.relation_name} incomplete")
+            for handler in self.relation_handlers
-                return False
+            if handler.mandatory and handler.ready
+        }
+        not_ready_relations = self.mandatory_relations.difference(
+            ready_relations)
+
+        if len(not_ready_relations) != 0:
+            logger.info(f"Relations {not_ready_relations} incomplete")
+            return False
+
         return True
 
     def contexts(self) -> sunbeam_core.OPSCharmContexts:
@@ -378,6 +399,12 @@ class OSBaseOperatorCharm(ops.charm.CharmBase):
 class OSBaseOperatorAPICharm(OSBaseOperatorCharm):
     """Base class for OpenStack API operators."""
 
+    mandatory_relations = {
+        'database',
+        'identity-service',
+        'ingress-public'
+    }
+
     def __init__(self, framework: ops.framework.Framework) -> None:
         """Run constructor."""
         super().__init__(framework)
@@ -406,6 +433,7 @@ class OSBaseOperatorAPICharm(OSBaseOperatorCharm):
                 self.service_name,
                 self.default_public_ingress_port,
                 self._ingress_changed,
+                "ingress-internal" in self.mandatory_relations,
             )
             handlers.append(self.ingress_internal)
         if self.can_add_handler("ingress-public", handlers):
@@ -415,6 +443,7 @@ class OSBaseOperatorAPICharm(OSBaseOperatorCharm):
                 self.service_name,
                 self.default_public_ingress_port,
                 self._ingress_changed,
+                "ingress-public" in self.mandatory_relations,
             )
             handlers.append(self.ingress_public)
         if self.can_add_handler("identity-service", handlers):
@@ -424,6 +453,7 @@ class OSBaseOperatorAPICharm(OSBaseOperatorCharm):
                 self.configure_charm,
                 self.service_endpoints,
                 self.model.config["region"],
+                "identity-service" in self.mandatory_relations,
             )
             handlers.append(self.id_svc)
         return super().get_relation_handlers(handlers)

ops-sunbeam/ops_sunbeam/ovn/charm.py

@@ -31,7 +31,10 @@ class OSBaseOVNOperatorCharm(sunbeam_charm.OSBaseOperatorCharm):
         handlers = handlers or []
         if self.can_add_handler("ovsdb-cms", handlers):
             self.ovsdb_cms = ovn_relation_handlers.OVSDBCMSRequiresHandler(
-                self, "ovsdb-cms", self.configure_charm,
+                self,
+                "ovsdb-cms",
+                self.configure_charm,
+                "ovsdb-cms" in self.mandatory_relations,
             )
             handlers.append(self.ovsdb_cms)
         handlers = super().get_relation_handlers(handlers)

ops-sunbeam/ops_sunbeam/ovn/relation_handlers.py

@@ -332,10 +332,11 @@ class OVSDBCMSProvidesHandler(sunbeam_rhandlers.RelationHandler,
         self,
         charm: ops.charm.CharmBase,
         relation_name: str,
-        callback_f: Callable
+        callback_f: Callable,
+        mandatory: bool = False,
     ) -> None:
         """Run constructor."""
-        super().__init__(charm, relation_name, callback_f)
+        super().__init__(charm, relation_name, callback_f, mandatory)
 
     def setup_event_handler(self) -> ops.charm.Object:
         """Configure event handlers for an Identity service relation."""
@@ -376,10 +377,11 @@ class OVSDBCMSRequiresHandler(sunbeam_rhandlers.RelationHandler,
         self,
         charm: ops.charm.CharmBase,
         relation_name: str,
-        callback_f: Callable
+        callback_f: Callable,
+        mandatory: bool = False,
     ) -> None:
         """Run constructor."""
-        super().__init__(charm, relation_name, callback_f)
+        super().__init__(charm, relation_name, callback_f, mandatory)
 
     def setup_event_handler(self) -> ops.charm.Object:
         """Configure event handlers for an Identity service relation."""

ops-sunbeam/ops_sunbeam/relation_handlers.py

@@ -50,6 +50,7 @@ class RelationHandler(ops.charm.Object):
         charm: ops.charm.CharmBase,
         relation_name: str,
         callback_f: Callable,
+        mandatory: bool = False,
     ) -> None:
         """Run constructor."""
         super().__init__(
@@ -62,6 +63,7 @@ class RelationHandler(ops.charm.Object):
         self.relation_name = relation_name
         self.callback_f = callback_f
         self.interface = self.setup_event_handler()
+        self.mandatory = mandatory
 
     def setup_event_handler(self) -> ops.charm.Object:
         """Configure event handlers for the relation.
@@ -113,11 +115,12 @@ class IngressHandler(RelationHandler):
         service_name: str,
         default_ingress_port: int,
         callback_f: Callable,
+        mandatory: bool = False,
     ) -> None:
         """Run constructor."""
         self.default_ingress_port = default_ingress_port
         self.service_name = service_name
-        super().__init__(charm, relation_name, callback_f)
+        super().__init__(charm, relation_name, callback_f, mandatory)
 
     def setup_event_handler(self) -> ops.charm.Object:
         """Configure event handlers for an Ingress relation."""
@@ -202,11 +205,12 @@ class DBHandler(RelationHandler):
         relation_name: str,
         callback_f: Callable,
         database: str,
+        mandatory: bool = False,
     ) -> None:
         """Run constructor."""
         # a database name as requested by the charm.
         self.database_name = database
-        super().__init__(charm, relation_name, callback_f)
+        super().__init__(charm, relation_name, callback_f, mandatory)
 
     def setup_event_handler(self) -> ops.charm.Object:
         """Configure event handlers for a MySQL relation."""
@@ -316,11 +320,12 @@ class AMQPHandler(RelationHandler):
         callback_f: Callable,
         username: str,
         vhost: int,
+        mandatory: bool = False,
     ) -> None:
         """Run constructor."""
         self.username = username
         self.vhost = vhost
-        super().__init__(charm, relation_name, callback_f)
+        super().__init__(charm, relation_name, callback_f, mandatory)
 
     def setup_event_handler(self) -> ops.charm.Object:
         """Configure event handlers for an AMQP relation."""
@@ -388,11 +393,12 @@ class IdentityServiceRequiresHandler(RelationHandler):
         callback_f: Callable,
         service_endpoints: dict,
         region: str,
+        mandatory: bool = False,
     ) -> None:
         """Run constructor."""
         self.service_endpoints = service_endpoints
         self.region = region
-        super().__init__(charm, relation_name, callback_f)
+        super().__init__(charm, relation_name, callback_f, mandatory)
 
     def setup_event_handler(self) -> ops.charm.Object:
         """Configure event handlers for an Identity service relation."""
@@ -514,12 +520,13 @@ class CephClientHandler(RelationHandler):
         relation_name: str,
         callback_f: Callable,
         allow_ec_overwrites: bool = True,
-        app_name: str = None
+        app_name: str = None,
+        mandatory: bool = False,
     ) -> None:
         """Run constructor."""
         self.allow_ec_overwrites = allow_ec_overwrites
         self.app_name = app_name
-        super().__init__(charm, relation_name, callback_f)
+        super().__init__(charm, relation_name, callback_f, mandatory)
 
     def setup_event_handler(self) -> ops.charm.Object:
         """Configure event handlers for an ceph-client interface."""
@@ -662,6 +669,7 @@ class CertificatesHandler(RelationHandler):
         relation_name: str,
         callback_f: Callable,
         sans: List[str] = None,
+        mandatory: bool = False,
     ) -> None:
         """Run constructor."""
         # Lazy import to ensure this lib is only required if the charm
@@ -669,7 +677,7 @@ class CertificatesHandler(RelationHandler):
         import interface_tls_certificates.ca_client as ca_client
         self.ca_client = ca_client
         self.sans = sans
-        super().__init__(charm, relation_name, callback_f)
+        super().__init__(charm, relation_name, callback_f, mandatory)
 
     def setup_event_handler(self) -> None:
         """Configure event handlers for peer relation."""
@@ -742,6 +750,7 @@ class CloudCredentialsRequiresHandler(RelationHandler):
         charm: ops.charm.CharmBase,
         relation_name: str,
         callback_f: Callable,
+        mandatory: bool = False,
     ) -> None:
         """Create a new cloud-credentials handler.
 
@@ -756,7 +765,7 @@ class CloudCredentialsRequiresHandler(RelationHandler):
         :param callback_f: the function to call when the nodes are connected
         :type callback_f: Callable
         """
-        super().__init__(charm, relation_name, callback_f)
+        super().__init__(charm, relation_name, callback_f, mandatory)
 
     def setup_event_handler(self) -> ops.charm.Object:
         """Configure event handlers for cloud-credentials relation."""

ops-sunbeam/unit_tests/test_charms.py

@@ -203,6 +203,9 @@ class MyAPICharm(sunbeam_charm.OSBaseOperatorAPICharm):
     service_name = "my-service"
     wsgi_admin_script = "/bin/wsgi_admin"
     wsgi_public_script = "/bin/wsgi_public"
+    mandatory_relations = {
+        "database", "amqp", "identity-service", "ingress-public"
+    }
 
     def __init__(self, framework: "ops.framework.Framework") -> None:
         """Run constructor."""

ops-sunbeam/unit_tests/test_core.py

@@ -250,3 +250,53 @@ class TestOSBaseOperatorAPICharm(test_utils.CharmTestCase):
         self.assertEqual(
             self.harness.charm.public_url,
             'http://10.0.0.10:789')
+
+    def test_relation_handlers_ready(self) -> None:
+        """Test relation handlers are ready."""
+        # 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())
+
+        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())
+
+        identity_rel_id = test_utils.add_base_identity_service_relation(
+            self.harness)
+        test_utils.add_identity_service_relation_response(
+            self.harness, identity_rel_id)
+        self.assertFalse(
+            self.harness.charm.relation_handlers_ready())
+
+        ingress_rel_id = test_utils.add_ingress_relation(
+            self.harness, 'public')
+        test_utils.add_ingress_relation_data(
+            self.harness, ingress_rel_id, 'public')
+        self.assertTrue(
+            self.harness.charm.relation_handlers_ready())
+
+        # Add an optional relation and test if relation_handlers_ready
+        # returns True
+        optional_rel_id = test_utils.add_ingress_relation(
+            self.harness, 'internal')
+        test_utils.add_ingress_relation_data(
+            self.harness, optional_rel_id, 'internal')
+        self.assertTrue(
+            self.harness.charm.relation_handlers_ready())
+
+        # 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())
+
+        # Add the mandatory relation back and retest relation_handlers_ready
+        ingress_rel_id = test_utils.add_ingress_relation(
+            self.harness, 'public')
+        test_utils.add_ingress_relation_data(
+            self.harness, ingress_rel_id, 'public')
+        self.assertTrue(
+            self.harness.charm.relation_handlers_ready())