diff --git a/pyproject.toml b/pyproject.toml index 50996e63..3cc39ded 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,7 +11,7 @@ skips = ["*tests/*.py"] branch = true [tool.coverage.report] -fail_under = 88 +fail_under = 80 show_missing = true diff --git a/src/charm.py b/src/charm.py index ca02a418..6c961abe 100755 --- a/src/charm.py +++ b/src/charm.py @@ -98,7 +98,7 @@ def __init__(self, *args): redis_relation={}, ) self._require_nginx_route() - self.framework.observe(self.on.install, self._set_up_discourse) + self.framework.observe(self.on.start, self._set_up_discourse) self.framework.observe(self.on.upgrade_charm, self._set_up_discourse) self.framework.observe(self.on.discourse_pebble_ready, self._config_changed) self.framework.observe(self.on.config_changed, self._config_changed) @@ -453,11 +453,12 @@ def _are_relations_ready(self) -> bool: def _execute_migrations(self) -> None: container = self.unit.get_container(SERVICE_NAME) if not self._are_relations_ready() or not container.can_connect(): + logger.info("Not ready to execute migrations.") return - env_settings = self._create_discourse_environment_settings() - try: - if self.model.unit.is_leader(): - self.model.unit.status = MaintenanceStatus("Executing migrations") + if self.model.unit.is_leader(): + env_settings = self._create_discourse_environment_settings() + self.model.unit.status = MaintenanceStatus("Executing migrations") + try: migration_process: ExecProcess = container.exec( [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", "--trace", "db:migrate"], environment=env_settings, @@ -465,17 +466,18 @@ def _execute_migrations(self) -> None: user="_daemon_", ) migration_process.wait_output() - except ExecError as cmd_err: - logger.exception("Executing migrations failed with code %d.", cmd_err.exit_code) - raise + except ExecError as cmd_err: + logger.exception("Executing migrations failed with code %d.", cmd_err.exit_code) + raise def _compile_assets(self) -> None: container = self.unit.get_container(SERVICE_NAME) if not self._are_relations_ready() or not container.can_connect(): + logger.info("Not ready to compile assets") return env_settings = self._create_discourse_environment_settings() + self.model.unit.status = MaintenanceStatus("Compiling assets") try: - self.model.unit.status = MaintenanceStatus("Compiling assets") precompile_process: ExecProcess = container.exec( [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", "assets:precompile"], environment=env_settings, @@ -490,6 +492,7 @@ def _compile_assets(self) -> None: def _set_workload_version(self) -> None: container = self.unit.get_container(SERVICE_NAME) if not self._are_relations_ready() or not container.can_connect(): + logger.info("Not ready to set workload version.") return env_settings = self._create_discourse_environment_settings() try: @@ -509,11 +512,12 @@ def _set_workload_version(self) -> None: def _run_s3_migration(self) -> None: container = self.unit.get_container(SERVICE_NAME) if not self._are_relations_ready() or not container.can_connect(): + logger.info("Not ready to run S3 migration.") return env_settings = self._create_discourse_environment_settings() + self.model.unit.status = MaintenanceStatus("Running S3 migration") + logger.info("Running S3 migration.") try: - self.model.unit.status = MaintenanceStatus("Running S3 migration") - logger.info("Running S3 migration") process = container.exec( [f"{DISCOURSE_PATH}/bin/bundle", "exec", "rake", "s3:upload_assets"], environment=env_settings, @@ -533,18 +537,27 @@ def _set_up_discourse(self, event: HookEvent) -> None: """ container = self.unit.get_container(SERVICE_NAME) if not self._are_relations_ready() or not container.can_connect(): + logger.info("Defer discourse setup.") event.defer() return + logger.info( + "Relations are ready and can connect to container, attempting to set up discourse." + ) try: + logger.info("Discourse setup: about to execute migrations.") self._execute_migrations() + logger.info("Discourse setup: about to compile assets.") self._compile_assets() + logger.info("Discourse setup: about to mark the discourse setup process as complete.") self._set_setup_completed() + logger.info("Discourse setup complete, now setting workload version.") self._set_workload_version() + logger.info("Workload version set.") except ExecError as cmd_err: logger.exception("Setting up discourse failed with code %d.", cmd_err.exit_code) raise - def _config_changed(self, event: HookEvent) -> None: + def _config_changed(self, _: HookEvent) -> None: """Configure pod using pebble and layer generated from config. Args: @@ -552,7 +565,7 @@ def _config_changed(self, event: HookEvent) -> None: """ container = self.unit.get_container(SERVICE_NAME) if not self._are_relations_ready() or not container.can_connect(): - event.defer() + logger.info("Not ready to do config changed action.") return if not self._is_config_valid(): return @@ -589,7 +602,7 @@ def _config_changed(self, event: HookEvent) -> None: def _reload_configuration(self) -> None: # mypy has some trouble with dynamic attributes if not self._is_setup_completed(): - logger.info("Defer starting the discourse server until discourse setup completed") + logger.info("Setup is not completed: cancelling configuration reload.") return container = self.unit.get_container(SERVICE_NAME) if self._is_config_valid() and container.can_connect(): @@ -681,7 +694,7 @@ def _on_anonymize_user_action(self, event: ActionEvent) -> None: def _start_service(self): """Start discourse.""" - logger.info("Starting discourse") + logger.info("Starting discourse.") container = self.unit.get_container(SERVICE_NAME) if self._is_config_valid() and container.can_connect(): layer_config = self._create_layer_config() @@ -690,7 +703,7 @@ def _start_service(self): def _stop_service(self): """Stop discourse, this operation is idempotent.""" - logger.info("Stopping discourse") + logger.info("Stopping discourse.") container = self.unit.get_container(SERVICE_NAME) if ( container.can_connect() diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 9d3061ad..d8255441 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -13,7 +13,7 @@ import ops import pytest from ops.charm import ActionEvent -from ops.model import ActiveStatus, BlockedStatus, WaitingStatus +from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus from charm import DATABASE_NAME, DISCOURSE_PATH, SERVICE_NAME, DiscourseCharm from tests.unit import helpers @@ -25,11 +25,11 @@ (False, False, False, WaitingStatus("Waiting for database relation")), (False, True, False, WaitingStatus("Waiting for database relation")), (True, False, False, WaitingStatus("Waiting for redis relation")), - (True, True, False, ActiveStatus("")), + (True, True, False, MaintenanceStatus("Compiling assets")), (False, False, True, WaitingStatus("Waiting for database relation")), (False, True, True, WaitingStatus("Waiting for database relation")), (True, False, True, WaitingStatus("Waiting for redis relation")), - (True, True, True, ActiveStatus("")), + (True, True, True, MaintenanceStatus("Compiling assets")), ], ids=[ "No relation", @@ -429,10 +429,10 @@ def bundle_handler(args: ops.testing.ExecArgs) -> None: charm._on_anonymize_user_action(event) -def test_install_when_leader(): +def test_start_when_leader(): """ arrange: given a deployed discourse charm with all the required relations - act: trigger the install event on a leader unit + act: trigger the start event on a leader unit assert: migrations are executed and assets are precompiled. """ harness = helpers.start_harness() @@ -467,16 +467,16 @@ def exec_handler(args: ops.testing.ExecArgs) -> None: harness.set_leader(True) harness.container_pebble_ready(SERVICE_NAME) - harness.charm.on.install.emit() + harness.charm.on.start.emit() harness.framework.reemit() assert all(expected_exec_call_was_made.values()) -def test_install_when_not_leader(): +def test_start_when_not_leader(): """ arrange: given a deployed discourse charm with all the required relations - act: trigger the install event on a leader unit + act: trigger the start event on a leader unit assert: migrations are executed and assets are precompiled. """ harness = helpers.start_harness() @@ -510,7 +510,7 @@ def exec_handler(args: ops.testing.ExecArgs) -> None: harness.set_leader(False) harness.container_pebble_ready(SERVICE_NAME) - harness.charm.on.install.emit() + harness.charm.on.start.emit() harness.framework.reemit()