[DPE-7421] Fix MySQL legacy interface relation (#639)

Author: sinclert-canonical

src/relations/mysql.py

@@ -212,7 +212,7 @@ def _on_mysql_relation_created(self, event: RelationCreatedEvent) -> None:  # no
         if (
             not self.charm._is_peer_data_set
             or not self.charm.unit_initialized()
-            or self.charm.unit_peer_data.get("member-state") != "online"
+            or not self.charm.unit_peer_data.get("member-state") == "online"
         ):
             logger.info("Unit not ready to execute `mysql` relation created. Deferring")
             event.defer()

src/relations/mysql_root.py

@@ -8,7 +8,11 @@
 import typing
 
 from charms.mysql.v0.mysql import MySQLCheckUserExistenceError, MySQLDeleteUsersForUnitError
-from ops.charm import RelationBrokenEvent, RelationCreatedEvent
+from ops.charm import (
+    LeaderElectedEvent,
+    RelationBrokenEvent,
+    RelationCreatedEvent,
+)
 from ops.framework import Object
 from ops.model import ActiveStatus, BlockedStatus
 
@@ -85,14 +89,21 @@ def _get_or_generate_database(self, event_relation_id: int) -> str:
             self.charm.config.mysql_root_interface_database or f"database-{event_relation_id}",
         )
 
-    def _on_leader_elected(self, _) -> None:
+    def _on_leader_elected(self, event: LeaderElectedEvent) -> None:
         """Handle the leader elected event.
 
         Retrieves relation data from the peer relation databag and copies
         the relation data into the new leader unit's databag.
         """
-        # Skip if the charm is not past the setup phase (config-changed event not executed yet)
-        if not self.charm._is_peer_data_set:
+        # Wait until on-config-changed event is executed (for root password to have been set)
+        # and for the member to be initialized and online
+        if (
+            not self.charm._is_peer_data_set
+            or not self.charm.unit_initialized()
+            or not self.charm.unit_peer_data.get("member-state") == "online"
+        ):
+            logger.info("Unit not ready to execute `mysql` leader elected. Deferring")
+            event.defer()
             return
 
         relation_data = json.loads(

tests/integration/relations/test_database.py

@@ -16,21 +16,20 @@
 logger = logging.getLogger(__name__)
 
 DB_METADATA = yaml.safe_load(Path("./metadata.yaml").read_text())
-DATABASE_APP_NAME = DB_METADATA["name"]
-CLUSTER_NAME = "test_cluster"
 
+DATABASE_APP_NAME = DB_METADATA["name"]
+DATABASE_ENDPOINT = "database"
 APPLICATION_APP_NAME = "mysql-test-app"
+APPLICATION_ENDPOINT = "database"
 
 APPS = [DATABASE_APP_NAME, APPLICATION_APP_NAME]
 
-ENDPOINT = "database"
-
 
 @pytest.mark.abort_on_fail
 @pytest.mark.skip_if_deployed
 async def test_build_and_deploy(ops_test: OpsTest, charm):
     """Build the charm and deploy 3 units to ensure a cluster is formed."""
-    config = {"cluster-name": CLUSTER_NAME, "profile": "testing"}
+    config = {"cluster-name": "test_cluster", "profile": "testing"}
     resources = {"mysql-image": DB_METADATA["resources"]["mysql-image"]["upstream-source"]}
 
     await asyncio.gather(
@@ -52,12 +51,26 @@ async def test_build_and_deploy(ops_test: OpsTest, charm):
         ),
     )
 
+
+@pytest.mark.abort_on_fail
+async def test_relation_creation_eager(ops_test: OpsTest):
+    """Relate charms before they have time to properly start.
+
+    It simulates a Terraform-like deployment strategy
+    """
+    await ops_test.model.relate(
+        f"{APPLICATION_APP_NAME}:{APPLICATION_ENDPOINT}",
+        f"{DATABASE_APP_NAME}:{DATABASE_ENDPOINT}",
+    )
+    await ops_test.model.block_until(
+        lambda: is_relation_joined(ops_test, APPLICATION_ENDPOINT, DATABASE_ENDPOINT) == True  # noqa: E712
+    )
+
     # Reduce the update_status frequency until the cluster is deployed
     async with ops_test.fast_forward("60s"):
         await ops_test.model.block_until(
             lambda: len(ops_test.model.applications[DATABASE_APP_NAME].units) == 3
         )
-
         await ops_test.model.block_until(
             lambda: len(ops_test.model.applications[APPLICATION_APP_NAME].units) == 2
         )
@@ -78,26 +91,14 @@ async def test_build_and_deploy(ops_test: OpsTest, charm):
             ),
         )
 
-    assert len(ops_test.model.applications[DATABASE_APP_NAME].units) == 3
-
-    for unit in ops_test.model.applications[DATABASE_APP_NAME].units:
-        assert unit.workload_status == "active"
-
-    assert len(ops_test.model.applications[APPLICATION_APP_NAME].units) == 2
-
 
 @pytest.mark.abort_on_fail
 @markers.only_without_juju_secrets
 async def test_relation_creation_databag(ops_test: OpsTest):
     """Relate charms and wait for the expected changes in status."""
-    await ops_test.model.relate(APPLICATION_APP_NAME, f"{DATABASE_APP_NAME}:{ENDPOINT}")
-
     async with ops_test.fast_forward("60s"):
-        await ops_test.model.block_until(
-            lambda: is_relation_joined(ops_test, ENDPOINT, ENDPOINT) == True  # noqa: E712
-        )
-
         await ops_test.model.wait_for_idle(apps=APPS, status="active")
+
     relation_data = await get_relation_data(ops_test, APPLICATION_APP_NAME, "database")
     assert {"password", "username"} <= set(relation_data[0]["application-data"])
 
@@ -106,14 +107,9 @@ async def test_relation_creation_databag(ops_test: OpsTest):
 @markers.only_with_juju_secrets
 async def test_relation_creation(ops_test: OpsTest):
     """Relate charms and wait for the expected changes in status."""
-    await ops_test.model.relate(APPLICATION_APP_NAME, f"{DATABASE_APP_NAME}:{ENDPOINT}")
-
     async with ops_test.fast_forward("60s"):
-        await ops_test.model.block_until(
-            lambda: is_relation_joined(ops_test, ENDPOINT, ENDPOINT) == True  # noqa: E712
-        )
-
         await ops_test.model.wait_for_idle(apps=APPS, status="active")
+
     relation_data = await get_relation_data(ops_test, APPLICATION_APP_NAME, "database")
     assert not {"password", "username"} <= set(relation_data[0]["application-data"])
     assert "secret-user" in relation_data[0]["application-data"]
@@ -123,11 +119,12 @@ async def test_relation_creation(ops_test: OpsTest):
 async def test_relation_broken(ops_test: OpsTest):
     """Remove relation and wait for the expected changes in status."""
     await ops_test.model.applications[DATABASE_APP_NAME].remove_relation(
-        f"{APPLICATION_APP_NAME}:{ENDPOINT}", f"{DATABASE_APP_NAME}:{ENDPOINT}"
+        f"{APPLICATION_APP_NAME}:{APPLICATION_ENDPOINT}",
+        f"{DATABASE_APP_NAME}:{DATABASE_ENDPOINT}",
     )
 
     await ops_test.model.block_until(
-        lambda: is_relation_broken(ops_test, ENDPOINT, ENDPOINT) == True  # noqa: E712
+        lambda: is_relation_broken(ops_test, APPLICATION_ENDPOINT, DATABASE_ENDPOINT) == True  # noqa: E712
     )
 
     async with ops_test.fast_forward("60s"):