From 298633f517bc865f6c235e0d0456a30538943686 Mon Sep 17 00:00:00 2001 From: Javier de la Puente Date: Mon, 3 Mar 2025 13:54:28 +0100 Subject: [PATCH] a few bugs about that may include unit-11 in unit-1. --- .../manager/github_runner_manager.py | 12 +++---- .../github_runner_manager/manager/models.py | 32 ++++++++++++++++--- .../manager/runner_scaler.py | 4 +-- .../openstack_cloud/openstack_cloud.py | 18 ++++------- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/github-runner-manager/src/github_runner_manager/manager/github_runner_manager.py b/github-runner-manager/src/github_runner_manager/manager/github_runner_manager.py index b7b0fc0c..e0930a6b 100644 --- a/github-runner-manager/src/github_runner_manager/manager/github_runner_manager.py +++ b/github-runner-manager/src/github_runner_manager/manager/github_runner_manager.py @@ -73,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) @@ -120,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: @@ -132,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 diff --git a/github-runner-manager/src/github_runner_manager/manager/models.py b/github-runner-manager/src/github_runner_manager/manager/models.py index ca6cd500..5f8779c8 100644 --- a/github-runner-manager/src/github_runner_manager/manager/models.py +++ b/github-runner-manager/src/github_runner_manager/manager/models.py @@ -51,20 +51,17 @@ def build_from_name(cls, prefix: str, name: str) -> "InstanceID": Returns: TODO """ - if name.startswith(prefix): + 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 - elif separator == "-": - reactive = False - else: - raise ValueError(f"Invalid runner name {name} for prefix {prefix}") suffix = suffix[1:] return cls( @@ -97,6 +94,31 @@ def build(cls, prefix: str, reactive: bool = False) -> "InstanceID": 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. diff --git a/github-runner-manager/src/github_runner_manager/manager/runner_scaler.py b/github-runner-manager/src/github_runner_manager/manager/runner_scaler.py index 7e984471..f7790c9b 100644 --- a/github-runner-manager/src/github_runner_manager/manager/runner_scaler.py +++ b/github-runner-manager/src/github_runner_manager/manager/runner_scaler.py @@ -95,7 +95,7 @@ class _ReconcileMetricData: metric_stats: IssuedMetricEventsStats runner_list: tuple[RunnerInstance] flavor: str - expected_runner_quantity: int | None = None + expected_runner_quantity: int class RunnerScaler: @@ -261,7 +261,7 @@ def reconcile(self) -> int: metric_stats = {} start_timestamp = time.time() - expected_runner_quantity = None if self._reactive_config else self._base_quantity + expected_runner_quantity = self._base_quantity # TODO THE NEW STEPS MAY BE AS FOLLOWS: diff --git a/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_cloud.py b/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_cloud.py index 2ab239c3..0a4debf6 100644 --- a/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_cloud.py +++ b/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_cloud.py @@ -62,15 +62,7 @@ def __init__(self, server: OpenstackServer, prefix: str): Args: server: The OpenStack server. prefix: The name prefix for the servers. - - Raises: - ValueError: Provided server should not be managed under this prefix. """ - if not server.name.startswith(f"{prefix}-"): - # Should never happen. - raise ValueError( - f"Found openstack server {server.name} managed under prefix {prefix}, contact devs" - ) self.addresses = [ address["addr"] for network_addresses in server.addresses.values() @@ -374,7 +366,11 @@ def _cleanup_key_files(self, exclude_instances: Iterable[str]) -> None: deleted = 0 for path in self._ssh_key_dir.iterdir(): # Find key file from this application. - if path.is_file() and path.name.startswith(self.prefix) and path.name.endswith(".key"): + if ( + path.is_file() + and InstanceID.name_has_prefix(self.prefix, path.name) + and path.name.endswith(".key") + ): total += 1 if path in exclude_filename: continue @@ -396,7 +392,7 @@ def _cleanup_openstack_keypairs( keypairs = conn.list_keypairs() for key in keypairs: # The `name` attribute is of resource.Body type. - if key.name and str(key.name).startswith(self.prefix): + if key.name and InstanceID.name_has_prefix(self.prefix, key.name): if str(key.name) in exclude_instance_set: continue try: @@ -419,7 +415,7 @@ def _get_openstack_instances(self, conn: OpenstackConnection) -> tuple[Openstack return tuple( server for server in cast(list[OpenstackServer], conn.list_servers()) - if server.name.startswith(f"{self.prefix}-") + if InstanceID.name_has_prefix(self.prefix, server.name) ) @staticmethod