no need for generic InstanceHelper

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()

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

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)

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.
 

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]:

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):

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

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.

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.

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.

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.

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.

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.