From 73e28b1fa381b5ffe74bddc73e2b9765f91627ec Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Wed, 12 Jul 2023 14:50:41 +0800
Subject: [PATCH 01/16] Remove unused scripts

---
 script/deploy_runner.sh          | 34 --------------
 script/remove_offline_runners.py | 76 --------------------------------
 script/remove_runner.sh          |  6 ---
 3 files changed, 116 deletions(-)
 delete mode 100644 script/deploy_runner.sh
 delete mode 100644 script/remove_offline_runners.py
 delete mode 100644 script/remove_runner.sh

diff --git a/script/deploy_runner.sh b/script/deploy_runner.sh
deleted file mode 100644
index 2a6f4215c..000000000
--- a/script/deploy_runner.sh
+++ /dev/null
@@ -1,34 +0,0 @@
-#!/usr/bin/env bash
-
-# Copyright 2023 Canonical Ltd.
-# See LICENSE file for licensing details.
-
-set -e
-
-rm -f github_runner.zip
-
-# Request a download URL for the artifact.
-echo "Requesting github runner charm download link..."
-DOWNLOAD_LOCATION=$(curl \
-  --head \
-  -H "Accept: application/vnd.github+json" \
-  -H "Authorization: Bearer ${GITHUB_TOKEN}"\
-  -H "X-GitHub-Api-Version: 2022-11-28" \
-  "https://api.github.com/repos/canonical/github-runner-operator/actions/artifacts/{$GITHUB_RUNNER_ARTIFACT_ID}/zip" \
-  | grep location)
-# Parse out the URL from the format "Location: URL\r".
-read -ra LOCATION_ARRAY <<< "$DOWNLOAD_LOCATION"
-URL=$(echo "${LOCATION_ARRAY[1]}" | tr -d '\r')
-
-# Download the github runner charm.
-echo "Downloading github runner charm..."
-curl -o github_runner.zip "$URL"
-
-# Decompress the zip.
-echo "Decompressing github runner charm..."
-unzip -p github_runner.zip > github-runner.charm
-rm github_runner.zip
-
-# Deploy the charm.
-juju deploy ./github-runner.charm --series=jammy --constraints="cores=4 mem=32G" e2e-runner
-juju config e2e-runner token="$GITHUB_TOKEN" path=canonical/github-runner-operator virtual-machines=1
diff --git a/script/remove_offline_runners.py b/script/remove_offline_runners.py
deleted file mode 100644
index 9df78d365..000000000
--- a/script/remove_offline_runners.py
+++ /dev/null
@@ -1,76 +0,0 @@
-# Copyright 2023 Canonical Ltd.
-# See LICENSE file for licensing details.
-
-import logging
-import sys
-
-import requests
-
-ORG = "<your org>"
-TOKEN = "<your token>"
-
-
-logging.basicConfig(level=logging.INFO)
-logger = logging.getLogger(__name__)
-
-
-def get_runners():
-    try:
-        response = requests.get(
-            f"https://api.github.com/orgs/{ORG}/actions/runners?per_page=100",
-            # "https://api.github.com/repos/canonical/github-runner-operator/actions/runners",
-            headers={
-                "X-GitHub-Api-Version": "2022-11-28",
-                "Authorization": "Bearer " + TOKEN,
-                "Accept": "application/vnd.github+json",
-            },
-            timeout=60,
-        )
-
-        response.raise_for_status()
-        runners = response.json()
-        logger.info("Runners found: %s", runners)
-        return runners
-    except requests.HTTPError as http_err:
-        sys.exit(f"HTTP error occurred: {http_err}")
-    except Exception as err:
-        sys.exit(f"Other error occurred: {err}")
-
-
-def filter_offline_runners(runners):
-    offline_runners = []
-
-    runner_list = runners["runners"]
-    for runner in runner_list:
-        if runner["status"] == "offline":
-            offline_runners.append(runner)
-
-    return offline_runners
-
-
-def delete_runner(runner):
-    logger.info("Deleting runner with id %s", runner["id"])
-
-    try:
-        response = requests.delete(
-            f"https://api.github.com/orgs/{ORG}/actions/runners/{runner['id']}",
-            # f"https://api.github.com/repos/canonical/github-runner-operator/actions/runners/{runner['id']}",
-            headers={
-                "X-GitHub-Api-Version": "2022-11-28",
-                "Authorization": "Bearer " + TOKEN,
-                "Accept": "application/vnd.github+json",
-            },
-            timeout=60,
-        )
-
-        response.raise_for_status()
-    except requests.HTTPError as http_err:
-        sys.exit(f"HTTP error occurred: {http_err}")
-    except Exception as err:
-        sys.exit(f"Other error occurred: {err}")
-
-
-if __name__ == "__main__":
-    while offline_runners := filter_offline_runners(get_runners()) > 0:
-        for runner in offline_runners:
-            delete_runner(runner)
diff --git a/script/remove_runner.sh b/script/remove_runner.sh
deleted file mode 100644
index 635d6beca..000000000
--- a/script/remove_runner.sh
+++ /dev/null
@@ -1,6 +0,0 @@
-#!/usr/bin/env bash
-
-# Copyright 2023 Canonical Ltd.
-# See LICENSE file for licensing details.
-
-juju remove-application e2e-runner --force --destroy-storage --no-wait

From da9502d44055e6f80ebf574f6a3d52796bc21913 Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Wed, 12 Jul 2023 19:53:56 +0800
Subject: [PATCH 02/16] Use runner application process as health check

---
 src/runner_manager.py | 84 +++++++++++++++++++------------------------
 src/runner_type.py    |  8 ++++-
 2 files changed, 44 insertions(+), 48 deletions(-)

diff --git a/src/runner_manager.py b/src/runner_manager.py
index 460f2369c..4c8e4a86e 100644
--- a/src/runner_manager.py
+++ b/src/runner_manager.py
@@ -34,7 +34,14 @@
 from lxd import LxdClient, LxdInstance
 from repo_policy_compliance_client import RepoPolicyComplianceClient
 from runner import Runner, RunnerClients, RunnerConfig, RunnerStatus
-from runner_type import GitHubOrg, GitHubPath, GitHubRepo, ProxySetting, VirtualMachineResources
+from runner_type import (
+    GitHubOrg,
+    GitHubPath,
+    GitHubRepo,
+    ProxySetting,
+    Runners,
+    VirtualMachineResources,
+)
 from utilities import retry, set_env_var
 
 logger = logging.getLogger(__name__)
@@ -252,6 +259,26 @@ def get_github_info(self) -> Iterator[RunnerInfo]:
         remote_runners = self._get_runner_github_info()
         return iter(RunnerInfo(runner.name, runner.status) for runner in remote_runners.values())
 
+    def _get_runner_by_health(self) -> Runners:
+        local_runners = {
+            instance.name: instance
+            # Pylint cannot find the `all` method.
+            for instance in self._clients.lxd.instances.all()  # pylint: disable=no-member
+            if instance.name.startswith(f"{self.instance_name}-")
+        }
+
+        healthy = []
+        unhealthy = []
+
+        for runner in local_runners:
+            _, stdout, stderr = runner.execute(["ps", "aux"])
+            if f"/bin/bash {Runner.runner_script}" in stdout.read():
+                healthy.append(runner)
+            else:
+                unhealthy.append(runner)
+
+        return Runners(healthy, unhealthy)
+
     def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int:
         """Bring runners in line with target.
 
@@ -262,44 +289,26 @@ def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int:
         Returns:
             Difference between intended runners and actual runners.
         """
-        runners = self._get_runners()
-
-        # Add/Remove runners to match the target quantity
-        online_runners = [
-            runner for runner in runners if runner.status.exist and runner.status.online
-        ]
-
-        offline_runners = [runner for runner in runners if not runner.status.online]
-
-        local_runners = {
-            instance.name: instance
-            # Pylint cannot find the `all` method.
-            for instance in self._clients.lxd.instances.all()  # pylint: disable=no-member
-            if instance.name.startswith(f"{self.instance_name}-")
-        }
+        runners = self._get_runner_by_health()
 
         logger.info(
-            (
-                "Expected runner count: %i, Online runner count: %i, Offline runner count: %i, "
-                "LXD instance count: %i"
-            ),
+            ("Expected runner count: %i, healthy count: %i, unhealthy count: %i"),
             quantity,
-            len(online_runners),
-            len(offline_runners),
-            len(local_runners),
+            len(runners.healthy),
+            len(runners.unhealthy),
         )
 
-        # Clean up offline runners
-        if offline_runners:
-            logger.info("Cleaning up offline runners.")
+        # Clean up unhealthy runners
+        if runners.unhealthy:
+            logger.info("Cleaning up unhealthy runners.")
 
             remove_token = self._get_github_remove_token()
 
-            for runner in offline_runners:
+            for runner in runners.unhealthy:
                 runner.remove(remove_token)
                 logger.info("Removed runner: %s", runner.config.name)
 
-        delta = quantity - len(online_runners)
+        delta = quantity - len(runners.healthy)
         # Spawn new runners
         if delta > 0:
             if RunnerManager.runner_bin_path is None:
@@ -333,25 +342,6 @@ def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int:
                     runner.remove(remove_token)
                     logger.info("Cleaned up runner: %s", runner.config.name)
                     raise
-
-        elif delta < 0:
-            # Idle runners are online runners that has not taken a job.
-            idle_runners = [runner for runner in online_runners if not runner.status.busy]
-            offset = min(-delta, len(idle_runners))
-            if offset != 0:
-                logger.info("Removing %i runner(s).", offset)
-                remove_runners = idle_runners[:offset]
-
-                logger.info("Cleaning up idle runners.")
-
-                remove_token = self._get_github_remove_token()
-
-                for runner in remove_runners:
-                    runner.remove(remove_token)
-                    logger.info("Removed runner: %s", runner.config.name)
-
-            else:
-                logger.info("There are no idle runner to remove.")
         else:
             logger.info("No changes to number of runner needed.")
 
diff --git a/src/runner_type.py b/src/runner_type.py
index 2d11bb32c..8bca91217 100644
--- a/src/runner_type.py
+++ b/src/runner_type.py
@@ -11,10 +11,16 @@
 import jinja2
 from ghapi.all import GhApi
 
-from lxd import LxdClient
+from lxd import LxdClient, LxdInstance
 from repo_policy_compliance_client import RepoPolicyComplianceClient
 
 
+@dataclass
+class Runners:
+    healthy: tuple[LxdInstance]
+    unhealthy: tuple[LxdInstance]
+
+
 class ProxySetting(TypedDict, total=False):
     """Represent HTTP-related proxy settings."""
 

From c9ee1bee093c204482cf8876b510fc2c15b5fc97 Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Wed, 12 Jul 2023 19:58:47 +0800
Subject: [PATCH 03/16] Rename type and add docstring

---
 src/runner_manager.py | 6 +++---
 src/runner_type.py    | 4 +++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/runner_manager.py b/src/runner_manager.py
index 4c8e4a86e..4def37e65 100644
--- a/src/runner_manager.py
+++ b/src/runner_manager.py
@@ -39,7 +39,7 @@
     GitHubPath,
     GitHubRepo,
     ProxySetting,
-    Runners,
+    RunnerSet,
     VirtualMachineResources,
 )
 from utilities import retry, set_env_var
@@ -259,7 +259,7 @@ def get_github_info(self) -> Iterator[RunnerInfo]:
         remote_runners = self._get_runner_github_info()
         return iter(RunnerInfo(runner.name, runner.status) for runner in remote_runners.values())
 
-    def _get_runner_by_health(self) -> Runners:
+    def _get_runner_by_health(self) -> RunnerSet:
         local_runners = {
             instance.name: instance
             # Pylint cannot find the `all` method.
@@ -277,7 +277,7 @@ def _get_runner_by_health(self) -> Runners:
             else:
                 unhealthy.append(runner)
 
-        return Runners(healthy, unhealthy)
+        return RunnerSet(healthy, unhealthy)
 
     def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int:
         """Bring runners in line with target.
diff --git a/src/runner_type.py b/src/runner_type.py
index 8bca91217..64439ce96 100644
--- a/src/runner_type.py
+++ b/src/runner_type.py
@@ -16,7 +16,9 @@
 
 
 @dataclass
-class Runners:
+class RunnerSet:
+    """Set of runners by health state."""
+
     healthy: tuple[LxdInstance]
     unhealthy: tuple[LxdInstance]
 

From 3af49c8c7b60e752b52906b4db7c22747941bd13 Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Fri, 14 Jul 2023 10:11:40 +0800
Subject: [PATCH 04/16] Hide logging ofsensitive commands

---
 src/lxd.py       |  7 +++++--
 src/runner.py    |  2 ++
 src/utilities.py | 11 +++++++++--
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/lxd.py b/src/lxd.py
index a9c1da2a3..2da02e3cb 100644
--- a/src/lxd.py
+++ b/src/lxd.py
@@ -222,7 +222,9 @@ def delete(self, wait: bool = False) -> None:
             logger.exception("Failed to delete LXD instance")
             raise LxdError(f"Unable to delete LXD instance {self.name}") from err
 
-    def execute(self, cmd: list[str], cwd: Optional[str] = None) -> Tuple[int, IO, IO]:
+    def execute(
+        self, cmd: list[str], cwd: Optional[str] = None, hide_cmd: bool = False
+    ) -> Tuple[int, IO, IO]:
         """Execute a command within the LXD instance.
 
         Exceptions are not raise if command execution failed. Caller should check the exit code and
@@ -231,6 +233,7 @@ def execute(self, cmd: list[str], cwd: Optional[str] = None) -> Tuple[int, IO, I
         Args:
             cmd: Commands to be executed.
             cwd: Working directory to execute the commands.
+            hide_cmd: Hide logging of cmd.
 
         Returns:
             Tuple containing the exit code, stdout, stderr.
@@ -241,7 +244,7 @@ def execute(self, cmd: list[str], cwd: Optional[str] = None) -> Tuple[int, IO, I
 
         lxc_cmd += ["--"] + cmd
 
-        result = secure_run_subprocess(lxc_cmd)
+        result = secure_run_subprocess(lxc_cmd, hide_cmd)
         return (result.returncode, io.BytesIO(result.stdout), io.BytesIO(result.stderr))
 
 
diff --git a/src/runner.py b/src/runner.py
index 0f5932add..27d79dd98 100644
--- a/src/runner.py
+++ b/src/runner.py
@@ -549,9 +549,11 @@ def _register_runner(self, registration_token: str, labels: Sequence[str]) -> No
         if isinstance(self.config.path, GitHubOrg):
             register_cmd += ["--runnergroup", self.config.path.group]
 
+        logger.info("Executing registration command...")
         self.instance.execute(
             register_cmd,
             cwd=str(self.runner_application),
+            hide_cmd=True,
         )
 
     @retry(tries=5, delay=30, local_logger=logger)
diff --git a/src/utilities.py b/src/utilities.py
index 9a1d3ef1e..56316bd78 100644
--- a/src/utilities.py
+++ b/src/utilities.py
@@ -95,7 +95,9 @@ def fn_with_retry(*args, **kwargs) -> ReturnT:
     return retry_decorator
 
 
-def secure_run_subprocess(cmd: Sequence[str], **kwargs) -> subprocess.CompletedProcess[bytes]:
+def secure_run_subprocess(
+    cmd: Sequence[str], hide_cmd: bool = False, **kwargs
+) -> subprocess.CompletedProcess[bytes]:
     """Run command in subprocess according to security recommendations.
 
     The argument `shell` is set to `False` for security reasons.
@@ -105,12 +107,17 @@ def secure_run_subprocess(cmd: Sequence[str], **kwargs) -> subprocess.CompletedP
 
     Args:
         cmd: Command in a list.
+        hide_cmd: Hide logging of cmd.
         kwargs: Additional keyword arguments for the `subprocess.run` call.
 
     Returns:
         Object representing the completed process. The outputs subprocess can accessed.
     """
-    logger.info("Executing command %s", cmd)
+    if not hide_cmd:
+        logger.info("Executing command %s", cmd)
+    else:
+        logger.info("Executing sensitive command")
+
     result = subprocess.run(  # nosec B603
         cmd,
         capture_output=True,

From a988f5c6e7724c34a79c03cf29260ab4ffb498dc Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Fri, 14 Jul 2023 11:16:29 +0800
Subject: [PATCH 05/16] Fix unit test

---
 src/lxd.py                        |  7 ++---
 src/runner_manager.py             | 49 +++++++++++++++++++++----------
 src/runner_type.py                |  4 +--
 tests/unit/mock.py                |  4 ++-
 tests/unit/test_runner_manager.py | 42 ++++++++++++--------------
 5 files changed, 61 insertions(+), 45 deletions(-)

diff --git a/src/lxd.py b/src/lxd.py
index 2da02e3cb..a5bcc7457 100644
--- a/src/lxd.py
+++ b/src/lxd.py
@@ -7,10 +7,9 @@
 """
 from __future__ import annotations
 
-import io
 import logging
 import tempfile
-from typing import IO, Optional, Tuple, Union
+from typing import Optional, Tuple, Union
 
 import pylxd.models
 
@@ -224,7 +223,7 @@ def delete(self, wait: bool = False) -> None:
 
     def execute(
         self, cmd: list[str], cwd: Optional[str] = None, hide_cmd: bool = False
-    ) -> Tuple[int, IO, IO]:
+    ) -> Tuple[int, str, str]:
         """Execute a command within the LXD instance.
 
         Exceptions are not raise if command execution failed. Caller should check the exit code and
@@ -245,7 +244,7 @@ def execute(
         lxc_cmd += ["--"] + cmd
 
         result = secure_run_subprocess(lxc_cmd, hide_cmd)
-        return (result.returncode, io.BytesIO(result.stdout), io.BytesIO(result.stderr))
+        return (result.returncode, result.stdout, result.stderr)
 
 
 class LxdInstanceManager:
diff --git a/src/runner_manager.py b/src/runner_manager.py
index 4def37e65..6926369c0 100644
--- a/src/runner_manager.py
+++ b/src/runner_manager.py
@@ -39,7 +39,7 @@
     GitHubPath,
     GitHubRepo,
     ProxySetting,
-    RunnerSet,
+    RunnerLxdInstance,
     VirtualMachineResources,
 )
 from utilities import retry, set_env_var
@@ -259,25 +259,25 @@ def get_github_info(self) -> Iterator[RunnerInfo]:
         remote_runners = self._get_runner_github_info()
         return iter(RunnerInfo(runner.name, runner.status) for runner in remote_runners.values())
 
-    def _get_runner_by_health(self) -> RunnerSet:
-        local_runners = {
-            instance.name: instance
+    def _get_lxd_instance_by_runner_health(self) -> RunnerLxdInstance:
+        local_runners = [
+            instance
             # Pylint cannot find the `all` method.
             for instance in self._clients.lxd.instances.all()  # pylint: disable=no-member
             if instance.name.startswith(f"{self.instance_name}-")
-        }
+        ]
 
         healthy = []
         unhealthy = []
 
         for runner in local_runners:
-            _, stdout, stderr = runner.execute(["ps", "aux"])
-            if f"/bin/bash {Runner.runner_script}" in stdout.read():
+            _, stdout, _ = runner.execute(["ps", "aux"])
+            if f"/bin/bash {Runner.runner_script}" in stdout:
                 healthy.append(runner)
             else:
                 unhealthy.append(runner)
 
-        return RunnerSet(healthy, unhealthy)
+        return RunnerLxdInstance(healthy, unhealthy)
 
     def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int:
         """Bring runners in line with target.
@@ -289,26 +289,34 @@ def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int:
         Returns:
             Difference between intended runners and actual runners.
         """
-        runners = self._get_runner_by_health()
+        runner_lxd_instances = self._get_lxd_instance_by_runner_health()
 
         logger.info(
             ("Expected runner count: %i, healthy count: %i, unhealthy count: %i"),
             quantity,
-            len(runners.healthy),
-            len(runners.unhealthy),
+            len(runner_lxd_instances.healthy),
+            len(runner_lxd_instances.unhealthy),
         )
 
         # Clean up unhealthy runners
-        if runners.unhealthy:
+        if runner_lxd_instances.unhealthy:
             logger.info("Cleaning up unhealthy runners.")
 
             remove_token = self._get_github_remove_token()
 
-            for runner in runners.unhealthy:
+            for instance in runner_lxd_instances.unhealthy:
+                config = RunnerConfig(
+                    self.app_name,
+                    self.config.path,
+                    self.proxies,
+                    self.config.lxd_storage_path,
+                    instance.name,
+                )
+                runner = Runner(self._clients, config, RunnerStatus())
                 runner.remove(remove_token)
                 logger.info("Removed runner: %s", runner.config.name)
 
-        delta = quantity - len(runners.healthy)
+        delta = quantity - len(runner_lxd_instances.healthy)
         # Spawn new runners
         if delta > 0:
             if RunnerManager.runner_bin_path is None:
@@ -319,7 +327,7 @@ def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int:
             registration_token = self._get_github_registration_token()
             remove_token = self._get_github_remove_token()
 
-            logger.info("Adding %i additional runner(s).", delta)
+            logger.info("Attempting to add %i runner(s).", delta)
             for _ in range(delta):
                 config = RunnerConfig(
                     self.app_name,
@@ -342,6 +350,17 @@ def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int:
                     runner.remove(remove_token)
                     logger.info("Cleaned up runner: %s", runner.config.name)
                     raise
+        if delta < 0:
+            logger.info("Attempting to remove %i runner(s).", -delta)
+            runners_to_remove = [
+                runner
+                for runner in self._get_runners()
+                if runner.status.exist and not runner.status.busy
+            ][:-delta]
+            logger.info("Found %i non-busy and online runner to remove", len(runners_to_remove))
+            for runner in runners_to_remove:
+                runner.remove(remove_token)
+                logger.info("Removed runner: %s", runner.config.name)
         else:
             logger.info("No changes to number of runner needed.")
 
diff --git a/src/runner_type.py b/src/runner_type.py
index 64439ce96..94f473da0 100644
--- a/src/runner_type.py
+++ b/src/runner_type.py
@@ -16,8 +16,8 @@
 
 
 @dataclass
-class RunnerSet:
-    """Set of runners by health state."""
+class RunnerLxdInstance:
+    """Set of runners LXD instance by health state."""
 
     healthy: tuple[LxdInstance]
     unhealthy: tuple[LxdInstance]
diff --git a/tests/unit/mock.py b/tests/unit/mock.py
index 7c3bfdac0..6a34b7781 100644
--- a/tests/unit/mock.py
+++ b/tests/unit/mock.py
@@ -102,7 +102,9 @@ def stop(self, wait: bool = True, timeout: int = 60):
     def delete(self, wait: bool = True):
         self.deleted = True
 
-    def execute(self, cmd: Sequence[str], cwd: Optional[str] = None) -> tuple[int, str, str]:
+    def execute(
+        self, cmd: Sequence[str], cwd: Optional[str] = None, hide_cmd: bool = False
+    ) -> tuple[int, str, str]:
         return 0, "", ""
 
 
diff --git a/tests/unit/test_runner_manager.py b/tests/unit/test_runner_manager.py
index 02358867e..c9c298133 100644
--- a/tests/unit/test_runner_manager.py
+++ b/tests/unit/test_runner_manager.py
@@ -5,15 +5,13 @@
 
 import secrets
 from pathlib import Path
-from unittest.mock import MagicMock
 
 import pytest
 
 from errors import RunnerBinaryError
-from runner import Runner, RunnerStatus
 from runner_manager import RunnerManager, RunnerManagerConfig
-from runner_type import GitHubOrg, GitHubRepo, VirtualMachineResources
-from tests.unit.mock import TEST_BINARY
+from runner_type import GitHubOrg, GitHubRepo, RunnerLxdInstance, VirtualMachineResources
+from tests.unit.mock import TEST_BINARY, MockLxdInstance
 
 
 @pytest.fixture(scope="function", name="token")
@@ -118,24 +116,19 @@ def test_reconcile_create_runner(runner_manager: RunnerManager):
 
 def test_reconcile_remove_runner(runner_manager: RunnerManager):
     """
-    arrange: Create online runners.
-    act: Reconcile to remove a runner.
-    assert: One runner should be removed.
+    arrange: Setup 2 runners.
+    act: Reconcile to 1 runners.
+    assert: Runner manager should attempt to remove one runner.
     """
+    runner_manager._get_lxd_instance_by_runner_health = lambda: RunnerLxdInstance(
+        (
+            MockLxdInstance(f"{runner_manager.instance_name}-0"),
+            MockLxdInstance(f"{runner_manager.instance_name}-1"),
+        ),
+        (),
+    )
 
-    def mock_get_runners():
-        """Create three mock runners."""
-        runners = []
-        for _ in range(3):
-            # 0 is a mock runner id.
-            status = RunnerStatus(0, True, True, False)
-            runners.append(Runner(MagicMock(), MagicMock(), status, None))
-        return runners
-
-    # Create online runners.
-    runner_manager._get_runners = mock_get_runners
-
-    delta = runner_manager.reconcile(2, VirtualMachineResources(2, "7GiB", "10Gib"))
+    delta = runner_manager.reconcile(1, VirtualMachineResources(2, "7GiB", "10Gib"))
 
     assert delta == -1
 
@@ -146,11 +139,14 @@ def test_reconcile(runner_manager: RunnerManager, tmp_path: Path):
     act: Reconcile with the current amount of runner.
     assert: Still have one runner.
     """
-    runner_manager.reconcile(1, VirtualMachineResources(2, "7GiB", "10Gib"))
+    runner_manager._get_lxd_instance_by_runner_health = lambda: RunnerLxdInstance(
+        (MockLxdInstance(f"{runner_manager.instance_name}-0"),),
+        (),
+    )
     # Reconcile with no change to runner count.
-    runner_manager.reconcile(1, VirtualMachineResources(2, "7GiB", "10Gib"))
+    delta = runner_manager.reconcile(1, VirtualMachineResources(2, "7GiB", "10Gib"))
 
-    assert len(runner_manager._get_runners()) == 1
+    assert delta == 0
 
 
 def test_empty_flush(runner_manager: RunnerManager):

From 43746b8cbb840681939669a28ca3775358e58a46 Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Fri, 14 Jul 2023 12:41:59 +0800
Subject: [PATCH 06/16] Increase per_page for GitHub API calls

---
 src/runner_manager.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/runner_manager.py b/src/runner_manager.py
index 6926369c0..d8d2165cc 100644
--- a/src/runner_manager.py
+++ b/src/runner_manager.py
@@ -407,11 +407,14 @@ def _get_runner_github_info(self) -> Dict[str, SelfHostedRunner]:
         remote_runners_list: list[SelfHostedRunner] = []
         if isinstance(self.config.path, GitHubRepo):
             remote_runners_list = self._clients.github.actions.list_self_hosted_runners_for_repo(
-                owner=self.config.path.owner, repo=self.config.path.repo
+                owner=self.config.path.owner,
+                repo=self.config.path.repo,
+                per_page=100,
             )["runners"]
         if isinstance(self.config.path, GitHubOrg):
             remote_runners_list = self._clients.github.actions.list_self_hosted_runners_for_org(
-                org=self.config.path.org
+                org=self.config.path.org,
+                per_page=100,
             )["runners"]
 
         logger.debug("List of runners found on GitHub:%s", remote_runners_list)

From 315dbcc64ff810664a7f8b83745752b79937c20b Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Fri, 14 Jul 2023 12:44:04 +0800
Subject: [PATCH 07/16] Increase gunicorn timeout setting

---
 templates/repo-policy-compliance.service.j2 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/templates/repo-policy-compliance.service.j2 b/templates/repo-policy-compliance.service.j2
index afde32b2e..20049e621 100644
--- a/templates/repo-policy-compliance.service.j2
+++ b/templates/repo-policy-compliance.service.j2
@@ -14,4 +14,4 @@ Environment="NO_PROXY={{proxies['no_proxy']}}"
 Environment="http_proxy={{proxies['http']}}"
 Environment="https_proxy={{proxies['https']}}"
 Environment="no_proxy={{proxies['no_proxy']}}"
-ExecStart=/usr/bin/gunicorn --bind 0.0.0.0:8080 app:app
+ExecStart=/usr/bin/gunicorn --bind 0.0.0.0:8080 --timeout 60 app:app

From 98bcf6925fe2ac98a2695657d17bde487bf1c787 Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Fri, 14 Jul 2023 16:33:07 +0800
Subject: [PATCH 08/16] Hide removal token logging

---
 src/runner.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/runner.py b/src/runner.py
index 27d79dd98..74c8d6f25 100644
--- a/src/runner.py
+++ b/src/runner.py
@@ -138,7 +138,7 @@ def remove(self, remove_token: str) -> None:
         logger.info("Removing LXD instance of runner: %s", self.config.name)
 
         if self.instance:
-            # Run script to remove the the runner and cleanup.
+            logger.info("Executing command to removal of runner and clean up...")
             self.instance.execute(
                 [
                     "/usr/bin/sudo",
@@ -149,6 +149,7 @@ def remove(self, remove_token: str) -> None:
                     "--token",
                     remove_token,
                 ],
+                hide_cmd=True,
             )
 
             if self.instance.status == "Running":

From bb94609c9bc095a5d7b3c29f386df2d32fb4a346 Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Fri, 14 Jul 2023 17:14:09 +0800
Subject: [PATCH 09/16] Fix type issue

---
 src/lxd.py            |  7 ++++---
 src/runner_manager.py |  2 +-
 tests/unit/mock.py    | 11 ++++++-----
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/lxd.py b/src/lxd.py
index a5bcc7457..2da02e3cb 100644
--- a/src/lxd.py
+++ b/src/lxd.py
@@ -7,9 +7,10 @@
 """
 from __future__ import annotations
 
+import io
 import logging
 import tempfile
-from typing import Optional, Tuple, Union
+from typing import IO, Optional, Tuple, Union
 
 import pylxd.models
 
@@ -223,7 +224,7 @@ def delete(self, wait: bool = False) -> None:
 
     def execute(
         self, cmd: list[str], cwd: Optional[str] = None, hide_cmd: bool = False
-    ) -> Tuple[int, str, str]:
+    ) -> Tuple[int, IO, IO]:
         """Execute a command within the LXD instance.
 
         Exceptions are not raise if command execution failed. Caller should check the exit code and
@@ -244,7 +245,7 @@ def execute(
         lxc_cmd += ["--"] + cmd
 
         result = secure_run_subprocess(lxc_cmd, hide_cmd)
-        return (result.returncode, result.stdout, result.stderr)
+        return (result.returncode, io.BytesIO(result.stdout), io.BytesIO(result.stderr))
 
 
 class LxdInstanceManager:
diff --git a/src/runner_manager.py b/src/runner_manager.py
index d8d2165cc..e5127573c 100644
--- a/src/runner_manager.py
+++ b/src/runner_manager.py
@@ -272,7 +272,7 @@ def _get_lxd_instance_by_runner_health(self) -> RunnerLxdInstance:
 
         for runner in local_runners:
             _, stdout, _ = runner.execute(["ps", "aux"])
-            if f"/bin/bash {Runner.runner_script}" in stdout:
+            if f"/bin/bash {Runner.runner_script}" in stdout.read().decode("utf-8"):
                 healthy.append(runner)
             else:
                 unhealthy.append(runner)
diff --git a/tests/unit/mock.py b/tests/unit/mock.py
index 6a34b7781..3c34aaf5f 100644
--- a/tests/unit/mock.py
+++ b/tests/unit/mock.py
@@ -6,9 +6,10 @@
 from __future__ import annotations
 
 import hashlib
+import io
 import logging
 import secrets
-from typing import Optional, Sequence, Union
+from typing import IO, Optional, Sequence, Union
 
 from errors import LxdError, RunnerError
 from github_type import RegistrationToken, RemoveToken, RunnerApplication
@@ -104,8 +105,8 @@ def delete(self, wait: bool = True):
 
     def execute(
         self, cmd: Sequence[str], cwd: Optional[str] = None, hide_cmd: bool = False
-    ) -> tuple[int, str, str]:
-        return 0, "", ""
+    ) -> tuple[int, IO, IO]:
+        return 0, io.BytesIO(b""), io.BytesIO(b"")
 
 
 class MockLxdInstanceFileManager:
@@ -240,10 +241,10 @@ def create_remove_token_for_org(self, org: str):
             {"token": self.remove_token_org, "expires_at": "2020-01-22T12:13:35.123-08:00"}
         )
 
-    def list_self_hosted_runners_for_repo(self, owner: str, repo: str):
+    def list_self_hosted_runners_for_repo(self, owner: str, repo: str, per_page: int):
         return {"runners": []}
 
-    def list_self_hosted_runners_for_org(self, org: str):
+    def list_self_hosted_runners_for_org(self, org: str, per_page: int):
         return {"runners": []}
 
     def delete_self_hosted_runner_from_repo(self, owner: str, repo: str, runner_id: str):

From ce84c243dd1dc89a7bff28a507655daf5b1457f7 Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Mon, 17 Jul 2023 10:38:22 +0800
Subject: [PATCH 10/16] Fix bug related to how unhealthy runners are removed

---
 src/runner.py         |  5 ++++-
 src/runner_manager.py | 10 ++--------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/runner.py b/src/runner.py
index 74c8d6f25..b686b809c 100644
--- a/src/runner.py
+++ b/src/runner.py
@@ -135,7 +135,7 @@ def remove(self, remove_token: str) -> None:
         Raises:
             RunnerRemoveError: Failure in removing runner.
         """
-        logger.info("Removing LXD instance of runner: %s", self.config.name)
+        logger.info("Removing runner: %s", self.config.name)
 
         if self.instance:
             logger.info("Executing command to removal of runner and clean up...")
@@ -153,6 +153,7 @@ def remove(self, remove_token: str) -> None:
             )
 
             if self.instance.status == "Running":
+                logger.info("Removing LXD instance of runner: %s", self.config.name)
                 try:
                     self.instance.stop(wait=True, timeout=60)
                 except LxdError:
@@ -180,6 +181,8 @@ def remove(self, remove_token: str) -> None:
         if self.status.runner_id is None:
             return
 
+        logger.info("Removing runner on GitHub: %s", self.config.name)
+
         # The runner should cleanup itself.  Cleanup on GitHub in case of runner cleanup error.
         if isinstance(self.config.path, GitHubRepo):
             logger.debug(
diff --git a/src/runner_manager.py b/src/runner_manager.py
index e5127573c..2e308f99d 100644
--- a/src/runner_manager.py
+++ b/src/runner_manager.py
@@ -303,16 +303,10 @@ def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int:
             logger.info("Cleaning up unhealthy runners.")
 
             remove_token = self._get_github_remove_token()
+            runners = {runner.config.name: runner for runner in self._get_runners()}
 
             for instance in runner_lxd_instances.unhealthy:
-                config = RunnerConfig(
-                    self.app_name,
-                    self.config.path,
-                    self.proxies,
-                    self.config.lxd_storage_path,
-                    instance.name,
-                )
-                runner = Runner(self._clients, config, RunnerStatus())
+                runner = runners[instance.name]
                 runner.remove(remove_token)
                 logger.info("Removed runner: %s", runner.config.name)
 

From 8d2d472ed8c3c8c522c835e1bc42083d3bc27516 Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Mon, 17 Jul 2023 12:49:32 +0800
Subject: [PATCH 11/16] Use poweroff over halt due to LXD issues

---
 templates/start.j2 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/templates/start.j2 b/templates/start.j2
index 7e6979ce5..e0d2bbef6 100644
--- a/templates/start.j2
+++ b/templates/start.j2
@@ -1,3 +1,3 @@
 #!/bin/bash
 
-(/home/ubuntu/github-runner/run.sh; sudo systemctl halt -i) &>/dev/null &
+(/home/ubuntu/github-runner/run.sh; sudo systemctl poweroff -i) &>/dev/null &

From 249db5789e3a4b211b6a6512a72ab68a70010971 Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Mon, 17 Jul 2023 14:28:19 +0800
Subject: [PATCH 12/16] Fix reconcile offline runners

---
 src/runner_manager.py             | 72 ++++++++++++++++++++-----------
 src/runner_type.py                |  8 ++--
 tests/unit/test_runner_manager.py | 42 +++++++++++-------
 3 files changed, 78 insertions(+), 44 deletions(-)

diff --git a/src/runner_manager.py b/src/runner_manager.py
index 2e308f99d..45ddf7420 100644
--- a/src/runner_manager.py
+++ b/src/runner_manager.py
@@ -39,7 +39,7 @@
     GitHubPath,
     GitHubRepo,
     ProxySetting,
-    RunnerLxdInstance,
+    RunnerByHealth,
     VirtualMachineResources,
 )
 from utilities import retry, set_env_var
@@ -259,7 +259,7 @@ def get_github_info(self) -> Iterator[RunnerInfo]:
         remote_runners = self._get_runner_github_info()
         return iter(RunnerInfo(runner.name, runner.status) for runner in remote_runners.values())
 
-    def _get_lxd_instance_by_runner_health(self) -> RunnerLxdInstance:
+    def _get_runner_health_states(self) -> RunnerByHealth:
         local_runners = [
             instance
             # Pylint cannot find the `all` method.
@@ -273,11 +273,11 @@ def _get_lxd_instance_by_runner_health(self) -> RunnerLxdInstance:
         for runner in local_runners:
             _, stdout, _ = runner.execute(["ps", "aux"])
             if f"/bin/bash {Runner.runner_script}" in stdout.read().decode("utf-8"):
-                healthy.append(runner)
+                healthy.append(runner.name)
             else:
-                unhealthy.append(runner)
+                unhealthy.append(runner.name)
 
-        return RunnerLxdInstance(healthy, unhealthy)
+        return RunnerByHealth(healthy, unhealthy)
 
     def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int:
         """Bring runners in line with target.
@@ -289,28 +289,42 @@ def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int:
         Returns:
             Difference between intended runners and actual runners.
         """
-        runner_lxd_instances = self._get_lxd_instance_by_runner_health()
+        runners = self._get_runners()
+
+        # Add/Remove runners to match the target quantity
+        online_runners = [
+            runner for runner in runners if runner.status.exist and runner.status.online
+        ]
+
+        runner_states = self._get_runner_health_states()
 
         logger.info(
-            ("Expected runner count: %i, healthy count: %i, unhealthy count: %i"),
+            (
+                "Expected runner count: %i, Online count: %i, Offline count: %i, "
+                "healthy count: %i, unhealthy count: %i"
+            ),
             quantity,
-            len(runner_lxd_instances.healthy),
-            len(runner_lxd_instances.unhealthy),
+            len(online_runners),
+            len(runners) - len(online_runners),
+            len(runner_states.healthy),
+            len(runner_states.unhealthy),
         )
 
-        # Clean up unhealthy runners
-        if runner_lxd_instances.unhealthy:
+        # Clean up offline runners
+        if runner_states.unhealthy:
             logger.info("Cleaning up unhealthy runners.")
 
             remove_token = self._get_github_remove_token()
-            runners = {runner.config.name: runner for runner in self._get_runners()}
 
-            for instance in runner_lxd_instances.unhealthy:
-                runner = runners[instance.name]
+            unhealthy_runners = [
+                runner for runner in runners if runner.config.name in set(runner_states.unhealthy)
+            ]
+
+            for runner in unhealthy_runners:
                 runner.remove(remove_token)
                 logger.info("Removed runner: %s", runner.config.name)
 
-        delta = quantity - len(runner_lxd_instances.healthy)
+        delta = quantity - len(runner_states.healthy)
         # Spawn new runners
         if delta > 0:
             if RunnerManager.runner_bin_path is None:
@@ -344,17 +358,25 @@ def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int:
                     runner.remove(remove_token)
                     logger.info("Cleaned up runner: %s", runner.config.name)
                     raise
-        if delta < 0:
+
+        elif delta < 0:
             logger.info("Attempting to remove %i runner(s).", -delta)
-            runners_to_remove = [
-                runner
-                for runner in self._get_runners()
-                if runner.status.exist and not runner.status.busy
-            ][:-delta]
-            logger.info("Found %i non-busy and online runner to remove", len(runners_to_remove))
-            for runner in runners_to_remove:
-                runner.remove(remove_token)
-                logger.info("Removed runner: %s", runner.config.name)
+            # Idle runners are online runners that has not taken a job.
+            idle_runners = [runner for runner in online_runners if not runner.status.busy]
+            offset = min(-delta, len(idle_runners))
+            if offset != 0:
+                logger.info("Removing %i runner(s).", offset)
+                remove_runners = idle_runners[:offset]
+
+                logger.info("Cleaning up idle runners.")
+
+                remove_token = self._get_github_remove_token()
+
+                for runner in remove_runners:
+                    runner.remove(remove_token)
+                    logger.info("Removed runner: %s", runner.config.name)
+            else:
+                logger.info("There are no idle runner to remove.")
         else:
             logger.info("No changes to number of runner needed.")
 
diff --git a/src/runner_type.py b/src/runner_type.py
index 94f473da0..62ce0b18b 100644
--- a/src/runner_type.py
+++ b/src/runner_type.py
@@ -11,16 +11,16 @@
 import jinja2
 from ghapi.all import GhApi
 
-from lxd import LxdClient, LxdInstance
+from lxd import LxdClient
 from repo_policy_compliance_client import RepoPolicyComplianceClient
 
 
 @dataclass
-class RunnerLxdInstance:
+class RunnerByHealth:
     """Set of runners LXD instance by health state."""
 
-    healthy: tuple[LxdInstance]
-    unhealthy: tuple[LxdInstance]
+    healthy: tuple[str]
+    unhealthy: tuple[str]
 
 
 class ProxySetting(TypedDict, total=False):
diff --git a/tests/unit/test_runner_manager.py b/tests/unit/test_runner_manager.py
index c9c298133..a7bc42ef9 100644
--- a/tests/unit/test_runner_manager.py
+++ b/tests/unit/test_runner_manager.py
@@ -5,13 +5,15 @@
 
 import secrets
 from pathlib import Path
+from unittest.mock import MagicMock
 
 import pytest
 
 from errors import RunnerBinaryError
+from runner import Runner, RunnerStatus
 from runner_manager import RunnerManager, RunnerManagerConfig
-from runner_type import GitHubOrg, GitHubRepo, RunnerLxdInstance, VirtualMachineResources
-from tests.unit.mock import TEST_BINARY, MockLxdInstance
+from runner_type import GitHubOrg, GitHubRepo, RunnerByHealth, VirtualMachineResources
+from tests.unit.mock import TEST_BINARY
 
 
 @pytest.fixture(scope="function", name="token")
@@ -116,19 +118,32 @@ def test_reconcile_create_runner(runner_manager: RunnerManager):
 
 def test_reconcile_remove_runner(runner_manager: RunnerManager):
     """
-    arrange: Setup 2 runners.
-    act: Reconcile to 1 runners.
-    assert: Runner manager should attempt to remove one runner.
+    arrange: Create online runners.
+    act: Reconcile to remove a runner.
+    assert: One runner should be removed.
     """
-    runner_manager._get_lxd_instance_by_runner_health = lambda: RunnerLxdInstance(
+
+    def mock_get_runners():
+        """Create three mock runners."""
+        runners = []
+        for _ in range(3):
+            # 0 is a mock runner id.
+            status = RunnerStatus(0, True, True, False)
+            runners.append(Runner(MagicMock(), MagicMock(), status, None))
+        return runners
+
+    # Create online runners.
+    runner_manager._get_runners = mock_get_runners
+    runner_manager._get_runner_health_states = lambda: RunnerByHealth(
         (
-            MockLxdInstance(f"{runner_manager.instance_name}-0"),
-            MockLxdInstance(f"{runner_manager.instance_name}-1"),
+            f"{runner_manager.instance_name}-0",
+            f"{runner_manager.instance_name}-1",
+            f"{runner_manager.instance_name}-2",
         ),
         (),
     )
 
-    delta = runner_manager.reconcile(1, VirtualMachineResources(2, "7GiB", "10Gib"))
+    delta = runner_manager.reconcile(2, VirtualMachineResources(2, "7GiB", "10Gib"))
 
     assert delta == -1
 
@@ -139,14 +154,11 @@ def test_reconcile(runner_manager: RunnerManager, tmp_path: Path):
     act: Reconcile with the current amount of runner.
     assert: Still have one runner.
     """
-    runner_manager._get_lxd_instance_by_runner_health = lambda: RunnerLxdInstance(
-        (MockLxdInstance(f"{runner_manager.instance_name}-0"),),
-        (),
-    )
+    runner_manager.reconcile(1, VirtualMachineResources(2, "7GiB", "10Gib"))
     # Reconcile with no change to runner count.
-    delta = runner_manager.reconcile(1, VirtualMachineResources(2, "7GiB", "10Gib"))
+    runner_manager.reconcile(1, VirtualMachineResources(2, "7GiB", "10Gib"))
 
-    assert delta == 0
+    assert len(runner_manager._get_runners()) == 1
 
 
 def test_empty_flush(runner_manager: RunnerManager):

From 685a2f20c56909bd70d25cba301d0627f1cdc2fb Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Mon, 17 Jul 2023 15:17:23 +0800
Subject: [PATCH 13/16] Fix pagination issue of ghapi calls

---
 src/runner_manager.py | 43 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/src/runner_manager.py b/src/runner_manager.py
index 45ddf7420..5d1c63569 100644
--- a/src/runner_manager.py
+++ b/src/runner_manager.py
@@ -20,6 +20,7 @@
 import requests.adapters
 import urllib3
 from ghapi.all import GhApi
+from ghapi.page import pages
 from typing_extensions import assert_never
 
 from errors import RunnerBinaryError, RunnerCreateError
@@ -422,16 +423,40 @@ def _generate_runner_name(self) -> str:
     def _get_runner_github_info(self) -> Dict[str, SelfHostedRunner]:
         remote_runners_list: list[SelfHostedRunner] = []
         if isinstance(self.config.path, GitHubRepo):
-            remote_runners_list = self._clients.github.actions.list_self_hosted_runners_for_repo(
-                owner=self.config.path.owner,
-                repo=self.config.path.repo,
-                per_page=100,
-            )["runners"]
+            # The documentation of ghapi for pagination is incorrect and examples will give errors.
+            # This workaround is a temp solution. Will be moving to PyGitHub in the future.
+            self._clients.github.actions.list_self_hosted_runners_for_repo(
+                owner=self.config.path.owner, repo=self.config.path.repo, per_page=100
+            )
+            num_of_pages = self._clients.github.actions.last_page()
+            remote_runners_list = [
+                item
+                for page in pages(
+                    self._clients.github.actions.list_self_hosted_runners_for_repo,
+                    num_of_pages + 1,
+                    owner=self.config.path.owner,
+                    repo=self.config.path.repo,
+                    per_page=100,
+                )
+                for item in page["runners"]
+            ]
         if isinstance(self.config.path, GitHubOrg):
-            remote_runners_list = self._clients.github.actions.list_self_hosted_runners_for_org(
-                org=self.config.path.org,
-                per_page=100,
-            )["runners"]
+            # The documentation of ghapi for pagination is incorrect and examples will give errors.
+            # This workaround is a temp solution. Will be moving to PyGitHub in the future.
+            self._clients.github.actions.list_self_hosted_runners_for_org(
+                org=self.config.path.org, per_page=100
+            )
+            num_of_pages = self._clients.github.actions.last_page()
+            remote_runners_list = [
+                item
+                for page in pages(
+                    self._clients.github.actions.list_self_hosted_runners_for_org,
+                    num_of_pages + 1,
+                    org=self.config.path.org,
+                    per_page=100,
+                )
+                for item in page["runners"]
+            ]
 
         logger.debug("List of runners found on GitHub:%s", remote_runners_list)
 

From 495f326269aa32cd7ec3ea4a1135187b90db5913 Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Mon, 17 Jul 2023 15:33:42 +0800
Subject: [PATCH 14/16] Fix unit test

---
 tests/unit/mock.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tests/unit/mock.py b/tests/unit/mock.py
index 3c34aaf5f..c0bb242c3 100644
--- a/tests/unit/mock.py
+++ b/tests/unit/mock.py
@@ -241,10 +241,12 @@ def create_remove_token_for_org(self, org: str):
             {"token": self.remove_token_org, "expires_at": "2020-01-22T12:13:35.123-08:00"}
         )
 
-    def list_self_hosted_runners_for_repo(self, owner: str, repo: str, per_page: int):
+    def list_self_hosted_runners_for_repo(
+        self, owner: str, repo: str, per_page: int, page: int = 0
+    ):
         return {"runners": []}
 
-    def list_self_hosted_runners_for_org(self, org: str, per_page: int):
+    def list_self_hosted_runners_for_org(self, org: str, per_page: int, page: int = 0):
         return {"runners": []}
 
     def delete_self_hosted_runner_from_repo(self, owner: str, repo: str, runner_id: str):
@@ -253,6 +255,9 @@ def delete_self_hosted_runner_from_repo(self, owner: str, repo: str, runner_id:
     def delete_self_hosted_runner_from_org(self, org: str, runner_id: str):
         pass
 
+    def last_page(self) -> int:
+        return 0
+
 
 class MockRepoPolicyComplianceClient:
     """Mock for RepoPolicyComplianceClient."""

From ac46f0fe94fd638a27075b5e54cf91580dc6abc1 Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Mon, 17 Jul 2023 15:42:58 +0800
Subject: [PATCH 15/16] Fix typo

---
 src/runner_manager.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/runner_manager.py b/src/runner_manager.py
index 5d1c63569..07b3a04a9 100644
--- a/src/runner_manager.py
+++ b/src/runner_manager.py
@@ -428,7 +428,7 @@ def _get_runner_github_info(self) -> Dict[str, SelfHostedRunner]:
             self._clients.github.actions.list_self_hosted_runners_for_repo(
                 owner=self.config.path.owner, repo=self.config.path.repo, per_page=100
             )
-            num_of_pages = self._clients.github.actions.last_page()
+            num_of_pages = self._clients.github.last_page()
             remote_runners_list = [
                 item
                 for page in pages(
@@ -446,7 +446,7 @@ def _get_runner_github_info(self) -> Dict[str, SelfHostedRunner]:
             self._clients.github.actions.list_self_hosted_runners_for_org(
                 org=self.config.path.org, per_page=100
             )
-            num_of_pages = self._clients.github.actions.last_page()
+            num_of_pages = self._clients.github.last_page()
             remote_runners_list = [
                 item
                 for page in pages(

From dc468adf89838f1b3e06cf2a61af070440069f26 Mon Sep 17 00:00:00 2001
From: Andrew Liaw <43424755+yhaliaw@users.noreply.github.com>
Date: Mon, 17 Jul 2023 15:58:26 +0800
Subject: [PATCH 16/16] Fix unit tests

---
 tests/unit/mock.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/unit/mock.py b/tests/unit/mock.py
index c0bb242c3..cd94cdc6f 100644
--- a/tests/unit/mock.py
+++ b/tests/unit/mock.py
@@ -189,6 +189,9 @@ def __init__(self, token: str):
         self.token = token
         self.actions = MockGhapiActions()
 
+    def last_page(self) -> int:
+        return 0
+
 
 class MockGhapiActions:
     """Mock for actions in Ghapi client."""
@@ -255,9 +258,6 @@ def delete_self_hosted_runner_from_repo(self, owner: str, repo: str, runner_id:
     def delete_self_hosted_runner_from_org(self, org: str, runner_id: str):
         pass
 
-    def last_page(self) -> int:
-        return 0
-
 
 class MockRepoPolicyComplianceClient:
     """Mock for RepoPolicyComplianceClient."""