diff --git a/.github/workflows/test_github_runner_manager.yaml b/.github/workflows/test_github_runner_manager.yaml index 449f8c06a1..f8f62a170e 100644 --- a/.github/workflows/test_github_runner_manager.yaml +++ b/.github/workflows/test_github_runner_manager.yaml @@ -9,8 +9,11 @@ jobs: uses: canonical/operator-workflows/.github/workflows/test.yaml@main secrets: inherit with: - self-hosted-runner: true - self-hosted-runner-label: edge + # Run on GitHub-hosted Noble (ubuntu-24.04) runners. Pin Python to 3.12: the + # open-ended requires-python (>=3.10) otherwise resolves to the newest available + # interpreter (3.14), whose default multiprocessing start method (forkserver) + # breaks tests that rely on fork (e.g. test_flush_runner_locked). + python-version: "3.12" working-directory: ./github-runner-manager/ application-integration-tests: name: Integration tests for github-runner-manager diff --git a/docs/changelog.md b/docs/changelog.md index 2bc6cf0ac3..70faf71a80 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,6 +2,10 @@ This changelog documents user-relevant changes to the GitHub runner charm. +## 2026-06-26 + +- Fixed runners whose cloud VM entered an error state being kept until the creation timeout (~23 minutes) before cleanup. Such VMs are now cleaned up immediately, freeing the slot for a replacement runner. + ## 2026-05-20 - Enhanced OpenTelemetry Collector configuration in the pre-job script to expose local OTLP (gRPC/HTTP) and Loki endpoints for workflow to push observability data. diff --git a/github-runner-manager/pyproject.toml b/github-runner-manager/pyproject.toml index 3899e8b5c0..95a7a78465 100644 --- a/github-runner-manager/pyproject.toml +++ b/github-runner-manager/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-manager" -version = "0.18.1" +version = "0.18.2" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] diff --git a/github-runner-manager/src/github_runner_manager/manager/runner_manager.py b/github-runner-manager/src/github_runner_manager/manager/runner_manager.py index 3565f6331c..8cb4ce624f 100644 --- a/github-runner-manager/src/github_runner_manager/manager/runner_manager.py +++ b/github-runner-manager/src/github_runner_manager/manager/runner_manager.py @@ -639,10 +639,11 @@ def _get_platform_runners_to_cleanup( ) -> set[str]: """Determine platform runners to clean up. - 1. Always clean up danging platform runners (platform runners that have no VM associated). + 1. Always clean up dangling platform runners (platform runners that have no VM associated). 2. Always clean up deletable platform runners (deletable decision made by platform provider). - 3. Clean up runners that in offline-idle status that have timed out where the possible - scenarios is: + 3. Always clean up runners whose VM is in an error state in the cloud, as such a VM will + never recover and would otherwise occupy a runner slot until it times out. + 4. Clean up runners that are offline and idle and have timed out. The possible scenario is: - runner registered (during registration token generation) but VM has failed to spawn. Args: @@ -670,6 +671,17 @@ def _get_platform_runners_to_cleanup( logger.debug("Deletable runner IDs: %s", deletable_runners) vm_instance_id_map = {vm.instance_id: vm for vm in vms} + # Kill runners whose cloud VM is in an error state. Such VMs never recover, so there is no + # point waiting for the creation timeout; deleting now frees the slot for a replacement. + errored_runners: set[str] = set( + runner.identity.metadata.runner_id + for runner in runners.requested_runners + if runner.identity.metadata.runner_id + and (vm := vm_instance_id_map.get(runner.identity.instance_id)) + and vm.state == VMState.ERROR + ) + logger.debug("Errored runner IDs: %s", errored_runners) + # Kill old runners that are offline and idle as they could be in failed state. # We may also kill here runners that were online and not busy and went temporarily to # offline, but that should not be an issue, as those runners will be spawned again. @@ -687,7 +699,7 @@ def _get_platform_runners_to_cleanup( reconcile_metrics.TIMED_OUT_RUNNERS_TOTAL.inc(len(timed_out_offline_idle_runners)) logger.debug("Timed out offline idle runner IDs: %s", timed_out_offline_idle_runners) - return dangling_runners | deletable_runners | timed_out_offline_idle_runners + return dangling_runners | deletable_runners | errored_runners | timed_out_offline_idle_runners def _get_platform_runners_to_flush( diff --git a/github-runner-manager/tests/unit/factories/runner_instance_factory.py b/github-runner-manager/tests/unit/factories/runner_instance_factory.py index eceadb66e2..8344730612 100644 --- a/github-runner-manager/tests/unit/factories/runner_instance_factory.py +++ b/github-runner-manager/tests/unit/factories/runner_instance_factory.py @@ -70,18 +70,22 @@ class Meta: created_at = factory.LazyFunction(lambda: datetime.now(tz=timezone.utc)) @classmethod - def from_self_hosted_runner(cls, self_hosted_runner: SelfHostedRunner) -> VM: + def from_self_hosted_runner( + cls, self_hosted_runner: SelfHostedRunner, state: VMState = VMState.ACTIVE + ) -> VM: """Construct CloudRunnerInstance associated to self hosted runner. Args: self_hosted_runner: The target self hosted runner to associate. + state: The cloud VM state of the instance. Returns: The Instantiated CloudRunnerInstance. """ - return CloudRunnerInstanceFactory( + return cls( instance_id=self_hosted_runner.identity.instance_id, metadata=RunnerMetadataFactory(runner_id=str(self_hosted_runner.id)), + state=state, ) diff --git a/github-runner-manager/tests/unit/manager/test_runner_manager.py b/github-runner-manager/tests/unit/manager/test_runner_manager.py index b85f3357a6..aab38bf725 100644 --- a/github-runner-manager/tests/unit/manager/test_runner_manager.py +++ b/github-runner-manager/tests/unit/manager/test_runner_manager.py @@ -14,7 +14,7 @@ RunnerInstance, RunnerManager, ) -from github_runner_manager.manager.vm_manager import VM, CloudRunnerManager +from github_runner_manager.manager.vm_manager import VM, CloudRunnerManager, VMState from github_runner_manager.platform.platform_provider import PlatformProvider from github_runner_manager.types_.github import SelfHostedRunner from tests.unit.factories.runner_instance_factory import ( @@ -179,6 +179,21 @@ def test_flush_runners( [runner_with_platform, runner_without_platform], id="some in cloud, some not in cloud", ), + pytest.param( + [ + errored_runner := SelfHostedRunnerFactory( + status="offline", busy=False, deletable=False + ) + ], + [ + CloudRunnerInstanceFactory.from_self_hosted_runner( + self_hosted_runner=errored_runner, state=VMState.ERROR + ) + ], + [], + [], + id="recent errored-state cloud VM is cleaned up despite not being timed out", + ), ], ) def test_runner_manager_cleanup(