Fix charm integration tests

src/charm.py

@@ -544,6 +544,11 @@ def _on_set_password(self, event: ActionEvent) -> None:
             return
 
         new_password = event.params.get(Config.Actions.PASSWORD_PARAM_NAME, generate_password())
+        if len(new_password) > Config.Secrets.MAX_PASSWORD_LENGTH:
+            event.fail(
+                f"Password cannot be longer than {Config.Secrets.MAX_PASSWORD_LENGTH} characters."
+            )
+            return
         with MongoDBConnection(self.mongodb_config) as mongo:
             try:
                 mongo.set_user_password(username, new_password)
@@ -556,7 +561,7 @@ def _on_set_password(self, event: ActionEvent) -> None:
                 event.fail(f"Failed changing the password: {e}")
                 return
 
-        self.set_secret(
+        secret_id = self.set_secret(
             APP_SCOPE, MongoDBUser.get_password_key_name_for_user(username), new_password
         )
 
@@ -566,7 +571,9 @@ def _on_set_password(self, event: ActionEvent) -> None:
         if username == MonitorUser.get_username():
             self._connect_mongodb_exporter()
 
-        event.set_results({Config.Actions.PASSWORD_PARAM_NAME: new_password})
+        event.set_results(
+            {Config.Actions.PASSWORD_PARAM_NAME: new_password, "secret-id": secret_id}
+        )
 
     def _on_secret_remove(self, event: SecretRemoveEvent):
         # We are keeping this function empty on purpose until the issue with secrets
@@ -1003,7 +1010,7 @@ def get_secret(self, scope: str, key: str) -> Optional[str]:
         else:
             raise RuntimeError("Unknown secret scope.")
 
-    def set_secret(self, scope: str, key: str, value: Optional[str]) -> None:
+    def set_secret(self, scope: str, key: str, value: Optional[str]) -> Optional[str]:
         """Set secret in the secret storage."""
         if self._juju_has_secrets:
             if not value:
@@ -1080,12 +1087,12 @@ def _juju_secret_set(self, scope: Scopes, key: str, value: str) -> str:
                 secret_cache[key] = value
                 try:
                     secret.set_content(secret_cache)
+                    logging.debug(f"Secret {scope}:{key} was {key} set")
                 except OSError as error:
                     logging.error(
-                        f"Error in attempt to set {scope}:{key}. "
+                        f"Error in attempt to set '{key}' secret for scope '{scope}'. "
                         f"Existing keys were: {list(secret_cache.keys())}. {error}"
                     )
-                logging.debug(f"Secret {scope}:{key} was {key} set")
 
         # We need to create a brand-new secret for this scope
         else:

src/config.py

@@ -77,3 +77,4 @@ class Secrets:
         SECRET_KEYFILE_NAME = "keyfile"
         SECRET_INTERNAL_LABEL = "internal-secret"
         SECRET_DELETED_LABEL = "None"
+        MAX_PASSWORD_LENGTH = 50  # TODO what is the max password length?

tests/integration/ha_tests/helpers.py

@@ -5,6 +5,7 @@
 import subprocess
 import time
 from datetime import datetime
+from importlib.metadata import version
 from pathlib import Path
 from subprocess import PIPE, check_output
 from typing import List, Optional
@@ -487,8 +488,11 @@ async def kill_unit_process(ops_test: OpsTest, unit_name: str, kill_code: str):
     if len(ops_test.model.applications[app].units) < 2:
         await ops_test.model.applications[app].add_unit(count=1)
         await ops_test.model.wait_for_idle(apps=[app], status="active", timeout=1000)
+    if version("juju") < "3":
+        kill_cmd = f"run --unit {unit_name} -- pkill --signal {kill_code} -f {DB_PROCESS}"
+    else:
+        kill_cmd = f"exec --unit {unit_name} -- pkill --signal {kill_code} -f {DB_PROCESS}"
 
-    kill_cmd = f"run --unit {unit_name} -- pkill --signal {kill_code} -f {DB_PROCESS}"
     return_code, _, _ = await ops_test.juju(*kill_cmd.split())
 
     if return_code != 0:

tests/integration/test_charm.py

@@ -6,6 +6,7 @@
 import logging
 import os
 import time
+from importlib.metadata import version
 from uuid import uuid4
 
 import pytest
@@ -180,62 +181,14 @@ async def test_monitor_user(ops_test: OpsTest) -> None:
     ]
     hosts = ",".join(replica_set_hosts)
     replica_set_uri = f"mongodb://monitor:{password}@{hosts}/admin?replicaSet=mongodb"
-
     admin_mongod_cmd = f"charmed-mongodb.mongo '{replica_set_uri}'  --eval 'rs.conf()'"
-    check_monitor_cmd = f"run --unit {unit.name} -- {admin_mongod_cmd}"
-    return_code, _, _ = await ops_test.juju(*check_monitor_cmd.split())
-    assert return_code == 0, "command rs.conf() on monitor user does not work"
-
-
-async def test_exactly_one_primary_reported_by_juju(ops_test: OpsTest) -> None:
-    """Tests that there is exactly one replica set primary unit reported by juju."""
-
-    async def get_unit_messages():
-        """Collects unit status messages."""
-        app = await app_name(ops_test)
-        unit_messages = {}
-
-        async with ops_test.fast_forward():
-            time.sleep(20)
 
-        for unit in ops_test.model.applications[app].units:
-            unit_messages[unit.entity_id] = unit.workload_status_message
-
-        return unit_messages
-
-    def juju_reports_one_primary(unit_messages):
-        """Confirms there is only one replica set primary unit reported by juju."""
-        count = 0
-        for value in unit_messages:
-            if unit_messages[value] == "Primary":
-                count += 1
-
-        assert count == 1, f"Juju is expected to report one primary not {count} primaries"
-
-    # collect unit status messages
-    unit_messages = await get_unit_messages()
-
-    # confirm there is only one replica set primary unit
-    juju_reports_one_primary(unit_messages)
-
-    # kill the mongod process on the replica set primary unit to force a re-election
-    for unit, message in unit_messages.items():
-        if message == "Primary":
-            target_unit = unit
-
-    await kill_unit_process(ops_test, target_unit, kill_code="SIGKILL")
-
-    # wait for re-election, sleep for twice the median election time
-    time.sleep(MEDIAN_REELECTION_TIME * 2)
-
-    # collect unit status messages
-    unit_messages = await get_unit_messages()
-
-    # confirm there is only one replica set primary unit
-    juju_reports_one_primary(unit_messages)
-
-    # cleanup, remove killed unit
-    await ops_test.model.destroy_unit(target_unit)
+    if version("juju") < "3":
+        check_monitor_cmd = f"run --unit {unit.name} -- {admin_mongod_cmd}"
+    else:
+        check_monitor_cmd = f"exec --unit {unit.name} -- {admin_mongod_cmd}"
+    return_code, _, sdterr = await ops_test.juju(*check_monitor_cmd.split())
+    assert return_code == 0, f"command rs.conf() on monitor user does not work. '{sdterr}'"
 
 
 async def test_only_leader_can_set_while_all_can_read_password_secret(ops_test: OpsTest) -> None:
@@ -247,12 +200,12 @@ async def test_only_leader_can_set_while_all_can_read_password_secret(ops_test:
 
     password = "blablabla"
     await set_password(ops_test, unit_id=non_leaders[0], username="monitor", password=password)
-    password1 = await get_password(ops_test, unit_id=leader_id, username="monitor")
+    password1 = await get_password(ops_test, username="monitor")
     assert password1 != password
 
     await set_password(ops_test, unit_id=leader_id, username="monitor", password=password)
-    for unit_id in UNIT_IDS:
-        password2 = await get_password(ops_test, unit_id=unit_id, username="monitor")
+    for _ in UNIT_IDS:
+        password2 = await get_password(ops_test, username="monitor")
         assert password2 == password
 
 
@@ -270,7 +223,7 @@ async def test_reset_and_get_password_secret_same_as_cli(ops_test: OpsTest) -> N
     secret_id = result["secret-id"].split("/")[-1]
 
     # Getting back the pw programmatically
-    password = await get_password(ops_test, unit_id=leader_id, username="monitor")
+    password = await get_password(ops_test, username="monitor")
 
     # Getting back the pw from juju CLI
     complete_command = f"show-secret {secret_id} --reveal --format=json"
@@ -291,7 +244,7 @@ async def test_reset_and_get_password_no_secret(ops_test: OpsTest, mocker) -> No
     await set_password(ops_test, unit_id=leader_id, username="monitor", password=new_password)
 
     # Getting back the pw programmatically
-    password = await get_password(ops_test, unit_id=leader_id, username="monitor")
+    password = await get_password(ops_test, username="monitor")
     assert password == new_password
 
 
@@ -300,9 +253,9 @@ async def test_empty_password(ops_test: OpsTest) -> None:
     """Test that the password can't be set to an empty string."""
     leader_id = await get_leader_id(ops_test)
 
-    password1 = await get_password(ops_test, unit_id=leader_id, username="monitor")
+    password1 = await get_password(ops_test, username="monitor")
     await set_password(ops_test, unit_id=leader_id, username="monitor", password="")
-    password2 = await get_password(ops_test, unit_id=leader_id, username="monitor")
+    password2 = await get_password(ops_test, username="monitor")
 
     # The password remained unchanged
     assert password1 == password2
@@ -312,11 +265,62 @@ async def test_empty_password(ops_test: OpsTest) -> None:
 async def test_no_password_change_on_invalid_password(ops_test: OpsTest) -> None:
     """Test that in general, there is no change when password validation fails."""
     leader_id = await get_leader_id(ops_test)
-    password1 = await get_password(ops_test, unit_id=leader_id, username="monitor")
+    password1 = await get_password(ops_test, username="monitor")
 
     # The password has to be minimum 3 characters
     await set_password(ops_test, unit_id=leader_id, username="monitor", password="ca" * 1000000)
-    password2 = await get_password(ops_test, unit_id=leader_id, username="monitor")
+    password2 = await get_password(ops_test, username="monitor")
 
     # The password didn't change
     assert password1 == password2
+
+
+async def test_exactly_one_primary_reported_by_juju(ops_test: OpsTest) -> None:
+    """Tests that there is exactly one replica set primary unit reported by juju."""
+
+    async def get_unit_messages():
+        """Collects unit status messages."""
+        app = await app_name(ops_test)
+        unit_messages = {}
+
+        async with ops_test.fast_forward():
+            time.sleep(20)
+
+        for unit in ops_test.model.applications[app].units:
+            unit_messages[unit.entity_id] = unit.workload_status_message
+
+        return unit_messages
+
+    def juju_reports_one_primary(unit_messages):
+        """Confirms there is only one replica set primary unit reported by juju."""
+        count = 0
+        for value in unit_messages:
+            if unit_messages[value] == "Primary":
+                count += 1
+
+        assert count == 1, f"Juju is expected to report one primary not {count} primaries"
+
+    # collect unit status messages
+    unit_messages = await get_unit_messages()
+
+    # confirm there is only one replica set primary unit
+    juju_reports_one_primary(unit_messages)
+
+    # kill the mongod process on the replica set primary unit to force a re-election
+    for unit, message in unit_messages.items():
+        if message == "Primary":
+            target_unit = unit
+
+    await kill_unit_process(ops_test, target_unit, kill_code="SIGKILL")
+
+    # wait for re-election, sleep for twice the median election time
+    time.sleep(MEDIAN_REELECTION_TIME * 2)
+
+    # collect unit status messages
+    unit_messages = await get_unit_messages()
+
+    # confirm there is only one replica set primary unit
+    juju_reports_one_primary(unit_messages)
+
+    # cleanup, remove killed unit
+    await ops_test.model.destroy_unit(target_unit)

tox.ini

@@ -74,6 +74,7 @@ deps =
     pytest
     juju==3.2.0.1 # juju 3.3.0 has issues with retrieving action results
     pytest-operator
+    pytest-mock
     -r {tox_root}/requirements.txt
 commands =
     pytest -v --tb native --log-cli-level=INFO -s --durations=0 {posargs} {[vars]tests_path}/integration/test_charm.py