diff --git a/requirements.txt b/requirements.txt index e8529761c..c0420e69d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,4 +12,4 @@ cosl ==0.0.15 # juju 3.1.2.0 depends on pyyaml<=6.0 and >=5.1.2 PyYAML ==6.0.* pyOpenSSL==24.2.1 -github_runner_manager @ git+https://github.com/canonical/github-runner-manager.git@2866c28b62e966a179931201e786c548573e6e7c +github_runner_manager @ git+https://github.com/canonical/github-runner-manager.git@30fcc502eba82c4c4216f66f61380cfcbed517b1 diff --git a/src-docs/charm.md b/src-docs/charm.md index 5458c6a13..7bed9337b 100644 --- a/src-docs/charm.md +++ b/src-docs/charm.md @@ -18,10 +18,11 @@ Charm for creating and managing GitHub self-hosted runner instances. - **RECONCILIATION_INTERVAL_TIMEOUT_FACTOR** - **RECONCILE_RUNNERS_EVENT** - **REACTIVE_MQ_DB_NAME** +- **GITHUB_SELF_HOSTED_ARCH_LABELS** --- - + ## function `catch_charm_errors` @@ -47,7 +48,7 @@ Catch common errors in charm. --- - + ## function `catch_action_errors` @@ -73,7 +74,7 @@ Catch common errors in actions. --- - + ## class `ReconcileRunnersEvent` Event representing a periodic check to ensure runners are ok. @@ -84,7 +85,7 @@ Event representing a periodic check to ensure runners are ok. --- - + ## class `GithubRunnerCharm` Charm for managing GitHub self-hosted runners. @@ -101,7 +102,7 @@ Charm for managing GitHub self-hosted runners. - `ram_pool_path`: The path to memdisk storage. - `kernel_module_path`: The path to kernel modules. - + ### method `__init__` diff --git a/src/charm.py b/src/charm.py index b5e437104..ed98094ee 100755 --- a/src/charm.py +++ b/src/charm.py @@ -107,6 +107,8 @@ REACTIVE_MQ_DB_NAME = "github-runner-webhook-router" +GITHUB_SELF_HOSTED_ARCH_LABELS = {"x64", "arm64"} + logger = logging.getLogger(__name__) @@ -1281,6 +1283,9 @@ def _get_runner_scaler( ) reactive_runner_config = None if reactive_config := state.reactive_config: + # The charm is not able to determine which architecture the runner is running on, + # so we add all architectures to the supported labels. + supported_labels = set(self._create_labels(state)) | GITHUB_SELF_HOSTED_ARCH_LABELS reactive_runner_config = ReactiveRunnerConfig( queue=ReactiveQueueConfig( mongodb_uri=reactive_config.mq_uri, queue_name=self.app.name @@ -1288,11 +1293,29 @@ def _get_runner_scaler( runner_manager=runner_manager_config, cloud_runner_manager=openstack_runner_manager_config, github_token=token, + supported_labels=supported_labels, ) return RunnerScaler( runner_manager=runner_manager, reactive_runner_config=reactive_runner_config ) + @staticmethod + def _create_labels(state: CharmState) -> list[str]: + """Create Labels instance. + + Args: + state: Charm state used to create the labels. + + Returns: + An instance of Labels. + """ + image_labels = [] + image = state.runner_config.openstack_image + if image and image.id and image.tags: + image_labels = image.tags + + return list(state.charm_config.labels) + image_labels + def _create_openstack_runner_manager_config( self, path: GitHubPath, state: CharmState ) -> OpenStackRunnerManagerConfig: @@ -1316,7 +1339,6 @@ def _create_openstack_runner_manager_config( cloud=clouds[0], ) server_config = None - image_labels = [] image = state.runner_config.openstack_image if image and image.id: server_config = OpenStackServerConfig( @@ -1324,11 +1346,8 @@ def _create_openstack_runner_manager_config( flavor=state.runner_config.openstack_flavor, network=state.runner_config.openstack_network, ) - if image.tags: - image_labels += image.tags - runner_config = GitHubRunnerConfig( - github_path=path, labels=(*state.charm_config.labels, *image_labels) - ) + labels = self._create_labels(state) + runner_config = GitHubRunnerConfig(github_path=path, labels=labels) service_config = SupportServiceConfig( proxy_config=state.proxy_config, dockerhub_mirror=state.charm_config.dockerhub_mirror, diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 4ee3200be..e88b399e8 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -761,18 +761,18 @@ async def mongodb_fixture(model: Model, existing_app: str | None) -> Application @pytest_asyncio.fixture(scope="module", name="app_for_reactive") async def app_for_reactive_fixture( model: Model, - basic_app: Application, + app_openstack_runner: Application, mongodb: Application, existing_app: Optional[str], ) -> Application: """Application for testing reactive.""" if not existing_app: - await model.relate(f"{basic_app.name}:mongodb", f"{mongodb.name}:database") + await model.relate(f"{app_openstack_runner.name}:mongodb", f"{mongodb.name}:database") - await basic_app.set_config({VIRTUAL_MACHINES_CONFIG_NAME: "1"}) - await model.wait_for_idle(apps=[basic_app.name, mongodb.name], status=ACTIVE) + await app_openstack_runner.set_config({VIRTUAL_MACHINES_CONFIG_NAME: "1"}) + await model.wait_for_idle(apps=[app_openstack_runner.name, mongodb.name], status=ACTIVE) - return basic_app + return app_openstack_runner @pytest_asyncio.fixture(scope="module", name="basic_app") diff --git a/tests/integration/test_reactive.py b/tests/integration/test_reactive.py index fc07154cd..ac4966d13 100644 --- a/tests/integration/test_reactive.py +++ b/tests/integration/test_reactive.py @@ -1,9 +1,9 @@ # Copyright 2024 Canonical Ltd. # See LICENSE file for licensing details. + +"""Testing reactive mode. This is only supported for the OpenStack cloud.""" import json -import random import re -import secrets import pytest from github import Branch, Repository @@ -21,9 +21,21 @@ wait_for_completion, ) +pytestmark = pytest.mark.openstack + + +@pytest.fixture(name="setup_queue", autouse=True) +async def setup_queue_fixture( + ops_test: OpsTest, + app_for_reactive: Application, +): + mongodb_uri = await _get_mongodb_uri(ops_test, app_for_reactive) + + _clear_queue(mongodb_uri, app_for_reactive.name) + _assert_queue_is_empty(mongodb_uri, app_for_reactive.name) + -@pytest.mark.openstack -async def test_reactive_mode_consumes_jobs( +async def test_reactive_mode_spawns_runner( ops_test: OpsTest, app_for_reactive: Application, github_repository: Repository, @@ -32,13 +44,9 @@ async def test_reactive_mode_consumes_jobs( """ arrange: A charm integrated with mongodb and a message is added to the queue. act: Call reconcile. - assert: The message is consumed and the job details are logged. + assert: The message is consumed and a runner is spawned. """ - unit = app_for_reactive.units[0] - mongodb_uri = await _get_mongodb_uri_from_integration_data(ops_test, unit) - if not mongodb_uri: - mongodb_uri = await _get_mongodb_uri_from_secrets(ops_test, unit.model) - assert mongodb_uri, "mongodb uri not found in integration data or secret" + mongodb_uri = await _get_mongodb_uri(ops_test, app_for_reactive) run = await dispatch_workflow( app=app_for_reactive, @@ -53,10 +61,12 @@ async def test_reactive_mode_consumes_jobs( job = jobs[0] job_url = job.url job = JobDetails( - labels=[secrets.token_hex(16) for _ in range(random.randint(1, 4))], url=job_url + labels={app_for_reactive.name, "x64"}, # The architecture label should be ignored in the + # label validation in the reactive consumer. + url=job_url, ) _add_to_queue( - json.dumps(job.dict() | {"ignored_noise": "foobar"}), + json.dumps(json.loads(job.json()) | {"ignored_noise": "foobar"}), mongodb_uri, app_for_reactive.name, ) @@ -70,6 +80,65 @@ async def test_reactive_mode_consumes_jobs( await reconcile(app_for_reactive, app_for_reactive.model) +async def test_reactive_mode_does_not_consume_jobs_with_unsupported_labels( + ops_test: OpsTest, + app_for_reactive: Application, + github_repository: Repository, + test_github_branch: Branch, +): + """ + arrange: A charm integrated with mongodb and an unsupported label is added to the queue. + act: Call reconcile. + assert: No runner is spawned and the message is requeued. + """ + mongodb_uri = await _get_mongodb_uri(ops_test, app_for_reactive) + + run = await dispatch_workflow( + app=app_for_reactive, + branch=test_github_branch, + github_repository=github_repository, + conclusion="success", # this is ignored currently if wait=False kwarg is used + workflow_id_or_name=DISPATCH_TEST_WORKFLOW_FILENAME, + wait=False, + ) + jobs = list(run.jobs()) + assert len(jobs) == 1, "Expected 1 job to be created" + job = jobs[0] + job_url = job.url + job = JobDetails(labels={"not supported label"}, url=job_url) + _add_to_queue( + job.json(), + mongodb_uri, + app_for_reactive.name, + ) + + await reconcile(app_for_reactive, app_for_reactive.model) + + run.update() + assert run.status == "queued" + run.cancel() + + _assert_queue_has_size(mongodb_uri, app_for_reactive.name, 1) + + +async def _get_mongodb_uri(ops_test: OpsTest, app_for_reactive: Application) -> str: + """Get the mongodb uri. + + Args: + ops_test: The ops_test plugin. + app_for_reactive: The juju application containing the unit. + + Returns: + The mongodb uri. + + """ + mongodb_uri = await _get_mongodb_uri_from_integration_data(ops_test, app_for_reactive.units[0]) + if not mongodb_uri: + mongodb_uri = await _get_mongodb_uri_from_secrets(ops_test, app_for_reactive.model) + assert mongodb_uri, "mongodb uri not found in integration data or secret" + return mongodb_uri + + async def _get_mongodb_uri_from_integration_data(ops_test: OpsTest, unit: Unit) -> str | None: """Get the mongodb uri from the relation data. @@ -136,6 +205,18 @@ def _add_to_queue(msg: str, mongodb_uri: str, queue_name: str) -> None: simple_queue.put(msg, retry=True) +def _clear_queue(mongodb_uri: str, queue_name: str) -> None: + """Clear the queue. + + Args: + mongodb_uri: The mongodb uri. + queue_name: The name of the queue to clear. + """ + with Connection(mongodb_uri) as conn: + with conn.SimpleQueue(queue_name) as simple_queue: + simple_queue.clear() + + def _assert_queue_is_empty(mongodb_uri: str, queue_name: str): """Assert that the queue is empty. @@ -143,6 +224,17 @@ def _assert_queue_is_empty(mongodb_uri: str, queue_name: str): mongodb_uri: The mongodb uri. queue_name: The name of the queue to check. """ + _assert_queue_has_size(mongodb_uri, queue_name, 0) + + +def _assert_queue_has_size(mongodb_uri: str, queue_name: str, size: int): + """Assert that the queue is empty. + + Args: + mongodb_uri: The mongodb uri. + queue_name: The name of the queue to check. + size: The expected size of the queue. + """ with Connection(mongodb_uri) as conn: with conn.SimpleQueue(queue_name) as simple_queue: - assert simple_queue.qsize() == 0 + assert simple_queue.qsize() == size