From 876bb68f2518c7af6ddf9f6158cb0155547a8ffe Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 13 Sep 2024 10:27:24 +0200 Subject: [PATCH 01/18] disable most integration tests --- .github/workflows/integration_test.yaml | 59 +++++++++++++------------ 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 8e0bc700a..b5319dc2c 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -8,45 +8,48 @@ concurrency: cancel-in-progress: true jobs: + +# TODO: disabled tests to reduce resource usage and speedup testing, re-enable before merging + # test option values defined at test/conftest.py are passed on via repository secret # INTEGRATION_TEST_ARGS to operator-workflows automatically. - integration-tests: - name: Integration test with juju 3.1 - uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main - secrets: inherit - with: - juju-channel: 3.1/stable - pre-run-script: scripts/pre-integration-test.sh - provider: lxd - test-tox-env: integration-juju3.1 - # These important local LXD test has no OpenStack integration versions. - # test_charm_scheduled_events ensures reconcile events are fired on a schedule. - # test_debug_ssh ensures tmate SSH actions works. - # TODO: Add OpenStack integration versions of these tests. - modules: '["test_charm_scheduled_events", "test_debug_ssh"]' - openstack-interface-tests-private-endpoint: - name: openstack interface test using private-endpoint - uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main - secrets: inherit - with: - juju-channel: 3.2/stable - pre-run-script: scripts/setup-lxd.sh - provider: lxd - test-tox-env: integration-juju3.2 - modules: '["test_runner_manager_openstack"]' - self-hosted-runner: true - self-hosted-runner-label: stg-private-endpoint +# integration-tests: +# name: Integration test with juju 3.1 +# uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main +# secrets: inherit +# with: +# juju-channel: 3.1/stable +# pre-run-script: scripts/pre-integration-test.sh +# provider: lxd +# test-tox-env: integration-juju3.1 +# # These important local LXD test has no OpenStack integration versions. +# # test_charm_scheduled_events ensures reconcile events are fired on a schedule. +# # test_debug_ssh ensures tmate SSH actions works. +# # TODO: Add OpenStack integration versions of these tests. +# modules: '["test_charm_scheduled_events", "test_debug_ssh"]' +# openstack-interface-tests-private-endpoint: +# name: openstack interface test using private-endpoint +# uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main +# secrets: inherit +# with: +# juju-channel: 3.2/stable +# pre-run-script: scripts/setup-lxd.sh +# provider: lxd +# test-tox-env: integration-juju3.2 +# modules: '["test_runner_manager_openstack"]' +# self-hosted-runner: true +# self-hosted-runner-label: stg-private-endpoint 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 +# needs: openstack-interface-tests-private-endpoint secrets: inherit with: juju-channel: 3.2/stable pre-run-script: scripts/setup-lxd.sh provider: lxd test-tox-env: integration-juju3.2 - modules: '["test_charm_metrics_failure", "test_charm_metrics_success", "test_charm_fork_repo", "test_charm_runner", "test_reactive"]' + modules: '["test_reactive"]' extra-arguments: "-m openstack" self-hosted-runner: true self-hosted-runner-label: stg-private-endpoint From a789c0d73f66748c50776b690b7b5c3bb6b52c39 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 13 Sep 2024 10:48:11 +0200 Subject: [PATCH 02/18] remove kombu from requirements.txt --- requirements.txt | 3 +-- tests/integration/requirements.txt | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index 6be16381d..2582464b1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,6 +12,5 @@ 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 -kombu==5.4.1 pymongo==4.8.0 -github_runner_manager @ git+https://github.com/canonical/github-runner-manager.git@771530633cd8b3cb18cb95399c234c82118b77e2 +github_runner_manager @ git+https://github.com/canonical/github-runner-manager.git@feat/reactive-spawning diff --git a/tests/integration/requirements.txt b/tests/integration/requirements.txt index 8b05cbf57..f6a874ceb 100644 --- a/tests/integration/requirements.txt +++ b/tests/integration/requirements.txt @@ -1,2 +1,3 @@ GitPython>3,<4 pygithub +kombu==5.4.* From 550751e890d4cc9d2739a1e49e1c4ee6b40df6a9 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Fri, 13 Sep 2024 10:54:56 +0200 Subject: [PATCH 03/18] use reactive_runner_config --- src-docs/runner_manager_type.md | 49 +-------------------------------- src/charm.py | 28 +++++++++++++++---- src/runner_manager_type.py | 30 -------------------- 3 files changed, 24 insertions(+), 83 deletions(-) diff --git a/src-docs/runner_manager_type.md b/src-docs/runner_manager_type.md index cd7eaf5d2..c44d5dfb0 100644 --- a/src-docs/runner_manager_type.md +++ b/src-docs/runner_manager_type.md @@ -121,54 +121,7 @@ Whether metrics for the runners should be collected. --- - - -## class `OpenstackRunnerManagerConfig` -Configuration of runner manager. - - - -**Attributes:** - - - `charm_state`: The state of the charm. - - `path`: GitHub repository path in the format '/', or the GitHub organization name. - - `labels`: Additional labels for the runners. - - `token`: GitHub personal access token to register runner to the repository or organization. - - `flavor`: OpenStack flavor for defining the runner resources. - - `image`: Openstack image id to boot the runner with. - - `network`: OpenStack network for runner network access. - - `dockerhub_mirror`: URL of dockerhub mirror to use. - - `reactive_config`: The configuration to spawn runners reactively. - - - -### method `__init__` - -```python -__init__( - charm_state: CharmState, - path: GitHubOrg | GitHubRepo, - labels: Iterable[str], - token: str, - flavor: str, - image: str, - network: str, - dockerhub_mirror: str | None, - reactive_config: ReactiveConfig | None = None -) → None -``` - - - - - - - - - ---- - - + ## class `RunnerInfo` Information from GitHub of a runner. diff --git a/src/charm.py b/src/charm.py index 9dae00059..46c1960df 100755 --- a/src/charm.py +++ b/src/charm.py @@ -20,8 +20,11 @@ from github_runner_manager.openstack_cloud.openstack_runner_manager import ( OpenStackCloudConfig, OpenStackRunnerManager, + OpenStackRunnerManagerConfig, OpenStackServerConfig, ) +from github_runner_manager.reactive.types_ import QueueConfig as ReactiveQueueConfig +from github_runner_manager.reactive.types_ import RunnerConfig as ReactiveRunnerConfig from github_runner_manager.types_.github import GitHubPath, GitHubRunnerStatus, parse_github_path from utilities import bytes_with_unit_to_kib, execute_command, remove_residual_venv_dirs, retry @@ -1276,25 +1279,40 @@ def _get_runner_scaler( ssh_debug_connections=state.ssh_debug_connections, repo_policy_compliance=state.charm_config.repo_policy_compliance, ) - # The prefix is set to f"{application_name}-{unit number}" - openstack_runner_manager = OpenStackRunnerManager( - manager_name=self.app.name, + + openstack_runner_manager_config = OpenStackRunnerManagerConfig( + manager_name=self.app.name, # TODO: rename to name + # The prefix is set to f"{application_name}-{unit number}" prefix=self.unit.name.replace("/", "-"), cloud_config=cloud_config, server_config=server_config, runner_config=runner_config, service_config=service_config, ) + openstack_runner_manager = OpenStackRunnerManager( + config=openstack_runner_manager_config, + ) runner_manager_config = RunnerManagerConfig( + name=self.app.name, token=token, path=path, ) runner_manager = RunnerManager( - manager_name=self.app.name, cloud_runner_manager=openstack_runner_manager, config=runner_manager_config, ) - return RunnerScaler(runner_manager=runner_manager, reactive_config=state.reactive_config) + reactive_runner_config = None + if reactive_config := state.reactive_config: + reactive_runner_config = ReactiveRunnerConfig( + queue=ReactiveQueueConfig( + mongodb_uri=reactive_config.mq_uri, queue_name=self.app.name + ), # TODO think if queue_name should be really decided by charm + runner_manager=runner_manager_config, + cloud_runner_manager=openstack_runner_manager_config, + ) + return RunnerScaler( + runner_manager=runner_manager, reactive_runner_config=reactive_runner_config + ) if __name__ == "__main__": diff --git a/src/runner_manager_type.py b/src/runner_manager_type.py index deb30540b..17e32fa46 100644 --- a/src/runner_manager_type.py +++ b/src/runner_manager_type.py @@ -93,36 +93,6 @@ def are_metrics_enabled(self) -> bool: return self.charm_state.is_metrics_logging_available -# This class is subject to refactor. -@dataclass -class OpenstackRunnerManagerConfig: # pylint: disable=too-many-instance-attributes - """Configuration of runner manager. - - Attributes: - charm_state: The state of the charm. - path: GitHub repository path in the format '/', or the - GitHub organization name. - labels: Additional labels for the runners. - token: GitHub personal access token to register runner to the - repository or organization. - flavor: OpenStack flavor for defining the runner resources. - image: Openstack image id to boot the runner with. - network: OpenStack network for runner network access. - dockerhub_mirror: URL of dockerhub mirror to use. - reactive_config: The configuration to spawn runners reactively. - """ - - charm_state: CharmState - path: GitHubPath - labels: Iterable[str] - token: str - flavor: str - image: str - network: str - dockerhub_mirror: str | None - reactive_config: ReactiveConfig | None = None - - @dataclass class RunnerInfo: """Information from GitHub of a runner. From 2fee798ecca1285c579db1fb625650c96f97ad45 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Mon, 16 Sep 2024 14:42:51 +0200 Subject: [PATCH 04/18] pass over github token --- src/charm.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/charm.py b/src/charm.py index 46c1960df..06927b521 100755 --- a/src/charm.py +++ b/src/charm.py @@ -7,6 +7,7 @@ # pylint: disable=too-many-lines """Charm for creating and managing GitHub self-hosted runner instances.""" +from ghapi.actions import github_token from github_runner_manager.manager.cloud_runner_manager import ( GitHubRunnerConfig, SupportServiceConfig, @@ -1309,6 +1310,7 @@ def _get_runner_scaler( ), # TODO think if queue_name should be really decided by charm runner_manager=runner_manager_config, cloud_runner_manager=openstack_runner_manager_config, + github_token=token ) return RunnerScaler( runner_manager=runner_manager, reactive_runner_config=reactive_runner_config From d9d7797a8f8cf42c69ba7f9334c091f095023a55 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Tue, 17 Sep 2024 14:03:37 +0200 Subject: [PATCH 05/18] add tmate --- .github/workflows/integration_test.yaml | 2 ++ src-docs/charm.md | 10 +++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index b5319dc2c..1ebda3c70 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -53,3 +53,5 @@ jobs: extra-arguments: "-m openstack" self-hosted-runner: true self-hosted-runner-label: stg-private-endpoint + tmate-debug: true + tmate-timeout: 60 diff --git a/src-docs/charm.md b/src-docs/charm.md index 5b1540fee..be44e0daa 100644 --- a/src-docs/charm.md +++ b/src-docs/charm.md @@ -21,7 +21,7 @@ Charm for creating and managing GitHub self-hosted runner instances. --- - + ## function `catch_charm_errors` @@ -47,7 +47,7 @@ Catch common errors in charm. --- - + ## function `catch_action_errors` @@ -73,7 +73,7 @@ Catch common errors in actions. --- - + ## class `ReconcileRunnersEvent` Event representing a periodic check to ensure runners are ok. @@ -84,7 +84,7 @@ Event representing a periodic check to ensure runners are ok. --- - + ## class `GithubRunnerCharm` Charm for managing GitHub self-hosted runners. @@ -101,7 +101,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__` From 73be9a920ea29c58af84432b34b29b4b0d5c1db7 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Tue, 17 Sep 2024 14:26:23 +0200 Subject: [PATCH 06/18] adapt reactive integration test --- tests/integration/helpers/common.py | 23 ++++++++---- tests/integration/test_reactive.py | 54 ++++++++++++----------------- 2 files changed, 39 insertions(+), 38 deletions(-) diff --git a/tests/integration/helpers/common.py b/tests/integration/helpers/common.py index 495c952b3..fa34c47a7 100644 --- a/tests/integration/helpers/common.py +++ b/tests/integration/helpers/common.py @@ -391,7 +391,7 @@ async def dispatch_workflow( app: The charm to dispatch the workflow for. branch: The branch to dispatch the workflow on. github_repository: The github repository to dispatch the workflow on. - conclusion: The expected workflow run conclusion. + conclusion: The expected workflow run conclusion. This argument is ignored if wait is False. workflow_id_or_name: The workflow filename in .github/workflows in main branch to run or its id. dispatch_input: Workflow input values. @@ -420,14 +420,25 @@ async def dispatch_workflow( if not wait: return run - await wait_for(partial(_is_workflow_run_complete, run=run), timeout=60 * 30, check_interval=60) - # The run object is updated by _is_workflow_run_complete function above. - assert ( - run.conclusion == conclusion - ), f"Unexpected run conclusion, expected: {conclusion}, got: {run.conclusion}" + await wait_for_completion(run=run, conclusion=conclusion) return run +async def wait_for_completion(run: WorkflowRun, conclusion: str) -> None: + """Wait for the workflow run to complete. + + Args: + run: The workflow run to wait for. + conclusion: The expected conclusion of the run. + """ + await wait_for( + partial(_is_workflow_run_complete, run=run), + timeout=60 * 30, + check_interval=60, + ) + # The run object is updated by _is_workflow_run_complete function above. + assert run.conclusion == conclusion, f"Unexpected run conclusion, expected: {conclusion}, got: {run.conclusion}" + P = ParamSpec("P") R = TypeVar("R") diff --git a/tests/integration/test_reactive.py b/tests/integration/test_reactive.py index 06dc6e48c..3d4979d76 100644 --- a/tests/integration/test_reactive.py +++ b/tests/integration/test_reactive.py @@ -6,21 +6,21 @@ import secrets import pytest +from github import Branch, Repository from github_runner_manager.reactive.consumer import JobDetails -from github_runner_manager.reactive.runner_manager import REACTIVE_RUNNER_LOG_DIR from juju.application import Application from juju.model import Model from juju.unit import Unit from kombu import Connection from pytest_operator.plugin import OpsTest -from tests.integration.helpers.common import get_file_content, reconcile, run_in_unit - -FAKE_URL = "http://example.com" +from tests.integration.helpers.common import get_file_content, reconcile, run_in_unit, \ + dispatch_workflow, DISPATCH_TEST_WORKFLOW_FILENAME, wait_for_completion @pytest.mark.openstack -async def test_reactive_mode_consumes_jobs(ops_test: OpsTest, app_for_reactive: Application): +async def test_reactive_mode_consumes_jobs(ops_test: OpsTest, app_for_reactive: Application, github_repository: Repository, + test_github_branch: Branch): """ arrange: A charm integrated with mongodb and a message is added to the queue. act: Call reconcile. @@ -32,8 +32,20 @@ async def test_reactive_mode_consumes_jobs(ops_test: OpsTest, app_for_reactive: mongodb_uri = await _get_mongodb_uri_from_secrets(ops_test, unit.model) assert mongodb_uri, "mongodb uri not found in integration data or secret" + run = await dispatch_workflow( + app=app_for_reactive, + branch=test_github_branch, + github_repository=github_repository, + conclusion="success", + 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=[secrets.token_hex(16) for _ in range(random.randint(1, 4))], run_url=FAKE_URL + labels=[secrets.token_hex(16) for _ in range(random.randint(1, 4))], job_url=job_url ) _add_to_queue( json.dumps(job.dict() | {"ignored_noise": "foobar"}), @@ -41,9 +53,11 @@ async def test_reactive_mode_consumes_jobs(ops_test: OpsTest, app_for_reactive: app_for_reactive.name, ) - await reconcile(app_for_reactive, app_for_reactive.model) + await wait_for_completion(run, conclusion="success") _assert_queue_is_empty(mongodb_uri, app_for_reactive.name) - await _assert_job_details_in_reactive_log(unit, jobs=[job]) + + # Call reconcile to enable cleanup of the runner + await reconcile(app_for_reactive, app_for_reactive.model) async def _get_mongodb_uri_from_integration_data(ops_test: OpsTest, unit: Unit) -> str | None: @@ -122,27 +136,3 @@ def _assert_queue_is_empty(mongodb_uri: str, queue_name: str): with Connection(mongodb_uri) as conn: with conn.SimpleQueue(queue_name) as simple_queue: assert simple_queue.qsize() == 0 - - -async def _assert_job_details_in_reactive_log(unit: Unit, jobs: list[JobDetails]): - """Assert that the job details are in the reactive log. - - Args: - unit: The juju unit. - jobs: The list of job details to check. - """ - retcode, stdout, stderr = await run_in_unit( - unit=unit, - command=f"ls {REACTIVE_RUNNER_LOG_DIR}", - ) - assert retcode == 0, f"Failed to list the reactive log dir: {stderr}" - assert stdout, "No log files found in the reactive log dir." - log_files = [log_file for log_file in stdout.split()] - - reactive_logs = "" - for log_file in log_files: - reactive_logs += await get_file_content(unit, REACTIVE_RUNNER_LOG_DIR / log_file) - - for job in jobs: - assert job.run_url in reactive_logs - assert str(job.labels) in reactive_logs From 6b389e624782c60dfe3f6db4527ec4f7ba366d89 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Tue, 17 Sep 2024 15:18:55 +0200 Subject: [PATCH 07/18] call reconcile before waiting for job --- tests/integration/test_reactive.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/test_reactive.py b/tests/integration/test_reactive.py index 3d4979d76..6172dd6cd 100644 --- a/tests/integration/test_reactive.py +++ b/tests/integration/test_reactive.py @@ -53,6 +53,8 @@ async def test_reactive_mode_consumes_jobs(ops_test: OpsTest, app_for_reactive: app_for_reactive.name, ) + await reconcile(app_for_reactive, app_for_reactive.model) + await wait_for_completion(run, conclusion="success") _assert_queue_is_empty(mongodb_uri, app_for_reactive.name) From fbe771ca3c9bca6b356272ab0c0966086c7c1aa8 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Tue, 17 Sep 2024 15:22:50 +0200 Subject: [PATCH 08/18] temporarily remove e2e test - REVERT ME --- .github/workflows/e2e_test.yaml | 33 -------- .github/workflows/e2e_test_run.yaml | 113 ---------------------------- 2 files changed, 146 deletions(-) delete mode 100644 .github/workflows/e2e_test.yaml delete mode 100644 .github/workflows/e2e_test_run.yaml diff --git a/.github/workflows/e2e_test.yaml b/.github/workflows/e2e_test.yaml deleted file mode 100644 index 7d0383c12..000000000 --- a/.github/workflows/e2e_test.yaml +++ /dev/null @@ -1,33 +0,0 @@ -name: End-to-End tests - -on: - pull_request: - - -jobs: - # test option values defined at test/conftest.py are passed on via repository secret - # INTEGRATION_TEST_ARGS to operator-workflows automatically. - openstack-integration-end-to-end-test: - name: end-to-end test using private-endpoint - uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main - secrets: inherit - with: - juju-channel: 3.2/stable - pre-run-script: scripts/setup-lxd.sh - provider: lxd - test-tox-env: integration-juju3.2 - modules: '["test_e2e"]' - extra-arguments: "-m openstack" - self-hosted-runner: true - self-hosted-runner-label: stg-private-endpoint - - required_status_checks: - name: Required E2E Test Status Checks - runs-on: ubuntu-latest - needs: - - openstack-integration-end-to-end-test - if: always() && !cancelled() - timeout-minutes: 5 - steps: - - run: | - [ '${{ needs.openstack-integration-end-to-end-test.result }}' = 'success' ] || (echo openstack-integration-end-to-end-test failed && false) diff --git a/.github/workflows/e2e_test_run.yaml b/.github/workflows/e2e_test_run.yaml deleted file mode 100644 index 4eeeccbe1..000000000 --- a/.github/workflows/e2e_test_run.yaml +++ /dev/null @@ -1,113 +0,0 @@ -# Copyright 2024 Canonical Ltd. -# See LICENSE file for licensing details. - -name: Run End-to-End test - -on: - # The inputs are the same but they cannot be shared between triggers. - # See https://github.com/orgs/community/discussions/39357 - workflow_call: - inputs: - runner-tag: - description: The e2e test runner tag to run the workflow on. - type: string - required: true - runner-virt-type: - description: The e2e test runner virtualization type. E.g. lxd, or openstack. - # workflow_call does not support choice type. - type: string - required: true - workflow_dispatch: - inputs: - runner-tag: - description: The e2e test runner tag to run the workflow on. - type: string - required: true - runner-virt-type: - description: The e2e test runner virtualization type. - type: choice - required: true - options: - - lxd - - openstack - -jobs: - e2e-test: - name: End-to-End Test Run - runs-on: [self-hosted, linux, "${{ inputs.runner-tag }}"] - steps: - - name: Hostname is set to "github-runner" - run: sudo hostnamectl hostname | grep github-runner - # Snapd can have some issues in privileged LXD containers without setting - # security.nesting=True and this. - - name: Fix snap issue in privileged LXD containers - run: ln -s /bin/true /usr/local/bin/udevadm - # Below is a series of simple tests to assess the functionality of the newly spawned runner. - - name: Echo hello world - run: echo "hello world" - - name: File permission for /usr/local/bin - run: ls -ld /usr/local/bin | grep drwxrwxrwx - - name: Test file permission for /usr/local/bin - run: touch /usr/local/bin/test_file - # "Install microk8s" step will test if the proxies settings are correct. - - name: Proxy set in /etc/environment - run: cat /etc/environment - # "Update apt in python docker container" step will test docker default proxy settings due to - # pulling the python image. - - name: Proxy set in docker daemon - run: | - [[ -z "${http_proxy}" && -z "${HTTP_PROXY}" ]] \ - || sudo cat /etc/systemd/system/docker.service.d/http-proxy.conf | grep HTTP_PROXY - # "Update apt in python docker container" step will test docker client default proxy settings. - - name: Proxy set in docker client - run: | - [[ -z "${http_proxy}" && -z "${HTTP_PROXY}" ]] \ - || cat /home/ubuntu/.docker/config.json | grep httpProxy - - name: Install microk8s - run: sudo snap install microk8s --classic - - name: Wait for microk8s - timeout-minutes: 10 - run: microk8s status --wait-ready - - name: Deploy nginx for testing - run: microk8s kubectl create deployment nginx --image=nginx - - name: Wait for nginx to be ready - run: microk8s kubectl rollout status deployment/nginx --timeout=30m - - name: Update apt in python docker container - run: docker run python:3.10-slim apt-get update - - name: Docker version - run: docker version - - name: Check python alias for python3 - run: python --version - - name: pip version - run: python3 -m pip --version - - name: npm version - run: npm --version - - name: shellcheck version - run: shellcheck --version - - name: jq version - run: jq --version - - name: yq version - run: yq --version - - name: apt update - run: sudo apt-get update -y - # Use pipx for 24.04 noble, check-jsonschema breaks OS system packages. - - name: install pipx - run: sudo apt-get install -y pipx - - name: install check-jsonschema - run: python3 -m pip install check-jsonschema || pipx install check-jsonschema - - name: unzip version - run: unzip -v - - name: gh version - run: gh --version - # `check-jsonschema` is installed using pip. The directory `~/.local/bin` needs to be added to PATH. - # ~/.local/bin is added to path runner env through in scripts/env.j2 - - name: test check-jsonschema - run: check-jsonschema --version - - name: Test Firewall - if: "${{ github.event.inputs.runner-virt-type == 'lxd' }}" - run: | - HOST_IP=$(ip route | grep default | cut -f 3 -d" ") - [ $((ping $HOST_IP -c 5 || :) | grep "Destination Port Unreachable" | wc -l) -eq 5 ] - - name: Test sctp support - if: "${{ github.event.inputs.runner-virt-type == 'lxd' }}" - run: sudo apt-get install lksctp-tools -yq && checksctp From 961155a2e72b4b731229cc6a3b0eb1e229587c32 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Wed, 18 Sep 2024 11:08:07 +0200 Subject: [PATCH 09/18] remove pymongo --- requirements.txt | 1 - tests/integration/requirements.txt | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 43d939acd..df9df30d3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,5 +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 -pymongo==4.8.0 github_runner_manager @ git+https://github.com/canonical/github-runner-manager.git@feat/reactive-spawning diff --git a/tests/integration/requirements.txt b/tests/integration/requirements.txt index f6a874ceb..c682c9050 100644 --- a/tests/integration/requirements.txt +++ b/tests/integration/requirements.txt @@ -1,3 +1,4 @@ GitPython>3,<4 pygithub kombu==5.4.* +pymongo==4.8.* From da158050aca2f620301ed609bf22d1eed1630f57 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Wed, 18 Sep 2024 11:56:18 +0200 Subject: [PATCH 10/18] remove reactive code for lxd --- src-docs/charm.md | 10 ++++---- src-docs/runner_manager.md | 22 ++++++++-------- src-docs/runner_manager_type.md | 8 +++--- src/charm_state.py | 8 ++++++ src/runner_manager.py | 19 -------------- tests/unit/test_charm_state.py | 36 +++++++++++++++++++++++++++ tests/unit/test_lxd_runner_manager.py | 31 ++--------------------- 7 files changed, 66 insertions(+), 68 deletions(-) diff --git a/src-docs/charm.md b/src-docs/charm.md index be44e0daa..5458c6a13 100644 --- a/src-docs/charm.md +++ b/src-docs/charm.md @@ -21,7 +21,7 @@ Charm for creating and managing GitHub self-hosted runner instances. --- - + ## function `catch_charm_errors` @@ -47,7 +47,7 @@ Catch common errors in charm. --- - + ## function `catch_action_errors` @@ -73,7 +73,7 @@ Catch common errors in actions. --- - + ## class `ReconcileRunnersEvent` Event representing a periodic check to ensure runners are ok. @@ -84,7 +84,7 @@ Event representing a periodic check to ensure runners are ok. --- - + ## class `GithubRunnerCharm` Charm for managing GitHub self-hosted runners. @@ -101,7 +101,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-docs/runner_manager.md b/src-docs/runner_manager.md index 883745753..ebf6806b8 100644 --- a/src-docs/runner_manager.md +++ b/src-docs/runner_manager.md @@ -13,7 +13,7 @@ Runner Manager manages the runners on LXD and GitHub. --- - + ## class `LXDRunnerManager` Manage a group of runners according to configuration. @@ -25,7 +25,7 @@ Manage a group of runners according to configuration. - `runner_bin_path`: The github runner app scripts path. - `cron_path`: The path to runner build image cron job. - + ### method `__init__` @@ -52,7 +52,7 @@ Construct RunnerManager object for creating and managing runners. --- - + ### method `build_runner_image` @@ -72,7 +72,7 @@ Build container image in test mode, else virtual machine image. --- - + ### method `check_runner_bin` @@ -89,7 +89,7 @@ Check if runner binary exists. --- - + ### method `flush` @@ -118,7 +118,7 @@ Remove existing runners. --- - + ### method `get_github_info` @@ -135,7 +135,7 @@ Get information on the runners from GitHub. --- - + ### method `get_latest_runner_bin_url` @@ -166,7 +166,7 @@ The runner binary URL changes when a new version is available. --- - + ### method `has_runner_image` @@ -183,7 +183,7 @@ Check if the runner image exists. --- - + ### method `reconcile` @@ -207,7 +207,7 @@ Bring runners in line with target. --- - + ### method `schedule_build_runner_image` @@ -219,7 +219,7 @@ Install cron job for building runner image. --- - + ### method `update_runner_bin` diff --git a/src-docs/runner_manager_type.md b/src-docs/runner_manager_type.md index c44d5dfb0..7cf5c5fe9 100644 --- a/src-docs/runner_manager_type.md +++ b/src-docs/runner_manager_type.md @@ -9,7 +9,7 @@ Types used by RunnerManager class. --- - + ## class `LXDFlushMode` Strategy for flushing runners. @@ -32,7 +32,7 @@ During pre-job (repo-check), the runners are marked as idle and if the pre-job f --- - + ## class `RunnerManagerClients` Clients for accessing various services. @@ -69,7 +69,7 @@ __init__( --- - + ## class `LXDRunnerManagerConfig` Configuration of runner manager. @@ -121,7 +121,7 @@ Whether metrics for the runners should be collected. --- - + ## class `RunnerInfo` Information from GitHub of a runner. diff --git a/src/charm_state.py b/src/charm_state.py index 6ae46d386..c473f26b9 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -1196,6 +1196,14 @@ def from_charm( # noqa: C901 reactive_config = ReactiveConfig.from_database(database) + if instance_type == InstanceType.LOCAL_LXD and reactive_config: + logger.error( + "Reactive mode not supported for local LXD instances. Please remove the mongodb integration." + ) + raise CharmConfigInvalidError( + "Reactive mode not supported for local LXD instances. Please remove the mongodb integration." + ) + state = cls( arch=arch, is_metrics_logging_available=bool(charm.model.relations[COS_AGENT_INTEGRATION_NAME]), diff --git a/src/runner_manager.py b/src/runner_manager.py index 66a7e03d3..ab4b232c5 100644 --- a/src/runner_manager.py +++ b/src/runner_manager.py @@ -13,7 +13,6 @@ from pathlib import Path from typing import Iterator, Optional, Type -import github_runner_manager.reactive.runner_manager as reactive_runner_manager import jinja2 import requests import requests.adapters @@ -534,9 +533,6 @@ def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int: Returns: Difference between intended runners and actual runners. """ - if self.config.reactive_config: - logger.info("Reactive configuration detected, going into experimental reactive mode.") - return self._reconcile_reactive(quantity) start_ts = time.time() runners = self._get_runners() @@ -581,21 +577,6 @@ def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int: ) return delta - def _reconcile_reactive(self, quantity: int) -> int: - """Reconcile runners reactively. - - Args: - quantity: Number of intended runners. - - Returns: - The difference between intended runners and actual runners. In reactive mode - this number is never negative as additional processes should terminate after a timeout. - """ - logger.info("Reactive mode is experimental and not yet fully implemented.") - return reactive_runner_manager.reconcile( - quantity=quantity, mq_uri=self.config.reactive_config.mq_uri, queue_name=self.app_name - ) - def _runners_in_pre_job(self) -> bool: """Check there exist runners in the pre-job script stage. diff --git a/tests/unit/test_charm_state.py b/tests/unit/test_charm_state.py index b7df8a5dc..52b19781c 100644 --- a/tests/unit/test_charm_state.py +++ b/tests/unit/test_charm_state.py @@ -48,6 +48,7 @@ OpenstackImage, OpenstackRunnerConfig, ProxyConfig, + ReactiveConfig, RunnerStorage, SSHDebugConnection, UnsupportedArchitectureError, @@ -1288,3 +1289,38 @@ def test_charm_state__log_prev_state_redacts_sensitive_information( assert mock_charm_state_data["charm_config"]["token"] not in caplog.text assert charm_state.SENSITIVE_PLACEHOLDER in caplog.text + + +def test_charm_state_from_charm_reactive_with_lxd_raises_error(monkeypatch: pytest.MonkeyPatch): + """ + arrange: Mock CharmBase and necessary methods to enable reactive config and lxd storage. + act: Call CharmState.from_charm. + assert: Ensure an error is raised + """ + mock_charm = MockGithubRunnerCharmFactory() + mock_database = MagicMock(spec=DatabaseRequires) + + monkeypatch.setattr( + ReactiveConfig, + "from_database", + MagicMock(return_value=ReactiveConfig(mq_uri="mongodb://localhost:27017")), + ) + charm_config_mock = MagicMock() + charm_config_mock.openstack_clouds_yaml = None + monkeypatch.setattr(CharmConfig, "from_charm", MagicMock(return_value=charm_config_mock)) + + # mock all other required methods + monkeypatch.setattr(ProxyConfig, "from_charm", MagicMock()) + monkeypatch.setattr(OpenstackRunnerConfig, "from_charm", MagicMock()) + monkeypatch.setattr(LocalLxdRunnerConfig, "from_charm", MagicMock()) + monkeypatch.setattr(CharmState, "_check_immutable_config_change", MagicMock()) + monkeypatch.setattr(charm_state, "_get_supported_arch", MagicMock()) + monkeypatch.setattr(SSHDebugConnection, "from_charm", MagicMock()) + monkeypatch.setattr(json, "loads", MagicMock()) + monkeypatch.setattr(json, "dumps", MagicMock()) + monkeypatch.setattr(charm_state, "CHARM_STATE_PATH", MagicMock()) + + with pytest.raises(CharmConfigInvalidError) as exc: + CharmState.from_charm(mock_charm, mock_database) + + assert "Reactive mode not supported for local LXD instances" in str(exc.value) diff --git a/tests/unit/test_lxd_runner_manager.py b/tests/unit/test_lxd_runner_manager.py index 215cbe7e0..2c5aca46c 100644 --- a/tests/unit/test_lxd_runner_manager.py +++ b/tests/unit/test_lxd_runner_manager.py @@ -18,17 +18,10 @@ from github_runner_manager.metrics.runner import RUNNER_INSTALLED_TS_FILE_NAME from github_runner_manager.metrics.storage import MetricsStorage from github_runner_manager.types_.github import GitHubOrg, GitHubRepo, RunnerApplication -from pytest import LogCaptureFixture, MonkeyPatch +from pytest import MonkeyPatch import shared_fs -from charm_state import ( - Arch, - CharmConfig, - CharmState, - ProxyConfig, - ReactiveConfig, - VirtualMachineResources, -) +from charm_state import Arch, CharmConfig, CharmState, ProxyConfig, VirtualMachineResources from errors import IssueMetricEventError, RunnerBinaryError from runner import Runner, RunnerStatus from runner_manager import BUILD_IMAGE_SCRIPT_FILENAME, LXDRunnerManager, LXDRunnerManagerConfig @@ -524,26 +517,6 @@ def test_reconcile_places_no_timestamp_in_newly_created_runner_if_metrics_disabl assert not (fs.path / RUNNER_INSTALLED_TS_FILE_NAME).exists() -def test_reconcile_reactive_mode( - runner_manager: LXDRunnerManager, - reactive_reconcile_mock: MagicMock, - caplog: LogCaptureFixture, -): - """ - arrange: Enable reactive mode and mock the job class to return a job. - act: Call reconcile with a random quantity n. - assert: The mocked job is picked up n times and the expected log message is present. - """ - count = random.randint(0, 5) - runner_manager.config.reactive_config = ReactiveConfig(mq_uri=FAKE_MONGODB_URI) - actual_count = runner_manager.reconcile(count, VirtualMachineResources(2, "7GiB", "10Gib")) - - assert actual_count == count - reactive_reconcile_mock.assert_called_with( - quantity=count, mq_uri=FAKE_MONGODB_URI, queue_name=runner_manager.app_name - ) - - def test_schedule_build_runner_image( runner_manager: LXDRunnerManager, tmp_path: Path, From 20e0fa70bd9b411df4d9e4638955fe8065e7da02 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Wed, 18 Sep 2024 12:04:25 +0200 Subject: [PATCH 11/18] lint --- src/charm.py | 3 +-- src/charm_state.py | 13 +++++++------ src/runner_manager_type.py | 1 - tests/integration/helpers/common.py | 8 ++++++-- tests/integration/test_reactive.py | 20 ++++++++++++++------ tests/unit/test_runner_scaler.py | 4 ++-- 6 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/charm.py b/src/charm.py index 06927b521..e11dd4f74 100755 --- a/src/charm.py +++ b/src/charm.py @@ -7,7 +7,6 @@ # pylint: disable=too-many-lines """Charm for creating and managing GitHub self-hosted runner instances.""" -from ghapi.actions import github_token from github_runner_manager.manager.cloud_runner_manager import ( GitHubRunnerConfig, SupportServiceConfig, @@ -1310,7 +1309,7 @@ def _get_runner_scaler( ), # TODO think if queue_name should be really decided by charm runner_manager=runner_manager_config, cloud_runner_manager=openstack_runner_manager_config, - github_token=token + github_token=token, ) return RunnerScaler( runner_manager=runner_manager, reactive_runner_config=reactive_runner_config diff --git a/src/charm_state.py b/src/charm_state.py index c473f26b9..afe174388 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -38,6 +38,11 @@ from firewall import FirewallEntry from utilities import get_env_var +REACTIVE_MODE_NOT_SUPPORTED_WITH_LXD_ERR_MSG = ( + "Reactive mode not supported for local LXD instances. " + "Please remove the mongodb integration." +) + logger = logging.getLogger(__name__) ARCHITECTURES_ARM64 = {"aarch64", "arm64"} @@ -1197,12 +1202,8 @@ def from_charm( # noqa: C901 reactive_config = ReactiveConfig.from_database(database) if instance_type == InstanceType.LOCAL_LXD and reactive_config: - logger.error( - "Reactive mode not supported for local LXD instances. Please remove the mongodb integration." - ) - raise CharmConfigInvalidError( - "Reactive mode not supported for local LXD instances. Please remove the mongodb integration." - ) + logger.error(REACTIVE_MODE_NOT_SUPPORTED_WITH_LXD_ERR_MSG) + raise CharmConfigInvalidError(REACTIVE_MODE_NOT_SUPPORTED_WITH_LXD_ERR_MSG) state = cls( arch=arch, diff --git a/src/runner_manager_type.py b/src/runner_manager_type.py index 17e32fa46..4c5ca9820 100644 --- a/src/runner_manager_type.py +++ b/src/runner_manager_type.py @@ -6,7 +6,6 @@ from dataclasses import dataclass from enum import Enum, auto from pathlib import Path -from typing import Iterable import jinja2 from github_runner_manager.repo_policy_compliance_client import RepoPolicyComplianceClient diff --git a/tests/integration/helpers/common.py b/tests/integration/helpers/common.py index fa34c47a7..95c8b051b 100644 --- a/tests/integration/helpers/common.py +++ b/tests/integration/helpers/common.py @@ -391,7 +391,8 @@ async def dispatch_workflow( app: The charm to dispatch the workflow for. branch: The branch to dispatch the workflow on. github_repository: The github repository to dispatch the workflow on. - conclusion: The expected workflow run conclusion. This argument is ignored if wait is False. + conclusion: The expected workflow run conclusion. + This argument is ignored if wait is False. workflow_id_or_name: The workflow filename in .github/workflows in main branch to run or its id. dispatch_input: Workflow input values. @@ -424,6 +425,7 @@ async def dispatch_workflow( return run + async def wait_for_completion(run: WorkflowRun, conclusion: str) -> None: """Wait for the workflow run to complete. @@ -437,7 +439,9 @@ async def wait_for_completion(run: WorkflowRun, conclusion: str) -> None: check_interval=60, ) # The run object is updated by _is_workflow_run_complete function above. - assert run.conclusion == conclusion, f"Unexpected run conclusion, expected: {conclusion}, got: {run.conclusion}" + assert ( + run.conclusion == conclusion + ), f"Unexpected run conclusion, expected: {conclusion}, got: {run.conclusion}" P = ParamSpec("P") diff --git a/tests/integration/test_reactive.py b/tests/integration/test_reactive.py index 6172dd6cd..461ec687c 100644 --- a/tests/integration/test_reactive.py +++ b/tests/integration/test_reactive.py @@ -14,13 +14,21 @@ from kombu import Connection from pytest_operator.plugin import OpsTest -from tests.integration.helpers.common import get_file_content, reconcile, run_in_unit, \ - dispatch_workflow, DISPATCH_TEST_WORKFLOW_FILENAME, wait_for_completion +from tests.integration.helpers.common import ( + DISPATCH_TEST_WORKFLOW_FILENAME, + dispatch_workflow, + reconcile, + wait_for_completion, +) @pytest.mark.openstack -async def test_reactive_mode_consumes_jobs(ops_test: OpsTest, app_for_reactive: Application, github_repository: Repository, - test_github_branch: Branch): +async def test_reactive_mode_consumes_jobs( + ops_test: OpsTest, + app_for_reactive: Application, + github_repository: Repository, + test_github_branch: Branch, +): """ arrange: A charm integrated with mongodb and a message is added to the queue. act: Call reconcile. @@ -32,13 +40,13 @@ async def test_reactive_mode_consumes_jobs(ops_test: OpsTest, app_for_reactive: mongodb_uri = await _get_mongodb_uri_from_secrets(ops_test, unit.model) assert mongodb_uri, "mongodb uri not found in integration data or secret" - run = await dispatch_workflow( + run = await dispatch_workflow( app=app_for_reactive, branch=test_github_branch, github_repository=github_repository, conclusion="success", workflow_id_or_name=DISPATCH_TEST_WORKFLOW_FILENAME, - wait=False + wait=False, ) jobs = list(run.jobs()) assert len(jobs) == 1, "Expected 1 job to be created" diff --git a/tests/unit/test_runner_scaler.py b/tests/unit/test_runner_scaler.py index f3199fd99..5cfae4c4f 100644 --- a/tests/unit/test_runner_scaler.py +++ b/tests/unit/test_runner_scaler.py @@ -73,8 +73,8 @@ def runner_manager_fixture( "github_runner_manager.manager.runner_manager.runner_metrics.issue_events", MagicMock() ) - config = RunnerManagerConfig("mock_token", github_path) - runner_manager = RunnerManager("mock_runners", mock_cloud, config) + config = RunnerManagerConfig("mock_runners", "mock_token", github_path) + runner_manager = RunnerManager(mock_cloud, config) runner_manager._github = mock_github return runner_manager From 9ad0ebd6fb9e478f558e02ccfc0c9ae9435e4c8e Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Wed, 18 Sep 2024 12:33:58 +0200 Subject: [PATCH 12/18] use name instead of manager_name & lint --- src/charm.py | 70 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 42 insertions(+), 28 deletions(-) diff --git a/src/charm.py b/src/charm.py index e11dd4f74..410b8951c 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1249,6 +1249,46 @@ def _get_runner_scaler( if path is None: path = state.charm_config.path + openstack_runner_manager_config = self._create_openstack_runner_manager_config(path, state) + openstack_runner_manager = OpenStackRunnerManager( + config=openstack_runner_manager_config, + ) + runner_manager_config = RunnerManagerConfig( + name=self.app.name, + token=token, + path=path, + ) + runner_manager = RunnerManager( + cloud_runner_manager=openstack_runner_manager, + config=runner_manager_config, + ) + reactive_runner_config = None + if reactive_config := state.reactive_config: + reactive_runner_config = ReactiveRunnerConfig( + queue=ReactiveQueueConfig( + mongodb_uri=reactive_config.mq_uri, queue_name=self.app.name + ), + runner_manager=runner_manager_config, + cloud_runner_manager=openstack_runner_manager_config, + github_token=token, + ) + return RunnerScaler( + runner_manager=runner_manager, reactive_runner_config=reactive_runner_config + ) + + def _create_openstack_runner_manager_config( + self, path: GitHubPath, state: CharmState + ) -> OpenStackRunnerManagerConfig: + """Create OpenStackRunnerManagerConfig instance. + + Args: + path: GitHub repository path in the format '/', or the GitHub organization + name. + state: Charm state. + + Returns: + An instance of OpenStackRunnerManagerConfig. + """ clouds = list(state.charm_config.openstack_clouds_yaml["clouds"].keys()) if len(clouds) > 1: logger.warning( @@ -1269,7 +1309,6 @@ def _get_runner_scaler( ) if image.tags: image_labels += image.tags - runner_config = GitHubRunnerConfig( github_path=path, labels=(*state.charm_config.labels, *image_labels) ) @@ -1279,9 +1318,8 @@ def _get_runner_scaler( ssh_debug_connections=state.ssh_debug_connections, repo_policy_compliance=state.charm_config.repo_policy_compliance, ) - openstack_runner_manager_config = OpenStackRunnerManagerConfig( - manager_name=self.app.name, # TODO: rename to name + name=self.app.name, # The prefix is set to f"{application_name}-{unit number}" prefix=self.unit.name.replace("/", "-"), cloud_config=cloud_config, @@ -1289,31 +1327,7 @@ def _get_runner_scaler( runner_config=runner_config, service_config=service_config, ) - openstack_runner_manager = OpenStackRunnerManager( - config=openstack_runner_manager_config, - ) - runner_manager_config = RunnerManagerConfig( - name=self.app.name, - token=token, - path=path, - ) - runner_manager = RunnerManager( - cloud_runner_manager=openstack_runner_manager, - config=runner_manager_config, - ) - reactive_runner_config = None - if reactive_config := state.reactive_config: - reactive_runner_config = ReactiveRunnerConfig( - queue=ReactiveQueueConfig( - mongodb_uri=reactive_config.mq_uri, queue_name=self.app.name - ), # TODO think if queue_name should be really decided by charm - runner_manager=runner_manager_config, - cloud_runner_manager=openstack_runner_manager_config, - github_token=token, - ) - return RunnerScaler( - runner_manager=runner_manager, reactive_runner_config=reactive_runner_config - ) + return openstack_runner_manager_config if __name__ == "__main__": From 4d46828bb31cc874eff3cfa1927a9f368c9ed9da Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Wed, 18 Sep 2024 12:39:02 +0200 Subject: [PATCH 13/18] add changelog entry --- docs/changelog.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/changelog.md b/docs/changelog.md index 200e06f05..07369049b 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,5 +1,10 @@ # Changelog +### 2024-09-18 + +- Changed code to be able to spawn a runner in reactive mode. +- Removed reactive mode support for LXD as it is not currently in development. + ## 2024-09-09 - Added changelog for tracking user-relevant changes. From 268d9a904928a09b25f0e5b5f735c948b86f5670 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Wed, 18 Sep 2024 14:36:57 +0200 Subject: [PATCH 14/18] Revert "temporarily remove e2e test - REVERT ME" This reverts commit fbe771ca3c9bca6b356272ab0c0966086c7c1aa8. --- .github/workflows/e2e_test.yaml | 33 ++++++++ .github/workflows/e2e_test_run.yaml | 113 ++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+) create mode 100644 .github/workflows/e2e_test.yaml create mode 100644 .github/workflows/e2e_test_run.yaml diff --git a/.github/workflows/e2e_test.yaml b/.github/workflows/e2e_test.yaml new file mode 100644 index 000000000..7d0383c12 --- /dev/null +++ b/.github/workflows/e2e_test.yaml @@ -0,0 +1,33 @@ +name: End-to-End tests + +on: + pull_request: + + +jobs: + # test option values defined at test/conftest.py are passed on via repository secret + # INTEGRATION_TEST_ARGS to operator-workflows automatically. + openstack-integration-end-to-end-test: + name: end-to-end test using private-endpoint + uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main + secrets: inherit + with: + juju-channel: 3.2/stable + pre-run-script: scripts/setup-lxd.sh + provider: lxd + test-tox-env: integration-juju3.2 + modules: '["test_e2e"]' + extra-arguments: "-m openstack" + self-hosted-runner: true + self-hosted-runner-label: stg-private-endpoint + + required_status_checks: + name: Required E2E Test Status Checks + runs-on: ubuntu-latest + needs: + - openstack-integration-end-to-end-test + if: always() && !cancelled() + timeout-minutes: 5 + steps: + - run: | + [ '${{ needs.openstack-integration-end-to-end-test.result }}' = 'success' ] || (echo openstack-integration-end-to-end-test failed && false) diff --git a/.github/workflows/e2e_test_run.yaml b/.github/workflows/e2e_test_run.yaml new file mode 100644 index 000000000..4eeeccbe1 --- /dev/null +++ b/.github/workflows/e2e_test_run.yaml @@ -0,0 +1,113 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +name: Run End-to-End test + +on: + # The inputs are the same but they cannot be shared between triggers. + # See https://github.com/orgs/community/discussions/39357 + workflow_call: + inputs: + runner-tag: + description: The e2e test runner tag to run the workflow on. + type: string + required: true + runner-virt-type: + description: The e2e test runner virtualization type. E.g. lxd, or openstack. + # workflow_call does not support choice type. + type: string + required: true + workflow_dispatch: + inputs: + runner-tag: + description: The e2e test runner tag to run the workflow on. + type: string + required: true + runner-virt-type: + description: The e2e test runner virtualization type. + type: choice + required: true + options: + - lxd + - openstack + +jobs: + e2e-test: + name: End-to-End Test Run + runs-on: [self-hosted, linux, "${{ inputs.runner-tag }}"] + steps: + - name: Hostname is set to "github-runner" + run: sudo hostnamectl hostname | grep github-runner + # Snapd can have some issues in privileged LXD containers without setting + # security.nesting=True and this. + - name: Fix snap issue in privileged LXD containers + run: ln -s /bin/true /usr/local/bin/udevadm + # Below is a series of simple tests to assess the functionality of the newly spawned runner. + - name: Echo hello world + run: echo "hello world" + - name: File permission for /usr/local/bin + run: ls -ld /usr/local/bin | grep drwxrwxrwx + - name: Test file permission for /usr/local/bin + run: touch /usr/local/bin/test_file + # "Install microk8s" step will test if the proxies settings are correct. + - name: Proxy set in /etc/environment + run: cat /etc/environment + # "Update apt in python docker container" step will test docker default proxy settings due to + # pulling the python image. + - name: Proxy set in docker daemon + run: | + [[ -z "${http_proxy}" && -z "${HTTP_PROXY}" ]] \ + || sudo cat /etc/systemd/system/docker.service.d/http-proxy.conf | grep HTTP_PROXY + # "Update apt in python docker container" step will test docker client default proxy settings. + - name: Proxy set in docker client + run: | + [[ -z "${http_proxy}" && -z "${HTTP_PROXY}" ]] \ + || cat /home/ubuntu/.docker/config.json | grep httpProxy + - name: Install microk8s + run: sudo snap install microk8s --classic + - name: Wait for microk8s + timeout-minutes: 10 + run: microk8s status --wait-ready + - name: Deploy nginx for testing + run: microk8s kubectl create deployment nginx --image=nginx + - name: Wait for nginx to be ready + run: microk8s kubectl rollout status deployment/nginx --timeout=30m + - name: Update apt in python docker container + run: docker run python:3.10-slim apt-get update + - name: Docker version + run: docker version + - name: Check python alias for python3 + run: python --version + - name: pip version + run: python3 -m pip --version + - name: npm version + run: npm --version + - name: shellcheck version + run: shellcheck --version + - name: jq version + run: jq --version + - name: yq version + run: yq --version + - name: apt update + run: sudo apt-get update -y + # Use pipx for 24.04 noble, check-jsonschema breaks OS system packages. + - name: install pipx + run: sudo apt-get install -y pipx + - name: install check-jsonschema + run: python3 -m pip install check-jsonschema || pipx install check-jsonschema + - name: unzip version + run: unzip -v + - name: gh version + run: gh --version + # `check-jsonschema` is installed using pip. The directory `~/.local/bin` needs to be added to PATH. + # ~/.local/bin is added to path runner env through in scripts/env.j2 + - name: test check-jsonschema + run: check-jsonschema --version + - name: Test Firewall + if: "${{ github.event.inputs.runner-virt-type == 'lxd' }}" + run: | + HOST_IP=$(ip route | grep default | cut -f 3 -d" ") + [ $((ping $HOST_IP -c 5 || :) | grep "Destination Port Unreachable" | wc -l) -eq 5 ] + - name: Test sctp support + if: "${{ github.event.inputs.runner-virt-type == 'lxd' }}" + run: sudo apt-get install lksctp-tools -yq && checksctp From e2e035afede7bbb2d32b7c4c533c1247dd7bfe32 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Wed, 18 Sep 2024 14:37:05 +0200 Subject: [PATCH 15/18] Revert "add tmate" This reverts commit d9d7797a --- .github/workflows/integration_test.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 1ebda3c70..b5319dc2c 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -53,5 +53,3 @@ jobs: extra-arguments: "-m openstack" self-hosted-runner: true self-hosted-runner-label: stg-private-endpoint - tmate-debug: true - tmate-timeout: 60 From b1d31ad4230d6ec557ba1f1f23813b29878a4902 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Wed, 18 Sep 2024 14:37:07 +0200 Subject: [PATCH 16/18] Revert "disable most integration tests" This reverts commit 876bb68f2518c7af6ddf9f6158cb0155547a8ffe. --- .github/workflows/integration_test.yaml | 59 ++++++++++++------------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index b5319dc2c..8e0bc700a 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -8,48 +8,45 @@ concurrency: cancel-in-progress: true jobs: - -# TODO: disabled tests to reduce resource usage and speedup testing, re-enable before merging - # test option values defined at test/conftest.py are passed on via repository secret # INTEGRATION_TEST_ARGS to operator-workflows automatically. -# integration-tests: -# name: Integration test with juju 3.1 -# uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main -# secrets: inherit -# with: -# juju-channel: 3.1/stable -# pre-run-script: scripts/pre-integration-test.sh -# provider: lxd -# test-tox-env: integration-juju3.1 -# # These important local LXD test has no OpenStack integration versions. -# # test_charm_scheduled_events ensures reconcile events are fired on a schedule. -# # test_debug_ssh ensures tmate SSH actions works. -# # TODO: Add OpenStack integration versions of these tests. -# modules: '["test_charm_scheduled_events", "test_debug_ssh"]' -# openstack-interface-tests-private-endpoint: -# name: openstack interface test using private-endpoint -# uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main -# secrets: inherit -# with: -# juju-channel: 3.2/stable -# pre-run-script: scripts/setup-lxd.sh -# provider: lxd -# test-tox-env: integration-juju3.2 -# modules: '["test_runner_manager_openstack"]' -# self-hosted-runner: true -# self-hosted-runner-label: stg-private-endpoint + integration-tests: + name: Integration test with juju 3.1 + uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main + secrets: inherit + with: + juju-channel: 3.1/stable + pre-run-script: scripts/pre-integration-test.sh + provider: lxd + test-tox-env: integration-juju3.1 + # These important local LXD test has no OpenStack integration versions. + # test_charm_scheduled_events ensures reconcile events are fired on a schedule. + # test_debug_ssh ensures tmate SSH actions works. + # TODO: Add OpenStack integration versions of these tests. + modules: '["test_charm_scheduled_events", "test_debug_ssh"]' + openstack-interface-tests-private-endpoint: + name: openstack interface test using private-endpoint + uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main + secrets: inherit + with: + juju-channel: 3.2/stable + pre-run-script: scripts/setup-lxd.sh + provider: lxd + test-tox-env: integration-juju3.2 + modules: '["test_runner_manager_openstack"]' + self-hosted-runner: true + self-hosted-runner-label: stg-private-endpoint 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 + needs: openstack-interface-tests-private-endpoint secrets: inherit with: juju-channel: 3.2/stable pre-run-script: scripts/setup-lxd.sh provider: lxd test-tox-env: integration-juju3.2 - modules: '["test_reactive"]' + modules: '["test_charm_metrics_failure", "test_charm_metrics_success", "test_charm_fork_repo", "test_charm_runner", "test_reactive"]' extra-arguments: "-m openstack" self-hosted-runner: true self-hosted-runner-label: stg-private-endpoint From 24437d33e057195186f96e6041a9bf3e83bbe963 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Thu, 19 Sep 2024 15:45:07 +0200 Subject: [PATCH 17/18] pin correct commit --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index df9df30d3..34c61f262 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@feat/reactive-spawning +github_runner_manager @ git+https://github.com/canonical/github-runner-manager.git@e53ed35048e124e3046b860bc0b8c7d89abd8b18 From 5bf027fd46a837a119a3d04aed0483eec31682a4 Mon Sep 17 00:00:00 2001 From: Christopher Bartz Date: Thu, 19 Sep 2024 16:13:41 +0200 Subject: [PATCH 18/18] fix test_runner_manager_openstack integration test --- tests/integration/test_runner_manager_openstack.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_runner_manager_openstack.py b/tests/integration/test_runner_manager_openstack.py index cb88d84ba..be7d324b8 100644 --- a/tests/integration/test_runner_manager_openstack.py +++ b/tests/integration/test_runner_manager_openstack.py @@ -31,7 +31,7 @@ from github_runner_manager.openstack_cloud.openstack_runner_manager import ( OpenStackCloudConfig, OpenStackRunnerManager, - OpenStackServerConfig, + OpenStackServerConfig, OpenStackRunnerManagerConfig, ) from github_runner_manager.types_.github import GitHubPath, parse_github_path from openstack.connection import Connection as OpenstackConnection @@ -133,8 +133,18 @@ async def openstack_runner_manager_fixture( ssh_debug_connections=None, repo_policy_compliance=None, ) + + openstack_runner_manager_config = OpenStackRunnerManagerConfig( + name=app_name, + prefix=f"{app_name}-0", + cloud_config=cloud_config, + server_config=server_config, + runner_config=runner_config, + service_config=service_config, + ) + return OpenStackRunnerManager( - app_name, f"{app_name}-0", cloud_config, server_config, runner_config, service_config + config=openstack_runner_manager_config, )