diff --git a/config.yaml b/config.yaml index e238e5381..9870c47d7 100644 --- a/config.yaml +++ b/config.yaml @@ -33,13 +33,14 @@ options: type: string default: 7GiB description: > - Amount of memory to allocate per virtual machine runner. Positive integers with GiB suffix. + Amount of memory to allocate per virtual machine runner. Positive integers with KiB, MiB, GiB, + TiB, PiB, EiB suffix. vm-disk: type: string default: 10GiB description: > Amount of disk space to allocate to root disk for virtual machine runner. Positive integers - with GiB suffix. + with KiB, MiB, GiB, TiB, PiB, EiB suffix. reconcile-interval: type: int default: 10 diff --git a/metadata.yaml b/metadata.yaml index 9f912f1e5..7cc5e3668 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -10,14 +10,13 @@ issues: https://github.com/canonical/github-runner-operator/issues source: https://github.com/canonical/github-runner-operator summary: Creates a cluster of self-hosted github runners. description: | - A [Juju](https://juju.is/) [charm](https://juju.is/docs/olm/charmed-operators) - deploying self-hosted GitHub runners. - - Each unit of this charm will start a configurable number of LXD based containers - and virtual machines to host them. Each runner performs only one job, after which - it unregisters from GitHub to ensure that each job runs in a clean environment. - The charm will periodically check the number of idle runners and spawn or destroy them as - necessary to maintain the configured number of runners. Both the reconciliation interval and the - number of runners to maintain are configurable. + A [Juju](https://juju.is/) [charm](https://juju.is/docs/olm/charmed-operators) deploying + self-hosted GitHub runners. + + Each unit of this charm will start a configurable number of LXD based virtual machines to host + them. Each runner performs only one job, after which it unregisters from GitHub to ensure that + each job runs in a clean environment. The charm will periodically check the number of idle runners + and spawn or destroy them as necessary to maintain the configured number of runners. Both the + reconciliation interval and the number of runners to maintain are configurable. series: - - jammy + - jammy diff --git a/script/deploy_runner.sh b/script/deploy_runner.sh index 58ba5b0c8..2a6f4215c 100644 --- a/script/deploy_runner.sh +++ b/script/deploy_runner.sh @@ -30,5 +30,5 @@ unzip -p github_runner.zip > github-runner.charm rm github_runner.zip # Deploy the charm. -juju deploy ./github-runner.charm --series=jammy e2e-runner +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/src/charm.py b/src/charm.py index 9cc18d0f2..4f830953d 100755 --- a/src/charm.py +++ b/src/charm.py @@ -27,12 +27,12 @@ from ops.main import main from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus -from errors import RunnerError, SubprocessError +from errors import MissingConfigurationError, RunnerError, SubprocessError from event_timer import EventTimer, TimerDisableError, TimerEnableError from github_type import GitHubRunnerStatus from runner_manager import RunnerManager, RunnerManagerConfig from runner_type import GitHubOrg, GitHubRepo, ProxySetting, VirtualMachineResources -from utilities import execute_command, get_env_var, retry +from utilities import execute_command, get_env_var, retry, secure_run_subprocess if TYPE_CHECKING: from ops.model import JsonObject # pragma: no cover @@ -52,9 +52,7 @@ class UpdateRunnerBinEvent(EventBase): EventT = TypeVar("EventT") -def catch_unexpected_charm_errors( - func: Callable[[CharmT, EventT], None] -) -> Callable[[CharmT, EventT], None]: +def catch_charm_errors(func: Callable[[CharmT, EventT], None]) -> Callable[[CharmT, EventT], None]: """Catch unexpected errors in charm. This decorator is for unrecoverable errors and sets the charm to @@ -72,14 +70,16 @@ def func_with_catch_unexpected_errors(self, event: EventT) -> None: # Safe guard against unexpected error. try: func(self, event) - except Exception as err: # pylint: disable=broad-exception-caught - logger.exception(err) - self.unit.status = BlockedStatus(str(err)) + except MissingConfigurationError as err: + logger.exception("Missing required charm configuration") + self.unit.status = BlockedStatus( + f"Missing required charm configuration: {err.configs}" + ) return func_with_catch_unexpected_errors -def catch_unexpected_action_errors( +def catch_action_errors( func: Callable[[CharmT, ActionEvent], None] ) -> Callable[[CharmT, ActionEvent], None]: """Catch unexpected errors in actions. @@ -92,15 +92,15 @@ def catch_unexpected_action_errors( """ @functools.wraps(func) - def func_with_catch_unexpected_errors(self, event: ActionEvent) -> None: + def func_with_catch_errors(self, event: ActionEvent) -> None: # Safe guard against unexpected error. try: func(self, event) - except Exception as err: # pylint: disable=broad-exception-caught - logger.exception(err) - event.fail(f"Failed to get runner info: {err}") + except MissingConfigurationError as err: + logger.exception("Missing required charm configuration") + event.fail(f"Missing required charm configuration: {err.configs}") - return func_with_catch_unexpected_errors + return func_with_catch_errors class GithubRunnerCharm(CharmBase): @@ -112,6 +112,7 @@ class GithubRunnerCharm(CharmBase): repo_check_web_service_path = Path("/home/ubuntu/repo_policy_compliance_service") repo_check_web_service_script = Path("src/repo_policy_compliance_service.py") repo_check_systemd_service = Path("/etc/systemd/system/repo-policy-compliance.service") + lvm_vg_name = "ramdisk_pool" def __init__(self, *args, **kargs) -> None: """Construct the charm. @@ -155,9 +156,24 @@ def __init__(self, *args, **kargs) -> None: self.framework.observe(self.on.flush_runners_action, self._on_flush_runners_action) self.framework.observe(self.on.update_runner_bin_action, self._on_update_runner_bin) + def _ensure_ramdisk_volume_group(self) -> None: + """Ensure ramdisk LVM volume group exists.""" + # Check if ramdisk at /dev/ram0 exists. + result = secure_run_subprocess(["test", "-e", "/dev/ram0"]) + if result.returncode != 0: + # The block ram disk is set to 1 TiB size, as a way to not limit it. + # Block ram disk does not pre-allocate the memory. + # Each LXD instance memory usage is restricted through the LXD profile. + execute_command(["modprobe", "brd", "rd_size=1073741824", "rd_nr=1"]) + + # Check if volume group exits. + result = secure_run_subprocess(["vgdisplay", self.lvm_vg_name]) + if result.returncode != 0: + execute_command(["vgcreate", self.lvm_vg_name, "/dev/ram0"]) + def _get_runner_manager( self, token: Optional[str] = None, path: Optional[str] = None - ) -> Optional[RunnerManager]: + ) -> RunnerManager: """Get a RunnerManager instance, or None if missing config. Args: @@ -166,15 +182,22 @@ def _get_runner_manager( name. Returns: - A instance of RunnerManager if the token and path configuration can be found. + A instance of RunnerManager. """ if token is None: token = self.config["token"] if path is None: path = self.config["path"] - if not token or not path: - return None + missing_configs = [] + if not token: + missing_configs.append("token") + if not path: + missing_configs.append("path") + if missing_configs: + raise MissingConfigurationError(missing_configs) + + self._ensure_ramdisk_volume_group() if self.service_token is None: self.service_token = self._get_service_token() @@ -194,11 +217,11 @@ def _get_runner_manager( return RunnerManager( app_name, unit, - RunnerManagerConfig(path, token, "jammy", self.service_token), + RunnerManagerConfig(path, token, "jammy", self.service_token, self.lvm_vg_name), proxies=self.proxies, ) - @catch_unexpected_charm_errors + @catch_charm_errors def _on_install(self, _event: InstallEvent) -> None: """Handle the installation of charm. @@ -244,7 +267,7 @@ def _on_install(self, _event: InstallEvent) -> None: else: self.unit.status = BlockedStatus("Missing token or org/repo path config") - @catch_unexpected_charm_errors + @catch_charm_errors def _on_upgrade_charm(self, _event: UpgradeCharmEvent) -> None: """Handle the update of charm. @@ -263,7 +286,7 @@ def _on_upgrade_charm(self, _event: UpgradeCharmEvent) -> None: runner_manager.flush() self._reconcile_runners(runner_manager) - @catch_unexpected_charm_errors + @catch_charm_errors def _on_config_changed(self, _event: ConfigChangedEvent) -> None: """Handle the configuration change. @@ -298,7 +321,7 @@ def _on_config_changed(self, _event: ConfigChangedEvent) -> None: else: self.unit.status = BlockedStatus("Missing token or org/repo path config") - @catch_unexpected_charm_errors + @catch_charm_errors def _on_update_runner_bin(self, _event: UpdateRunnerBinEvent) -> None: """Handle checking update of runner binary event. @@ -337,7 +360,7 @@ def _on_update_runner_bin(self, _event: UpdateRunnerBinEvent) -> None: self.unit.status = ActiveStatus() - @catch_unexpected_charm_errors + @catch_charm_errors def _on_reconcile_runners(self, _event: ReconcileRunnersEvent) -> None: """Handle the reconciliation of runners. @@ -363,7 +386,7 @@ def _on_reconcile_runners(self, _event: ReconcileRunnersEvent) -> None: self.unit.status = ActiveStatus() - @catch_unexpected_action_errors + @catch_action_errors def _on_check_runners_action(self, event: ActionEvent) -> None: """Handle the action of checking of runner state. @@ -403,7 +426,7 @@ def _on_check_runners_action(self, event: ActionEvent) -> None: } ) - @catch_unexpected_action_errors + @catch_action_errors def _on_reconcile_runners_action(self, event: ActionEvent) -> None: """Handle the action of reconcile of runner state. @@ -420,7 +443,7 @@ def _on_reconcile_runners_action(self, event: ActionEvent) -> None: self._on_check_runners_action(event) event.set_results(delta) - @catch_unexpected_action_errors + @catch_action_errors def _on_flush_runners_action(self, event: ActionEvent) -> None: """Handle the action of flushing all runner and reconciling afterwards. @@ -438,7 +461,7 @@ def _on_flush_runners_action(self, event: ActionEvent) -> None: self._on_check_runners_action(event) event.set_results(delta) - @catch_unexpected_charm_errors + @catch_charm_errors def _on_stop(self, _: StopEvent) -> None: """Handle the stopping of the charm. @@ -483,7 +506,7 @@ def _reconcile_runners(self, runner_manager: RunnerManager) -> Dict[str, "JsonOb return {"delta": {"virtual-machines": delta_virtual_machines}} # Safe guard against transient unexpected error. except Exception as err: # pylint: disable=broad-exception-caught - logger.exception("Failed to update runner binary") + logger.exception("Failed to reconcile runners.") # Failure to reconcile runners is a transient error. # The charm automatically reconciles runners on a schedule. self.unit.status = MaintenanceStatus(f"Failed to reconcile runners: {err}") @@ -511,7 +534,6 @@ def _install_deps(self) -> None: [ "/usr/bin/pip", "install", - "flask", "git+https://github.com/canonical/repo-policy-compliance@main", ], env=env, @@ -528,6 +550,7 @@ def _install_deps(self) -> None: "cpu-checker", "libvirt-clients", "libvirt-daemon-driver-qemu", + "apparmor-utils", ], ) execute_command(["/usr/bin/snap", "install", "lxd", "--channel=latest/stable"]) @@ -538,6 +561,28 @@ def _install_deps(self) -> None: execute_command(["/snap/bin/lxc", "network", "set", "lxdbr0", "ipv6.address", "none"]) logger.info("Finished installing charm dependencies.") + logger.info("Setting apparmor to complain mode.") + self._apparmor_complain_mode( + [ + "/etc/apparmor.d/lsb_release", + "/etc/apparmor.d/nvidia_modprobe", + "/etc/apparmor.d/sbin.dhclient", + "/etc/apparmor.d/usr.bin.man", + "/etc/apparmor.d/usr.bin.tcpdump", + "/etc/apparmor.d/usr.lib.snapd.snap-confine.real", + "/etc/apparmor.d/usr.sbin.rsyslogd", + ] + ) + + def _apparmor_complain_mode(self, profiles: list[str]) -> None: + """Enable complain mode for the apparmor profile. + + Args: + profiles: Profiles to enable complain mode. + """ + for profile in profiles: + execute_command(["aa-complain", profile]) + @retry(tries=10, delay=15, max_delay=60, backoff=1.5) def _start_services(self) -> None: """Start services.""" diff --git a/src/errors.py b/src/errors.py index 7f8949410..85ce3a894 100644 --- a/src/errors.py +++ b/src/errors.py @@ -35,6 +35,24 @@ class RunnerBinaryError(RunnerError): """Error of getting runner binary.""" +class MissingConfigurationError(Exception): + """Error for missing juju configuration. + + Attrs: + configs: The missing configurations. + """ + + def __init__(self, configs: list[str]): + """Construct the MissingConfigurationError. + + Args: + configs: The missing configurations. + """ + super().__init__(f"Missing required charm configuration: {configs}") + + self.configs = configs + + class LxdError(Exception): """Error for executing LXD actions.""" diff --git a/src/lxd.py b/src/lxd.py index 4d2aaaa49..a9c1da2a3 100644 --- a/src/lxd.py +++ b/src/lxd.py @@ -20,6 +20,7 @@ LxdNetwork, LxdResourceProfileConfig, LxdResourceProfileDevices, + LxdStoragePoolConfiguration, ) from utilities import execute_command, secure_run_subprocess @@ -153,15 +154,14 @@ class LxdInstance: status (str): Status of the LXD instance. """ - def __init__(self, name: str, pylxd_instance: pylxd.models.Instance): + def __init__(self, pylxd_instance: pylxd.models.Instance): """Construct the LXD instance representation. Args: - name: Name of the LXD instance. pylxd_instance: Instance of pylxd.models.Instance for the LXD instance. """ - self.name = name self._pylxd_instance = pylxd_instance + self.name = self._pylxd_instance.name self.files = LxdInstanceFileManager(self._pylxd_instance) @property @@ -266,10 +266,7 @@ def all(self) -> list[LxdInstance]: List of LXD instances. """ try: - return [ - LxdInstance(instance.name, instance) - for instance in self._pylxd_client.instances.all() - ] + return [LxdInstance(instance) for instance in self._pylxd_client.instances.all()] except pylxd.exceptions.LXDAPIException as err: logger.exception("Failed to get all LXD instance") raise LxdError("Unable to get all LXD instances") from err @@ -289,7 +286,7 @@ def create(self, config: LxdInstanceConfig, wait: bool) -> LxdInstance: """ try: pylxd_instance = self._pylxd_client.instances.create(config=config, wait=wait) - return LxdInstance(config["name"], pylxd_instance) + return LxdInstance(pylxd_instance) except pylxd.exceptions.LXDAPIException as err: logger.exception("Failed to create LXD instance") raise LxdError(f"Unable to create LXD instance {config['name']}") from err @@ -376,6 +373,101 @@ def get(self, name: str) -> LxdNetwork: ) +class LxdStoragePoolManager: + """LXD storage pool manager.""" + + def __init__(self, pylxd_client: pylxd.Client): + """Construct the LXD storage pool manager. + + Args: + pylxd_client: Instance of pylxd.Client. + """ + self._pylxd_client = pylxd_client + + def all(self) -> list[LxdStoragePool]: + """Get all LXD storage pool. + + Returns: + List of LXD storage pools. + """ + return [LxdStoragePool(pool) for pool in self._pylxd_client.storage_pools.all()] + + def get(self, name: str) -> LxdStoragePool: + """Get a LXD storage pool. + + Args: + name: Name of the storage pool. + + Returns: + The LXD storage pool. + """ + try: + return LxdStoragePool(self._pylxd_client.storage_pools.get(name)) + except pylxd.exceptions.NotFound as err: + logger.exception("LXD storage pool not found") + raise LxdError(f"LXD storage pool {name} not found") from err + + def exists(self, name: str) -> bool: + """Check if a LXD storage pool exists. + + Args: + name: Name to check for. + + Returns: + Whether the storage pool exists. + """ + return self._pylxd_client.storage_pools.exists(name) + + def create(self, config: LxdStoragePoolConfiguration) -> LxdStoragePool: + """Create a LXD storage pool. + + Args: + config: Configuration for the storage pool. + + Returns: + The LXD storage pool. + """ + return self._pylxd_client.storage_pools.create(config) + + +class LxdStoragePool: + """A LXD storage pool. + + Attrs: + name (str): Name of the storage pool. + driver (str): Type of driver of the storage pool. + used_by (list[str]): LXD instance that uses the storage pool. + config (dict[str, any]): Dictionary of the configuration of the storage pool. + managed (bool): Whether LXD manages the storage pool. + """ + + def __init__( + self, + pylxd_storage_pool: pylxd.models.StoragePool, + ): + """Construct the LXD storage pool. + + Args: + pylxd_storage_pool: Instance of the pylxd.models.StoragePool. + """ + self._pylxd_storage_pool = pylxd_storage_pool + + self.name = self._pylxd_storage_pool.name + self.driver = self._pylxd_storage_pool.driver + self.used_by = self._pylxd_storage_pool.used_by + self.config = self._pylxd_storage_pool.config + self.managed = self._pylxd_storage_pool.managed + + def save(self): + """Save the current configuration of storage pool.""" + self._pylxd_storage_pool.config = self.config + self._pylxd_storage_pool.save() + + def delete(self): + """Delete the storage pool.""" + self._pylxd_storage_pool.delete() + + # Disable pylint as the public methods of this class in split into instances and profiles. class LxdClient: # pylint: disable=too-few-public-methods """LXD client.""" @@ -386,3 +478,4 @@ def __init__(self): self.instances = LxdInstanceManager(pylxd_client) self.profiles = LxdProfileManager(pylxd_client) self.networks = LxdNetworkManager(pylxd_client) + self.storage_pools = LxdStoragePoolManager(pylxd_client) diff --git a/src/lxd_type.py b/src/lxd_type.py index 7e6d0dbc4..c1d0c7a5b 100644 --- a/src/lxd_type.py +++ b/src/lxd_type.py @@ -1,7 +1,17 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. -"""Types used by Lxd class.""" +"""Types used by Lxd class. + +The details of the configuration of different types of devices can be found here: +https://linuxcontainers.org/lxd/docs/latest/reference/devices/ + +For example, configuration for disk: +https://linuxcontainers.org/lxd/docs/latest/reference/devices_disk/# + +The unit of storage and network limits can be found here: +https://linuxcontainers.org/lxd/docs/latest/reference/instance_units/#instances-limit-units +""" from __future__ import annotations @@ -16,17 +26,7 @@ class LxdResourceProfileDevicesDisk(TypedDict): - """LXD device profile of disk. - - The details of the configuration of different types of devices can be found here: - https://linuxcontainers.org/lxd/docs/latest/reference/devices/ - - For example, configuration for disk: - https://linuxcontainers.org/lxd/docs/latest/reference/devices_disk/# - - The unit of storage and network limits can be found here: - https://linuxcontainers.org/lxd/docs/latest/reference/instance_units/#instances-limit-units - """ + """LXD device profile of disk.""" path: str pool: str @@ -65,6 +65,21 @@ class LxdInstanceConfig(TypedDict): LxdNetworkConfig.__doc__ = "Represent LXD network configuration." +class LxdStoragePoolConfig(TypedDict): + """Configuration of the storage pool.""" + + source: str + size: str + + +class LxdStoragePoolConfiguration(TypedDict): + """Configuration for LXD storage pool.""" + + name: str + driver: str + config: LxdStoragePoolConfig + + @dataclass class LxdNetwork: """LXD network information.""" diff --git a/src/runner.py b/src/runner.py index 94b2c3ff5..87fc95efa 100644 --- a/src/runner.py +++ b/src/runner.py @@ -201,6 +201,7 @@ def _create_instance( """ logger.info("Creating an LXD instance for runner: %s", self.config.name) + self._ensure_runner_storage_pool() self._ensure_runner_profile() resource_profile = self._get_resource_profile(resources) @@ -243,6 +244,24 @@ def _ensure_runner_profile(self) -> None: else: logger.info("Found existing runner LXD profile") + @retry(tries=5, delay=5, local_logger=logger) + def _ensure_runner_storage_pool(self) -> None: + if not self._clients.lxd.storage_pools.exists("ram"): + logger.info("Creating runner LXD storage pool.") + self._clients.lxd.storage_pools.create( + { + "name": "ram", + "driver": "lvm", + "config": {"source": self.config.lvm_vg_name}, + } + ) + + # Verify the action is successful. + if not self._clients.lxd.storage_pools.exists("ram"): + raise RunnerError("Failed to create ram LXD storage pool") + else: + logger.info("Found existing ram LXD storage pool.") + @retry(tries=5, delay=1, local_logger=logger) def _get_resource_profile(self, resources: VirtualMachineResources) -> str: """Get the LXD profile name of given resource limit. @@ -268,7 +287,7 @@ def _get_resource_profile(self, resources: VirtualMachineResources) -> str: resource_profile_devices = { "root": { "path": "/", - "pool": "default", + "pool": "ram", "type": "disk", "size": resources.disk, } diff --git a/src/runner_manager.py b/src/runner_manager.py index eebe7dddc..58d106816 100644 --- a/src/runner_manager.py +++ b/src/runner_manager.py @@ -49,12 +49,16 @@ class RunnerManagerConfig: name. token: GitHub personal access token to register runner to the repository or organization. + image: Name of the image for creating LXD instance. + service_token: Token for accessing local service. + lvm_vg_name: LVM volume group for use as runner storage. """ path: GitHubPath token: str image: str service_token: str + lvm_vg_name: str @dataclass @@ -204,7 +208,7 @@ def update_runner_bin(self, binary: RunnerApplication) -> None: sha256 = hashlib.sha256() with RunnerManager.runner_bin_path.open(mode="wb") as file: - for chunk in response.iter_content(decode_unicode=False): + for chunk in response.iter_content(chunk_size=128 * 1024, decode_unicode=False): file.write(chunk) sha256.update(chunk) @@ -302,6 +306,7 @@ def reconcile(self, quantity: int, resources: VirtualMachineResources) -> int: self.app_name, self.config.path, self.proxies, + self.config.lvm_vg_name, self._generate_runner_name(), ) runner = Runner(self._clients, config, RunnerStatus()) @@ -428,7 +433,9 @@ def create_runner_info( online = getattr(remote_runner, "status", None) == "online" busy = getattr(remote_runner, "busy", None) - config = RunnerConfig(self.app_name, self.config.path, self.proxies, name) + config = RunnerConfig( + self.app_name, self.config.path, self.proxies, self.config.lvm_vg_name, name + ) return Runner( self._clients, config, diff --git a/src/runner_type.py b/src/runner_type.py index 83530a5b2..755d44c23 100644 --- a/src/runner_type.py +++ b/src/runner_type.py @@ -80,6 +80,7 @@ class RunnerConfig: app_name: str path: GitHubPath proxies: ProxySetting + lvm_vg_name: str name: str diff --git a/tests/unit/mock.py b/tests/unit/mock.py index 1238c69e8..7c3bfdac0 100644 --- a/tests/unit/mock.py +++ b/tests/unit/mock.py @@ -30,16 +30,17 @@ class MockLxdClient: - """Mock the behavior of the lxd client.""" + """Mock the behavior of the LXD client.""" def __init__(self): self.instances = MockLxdInstanceManager() self.profiles = MockLxdProfileManager() self.networks = MockLxdNetworkManager() + self.storage_pools = MockLxdStoragePoolManager() class MockLxdInstanceManager: - """Mock the behavior of the lxd Instances.""" + """Mock the behavior of the LXD Instances.""" def __init__(self): self.instances = {} @@ -56,7 +57,7 @@ def all(self): class MockLxdProfileManager: - """Mock the behavior of the lxd Profiles.""" + """Mock the behavior of the LXD Profiles.""" def __init__(self): self.profiles = set() @@ -69,7 +70,7 @@ def exists(self, name) -> bool: class MockLxdNetworkManager: - """Mock the behavior of the lxd networks""" + """Mock the behavior of the LXD networks""" def __init__(self): pass @@ -81,7 +82,7 @@ def get(self, name: str) -> LxdNetwork: class MockLxdInstance: - """Mock the behavior of a lxd Instance.""" + """Mock the behavior of a LXD Instance.""" def __init__(self, name: str): self.name = name @@ -106,7 +107,7 @@ def execute(self, cmd: Sequence[str], cwd: Optional[str] = None) -> tuple[int, s class MockLxdInstanceFileManager: - """Mock the behavior of a lxd Instance files.""" + """Mock the behavior of a LXD Instance files.""" def __init__(self): self.files = {} @@ -124,6 +125,42 @@ def read_file(self, filepath: str): return self.files.get(str(filepath), None) +class MockLxdStoragePoolManager: + """Mock the behavior of LXD storage pools.""" + + def __init__(self): + self.pools = {} + + def all(self): + return [pool for pool in self.pools.values() if not pool.delete] + + def get(self, name): + return self.pools[name] + + def exists(self, name): + if name in self.pools: + return not self.pools[name].delete + else: + return False + + def create(self, config): + self.pools[config["name"]] = MockLxdStoragePool() + return self.pools[config["name"]] + + +class MockLxdStoragePool: + """Mock the behavior of a LXD storage pool.""" + + def __init__(self): + self.delete = False + + def save(self): + pass + + def delete(self): + self.delete = True + + class MockErrorResponse: """Mock of an error response for request library.""" diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 72e4f00af..8294e2bf4 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -12,16 +12,12 @@ from ops.testing import Harness from charm import GithubRunnerCharm -from errors import RunnerError, SubprocessError +from errors import MissingConfigurationError, RunnerError, SubprocessError from github_type import GitHubRunnerStatus from runner_manager import RunnerInfo, RunnerManagerConfig from runner_type import GitHubOrg, GitHubRepo, VirtualMachineResources -def raise_error(*args, **kargs): - raise Exception("mock error") - - def raise_runner_error(*args, **kargs): raise RunnerError("mock error") @@ -89,9 +85,10 @@ def test_install(self, run, wt): run.assert_has_calls(calls, any_order=True) @patch("charm.RunnerManager") + @patch("pathlib.Path.mkdir") @patch("pathlib.Path.write_text") @patch("subprocess.run") - def test_org_register(self, run, wt, rm): + def test_org_register(self, run, wt, mkdir, rm): harness = Harness(GithubRunnerCharm) harness.update_config( { @@ -112,14 +109,16 @@ def test_org_register(self, run, wt, rm): token="mocktoken", image="jammy", service_token=token, + lvm_vg_name=GithubRunnerCharm.lvm_vg_name, ), proxies={}, ) @patch("charm.RunnerManager") + @patch("pathlib.Path.mkdir") @patch("pathlib.Path.write_text") @patch("subprocess.run") - def test_repo_register(self, run, wt, rm): + def test_repo_register(self, run, wt, mkdir, rm): harness = Harness(GithubRunnerCharm) harness.update_config( {"path": "mockorg/repo", "token": "mocktoken", "reconcile-interval": 5} @@ -135,14 +134,16 @@ def test_repo_register(self, run, wt, rm): token="mocktoken", image="jammy", service_token=token, + lvm_vg_name=GithubRunnerCharm.lvm_vg_name, ), proxies={}, ) @patch("charm.RunnerManager") + @patch("pathlib.Path.mkdir") @patch("pathlib.Path.write_text") @patch("subprocess.run") - def test_update_config(self, run, wt, rm): + def test_update_config(self, run, wt, mkdir, rm): rm.return_value = mock_rm = MagicMock() harness = Harness(GithubRunnerCharm) harness.update_config({"path": "mockorg/repo", "token": "mocktoken"}) @@ -160,6 +161,7 @@ def test_update_config(self, run, wt, rm): token="mocktoken", image="jammy", service_token=token, + lvm_vg_name=GithubRunnerCharm.lvm_vg_name, ), proxies={}, ) @@ -178,6 +180,7 @@ def test_update_config(self, run, wt, rm): token="mocktoken", image="jammy", service_token=token, + lvm_vg_name=GithubRunnerCharm.lvm_vg_name, ), proxies={}, ) @@ -187,9 +190,10 @@ def test_update_config(self, run, wt, rm): mock_rm.reset_mock() @patch("charm.RunnerManager") + @patch("pathlib.Path.mkdir") @patch("pathlib.Path.write_text") @patch("subprocess.run") - def test_on_stop(self, run, wt, rm): + def test_on_stop(self, run, wt, mkdir, rm): rm.return_value = mock_rm = MagicMock() harness = Harness(GithubRunnerCharm) harness.update_config({"path": "mockorg/repo", "token": "mocktoken"}) @@ -197,16 +201,18 @@ def test_on_stop(self, run, wt, rm): harness.charm.on.stop.emit() mock_rm.flush.assert_called() + @patch("pathlib.Path.mkdir") @patch("pathlib.Path.write_text") @patch("subprocess.run") - def test_get_runner_manager(self, run, wt): + def test_get_runner_manager(self, run, wt, mkdir): harness = Harness(GithubRunnerCharm) harness.begin() # Get runner manager via input. assert harness.charm._get_runner_manager("mocktoken", "mockorg/repo") is not None - assert harness.charm._get_runner_manager() is None + with self.assertRaises(MissingConfigurationError): + harness.charm._get_runner_manager() # Get runner manager via config. harness.update_config({"path": "mockorg/repo", "token": "mocktoken"}) @@ -216,9 +222,10 @@ def test_get_runner_manager(self, run, wt): assert harness.charm._get_runner_manager("mocktoken", "mock/invalid/path") is None @patch("charm.RunnerManager") + @patch("pathlib.Path.mkdir") @patch("pathlib.Path.write_text") @patch("subprocess.run") - def test_on_install_failure(self, run, wt, rm): + def test_on_install_failure(self, run, wt, mkdir, rm): """Test various error thrown during install.""" rm.return_value = mock_rm = MagicMock() @@ -238,28 +245,15 @@ def test_on_install_failure(self, run, wt, rm): "Failed to start runners: mock error" ) - harness.charm._reconcile_runners = raise_error - harness.charm.on.install.emit() - assert harness.charm.unit.status == BlockedStatus("mock error") - - mock_rm.update_runner_bin = raise_error - harness.charm.on.install.emit() - assert harness.charm.unit.status == MaintenanceStatus( - "Failed to update runner binary: mock error" - ) - GithubRunnerCharm._install_deps = raise_subprocess_error harness.charm.on.install.emit() assert harness.charm.unit.status == BlockedStatus("Failed to install dependencies") - GithubRunnerCharm._install_deps = raise_error - harness.charm.on.install.emit() - assert harness.charm.unit.status == BlockedStatus("mock error") - @patch("charm.RunnerManager") + @patch("pathlib.Path.mkdir") @patch("pathlib.Path.write_text") @patch("subprocess.run") - def test_on_update_runner_bin(self, run, wt, rm): + def test_on_update_runner_bin(self, run, wt, mkdir, rm): rm.return_value = mock_rm = MagicMock() mock_rm.get_latest_runner_bin_url = mock_get_latest_runner_bin_url @@ -269,10 +263,6 @@ def test_on_update_runner_bin(self, run, wt, rm): harness.charm.on.update_runner_bin.emit() - mock_rm.get_latest_runner_bin_url = raise_error - harness.charm.on.update_runner_bin.emit() - assert harness.charm.unit.status == BlockedStatus("mock error") - mock_rm.get_latest_runner_bin_url = raise_url_error harness.charm.on.update_runner_bin.emit() assert harness.charm.unit.status == MaintenanceStatus( @@ -280,9 +270,10 @@ def test_on_update_runner_bin(self, run, wt, rm): ) @patch("charm.RunnerManager") + @patch("pathlib.Path.mkdir") @patch("pathlib.Path.write_text") @patch("subprocess.run") - def test_check_runners_action(self, run, wt, rm): + def test_check_runners_action(self, run, wt, mkdir, rm): rm.return_value = mock_rm = MagicMock() mock_event = MagicMock() @@ -298,9 +289,10 @@ def test_check_runners_action(self, run, wt, rm): ) @patch("charm.RunnerManager") + @patch("pathlib.Path.mkdir") @patch("pathlib.Path.write_text") @patch("subprocess.run") - def test_check_runners_action_with_errors(self, run, wt, rm): + def test_check_runners_action_with_errors(self, run, wt, mkdir, rm): mock_event = MagicMock() harness = Harness(GithubRunnerCharm) @@ -308,27 +300,27 @@ def test_check_runners_action_with_errors(self, run, wt, rm): # No config harness.charm._on_check_runners_action(mock_event) - mock_event.fail.assert_called_with("Missing token or org/repo path config") + mock_event.fail.assert_called_with( + "Missing required charm configuration: ['token', 'path']" + ) @patch("charm.RunnerManager") + @patch("pathlib.Path.mkdir") @patch("pathlib.Path.write_text") @patch("subprocess.run") - def test_on_flush_runners_action(self, run, wt, rm): + def test_on_flush_runners_action(self, run, wt, mkdir, rm): mock_event = MagicMock() harness = Harness(GithubRunnerCharm) harness.begin() harness.charm._on_flush_runners_action(mock_event) - mock_event.fail.assert_called_with("Missing token or org/repo path config") + mock_event.fail.assert_called_with( + "Missing required charm configuration: ['token', 'path']" + ) mock_event.reset_mock() harness.update_config({"path": "mockorg/repo", "token": "mocktoken"}) harness.charm._on_flush_runners_action(mock_event) mock_event.set_results.assert_called() mock_event.reset_mock() - - harness.charm._reconcile_runners = raise_error - harness.charm._on_flush_runners_action(mock_event) - mock_event.fail.assert_called() - mock_event.reset_mock() diff --git a/tests/unit/test_runner.py b/tests/unit/test_runner.py index 32e89811a..e4f9bf71b 100644 --- a/tests/unit/test_runner.py +++ b/tests/unit/test_runner.py @@ -10,7 +10,7 @@ import pytest -from errors import RunnerCreateError +from errors import RunnerCreateError, RunnerRemoveError from runner import Runner, RunnerClients, RunnerConfig, RunnerStatus from runner_type import GitHubOrg, GitHubRepo, VirtualMachineResources from tests.unit.mock import ( @@ -62,14 +62,16 @@ def mock_lxd_client_fixture(): ), ], ) -def runner_fixture(request, lxd: MockLxdClient): +def runner_fixture(request, lxd: MockLxdClient, tmp_path: Path): client = RunnerClients( MagicMock(), MagicMock(), lxd, MockRepoPolicyComplianceClient(), ) - config = RunnerConfig("test_app", request.param[0], request.param[1], "test_runner") + pool_path = tmp_path / "test_storage" + pool_path.mkdir(exist_ok=True) + config = RunnerConfig("test_app", request.param[0], request.param[1], pool_path, "test_runner") status = RunnerStatus() return Runner( client, @@ -199,3 +201,42 @@ def test_remove_none( runner.remove(token) assert len(lxd.instances.all()) == 0 + + +def test_remove_with_stop_error( + runner: Runner, + vm_resources: VirtualMachineResources, + token: str, + binary_path: Path, + lxd: MockLxdClient, +): + """ + arrange: Create a runner. Set up LXD stop fails with LxdError. + act: Remove the runner. + assert: RunnerRemoveError is raised. + """ + runner.create("test_image", vm_resources, binary_path, token) + runner.instance.stop = mock_lxd_error_func + + with pytest.raises(RunnerRemoveError): + runner.remove("test_token") + + +def test_remove_with_delete_error( + runner: Runner, + vm_resources: VirtualMachineResources, + token: str, + binary_path: Path, + lxd: MockLxdClient, +): + """ + arrange: Create a runner. Set up LXD delete fails with LxdError. + act: Remove the runner. + assert: RunnerRemoveError is raised. + """ + runner.create("test_image", vm_resources, binary_path, token) + runner.instance.status = "Stopped" + runner.instance.delete = mock_lxd_error_func + + with pytest.raises(RunnerRemoveError): + runner.remove("test_token") diff --git a/tests/unit/test_runner_manager.py b/tests/unit/test_runner_manager.py index 31fac9d89..02358867e 100644 --- a/tests/unit/test_runner_manager.py +++ b/tests/unit/test_runner_manager.py @@ -34,12 +34,14 @@ def token_fixture(): ) def runner_manager_fixture(request, tmp_path, monkeypatch, token): monkeypatch.setattr( - "runner_manager.RunnerManager.runner_bin_path", Path(tmp_path / "mock_runner_binary") + "runner_manager.RunnerManager.runner_bin_path", tmp_path / "mock_runner_binary" ) + pool_path = tmp_path / "test_storage" + pool_path.mkdir(exist_ok=True) runner_manager = RunnerManager( "test app", "0", - RunnerManagerConfig(request.param[0], token, "jammy", secrets.token_hex(16)), + RunnerManagerConfig(request.param[0], token, "jammy", secrets.token_hex(16), pool_path), proxies=request.param[1], ) return runner_manager