diff --git a/src/charm.py b/src/charm.py index 8a45bbb23..643f977ff 100755 --- a/src/charm.py +++ b/src/charm.py @@ -245,7 +245,7 @@ def _setup_state(self) -> CharmState: except CharmConfigInvalidError as exc: raise ConfigurationError(exc.msg) from exc - def _common_install_code(self) -> bool: # noqa: C901 + def _common_install_code(self) -> bool: """Installation code shared between install and upgrade hook. Raises: @@ -333,9 +333,8 @@ def _on_upgrade_charm(self, _: UpgradeCharmEvent) -> None: logger.info("Reinstalling dependencies...") self._common_install_code() - # Temporarily ignore too-complex since this is subject to refactor. @catch_charm_errors - def _on_config_changed(self, _: ConfigChangedEvent) -> None: # noqa: C901 + def _on_config_changed(self, _: ConfigChangedEvent) -> None: """Handle the configuration change.""" state = self._setup_state() self._set_reconcile_timer() diff --git a/src/charm_state.py b/src/charm_state.py index f44bb4125..ea8c69c31 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -1,10 +1,6 @@ # Copyright 2024 Canonical Ltd. # See LICENSE file for licensing details. -# TODO: 2024-06-26 The charm contains a lot of states and configuration. The upcoming refactor will -# split each/related class to a file. -# pylint: disable=too-many-lines - """State of the Charm.""" import dataclasses diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index ac2e6e905..b3db582be 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -37,7 +37,6 @@ ) from tests.integration.helpers.common import ( MONGODB_APP_NAME, - InstanceHelper, deploy_github_runner_charm, reconcile, wait_for, @@ -694,7 +693,7 @@ async def basic_app_fixture(request: pytest.FixtureRequest) -> Application: @pytest_asyncio.fixture(scope="function", name="instance_helper") -async def instance_helper_fixture(request: pytest.FixtureRequest) -> InstanceHelper: +async def instance_helper_fixture(request: pytest.FixtureRequest) -> OpenStackInstanceHelper: """Instance helper fixture.""" openstack_connection = request.getfixturevalue("openstack_connection") return OpenStackInstanceHelper(openstack_connection=openstack_connection) diff --git a/tests/integration/helpers/charm_metrics.py b/tests/integration/helpers/charm_metrics.py index 646c6ae9b..330da17f1 100644 --- a/tests/integration/helpers/charm_metrics.py +++ b/tests/integration/helpers/charm_metrics.py @@ -20,12 +20,8 @@ from juju.application import Application from juju.unit import Unit -from tests.integration.helpers.common import ( - InstanceHelper, - get_file_content, - run_in_unit, - wait_for, -) +from tests.integration.helpers.common import get_file_content, run_in_unit, wait_for +from tests.integration.helpers.openstack import OpenStackInstanceHelper logger = logging.getLogger(__name__) @@ -39,7 +35,7 @@ async def wait_for_workflow_to_start( unit: Unit, workflow: Workflow, - instance_helper: InstanceHelper, + instance_helper: OpenStackInstanceHelper, branch: Branch | None = None, started_time: float | None = None, timeout: int = 20 * 60, @@ -118,7 +114,10 @@ async def get_metrics_log(unit: Unit) -> str: async def cancel_workflow_run( - unit: Unit, workflow: Workflow, instance_helper: InstanceHelper, branch: Branch | None = None + unit: Unit, + workflow: Workflow, + instance_helper: OpenStackInstanceHelper, + branch: Branch | None = None, ): """Cancel the workflow run. diff --git a/tests/integration/helpers/common.py b/tests/integration/helpers/common.py index a7ffb8aa3..b4ac857cb 100644 --- a/tests/integration/helpers/common.py +++ b/tests/integration/helpers/common.py @@ -46,79 +46,6 @@ logger = logging.getLogger(__name__) -class InstanceHelper(typing.Protocol): - """Helper for running commands in instances.""" - - async def run_in_instance( - self, - unit: Unit, - command: str, - timeout: int | None = None, - assert_on_failure: bool = False, - assert_msg: str | None = None, - ) -> tuple[int, str | None, str | None]: - """Run command in instance. - - Args: - unit: Juju unit to execute the command in. - command: Command to execute. - timeout: Amount of time to wait for the execution. - assert_on_failure: Perform assertion on non-zero exit code. - assert_msg: Message for the failure assertion. - """ - ... - - async def expose_to_instance( - self, - unit: Unit, - port: int, - host: str = "localhost", - ) -> None: - """Expose a port on the juju machine to the OpenStack instance. - - Uses SSH remote port forwarding from the juju machine to the OpenStack instance containing - the runner. - - Args: - unit: The juju unit of the github-runner charm. - port: The port on the juju machine to expose to the runner. - host: Host for the reverse tunnel. - """ - ... - - async def ensure_charm_has_runner(self, app: Application): - """Ensure charm has a runner. - - Args: - app: The GitHub Runner Charm app to create the runner for. - """ - ... - - async def get_runner_names(self, unit: Unit) -> list[str]: - """Get the name of all the runners in the unit. - - Args: - unit: The GitHub Runner Charm unit to get the runner names for. - """ - ... - - async def get_runner_name(self, unit: Unit) -> str: - """Get the name of the runner. - - Args: - unit: The GitHub Runner Charm unit to get the runner name for. - """ - ... - - async def delete_single_runner(self, unit: Unit) -> None: - """Delete the only runner. - - Args: - unit: The GitHub Runner Charm unit to delete the runner name for. - """ - ... - - async def run_in_unit( unit: Unit, command: str, timeout=None, assert_on_failure=False, assert_msg="" ) -> tuple[int, str | None, str | None]: diff --git a/tests/integration/helpers/openstack.py b/tests/integration/helpers/openstack.py index 4991ad8af..7fc3c0b90 100644 --- a/tests/integration/helpers/openstack.py +++ b/tests/integration/helpers/openstack.py @@ -12,12 +12,12 @@ from charm import RUNNER_MANAGER_USER from charm_state import VIRTUAL_MACHINES_CONFIG_NAME -from tests.integration.helpers.common import InstanceHelper, reconcile, run_in_unit, wait_for +from tests.integration.helpers.common import reconcile, run_in_unit, wait_for logger = logging.getLogger(__name__) -class OpenStackInstanceHelper(InstanceHelper): +class OpenStackInstanceHelper: """Helper class to interact with OpenStack instances.""" def __init__(self, openstack_connection: openstack.connection.Connection): diff --git a/tests/integration/test_charm_fork_repo.py b/tests/integration/test_charm_fork_repo.py index b75b04947..607721e3e 100644 --- a/tests/integration/test_charm_fork_repo.py +++ b/tests/integration/test_charm_fork_repo.py @@ -17,11 +17,9 @@ from tests.integration.helpers.common import ( DISPATCH_FAILURE_TEST_WORKFLOW_FILENAME, - InstanceHelper, dispatch_workflow, ) from tests.integration.helpers.openstack import OpenStackInstanceHelper, setup_repo_policy -from tests.status_name import ACTIVE @pytest.mark.openstack @@ -32,7 +30,7 @@ async def test_dispatch_workflow_failure( app_with_forked_repo: Application, forked_github_repository: Repository, forked_github_branch: Branch, - instance_helper: InstanceHelper, + instance_helper: OpenStackInstanceHelper, token: str, https_proxy: str, ) -> None: @@ -46,25 +44,12 @@ async def test_dispatch_workflow_failure( """ start_time = datetime.now(timezone.utc) - if isinstance(instance_helper, OpenStackInstanceHelper): - await setup_repo_policy( - app=app_with_forked_repo, - openstack_connection=instance_helper.openstack_connection, - token=token, - https_proxy=https_proxy, - ) - else: - grafana_agent = await model.deploy( - "grafana-agent", - application_name=f"grafana-agent-{app_with_forked_repo.name}", - channel="latest/edge", - ) - await model.relate( - f"{app_with_forked_repo.name}:cos-agent", f"{grafana_agent.name}:cos-agent" - ) - await model.wait_for_idle(apps=[app_with_forked_repo.name], status=ACTIVE) - await model.wait_for_idle(apps=[grafana_agent.name]) - await instance_helper.ensure_charm_has_runner(app_with_forked_repo) + await setup_repo_policy( + app=app_with_forked_repo, + openstack_connection=instance_helper.openstack_connection, + token=token, + https_proxy=https_proxy, + ) workflow = forked_github_repository.get_workflow( id_or_file_name=DISPATCH_FAILURE_TEST_WORKFLOW_FILENAME diff --git a/tests/integration/test_charm_metrics_failure.py b/tests/integration/test_charm_metrics_failure.py index 8f7c9b411..5a7636b3b 100644 --- a/tests/integration/test_charm_metrics_failure.py +++ b/tests/integration/test_charm_metrics_failure.py @@ -25,7 +25,6 @@ from tests.integration.helpers.common import ( DISPATCH_CRASH_TEST_WORKFLOW_FILENAME, DISPATCH_FAILURE_TEST_WORKFLOW_FILENAME, - InstanceHelper, dispatch_workflow, reconcile, ) @@ -62,7 +61,7 @@ async def test_charm_issues_metrics_for_failed_repo_policy( forked_github_branch: Branch, token: str, https_proxy: str, - instance_helper: InstanceHelper, + instance_helper: OpenStackInstanceHelper, ): """ arrange: A properly integrated charm with a runner registered on the fork repo. @@ -72,15 +71,12 @@ async def test_charm_issues_metrics_for_failed_repo_policy( """ await app.set_config({PATH_CONFIG_NAME: forked_github_repository.full_name}) - if isinstance(instance_helper, OpenStackInstanceHelper): - await setup_repo_policy( - app=app, - openstack_connection=instance_helper.openstack_connection, - token=token, - https_proxy=https_proxy, - ) - else: - await instance_helper.ensure_charm_has_runner(app) + await setup_repo_policy( + app=app, + openstack_connection=instance_helper.openstack_connection, + token=token, + https_proxy=https_proxy, + ) # Clear metrics log to make reconciliation event more predictable unit = app.units[0] @@ -116,7 +112,7 @@ async def test_charm_issues_metrics_for_abnormal_termination( app: Application, github_repository: Repository, test_github_branch: Branch, - instance_helper: InstanceHelper, + instance_helper: OpenStackInstanceHelper, ): """ arrange: A properly integrated charm with a runner registered on the fork repo. diff --git a/tests/integration/test_charm_metrics_success.py b/tests/integration/test_charm_metrics_success.py index 932dc5d6e..24261ed41 100644 --- a/tests/integration/test_charm_metrics_success.py +++ b/tests/integration/test_charm_metrics_success.py @@ -22,10 +22,10 @@ ) from tests.integration.helpers.common import ( DISPATCH_TEST_WORKFLOW_FILENAME, - InstanceHelper, dispatch_workflow, reconcile, ) +from tests.integration.helpers.openstack import OpenStackInstanceHelper @pytest_asyncio.fixture(scope="function", name="app") @@ -44,7 +44,7 @@ async def app_fixture(model: Model, app_for_metric: Application) -> AsyncIterato @pytest.mark.asyncio @pytest.mark.abort_on_fail async def test_charm_issues_runner_installed_metric( - app: Application, model: Model, instance_helper: InstanceHelper + app: Application, model: Model, instance_helper: OpenStackInstanceHelper ): """ arrange: A charm integrated with grafana-agent using the cos-agent integration. @@ -77,7 +77,7 @@ async def test_charm_issues_metrics_after_reconciliation( app: Application, github_repository: Repository, test_github_branch: Branch, - instance_helper: InstanceHelper, + instance_helper: OpenStackInstanceHelper, ): """ arrange: A properly integrated charm with a runner registered on the fork repo. diff --git a/tests/integration/test_charm_runner.py b/tests/integration/test_charm_runner.py index d540e5661..3279e4598 100644 --- a/tests/integration/test_charm_runner.py +++ b/tests/integration/test_charm_runner.py @@ -16,7 +16,6 @@ from tests.integration.helpers.common import ( DISPATCH_TEST_WORKFLOW_FILENAME, DISPATCH_WAIT_TEST_WORKFLOW_FILENAME, - InstanceHelper, dispatch_workflow, reconcile, wait_for, @@ -28,7 +27,7 @@ async def app_fixture( model: Model, basic_app: Application, - instance_helper: InstanceHelper, + instance_helper: OpenStackInstanceHelper, ) -> AsyncIterator[Application]: """Setup and teardown the charm after each test. @@ -139,7 +138,7 @@ async def test_repo_policy_enabled( test_github_branch: Branch, token: str, https_proxy: str, - instance_helper: InstanceHelper, + instance_helper: OpenStackInstanceHelper, ) -> None: """ arrange: A working application with one runner with repo policy enabled. diff --git a/tests/integration/test_charm_scheduled_events.py b/tests/integration/test_charm_scheduled_events.py index 6e2224f85..0448521fc 100644 --- a/tests/integration/test_charm_scheduled_events.py +++ b/tests/integration/test_charm_scheduled_events.py @@ -13,7 +13,8 @@ from juju.application import Application from juju.model import Model -from tests.integration.helpers.common import InstanceHelper, wait_for +from tests.integration.helpers.common import wait_for +from tests.integration.helpers.openstack import OpenStackInstanceHelper from tests.status_name import ACTIVE pytestmark = pytest.mark.openstack @@ -24,7 +25,7 @@ async def test_update_interval( model: Model, app_scheduled_events: Application, - instance_helper: InstanceHelper, + instance_helper: OpenStackInstanceHelper, ) -> None: """ arrange: A working application with one runner. diff --git a/tests/integration/test_debug_ssh.py b/tests/integration/test_debug_ssh.py index 425910567..eefaf5854 100644 --- a/tests/integration/test_debug_ssh.py +++ b/tests/integration/test_debug_ssh.py @@ -10,7 +10,8 @@ from juju.application import Application from juju.model import Model -from tests.integration.helpers.common import InstanceHelper, dispatch_workflow, get_job_logs +from tests.integration.helpers.common import dispatch_workflow, get_job_logs +from tests.integration.helpers.openstack import OpenStackInstanceHelper from tests.status_name import ACTIVE logger = logging.getLogger(__name__) @@ -26,7 +27,7 @@ async def test_ssh_debug( github_repository: Repository, test_github_branch: Branch, tmate_ssh_server_unit_ip: str, - instance_helper: InstanceHelper, + instance_helper: OpenStackInstanceHelper, ): """ arrange: given an integrated GitHub-Runner charm and tmate-ssh-server charm. diff --git a/tests/integration/test_e2e.py b/tests/integration/test_e2e.py index 77ef2bfb7..ffc371649 100644 --- a/tests/integration/test_e2e.py +++ b/tests/integration/test_e2e.py @@ -11,16 +11,16 @@ from tests.integration.helpers.common import ( DISPATCH_E2E_TEST_RUN_WORKFLOW_FILENAME, - InstanceHelper, dispatch_workflow, ) +from tests.integration.helpers.openstack import OpenStackInstanceHelper @pytest_asyncio.fixture(scope="function", name="app") async def app_fixture( model: Model, basic_app: Application, - instance_helper: InstanceHelper, + instance_helper: OpenStackInstanceHelper, ) -> AsyncIterator[Application]: """Setup and teardown the charm after each test.