[DPE-5237] move revision checker to re-usable helpers (#482)

## Issue revision checker is needed by K8s charm. ## Solution rather than copy + pasting it into `src/charm.py` lets move it to a re-usable helper
canonical · Sep 11, 2024 · 1e823b3 · 1e823b3
1 parent 6132042
commit 1e823b3
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 87 deletions.
diff --git a/lib/charms/mongodb/v0/set_status.py b/lib/charms/mongodb/v0/set_status.py
@@ -7,6 +7,7 @@
 from typing import Optional, Tuple
 
 from charms.mongodb.v1.mongodb import MongoConfiguration, MongoDBConnection
+from data_platform_helpers.version_check import NoVersionError, get_charm_revision
 from ops.charm import CharmBase
 from ops.framework import Object
 from ops.model import ActiveStatus, BlockedStatus, StatusBase, WaitingStatus
@@ -22,7 +23,7 @@
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 4
+LIBPATCH = 5
 
 AUTH_FAILED_CODE = 18
 UNAUTHORISED_CODE = 13
@@ -88,7 +89,7 @@ def is_status_related_to_mismatched_revision(self, status_type: str) -> bool:
         "goal state" which processes data differently and the other via the ".status" property.
         Hence we have to be flexible to handle each.
         """
-        if not self.charm.get_cluster_mismatched_revision_status():
+        if not self.get_cluster_mismatched_revision_status():
             return False
 
         if "waiting" in status_type and self.charm.is_role(Config.Role.CONFIG_SERVER):
@@ -235,7 +236,49 @@ def get_invalid_integration_status(self) -> Optional[StatusBase]:
                 "Relation to s3-integrator is not supported, config role must be config-server"
             )
 
-        return self.charm.get_cluster_mismatched_revision_status()
+        return self.get_cluster_mismatched_revision_status()
+
+    def get_cluster_mismatched_revision_status(self) -> Optional[StatusBase]:
+        """Returns a Status if the cluster has mismatched revisions."""
+        # check for invalid versions in sharding integrations, i.e. a shard running on
+        # revision  88 and a config-server running on revision 110
+        current_charms_version = get_charm_revision(
+            self.charm.unit, local_version=self.charm.get_charm_internal_revision
+        )
+        local_identifier = (
+            "-locally built"
+            if self.charm.version_checker.is_local_charm(self.charm.app.name)
+            else ""
+        )
+        try:
+            if self.charm.version_checker.are_related_apps_valid():
+                return
+        except NoVersionError as e:
+            # relations to shards/config-server are expected to provide a version number. If they
+            # do not, it is because they are from an earlier charm revision, i.e. pre-revison X.
+            logger.debug(e)
+            if self.charm.is_role(Config.Role.SHARD):
+                return BlockedStatus(
+                    f"Charm revision ({current_charms_version}{local_identifier}) is not up-to date with config-server."
+                )
+
+        if self.charm.is_role(Config.Role.SHARD):
+            config_server_revision = self.charm.version_checker.get_version_of_related_app(
+                self.get_config_server_name()
+            )
+            remote_local_identifier = (
+                "-locally built"
+                if self.charm.version_checker.is_local_charm(self.get_config_server_name())
+                else ""
+            )
+            return BlockedStatus(
+                f"Charm revision ({current_charms_version}{local_identifier}) is not up-to date with config-server ({config_server_revision}{remote_local_identifier})."
+            )
+
+        if self.charm.is_role(Config.Role.CONFIG_SERVER):
+            return WaitingStatus(
+                f"Waiting for shards to upgrade/downgrade to revision {current_charms_version}{local_identifier}."
+            )
 
 
 def build_unit_status(mongodb_config: MongoConfiguration, unit_host: str) -> StatusBase:

diff --git a/src/charm.py b/src/charm.py
@@ -44,7 +44,6 @@
 from charms.operator_libs_linux.v2 import snap
 from data_platform_helpers.version_check import (
     CrossAppVersionChecker,
-    NoVersionError,
     get_charm_revision,
 )
 from ops import StorageAttachedEvent
@@ -70,7 +69,6 @@
     BlockedStatus,
     MaintenanceStatus,
     Relation,
-    StatusBase,
     Unit,
     WaitingStatus,
 )
@@ -139,11 +137,8 @@ def __init__(self, *args):
         self.tls = MongoDBTLS(self, Config.Relations.PEERS, substrate=Config.SUBSTRATE)
         self.backups = MongoDBBackups(self)
 
-        # TODO future PR - support pinning mongos version
         self.version_checker = CrossAppVersionChecker(
             self,
-            # TODO future PR add ops model revision variable:
-            # https://github.com/canonical/operator/issues/1255
             version=get_charm_revision(self.unit, local_version=self.get_charm_internal_revision),
             relations_to_check=[
                 Config.Relations.SHARDING_RELATIONS_NAME,
@@ -1541,7 +1536,7 @@ def is_relation_feasible(self, rel_interface) -> bool:
             )
             return False
 
-        if revision_mismatch_status := self.get_cluster_mismatched_revision_status():
+        if revision_mismatch_status := self.status.get_cluster_mismatched_revision_status():
             self.status.set_and_share_status(revision_mismatch_status)
             return False
 
@@ -1562,48 +1557,6 @@ def is_cluster_on_same_revision(self) -> bool:
 
         return self.version_checker.are_related_apps_valid()
 
-    def get_cluster_mismatched_revision_status(self) -> Optional[StatusBase]:
-        """Returns a Status if the cluster has mismatched revisions."""
-        # check for invalid versions in sharding integrations, i.e. a shard running on
-        # revision  88 and a config-server running on revision 110
-        current_charms_version = get_charm_revision(
-            self.unit, local_version=self.get_charm_internal_revision
-        )
-        local_identifier = (
-            "-locally built" if self.version_checker.is_local_charm(self.app.name) else ""
-        )
-        try:
-            if self.version_checker.are_related_apps_valid():
-                return
-        except NoVersionError as e:
-            # relations to shards/config-server are expected to provide a version number. If they
-            # do not, it is because they are from an earlier charm revision, i.e. pre-revison X.
-            # TODO once charm is published, determine which revision X is.
-            logger.debug(e)
-            if self.is_role(Config.Role.SHARD):
-                return BlockedStatus(
-                    f"Charm revision ({current_charms_version}{local_identifier}) is not up-to date with config-server."
-                )
-
-        if self.is_role(Config.Role.SHARD):
-            config_server_revision = self.version_checker.get_version_of_related_app(
-                self.get_config_server_name()
-            )
-            remote_local_identifier = (
-                "-locally built"
-                if self.version_checker.is_local_charm(self.get_config_server_name())
-                else ""
-            )
-            return BlockedStatus(
-                f"Charm revision ({current_charms_version}{local_identifier}) is not up-to date with config-server ({config_server_revision}{remote_local_identifier})."
-            )
-
-        if self.is_role(Config.Role.CONFIG_SERVER):
-            # TODO Future PR add handling for integrated mongos charms
-            return WaitingStatus(
-                f"Waiting for shards to upgrade/downgrade to revision {current_charms_version}{local_identifier}."
-            )
-
     def get_config_server_name(self) -> Optional[str]:
         """Returns the name of the Juju Application that the shard is using as a config server."""
         if not self.is_role(Config.Role.SHARD):

diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
@@ -210,7 +210,7 @@ def test_mongodb_relation_joined_non_leader_does_nothing(self, connection):
     @patch("charm.MongoDBConnection")
     @patch("charm.CrossAppVersionChecker.is_local_charm")
     @patch("charm.CrossAppVersionChecker.is_integrated_to_locally_built_charm")
-    @patch("charm.get_charm_revision")
+    @patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch("charm.MongodbOperatorCharm._connect_mongodb_exporter")
     def test_mongodb_relation_joined_all_replicas_not_ready(
         self, _, rev, local, is_local, connection
@@ -243,7 +243,7 @@ def test_mongodb_relation_joined_all_replicas_not_ready(
     @patch("charms.mongodb.v0.mongo.MongoClient")
     @patch("charm.CrossAppVersionChecker.is_local_charm")
     @patch("charm.CrossAppVersionChecker.is_integrated_to_locally_built_charm")
-    @patch("charm.get_charm_revision")
+    @patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch("charm.MongodbOperatorCharm._connect_mongodb_exporter")
     def test_relation_joined_get_members_failure(
         self, _, rev, local, is_local, client, connection, defer
@@ -283,7 +283,7 @@ def test_relation_joined_get_members_failure(
     @patch("charm.MongoDBConnection")
     @patch("charm.CrossAppVersionChecker.is_local_charm")
     @patch("charm.CrossAppVersionChecker.is_integrated_to_locally_built_charm")
-    @patch("charm.get_charm_revision")
+    @patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch("charm.MongodbOperatorCharm._connect_mongodb_exporter")
     def test_reconfigure_add_member_failure(self, _, rev, local, is_local, connection, defer):
         """Tests reconfigure does not proceed when unable to add a member.
@@ -350,7 +350,7 @@ def test_initialise_replica_failure_leads_to_waiting_state(
     @patch_network_get(private_address="1.1.1.1")
     @patch("charm.CrossAppVersionChecker.is_local_charm")
     @patch("charm.CrossAppVersionChecker.is_integrated_to_locally_built_charm")
-    @patch("charm.get_charm_revision")
+    @patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch("charms.mongodb.v0.set_status.MongoDBConnection")
     @patch("charm.MongoDBConnection")
     @patch("charm.MongoDBBackups.get_pbm_status")
@@ -396,7 +396,7 @@ def test_update_status_mongodb_error(
     @patch_network_get(private_address="1.1.1.1")
     @patch("charm.CrossAppVersionChecker.is_local_charm")
     @patch("charm.CrossAppVersionChecker.is_integrated_to_locally_built_charm")
-    @patch("charm.get_charm_revision")
+    @patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch("charms.mongodb.v0.set_status.MongoDBConnection")
     @patch("charm.MongoDBConnection")
     @patch("charm.MongoDBBackups.get_pbm_status")
@@ -437,7 +437,7 @@ def test_update_status_pbm_error(
     @patch_network_get(private_address="1.1.1.1")
     @patch("charm.CrossAppVersionChecker.is_local_charm")
     @patch("charm.CrossAppVersionChecker.is_integrated_to_locally_built_charm")
-    @patch("charm.get_charm_revision")
+    @patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch("charms.mongodb.v0.set_status.MongoDBConnection")
     @patch("charm.MongoDBConnection")
     @patch("charm.MongoDBBackups.get_pbm_status")
@@ -470,7 +470,7 @@ def test_update_status_pbm_and_mongodb_ready(
     @patch_network_get(private_address="1.1.1.1")
     @patch("charm.CrossAppVersionChecker.is_local_charm")
     @patch("charm.CrossAppVersionChecker.is_integrated_to_locally_built_charm")
-    @patch("charm.get_charm_revision")
+    @patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch("charms.mongodb.v0.set_status.MongoDBConnection")
     @patch("charm.MongoDBConnection")
     @patch("charms.mongodb.v0.set_status.build_unit_status")
@@ -501,7 +501,7 @@ def test_update_status_no_s3(
     @patch_network_get(private_address="1.1.1.1")
     @patch("charm.CrossAppVersionChecker.is_local_charm")
     @patch("charm.CrossAppVersionChecker.is_integrated_to_locally_built_charm")
-    @patch("charm.get_charm_revision")
+    @patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch("charms.mongodb.v0.set_status.MongoDBConnection")
     @patch("charm.MongoDBConnection")
     @patch("charm.MongoDBBackups.get_pbm_status")
@@ -533,7 +533,7 @@ def test_update_status_primary(
     @patch_network_get(private_address="1.1.1.1")
     @patch("charm.CrossAppVersionChecker.is_local_charm")
     @patch("charm.CrossAppVersionChecker.is_integrated_to_locally_built_charm")
-    @patch("charm.get_charm_revision")
+    @patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch("charms.mongodb.v0.set_status.MongoDBConnection")
     @patch("charm.MongoDBConnection")
     @patch("charm.MongoDBBackups.get_pbm_status")
@@ -565,7 +565,7 @@ def test_update_status_secondary(
     @patch_network_get(private_address="1.1.1.1")
     @patch("charm.CrossAppVersionChecker.is_local_charm")
     @patch("charm.CrossAppVersionChecker.is_integrated_to_locally_built_charm")
-    @patch("charm.get_charm_revision")
+    @patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch("charms.mongodb.v0.set_status.MongoDBConnection")
     @patch("charm.MongoDBConnection")
     @patch("charm.MongoDBBackups.get_pbm_status")
@@ -617,7 +617,7 @@ def test_update_status_additional_messages(
 
     @patch("charm.CrossAppVersionChecker.is_local_charm")
     @patch("charm.CrossAppVersionChecker.is_integrated_to_locally_built_charm")
-    @patch("charm.get_charm_revision")
+    @patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch("charm.MongodbOperatorCharm.get_secret")
     @patch_network_get(private_address="1.1.1.1")
     @patch("charm.MongoDBConnection")

diff --git a/tests/unit/test_config_server_lib.py b/tests/unit/test_config_server_lib.py
@@ -15,6 +15,7 @@
 
 class TestConfigServerInterface(unittest.TestCase):
     @mock.patch("charm.get_charm_revision")
+    @mock.patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch_network_get(private_address="1.1.1.1")
     def setUp(self, *unused):
         self.harness = Harness(MongodbOperatorCharm)
@@ -61,12 +62,11 @@ def is_config_mock_call(*args):
     @mock.patch(
         "data_platform_helpers.version_check.CrossAppVersionChecker.is_integrated_to_locally_built_charm"
     )
-    @mock.patch("charm.get_charm_revision")
+    @mock.patch("charms.mongodb.v0.set_status.get_charm_revision")
     @mock.patch("ops.framework.EventBase.defer")
     def test_relation_changed_fail_if_no_database_field(
         self, defer, get_charm_rev, is_integrated, is_local, oversee
     ):
-
         def is_config_mock_call(*args):
             assert args == ("config-server",)
             return True
@@ -125,7 +125,7 @@ def is_config_mock_call(*args):
     @mock.patch(
         "data_platform_helpers.version_check.CrossAppVersionChecker.is_integrated_to_locally_built_charm"
     )
-    @mock.patch("charm.get_charm_revision")
+    @mock.patch("charms.mongodb.v0.set_status.get_charm_revision")
     def test_pass_hooks_check_waits_for_start_config_server(
         self,
         get_rev,
@@ -163,7 +163,7 @@ def is_shard_mock_call(*args):
     @mock.patch(
         "data_platform_helpers.version_check.CrossAppVersionChecker.is_integrated_to_locally_built_charm"
     )
-    @mock.patch("charm.get_charm_revision")
+    @mock.patch("charms.mongodb.v0.set_status.get_charm_revision")
     def test_pass_hooks_check_waits_for_start_shard(
         self, get_rev, is_local, is_integrated_to_local
     ):
@@ -208,7 +208,7 @@ def get_cluster_mismatched_revision_status_mock_success(*unused):
 
         self.harness.charm.is_role = is_config_mock_call
 
-        self.harness.charm.get_cluster_mismatched_revision_status = (
+        self.harness.charm.status.get_cluster_mismatched_revision_status = (
             get_cluster_mismatched_revision_status_mock_fail
         )
 
@@ -220,7 +220,7 @@ def get_cluster_mismatched_revision_status_mock_success(*unused):
         event.defer.assert_called()
 
         # If we return a matching revision status, then we won't defer anymore.
-        self.harness.charm.get_cluster_mismatched_revision_status = (
+        self.harness.charm.status.get_cluster_mismatched_revision_status = (
             get_cluster_mismatched_revision_status_mock_success
         )
         event = mock.Mock()
@@ -243,7 +243,7 @@ def get_cluster_mismatched_revision_status_mock_success(*unused):
         self.harness.charm.is_role = is_config_mock_call
 
         # First, we'll return a blocked status because it should defer.
-        self.harness.charm.get_cluster_mismatched_revision_status = (
+        self.harness.charm.status.get_cluster_mismatched_revision_status = (
             get_cluster_mismatched_revision_status_mock_fail
         )
 
@@ -255,7 +255,7 @@ def get_cluster_mismatched_revision_status_mock_success(*unused):
         event.defer.assert_called()
 
         # Then, if we return a matching revision status, then we won't defer anymore.
-        self.harness.charm.get_cluster_mismatched_revision_status = (
+        self.harness.charm.status.get_cluster_mismatched_revision_status = (
             get_cluster_mismatched_revision_status_mock_success
         )
         event = mock.MagicMock()

diff --git a/tests/unit/test_mongodb_provider.py b/tests/unit/test_mongodb_provider.py
@@ -62,7 +62,7 @@ def test_relation_event_db_not_initialised(self, oversee_users, defer):
     @patch_network_get(private_address="1.1.1.1")
     @patch("charm.CrossAppVersionChecker.is_local_charm")
     @patch("charm.CrossAppVersionChecker.is_integrated_to_locally_built_charm")
-    @patch("charm.get_charm_revision")
+    @patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch("ops.framework.EventBase.defer")
     @patch("charm.MongoDBProvider.oversee_users")
     @patch("charm.MongodbOperatorCharm.auth_enabled", return_value=True)
@@ -100,7 +100,7 @@ def test_relation_event_oversee_users_mongo_failure(
     @patch(
         "data_platform_helpers.version_check.CrossAppVersionChecker.is_integrated_to_locally_built_charm"
     )
-    @patch("charm.get_charm_revision")
+    @patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch("ops.framework.EventBase.defer")
     @patch("charm.MongoDBProvider.oversee_users")
     @patch("charm.MongodbOperatorCharm.auth_enabled", return_value=True)
@@ -356,7 +356,7 @@ def test_oversee_users_drop_database_failure(
     @patch_network_get(private_address="1.1.1.1")
     @patch("charm.CrossAppVersionChecker.is_local_charm")
     @patch("charm.CrossAppVersionChecker.is_integrated_to_locally_built_charm")
-    @patch("charm.get_charm_revision")
+    @patch("charms.mongodb.v0.set_status.get_charm_revision")
     @patch("charms.mongodb.v1.mongodb_provider.MongoDBProvider._get_relations")
     def test_update_app_relation_data_protected(
         self,

diff --git a/tests/unit/test_set_status.py b/tests/unit/test_set_status.py
@@ -33,7 +33,7 @@ def test_are_all_units_ready_for_upgrade(self) -> None:
         run_mock = mock.Mock()
         run_mock._run.return_value = goal_state
         self.harness.charm.model._backend = run_mock
-        self.harness.charm.get_cluster_mismatched_revision_status = get_mismatched_revsion
+        self.harness.charm.status.get_cluster_mismatched_revision_status = get_mismatched_revsion
 
         assert self.harness.charm.status.are_all_units_ready_for_upgrade()
 
@@ -134,7 +134,9 @@ def test_get_invalid_integration_status(
         valid_s3_integration_mock = mock.Mock()
         valid_s3_integration_mock.return_value = valid_s3_integration
 
-        self.harness.charm.get_cluster_mismatched_revision_status = get_mismatched_revision_mock
+        self.harness.charm.status.get_cluster_mismatched_revision_status = (
+            get_mismatched_revision_mock
+        )
         self.harness.charm.cluster.is_valid_mongos_integration = mongos_integration_mock
         self.harness.charm.backups.is_valid_s3_integration = valid_s3_integration_mock