diff --git a/ops-sunbeam/ops_sunbeam/charm.py b/ops-sunbeam/ops_sunbeam/charm.py index 92f5fdd7..519bd044 100644 --- a/ops-sunbeam/ops_sunbeam/charm.py +++ b/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: - if not handler.ready: - logger.info(f"Relation {handler.relation_name} incomplete") - return False + ready_relations = { + handler.relation_name + for handler in self.relation_handlers + 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) diff --git a/ops-sunbeam/ops_sunbeam/ovn/charm.py b/ops-sunbeam/ops_sunbeam/ovn/charm.py index f566938b..4d105452 100644 --- a/ops-sunbeam/ops_sunbeam/ovn/charm.py +++ b/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) diff --git a/ops-sunbeam/ops_sunbeam/ovn/relation_handlers.py b/ops-sunbeam/ops_sunbeam/ovn/relation_handlers.py index c36835fe..c7397c3b 100644 --- a/ops-sunbeam/ops_sunbeam/ovn/relation_handlers.py +++ b/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.""" diff --git a/ops-sunbeam/ops_sunbeam/relation_handlers.py b/ops-sunbeam/ops_sunbeam/relation_handlers.py index ce22e6e8..ac9699f2 100644 --- a/ops-sunbeam/ops_sunbeam/relation_handlers.py +++ b/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.""" diff --git a/ops-sunbeam/unit_tests/test_charms.py b/ops-sunbeam/unit_tests/test_charms.py index 04ab7cfc..6afccf91 100644 --- a/ops-sunbeam/unit_tests/test_charms.py +++ b/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.""" diff --git a/ops-sunbeam/unit_tests/test_core.py b/ops-sunbeam/unit_tests/test_core.py index cc2a8a01..f4f3d3a5 100644 --- a/ops-sunbeam/unit_tests/test_core.py +++ b/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())