chore: update metrics classes issuance logic [DO NOT MERGE WITHOUT CHANGING BASE TO MAIN]

github-runner-manager/src/github_runner_manager/manager/runner_manager.py

@@ -13,13 +13,8 @@
 
 from github_runner_manager import constants
 from github_runner_manager.errors import GithubMetricsError, RunnerError
-from github_runner_manager.manager.cloud_runner_manager import (
-    CloudRunnerInstance,
-    CloudRunnerManager,
-    CloudRunnerState,
-    HealthState,
-)
 from github_runner_manager.manager.models import InstanceID, RunnerIdentity, RunnerMetadata
+from github_runner_manager.manager.vm_manager import VM, CloudRunnerManager, HealthState, VMState
 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
@@ -76,14 +71,25 @@ class RunnerInstance:
     name: str
     instance_id: InstanceID
     metadata: RunnerMetadata
-    health: HealthState
     platform_state: PlatformRunnerState | None
-    cloud_state: CloudRunnerState
+    cloud_state: VMState
+
+    @property
+    def health(self) -> HealthState:
+        """Overall health state of the runner instance."""
+        if not self.platform_state:
+            return HealthState.UNHEALTHY
+        if self.platform_state == (
+            PlatformRunnerState.BUSY,
+            PlatformRunnerState.IDLE,
+        ):
+            return HealthState.HEALTHY
+        return HealthState.UNKNOWN
 
     @classmethod
     def from_cloud_and_platform_health(
         cls,
-        cloud_instance: CloudRunnerInstance,
+        cloud_instance: VM,
         platform_health_state: PlatformRunnerHealth | None,
     ) -> "RunnerInstance":
         """Construct an instance.
@@ -96,10 +102,9 @@ def from_cloud_and_platform_health(
             The RunnerInstance instantiated from cloud instance and platform state.
         """
         return cls(
-            name=cloud_instance.name,
+            name=cloud_instance.instance_id.name,
             instance_id=cloud_instance.instance_id,
             metadata=cloud_instance.metadata,
-            health=cloud_instance.health,
             platform_state=(
                 PlatformRunnerState.from_platform_health(platform_health_state)
                 if platform_health_state is not None
@@ -178,31 +183,18 @@ def get_runners(self) -> tuple[RunnerInstance, ...]:
             Information on the runners.
         """
         logger.debug("runner_manager::get_runners")
-        runner_instances = []
         cloud_runners = self._cloud.get_runners()
         runners_health_response = self._platform.get_runners_health(cloud_runners)
         logger.info("clouds runners response %s", cloud_runners)
         logger.info("runner health response %s", runners_health_response)
         runners_health = runners_health_response.requested_runners
         health_runners_map = {runner.identity.instance_id: runner for runner in runners_health}
-        for cloud_runner in cloud_runners:
-            if cloud_runner.instance_id not in health_runners_map:
-                runner_instance = RunnerInstance.from_cloud_and_platform_health(cloud_runner, None)
-                runner_instance.health = HealthState.UNKNOWN
-                runner_instances.append(runner_instance)
-                continue
-            health_runner = health_runners_map[cloud_runner.instance_id]
-            if health_runner.deletable:
-                cloud_runner.health = HealthState.UNHEALTHY
-            elif health_runner.online:
-                cloud_runner.health = HealthState.HEALTHY
-            else:
-                cloud_runner.health = HealthState.UNHEALTHY
-            runner_instance = RunnerInstance.from_cloud_and_platform_health(
-                cloud_runner, health_runner
+        return tuple(
+            RunnerInstance.from_cloud_and_platform_health(
+                cloud_runner, health_runners_map.get(cloud_runner.instance_id, None)
             )
-            runner_instances.append(runner_instance)
-        return cast(tuple[RunnerInstance], tuple(runner_instances))
+            for cloud_runner in cloud_runners
+        )
 
     def delete_runners(self, num: int) -> IssuedMetricEventsStats:
         """Delete runners.
@@ -317,7 +309,7 @@ def _cleanup_resources(
 
     def _delete_cloud_runners(
         self,
-        cloud_runners: Sequence[CloudRunnerInstance],
+        cloud_runners: Sequence[VM],
         runners_health: Sequence[PlatformRunnerHealth],
         delete_busy_runners: bool = False,
     ) -> Iterable[runner_metrics.RunnerMetrics]:
@@ -551,7 +543,7 @@ def _create_runner(args: _CreateRunnerArgs) -> InstanceID:
 
 
 def _filter_runner_to_delete(
-    cloud_runner: CloudRunnerInstance,
+    cloud_runner: VM,
     health: PlatformRunnerHealth | None,
     *,
     clean_idle: bool = False,
@@ -598,7 +590,7 @@ def _filter_runner_to_delete(
 
 
 def _runner_deletion_sort_key(
-    health_runners_map: dict[InstanceID, PlatformRunnerHealth], cloud_runner: CloudRunnerInstance
+    health_runners_map: dict[InstanceID, PlatformRunnerHealth], cloud_runner: VM
 ) -> int:
     """Order the runners in accordance to how inconvenient it is to delete them.
 

github-runner-manager/src/github_runner_manager/manager/runner_scaler.py

@@ -16,14 +16,14 @@
     MissingServerConfigError,
     ReconcileError,
 )
-from github_runner_manager.manager.cloud_runner_manager import HealthState
 from github_runner_manager.manager.runner_manager import (
     FlushMode,
     IssuedMetricEventsStats,
     RunnerInstance,
     RunnerManager,
     RunnerMetadata,
 )
+from github_runner_manager.manager.vm_manager import HealthState
 from github_runner_manager.metrics import events as metric_events
 from github_runner_manager.metrics.reconcile import (
     BUSY_RUNNERS_COUNT,

github-runner-manager/src/github_runner_manager/manager/cloud_runner_manager.py → github-runner-manager/src/github_runner_manager/manager/vm_manager.py

@@ -8,7 +8,7 @@
 from dataclasses import dataclass
 from datetime import datetime, timezone
 from enum import Enum, auto
-from typing import Optional, Sequence
+from typing import Optional, Protocol, Sequence
 
 from pydantic import BaseModel, Field, NonNegativeFloat
 
@@ -36,7 +36,7 @@ class HealthState(Enum):
     UNKNOWN = auto()
 
 
-class CloudRunnerState(str, Enum):
+class VMState(str, Enum):
     """Represent state of the instance hosting the runner.
 
     Attributes:
@@ -61,7 +61,7 @@ class CloudRunnerState(str, Enum):
     @staticmethod
     def from_openstack_server_status(  # pragma: no cover
         openstack_server_status: str,
-    ) -> "CloudRunnerState":
+    ) -> "VMState":
         """Create from openstack server status.
 
         The openstack server status are documented here:
@@ -73,72 +73,41 @@ def from_openstack_server_status(  # pragma: no cover
         Returns:
             The state of the runner.
         """
-        state = CloudRunnerState.UNEXPECTED
+        state = VMState.UNEXPECTED
         match openstack_server_status:
             case "BUILD":
-                state = CloudRunnerState.CREATED
+                state = VMState.CREATED
             case "REBUILD":
-                state = CloudRunnerState.CREATED
+                state = VMState.CREATED
             case "ACTIVE":
-                state = CloudRunnerState.ACTIVE
+                state = VMState.ACTIVE
             case "ERROR":
-                state = CloudRunnerState.ERROR
+                state = VMState.ERROR
             case "STOPPED":
-                state = CloudRunnerState.STOPPED
+                state = VMState.STOPPED
             case "DELETED":
-                state = CloudRunnerState.DELETED
+                state = VMState.DELETED
             case "UNKNOWN":
-                state = CloudRunnerState.UNKNOWN
+                state = VMState.UNKNOWN
             case _:
-                state = CloudRunnerState.UNEXPECTED
+                state = VMState.UNEXPECTED
         return state
 
 
-class CloudInitStatus(str, Enum):
-    """Represents the state of cloud-init script.
-
-    The cloud init script is used to launch ephemeral GitHub runners. If the script is being
-    initialized, GitHub runner is listening for jobs or GitHub runner is running the job, the
-    cloud-init script should report "running" status.
-
-    Refer to the official documentation on cloud-init status:
-    https://cloudinit.readthedocs.io/en/latest/howto/status.html.
-
-    Attributes:
-        NOT_STARTED: The cloud-init script has not yet been started.
-        RUNNING: The cloud-init script is running.
-        DONE: The cloud-init script has completed successfully.
-        ERROR: There was an error while running the cloud-init script.
-        DEGRADED: There was a non-critical issue while running the cloud-inits script.
-        DISABLED: Cloud init was disabled by other system configurations.
-    """
-
-    NOT_STARTED = "not started"
-    RUNNING = "running"
-    DONE = "done"
-    ERROR = "error"
-    DEGRADED = "degraded"
-    DISABLED = "disabled"
-
-
 @dataclass
-class CloudRunnerInstance:
+class VM:
     """Information on the runner on the cloud.
 
     Attributes:
-        name: Name of the instance hosting the runner.
-        instance_id: ID of the instance.
-        metadata: Metadata of the runner.
-        health: Health state of the runner.
-        state: State of the instance hosting the runner.
+        instance_id: VM instance identifier (NOT VM UUID).
+        metadata: Metadata associated with the VM.
+        state: The VM state.
         created_at: Creation time of the runner in the cloud provider.
     """
 
-    name: str
     instance_id: InstanceID
     metadata: RunnerMetadata
-    health: HealthState
-    state: CloudRunnerState
+    state: VMState
     created_at: datetime
 
     def is_older_than(self, seconds: float) -> bool:
@@ -210,25 +179,44 @@ class PostJobMetrics(BaseModel):
     status_info: Optional[CodeInformation]
 
 
-class RunnerMetrics(BaseModel):
+class RunnerMetrics(Protocol):
     """Metrics for a runner.
 
     Attributes:
-        instance_id: The name of the runner.
-        metadata: Runner metadata.
-        installation_start_timestamp: The UNIX time stamp of the time at which the runner
-            installation started.
-        installed_timestamp: The UNIX time stamp of the time at which the runner was installed.
+        metadata: The metadata of the VM in which the metrics are fetched from.
+        instance_id: The instance ID of the VM in which the metrics are fetched from.
+        installation_start_timestamp: The UNIX timestamp of in which the VM setup started.
+        installation_end_timestamp: The UNIX timestamp of in which the VM setup ended.
         pre_job: The metrics for the pre-job phase.
         post_job: The metrics for the post-job phase.
     """
 
-    instance_id: InstanceID
-    metadata: RunnerMetadata
-    installation_start_timestamp: NonNegativeFloat | None
-    installed_timestamp: NonNegativeFloat
-    pre_job: PreJobMetrics | None
-    post_job: PostJobMetrics | None
+    @property
+    def pre_job(self) -> PreJobMetrics | None:
+        """Metrics from the pre-job phase."""
+
+    @property
+    def post_job(self) -> PostJobMetrics | None:
+        """Metrics from the post-job phase."""
+
+    @property
+    # Ignore no return implementation because this is a protocol class.
+    def metadata(self) -> RunnerMetadata:  # type: ignore
+        """Metadata of the VM in which the metrics are fetche from."""
+
+    @property
+    # Ignore no return implementation because this is a protocol class.
+    def instance_id(self) -> InstanceID:  # type: ignore
+        """Instance ID of the VM in which the metrics are fetched from."""
+
+    @property
+    # Ignore no return implementation because this is a protocol class.
+    def installation_start_timestamp(self) -> NonNegativeFloat:  # type: ignore
+        """UNIX timestamp of in which the VM setup started."""
+
+    @property
+    def installation_end_timestamp(self) -> NonNegativeFloat | None:
+        """UNIX timestamp of in which the VM setup ended."""
 
 
 class CloudRunnerManager(abc.ABC):
@@ -248,7 +236,7 @@ def create_runner(
         self,
         runner_identity: RunnerIdentity,
         runner_context: RunnerContext,
-    ) -> CloudRunnerInstance:
+    ) -> VM:
         """Create a self-hosted runner.
 
         Args:
@@ -257,10 +245,11 @@ def create_runner(
         """
 
     @abc.abstractmethod
-    def get_runners(self) -> Sequence[CloudRunnerInstance]:
+    def get_runners(self) -> Sequence[VM]:
         """Get cloud self-hosted runners."""
 
     @abc.abstractmethod
+    # Abstract methods do not have a return value, ignore the docstring error DCO031
     def delete_vms(self, instance_ids: Sequence[InstanceID]) -> list[InstanceID]:
         """Delete cloud VM instances.
 
@@ -269,9 +258,10 @@ def delete_vms(self, instance_ids: Sequence[InstanceID]) -> list[InstanceID]:
 
         Returns:
             The deleted instance IDs.
-        """
+        """  # noqa: DCO031
 
     @abc.abstractmethod
+    # Abstract methods do not have a return value, ignore the docstring error DCO031
     def extract_metrics(self, instance_ids: Sequence[InstanceID]) -> list[RunnerMetrics]:
         """Extract metrics from cloud VMs.
 
@@ -280,7 +270,7 @@ def extract_metrics(self, instance_ids: Sequence[InstanceID]) -> list[RunnerMetr
 
         Returns:
             The fetched runner metrics.
-        """
+        """  # noqa: DCO031
 
     @abc.abstractmethod
     def cleanup(self) -> None:

github-runner-manager/src/github_runner_manager/metrics/events.py

@@ -9,7 +9,7 @@
 from pydantic import BaseModel, NonNegativeFloat
 
 from github_runner_manager.errors import IssueMetricEventError
-from github_runner_manager.manager.cloud_runner_manager import CodeInformation
+from github_runner_manager.manager.vm_manager import CodeInformation
 
 METRICS_LOG_PATH = Path("/var/log/github-runner-metrics.log")