From 3dba162d3a0bede5ada4d0c35b88e95a876ca8a0 Mon Sep 17 00:00:00 2001 From: javierdelapuente Date: Wed, 18 Dec 2024 08:37:54 +0100 Subject: [PATCH] All integrations tests should run in openstack (#413) --- .github/workflows/integration_test.yaml | 10 ++-- tests/integration/conftest.py | 57 +++++++------------ tests/integration/helpers/common.py | 43 +++++++++++++- tests/integration/helpers/lxd.py | 53 ++++++++++++++++- tests/integration/helpers/openstack.py | 47 +++++++++------ .../test_charm_scheduled_events.py | 49 ++++++++-------- tests/integration/test_charm_upgrade.py | 44 ++++++++++---- tests/integration/test_debug_ssh.py | 23 ++++++-- 8 files changed, 225 insertions(+), 101 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index c0a1f66cf..f6ec9226e 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -16,14 +16,13 @@ jobs: secrets: inherit with: juju-channel: 3.1/stable - pre-run-script: scripts/pre-integration-test.sh + pre-run-script: scripts/setup-lxd.sh provider: lxd test-tox-env: integration-juju3.1 - # These important local LXD test have no OpenStack integration versions. - # test_charm_scheduled_events ensures reconcile events are fired on a schedule. - # test_debug_ssh ensures tmate SSH actions works. - # The test test_charm_upgrade needs to run to ensure the charm can be upgraded. modules: '["test_charm_scheduled_events", "test_debug_ssh", "test_charm_upgrade"]' + extra-arguments: "-m openstack" + self-hosted-runner: true + self-hosted-runner-label: stg-private-endpoint openstack-interface-tests-private-endpoint: name: openstack interface test using private-endpoint uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main @@ -39,7 +38,6 @@ jobs: openstack-integration-tests-private-endpoint: name: Integration test using private-endpoint uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main - needs: openstack-interface-tests-private-endpoint secrets: inherit with: juju-channel: 3.6/stable diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index e52fa99d3..cb41d84a5 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -440,7 +440,7 @@ async def app_openstack_runner_fixture( reconcile_interval=60, constraints={ "root-disk": 50 * 1024, - "mem": 16 * 1024, + "mem": 2 * 1024, }, config={ OPENSTACK_CLOUDS_YAML_CONFIG_NAME: clouds_yaml_contents, @@ -470,43 +470,28 @@ async def app_one_runner(model: Model, app_no_runner: Application) -> AsyncItera return app_no_runner -@pytest_asyncio.fixture(scope="module") -async def app_scheduled_events( +@pytest_asyncio.fixture(scope="module", name="app_scheduled_events") +async def app_scheduled_events_fixture( model: Model, - charm_file: str, - app_name: str, - path: str, - token: str, - http_proxy: str, - https_proxy: str, - no_proxy: str, -) -> AsyncIterator[Application]: - """Application with no token. - - Test should ensure it returns with the application having one runner. - - This fixture has to deploy a new application. The scheduled events are set - to one hour in other application to avoid conflicting with the tests. - Changes to the duration of scheduled interval only takes effect after the - next trigger. Therefore, it would take a hour for the duration change to - take effect. - """ - application = await deploy_github_runner_charm( - model=model, - charm_file=charm_file, - app_name=app_name, - path=path, - token=token, - runner_storage="memory", - http_proxy=http_proxy, - https_proxy=https_proxy, - no_proxy=no_proxy, - reconcile_interval=8, - ) - + app_openstack_runner, +): + """Application to check scheduled events.""" + application = app_openstack_runner + await application.set_config({"reconcile-interval": "8"}) await application.set_config({VIRTUAL_MACHINES_CONFIG_NAME: "1"}) + await model.wait_for_idle(apps=[application.name], status=ACTIVE, timeout=90 * 60) await reconcile(app=application, model=model) + return application + +@pytest_asyncio.fixture(scope="module", name="app_no_wait_tmate") +async def app_no_wait_tmate_fixture( + model: Model, + app_openstack_runner, +): + """Application to check tmate ssh with openstack without waiting for active.""" + application = app_openstack_runner + await application.set_config({"reconcile-interval": "60", VIRTUAL_MACHINES_CONFIG_NAME: "1"}) return application @@ -569,11 +554,11 @@ async def app_no_wait_fixture( @pytest_asyncio.fixture(scope="module", name="tmate_ssh_server_app") async def tmate_ssh_server_app_fixture( - model: Model, app_no_wait: Application + model: Model, app_no_wait_tmate: Application ) -> AsyncIterator[Application]: """tmate-ssh-server charm application related to GitHub-Runner app charm.""" tmate_app: Application = await model.deploy("tmate-ssh-server", channel="edge") - await app_no_wait.relate("debug-ssh", f"{tmate_app.name}:debug-ssh") + await app_no_wait_tmate.relate("debug-ssh", f"{tmate_app.name}:debug-ssh") await model.wait_for_idle(apps=[tmate_app.name], status=ACTIVE, timeout=60 * 30) return tmate_app diff --git a/tests/integration/helpers/common.py b/tests/integration/helpers/common.py index 06e847665..fcc0731fa 100644 --- a/tests/integration/helpers/common.py +++ b/tests/integration/helpers/common.py @@ -55,7 +55,12 @@ class InstanceHelper(typing.Protocol): """Helper for running commands in instances.""" async def run_in_instance( - self, unit: Unit, command: str, timeout: int | None = None + 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. @@ -63,6 +68,26 @@ async def run_in_instance( 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. """ ... @@ -74,6 +99,14 @@ async def ensure_charm_has_runner(self, app: Application): """ ... + 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. @@ -82,6 +115,14 @@ async def get_runner_name(self, unit: Unit) -> str: """ ... + 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 check_runner_binary_exists(unit: Unit) -> bool: """Checks if runner binary exists in the charm. diff --git a/tests/integration/helpers/lxd.py b/tests/integration/helpers/lxd.py index f1e2099ef..aee139ca4 100644 --- a/tests/integration/helpers/lxd.py +++ b/tests/integration/helpers/lxd.py @@ -20,7 +20,12 @@ class LXDInstanceHelper(InstanceHelper): """Helper class to interact with LXD instances.""" async def run_in_instance( - self, unit: Unit, command: str, timeout: int | None = None + 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 LXD instance. @@ -28,6 +33,8 @@ async def run_in_instance( unit: Juju unit to execute the command in. command: Command to execute. timeout: Amount of time to wait for the execution. + assert_on_failure: Not used in lxd + assert_msg: Not used in lxd Returns: Tuple of return code, stdout and stderr. @@ -35,6 +42,27 @@ async def run_in_instance( name = await self.get_runner_name(unit) return await run_in_lxd_instance(unit, name, command, timeout=timeout) + 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. + + Raises: + NotImplementedError: Not implemented yet. + """ + raise NotImplementedError + async def ensure_charm_has_runner(self, app: Application): """Reconcile the charm to contain one runner. @@ -56,6 +84,29 @@ async def get_runner_name(self, unit: Unit) -> str: """ return await get_runner_name(unit) + 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. + + Raises: + NotImplementedError: Not implemented yet. + """ + raise NotImplementedError + + async def delete_single_runner(self, unit: Unit) -> None: + """Delete the only runner. + + + Args: + unit: The GitHub Runner Charm unit to check. + + Raises: + NotImplementedError: Not implemented yet. + """ + raise NotImplementedError + async def assert_resource_lxd_profile(unit: Unit, configs: dict[str, Any]) -> None: """Check for LXD profile of the matching resource config. diff --git a/tests/integration/helpers/openstack.py b/tests/integration/helpers/openstack.py index ddb89bfd8..9696f5d10 100644 --- a/tests/integration/helpers/openstack.py +++ b/tests/integration/helpers/openstack.py @@ -3,7 +3,7 @@ import logging import secrets from asyncio import sleep -from typing import Optional, TypedDict, cast +from typing import Optional, TypedDict import openstack.connection from juju.application import Application @@ -32,6 +32,7 @@ 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. @@ -41,6 +42,7 @@ async def expose_to_instance( 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. """ runner = self._get_single_runner(unit=unit) assert runner, f"Runner not found for unit {unit.name}" @@ -61,7 +63,7 @@ async def expose_to_instance( key_path = f"/home/{RUNNER_MANAGER_USER}/.ssh/{runner.name}.key" exit_code, _, _ = await run_in_unit(unit, f"ls {key_path}") assert exit_code == 0, f"Unable to find key file {key_path}" - ssh_cmd = f'ssh -fNT -R {port}:localhost:{port} -i {key_path} -o "StrictHostKeyChecking no" -o "ControlPersist yes" ubuntu@{ip} &' + ssh_cmd = f'ssh -fNT -R {port}:{host}:{port} -i {key_path} -o "StrictHostKeyChecking no" -o "ControlPersist yes" ubuntu@{ip} &' exit_code, _, stderr = await run_in_unit(unit, ssh_cmd) assert ( exit_code == 0 @@ -150,6 +152,18 @@ async def _set_app_runner_amount(app: Application, num_runners: int) -> None: await app.set_config({VIRTUAL_MACHINES_CONFIG_NAME: f"{num_runners}"}) await reconcile(app=app, model=app.model) + 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. + + Returns: + List of names for the runners. + """ + runners = self._get_runners(unit) + return [runner.name for runner in runners] + async def get_runner_name(self, unit: Unit) -> str: """Get the name of the runner. @@ -161,24 +175,27 @@ async def get_runner_name(self, unit: Unit) -> str: Returns: The Github runner name deployed in the given unit. """ - runners = await self._get_runner_names(unit) + runners = self._get_runners(unit) assert len(runners) == 1 - return runners[0] + return runners[0].name - async def _get_runner_names(self, unit: Unit) -> tuple[str, ...]: - """Get names of the runners in LXD. + async def delete_single_runner(self, unit: Unit) -> None: + """Delete the only runner. Args: - unit: Unit instance to check for the LXD profile. - - Returns: - Tuple of runner names. + unit: The GitHub Runner Charm unit to delete the runner name for. """ runner = self._get_single_runner(unit) - assert runner, "Failed to find runner server" - return (cast(str, runner.name),) + self.openstack_connection.delete_server(name_or_id=runner.id) + + def _get_runners(self, unit: Unit) -> list[Server]: + """Get all runners for the unit.""" + servers: list[Server] = self.openstack_connection.list_servers() + unit_name_without_slash = unit.name.replace("/", "-") + runners = [server for server in servers if server.name.startswith(unit_name_without_slash)] + return runners - def _get_single_runner(self, unit: Unit) -> Server | None: + def _get_single_runner(self, unit: Unit) -> Server: """Get the only runner for the unit. This method asserts for exactly one runner for the unit. @@ -189,9 +206,7 @@ def _get_single_runner(self, unit: Unit) -> Server | None: Returns: The runner server. """ - servers: list[Server] = self.openstack_connection.list_servers() - unit_name_without_slash = unit.name.replace("/", "-") - runners = [server for server in servers if server.name.startswith(unit_name_without_slash)] + runners = self._get_runners(unit) assert ( len(runners) == 1 ), f"In {unit.name} found more than one runners or no runners: {runners}" diff --git a/tests/integration/test_charm_scheduled_events.py b/tests/integration/test_charm_scheduled_events.py index 5e9819f23..6e2224f85 100644 --- a/tests/integration/test_charm_scheduled_events.py +++ b/tests/integration/test_charm_scheduled_events.py @@ -13,49 +13,48 @@ from juju.application import Application from juju.model import Model -from runner_manager import LXDRunnerManager -from tests.integration.helpers.common import check_runner_binary_exists -from tests.integration.helpers.lxd import get_runner_names, run_in_unit, wait_till_num_of_runners +from tests.integration.helpers.common import InstanceHelper, wait_for from tests.status_name import ACTIVE +pytestmark = pytest.mark.openstack + @pytest.mark.asyncio @pytest.mark.abort_on_fail -async def test_update_interval(model: Model, app_scheduled_events: Application) -> None: +async def test_update_interval( + model: Model, + app_scheduled_events: Application, + instance_helper: InstanceHelper, +) -> None: """ arrange: A working application with one runner. act: - 1. a. Remove runner binary. - b. Crash the one runner + 1. a. Crash/delete the one runner 2. Wait for 6 minutes, and then wait for ActiveStatus. assert: - 1. a. No runner binary exists. - b. No runner exists. - 2. a. Runner binary exists. - b. One runner exists. The runner name should not be the same as the starting one. + 1. a. No runner exists. + 2. a. One runner exists. The runner name should not be the same as the starting one. This tests whether the reconcile-runner event is triggered, and updates the dependencies. The reconciliation logic is tested with the reconcile-runners action. """ unit = app_scheduled_events.units[0] - assert await check_runner_binary_exists(unit) - ret_code, stdout, stderr = await run_in_unit(unit, f"rm -f {LXDRunnerManager.runner_bin_path}") - assert ret_code == 0, f"Failed to remove runner binary {stdout} {stderr}" - assert not await check_runner_binary_exists(unit) + oldnames = await instance_helper.get_runner_names(unit) + assert len(oldnames) == 1, "There should be one runner" + + # delete the only runner + await instance_helper.delete_single_runner(unit) - runner_names = await get_runner_names(unit) - assert len(runner_names) == 1 - runner_name = runner_names[0] - ret_code, stdout, stderr = await run_in_unit(unit, f"lxc stop --force {runner_name}") - assert ret_code == 0, f"Failed to stop lxd instance, {stdout} {stderr}" - await wait_till_num_of_runners(unit, 0) + async def _no_runners_available() -> bool: + """Check if there is only one runner.""" + return len(await instance_helper.get_runner_names(unit)) == 0 + + await wait_for(_no_runners_available, timeout=30, check_interval=3) await sleep(10 * 60) await model.wait_for_idle(status=ACTIVE, timeout=20 * 60) - assert await check_runner_binary_exists(unit) - - runner_names = await get_runner_names(unit) - assert len(runner_names) == 1 - assert runner_name != runner_names[0] + newnames = await instance_helper.get_runner_names(unit) + assert len(newnames) == 1, "There should be one runner after reconciliation" + assert newnames[0] != oldnames[0] diff --git a/tests/integration/test_charm_upgrade.py b/tests/integration/test_charm_upgrade.py index 071f89278..2f9fb3caf 100644 --- a/tests/integration/test_charm_upgrade.py +++ b/tests/integration/test_charm_upgrade.py @@ -7,18 +7,26 @@ import pathlib import pytest +from juju.application import Application from juju.client import client from juju.model import Model from pytest_operator.plugin import OpsTest -from charm_state import VIRTUAL_MACHINES_CONFIG_NAME +from charm_state import ( + OPENSTACK_CLOUDS_YAML_CONFIG_NAME, + OPENSTACK_FLAVOR_CONFIG_NAME, + OPENSTACK_NETWORK_CONFIG_NAME, + USE_APROXY_CONFIG_NAME, + VIRTUAL_MACHINES_CONFIG_NAME, +) from tests.integration.helpers.common import ( deploy_github_runner_charm, - inject_lxd_profile, is_upgrade_charm_event_emitted, wait_for, ) +pytestmark = pytest.mark.openstack + @pytest.mark.asyncio async def test_charm_upgrade( @@ -29,18 +37,22 @@ async def test_charm_upgrade( app_name: str, path: str, token: str, - http_proxy: str, - https_proxy: str, - no_proxy: str, + openstack_http_proxy: str, + openstack_https_proxy: str, + openstack_no_proxy: str, tmp_path: pathlib.Path, + clouds_yaml_contents: str, + network_name: str, + flavor_name: str, + image_builder: Application, ): """ - arrange: given latest stable version of the charm (current 161). + arrange: given latest stable version of the charm. act: charm upgrade is called. assert: the charm is upgraded successfully. """ latest_stable_path = tmp_path / "github-runner.charm" - latest_stable_revision = 282 # update this value every release to stable. + latest_stable_revision = 302 # update this value every release to stable. # download the charm and inject lxd profile for testing retcode, stdout, stderr = await ops_test.juju( "download", @@ -56,7 +68,6 @@ async def test_charm_upgrade( "--no-progress", ) assert retcode == 0, f"failed to download charm, {stdout} {stderr}" - inject_lxd_profile(pathlib.Path(latest_stable_path), loop_device=loop_device) # deploy latest stable version of the charm application = await deploy_github_runner_charm( @@ -66,13 +77,22 @@ async def test_charm_upgrade( path=path, token=token, runner_storage="juju-storage", - http_proxy=http_proxy, - https_proxy=https_proxy, - no_proxy=no_proxy, + http_proxy=openstack_http_proxy, + https_proxy=openstack_https_proxy, + no_proxy=openstack_no_proxy, reconcile_interval=5, # override default virtual_machines=0 config. - config={VIRTUAL_MACHINES_CONFIG_NAME: 1}, + config={ + OPENSTACK_CLOUDS_YAML_CONFIG_NAME: clouds_yaml_contents, + OPENSTACK_NETWORK_CONFIG_NAME: network_name, + OPENSTACK_FLAVOR_CONFIG_NAME: flavor_name, + USE_APROXY_CONFIG_NAME: "true", + VIRTUAL_MACHINES_CONFIG_NAME: 1, + }, + wait_idle=False, + use_local_lxd=False, ) + await model.integrate(f"{image_builder.name}:image", f"{application.name}:image") await model.wait_for_idle( apps=[application.name], raise_on_error=False, diff --git a/tests/integration/test_debug_ssh.py b/tests/integration/test_debug_ssh.py index 6bddc0ace..d153b6591 100644 --- a/tests/integration/test_debug_ssh.py +++ b/tests/integration/test_debug_ssh.py @@ -4,26 +4,30 @@ """Integration tests for github-runner charm with ssh-debug integration.""" import logging +import pytest from github.Branch import Branch from github.Repository import Repository from juju.application import Application from juju.model import Model from charm_state import DENYLIST_CONFIG_NAME -from tests.integration.helpers.common import dispatch_workflow, get_job_logs +from tests.integration.helpers.common import InstanceHelper, dispatch_workflow, get_job_logs from tests.status_name import ACTIVE logger = logging.getLogger(__name__) SSH_DEBUG_WORKFLOW_FILE_NAME = "workflow_dispatch_ssh_debug.yaml" +pytestmark = pytest.mark.openstack + async def test_ssh_debug( model: Model, - app_no_wait: Application, + app_no_wait_tmate: Application, github_repository: Repository, test_github_branch: Branch, tmate_ssh_server_unit_ip: str, + instance_helper: InstanceHelper, ): """ arrange: given an integrated GitHub-Runner charm and tmate-ssh-server charm with a denylist \ @@ -31,7 +35,7 @@ async def test_ssh_debug( act: when canonical/action-tmate is triggered. assert: the ssh connection info from action-log and tmate-ssh-server matches. """ - await app_no_wait.set_config( + await app_no_wait_tmate.set_config( { DENYLIST_CONFIG_NAME: ( "0.0.0.0/8,10.0.0.0/8,100.64.0.0/10,169.254.0.0/16," @@ -43,12 +47,23 @@ async def test_ssh_debug( ) await model.wait_for_idle(status=ACTIVE, timeout=60 * 120) + unit = app_no_wait_tmate.units[0] + # We need the runner to connect to the current machine, instead of the tmate_ssh_server unit, + # as the tmate_ssh_server is not routable. + dnat_comman_in_runner = "sudo iptables -t nat -A OUTPUT -p tcp --dport 10022 -j DNAT --to-destination 127.0.0.1:10022" + _, _, _ = await instance_helper.run_in_instance( + unit, + dnat_comman_in_runner, + assert_on_failure=True, + ) + await instance_helper.expose_to_instance(unit=unit, port=10022, host=tmate_ssh_server_unit_ip) + # trigger tmate action logger.info("Dispatching workflow_dispatch_ssh_debug.yaml workflow.") # expect failure since the ssh workflow will timeout workflow_run = await dispatch_workflow( - app=app_no_wait, + app=app_no_wait_tmate, branch=test_github_branch, github_repository=github_repository, conclusion="failure",