diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 979e6975..cb971cf1 100644 --- a/.github/workflows/integration_test.yaml +++ b/.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 diff --git a/config.yaml b/config.yaml index 1258ae5a..021b63ab 100644 --- a/config.yaml +++ b/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: > diff --git a/src/charm.py b/src/charm.py index e4e9e537..40340257 100755 --- a/src/charm.py +++ b/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. diff --git a/tests/integration/test_db_config.py b/tests/integration/test_db_config.py deleted file mode 100644 index f2f6b142..00000000 --- a/tests/integration/test_db_config.py +++ /dev/null @@ -1,24 +0,0 @@ -# Copyright 2023 Canonical Ltd. -# See LICENSE file for licensing details. - -"""Integration test module for database configuration.""" - -import pytest - -from tests.integration.helper import WordpressApp, WordpressClient - - -@pytest.mark.usefixtures("prepare_mysql_pod") -async def test_db_config(wordpress: WordpressApp): - """ - arrange: config wordpress to connect to the non-charmed mysql server. - act: test the wordpress functionality of the wordpress charm. - assert: wordpress charm should provide wordpress functionality correctly. - """ - default_admin_password = await wordpress.get_default_admin_password() - for unit_ip in await wordpress.get_unit_ips(): - WordpressClient.run_wordpress_functionality_test( - host=unit_ip, - admin_username="admin", - admin_password=default_admin_password, - ) diff --git a/tests/integration/test_error.py b/tests/integration/test_error.py deleted file mode 100644 index 5c489c77..00000000 --- a/tests/integration/test_error.py +++ /dev/null @@ -1,36 +0,0 @@ -# Copyright 2023 Canonical Ltd. -# See LICENSE file for licensing details. - -"""Integration tests for WordPress charm in error.""" - -from tests.integration.helper import WordpressApp - - -async def test_incorrect_db_config(wordpress: WordpressApp): - """ - arrange: after WordPress charm has been deployed. - act: provide incorrect database info via config. - assert: charm should be blocked by WordPress installation errors, instead of lacking - of database connection info. - """ - # Database configuration can retry for up to 300 seconds before giving up and showing an error. - # Default wait_for_idle 15 seconds in ``app_config`` fixture is too short for incorrect - # db config. - await wordpress.set_config( - { - "db_host": "test_db_host", - "db_name": "test_db_name", - "db_user": "test_db_user", - "db_password": "test_db_password", - } - ) - await wordpress.model.wait_for_idle( - idle_period=360, status="blocked", apps=[wordpress.name], timeout=45 * 60 - ) - - for unit in wordpress.get_units(): - assert unit.workload_status == "blocked", "unit status should be blocked" - msg = unit.workload_status_message - assert ("MySQL error" in msg and ("2003" in msg or "2005" in msg)) or ( - "leader unit failed" in msg - ), "unit status message should show detailed installation failure" diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 11acd282..125dcd17 100644 --- a/tests/unit/conftest.py +++ b/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 ( diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 2a04a375..a1a6c9ae 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -201,15 +201,6 @@ def in_same_line(content: str, *matches: str): wp_config = charm._gen_wp_config() - db_info_in_config = { - "db_host": "config_db_host", - "db_name": "config_db_name", - "db_user": "config_db_user", - "db_password": "config_db_password", - } - harness.update_config(db_info_in_config) - wp_config = charm._gen_wp_config() - @pytest.mark.usefixtures("attach_storage") def test_wp_install_cmd( @@ -320,6 +311,7 @@ def test_prom_exporter_pebble_ready( patch: WordpressPatch, harness: ops.testing.Harness, setup_replica_consensus: typing.Callable[[], dict], + setup_database_relation_no_port: typing.Callable[[], typing.Tuple[int, dict]], ): """ arrange: after required relations ready but before prometheus exporter pebble ready. @@ -329,19 +321,13 @@ def test_prom_exporter_pebble_ready( harness.set_can_connect(harness.model.unit.containers["wordpress"], True) setup_replica_consensus() charm: WordpressCharm = typing.cast(WordpressCharm, harness.charm) - 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) mock_event = unittest.mock.MagicMock(spec=PebbleReadyEvent) mock_event.workload = unittest.mock.MagicMock(spec=Container) mock_event.workload.name = "apache-prometheus-exporter" @@ -359,6 +345,8 @@ def test_core_reconciliation( patch: WordpressPatch, harness: ops.testing.Harness, setup_replica_consensus: typing.Callable[[], dict], + setup_database_relation_no_port: typing.Callable[[], typing.Tuple[int, dict]], + example_database_info_no_port_diff_host: dict, ): """ arrange: after peer relation established and database configured. @@ -368,35 +356,31 @@ def test_core_reconciliation( """ 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_relation_id, 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() assert patch.database.is_wordpress_installed( - db_config["db_host"], db_config["db_name"] + db_info["endpoints"], db_info["database"] ), "WordPress should be installed after core reconciliation" - db_config.update({"db_host": "config_db_host_2"}) + harness.update_relation_data(db_relation_id, "mysql", example_database_info_no_port_diff_host) + harness.update_config() + 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=example_database_info_no_port_diff_host["endpoints"], + database=example_database_info_no_port_diff_host["database"], + user=example_database_info_no_port_diff_host["username"], + password=example_database_info_no_port_diff_host["password"], ) - harness.update_config({"db_host": "config_db_host_2"}) assert patch.database.is_wordpress_installed( - "config_db_host_2", db_config["db_name"] + db_info["endpoints"], db_info["database"] ), "WordPress should be installed after database config changed" @@ -530,6 +514,7 @@ def test_theme_reconciliation( patch: WordpressPatch, harness: ops.testing.Harness, setup_replica_consensus: typing.Callable[[], dict], + setup_database_relation_no_port: typing.Callable[[], typing.Tuple[int, dict]], ): """ arrange: after peer relation established and database ready. @@ -539,19 +524,13 @@ def test_theme_reconciliation( harness.set_can_connect(harness.model.unit.containers["wordpress"], True) setup_replica_consensus() charm: WordpressCharm = typing.cast(WordpressCharm, harness.charm) - 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) assert patch.container.installed_themes == set( charm._WORDPRESS_DEFAULT_THEMES @@ -575,6 +554,7 @@ def test_plugin_reconciliation( patch: WordpressPatch, harness: ops.testing.Harness, setup_replica_consensus: typing.Callable[[], dict], + setup_database_relation_no_port: typing.Callable[[], typing.Tuple[int, dict]], ): """ arrange: after peer relation established and database ready. @@ -584,19 +564,13 @@ def test_plugin_reconciliation( harness.set_can_connect(harness.model.unit.containers["wordpress"], True) setup_replica_consensus() charm: WordpressCharm = typing.cast(WordpressCharm, harness.charm) - 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) assert patch.container.installed_plugins == set( charm._WORDPRESS_DEFAULT_PLUGINS @@ -880,20 +854,16 @@ def test_missing_peer_relation(harness: ops.testing.Harness): @pytest.mark.usefixtures("attach_storage") -def test_mysql_connection_error(harness: ops.testing.Harness, setup_replica_consensus): +def test_mysql_connection_error( + harness: ops.testing.Harness, setup_replica_consensus, setup_database_relation_connection_error +): """ arrange: charm peer relation is ready and the storage is attached. act: config the charm to connect to a non-existent database. assert: charm should enter blocked state, and the database error should be seen in the status. """ + setup_database_relation_connection_error() setup_replica_consensus() - db_config = { - "db_host": "a", - "db_name": "b", - "db_user": "c", - "db_password": "d", - } - harness.update_config(db_config) assert isinstance(harness.model.unit.status, ops.charm.model.BlockedStatus) assert harness.model.unit.status.message == "MySQL error 2003"