Skip to content

Commit

Permalink
a few bugs about that may include unit-11 in unit-1.
Browse files Browse the repository at this point in the history
  • Loading branch information
javierdelapuente committed Mar 3, 2025
1 parent 462482a commit 298633f
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand All @@ -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
32 changes: 27 additions & 5 deletions github-runner-manager/src/github_runner_manager/manager/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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
Expand Down

0 comments on commit 298633f

Please sign in to comment.