apply PR suggestions rd 1

lib/charms/mongodb/v1/helpers.py

@@ -29,7 +29,7 @@
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 1
+LIBPATCH = 0
 
 # path to store mongodb ketFile
 KEY_FILE = "keyFile"
@@ -201,35 +201,6 @@ def generate_keyfile() -> str:
     return "".join([secrets.choice(choices) for _ in range(1024)])
 
 
-def prioritise_statuses(mongodb_status, pbm_status, shard_status, config_server_status):
-    """Returns the status with the highest priority of the given statuses.
-
-    Note: it will never be the case that shard_status and config_server_status are both present
-    since the mongodb app can either be a shard or a config server, but not both.
-    """
-    # failure in mongodb takes precedence over sharding and config server
-    if not isinstance(mongodb_status, ActiveStatus):
-        return mongodb_status
-
-    if shard_status and not isinstance(shard_status, ActiveStatus):
-        return shard_status
-
-    if config_server_status and not isinstance(config_server_status, ActiveStatus):
-        return config_server_status
-
-    if pbm_status and not isinstance(pbm_status, ActiveStatus):
-        return pbm_status
-
-    # if all statuses are active report sharding statuses over mongodb status
-    if isinstance(shard_status, ActiveStatus):
-        return shard_status
-
-    if isinstance(config_server_status, ActiveStatus):
-        return config_server_status
-
-    return mongodb_status
-
-
 def build_unit_status(mongodb_config: MongoDBConfiguration, unit_ip: str) -> StatusBase:
     """Generates the status of a unit based on its status reported by mongod."""
     try:

lib/charms/mongodb/v1/mongodb_provider.py

@@ -21,8 +21,6 @@
 from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, Relation
 from pymongo.errors import PyMongoError
 
-from config import Config
-
 # The unique Charmhub library identifier, never change it
 LIBID = "4067879ef7dd4261bf6c164bc29d94b1"
 
@@ -86,14 +84,8 @@ def __init__(self, charm: CharmBase, substrate="k8s", relation_name: str = "data
 
     def pass_hook_checks(self) -> bool:
         """Runs the pre-hooks checks for MongoDBProvider, returns True if all pass."""
-        if self.charm.is_role(Config.Role.SHARD) or self.charm.is_role(Config.Role.CONFIG_SERVER):
-            self.charm.unit.status = BlockedStatus(
-                "Sharding roles do not support mongodb_client interface."
-            )
-            logger.error(
-                "Charm is in sharding role: %s. Does not support mongodb_client interface.",
-                self.charm.role,
-            )
+        if not self.charm.is_relation_feasible(self.relation_name):
+            logger.info("Skipping code for relations.")
             return False
 
         # legacy relations have auth disabled, which new relations require

lib/charms/mongodb/v1/mongodb_vm_legacy_provider.py

@@ -13,8 +13,6 @@
 from ops.framework import Object
 from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus
 
-from config import Config
-
 # The unique Charmhub library identifier, never change it
 LIBID = "896a48bc89b84d30839335bb37170509"
 
@@ -43,6 +41,7 @@ def __init__(self, charm):
         """Manager of MongoDB client relations."""
         super().__init__(charm, "client-relations")
         self.charm = charm
+        self.relation_name = LEGACY_REL_NAME
         self.framework.observe(
             self.charm.on[LEGACY_REL_NAME].relation_created, self._on_legacy_relation_created
         )
@@ -66,14 +65,8 @@ def _on_legacy_relation_created(self, event):
             )
             return
 
-        if self.charm.is_role(Config.Role.SHARD) or self.charm.is_role(Config.Role.CONFIG_SERVER):
-            self.charm.unit.status = BlockedStatus(
-                "Sharding roles do not support mongodb interface."
-            )
-            logger.error(
-                "Charm is in sharding role: %s. Does not support mongodb_client interface.",
-                self.charm.role,
-            )
+        if not self.charm.is_relation_feasible(self.relation_name):
+            logger.info("Skipping code for legacy relations.")
             return
 
         # If auth is already disabled its likely it has a connection with another legacy relation

lib/charms/mongodb/v1/shards_interface.py

@@ -29,7 +29,13 @@
 from charms.mongodb.v1.users import MongoDBUser, OperatorUser
 from ops.charm import CharmBase, EventBase, RelationBrokenEvent
 from ops.framework import Object
-from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus
+from ops.model import (
+    ActiveStatus,
+    BlockedStatus,
+    MaintenanceStatus,
+    StatusBase,
+    WaitingStatus,
+)
 from tenacity import RetryError, Retrying, stop_after_delay, wait_fixed
 
 from config import Config
@@ -107,12 +113,8 @@ def _on_relation_joined(self, event):
 
     def pass_hook_checks(self, event: EventBase) -> bool:
         """Runs the pre-hooks checks for ShardingProvider, returns True if all pass."""
-        if self.charm.is_role(Config.Role.REPLICATION):
-            self.charm.unit.status = BlockedStatus("role replication does not support sharding")
-            logger.error(
-                "Skipping %s. Sharding interface not supported with config role=replication.",
-                type(event),
-            )
+        if not self.charm.is_relation_feasible(self.relation_name):
+            logger.info("Skipping event %s , relation not feasible.", type(event))
             return False
 
         if not self.charm.is_role(Config.Role.CONFIG_SERVER):
@@ -388,9 +390,8 @@ def _on_relation_changed(self, event):
 
     def pass_hook_checks(self, event):
         """Runs the pre-hooks checks for ConfigServerRequirer, returns True if all pass."""
-        if self.charm.is_role(Config.Role.REPLICATION):
-            self.charm.unit.status = BlockedStatus("role replication does not support sharding")
-            logger.error("sharding interface not supported with config role=replication")
+        if not self.charm.is_relation_feasible(self.relation_name):
+            logger.info("Skipping event %s , relation not feasible.", type(event))
             return False
 
         if not self.charm.is_role(Config.Role.SHARD):
@@ -468,7 +469,7 @@ def wait_for_draining(self, mongos_hosts: List[str]):
 
                 break
 
-    def get_shard_status(self):
+    def get_shard_status(self) -> StatusBase:
         """Returns the current status of the shard.
 
         Note: No need to report if currently draining, since that check block other hooks from
@@ -498,7 +499,7 @@ def get_shard_status(self):
             return BlockedStatus("Config server unreachable")
 
         if not self._is_added_to_cluster():
-            self.charm.unit.status = MaintenanceStatus("Adding shard to config-server")
+            return MaintenanceStatus("Adding shard to config-server")
 
         if not self._is_shard_aware():
             return BlockedStatus("Shard is not yet shard aware")
@@ -618,6 +619,9 @@ def _is_mongos_reachable(self) -> bool:
             return False
 
         mongos_hosts = self.get_mongos_hosts()
+        if not mongos_hosts:
+            return False
+
         self.charm.remote_mongos_config(set(mongos_hosts))
         config = self.charm.remote_mongos_config(set(mongos_hosts))
 
@@ -649,13 +653,17 @@ def has_config_server(self) -> bool:
 
     def get_related_config_server(self) -> str:
         """Returns the related config server."""
-        config_server = [rel.app.name for rel in self.charm.model.relations[self.relation_name]]
+        if self.relation_name not in self.charm.model.relations:
+            return None
 
         # metadata.yaml prevents having multiple config servers
-        return config_server[0]
+        return self.charm.model.relations[self.relation_name][0].app.name
 
     def get_mongos_hosts(self) -> List[str]:
         """Returns a list of IP addresses for the mongos hosts."""
         # only one related config-server is possible
         config_server_relation = self.charm.model.relations[self.relation_name][0]
+        if HOSTS_KEY not in config_server_relation.data[config_server_relation.app]:
+            return
+
         return json.loads(config_server_relation.data[config_server_relation.app].get(HOSTS_KEY))

src/charm.py

@@ -31,7 +31,6 @@
     generate_keyfile,
     generate_password,
     get_create_user_cmd,
-    prioritise_statuses,
 )
 from charms.mongodb.v1.mongodb_backups import S3_RELATION, MongoDBBackups
 from charms.mongodb.v1.mongodb_provider import MongoDBProvider
@@ -67,6 +66,7 @@
     BlockedStatus,
     MaintenanceStatus,
     Relation,
+    StatusBase,
     Unit,
     WaitingStatus,
 )
@@ -575,25 +575,7 @@ def _on_update_status(self, event: UpdateStatusEvent):
         if self.unit.is_leader():
             self._handle_reconfigure(event)
 
-        # retrieve statuses of different services running on Charmed MongoDB
-        mongodb_status = build_unit_status(self.mongodb_config, self._unit_ip(self.unit))
-        shard_status = self.shard.get_shard_status() if self.is_role(Config.Role.SHARD) else None
-        config_server_status = (
-            self.config_server.get_config_server_status()
-            if self.is_role(Config.Role.CONFIG_SERVER)
-            else None
-        )
-        pbm_status = (
-            self.backups.get_pbm_status() if self.model.get_relation(S3_RELATION) else None
-        )
-
-        # prioritize the statuses of the different services running on Charmed MongoDB
-        self.unit.status = prioritise_statuses(
-            mongodb_status,
-            pbm_status,
-            shard_status,
-            config_server_status,
-        )
+        self.unit.status = self.get_status()
 
     def _on_get_primary_action(self, event: ActionEvent):
         event.set_results({"replica-set-primary": self._primary})
@@ -1367,6 +1349,77 @@ def _is_removing_last_replica(self) -> bool:
         """Returns True if the last replica (juju unit) is getting removed."""
         return self.app.planned_units() == 0 and len(self._peers.units) == 0
 
+    def get_status(self) -> StatusBase:
+        """Returns the status with the highest priority from backups, sharding, and mongod.
+
+        Note: it will never be the case that shard_status and config_server_status are both present
+        since the mongodb app can either be a shard or a config server, but not both.
+        """
+        # retrieve statuses of different services running on Charmed MongoDB
+        mongodb_status = build_unit_status(self.mongodb_config, self._unit_ip(self.unit))
+        shard_status = self.shard.get_shard_status() if self.is_role(Config.Role.SHARD) else None
+        config_server_status = (
+            self.config_server.get_config_server_status()
+            if self.is_role(Config.Role.CONFIG_SERVER)
+            else None
+        )
+        pbm_status = (
+            self.backups.get_pbm_status() if self.model.get_relation(S3_RELATION) else None
+        )
+
+        # failure in mongodb takes precedence over sharding and config server
+        if not isinstance(mongodb_status, ActiveStatus):
+            return mongodb_status
+
+        if shard_status and not isinstance(shard_status, ActiveStatus):
+            return shard_status
+
+        if config_server_status and not isinstance(config_server_status, ActiveStatus):
+            return config_server_status
+
+        if pbm_status and not isinstance(pbm_status, ActiveStatus):
+            return pbm_status
+
+        # if all statuses are active report sharding statuses over mongodb status
+        if isinstance(shard_status, ActiveStatus):
+            return shard_status
+
+        if isinstance(config_server_status, ActiveStatus):
+            return config_server_status
+
+        return mongodb_status
+
+    def is_relation_feasible(self, rel_interface) -> bool:
+        """Returns true if the proposed relation is feasible."""
+        if self.is_sharding_component() and rel_interface in Config.Relations.DB_RELATIONS:
+            self.unit.status = BlockedStatus(
+                f"Sharding roles do not support {rel_interface} interface."
+            )
+            logger.error(
+                "Charm is in sharding role: %s. Does not support %s interface.",
+                rel_interface,
+                self.role,
+            )
+            return False
+
+        if (
+            not self.is_sharding_component()
+            and rel_interface != Config.Relations.SHARDING_RELATIONS_NAME
+        ):
+            self.unit.status = BlockedStatus("role replication does not support sharding")
+            logger.error(
+                "Charm is in sharding role: %s. Does not support %s interface.",
+                self.role,
+                rel_interface,
+            )
+            return False
+
+        return True
+
+    def is_sharding_component(self) -> bool:
+        """Returns true if charm is running as a sharded component."""
+        return self.is_role(Config.Role.SHARD) or self.is_role(Config.Role.CONFIG_SERVER)
+
     # END: helper functions
 
 

src/config.py

@@ -76,6 +76,7 @@ class Relations:
         CONFIG_SERVER_RELATIONS_NAME = "config-server"
         APP_SCOPE = "app"
         UNIT_SCOPE = "unit"
+        DB_RELATIONS = [OBSOLETE_RELATIONS_NAME, NAME]
         Scopes = Literal[APP_SCOPE, UNIT_SCOPE]
 
     class Secrets: