diff --git a/pyproject.toml b/pyproject.toml index aef6309..9f775f0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-manager" -version = "0.1.5" +version = "0.2.0" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] @@ -46,7 +46,7 @@ omit = [ ] [tool.coverage.report] -fail_under = 79 +fail_under = 80 show_missing = true diff --git a/requirements.txt b/requirements.txt index 15847d7..df2f853 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,3 +4,4 @@ jinja2 kombu==5.4.1 openstacksdk>=3,<4 pydantic < 2 +pymongo==4.8.0 diff --git a/src-docs/github_client.md b/src-docs/github_client.md index b452914..7e06d56 100644 --- a/src-docs/github_client.md +++ b/src-docs/github_client.md @@ -81,19 +81,43 @@ Delete the self-hosted runner from GitHub. --- - + ### method `get_job_info` ```python -get_job_info( +get_job_info(path: GitHubRepo, job_id: str) → JobInfo +``` + +Get information about a job identified by the job id. + + + +**Args:** + + - `path`: GitHub repository path in the format '/'. + - `job_id`: The job id. + + + +**Returns:** + The JSON response from the API. + +--- + + + +### method `get_job_info_by_runner_name` + +```python +get_job_info_by_runner_name( path: GitHubRepo, workflow_run_id: str, runner_name: str -) → JobStats +) → JobInfo ``` -Get information about a job for a specific workflow run. +Get information about a job for a specific workflow run identified by the runner name. @@ -107,7 +131,7 @@ Get information about a job for a specific workflow run. **Raises:** - - `TokenError`: if there was an error with the Github token crdential provided. + - `TokenError`: if there was an error with the Github token credential provided. - `JobNotFoundError`: If no jobs were found. diff --git a/src-docs/manager.runner_manager.md b/src-docs/manager.runner_manager.md index 652f3e9..92997c8 100644 --- a/src-docs/manager.runner_manager.md +++ b/src-docs/manager.runner_manager.md @@ -77,6 +77,7 @@ Configuration for the runner manager. **Attributes:** + - `name`: A name to identify this manager. - `token`: GitHub personal access token to query GitHub API. - `path`: Path to GitHub repository or organization to registry the runners. @@ -85,7 +86,7 @@ Configuration for the runner manager. ### method `__init__` ```python -__init__(token: str, path: GitHubOrg | GitHubRepo) → None +__init__(name: str, token: str, path: GitHubOrg | GitHubRepo) → None ``` @@ -98,7 +99,7 @@ __init__(token: str, path: GitHubOrg | GitHubRepo) → None --- - + ## class `RunnerManager` Manage the runners. @@ -110,16 +111,12 @@ Manage the runners. - `manager_name`: A name to identify this manager. - `name_prefix`: The name prefix of the runners. - + ### method `__init__` ```python -__init__( - manager_name: str, - cloud_runner_manager: CloudRunnerManager, - config: RunnerManagerConfig -) +__init__(cloud_runner_manager: CloudRunnerManager, config: RunnerManagerConfig) ``` Construct the object. @@ -128,7 +125,6 @@ Construct the object. **Args:** - - `manager_name`: A name to identify this manager. - `cloud_runner_manager`: For managing the cloud instance of the runner. - `config`: Configuration of this class. diff --git a/src-docs/manager.runner_scaler.md b/src-docs/manager.runner_scaler.md index e5abac0..c567451 100644 --- a/src-docs/manager.runner_scaler.md +++ b/src-docs/manager.runner_scaler.md @@ -9,7 +9,7 @@ Module for scaling the runners amount. --- - + ## class `RunnerInfo` Information on the runners. @@ -50,17 +50,20 @@ __init__( --- - + ## class `RunnerScaler` Manage the reconcile of runners. - + ### method `__init__` ```python -__init__(runner_manager: RunnerManager, reactive_config: ReactiveConfig | None) +__init__( + runner_manager: RunnerManager, + reactive_runner_config: RunnerConfig | None +) ``` Construct the object. @@ -70,7 +73,7 @@ Construct the object. **Args:** - `runner_manager`: The RunnerManager to perform runner reconcile. - - `reactive_config`: Reactive runner configuration. + - `reactive_runner_config`: Reactive runner configuration. diff --git a/src-docs/openstack_cloud.openstack_runner_manager.md b/src-docs/openstack_cloud.openstack_runner_manager.md index be23251..ba50e05 100644 --- a/src-docs/openstack_cloud.openstack_runner_manager.md +++ b/src-docs/openstack_cloud.openstack_runner_manager.md @@ -78,24 +78,29 @@ __init__(image: str, flavor: str, network: str) → None --- - + -## class `OpenStackRunnerManager` -Manage self-hosted runner on OpenStack cloud. +## class `OpenStackRunnerManagerConfig` +Configuration for OpenStack runner manager. **Attributes:** - - `name_prefix`: The name prefix of the runners created. + - `name`: The name of the manager. + - `prefix`: The prefix of the runner names. + - `cloud_config`: The configuration for OpenStack cloud. + - `server_config`: The configuration for OpenStack server. + - `runner_config`: The configuration for the GitHub runner. + - `service_config`: The configuration for supporting services. - + ### method `__init__` ```python __init__( - manager_name: str, + name: str, prefix: str, cloud_config: OpenStackCloudConfig, server_config: OpenStackServerConfig | None, @@ -104,18 +109,42 @@ __init__( ) → None ``` + + + + + + + + +--- + + + +## class `OpenStackRunnerManager` +Manage self-hosted runner on OpenStack cloud. + + + +**Attributes:** + + - `name_prefix`: The name prefix of the runners created. + + + +### method `__init__` + +```python +__init__(config: OpenStackRunnerManagerConfig) → None +``` + Construct the object. **Args:** - - `manager_name`: A name to identify this manager. - - `prefix`: The prefix to runner name. - - `cloud_config`: The configuration for OpenStack authorisation. - - `server_config`: The configuration for creating OpenStack server. Unable to create runner if None. - - `runner_config`: The configuration for the runner. - - `service_config`: The configuration of supporting services of the runners. + - `config`: The configuration for the openstack runner manager. --- @@ -133,7 +162,7 @@ The prefix of runner names. --- - + ### method `cleanup` @@ -156,7 +185,7 @@ Cleanup runner and resource on the cloud. --- - + ### method `create_runner` @@ -186,7 +215,7 @@ Create a self-hosted runner. --- - + ### method `delete_runner` @@ -210,7 +239,7 @@ Delete self-hosted runners. --- - + ### method `flush_runners` @@ -233,7 +262,7 @@ Remove idle and/or busy runners. --- - + ### method `get_runner` @@ -256,7 +285,7 @@ Get a self-hosted runner by instance id. --- - + ### method `get_runners` diff --git a/src-docs/reactive.consumer.md b/src-docs/reactive.consumer.md index 99ce9ed..b8433b4 100644 --- a/src-docs/reactive.consumer.md +++ b/src-docs/reactive.consumer.md @@ -8,12 +8,16 @@ Module responsible for consuming jobs from the message queue. --- - + ## function `consume` ```python -consume(mongodb_uri: str, queue_name: str) → None +consume( + queue_config: QueueConfig, + runner_manager: RunnerManager, + github_client: GithubClient +) → None ``` Consume a job from the message queue. @@ -24,8 +28,9 @@ Log the job details and acknowledge the message. If the job details are invalid, **Args:** - - `mongodb_uri`: The URI of the MongoDB database. - - `queue_name`: The name of the queue. + - `queue_config`: The configuration for the message queue. + - `runner_manager`: The runner manager used to create the runner. + - `github_client`: The GitHub client to use to check the job status. @@ -36,7 +41,7 @@ Log the job details and acknowledge the message. If the job details are invalid, --- - + ## function `signal_handler` @@ -57,7 +62,25 @@ The signal handler exits the process. --- - + + +## class `JobPickedUpStates` +The states of a job that indicate it has been picked up. + + + +**Attributes:** + + - `COMPLETED`: The job has completed. + - `IN_PROGRESS`: The job is in progress. + + + + + +--- + + ## class `JobDetails` A class to translate the payload. @@ -67,15 +90,44 @@ A class to translate the payload. **Attributes:** - `labels`: The labels of the job. - - `run_url`: The URL of the job. + - `job_url`: The URL of the job. + + + + +--- + + + +### classmethod `check_job_url_path_is_not_empty` +```python +check_job_url_path_is_not_empty(v: HttpUrl) → HttpUrl +``` +Check that the job_url path is not empty. +**Args:** + + - `v`: The job_url to check. + + + +**Returns:** + The job_url if it is valid. + + + +**Raises:** + + - `ValueError`: If the job_url path is empty. + + --- - + ## class `JobError` Raised when a job error occurs. diff --git a/src-docs/reactive.runner.md b/src-docs/reactive.runner.md index 83fd8b5..266c19d 100644 --- a/src-docs/reactive.runner.md +++ b/src-docs/reactive.runner.md @@ -7,12 +7,11 @@ Script to spawn a reactive runner process. **Global Variables** --------------- -- **MQ_URI_ENV_VAR** -- **QUEUE_NAME_ENV_VAR** +- **RUNNER_CONFIG_ENV_VAR** --- - + ## function `setup_root_logging` @@ -25,7 +24,7 @@ Set up logging for the reactive runner process. --- - + ## function `main` diff --git a/src-docs/reactive.runner_manager.md b/src-docs/reactive.runner_manager.md index 774c368..b426d52 100644 --- a/src-docs/reactive.runner_manager.md +++ b/src-docs/reactive.runner_manager.md @@ -7,14 +7,13 @@ Module for managing reactive runners. **Global Variables** --------------- -- **MQ_URI_ENV_VAR** -- **QUEUE_NAME_ENV_VAR** - **PYTHON_BIN** - **REACTIVE_RUNNER_SCRIPT_MODULE** - **REACTIVE_RUNNER_CMD_LINE_PREFIX** - **PID_CMD_COLUMN_WIDTH** - **PIDS_COMMAND_LINE** - **UBUNTU_USER** +- **RUNNER_CONFIG_ENV_VAR** --- @@ -23,7 +22,7 @@ Module for managing reactive runners. ## function `reconcile` ```python -reconcile(quantity: int, mq_uri: str, queue_name: str) → int +reconcile(quantity: int, runner_config: RunnerConfig) → int ``` Spawn a runner reactively. @@ -33,8 +32,7 @@ Spawn a runner reactively. **Args:** - `quantity`: The number of runners to spawn. - - `mq_uri`: The message queue URI. - - `queue_name`: The name of the queue. + - `runner_config`: The reactive runner configuration. Raises a ReactiveRunnerError if the runner fails to spawn. diff --git a/src-docs/reactive.types_.md b/src-docs/reactive.types_.md new file mode 100644 index 0000000..18b7ad8 --- /dev/null +++ b/src-docs/reactive.types_.md @@ -0,0 +1,47 @@ + + + + +# module `reactive.types_` +Module containing reactive scheduling related types. + + + +--- + + + +## class `QueueConfig` +The configuration for the message queue. + + + +**Attributes:** + + - `mongodb_uri`: The URI of the MongoDB database. + - `queue_name`: The name of the queue. + + + + + +--- + + + +## class `RunnerConfig` +The configuration for the reactive runner to spawn. + + + +**Attributes:** + + - `queue`: The queue configuration. + - `runner_manager`: The runner manager configuration. + - `cloud_runner_manager`: The OpenStack runner manager configuration. + - `github_token`: str + + + + + diff --git a/src-docs/types_.general.md b/src-docs/types_.general.md deleted file mode 100644 index 3a0a036..0000000 --- a/src-docs/types_.general.md +++ /dev/null @@ -1,121 +0,0 @@ - - - - -# module `types_.general` -Module containing a collection of unrelated types. - - - ---- - - - -## class `ReactiveConfig` -Represents the configuration for reactive scheduling. - - - -**Attributes:** - - - `mq_uri`: The URI of the MQ to use to spawn runners reactively. - - - - - ---- - - - -## class `ProxyConfig` -Proxy configuration. - - - -**Attributes:** - - - `aproxy_address`: The address of aproxy snap instance if use_aproxy is enabled. - - `http`: HTTP proxy address. - - `https`: HTTPS proxy address. - - `no_proxy`: Comma-separated list of hosts that should not be proxied. - - `use_aproxy`: Whether aproxy should be used for the runners. - - ---- - -#### property aproxy_address - -Return the aproxy address. - - - ---- - - - -### classmethod `check_use_aproxy` - -```python -check_use_aproxy(use_aproxy: bool, values: dict) → bool -``` - -Validate the proxy configuration. - - - -**Args:** - - - `use_aproxy`: Value of use_aproxy variable. - - `values`: Values in the pydantic model. - - - -**Raises:** - - - `ValueError`: if use_aproxy was set but no http/https was passed. - - - -**Returns:** - Validated use_aproxy value. - - ---- - - - -## class `SSHDebugConnection` -SSH connection information for debug workflow. - - - -**Attributes:** - - - `host`: The SSH relay server host IP address inside the VPN. - - `port`: The SSH relay server port. - - `rsa_fingerprint`: The host SSH server public RSA key fingerprint. - - `ed25519_fingerprint`: The host SSH server public ed25519 key fingerprint. - - - - - ---- - - - -## class `RepoPolicyComplianceConfig` -Configuration for the repo policy compliance service. - - - -**Attributes:** - - - `token`: Token for the repo policy compliance service. - - `url`: URL of the repo policy compliance service. - - - - - diff --git a/src-docs/types_.github.md b/src-docs/types_.github.md index d410455..195a459 100644 --- a/src-docs/types_.github.md +++ b/src-docs/types_.github.md @@ -8,7 +8,7 @@ Module containing GitHub API related types. --- - + ## function `parse_github_path` @@ -201,7 +201,29 @@ See :https://docs.github.com/en/rest/actions/workflow-runs?apiVersion=2022-11-28 -## class `JobStats` +## class `JobStatus` +Status of a job on GitHub. + + + +**Attributes:** + + - `QUEUED`: Represents a job that is queued. + - `IN_PROGRESS`: Represents a job that is in progress. + - `COMPLETED`: Represents a job that is completed. + - `WAITING`: Represents a job that is waiting. + - `REQUESTED`: Represents a job that is requested. + - `PENDING`: Represents a job that is pending. + + + + + +--- + + + +## class `JobInfo` Stats for a job on GitHub. @@ -212,6 +234,7 @@ Stats for a job on GitHub. - `created_at`: The time the job was created. - `started_at`: The time the job was started. - `conclusion`: The end result of a job. + - `status`: The status of the job. @@ -219,7 +242,7 @@ Stats for a job on GitHub. --- - + ## class `GitHubRepo` Represent GitHub repository. @@ -248,7 +271,7 @@ __init__(owner: 'str', repo: 'str') → None --- - + ### method `path` @@ -266,7 +289,7 @@ Return a string representing the path. --- - + ## class `GitHubOrg` Represent GitHub organization. @@ -295,7 +318,7 @@ __init__(org: 'str', group: 'str') → None --- - + ### method `path` diff --git a/src-docs/types_.md b/src-docs/types_.md index b423cd2..8f73fad 100644 --- a/src-docs/types_.md +++ b/src-docs/types_.md @@ -11,23 +11,6 @@ Package containing modules with type definitions. -## class `ReactiveConfig` -Represents the configuration for reactive scheduling. - - - -**Attributes:** - - - `mq_uri`: The URI of the MQ to use to spawn runners reactively. - - - - - ---- - - - ## class `ProxyConfig` Proxy configuration. @@ -52,7 +35,7 @@ Return the aproxy address. --- - + ### classmethod `check_use_aproxy` @@ -83,7 +66,7 @@ Validate the proxy configuration. --- - + ## class `SSHDebugConnection` SSH connection information for debug workflow. @@ -103,7 +86,7 @@ SSH connection information for debug workflow. --- - + ## class `RepoPolicyComplianceConfig` Configuration for the repo policy compliance service. diff --git a/src/github_runner_manager/github_client.py b/src/github_runner_manager/github_client.py index a888724..4b4ebbe 100644 --- a/src/github_runner_manager/github_client.py +++ b/src/github_runner_manager/github_client.py @@ -21,7 +21,7 @@ GitHubOrg, GitHubPath, GitHubRepo, - JobStats, + JobInfo, RegistrationToken, RemoveToken, SelfHostedRunner, @@ -200,8 +200,10 @@ def delete_runner(self, path: GitHubPath, runner_id: int) -> None: runner_id=runner_id, ) - def get_job_info(self, path: GitHubRepo, workflow_run_id: str, runner_name: str) -> JobStats: - """Get information about a job for a specific workflow run. + def get_job_info_by_runner_name( + self, path: GitHubRepo, workflow_run_id: str, runner_name: str + ) -> JobInfo: + """Get information about a job for a specific workflow run identified by the runner name. Args: path: GitHub repository path in the format '/'. @@ -209,7 +211,7 @@ def get_job_info(self, path: GitHubRepo, workflow_run_id: str, runner_name: str) runner_name: Name of the runner. Raises: - TokenError: if there was an error with the Github token crdential provided. + TokenError: if there was an error with the Github token credential provided. JobNotFoundError: If no jobs were found. Returns: @@ -227,27 +229,7 @@ def get_job_info(self, path: GitHubRepo, workflow_run_id: str, runner_name: str) break for job in jobs: if job["runner_name"] == runner_name: - # datetime strings should be in ISO 8601 format, - # but they can also use Z instead of - # +00:00, which is not supported by datetime.fromisoformat - created_at = datetime.fromisoformat( - job["created_at"].replace("Z", "+00:00") - ) - started_at = datetime.fromisoformat( - job["started_at"].replace("Z", "+00:00") - ) - # conclusion could be null per api schema, so we need to handle that - # though we would assume that it should always be present, - # as the job should be finished - conclusion = job.get("conclusion", None) - - job_id = job["id"] - return JobStats( - job_id=job_id, - created_at=created_at, - started_at=started_at, - conclusion=conclusion, - ) + return self._to_job_info(job) except HTTPError as exc: if exc.code in (401, 403): @@ -258,3 +240,51 @@ def get_job_info(self, path: GitHubRepo, workflow_run_id: str, runner_name: str) ) from exc raise JobNotFoundError(f"Could not find job for runner {runner_name}.") + + @catch_http_errors + def get_job_info(self, path: GitHubRepo, job_id: str) -> JobInfo: + """Get information about a job identified by the job id. + + Args: + path: GitHub repository path in the format '/'. + job_id: The job id. + + Returns: + The JSON response from the API. + """ + job_raw = self._client.actions.get_job_for_workflow_run( + owner=path.owner, + repo=path.repo, + job_id=job_id, + ) + return self._to_job_info(job_raw) + + @staticmethod + def _to_job_info(job: dict) -> JobInfo: + """Convert the job dict to JobInfo. + + Args: + job: The job dict. + + Returns: + The JobInfo object. + """ + # datetime strings should be in ISO 8601 format, + # but they can also use Z instead of + # +00:00, which is not supported by datetime.fromisoformat + created_at = datetime.fromisoformat(job["created_at"].replace("Z", "+00:00")) + started_at = datetime.fromisoformat(job["started_at"].replace("Z", "+00:00")) + # conclusion could be null per api schema, so we need to handle that + # though we would assume that it should always be present, + # as the job should be finished + conclusion = job.get("conclusion", None) + + status = job["status"] + job_id = job["id"] + return JobInfo( + job_id=job_id, + created_at=created_at, + started_at=started_at, + conclusion=conclusion, + status=status, + ) diff --git a/src/github_runner_manager/manager/runner_manager.py b/src/github_runner_manager/manager/runner_manager.py index f8ef0ae..76f6b34 100644 --- a/src/github_runner_manager/manager/runner_manager.py +++ b/src/github_runner_manager/manager/runner_manager.py @@ -83,10 +83,12 @@ class RunnerManagerConfig: """Configuration for the runner manager. Attributes: + name: A name to identify this manager. token: GitHub personal access token to query GitHub API. path: Path to GitHub repository or organization to registry the runners. """ + name: str token: str path: GitHubPath @@ -101,18 +103,16 @@ class RunnerManager: def __init__( self, - manager_name: str, cloud_runner_manager: CloudRunnerManager, config: RunnerManagerConfig, ): """Construct the object. Args: - manager_name: A name to identify this manager. cloud_runner_manager: For managing the cloud instance of the runner. config: Configuration of this class. """ - self.manager_name = manager_name + self.manager_name = config.name self._config = config self._cloud = cloud_runner_manager self.name_prefix = self._cloud.name_prefix diff --git a/src/github_runner_manager/manager/runner_scaler.py b/src/github_runner_manager/manager/runner_scaler.py index df92b3f..c13a9c3 100644 --- a/src/github_runner_manager/manager/runner_scaler.py +++ b/src/github_runner_manager/manager/runner_scaler.py @@ -7,8 +7,6 @@ import time from dataclasses import dataclass -from pydantic import MongoDsn - import github_runner_manager.reactive.runner_manager as reactive_runner_manager from github_runner_manager.errors import IssueMetricEventError, MissingServerConfigError from github_runner_manager.manager.cloud_runner_manager import HealthState @@ -20,7 +18,7 @@ RunnerManager, ) from github_runner_manager.metrics import events as metric_events -from github_runner_manager.types_ import ReactiveConfig +from github_runner_manager.reactive.types_ import RunnerConfig as ReactiveRunnerConfig logger = logging.getLogger(__name__) @@ -49,15 +47,17 @@ class RunnerInfo: class RunnerScaler: """Manage the reconcile of runners.""" - def __init__(self, runner_manager: RunnerManager, reactive_config: ReactiveConfig | None): + def __init__( + self, runner_manager: RunnerManager, reactive_runner_config: ReactiveRunnerConfig | None + ): """Construct the object. Args: runner_manager: The RunnerManager to perform runner reconcile. - reactive_config: Reactive runner configuration. + reactive_runner_config: Reactive runner configuration. """ self._manager = runner_manager - self._reactive_config = reactive_config + self._reactive_config = reactive_runner_config def get_runner_info(self) -> RunnerInfo: """Get information on the runners. @@ -126,7 +126,7 @@ def reconcile(self, quantity: int) -> int: if self._reactive_config is not None: logger.info("Reactive configuration detected, going into experimental reactive mode.") - return self._reconcile_reactive(quantity, self._reactive_config.mq_uri) + return self._reconcile_reactive(quantity) metric_stats = {} start_timestamp = time.time() @@ -249,20 +249,19 @@ def _issue_reconciliation_metric( except IssueMetricEventError: logger.exception("Failed to issue Reconciliation metric") - def _reconcile_reactive(self, quantity: int, mq_uri: MongoDsn) -> int: + def _reconcile_reactive(self, quantity: int) -> int: """Reconcile runners reactively. Args: quantity: Number of intended runners. - mq_uri: The URI of the MQ to use to spawn runners reactively. 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.") + self._manager.cleanup() return reactive_runner_manager.reconcile( quantity=quantity, - mq_uri=mq_uri, - queue_name=self._manager.manager_name, + runner_config=self._reactive_config, ) diff --git a/src/github_runner_manager/metrics/github.py b/src/github_runner_manager/metrics/github.py index 19b4b56..1afc294 100644 --- a/src/github_runner_manager/metrics/github.py +++ b/src/github_runner_manager/metrics/github.py @@ -34,7 +34,7 @@ def job( owner, repo = pre_job_metrics.repository.split("/", maxsplit=1) try: - job_info = github_client.get_job_info( + job_info = github_client.get_job_info_by_runner_name( path=GitHubRepo(owner=owner, repo=repo), workflow_run_id=pre_job_metrics.workflow_run_id, runner_name=runner_name, diff --git a/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py b/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py index f7cc5d2..fe6b322 100644 --- a/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py +++ b/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py @@ -98,6 +98,27 @@ class OpenStackServerConfig: network: str +@dataclass +class OpenStackRunnerManagerConfig: + """Configuration for OpenStack runner manager. + + Attributes: + name: The name of the manager. + prefix: The prefix of the runner names. + cloud_config: The configuration for OpenStack cloud. + server_config: The configuration for OpenStack server. + runner_config: The configuration for the GitHub runner. + service_config: The configuration for supporting services. + """ + + name: str + prefix: str + cloud_config: OpenStackCloudConfig + server_config: OpenStackServerConfig | None + runner_config: GitHubRunnerConfig + service_config: SupportServiceConfig + + @dataclass class _RunnerHealth: """Runners with health state. @@ -118,33 +139,21 @@ class OpenStackRunnerManager(CloudRunnerManager): name_prefix: The name prefix of the runners created. """ - # Ignore "Too many arguments", as the class requires a lot of configurations. def __init__( # pylint: disable=R0913 self, - manager_name: str, - prefix: str, - cloud_config: OpenStackCloudConfig, - server_config: OpenStackServerConfig | None, - runner_config: GitHubRunnerConfig, - service_config: SupportServiceConfig, + config: OpenStackRunnerManagerConfig, ) -> None: """Construct the object. Args: - manager_name: A name to identify this manager. - prefix: The prefix to runner name. - cloud_config: The configuration for OpenStack authorisation. - server_config: The configuration for creating OpenStack server. Unable to create - runner if None. - runner_config: The configuration for the runner. - service_config: The configuration of supporting services of the runners. + config: The configuration for the openstack runner manager. """ - self._manager_name = manager_name - self._prefix = prefix - self._cloud_config = cloud_config - self._server_config = server_config - self._runner_config = runner_config - self._service_config = service_config + self._manager_name = config.name + self._prefix = config.prefix + self._cloud_config = config.cloud_config + self._server_config = config.server_config + self._runner_config = config.runner_config + self._service_config = config.service_config self._openstack_cloud = OpenstackCloud( clouds_config=self._cloud_config.clouds_config, cloud=self._cloud_config.cloud, @@ -152,7 +161,7 @@ def __init__( # pylint: disable=R0913 ) # Setting the env var to this process and any child process spawned. - proxies = service_config.proxy_config + proxies = self._service_config.proxy_config if proxies and (no_proxy := proxies.no_proxy): set_env_var("NO_PROXY", no_proxy) if proxies and (http_proxy := proxies.http): diff --git a/src/github_runner_manager/reactive/consumer.py b/src/github_runner_manager/reactive/consumer.py index f868fed..d35b8f1 100644 --- a/src/github_runner_manager/reactive/consumer.py +++ b/src/github_runner_manager/reactive/consumer.py @@ -7,47 +7,87 @@ import signal import sys from contextlib import closing +from enum import Enum +from time import sleep from types import FrameType from typing import Generator, cast -from kombu import Connection +from kombu import Connection, Message from kombu.simple import SimpleQueue -from pydantic import BaseModel, HttpUrl, ValidationError +from pydantic import BaseModel, HttpUrl, ValidationError, validator + +from github_runner_manager.github_client import GithubClient +from github_runner_manager.manager.runner_manager import RunnerManager +from github_runner_manager.reactive.types_ import QueueConfig +from github_runner_manager.types_.github import GitHubRepo logger = logging.getLogger(__name__) +class JobPickedUpStates(str, Enum): + """The states of a job that indicate it has been picked up. + + Attributes: + COMPLETED: The job has completed. + IN_PROGRESS: The job is in progress. + """ + + COMPLETED = "completed" + IN_PROGRESS = "in_progress" + + class JobDetails(BaseModel): """A class to translate the payload. Attributes: labels: The labels of the job. - run_url: The URL of the job. + job_url: The URL of the job. """ labels: list[str] - run_url: HttpUrl + job_url: HttpUrl + + @validator("job_url") + @classmethod + def check_job_url_path_is_not_empty(cls, v: HttpUrl) -> HttpUrl: + """Check that the job_url path is not empty. + + Args: + v: The job_url to check. + + Returns: + The job_url if it is valid. + + Raises: + ValueError: If the job_url path is empty. + """ + if not v.path: + raise ValueError("path must be provided") + return v class JobError(Exception): """Raised when a job error occurs.""" -def consume(mongodb_uri: str, queue_name: str) -> None: +def consume( + queue_config: QueueConfig, runner_manager: RunnerManager, github_client: GithubClient +) -> None: """Consume a job from the message queue. Log the job details and acknowledge the message. If the job details are invalid, reject the message and raise an error. Args: - mongodb_uri: The URI of the MongoDB database. - queue_name: The name of the queue. + queue_config: The configuration for the message queue. + runner_manager: The runner manager used to create the runner. + github_client: The GitHub client to use to check the job status. Raises: JobError: If the job details are invalid. """ - with Connection(mongodb_uri) as conn: - with closing(SimpleQueue(conn, queue_name)) as simple_queue: + with Connection(queue_config.mongodb_uri) as conn: + with closing(SimpleQueue(conn, queue_config.queue_name)) as simple_queue: with signal_handler(signal.SIGTERM): msg = simple_queue.get(block=True) try: @@ -56,11 +96,75 @@ def consume(mongodb_uri: str, queue_name: str) -> None: msg.reject(requeue=True) raise JobError(f"Invalid job details: {msg.payload}") from exc logger.info( - "Received job with labels %s and run_url %s", + "Received job with labels %s and job_url %s", job_details.labels, - job_details.run_url, + job_details.job_url, ) - msg.ack() + _spawn_runner( + runner_manager=runner_manager, + job_url=job_details.job_url, + msg=msg, + github_client=github_client, + ) + + +def _spawn_runner( + runner_manager: RunnerManager, job_url: HttpUrl, msg: Message, github_client: GithubClient +) -> None: + """Spawn a runner. + + A runner is only spawned if the job has not yet been picked up by a runner. + After spawning a runner, it is checked if the job has been picked up. + + If the job has been picked up, the message is acknowledged. + If the job has not been picked up after 5 minutes, the message is rejected and requeued. + + Args: + runner_manager: The runner manager to use. + job_url: The URL of the job. + msg: The message to acknowledge or reject. + github_client: The GitHub client to use to check the job status. + """ + if _check_job_been_picked_up(job_url=job_url, github_client=github_client): + msg.ack() + return + runner_manager.create_runners(1) + for _ in range(10): + if _check_job_been_picked_up(job_url=job_url, github_client=github_client): + msg.ack() + break + sleep(30) + else: + msg.reject(requeue=True) + + +def _check_job_been_picked_up(job_url: HttpUrl, github_client: GithubClient) -> bool: + """Check if the job has already been picked up. + + Args: + job_url: The URL of the job. + github_client: The GitHub client to use to check the job status. + + Returns: + True if the job has been picked up, False otherwise. + """ + # job_url has the format: + # "https://api.github.com/repos/cbartz/gh-runner-test/actions/jobs/22428484402" + path = job_url.path + # we know that path is not empty as it is validated by the JobDetails model + job_url_path_parts = path.split("/") # type: ignore + job_id = job_url_path_parts[-1] + owner = job_url_path_parts[2] + repo = job_url_path_parts[3] + logging.debug( + "Parsed job_id: %s, owner: %s, repo: %s from job_url path %s", job_id, owner, repo, path + ) + + # See response format: + # https://docs.github.com/en/rest/actions/workflow-jobs?apiVersion=2022-11-28#get-a-job-for-a-workflow-run + + job_info = github_client.get_job_info(path=GitHubRepo(owner=owner, repo=repo), job_id=job_id) + return job_info.status in [*JobPickedUpStates] @contextlib.contextmanager diff --git a/src/github_runner_manager/reactive/runner.py b/src/github_runner_manager/reactive/runner.py index 415ce6d..9358f28 100644 --- a/src/github_runner_manager/reactive/runner.py +++ b/src/github_runner_manager/reactive/runner.py @@ -7,8 +7,12 @@ import os import sys +from github_runner_manager.github_client import GithubClient +from github_runner_manager.manager.runner_manager import RunnerManager +from github_runner_manager.openstack_cloud.openstack_runner_manager import OpenStackRunnerManager from github_runner_manager.reactive.consumer import consume -from github_runner_manager.reactive.runner_manager import MQ_URI_ENV_VAR, QUEUE_NAME_ENV_VAR +from github_runner_manager.reactive.runner_manager import RUNNER_CONFIG_ENV_VAR +from github_runner_manager.reactive.types_ import RunnerConfig def setup_root_logging() -> None: @@ -27,23 +31,25 @@ def main() -> None: Raises: ValueError: If the required environment variables are not set """ - mq_uri = os.environ.get(MQ_URI_ENV_VAR) - queue_name = os.environ.get(QUEUE_NAME_ENV_VAR) + runner_config_str = os.environ.get(RUNNER_CONFIG_ENV_VAR) - if not mq_uri: + if not runner_config_str: raise ValueError( - f"Missing {MQ_URI_ENV_VAR} environment variable. " + f"Missing {RUNNER_CONFIG_ENV_VAR} environment variable. " "Please set it to the message queue URI." ) - if not queue_name: - raise ValueError( - f"Missing {QUEUE_NAME_ENV_VAR} environment variable. " - "Please set it to the name of the queue." - ) + runner_config = RunnerConfig.parse_raw(runner_config_str) setup_root_logging() - consume(mq_uri, queue_name) + queue_config = runner_config.queue + openstack_runner_manager = OpenStackRunnerManager(config=runner_config.cloud_runner_manager) + runner_manager = RunnerManager( + cloud_runner_manager=openstack_runner_manager, + config=runner_config.runner_manager, + ) + github_client = GithubClient(token=runner_config.github_token) + consume(queue_config=queue_config, runner_manager=runner_manager, github_client=github_client) if __name__ == "__main__": diff --git a/src/github_runner_manager/reactive/runner_manager.py b/src/github_runner_manager/reactive/runner_manager.py index fd5fddc..051a081 100644 --- a/src/github_runner_manager/reactive/runner_manager.py +++ b/src/github_runner_manager/reactive/runner_manager.py @@ -11,12 +11,11 @@ import subprocess # nosec from pathlib import Path +from github_runner_manager.reactive.types_ import RunnerConfig from github_runner_manager.utilities import secure_run_subprocess logger = logging.getLogger(__name__) -MQ_URI_ENV_VAR = "MQ_URI" -QUEUE_NAME_ENV_VAR = "QUEUE_NAME" REACTIVE_RUNNER_LOG_DIR = Path("/var/log/reactive_runner") PYTHON_BIN = "/usr/bin/python3" @@ -31,19 +30,19 @@ "--sort=-start_time", ] UBUNTU_USER = "ubuntu" +RUNNER_CONFIG_ENV_VAR = "RUNNER_CONFIG" class ReactiveRunnerError(Exception): """Raised when a reactive runner error occurs.""" -def reconcile(quantity: int, mq_uri: str, queue_name: str) -> int: +def reconcile(quantity: int, runner_config: RunnerConfig) -> int: """Spawn a runner reactively. Args: quantity: The number of runners to spawn. - mq_uri: The message queue URI. - queue_name: The name of the queue. + runner_config: The reactive runner configuration. Raises a ReactiveRunnerError if the runner fails to spawn. @@ -58,7 +57,7 @@ def reconcile(quantity: int, mq_uri: str, queue_name: str) -> int: logger.info("Will spawn %d new reactive runner process(es)", delta) _setup_logging_for_processes() for _ in range(delta): - _spawn_runner(mq_uri=mq_uri, queue_name=queue_name) + _spawn_runner(runner_config) elif delta < 0: logger.info("Will kill %d process(es).", -delta) for pid in pids[:-delta]: @@ -105,17 +104,15 @@ def _setup_logging_for_processes() -> None: shutil.chown(REACTIVE_RUNNER_LOG_DIR, user=UBUNTU_USER, group=UBUNTU_USER) -def _spawn_runner(mq_uri: str, queue_name: str) -> None: +def _spawn_runner(runner_config: RunnerConfig) -> None: """Spawn a runner. Args: - mq_uri: The message queue URI. - queue_name: The name of the queue. + runner_config: The runner configuration to pass to the spawned runner process. """ env = { "PYTHONPATH": os.environ["PYTHONPATH"], - MQ_URI_ENV_VAR: mq_uri, - QUEUE_NAME_ENV_VAR: queue_name, + RUNNER_CONFIG_ENV_VAR: runner_config.json(), } # We do not want to wait for the process to finish, so we do not use with statement. # We trust the command. diff --git a/src/github_runner_manager/reactive/types_.py b/src/github_runner_manager/reactive/types_.py new file mode 100644 index 0000000..47cb18b --- /dev/null +++ b/src/github_runner_manager/reactive/types_.py @@ -0,0 +1,39 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Module containing reactive scheduling related types.""" + +from pydantic import BaseModel, MongoDsn + +from github_runner_manager.manager.runner_manager import RunnerManagerConfig +from github_runner_manager.openstack_cloud.openstack_runner_manager import ( + OpenStackRunnerManagerConfig, +) + + +class QueueConfig(BaseModel): + """The configuration for the message queue. + + Attributes: + mongodb_uri: The URI of the MongoDB database. + queue_name: The name of the queue. + """ + + mongodb_uri: MongoDsn + queue_name: str + + +class RunnerConfig(BaseModel): + """The configuration for the reactive runner to spawn. + + Attributes: + queue: The queue configuration. + runner_manager: The runner manager configuration. + cloud_runner_manager: The OpenStack runner manager configuration. + github_token: str + """ + + queue: QueueConfig + runner_manager: RunnerManagerConfig + cloud_runner_manager: OpenStackRunnerManagerConfig + github_token: str diff --git a/src/github_runner_manager/types_/__init__.py b/src/github_runner_manager/types_/__init__.py index cd712da..70127e1 100644 --- a/src/github_runner_manager/types_/__init__.py +++ b/src/github_runner_manager/types_/__init__.py @@ -4,17 +4,7 @@ """Package containing modules with type definitions.""" from typing import Optional -from pydantic import AnyHttpUrl, BaseModel, Field, IPvAnyAddress, MongoDsn, validator - - -class ReactiveConfig(BaseModel): - """Represents the configuration for reactive scheduling. - - Attributes: - mq_uri: The URI of the MQ to use to spawn runners reactively. - """ - - mq_uri: MongoDsn +from pydantic import AnyHttpUrl, BaseModel, Field, IPvAnyAddress, validator class ProxyConfig(BaseModel): diff --git a/src/github_runner_manager/types_/github.py b/src/github_runner_manager/types_/github.py index 89196a7..d0198d8 100644 --- a/src/github_runner_manager/types_/github.py +++ b/src/github_runner_manager/types_/github.py @@ -149,7 +149,27 @@ class JobConclusion(str, Enum): TIMED_OUT = "timed_out" -class JobStats(BaseModel): +class JobStatus(str, Enum): + """Status of a job on GitHub. + + Attributes: + QUEUED: Represents a job that is queued. + IN_PROGRESS: Represents a job that is in progress. + COMPLETED: Represents a job that is completed. + WAITING: Represents a job that is waiting. + REQUESTED: Represents a job that is requested. + PENDING: Represents a job that is pending. + """ + + QUEUED = "queued" + IN_PROGRESS = "in_progress" + COMPLETED = "completed" + WAITING = "waiting" + REQUESTED = "requested" + PENDING = "pending" + + +class JobInfo(BaseModel): """Stats for a job on GitHub. Attributes: @@ -157,12 +177,14 @@ class JobStats(BaseModel): created_at: The time the job was created. started_at: The time the job was started. conclusion: The end result of a job. + status: The status of the job. """ job_id: int created_at: datetime started_at: datetime conclusion: Optional[JobConclusion] + status: JobStatus @dataclasses.dataclass diff --git a/tests/unit/metrics/test_github.py b/tests/unit/metrics/test_github.py index 7d13495..44ce86d 100644 --- a/tests/unit/metrics/test_github.py +++ b/tests/unit/metrics/test_github.py @@ -11,7 +11,7 @@ from github_runner_manager.github_client import GithubClient from github_runner_manager.metrics import github as github_metrics from github_runner_manager.metrics.runner import PreJobMetrics -from github_runner_manager.types_.github import JobConclusion, JobStats +from github_runner_manager.types_.github import JobConclusion, JobInfo, JobStatus @pytest.fixture(name="pre_job_metrics") @@ -38,11 +38,11 @@ def test_job(pre_job_metrics: PreJobMetrics): runner_name = secrets.token_hex(16) created_at = datetime(2021, 10, 1, 0, 0, 0, tzinfo=timezone.utc) started_at = created_at + timedelta(seconds=3600) - github_client.get_job_info.return_value = JobStats( + github_client.get_job_info_by_runner_name.return_value = JobInfo( created_at=created_at, started_at=started_at, - runner_name=runner_name, conclusion=JobConclusion.SUCCESS, + status=JobStatus.COMPLETED, job_id=randint(1, 1000), ) @@ -62,7 +62,7 @@ def test_job_job_not_found(pre_job_metrics: PreJobMetrics): """ github_client = MagicMock(spec=GithubClient) runner_name = secrets.token_hex(16) - github_client.get_job_info.side_effect = JobNotFoundError("Job not found") + github_client.get_job_info_by_runner_name.side_effect = JobNotFoundError("Job not found") with pytest.raises(GithubMetricsError): github_metrics.job( diff --git a/tests/unit/reactive/test_consumer.py b/tests/unit/reactive/test_consumer.py index c036b92..7450535 100644 --- a/tests/unit/reactive/test_consumer.py +++ b/tests/unit/reactive/test_consumer.py @@ -3,59 +3,135 @@ import secrets from contextlib import closing +from datetime import datetime, timezone +from queue import Empty +from random import randint +from unittest.mock import MagicMock import pytest from kombu import Connection, Message from github_runner_manager.reactive import consumer from github_runner_manager.reactive.consumer import JobError +from github_runner_manager.reactive.types_ import QueueConfig +from github_runner_manager.types_.github import JobConclusion, JobInfo, JobStatus IN_MEMORY_URI = "memory://" -FAKE_RUN_URL = "https://api.github.com/repos/fakeusergh-runner-test/actions/runs/8200803099" +FAKE_JOB_URL = "https://api.github.com/repos/fakeuser/gh-runner-test/actions/runs/8200803099" -def test_consume(caplog: pytest.LogCaptureFixture): +@pytest.fixture(name="queue_config") +def queue_config_fixture() -> QueueConfig: + """Return a QueueConfig object.""" + queue_name = secrets.token_hex(16) + + # we use construct to avoid pydantic validation as IN_MEMORY_URI is not a valid URL + return QueueConfig.construct(mongodb_uri=IN_MEMORY_URI, queue_name=queue_name) + + +@pytest.fixture(name="mock_sleep", autouse=True) +def mock_sleep_fixture(monkeypatch: pytest.MonkeyPatch) -> None: + """Mock the sleep function.""" + monkeypatch.setattr(consumer, "sleep", lambda _: None) + + +def test_consume(monkeypatch: pytest.MonkeyPatch, queue_config: QueueConfig): """ - arrange: A job placed in the message queue. - act: Call consume - assert: The job is logged. + arrange: A job placed in the message queue which has not yet been picked up. + act: Call consume. + assert: A runner is created and the message is acknowledged. """ - queue_name = secrets.token_hex(16) job_details = consumer.JobDetails( labels=[secrets.token_hex(16), secrets.token_hex(16)], - run_url=FAKE_RUN_URL, + job_url=FAKE_JOB_URL, + ) + _put_in_queue(job_details.json(), queue_config.queue_name) + + runner_manager_mock = MagicMock(spec=consumer.RunnerManager) + github_client_mock = MagicMock(spec=consumer.GithubClient) + github_client_mock.get_job_info.side_effect = [ + _create_job_info(JobStatus.QUEUED), + _create_job_info(JobStatus.IN_PROGRESS), + ] + + consumer.consume( + queue_config=queue_config, + runner_manager=runner_manager_mock, + github_client=github_client_mock, ) - _put_in_queue(job_details.json(), queue_name) - # we use construct to avoid pydantic validation as IN_MEMORY_URI is not a valid URL - consumer.consume(IN_MEMORY_URI, queue_name) - assert str(job_details.labels) in caplog.text - assert job_details.run_url in caplog.text + runner_manager_mock.create_runners.assert_called_once_with(1) + + # Ensure message has been acknowledged by assuming an Empty exception is raised + with pytest.raises(Empty): + _consume_from_queue(queue_config.queue_name) + + +def test_consume_reject_if_job_gets_not_picked_up( + monkeypatch: pytest.MonkeyPatch, queue_config: QueueConfig +): + """ + arrange: A job placed in the message queue which will not get picked up. + act: Call consume. + assert: The message is requeued. + """ + job_details = consumer.JobDetails( + labels=[secrets.token_hex(16), secrets.token_hex(16)], + job_url=FAKE_JOB_URL, + ) + _put_in_queue(job_details.json(), queue_config.queue_name) + + runner_manager_mock = MagicMock(spec=consumer.RunnerManager) + github_client_mock = MagicMock(spec=consumer.GithubClient) + github_client_mock.get_job_info.return_value = _create_job_info(JobStatus.QUEUED) + + consumer.consume( + queue_config=queue_config, + runner_manager=runner_manager_mock, + github_client=github_client_mock, + ) + + # Ensure message has been requeued by reconsuming it + msg = _consume_from_queue(queue_config.queue_name) + assert msg.payload == job_details.json() @pytest.mark.parametrize( "job_str", [ pytest.param( - '{"labels": ["label1", "label2"], "status": "completed"}', id="run_url missing" + '{"labels": ["label1", "label2"], "status": "completed"}', id="job_url missing" + ), + pytest.param( + '{"status": "completed", "job_url": "https://example.com/path"}', id="labels missing" ), pytest.param( - '{"status": "completed", "run_url": "https://example.com"}', id="labels missing" + '{"labels": ["label1", "label2"], "status": "completed", ' + '"job_url": "https://example.com"}', + id="job_url without path", ), pytest.param("no json at all", id="invalid json"), ], ) -def test_job_details_validation_error(job_str: str): +def test_job_details_validation_error(job_str: str, queue_config: QueueConfig): """ arrange: A job placed in the message queue with invalid details. act: Call consume assert: A JobError is raised and the message is requeued. """ - queue_name = secrets.token_hex(16) + queue_name = queue_config.queue_name _put_in_queue(job_str, queue_name) + runner_manager_mock = MagicMock(spec=consumer.RunnerManager) + github_client_mock = MagicMock(spec=consumer.GithubClient) + github_client_mock.get_job_info.return_value = _create_job_info(JobStatus.IN_PROGRESS) + with pytest.raises(JobError) as exc_info: - consumer.consume(IN_MEMORY_URI, queue_name) + consumer.consume( + queue_config=queue_config, + runner_manager=runner_manager_mock, + github_client=github_client_mock, + ) assert "Invalid job details" in str(exc_info.value) # Ensure message has been requeued by reconsuming it @@ -63,6 +139,24 @@ def test_job_details_validation_error(job_str: str): assert msg.payload == job_str +def _create_job_info(status: JobStatus) -> JobInfo: + """Create a JobInfo object with the given status. + + Args: + status: The status of the job. + + Returns: + The JobInfo object. + """ + return JobInfo( + created_at=datetime(2021, 10, 1, 0, 0, 0, tzinfo=timezone.utc), + started_at=datetime(2021, 10, 1, 1, 0, 0, tzinfo=timezone.utc), + conclusion=JobConclusion.SUCCESS, + status=status, + job_id=randint(1, 1000), + ) + + def _put_in_queue(msg: str, queue_name: str) -> None: """Put a job in the message queue. diff --git a/tests/unit/reactive/test_runner_manager.py b/tests/unit/reactive/test_runner_manager.py index b6de746..714523b 100644 --- a/tests/unit/reactive/test_runner_manager.py +++ b/tests/unit/reactive/test_runner_manager.py @@ -16,6 +16,7 @@ ReactiveRunnerError, reconcile, ) +from github_runner_manager.reactive.types_ import QueueConfig, RunnerConfig from github_runner_manager.utilities import secure_run_subprocess EXAMPLE_MQ_URI = "http://example.com" @@ -63,18 +64,30 @@ def subprocess_popen_mock_fixture(monkeypatch: pytest.MonkeyPatch) -> MagicMock: return subprocess_popen_mock +@pytest.fixture(name="runner_config") +def runner_config_fixture() -> RunnerConfig: + """Return a RunnerConfig object.""" + queue_name = secrets.token_hex(16) + + # we use construct to avoid pydantic validation as IN_MEMORY_URI is not a valid URL + queue_config = QueueConfig.construct(mongodb_uri=EXAMPLE_MQ_URI, queue_name=queue_name) + return RunnerConfig.construct(queue=queue_config) + + def test_reconcile_spawns_runners( - secure_run_subprocess_mock: MagicMock, subprocess_popen_mock: MagicMock, log_dir: Path + secure_run_subprocess_mock: MagicMock, + subprocess_popen_mock: MagicMock, + log_dir: Path, + runner_config: RunnerConfig, ): """ arrange: Mock that two reactive runner processes are active. act: Call reconcile with a quantity of 5. assert: Three runners are spawned. Log file is setup. """ - queue_name = secrets.token_hex(16) _arrange_reactive_processes(secure_run_subprocess_mock, count=2) - delta = reconcile(5, mq_uri=EXAMPLE_MQ_URI, queue_name=queue_name) + delta = reconcile(5, runner_config=runner_config) assert delta == 3 assert subprocess_popen_mock.call_count == 3 @@ -82,17 +95,18 @@ def test_reconcile_spawns_runners( def test_reconcile_does_not_spawn_runners( - secure_run_subprocess_mock: MagicMock, subprocess_popen_mock: MagicMock + secure_run_subprocess_mock: MagicMock, + subprocess_popen_mock: MagicMock, + runner_config: RunnerConfig, ): """ arrange: Mock that two reactive runner processes are active. act: Call reconcile with a quantity of 2. assert: No runners are spawned. """ - queue_name = secrets.token_hex(16) _arrange_reactive_processes(secure_run_subprocess_mock, count=2) - delta = reconcile(2, mq_uri=EXAMPLE_MQ_URI, queue_name=queue_name) + delta = reconcile(2, runner_config=runner_config) assert delta == 0 assert subprocess_popen_mock.call_count == 0 @@ -102,15 +116,15 @@ def test_reconcile_kills_processes_for_too_many_processes( secure_run_subprocess_mock: MagicMock, subprocess_popen_mock: MagicMock, os_kill_mock: MagicMock, + runner_config: RunnerConfig, ): """ arrange: Mock that 3 reactive runner processes are active. act: Call reconcile with a quantity of 1. assert: 2 processes are killed. """ - queue_name = secrets.token_hex(16) _arrange_reactive_processes(secure_run_subprocess_mock, count=3) - delta = reconcile(1, mq_uri=EXAMPLE_MQ_URI, queue_name=queue_name) + delta = reconcile(1, runner_config=runner_config) assert delta == -2 assert subprocess_popen_mock.call_count == 0 @@ -121,16 +135,16 @@ def test_reconcile_ignore_process_not_found_on_kill( secure_run_subprocess_mock: MagicMock, subprocess_popen_mock: MagicMock, os_kill_mock: MagicMock, + runner_config: RunnerConfig, ): """ arrange: Mock 3 reactive processes and os.kill to fail once with a ProcessLookupError. act: Call reconcile with a quantity of 1. assert: The returned delta is still -2. """ - queue_name = secrets.token_hex(16) _arrange_reactive_processes(secure_run_subprocess_mock, count=3) os_kill_mock.side_effect = [None, ProcessLookupError] - delta = reconcile(1, mq_uri=EXAMPLE_MQ_URI, queue_name=queue_name) + delta = reconcile(1, runner_config=runner_config) assert delta == -2 assert subprocess_popen_mock.call_count == 0 @@ -138,14 +152,13 @@ def test_reconcile_ignore_process_not_found_on_kill( def test_reconcile_raises_reactive_runner_error_on_ps_failure( - secure_run_subprocess_mock: MagicMock, + secure_run_subprocess_mock: MagicMock, runner_config: RunnerConfig ): """ arrange: Mock that the ps command fails. act: Call reconcile with a quantity of 1. assert: A ReactiveRunnerError is raised. """ - queue_name = secrets.token_hex(16) secure_run_subprocess_mock.return_value = CompletedProcess( args=PIDS_COMMAND_LINE, returncode=1, @@ -154,7 +167,7 @@ def test_reconcile_raises_reactive_runner_error_on_ps_failure( ) with pytest.raises(ReactiveRunnerError) as err: - reconcile(1, mq_uri=EXAMPLE_MQ_URI, queue_name=queue_name) + reconcile(1, runner_config=runner_config) assert "Failed to get list of processes" in str(err.value) diff --git a/tests/unit/test_github_client.py b/tests/unit/test_github_client.py index 2cea912..e184cfb 100644 --- a/tests/unit/test_github_client.py +++ b/tests/unit/test_github_client.py @@ -1,6 +1,7 @@ # Copyright 2024 Canonical Ltd. # See LICENSE file for licensing details. import http +import json import random import secrets from collections import namedtuple @@ -12,13 +13,15 @@ from github_runner_manager.errors import JobNotFoundError from github_runner_manager.github_client import GithubClient -from github_runner_manager.types_.github import GitHubRepo, JobConclusion, JobStats +from github_runner_manager.types_.github import GitHubRepo, JobConclusion, JobInfo, JobStatus JobStatsRawData = namedtuple( "JobStatsRawData", - ["created_at", "started_at", "runner_name", "conclusion", "id"], + ["created_at", "started_at", "runner_name", "conclusion", "id", "status"], ) +TEST_URLLIB_RESPONSE_JSON = {"test": "test"} + @pytest.fixture(name="job_stats_raw") def job_stats_fixture() -> JobStatsRawData: @@ -28,13 +31,24 @@ def job_stats_fixture() -> JobStatsRawData: created_at="2021-10-01T00:00:00Z", started_at="2021-10-01T01:00:00Z", conclusion="success", + status="completed", runner_name=runner_name, id=random.randint(1, 1000), ) +@pytest.fixture(name="urllib_urlopen_mock") +def urllib_open_mock_fixture(monkeypatch: pytest.MonkeyPatch) -> MagicMock: + """Mock the urllib.request.urlopen function.""" + urllib_open_mock = MagicMock() + monkeypatch.setattr("urllib.request.urlopen", urllib_open_mock) + return urllib_open_mock + + @pytest.fixture(name="github_client") -def github_client_fixture(job_stats_raw: JobStatsRawData) -> GithubClient: +def github_client_fixture( + job_stats_raw: JobStatsRawData, urllib_urlopen_mock: MagicMock +) -> GithubClient: """Create a GithubClient object with a mocked GhApi object.""" gh_client = GithubClient("token") gh_client._client = MagicMock() @@ -45,10 +59,14 @@ def github_client_fixture(job_stats_raw: JobStatsRawData) -> GithubClient: "started_at": job_stats_raw.started_at, "runner_name": job_stats_raw.runner_name, "conclusion": job_stats_raw.conclusion, + "status": job_stats_raw.status, "id": job_stats_raw.id, } ] } + urllib_urlopen_mock.return_value.__enter__.return_value.read.return_value = json.dumps( + TEST_URLLIB_RESPONSE_JSON + ).encode("utf-8") return gh_client @@ -78,6 +96,7 @@ def _mock_multiple_pages_for_job_response( "started_at": job_stats_raw.started_at, "runner_name": runner_names[i * no_of_jobs_per_page + j], "conclusion": job_stats_raw.conclusion, + "status": job_stats_raw.status, "id": job_stats_raw.id, } for j in range(no_of_jobs_per_page) @@ -87,33 +106,36 @@ def _mock_multiple_pages_for_job_response( ] + [{"jobs": []}] -def test_get_job_info(github_client: GithubClient, job_stats_raw: JobStatsRawData): +def test_get_job_info_by_runner_name(github_client: GithubClient, job_stats_raw: JobStatsRawData): """ arrange: A mocked Github Client that returns one page of jobs containing one job \ with the runner. - act: Call get_job_info. + act: Call get_job_info_by_runner_name. assert: The correct JobStats object is returned. """ github_repo = GitHubRepo(owner=secrets.token_hex(16), repo=secrets.token_hex(16)) - job_stats = github_client.get_job_info( + job_stats = github_client.get_job_info_by_runner_name( path=github_repo, workflow_run_id=secrets.token_hex(16), runner_name=job_stats_raw.runner_name, ) - assert job_stats == JobStats( + assert job_stats == JobInfo( created_at=datetime(2021, 10, 1, 0, 0, 0, tzinfo=timezone.utc), started_at=datetime(2021, 10, 1, 1, 0, 0, tzinfo=timezone.utc), runner_name=job_stats_raw.runner_name, conclusion=JobConclusion.SUCCESS, + status=JobStatus.COMPLETED, job_id=job_stats_raw.id, ) -def test_get_job_info_no_conclusion(github_client: GithubClient, job_stats_raw: JobStatsRawData): +def test_get_job_info_by_runner_name_no_conclusion( + github_client: GithubClient, job_stats_raw: JobStatsRawData +): """ arrange: A mocked Github Client that returns one page of jobs containing one job \ with the runner with conclusion set to None. - act: Call get_job_info. + act: Call get_job_info_by_runner_name. assert: JobStats object with conclusion set to None is returned. """ github_client._client.actions.list_jobs_for_workflow_run.return_value = { @@ -123,21 +145,49 @@ def test_get_job_info_no_conclusion(github_client: GithubClient, job_stats_raw: "started_at": job_stats_raw.started_at, "runner_name": job_stats_raw.runner_name, "conclusion": None, + "status": job_stats_raw.status, "id": job_stats_raw.id, } ] } github_repo = GitHubRepo(owner=secrets.token_hex(16), repo=secrets.token_hex(16)) - job_stats = github_client.get_job_info( + job_stats = github_client.get_job_info_by_runner_name( path=github_repo, workflow_run_id=secrets.token_hex(16), runner_name=job_stats_raw.runner_name, ) - assert job_stats == JobStats( + assert job_stats == JobInfo( created_at=datetime(2021, 10, 1, 0, 0, 0, tzinfo=timezone.utc), started_at=datetime(2021, 10, 1, 1, 0, 0, tzinfo=timezone.utc), runner_name=job_stats_raw.runner_name, conclusion=None, + status=JobStatus.COMPLETED, + job_id=job_stats_raw.id, + ) + + +def test_get_job_info(github_client: GithubClient, job_stats_raw: JobStatsRawData): + """ + arrange: A mocked Github Client that returns a response. + act: Call get_job_info. + assert: The response is returned. + """ + github_client._client.actions.get_job_for_workflow_run.return_value = { + "created_at": job_stats_raw.created_at, + "started_at": job_stats_raw.started_at, + "runner_name": job_stats_raw.runner_name, + "conclusion": job_stats_raw.conclusion, + "status": job_stats_raw.status, + "id": job_stats_raw.id, + } + github_repo = GitHubRepo(owner=secrets.token_hex(16), repo=secrets.token_hex(16)) + job_stats = github_client.get_job_info(path=github_repo, job_id=job_stats_raw.id) + assert job_stats == JobInfo( + created_at=datetime(2021, 10, 1, 0, 0, 0, tzinfo=timezone.utc), + started_at=datetime(2021, 10, 1, 1, 0, 0, tzinfo=timezone.utc), + runner_name=job_stats_raw.runner_name, + conclusion=JobConclusion.SUCCESS, + status=JobStatus.COMPLETED, job_id=job_stats_raw.id, ) @@ -156,16 +206,17 @@ def test_github_api_pagination_multiple_pages( ) github_repo = GitHubRepo(owner=secrets.token_hex(16), repo=secrets.token_hex(16)) - job_stats = github_client.get_job_info( + job_stats = github_client.get_job_info_by_runner_name( path=github_repo, workflow_run_id=secrets.token_hex(16), runner_name=job_stats_raw.runner_name, ) - assert job_stats == JobStats( + assert job_stats == JobInfo( created_at=datetime(2021, 10, 1, 0, 0, 0, tzinfo=timezone.utc), started_at=datetime(2021, 10, 1, 1, 0, 0, tzinfo=timezone.utc), runner_name=job_stats_raw.runner_name, conclusion=JobConclusion.SUCCESS, + status=JobStatus.COMPLETED, job_id=job_stats_raw.id, ) @@ -186,7 +237,7 @@ def test_github_api_pagination_job_not_found( github_repo = GitHubRepo(owner=secrets.token_hex(16), repo=secrets.token_hex(16)) with pytest.raises(JobNotFoundError): - github_client.get_job_info( + github_client.get_job_info_by_runner_name( path=github_repo, workflow_run_id=secrets.token_hex(16), runner_name=job_stats_raw.runner_name, @@ -200,7 +251,7 @@ def test_github_api_http_error(github_client: GithubClient, job_stats_raw: JobSt github_repo = GitHubRepo(owner=secrets.token_hex(16), repo=secrets.token_hex(16)) with pytest.raises(JobNotFoundError): - github_client.get_job_info( + github_client.get_job_info_by_runner_name( path=github_repo, workflow_run_id=secrets.token_hex(16), runner_name=job_stats_raw.runner_name, diff --git a/tests/unit/test_openstack_runner_manager.py b/tests/unit/test_openstack_runner_manager.py index 9550dc1..c16ea62 100644 --- a/tests/unit/test_openstack_runner_manager.py +++ b/tests/unit/test_openstack_runner_manager.py @@ -16,8 +16,8 @@ @pytest.fixture(scope="function", name="mock_openstack_runner_manager") def mock_openstack_runner_manager_fixture(): """The mocked OpenStackRunnerManager instance.""" - return openstack_runner_manager.OpenStackRunnerManager( - manager_name="mock-manager", + config = openstack_runner_manager.OpenStackRunnerManagerConfig( + name="mock-manager", prefix="mock-manager", cloud_config=openstack_runner_manager.OpenStackCloudConfig( clouds_config={ @@ -49,6 +49,7 @@ def mock_openstack_runner_manager_fixture(): repo_policy_compliance=None, ), ) + return openstack_runner_manager.OpenStackRunnerManager(config=config) @pytest.mark.parametrize( diff --git a/tests/unit/test_runner_scaler.py b/tests/unit/test_runner_scaler.py index 1ebaffc..d480ce2 100644 --- a/tests/unit/test_runner_scaler.py +++ b/tests/unit/test_runner_scaler.py @@ -83,8 +83,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