Remove DB static config (#106)

* remove db static config

* delete useless tests and address comment

---------

Co-authored-by: arturo-seijas <[email protected]>

.github/workflows/integration_test.yaml

@@ -15,7 +15,7 @@ jobs:
         --openstack-rc ${GITHUB_WORKSPACE}/openrc
         --kube-config ${GITHUB_WORKSPACE}/kube-config
         --screenshot-dir /tmp
-      modules: '["test_addon", "test_core", "test_cos", "test_db_config", "test_error", "test_ingress"]'
+      modules: '["test_addon", "test_core", "test_cos", "test_ingress"]'
       pre-run-script: |
         -c "sudo microk8s enable hostpath-storage
           sudo microk8s kubectl -n kube-system rollout status -w deployment/hostpath-provisioner

config.yaml

@@ -7,22 +7,6 @@ options:
       Hostname for accessing WordPress, if ingress relation is active. Defaults to the application
       name.
     default: ""
-  db_host:
-    type: string
-    description: "MySQL database host"
-    default: ""
-  db_name:
-    type: string
-    description: "MySQL database name"
-    default: ""
-  db_user:
-    type: string
-    description: "MySQL database user"
-    default: ""
-  db_password:
-    type: string
-    description: "MySQL database user's password"
-    default: ""
   initial_settings:
       type: string
       description: >

src/charm.py

@@ -566,25 +566,6 @@ def _wp_is_installed(self):
         logger.debug("Check if WordPress is installed")
         return self._run_wp_cli(["wp", "core", "is-installed"]).return_code == 0
 
-    @property
-    def _config_database_config(self) -> Optional[types_.DatabaseConfig]:
-        """Database configuration details from charm config.
-
-        Returns:
-            Database configuration required to establish database connection.
-            None if not exists.
-        """
-        if any(
-            self.model.config.get(key) for key in ("db_host", "db_name", "db_user", "db_password")
-        ):
-            return types_.DatabaseConfig(
-                hostname=self.model.config.get("db_host"),
-                database=self.model.config.get("db_name"),
-                username=self.model.config.get("db_user"),
-                password=self.model.config.get("db_password"),
-            )
-        return None
-
     def _parse_database_endpoints(self, endpoint: Optional[str]) -> Optional[str]:
         """Retrieve a single database endpoint.
 
@@ -609,8 +590,8 @@ def _parse_database_endpoints(self, endpoint: Optional[str]) -> Optional[str]:
         return host_port[0]
 
     @property
-    def _database_relation_database_config(self) -> Optional[types_.DatabaseConfig]:
-        """Database configuration details from database relation.
+    def _current_effective_db_info(self) -> Optional[types_.DatabaseConfig]:
+        """Get the current effective db connection information.
 
         Returns:
             Database configuration required to establish database connection.
@@ -626,20 +607,6 @@ def _database_relation_database_config(self) -> Optional[types_.DatabaseConfig]:
             password=relation.data[relation.app].get("password"),
         )
 
-    @property
-    def _current_effective_db_info(self) -> Optional[types_.DatabaseConfig]:
-        """Get the current effective db connection information.
-
-        The order of precedence for effective configurations are as follows:
-        1. Charm config
-        2. database relation
-
-        Returns:
-            Database configuration required to establish database connection.
-            None if not exists.
-        """
-        return self._config_database_config or self._database_relation_database_config
-
     def _test_database_connectivity(self):
         """Test the connectivity of the current database config/relation.
 

tests/unit/conftest.py

@@ -45,7 +45,7 @@ def app_name_fixture():
 
 @pytest.fixture(scope="function", name="setup_replica_consensus")
 def setup_replica_consensus_fixture(harness: ops.testing.Harness, app_name: str):
-    """Yields a function that can be used to set up peer relation.
+    """Returns a function that can be used to set up peer relation.
 
     After calling the yielded function, the replica consensus including WordPress salt keys and
     secrets will be populated. The unit will become a leader unit in this process.
@@ -96,11 +96,44 @@ def example_invalid_database_info_fixture():
     }
 
 
+@pytest.fixture(scope="function", name="example_database_info_no_port")
+def example_database_info_no_port_fixture():
+    """An example database connection info from mysql_client interface."""
+    return {
+        "endpoints": "test_database_host",
+        "database": "test_database_name",
+        "username": "test_database_user",
+        "password": "test_database_password",
+    }
+
+
+@pytest.fixture(scope="function", name="example_database_info_no_port_diff_host")
+def example_database_info_no_port_diff_host_fixture():
+    """An example database connection info from mysql_client interface."""
+    return {
+        "endpoints": "test_database_host2",
+        "database": "test_database_name",
+        "username": "test_database_user",
+        "password": "test_database_password",
+    }
+
+
+@pytest.fixture(scope="function", name="example_database_info_connection_error")
+def example_database_info_connection_error_fixture():
+    """An example database connection info from mysql_client interface."""
+    return {
+        "endpoints": "a",
+        "database": "b",
+        "username": "c",
+        "password": "d",
+    }
+
+
 @pytest.fixture(scope="function")
 def setup_database_relation(
     harness: ops.testing.Harness, example_database_info: typing.Dict[str, str]
 ):
-    """Yields a function that can be used to set up database relation.
+    """Returns a function that can be used to set up database relation.
 
     After calling the yielded function, a database relation will be set up. example_database_info
     will be used as the relation data. Return a tuple of relation id and the relation data.
@@ -120,11 +153,35 @@ def _setup_database_relation():
     return _setup_database_relation
 
 
+@pytest.fixture(scope="function", name="setup_database_relation_no_port")
+def setup_database_relation_no_port_fixture(
+    harness: ops.testing.Harness, example_database_info_no_port: typing.Dict[str, str]
+):
+    """Returns a function that can be used to set up database relation.
+
+    After calling the yielded function, a database relation will be set up. example_database_info
+    will be used as the relation data. Return a tuple of relation id and the relation data.
+    """
+
+    def _setup_database_relation():
+        """Function to set up database relation. See fixture docstring for more information.
+
+        Returns:
+            Tuple of relation id and relation data.
+        """
+        db_relation_id = harness.add_relation("database", "mysql")
+        harness.add_relation_unit(db_relation_id, "mysql/0")
+        harness.update_relation_data(db_relation_id, "mysql", example_database_info_no_port)
+        return db_relation_id, example_database_info_no_port
+
+    return _setup_database_relation
+
+
 @pytest.fixture(scope="function")
 def setup_database_relation_invalid_port(
     harness: ops.testing.Harness, example_invalid_database_info: typing.Dict[str, str]
 ):
-    """Yields a function that can be used to set up database relation with a non 3306 port.
+    """Returns a function that can be used to set up database relation with a non 3306 port.
 
     After calling the yielded function, a database relation will be set up. example_database_info
     will be used as the relation data. Return a tuple of relation id and the relation data.
@@ -144,6 +201,33 @@ def _setup_database_relation():
     return _setup_database_relation
 
 
+@pytest.fixture(scope="function")
+def setup_database_relation_connection_error(
+    harness: ops.testing.Harness, example_database_info_connection_error: typing.Dict[str, str]
+):
+    """Returns a function that can be used to set up database relation with a non 3306 port.
+
+    After calling the yielded function, a database relation will be set up.
+    example_database_info_connection_error will be used as the relation data.
+    Return a tuple of relation id and the relation data.
+    """
+
+    def _setup_database_relation():
+        """Function to set up database relation. See fixture docstring for more information.
+
+        Returns:
+            Tuple of relation id and relation data.
+        """
+        db_relation_id = harness.add_relation("database", "mysql")
+        harness.add_relation_unit(db_relation_id, "mysql/0")
+        harness.update_relation_data(
+            db_relation_id, "mysql", example_database_info_connection_error
+        )
+        return db_relation_id, example_database_info_connection_error
+
+    return _setup_database_relation
+
+
 @pytest.fixture(scope="function")
 def action_event_mock():
     """Creates a mock object for :class:`ops.charm.ActionEvent`."""
@@ -158,8 +242,9 @@ def run_standard_plugin_test(
     patch: WordpressPatch,
     harness: ops.testing.Harness,
     setup_replica_consensus: typing.Callable[[], dict],
+    setup_database_relation_no_port: typing.Callable[[], typing.Tuple[int, dict]],
 ):
-    """Yields a function that can be used to perform some general test for different plugins."""
+    """Returns a function that can be used to perform some general test for different plugins."""
 
     def _run_standard_plugin_test(
         plugin: str,
@@ -182,25 +267,18 @@ def _run_standard_plugin_test(
         plugin_config_keys = list(plugin_config.keys())
         harness.set_can_connect(harness.model.unit.containers["wordpress"], True)
         setup_replica_consensus()
-        db_config = {
-            "db_host": "config_db_host",
-            "db_name": "config_db_name",
-            "db_user": "config_db_user",
-            "db_password": "config_db_password",
-        }
+        _, db_info = setup_database_relation_no_port()
         patch.database.prepare_database(
-            host=db_config["db_host"],
-            database=db_config["db_name"],
-            user=db_config["db_user"],
-            password=db_config["db_password"],
+            host=db_info["endpoints"],
+            database=db_info["database"],
+            user=db_info["username"],
+            password=db_info["password"],
         )
 
-        harness.update_config(db_config)
-
         harness.update_config(plugin_config)
 
         database_instance = patch.database.get_wordpress_database(
-            host="config_db_host", database="config_db_name"
+            host="test_database_host", database="test_database_name"
         )
         assert database_instance
         assert (