Skip to content

Commit

Permalink
Merge 298633f into bbdbdc5
Browse files Browse the repository at this point in the history
  • Loading branch information
javierdelapuente authored Mar 3, 2025
2 parents bbdbdc5 + 298633f commit d06bfb6
Show file tree
Hide file tree
Showing 22 changed files with 564 additions and 633 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
GitHubRepo,
)
from github_runner_manager.errors import GithubApiError, JobNotFoundError, TokenError
from github_runner_manager.manager.models import InstanceID
from github_runner_manager.types_.github import (
JobInfo,
RegistrationToken,
Expand Down Expand Up @@ -161,7 +162,7 @@ def get_runner_remove_token(self, path: GitHubPath) -> str:

@catch_http_errors
def get_runner_registration_jittoken(
self, path: GitHubPath, instance_id: str, labels: list[str]
self, path: GitHubPath, instance_id: InstanceID, labels: list[str]
) -> str:
"""Get token from GitHub used for registering runners.
Expand All @@ -181,7 +182,7 @@ def get_runner_registration_jittoken(
token = self._client.actions.generate_runner_jitconfig_for_repo(
owner=path.owner,
repo=path.repo,
name=instance_id,
name=instance_id.name,
runner_group_id=1,
labels=labels,
)
Expand All @@ -191,7 +192,7 @@ def get_runner_registration_jittoken(
runner_group_id = self._get_runner_group_id(path)
token = self._client.actions.generate_runner_jitconfig_for_org(
org=path.org,
name=instance_id,
name=instance_id.name,
runner_group_id=runner_group_id,
labels=labels,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@
from enum import Enum, auto
from typing import Iterator, Sequence, Tuple

from github_runner_manager.manager.models import InstanceID
from github_runner_manager.metrics.runner import RunnerMetrics

logger = logging.getLogger(__name__)

InstanceId = str


class HealthState(Enum):
"""Health state of the runners.
Expand Down Expand Up @@ -141,7 +140,7 @@ class CloudRunnerInstance:
"""

name: str
instance_id: InstanceId
instance_id: InstanceID
health: HealthState
state: CloudRunnerState

Expand All @@ -159,11 +158,7 @@ def name_prefix(self) -> str:
"""Get the name prefix of the self-hosted runners."""

@abc.abstractmethod
def generate_instance_id(self) -> InstanceId:
"""Generate an intance_id to name a runner."""

@abc.abstractmethod
def create_runner(self, instance_id: InstanceId, registration_jittoken: str) -> None:
def create_runner(self, instance_id: InstanceID, registration_jittoken: str) -> None:
"""Create a self-hosted runner.
Args:
Expand All @@ -181,7 +176,7 @@ def get_runners(self, states: Sequence[CloudRunnerState]) -> Tuple[CloudRunnerIn
"""

@abc.abstractmethod
def delete_runner(self, instance_id: InstanceId, remove_token: str) -> RunnerMetrics | None:
def delete_runner(self, instance_id: InstanceID, remove_token: str) -> RunnerMetrics | None:
"""Delete self-hosted runner.
Args:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from github_runner_manager.configuration.github import GitHubConfiguration
from github_runner_manager.github_client import GithubClient
from github_runner_manager.manager.models import InstanceID
from github_runner_manager.types_.github import GitHubRunnerStatus, SelfHostedRunner


Expand Down Expand Up @@ -72,7 +73,11 @@ def get_runners(
Information on the runners.
"""
runner_list = self.github.get_runner_github_info(self._path)
runner_list = [runner for runner in runner_list if runner.name.startswith(self._prefix)]
runner_list = [
runner
for runner in runner_list
if InstanceID.name_has_prefix(self._prefix, runner.name)
]

if states is None:
return tuple(runner_list)
Expand All @@ -94,7 +99,7 @@ def delete_runners(self, states: Iterable[GitHubRunnerState] | None = None) -> N
for runner in runner_list:
self.github.delete_runner(self._path, runner.id)

def get_registration_jittoken(self, instance_id: str, labels: list[str]) -> str:
def get_registration_jittoken(self, instance_id: InstanceID, labels: list[str]) -> str:
"""Get registration JIT token from GitHub.
This token is used for registering self-hosted runners.
Expand All @@ -119,9 +124,7 @@ def get_removal_token(self) -> str:
return self.github.get_runner_remove_token(self._path)

@staticmethod
def _is_runner_in_state(
runner: SelfHostedRunner, states: set[GitHubRunnerState] | None
) -> bool:
def _is_runner_in_state(runner: SelfHostedRunner, states: set[GitHubRunnerState]) -> bool:
"""Check that the runner is in one of the states provided.
Args:
Expand All @@ -131,6 +134,4 @@ def _is_runner_in_state(
Returns:
True if the runner is in one of the state, else false.
"""
if states is None:
return True
return GitHubRunnerState.from_runner(runner) in states
136 changes: 136 additions & 0 deletions github-runner-manager/src/github_runner_manager/manager/models.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
# Copyright 2025 Canonical Ltd.
# See LICENSE file for licensing details.

"""TODO."""

import secrets
from dataclasses import dataclass

INSTANCE_SUFFIX_LENGTH = 12


@dataclass(eq=True, frozen=True)
class InstanceID:
"""TODO.
This class needs to add compatibility for all cloud providers and GitHub.
Attributes:
name: TODO
prefix: TODO
reactive: TODO
suffix: TODO
"""

prefix: str
reactive: bool
suffix: str

@property
def name(self) -> str:
"""TODO.
Returns:
TODO
"""
# Having a not number as a separator is good, as the prefix should end
# with a number (it is the unit number).
return f"{self.prefix}{'r' if self.reactive else '-'}{self.suffix}"

@classmethod
def build_from_name(cls, prefix: str, name: str) -> "InstanceID":
"""TODO.
Args:
prefix: TODO
name: TODO
Raises:
ValueError: TODO
Returns:
TODO
"""
if InstanceID.name_has_prefix(prefix, name):
suffix = name.removeprefix(f"{prefix}")
else:
# TODO should we raise if invalid name?
raise ValueError(f"Invalid runner name {name} for prefix {prefix}")

# The separator from prefix and suffix may indicate if the runner is reactive.
reactive = False
separator = suffix[:1]
if separator == "r":
reactive = True
suffix = suffix[1:]

return cls(
prefix=prefix,
reactive=reactive,
suffix=suffix,
)

@classmethod
def build(cls, prefix: str, reactive: bool = False) -> "InstanceID":
r"""Generate an intance_id to name a runner.
It should be valid for all the CloudProviders and GitHub.
The GitHub runner name convention is as following:
A valid runner name is 64 characters or less in length and does not include '"', '/', ':',
'<', '>', '\', '|', '*' and '?'.
The collision rate calculation:
alphanumeric 12 chars long (26 alphabet + 10 digits = 36)
36^12 is big enough for our use-case.
Args:
prefix: TODO
reactive: TODO
Returns:
Instance ID of the runner.
"""
suffix = secrets.token_hex(INSTANCE_SUFFIX_LENGTH // 2)
return cls(prefix=prefix, reactive=reactive, suffix=suffix)

@staticmethod
def name_has_prefix(prefix: str, name: str) -> bool:
"""TODO.
TODO THIS CHECKS THE DIFFERENCE BETWEEN
name-1-suffix
and
namd-11-suffix
that is a bug in many places now.
name-11 is not a name in the prefix name-11
Args:
prefix: TODO
name: TODO
Returns:
TODO
"""
if name.startswith(prefix):
suffix = name.removeprefix(f"{prefix}")
if suffix[:1] in ("-", "r"):
return True
return False

def __str__(self) -> str:
"""TODO.
Returns:
TODO.
"""
return self.name

def __repr__(self) -> str:
"""TODO.
Returns:
TODO.
"""
return f"InstanceID({self.name!r})"
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Copyright 2025 Canonical Ltd.
# See LICENSE file for licensing details.

"""Class for managing the GitHub self-hosted runners hosted on cloud instances."""
"""Module for managing the GitHub self-hosted runners hosted on cloud instances."""

import logging
from dataclasses import dataclass
Expand All @@ -17,12 +17,12 @@
CloudRunnerManager,
CloudRunnerState,
HealthState,
InstanceId,
)
from github_runner_manager.manager.github_runner_manager import (
GitHubRunnerManager,
GitHubRunnerState,
)
from github_runner_manager.manager.models import InstanceID
from github_runner_manager.metrics import events as metric_events
from github_runner_manager.metrics import github as github_metrics
from github_runner_manager.metrics import runner as runner_metrics
Expand Down Expand Up @@ -59,7 +59,7 @@ class RunnerInstance:
"""

name: str
instance_id: InstanceId
instance_id: InstanceID
health: HealthState
github_state: GitHubRunnerState | None
cloud_state: CloudRunnerState
Expand Down Expand Up @@ -112,11 +112,12 @@ def __init__(
)
self._labels = labels

def create_runners(self, num: int) -> tuple[InstanceId, ...]:
def create_runners(self, num: int, reactive: bool = False) -> tuple[InstanceID, ...]:
"""Create runners.
Args:
num: Number of runners to create.
reactive: TODO
Returns:
List of instance ID of the runners.
Expand All @@ -128,7 +129,8 @@ def create_runners(self, num: int) -> tuple[InstanceId, ...]:
# we have to add them manually.
labels += constants.GITHUB_DEFAULT_LABELS
create_runner_args = [
RunnerManager._CreateRunnerArgs(self._cloud, self._github, labels) for _ in range(num)
RunnerManager._CreateRunnerArgs(self._cloud, self._github, labels, reactive)
for _ in range(num)
]
return RunnerManager._spawn_runners(create_runner_args)

Expand Down Expand Up @@ -248,7 +250,7 @@ def cleanup(self) -> IssuedMetricEventsStats:
@staticmethod
def _spawn_runners(
create_runner_args_sequence: Sequence["RunnerManager._CreateRunnerArgs"],
) -> tuple[InstanceId, ...]:
) -> tuple[InstanceID, ...]:
"""Spawn runners in parallel using multiprocessing.
Multiprocessing is only used if there are more than one runner to spawn. Otherwise,
Expand Down Expand Up @@ -278,7 +280,7 @@ def _spawn_runners(
@staticmethod
def _spawn_runners_using_multiprocessing(
create_runner_args_sequence: Sequence["RunnerManager._CreateRunnerArgs"], num: int
) -> tuple[InstanceId, ...]:
) -> tuple[InstanceID, ...]:
"""Parallel spawn of runners.
The length of the create_runner_args is number _create_runner invocation, and therefore the
Expand Down Expand Up @@ -349,16 +351,17 @@ def _issue_runner_metrics(self, metrics: Iterator[RunnerMetrics]) -> IssuedMetri
job_metrics = github_metrics.job(
github_client=self._github.github,
pre_job_metrics=extracted_metrics.pre_job,
runner_name=extracted_metrics.runner_name,
runner_name=extracted_metrics.instance_id.name,
)
except GithubMetricsError:
logger.exception(
"Failed to calculate job metrics for %s", extracted_metrics.runner_name
"Failed to calculate job metrics for %s",
extracted_metrics.instance_id,
)
else:
logger.debug(
"No pre-job metrics found for %s, will not calculate job metrics.",
extracted_metrics.runner_name,
extracted_metrics.instance_id,
)

issued_events = runner_metrics.issue_events(
Expand All @@ -382,14 +385,16 @@ class _CreateRunnerArgs:
cloud_runner_manager: For managing the cloud instance of the runner.
github_runner_manager: To manage self-hosted runner on the GitHub side.
labels: List of labels to add to the runners.
reactive: TODO.
"""

cloud_runner_manager: CloudRunnerManager
github_runner_manager: GitHubRunnerManager
labels: list[str]
reactive: bool

@staticmethod
def _create_runner(args: _CreateRunnerArgs) -> InstanceId:
def _create_runner(args: _CreateRunnerArgs) -> InstanceID:
"""Create a single runner.
This is a staticmethod for usage with multiprocess.Pool.
Expand All @@ -400,7 +405,7 @@ def _create_runner(args: _CreateRunnerArgs) -> InstanceId:
Returns:
The instance ID of the runner created.
"""
instance_id = args.cloud_runner_manager.generate_instance_id()
instance_id = InstanceID.build(args.cloud_runner_manager.name_prefix, args.reactive)
registration_jittoken = args.github_runner_manager.get_registration_jittoken(
instance_id, args.labels
)
Expand Down
Loading

0 comments on commit d06bfb6

Please sign in to comment.