From 3ba49e2e2057ceb0b94f76018196a28e0f19b23d Mon Sep 17 00:00:00 2001 From: Yanks Yoon <37652070+yanksyoon@users.noreply.github.com> Date: Tue, 10 Sep 2024 13:08:55 +0800 Subject: [PATCH 01/21] hotfix: go modules (#23) * hotfix: go mod tidy * hotfix: add test * chore: increment version --- pyproject.toml | 2 +- src-docs/builder.md | 66 ++++ src-docs/chroot.md | 73 ++++ src-docs/cli.md | 14 + src-docs/cloud_image.md | 42 +++ src-docs/config.md | 77 ++++ src-docs/errors.md | 328 ++++++++++++++++++ src-docs/openstack_builder.md | 137 ++++++++ src-docs/store.md | 111 ++++++ src-docs/utils.md | 44 +++ src/github_runner_image_builder/builder.py | 5 + .../templates/cloud-init.sh.j2 | 1 + tests/unit/test_openstack_builder.py | 1 + 13 files changed, 900 insertions(+), 1 deletion(-) create mode 100644 src-docs/builder.md create mode 100644 src-docs/chroot.md create mode 100644 src-docs/cli.md create mode 100644 src-docs/cloud_image.md create mode 100644 src-docs/config.md create mode 100644 src-docs/errors.md create mode 100644 src-docs/openstack_builder.md create mode 100644 src-docs/store.md create mode 100644 src-docs/utils.md diff --git a/pyproject.toml b/pyproject.toml index 03abc5c..db470d0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-image-builder" -version = "0.4.2" +version = "0.4.3" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] diff --git a/src-docs/builder.md b/src-docs/builder.md new file mode 100644 index 0000000..b84f417 --- /dev/null +++ b/src-docs/builder.md @@ -0,0 +1,66 @@ + + + + +# module `builder` +Module for interacting with qemu image builder. + +**Global Variables** +--------------- +- **IMAGE_DEFAULT_APT_PACKAGES** +- **APT_DEPENDENCIES** +- **APT_NONINTERACTIVE_ENV** +- **SNAP_GO** +- **RESIZE_AMOUNT** +- **APT_TIMER** +- **APT_SVC** +- **APT_UPGRADE_TIMER** +- **APT_UPGRAD_SVC** +- **UBUNTU_USER** +- **DOCKER_GROUP** +- **MICROK8S_GROUP** +- **LXD_GROUP** +- **SUDOERS_GROUP** +- **YQ_REPOSITORY_URL** +- **IMAGE_HWE_PKG_FORMAT** + +--- + + + +## function `initialize` + +```python +initialize() → None +``` + +Configure the host machine to build images. + + +--- + + + +## function `run` + +```python +run(arch: Arch, base_image: BaseImage, runner_version: str) → None +``` + +Build and save the image locally. + + + +**Args:** + + - `arch`: The CPU architecture to build the image for. + - `base_image`: The ubuntu image to use as build base. + - `runner_version`: The GitHub runner version to embed. + + + +**Raises:** + + - `BuildImageError`: If there was an error building the image. + + diff --git a/src-docs/chroot.md b/src-docs/chroot.md new file mode 100644 index 0000000..ecb7c1e --- /dev/null +++ b/src-docs/chroot.md @@ -0,0 +1,73 @@ + + + + +# module `chroot` +Context manager for chrooting. + +**Global Variables** +--------------- +- **CHROOT_DEVICE_DIR** +- **CHROOT_SHARED_DIRS** + + +--- + + + +## class `ChrootBaseError` +Represents the errors with chroot. + + + + + +--- + + + +## class `MountError` +Represents an error while (un)mounting shared dirs. + + + + + +--- + + + +## class `SyncError` +Represents an error while syncing chroot dir. + + + + + +--- + + + +## class `ChrootContextManager` +A helper class for managing chroot environments. + + + +### method `__init__` + +```python +__init__(chroot_path: Path) +``` + +Initialize the chroot context manager. + + + +**Args:** + + - `chroot_path`: The path to set as new root. + + + + + diff --git a/src-docs/cli.md b/src-docs/cli.md new file mode 100644 index 0000000..8eb9e32 --- /dev/null +++ b/src-docs/cli.md @@ -0,0 +1,14 @@ + + + + +# module `cli` +Main entrypoint for github-runner-image-builder cli application. + +**Global Variables** +--------------- +- **click** +- **BASE_CHOICES** +- **LOG_LEVELS** + + diff --git a/src-docs/cloud_image.md b/src-docs/cloud_image.md new file mode 100644 index 0000000..f2d8e10 --- /dev/null +++ b/src-docs/cloud_image.md @@ -0,0 +1,42 @@ + + + + +# module `cloud_image` +Module for downloading images from cloud-images.ubuntu.com. + +**Global Variables** +--------------- +- **CHECKSUM_BUF_SIZE** + +--- + + + +## function `download_and_validate_image` + +```python +download_and_validate_image(arch: Arch, base_image: BaseImage) → Path +``` + +Download and verify the base image from cloud-images.ubuntu.com. + + + +**Args:** + + - `arch`: The base image architecture to download. + - `base_image`: The ubuntu base image OS to download. + + + +**Returns:** + The downloaded image path. + + + +**Raises:** + + - `BaseImageDownloadError`: If there was an error with downloading/verifying the image. + + diff --git a/src-docs/config.md b/src-docs/config.md new file mode 100644 index 0000000..595d867 --- /dev/null +++ b/src-docs/config.md @@ -0,0 +1,77 @@ + + + + +# module `config` +Module containing configurations. + +**Global Variables** +--------------- +- **ARCHITECTURES_ARM64** +- **ARCHITECTURES_X86** +- **LTS_IMAGE_VERSION_TAG_MAP** +- **BASE_CHOICES** +- **IMAGE_DEFAULT_APT_PACKAGES** +- **LOG_LEVELS** + +--- + + + +## function `get_supported_arch` + +```python +get_supported_arch() → +``` + +Get current machine architecture. + + + +**Raises:** + + - `UnsupportedArchitectureError`: if the current architecture is unsupported. + + + +**Returns:** + + - `Arch`: Current machine architecture. + + +--- + + + +## class `Arch` +Supported system architectures. + + + +**Attributes:** + + - `ARM64`: Represents an ARM64 system architecture. + - `X64`: Represents an X64/AMD64 system architecture. + + + + + +--- + + + +## class `BaseImage` +The ubuntu OS base image to build and deploy runners on. + + + +**Attributes:** + + - `JAMMY`: The jammy ubuntu LTS image. + - `NOBLE`: The noble ubuntu LTS image. + + + + + diff --git a/src-docs/errors.md b/src-docs/errors.md new file mode 100644 index 0000000..bf51e74 --- /dev/null +++ b/src-docs/errors.md @@ -0,0 +1,328 @@ + + + + +# module `errors` +Module containing error definitions. + + + +--- + + + +## class `ImageBuilderBaseError` +Represents an error with any builder related executions. + + + + + +--- + + + +## class `BuilderInitializeError` +Represents an error while setting up host machine as builder. + + + + + +--- + + + +## class `DependencyInstallError` +Represents an error while installing required dependencies. + + + + + +--- + + + +## class `NetworkBlockDeviceError` +Represents an error while enabling network block device. + + + + + +--- + + + +## class `UnsupportedArchitectureError` +Raised when given machine architecture is unsupported. + + + + + +--- + + + +## class `BuildImageError` +Represents an error while building the image. + + + + + +--- + + + +## class `UnmountBuildPathError` +Represents an error while unmounting build path. + + + + + +--- + + + +## class `BaseImageDownloadError` +Represents an error downloading base image. + + + + + +--- + + + +## class `ImageResizeError` +Represents an error while resizing the image. + + + + + +--- + + + +## class `ImageConnectError` +Represents an error while connecting the image to network block device. + + + + + +--- + + + +## class `ResizePartitionError` +Represents an error while resizing network block device partitions. + + + + + +--- + + + +## class `UnattendedUpgradeDisableError` +Represents an error while disabling unattended-upgrade related services. + + + + + +--- + + + +## class `SystemUserConfigurationError` +Represents an error while adding user to chroot env. + + + + + +--- + + + +## class `PermissionConfigurationError` +Represents an error while modifying dir permissions. + + + + + +--- + + + +## class `YQBuildError` +Represents an error while building yq binary from source. + + + + + +--- + + + +## class `YarnInstallError` +Represents an error installilng Yarn. + + + + + +--- + + + +## class `RunnerDownloadError` +Represents an error downloading GitHub runner tar archive. + + + + + +--- + + + +## class `ImageCompressError` +Represents an error while compressing cloud-img. + + + + + +--- + + + +## class `OpenstackBaseError` +Represents an error while interacting with Openstack. + + + + + +--- + + + +## class `UnauthorizedError` +Represents an unauthorized connection to Openstack. + + + + + +--- + + + +## class `UploadImageError` +Represents an error when uploading image to Openstack. + + + + + +--- + + + +## class `OpenstackError` +Represents an error while communicating with Openstack. + + + + + +--- + + + +## class `CloudsYAMLError` +Represents an error with clouds.yaml for OpenStack connection. + + + + + +--- + + + +## class `NotFoundError` +Represents an error with not matching OpenStack object found. + + + + + +--- + + + +## class `FlavorNotFoundError` +Represents an error with given OpenStack flavor not found. + + + + + +--- + + + +## class `FlavorRequirementsNotMetError` +Represents an error with given OpenStack flavor not meeting the minimum requirements. + + + + + +--- + + + +## class `NetworkNotFoundError` +Represents an error with given OpenStack network not found. + + + + + +--- + + + +## class `AddressNotFoundError` +Represents an error with OpenStack instance not receiving an IP address. + + + + + +--- + + + +## class `CloudInitFailError` +Represents an error with cloud-init. + + + + + diff --git a/src-docs/openstack_builder.md b/src-docs/openstack_builder.md new file mode 100644 index 0000000..b9137c3 --- /dev/null +++ b/src-docs/openstack_builder.md @@ -0,0 +1,137 @@ + + + + +# module `openstack_builder` +Module for interacting with external openstack VM image builder. + +**Global Variables** +--------------- +- **IMAGE_DEFAULT_APT_PACKAGES** +- **CLOUD_YAML_PATHS** +- **BUILDER_SSH_KEY_NAME** +- **SHARED_SECURITY_GROUP_NAME** +- **IMAGE_SNAPSHOT_NAME** +- **CREATE_SERVER_TIMEOUT** +- **MIN_CPU** +- **MIN_RAM** +- **MIN_DISK** + +--- + + + +## function `determine_cloud` + +```python +determine_cloud(cloud_name: str | None = None) → str +``` + +Automatically determine cloud to use from clouds.yaml by selecting the first cloud. + + + +**Args:** + + - `cloud_name`: str + + + +**Raises:** + + - `CloudsYAMLError`: if clouds.yaml was not found. + + + +**Returns:** + The cloud name to use. + + +--- + + + +## function `initialize` + +```python +initialize(arch: Arch, cloud_name: str) → None +``` + +Initialize the OpenStack external image builder. + +Upload ubuntu base images to openstack to use as builder base. This is a separate method to mitigate race conditions from happening during parallel runs (multiprocess) of the image builder, by creating shared resources beforehand. + + + +**Args:** + + - `arch`: The architecture of the image to seed. + - `cloud_name`: The cloud to use from the clouds.yaml file. + + +--- + + + +## function `run` + +```python +run( + arch: Arch, + base: BaseImage, + cloud_config: CloudConfig, + runner_version: str, + keep_revisions: int +) → str +``` + +Run external OpenStack builder instance and create a snapshot. + + + +**Args:** + + - `arch`: The architecture of the image to seed. + - `base`: The Ubuntu base to use as builder VM base. + - `cloud_config`: The OpenStack cloud configuration values for builder VM. + - `runner_version`: The GitHub runner version to install on the VM. Defaults to latest. + - `keep_revisions`: The number of image to keep for snapshot before deletion. + + + +**Returns:** + The Openstack snapshot image ID. + + +--- + + + +## class `CloudConfig` +The OpenStack cloud configuration values. + + + +**Attributes:** + + - `cloud_name`: The OpenStack cloud name to use. + - `flavor`: The OpenStack flavor to launch builder VMs on. + - `network`: The OpenStack network to launch the builder VMs on. + - `proxy`: The proxy to enable on builder VMs. + + + +### method `__init__` + +```python +__init__(cloud_name: str, flavor: str, network: str, proxy: str) → None +``` + + + + + + + + + diff --git a/src-docs/store.md b/src-docs/store.md new file mode 100644 index 0000000..a48dff9 --- /dev/null +++ b/src-docs/store.md @@ -0,0 +1,111 @@ + + + + +# module `store` +Module for uploading images to shareable storage. + + +--- + + + +## function `create_snapshot` + +```python +create_snapshot( + cloud_name: str, + image_name: str, + server: Server, + keep_revisions: int +) → Image +``` + +Upload image to openstack glance. + + + +**Args:** + + - `cloud_name`: The Openstack cloud to use from clouds.yaml. + - `image_name`: The image name to upload as. + - `server`: The running OpenStack server to snapshot. + - `keep_revisions`: The number of revisions to keep for an image. + + + +**Raises:** + + - `UploadImageError`: If there was an error uploading the image to Openstack Glance. + + + +**Returns:** + The created image. + + +--- + + + +## function `upload_image` + +```python +upload_image( + arch: Arch, + cloud_name: str, + image_name: str, + image_path: Path, + keep_revisions: int +) → str +``` + +Upload image to openstack glance. + + + +**Args:** + + - `arch`: The image architecture. + - `cloud_name`: The Openstack cloud to use from clouds.yaml. + - `image_name`: The image name to upload as. + - `image_path`: The path to image to upload. + - `keep_revisions`: The number of revisions to keep for an image. + + + +**Raises:** + + - `UploadImageError`: If there was an error uploading the image to Openstack Glance. + + + +**Returns:** + The created image ID. + + +--- + + + +## function `get_latest_build_id` + +```python +get_latest_build_id(cloud_name: str, image_name: str) → str +``` + +Fetch the latest image id. + + + +**Args:** + + - `cloud_name`: The Openstack cloud to use from clouds.yaml. + - `image_name`: The image name to search for. + + + +**Returns:** + The image ID if exists, None otherwise. + + diff --git a/src-docs/utils.md b/src-docs/utils.md new file mode 100644 index 0000000..2a6f4ea --- /dev/null +++ b/src-docs/utils.md @@ -0,0 +1,44 @@ + + + + +# module `utils` +Utilities used by the app. + + +--- + + + +## function `retry` + +```python +retry( + exception: Type[Exception] = , + tries: int = 1, + delay: float = 0, + max_delay: Optional[float] = None, + backoff: float = 1, + local_logger: Logger = +) → Callable[[Callable[~ParamT, ~ReturnT]], Callable[~ParamT, ~ReturnT]] +``` + +Parameterize the decorator for adding retry to functions. + + + +**Args:** + + - `exception`: Exception type to be retried. + - `tries`: Number of attempts at retry. + - `delay`: Time in seconds to wait between retry. + - `max_delay`: Max time in seconds to wait between retry. + - `backoff`: Factor to increase the delay by each retry. + - `local_logger`: Logger for logging. + + + +**Returns:** + The function decorator for retry. + + diff --git a/src/github_runner_image_builder/builder.py b/src/github_runner_image_builder/builder.py index 0c76a87..f5307fa 100644 --- a/src/github_runner_image_builder/builder.py +++ b/src/github_runner_image_builder/builder.py @@ -435,6 +435,11 @@ def _install_yq() -> None: timeout=60 * 10, ) logger.info("git pull out: %s", output) + output = subprocess.check_output( # nosec: B603 + ["/snap/bin/go", "mod tidy", "-C", str(YQ_REPOSITORY_PATH)], + timeout=20 * 60, + ) + logger.info("go mod tidy out: %s", output) output = subprocess.check_output( # nosec: B603 ["/snap/bin/go", "build", "-C", str(YQ_REPOSITORY_PATH), "-o", str(HOST_YQ_BIN_PATH)], timeout=20 * 60, diff --git a/src/github_runner_image_builder/templates/cloud-init.sh.j2 b/src/github_runner_image_builder/templates/cloud-init.sh.j2 index 3150a78..4a21aa4 100644 --- a/src/github_runner_image_builder/templates/cloud-init.sh.j2 +++ b/src/github_runner_image_builder/templates/cloud-init.sh.j2 @@ -71,6 +71,7 @@ function install_yarn() { function install_yq() { /usr/bin/sudo snap install go --classic /usr/bin/git clone https://github.com/mikefarah/yq.git + /snap/bin/go mod tidy -C yq /snap/bin/go build -C yq -o /usr/bin/yq /usr/bin/rm -rf yq /usr/bin/sudo snap remove go diff --git a/tests/unit/test_openstack_builder.py b/tests/unit/test_openstack_builder.py index c0c5a78..54b37d4 100644 --- a/tests/unit/test_openstack_builder.py +++ b/tests/unit/test_openstack_builder.py @@ -513,6 +513,7 @@ def test__generate_cloud_init_script(): function install_yq() { /usr/bin/sudo snap install go --classic /usr/bin/git clone https://github.com/mikefarah/yq.git + /snap/bin/go mod tidy -C yq /snap/bin/go build -C yq -o /usr/bin/yq /usr/bin/rm -rf yq /usr/bin/sudo snap remove go From dbe40b0c4feaffcf4be01bfff46e7db65df8cf49 Mon Sep 17 00:00:00 2001 From: Yanks Yoon Date: Wed, 11 Sep 2024 09:48:52 +0000 Subject: [PATCH 02/21] chore: go mod tidy --- src/github_runner_image_builder/builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/github_runner_image_builder/builder.py b/src/github_runner_image_builder/builder.py index f5307fa..938f96a 100644 --- a/src/github_runner_image_builder/builder.py +++ b/src/github_runner_image_builder/builder.py @@ -436,7 +436,7 @@ def _install_yq() -> None: ) logger.info("git pull out: %s", output) output = subprocess.check_output( # nosec: B603 - ["/snap/bin/go", "mod tidy", "-C", str(YQ_REPOSITORY_PATH)], + ["/snap/bin/go", "mod", "tidy", "-C", str(YQ_REPOSITORY_PATH)], timeout=20 * 60, ) logger.info("go mod tidy out: %s", output) From 186acb265d7edb79752f99111d27d15e994344ff Mon Sep 17 00:00:00 2001 From: Yanks Yoon Date: Wed, 11 Sep 2024 09:52:18 +0000 Subject: [PATCH 03/21] chore: increment version --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index db470d0..b8b6c89 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-image-builder" -version = "0.4.3" +version = "0.4.4" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] From 26d5eb470d48aa4942bf157abc11e276e94cd880 Mon Sep 17 00:00:00 2001 From: Yanks Yoon Date: Fri, 20 Sep 2024 09:26:28 +0000 Subject: [PATCH 04/21] feat: network congestion algo & fix: ubuntu home dir --- src-docs/builder.md | 4 +- src-docs/errors.md | 54 +++++++++---- src/github_runner_image_builder/builder.py | 61 +++++++++++++++ src/github_runner_image_builder/errors.py | 8 ++ .../templates/cloud-init.sh.j2 | 14 +++- tests/integration/commands.py | 8 ++ tests/unit/test_builder.py | 75 +++++++++++++++++++ tests/unit/test_openstack_builder.py | 16 +++- 8 files changed, 219 insertions(+), 21 deletions(-) diff --git a/src-docs/builder.md b/src-docs/builder.md index b84f417..95f7f12 100644 --- a/src-docs/builder.md +++ b/src-docs/builder.md @@ -26,7 +26,7 @@ Module for interacting with qemu image builder. --- - + ## function `initialize` @@ -39,7 +39,7 @@ Configure the host machine to build images. --- - + ## function `run` diff --git a/src-docs/errors.md b/src-docs/errors.md index bf51e74..e12ec4b 100644 --- a/src-docs/errors.md +++ b/src-docs/errors.md @@ -143,6 +143,17 @@ Represents an error while disabling unattended-upgrade related services. +## class `NetworkFairQueuingEnableError` +Represents an error while enabling network fair queueing policy. + + + + + +--- + + + ## class `SystemUserConfigurationError` Represents an error while adding user to chroot env. @@ -152,7 +163,7 @@ Represents an error while adding user to chroot env. --- - + ## class `PermissionConfigurationError` Represents an error while modifying dir permissions. @@ -163,7 +174,7 @@ Represents an error while modifying dir permissions. --- - + ## class `YQBuildError` Represents an error while building yq binary from source. @@ -174,7 +185,7 @@ Represents an error while building yq binary from source. --- - + ## class `YarnInstallError` Represents an error installilng Yarn. @@ -185,7 +196,7 @@ Represents an error installilng Yarn. --- - + ## class `RunnerDownloadError` Represents an error downloading GitHub runner tar archive. @@ -196,7 +207,7 @@ Represents an error downloading GitHub runner tar archive. --- - + ## class `ImageCompressError` Represents an error while compressing cloud-img. @@ -207,7 +218,18 @@ Represents an error while compressing cloud-img. --- - + + +## class `HomeDirectoryChangeOwnershipError` +Represents an error while changing ubuntu home directory. + + + + + +--- + + ## class `OpenstackBaseError` Represents an error while interacting with Openstack. @@ -218,7 +240,7 @@ Represents an error while interacting with Openstack. --- - + ## class `UnauthorizedError` Represents an unauthorized connection to Openstack. @@ -229,7 +251,7 @@ Represents an unauthorized connection to Openstack. --- - + ## class `UploadImageError` Represents an error when uploading image to Openstack. @@ -240,7 +262,7 @@ Represents an error when uploading image to Openstack. --- - + ## class `OpenstackError` Represents an error while communicating with Openstack. @@ -251,7 +273,7 @@ Represents an error while communicating with Openstack. --- - + ## class `CloudsYAMLError` Represents an error with clouds.yaml for OpenStack connection. @@ -262,7 +284,7 @@ Represents an error with clouds.yaml for OpenStack connection. --- - + ## class `NotFoundError` Represents an error with not matching OpenStack object found. @@ -273,7 +295,7 @@ Represents an error with not matching OpenStack object found. --- - + ## class `FlavorNotFoundError` Represents an error with given OpenStack flavor not found. @@ -284,7 +306,7 @@ Represents an error with given OpenStack flavor not found. --- - + ## class `FlavorRequirementsNotMetError` Represents an error with given OpenStack flavor not meeting the minimum requirements. @@ -295,7 +317,7 @@ Represents an error with given OpenStack flavor not meeting the minimum requirem --- - + ## class `NetworkNotFoundError` Represents an error with given OpenStack network not found. @@ -306,7 +328,7 @@ Represents an error with given OpenStack network not found. --- - + ## class `AddressNotFoundError` Represents an error with OpenStack instance not receiving an IP address. @@ -317,7 +339,7 @@ Represents an error with OpenStack instance not receiving an IP address. --- - + ## class `CloudInitFailError` Represents an error with cloud-init. diff --git a/src/github_runner_image_builder/builder.py b/src/github_runner_image_builder/builder.py index 938f96a..8163f91 100644 --- a/src/github_runner_image_builder/builder.py +++ b/src/github_runner_image_builder/builder.py @@ -34,10 +34,12 @@ from github_runner_image_builder.errors import ( BuildImageError, DependencyInstallError, + HomeDirectoryChangeOwnershipError, ImageCompressError, ImageConnectError, ImageResizeError, NetworkBlockDeviceError, + NetworkFairQueuingEnableError, PermissionConfigurationError, ResizePartitionError, RunnerDownloadError, @@ -196,6 +198,8 @@ def run(arch: Arch, base_image: BaseImage, runner_version: str) -> None: _install_apt_packages(base_image=base_image) logger.info("Disabling unattended upgrades.") _disable_unattended_upgrades() + logger.info("Enabling network optimization policy.") + _enable_network_fair_queuing_congestion() logger.info("Configuring system users.") _configure_system_users() logger.info("Configuring /usr/local/bin directory.") @@ -204,6 +208,8 @@ def run(arch: Arch, base_image: BaseImage, runner_version: str) -> None: _install_yarn() logger.info("Installing GitHub runner.") _install_github_runner(arch=arch, version=runner_version) + logger.info("Changing ownership of home directory.") + _chown_home() except ChrootBaseError as exc: logger.exception("Error chrooting into %s", IMAGE_MOUNT_DIR) raise BuildImageError from exc @@ -552,6 +558,36 @@ def _disable_unattended_upgrades() -> None: raise UnattendedUpgradeDisableError from exc +def _enable_network_fair_queuing_congestion() -> None: + """Disable unatteneded upgrades to prevent apt locks. + + Raises: + NetworkFairQueuingEnableError: If there was an error disabling unattended upgrade related + services. + """ + try: + output = subprocess.check_output( + ["/usr/bin/systemctl", "net.core.default_qdisc=fq"], timeout=30 + ) # nosec: B603 + logger.info("net core default_qdisk out: %s", output) + output = subprocess.check_output( + ["/usr/bin/systemctl", "net.ipv4.tcp_congestion_control=bbr"], timeout=30 + ) # nosec: B603 + # There is no need to test the log output. + logger.info("net.ipv4.tcp_congestion_control=bbr out: %s", output) # pragma: nocover + except subprocess.CalledProcessError as exc: + logger.exception( + "Error enabling network congestion policy, cmd: %s, code: %s, err: %s", + exc.cmd, + exc.returncode, + exc.output, + ) + raise NetworkFairQueuingEnableError from exc + except subprocess.SubprocessError as exc: + logger.exception("Error enabling network congestion policy.") + raise NetworkFairQueuingEnableError from exc + + def _configure_system_users() -> None: """Configure system users. @@ -733,6 +769,31 @@ def _get_github_runner_version(version: str) -> str: return latest_version.lstrip("v") +def _chown_home() -> None: + """Change the ownership of Ubuntu home directory. + + Raises: + HomeDirectoryChangeOwnershipError: If there was an error changing the home directory + ownership to ubuntu:ubuntu. + """ + try: + subprocess.check_call( + ["/usr/bin/chown", "--recursive", "ubuntu:ubuntu", "/home/ubuntu"], # nosec + timeout=60 * 10, + ) + except subprocess.CalledProcessError as exc: + logger.exception( + "Error changing home directory ownership, cmd: %s, code: %s, err: %s", + exc.cmd, + exc.returncode, + exc.output, + ) + raise HomeDirectoryChangeOwnershipError from exc + except subprocess.SubprocessError as exc: + logger.exception("Error changing home directory ownership command.") + raise HomeDirectoryChangeOwnershipError from exc + + # Image compression might fail for arbitrary reasons - retrying usually solves this. @retry(tries=5, delay=5, max_delay=60, backoff=2, local_logger=logger) def _compress_image(image: Path) -> None: diff --git a/src/github_runner_image_builder/errors.py b/src/github_runner_image_builder/errors.py index 6a6db63..9d2efb8 100644 --- a/src/github_runner_image_builder/errors.py +++ b/src/github_runner_image_builder/errors.py @@ -53,6 +53,10 @@ class UnattendedUpgradeDisableError(BuildImageError): """Represents an error while disabling unattended-upgrade related services.""" +class NetworkFairQueuingEnableError(BuildImageError): + """Represents an error while enabling network fair queueing policy.""" + + class SystemUserConfigurationError(BuildImageError): """Represents an error while adding user to chroot env.""" @@ -77,6 +81,10 @@ class ImageCompressError(BuildImageError): """Represents an error while compressing cloud-img.""" +class HomeDirectoryChangeOwnershipError(BuildImageError): + """Represents an error while changing ubuntu home directory.""" + + class OpenstackBaseError(Exception): """Represents an error while interacting with Openstack.""" diff --git a/src/github_runner_image_builder/templates/cloud-init.sh.j2 b/src/github_runner_image_builder/templates/cloud-init.sh.j2 index 4a21aa4..689d948 100644 --- a/src/github_runner_image_builder/templates/cloud-init.sh.j2 +++ b/src/github_runner_image_builder/templates/cloud-init.sh.j2 @@ -47,6 +47,13 @@ function disable_unattended_upgrades() { /usr/bin/apt-get remove -y unattended-upgrades } +function enable_network_fair_queuing_congestion() { + /usr/bin/cat < Date: Tue, 3 Sep 2024 23:24:05 +0800 Subject: [PATCH 05/21] feat: external cloud upload (#19) * feat: upload-cloud parameter * add src-docs * documentation * fix: use user defined name * change docs to doc (no charmhub docs) * chore: refactor builder run * test: increase retry * remove deprecated reference to chroot builder * docs: update docs * swap download function * fix: enable stream download * fix: package form * chore: add server log output * fix typo * docs update --- doc/explanation/external-builder.md | 5 ++ doc/how-to/pin-github-runner-version.md | 14 +++++ doc/how-to/use-external-builder-vm.md | 17 ++++++ doc/tutorial/quick-start.md | 47 ++++++++++++++++ pyproject.toml | 2 +- requirements.txt | 2 +- src-docs/builder.md | 18 +++--- src/github_runner_image_builder/builder.py | 50 ++++++++++------- src/github_runner_image_builder/cli.py | 55 +++++++++++-------- src/github_runner_image_builder/config.py | 18 ++++++ .../openstack_builder.py | 48 ++++++++++++---- src/github_runner_image_builder/store.py | 6 +- tests/integration/helpers.py | 6 +- tests/integration/test_openstack_builder.py | 12 ++-- tests/unit/test_builder.py | 48 ++++++++++++++-- tests/unit/test_openstack_builder.py | 33 +++++++++-- tests/unit/test_store.py | 4 +- 17 files changed, 303 insertions(+), 82 deletions(-) create mode 100644 doc/explanation/external-builder.md create mode 100644 doc/how-to/pin-github-runner-version.md create mode 100644 doc/how-to/use-external-builder-vm.md create mode 100644 doc/tutorial/quick-start.md diff --git a/doc/explanation/external-builder.md b/doc/explanation/external-builder.md new file mode 100644 index 0000000..7ca4aba --- /dev/null +++ b/doc/explanation/external-builder.md @@ -0,0 +1,5 @@ +# External builder + +In order to pre-bootstrap LXD, Microk8s snaps to Juju, external VMs are spanwed and then run with +cloud-init script that installs the required components. Then, it is snapshot to work around +the limitation of booting up a fresh image with snaps with state perseverance. diff --git a/doc/how-to/pin-github-runner-version.md b/doc/how-to/pin-github-runner-version.md new file mode 100644 index 0000000..2af1ceb --- /dev/null +++ b/doc/how-to/pin-github-runner-version.md @@ -0,0 +1,14 @@ +# How to pin GitHub runner version + +The GitHub runner API for fetching the [GitHub runner applications download URL](https://docs.github.com/en/rest/actions/self-hosted-runners?apiVersion=2022-11-28#list-runner-applications-for-an-organization) may provide different versions +of the GitHub actions runner from the [latest release](https://github.com/actions/runner/releases). + +In order to pin a specific GitHub [actions runner](https://github.com/actions/runner) version, add +the `--runner-version` argument with the desired version during the build. + +``` +github-runner-image-builder --runner-version= +``` + +Find out what versions of runner versions are available +[here](https://github.com/actions/runner/releases). diff --git a/doc/how-to/use-external-builder-vm.md b/doc/how-to/use-external-builder-vm.md new file mode 100644 index 0000000..ca7476c --- /dev/null +++ b/doc/how-to/use-external-builder-vm.md @@ -0,0 +1,17 @@ +# How to use external builder VM + +This guide will cover how to use an external OpenStack builder VM to build and snapshot images. + +### Command + +Run the following command to create a snapshot image of a VM launched by the +github-runner-image-builder. + +Run `openstack list flavor` to find out what flavour is available. +Run `openstack list network` to find out what network is available. + +``` +FLAVOR= +NETWORK= +github-runner-image-builder --experimental-external True --flavor $FLAVOR --network $NETWORK +``` diff --git a/doc/tutorial/quick-start.md b/doc/tutorial/quick-start.md new file mode 100644 index 0000000..20efbac --- /dev/null +++ b/doc/tutorial/quick-start.md @@ -0,0 +1,47 @@ +# Build your first image + +## What you'll do + +- Install the CLI +- Initialise the builder +- Run the image build + +## Requirements + +- [Pipx installed](https://pipx.pypa.io/stable/installation/) +- Apt packages gcc, pipx, python3-dev + - `sudo apt-get install -y python3-dev gcc pipx` +- Working [OpenStack environment](https://microstack.run/docs/single-node) +- A clouds.yaml configuration with the OpenStack environment + +## Steps + +### Install the CLI + +- Install the CLI + - `pipx install git+https://github.com/canonical/github-runner-image-builder@stable` + +### Initialise the builder + +- Run `github-runner-image-builder init` to install the dependencies for building the image. + +### Run the image build + +- Run +``` +CLOUD_NAME= +IMAGE_NAME= +github-runner-image-builder run +``` +to start building the image. + +### Verify that the image is available on OpenStack + +- Run `openstack image list | grep ` to see the image in "active" status. +- You can also create a server with the image above to check the contents installed on the image. +For more information, refer to the official OpenStack documentation on creating servers here: +https://docs.openstack.org/python-openstackclient/pike/cli/command-objects/server.html#server-create + +### Cleanup + +- Run `openstack image delete ` after you're done following the tutorial to clean up. diff --git a/pyproject.toml b/pyproject.toml index b8b6c89..e6dfdca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-image-builder" -version = "0.4.4" +version = "0.5.0" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] diff --git a/requirements.txt b/requirements.txt index c3c8905..16d53f2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ click==8.1.7 fabric==3.2.2 Jinja2==3.1.4 -openstacksdk==3.1.0 +openstacksdk @ git+https://github.com/yanksyoon/openstacksdk.git@fix/download_image_stream pyyaml==6.0.1 tenacity==9.0.0 typing_extensions==4.12.2 diff --git a/src-docs/builder.md b/src-docs/builder.md index 95f7f12..b95c1ad 100644 --- a/src-docs/builder.md +++ b/src-docs/builder.md @@ -7,7 +7,6 @@ Module for interacting with qemu image builder. **Global Variables** --------------- -- **IMAGE_DEFAULT_APT_PACKAGES** - **APT_DEPENDENCIES** - **APT_NONINTERACTIVE_ENV** - **SNAP_GO** @@ -26,7 +25,7 @@ Module for interacting with qemu image builder. --- - + ## function `initialize` @@ -39,12 +38,12 @@ Configure the host machine to build images. --- - + ## function `run` ```python -run(arch: Arch, base_image: BaseImage, runner_version: str) → None +run(cloud_name: str, image_config: ImageConfig, keep_revisions: int) → str ``` Build and save the image locally. @@ -53,9 +52,9 @@ Build and save the image locally. **Args:** - - `arch`: The CPU architecture to build the image for. - - `base_image`: The ubuntu image to use as build base. - - `runner_version`: The GitHub runner version to embed. + - `cloud_name`: The OpenStack cloud to use from clouds.yaml. + - `image_config`: The target image configuration values. + - `keep_revisions`: The number of image to keep for snapshot before deletion. @@ -64,3 +63,8 @@ Build and save the image locally. - `BuildImageError`: If there was an error building the image. + +**Returns:** + The built image ID. + + diff --git a/src/github_runner_image_builder/builder.py b/src/github_runner_image_builder/builder.py index 8163f91..5445f2e 100644 --- a/src/github_runner_image_builder/builder.py +++ b/src/github_runner_image_builder/builder.py @@ -23,14 +23,8 @@ import requests -from github_runner_image_builder import cloud_image +from github_runner_image_builder import cloud_image, config, store from github_runner_image_builder.chroot import ChrootBaseError, ChrootContextManager -from github_runner_image_builder.config import ( - IMAGE_DEFAULT_APT_PACKAGES, - IMAGE_OUTPUT_PATH, - Arch, - BaseImage, -) from github_runner_image_builder.errors import ( BuildImageError, DependencyInstallError, @@ -162,16 +156,23 @@ def _enable_network_block_device() -> None: raise NetworkBlockDeviceError from exc -def run(arch: Arch, base_image: BaseImage, runner_version: str) -> None: +def run( + cloud_name: str, + image_config: config.ImageConfig, + keep_revisions: int, +) -> str: """Build and save the image locally. Args: - arch: The CPU architecture to build the image for. - base_image: The ubuntu image to use as build base. - runner_version: The GitHub runner version to embed. + cloud_name: The OpenStack cloud to use from clouds.yaml. + image_config: The target image configuration values. + keep_revisions: The number of image to keep for snapshot before deletion. Raises: BuildImageError: If there was an error building the image. + + Returns: + The built image ID. """ # ensure clean state - if there were errors within the chroot environment (e.g. network error) # this guarantees retry-ability @@ -180,7 +181,9 @@ def run(arch: Arch, base_image: BaseImage, runner_version: str) -> None: IMAGE_MOUNT_DIR.mkdir(parents=True, exist_ok=True) logger.info("Downloading base image.") - base_image_path = cloud_image.download_and_validate_image(arch=arch, base_image=base_image) + base_image_path = cloud_image.download_and_validate_image( + arch=image_config.arch, base_image=image_config.base + ) logger.info("Resizing base image.") _resize_image(image_path=base_image_path) logger.info("Connecting image to network block device.") @@ -195,7 +198,7 @@ def run(arch: Arch, base_image: BaseImage, runner_version: str) -> None: _replace_mounted_resolv_conf() try: with ChrootContextManager(IMAGE_MOUNT_DIR): - _install_apt_packages(base_image=base_image) + _install_apt_packages(base_image=image_config.base) logger.info("Disabling unattended upgrades.") _disable_unattended_upgrades() logger.info("Enabling network optimization policy.") @@ -207,7 +210,7 @@ def run(arch: Arch, base_image: BaseImage, runner_version: str) -> None: logger.info("Installing Yarn.") _install_yarn() logger.info("Installing GitHub runner.") - _install_github_runner(arch=arch, version=runner_version) + _install_github_runner(arch=image_config.arch, version=image_config.runner_version) logger.info("Changing ownership of home directory.") _chown_home() except ChrootBaseError as exc: @@ -220,6 +223,15 @@ def run(arch: Arch, base_image: BaseImage, runner_version: str) -> None: logger.info("Compressing image.") _compress_image(base_image_path) + image = store.upload_image( + arch=image_config.arch, + cloud_name=cloud_name, + image_name=image_config.name, + image_path=config.IMAGE_OUTPUT_PATH, + keep_revisions=keep_revisions, + ) + return image.id + def _disconnect_image_to_network_block_device(check: bool = True) -> None: """Disconnect the image to network block device in cleanup for chroot. @@ -471,7 +483,7 @@ def _replace_mounted_resolv_conf() -> None: shutil.copy(str(HOST_RESOLV_CONF_PATH), str(MOUNTED_RESOLV_CONF_PATH)) -def _install_apt_packages(base_image: BaseImage) -> None: +def _install_apt_packages(base_image: config.BaseImage) -> None: """Install APT packages on the chroot env. Args: @@ -491,7 +503,7 @@ def _install_apt_packages(base_image: BaseImage) -> None: "install", "-y", "--no-install-recommends", - *IMAGE_DEFAULT_APT_PACKAGES, + *config.IMAGE_DEFAULT_APT_PACKAGES, ], timeout=60 * 20, env=APT_NONINTERACTIVE_ENV, @@ -505,7 +517,7 @@ def _install_apt_packages(base_image: BaseImage) -> None: "-y", # https://ubuntu.com/kernel/lifecycle installs recommended packages "--install-recommends", - IMAGE_HWE_PKG_FORMAT.format(VERSION=BaseImage.get_version(base_image)), + IMAGE_HWE_PKG_FORMAT.format(VERSION=config.BaseImage.get_version(base_image)), ], timeout=60 * 20, env=APT_NONINTERACTIVE_ENV, @@ -684,7 +696,7 @@ def _install_yarn() -> None: raise YarnInstallError from exc -def _install_github_runner(arch: Arch, version: str) -> None: +def _install_github_runner(arch: config.Arch, version: str) -> None: """Download and install github runner. Args: @@ -817,7 +829,7 @@ def _compress_image(image: Path) -> None: "-O", # output format "qcow2", str(image), - str(IMAGE_OUTPUT_PATH), + str(config.IMAGE_OUTPUT_PATH), ], timeout=60 * 10, ) diff --git a/src/github_runner_image_builder/cli.py b/src/github_runner_image_builder/cli.py index 269c79a..05b656a 100644 --- a/src/github_runner_image_builder/cli.py +++ b/src/github_runner_image_builder/cli.py @@ -9,19 +9,12 @@ import click -from github_runner_image_builder import builder, logging, openstack_builder, store -from github_runner_image_builder.config import ( - BASE_CHOICES, - IMAGE_OUTPUT_PATH, - LOG_LEVELS, - BaseImage, - get_supported_arch, -) +from github_runner_image_builder import builder, config, logging, openstack_builder, store @click.option( "--log-level", - type=click.Choice(LOG_LEVELS), + type=click.Choice(config.LOG_LEVELS), default="info", help="Configure logging verbosity.", ) @@ -57,7 +50,7 @@ def initialize(experimental_external: bool, cloud_name: str) -> None: if not experimental_external: builder.initialize() return - arch = get_supported_arch() + arch = config.get_supported_arch() openstack_builder.initialize( arch=arch, cloud_name=openstack_builder.determine_cloud(cloud_name=cloud_name) @@ -88,7 +81,7 @@ def get_latest_build_id(cloud_name: str, image_name: str) -> None: @click.option( "-b", "--base-image", - type=click.Choice(BASE_CHOICES), + type=click.Choice(config.BASE_CHOICES), default="noble", help=("The Ubuntu base image to use as build base."), ) @@ -140,6 +133,13 @@ def get_latest_build_id(cloud_name: str, image_name: str) -> None: help="EXPERIMENTAL: Proxy to use for external build VMs in host:port format (without scheme). " "Ignored if --experimental-external is not enabled", ) +@click.option( + "--upload-cloud", + default="", + help="EXPERIMENTAL: Different cloud to use to upload the externally built image. The cloud " + "connection parameters should exist in the clouds.yaml. Ignored if --experimental-external is" + " not enabled, as a part of external build mode parameter.", +) # click doesn't yet support dataclasses, hence all arguments are required. def run( # pylint: disable=too-many-arguments cloud_name: str, @@ -152,6 +152,7 @@ def run( # pylint: disable=too-many-arguments flavor: str, network: str, proxy: str, + upload_cloud: str, ) -> None: """Build a cloud image using chroot and upload it to OpenStack. @@ -167,26 +168,36 @@ def run( # pylint: disable=too-many-arguments flavor: The Openstack flavor to create server to build images. network: The Openstack network to assign to server to build images. proxy: Proxy to use for external build VMs. + upload_cloud: The Opensttack cloud to use to upload externally built image. """ - arch = get_supported_arch() - base = BaseImage.from_str(base_image) + arch = config.get_supported_arch() + base = config.BaseImage.from_str(base_image) if not experimental_external: - builder.run(arch=arch, base_image=base, runner_version=runner_version) - image_id = store.upload_image( - arch=arch, + image_id = builder.run( cloud_name=cloud_name, - image_name=image_name, - image_path=IMAGE_OUTPUT_PATH, + image_config=config.ImageConfig( + arch=arch, + base=base, + runner_version=runner_version, + name=image_name, + ), keep_revisions=keep_revisions, ) else: image_id = openstack_builder.run( - arch=arch, - base=base, cloud_config=openstack_builder.CloudConfig( - cloud_name=cloud_name, flavor=flavor, network=network, proxy=proxy + cloud_name=cloud_name, + flavor=flavor, + network=network, + proxy=proxy, + upload_cloud_name=upload_cloud, + ), + image_config=config.ImageConfig( + arch=arch, + base=base, + runner_version=runner_version, + name=image_name, ), - runner_version=runner_version, keep_revisions=keep_revisions, ) if callback_script: diff --git a/src/github_runner_image_builder/config.py b/src/github_runner_image_builder/config.py index 4daa758..6846e20 100644 --- a/src/github_runner_image_builder/config.py +++ b/src/github_runner_image_builder/config.py @@ -3,6 +3,7 @@ """Module containing configurations.""" +import dataclasses import itertools import logging import platform @@ -133,3 +134,20 @@ def from_str(cls, tag_or_name: str) -> "BaseImage": (logging.getLevelName(level).lower() for level in _LOG_LEVELS), ) ) + + +@dataclasses.dataclass +class ImageConfig: + """The build image configuration values. + + Attributes: + arch: The architecture of the target image. + base: The ubuntu base OS of the image. + runner_version: The GitHub runner version to install on the VM. Defaults to latest. + name: The image name to upload on OpenStack. + """ + + arch: Arch + base: BaseImage + runner_version: str + name: str diff --git a/src/github_runner_image_builder/openstack_builder.py b/src/github_runner_image_builder/openstack_builder.py index 2171ec9..8d2a4ec 100644 --- a/src/github_runner_image_builder/openstack_builder.py +++ b/src/github_runner_image_builder/openstack_builder.py @@ -29,7 +29,7 @@ import yaml import github_runner_image_builder.errors -from github_runner_image_builder import cloud_image, store +from github_runner_image_builder import cloud_image, config, store from github_runner_image_builder.config import IMAGE_DEFAULT_APT_PACKAGES, Arch, BaseImage logger = logging.getLogger(__name__) @@ -45,7 +45,7 @@ BUILDER_KEY_PATH = pathlib.Path("/home/ubuntu/.ssh/builder_key") SHARED_SECURITY_GROUP_NAME = "github-runner-image-builder-v1" -IMAGE_SNAPSHOT_NAME = "github-runner-image-builder-snapshot-v0" +IMAGE_SNAPSHOT_FILE_PATH = pathlib.Path("github-runner-image-snapshot.img") CREATE_SERVER_TIMEOUT = 5 * 60 # seconds @@ -197,35 +197,36 @@ class CloudConfig: flavor: The OpenStack flavor to launch builder VMs on. network: The OpenStack network to launch the builder VMs on. proxy: The proxy to enable on builder VMs. + upload_cloud_name: The OpenStack cloud name to upload the snapshot to. (Default same cloud) """ cloud_name: str flavor: str network: str proxy: str + upload_cloud_name: str | None def run( - arch: Arch, - base: BaseImage, cloud_config: CloudConfig, - runner_version: str, + image_config: config.ImageConfig, keep_revisions: int, ) -> str: """Run external OpenStack builder instance and create a snapshot. Args: - arch: The architecture of the image to seed. - base: The Ubuntu base to use as builder VM base. cloud_config: The OpenStack cloud configuration values for builder VM. - runner_version: The GitHub runner version to install on the VM. Defaults to latest. + image_config: The target image configuration values. keep_revisions: The number of image to keep for snapshot before deletion. Returns: The Openstack snapshot image ID. """ cloud_init_script = _generate_cloud_init_script( - arch=arch, base=base, runner_version=runner_version, proxy=cloud_config.proxy + arch=image_config.arch, + base=image_config.base, + runner_version=image_config.runner_version, + proxy=cloud_config.proxy, ) with openstack.connect(cloud=cloud_config.cloud_name) as conn: flavor = _determine_flavor(conn=conn, flavor_name=cloud_config.flavor) @@ -233,8 +234,8 @@ def run( network = _determine_network(conn=conn, network_name=cloud_config.network) logger.info("Using network ID: %s.", network) builder: openstack.compute.v2.server.Server = conn.create_server( - name=_get_builder_name(arch=arch, base=base), - image=_get_base_image_name(arch=arch, base=base), + name=_get_builder_name(arch=image_config.arch, base=image_config.base), + image=_get_base_image_name(arch=image_config.arch, base=image_config.base), key_name=BUILDER_SSH_KEY_NAME, flavor=flavor, network=network, @@ -246,15 +247,38 @@ def run( ) logger.info("Launched builder, waiting for cloud-init to complete: %s.", builder.id) _wait_for_cloud_init_complete(conn=conn, server=builder, ssh_key=BUILDER_KEY_PATH) + log_output = conn.get_server_console(server=builder) + logger.info("Build output: %s", log_output) image = store.create_snapshot( cloud_name=cloud_config.cloud_name, - image_name=IMAGE_SNAPSHOT_NAME, + image_name=image_config.name, server=builder, keep_revisions=keep_revisions, ) logger.info("Requested snapshot, waiting for snapshot to complete: %s.", image.id) _wait_for_snapshot_complete(conn=conn, image=image) + if cloud_config.upload_cloud_name: + logger.info("Downloading snapshot to %s.", IMAGE_SNAPSHOT_FILE_PATH) + conn.download_image( + name_or_id=image.id, output_file=IMAGE_SNAPSHOT_FILE_PATH, stream=True + ) + logger.info("Uploading downloaded snapshot to %s.", cloud_config.upload_cloud_name) + image = store.upload_image( + arch=image_config.arch, + cloud_name=cloud_config.upload_cloud_name, + image_name=image_config.name, + image_path=IMAGE_SNAPSHOT_FILE_PATH, + keep_revisions=keep_revisions, + ) + logger.info( + "Uploaded snapshot on cloud %s, id: %s, name: %s", + cloud_config.upload_cloud_name, + image.id, + image.name, + ) + logger.info("Deleting builder VM: %s (%s)", builder.name, builder.id) conn.delete_server(name_or_id=builder.id, wait=True, timeout=5 * 60) + logger.info("Image builder run complete.") return str(image.id) diff --git a/src/github_runner_image_builder/store.py b/src/github_runner_image_builder/store.py index 7c6708a..6432a31 100644 --- a/src/github_runner_image_builder/store.py +++ b/src/github_runner_image_builder/store.py @@ -57,7 +57,7 @@ def create_snapshot( def upload_image( arch: Arch, cloud_name: str, image_name: str, image_path: Path, keep_revisions: int -) -> str: +) -> Image: """Upload image to openstack glance. Args: @@ -71,7 +71,7 @@ def upload_image( UploadImageError: If there was an error uploading the image to Openstack Glance. Returns: - The created image ID. + The created image. """ with openstack.connect(cloud=cloud_name) as connection: try: @@ -88,7 +88,7 @@ def upload_image( connection=connection, image_name=image_name, num_revisions=keep_revisions ) logger.info("Image created successfully, %s %s.", image_name, image.id) - return image.id + return image except openstack.exceptions.OpenStackCloudException as exc: logger.exception("Error while uploading image.") raise UploadImageError from exc diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 6972a11..287a045 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -350,7 +350,7 @@ def _snap_ready(conn: SSHConnection) -> bool: @tenacity.retry( wait=tenacity.wait_exponential(multiplier=2, min=10, max=60), - stop=tenacity.stop_after_attempt(5), + stop=tenacity.stop_after_attempt(10), ) def _configure_dockerhub_mirror(conn: SSHConnection, dockerhub_mirror: str | None): """Use dockerhub mirror if provided. @@ -495,6 +495,10 @@ def create_openstack_server( timeout=60 * 20, wait=True, ) + logger.info( + "server console log output: %s", + openstack_metadata.connection.get_server_console(server=server), + ) yield server except openstack.exceptions.SDKException: server = openstack_metadata.connection.get_server(name_or_id=server_name) diff --git a/tests/integration/test_openstack_builder.py b/tests/integration/test_openstack_builder.py index 0339f7a..7d8e2c0 100644 --- a/tests/integration/test_openstack_builder.py +++ b/tests/integration/test_openstack_builder.py @@ -85,15 +85,19 @@ def cli_run_fixture( been installed using pipx. See testenv:integration section of tox.ini. """ openstack_builder.run( - arch=arch, - base=config.BaseImage.from_str(image), cloud_config=openstack_builder.CloudConfig( cloud_name=cloud_name, flavor=openstack_metadata.flavor, network=openstack_metadata.network, proxy=proxy.http, + upload_cloud_name=cloud_name, + ), + image_config=config.ImageConfig( + arch=arch, + base=config.BaseImage.from_str(image), + runner_version="", + name="github-runner-image-builder-snapshot-v0", ), - runner_version="", keep_revisions=1, ) @@ -108,7 +112,7 @@ async def openstack_server_fixture( ): """A testing openstack instance.""" image: Image = openstack_metadata.connection.get_image( - name_or_id=openstack_builder.IMAGE_SNAPSHOT_NAME + name_or_id="github-runner-image-builder-snapshot-v0" ) server_name = f"test-image-builder-run-{test_id}" for server in helpers.create_openstack_server( diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index 483af7b..16c9a5f 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -13,9 +13,8 @@ import pytest -from github_runner_image_builder import builder, cloud_image +from github_runner_image_builder import builder, cloud_image, config from github_runner_image_builder.builder import ( - Arch, BuildImageError, ChrootBaseError, DependencyInstallError, @@ -219,6 +218,13 @@ def test__enable_network_block_device_fail(monkeypatch: pytest.MonkeyPatch): "Failed to compress image", id="Failed to compress image", ), + pytest.param( + builder.store, + "upload_image", + MagicMock(side_effect=ImageCompressError("Failed to upload image")), + "Failed to upload image", + id="Failed to upload image", + ), ], ) def test_run_error( @@ -254,11 +260,43 @@ def test_run_error( monkeypatch.setattr(patch_obj, sub_func, mock) with pytest.raises(BuildImageError) as exc: - builder.run(arch=MagicMock(), base_image=MagicMock(), runner_version=MagicMock()) + builder.run(cloud_name=MagicMock(), image_config=MagicMock(), keep_revisions=MagicMock()) assert expected_message in str(exc.getrepr()) +def test_run(monkeypatch: pytest.MonkeyPatch): + """ + arrange: given a monkeypatched functions of run that raises exceptions. + act: when run is called. + assert: BuildImageError is raised. + """ + monkeypatch.setattr(builder, "IMAGE_MOUNT_DIR", MagicMock()) + monkeypatch.setattr(cloud_image, "download_and_validate_image", MagicMock()) + monkeypatch.setattr(builder, "_resize_image", MagicMock()) + monkeypatch.setattr(builder, "_connect_image_to_network_block_device", MagicMock()) + monkeypatch.setattr(builder, "_resize_mount_partitions", MagicMock()) + monkeypatch.setattr(builder, "_replace_mounted_resolv_conf", MagicMock()) + monkeypatch.setattr(builder, "_install_yq", MagicMock()) + monkeypatch.setattr(builder, "ChrootContextManager", MagicMock()) + monkeypatch.setattr(builder.subprocess, "check_output", MagicMock()) + monkeypatch.setattr(builder.subprocess, "run", MagicMock()) + monkeypatch.setattr(builder, "_disable_unattended_upgrades", MagicMock()) + monkeypatch.setattr(builder, "_configure_system_users", MagicMock()) + monkeypatch.setattr(builder, "_install_yarn", MagicMock()) + monkeypatch.setattr(builder, "_install_github_runner", MagicMock()) + monkeypatch.setattr(builder, "_disconnect_image_to_network_block_device", MagicMock()) + monkeypatch.setattr(builder, "_compress_image", MagicMock()) + monkeypatch.setattr( + builder.store, "upload_image", MagicMock(return_value=(test_image := MagicMock())) + ) + + assert ( + builder.run(cloud_name=MagicMock(), image_config=MagicMock(), keep_revisions=MagicMock()) + == test_image.id + ) + + def test__resize_image_fail(monkeypatch: pytest.MonkeyPatch): """ arrange: given a monkeypatched subprocess.run that raises an exception. @@ -621,7 +659,7 @@ def test__install_github_runner_error( monkeypatch.setattr(module, func, mock) with pytest.raises(builder.RunnerDownloadError) as exc: - builder._install_github_runner(arch=Arch.X64, version="") + builder._install_github_runner(arch=config.Arch.X64, version="") assert expected_message in str(exc.getrepr()) @@ -640,7 +678,7 @@ def test__install_github_runner(monkeypatch: pytest.MonkeyPatch): monkeypatch.setattr(builder.pwd, "getpwnam", MagicMock()) monkeypatch.setattr(builder.subprocess, "check_call", MagicMock()) - builder._install_github_runner(arch=Arch.ARM64, version="") + builder._install_github_runner(arch=config.Arch.ARM64, version="") @pytest.mark.parametrize( diff --git a/tests/unit/test_openstack_builder.py b/tests/unit/test_openstack_builder.py index e6d8231..894f197 100644 --- a/tests/unit/test_openstack_builder.py +++ b/tests/unit/test_openstack_builder.py @@ -165,7 +165,32 @@ def test__create_security_group(): connection_mock.create_security_group.assert_called() -def test_run(monkeypatch: pytest.MonkeyPatch): +@pytest.mark.parametrize( + "cloud_config", + [ + pytest.param( + openstack_builder.CloudConfig( + cloud_name="test-cloud", + flavor="test-flavor", + network="test-network", + proxy="test-proxy", + upload_cloud_name="", + ), + id="no upload-cloud-name", + ), + pytest.param( + openstack_builder.CloudConfig( + cloud_name="test-cloud", + flavor="test-flavor", + network="test-network", + proxy="test-proxy", + upload_cloud_name="test-cloud-2", + ), + id="upload-cloud-name defined", + ), + ], +) +def test_run(monkeypatch: pytest.MonkeyPatch, cloud_config: openstack_builder.CloudConfig): """ arrange: given monkeypatched sub functions for openstack_builder.run. act: when run is called. @@ -197,10 +222,8 @@ def test_run(monkeypatch: pytest.MonkeyPatch): ) openstack_builder.run( - arch=MagicMock(), - base=MagicMock(), - cloud_config=MagicMock(), - runner_version=MagicMock(), + cloud_config=cloud_config, + image_config=MagicMock(), keep_revisions=5, ) diff --git a/tests/unit/test_store.py b/tests/unit/test_store.py index 14804c8..3d99905 100644 --- a/tests/unit/test_store.py +++ b/tests/unit/test_store.py @@ -181,7 +181,7 @@ def test_upload_image(mock_connection: MagicMock): act: when upload_image is called. assert: UploadImageError is raised. """ - mock_connection.create_image.return_value = MockOpenstackImageFactory(id="1") + mock_connection.create_image.return_value = (test_image := MockOpenstackImageFactory(id="1")) assert ( store.upload_image( @@ -191,7 +191,7 @@ def test_upload_image(mock_connection: MagicMock): image_path=MagicMock(), keep_revisions=MagicMock(), ) - == "1" + == test_image ) From a300e6abca437fd2de5ac39e903cc9106028955a Mon Sep 17 00:00:00 2001 From: Yanks Yoon <37652070+yanksyoon@users.noreply.github.com> Date: Fri, 6 Sep 2024 09:37:52 +0800 Subject: [PATCH 06/21] feat: use external arch param (#21) * use external arch param * bump rev --- pyproject.toml | 2 +- src/github_runner_image_builder/cli.py | 30 ++++++++++++++++++++------ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index e6dfdca..c678652 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-image-builder" -version = "0.5.0" +version = "0.5.1" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] diff --git a/src/github_runner_image_builder/cli.py b/src/github_runner_image_builder/cli.py index 05b656a..a3ca47b 100644 --- a/src/github_runner_image_builder/cli.py +++ b/src/github_runner_image_builder/cli.py @@ -30,9 +30,11 @@ def main(log_level: str | int) -> None: @main.command(name="init") @click.option( - "--experimental-external", - default=False, - help="EXPERIMENTAL: Use external Openstack builder to build images.", + "--arch", + type=click.Choice((config.Arch.ARM64, config.Arch.X64)), + default=None, + help="Image architecture to initialize for. Defaults the host architecture. " + "Ignored if --experimental-external is not enabled", ) @click.option( "--cloud-name", @@ -40,17 +42,23 @@ def main(log_level: str | int) -> None: help="The cloud to use from the clouds.yaml file. The CLI looks for clouds.yaml in paths of " "the following order: current directory, ~/.config/openstack, /etc/openstack.", ) -def initialize(experimental_external: bool, cloud_name: str) -> None: +@click.option( + "--experimental-external", + default=False, + help="EXPERIMENTAL: Use external Openstack builder to build images.", +) +def initialize(arch: config.Arch | None, cloud_name: str, experimental_external: bool) -> None: """Initialize builder CLI function wrapper. Args: - experimental_external: Whether to use external Openstack builder to build images. + arch: The architecture to build for. cloud_name: The cloud name to use from clouds.yaml. + experimental_external: Whether to use external Openstack builder to build images. """ if not experimental_external: builder.initialize() return - arch = config.get_supported_arch() + arch = arch if arch else config.get_supported_arch() openstack_builder.initialize( arch=arch, cloud_name=openstack_builder.determine_cloud(cloud_name=cloud_name) @@ -78,6 +86,12 @@ def get_latest_build_id(cloud_name: str, image_name: str) -> None: @main.command(name="run") @click.argument("cloud_name") @click.argument("image_name") +@click.option( + "--arch", + type=click.Choice((config.Arch.ARM64, config.Arch.X64)), + default=None, + help="Image architecture to initialize for. Defaults the host architecture.", +) @click.option( "-b", "--base-image", @@ -142,6 +156,7 @@ def get_latest_build_id(cloud_name: str, image_name: str) -> None: ) # click doesn't yet support dataclasses, hence all arguments are required. def run( # pylint: disable=too-many-arguments + arch: config.Arch | None, cloud_name: str, image_name: str, base_image: str, @@ -157,6 +172,7 @@ def run( # pylint: disable=too-many-arguments """Build a cloud image using chroot and upload it to OpenStack. Args: + arch: The architecture to run build for. cloud_name: The cloud to use from the clouds.yaml file. The CLI looks for clouds.yaml in paths of the following order: current directory, ~/.config/openstack, /etc/openstack. image_name: The image name uploaded to Openstack. @@ -170,7 +186,7 @@ def run( # pylint: disable=too-many-arguments proxy: Proxy to use for external build VMs. upload_cloud: The Opensttack cloud to use to upload externally built image. """ - arch = config.get_supported_arch() + arch = arch if arch else config.get_supported_arch() base = config.BaseImage.from_str(base_image) if not experimental_external: image_id = builder.run( From 552c090c98a318ce29df79e0ce89547614245503 Mon Sep 17 00:00:00 2001 From: Yanks Yoon <37652070+yanksyoon@users.noreply.github.com> Date: Mon, 9 Sep 2024 18:35:38 +0800 Subject: [PATCH 07/21] fix: parsable image output (#22) * fix: suppress stdout * increment patch version * run go mod tidy * fix lint * feat: add parsable output --- pyproject.toml | 2 +- src/github_runner_image_builder/builder.py | 2 +- src/github_runner_image_builder/cli.py | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index c678652..7cb8763 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-image-builder" -version = "0.5.1" +version = "0.5.2" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] diff --git a/src/github_runner_image_builder/builder.py b/src/github_runner_image_builder/builder.py index 5445f2e..a416981 100644 --- a/src/github_runner_image_builder/builder.py +++ b/src/github_runner_image_builder/builder.py @@ -455,7 +455,7 @@ def _install_yq() -> None: logger.info("git pull out: %s", output) output = subprocess.check_output( # nosec: B603 ["/snap/bin/go", "mod", "tidy", "-C", str(YQ_REPOSITORY_PATH)], - timeout=20 * 60, + timeout=60 * 10, ) logger.info("go mod tidy out: %s", output) output = subprocess.check_output( # nosec: B603 diff --git a/src/github_runner_image_builder/cli.py b/src/github_runner_image_builder/cli.py index a3ca47b..f24104e 100644 --- a/src/github_runner_image_builder/cli.py +++ b/src/github_runner_image_builder/cli.py @@ -199,6 +199,9 @@ def run( # pylint: disable=too-many-arguments ), keep_revisions=keep_revisions, ) + # 2024/07/09: Only print image_id for chroot building for backwards compatibility. To be + # deprecated when external builder is in stable. + click.echo(image_id, nl=False) else: image_id = openstack_builder.run( cloud_config=openstack_builder.CloudConfig( @@ -216,6 +219,7 @@ def run( # pylint: disable=too-many-arguments ), keep_revisions=keep_revisions, ) + click.echo(f"Image build success:\n{image_id}", nl=False) if callback_script: # The callback script is a user trusted script. subprocess.check_call([str(callback_script), image_id]) # nosec: B603 From 901a31c416690b8bab5006913893ca0690e798a0 Mon Sep 17 00:00:00 2001 From: Yanks Yoon <37652070+yanksyoon@users.noreply.github.com> Date: Wed, 11 Sep 2024 16:28:45 +0800 Subject: [PATCH 08/21] fix: cloud init (#25) * fix: cloud init script race condition problems * test: fixup tests * chore: increment version --- pyproject.toml | 2 +- .../templates/cloud-init.sh.j2 | 23 ++++++++++++------- tests/unit/test_openstack_builder.py | 23 ++++++++++++------- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 7cb8763..2b18d10 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-image-builder" -version = "0.5.2" +version = "0.5.3" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] diff --git a/src/github_runner_image_builder/templates/cloud-init.sh.j2 b/src/github_runner_image_builder/templates/cloud-init.sh.j2 index 689d948..e04f2f5 100644 --- a/src/github_runner_image_builder/templates/cloud-init.sh.j2 +++ b/src/github_runner_image_builder/templates/cloud-init.sh.j2 @@ -1,5 +1,7 @@ #!/bin/bash +set -e + function configure_proxy() { local proxy="$1" if [[ -z "$proxy" ]]; then @@ -25,6 +27,8 @@ table ip aproxy { EOF echo "Configuring aproxy" /usr/bin/sudo snap set aproxy proxy=${proxy} listen=:8444; + echo "Wait for aproxy to start" + sleep 5 } function install_apt_packages() { @@ -60,7 +64,8 @@ function configure_system_users() { /usr/bin/id -u ubuntu &>/dev/null || useradd --create-home ubuntu echo "PATH=\$PATH:/home/ubuntu/.local/bin" >> /home/ubuntu/.profile echo "PATH=\$PATH:/home/ubuntu/.local/bin" >> /home/ubuntu/.bashrc - /usr/sbin/groupadd microk8s + /usr/sbin/groupadd -f microk8s + /usr/sbin/groupadd -f docker /usr/sbin/usermod --append --groups docker,microk8s,lxd,sudo ubuntu } @@ -76,12 +81,12 @@ function install_yarn() { } function install_yq() { - /usr/bin/sudo snap install go --classic - /usr/bin/git clone https://github.com/mikefarah/yq.git - /snap/bin/go mod tidy -C yq - /snap/bin/go build -C yq -o /usr/bin/yq - /usr/bin/rm -rf yq - /usr/bin/sudo snap remove go + /usr/bin/sudo -E /usr/bin/snap install go --classic + /usr/bin/sudo -E /usr/bin/git clone https://github.com/mikefarah/yq.git + /usr/bin/sudo -E /snap/bin/go mod tidy -C yq + /usr/bin/sudo -E /snap/bin/go build -C yq -o /usr/bin/yq + /usr/bin/sudo -E /usr/bin/rm -rf yq + /usr/bin/sudo -E /usr/bin/snap remove go } function install_github_runner() { @@ -120,6 +125,8 @@ enable_network_fair_queuing_congestion configure_system_users configure_usr_local_bin install_yarn -install_yq +# install yq with ubuntu user due to GOPATH related go configuration settings +export -f install_yq +su ubuntu -c "bash -c 'install_yq'" install_github_runner "$github_runner_version" "$github_runner_arch" chown_home diff --git a/tests/unit/test_openstack_builder.py b/tests/unit/test_openstack_builder.py index 894f197..ac1d23e 100644 --- a/tests/unit/test_openstack_builder.py +++ b/tests/unit/test_openstack_builder.py @@ -463,6 +463,8 @@ def test__generate_cloud_init_script(): # pylint: disable=R0801 == """#!/bin/bash +set -e + function configure_proxy() { local proxy="$1" if [[ -z "$proxy" ]]; then @@ -489,6 +491,8 @@ def test__generate_cloud_init_script(): EOF echo "Configuring aproxy" /usr/bin/sudo snap set aproxy proxy=${proxy} listen=:8444; + echo "Wait for aproxy to start" + sleep 5 } function install_apt_packages() { @@ -525,7 +529,8 @@ def test__generate_cloud_init_script(): /usr/bin/id -u ubuntu &>/dev/null || useradd --create-home ubuntu echo "PATH=\\$PATH:/home/ubuntu/.local/bin" >> /home/ubuntu/.profile echo "PATH=\\$PATH:/home/ubuntu/.local/bin" >> /home/ubuntu/.bashrc - /usr/sbin/groupadd microk8s + /usr/sbin/groupadd -f microk8s + /usr/sbin/groupadd -f docker /usr/sbin/usermod --append --groups docker,microk8s,lxd,sudo ubuntu } @@ -541,12 +546,12 @@ def test__generate_cloud_init_script(): } function install_yq() { - /usr/bin/sudo snap install go --classic - /usr/bin/git clone https://github.com/mikefarah/yq.git - /snap/bin/go mod tidy -C yq - /snap/bin/go build -C yq -o /usr/bin/yq - /usr/bin/rm -rf yq - /usr/bin/sudo snap remove go + /usr/bin/sudo -E /usr/bin/snap install go --classic + /usr/bin/sudo -E /usr/bin/git clone https://github.com/mikefarah/yq.git + /usr/bin/sudo -E /snap/bin/go mod tidy -C yq + /usr/bin/sudo -E /snap/bin/go build -C yq -o /usr/bin/yq + /usr/bin/sudo -E /usr/bin/rm -rf yq + /usr/bin/sudo -E /usr/bin/snap remove go } function install_github_runner() { @@ -589,7 +594,9 @@ def test__generate_cloud_init_script(): configure_system_users configure_usr_local_bin install_yarn -install_yq +# install yq with ubuntu user due to GOPATH related go configuration settings +export -f install_yq +su ubuntu -c "bash -c 'install_yq'" install_github_runner "$github_runner_version" "$github_runner_arch" chown_home\ """ From 3a64cfa3238dd90cb0c0b40c02f9b881d7e6595f Mon Sep 17 00:00:00 2001 From: Yanks Yoon <37652070+yanksyoon@users.noreply.github.com> Date: Thu, 19 Sep 2024 10:16:25 +0800 Subject: [PATCH 09/21] feat: multicloud upload (#27) * feat: multicloud upload * chore: increment version * test: add multiple clouds case * fix: rename var * fix: image None * chore: remove debug --- .github/workflows/integration_test.yaml | 3 - pyproject.toml | 2 +- src-docs/openstack_builder.md | 24 ++++- src/github_runner_image_builder/cli.py | 32 ++++--- src/github_runner_image_builder/config.py | 3 +- .../openstack_builder.py | 89 ++++++++++++++----- tests/integration/test_openstack_builder.py | 2 +- tests/unit/test_openstack_builder.py | 16 +++- 8 files changed, 126 insertions(+), 45 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index f7190bc..27228b8 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -25,9 +25,6 @@ jobs: pipx install tox - name: Run integration tests run: tox -e integration -- -m arm64 --image=${{ matrix.image }} ${{ secrets.INTEGRATION_TEST_ARGS }} - - name: Tmate - if: ${{ failure() }} - uses: canonical/action-tmate@main integration-tests-amd: name: Integration test (X64) runs-on: [self-hosted, X64, jammy, stg-private-endpoint] diff --git a/pyproject.toml b/pyproject.toml index 2b18d10..694c027 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-image-builder" -version = "0.5.3" +version = "0.5.4" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] diff --git a/src-docs/openstack_builder.md b/src-docs/openstack_builder.md index b9137c3..8d30076 100644 --- a/src-docs/openstack_builder.md +++ b/src-docs/openstack_builder.md @@ -19,7 +19,7 @@ Module for interacting with external openstack VM image builder. --- - + ## function `determine_cloud` @@ -49,7 +49,7 @@ Automatically determine cloud to use from clouds.yaml by selecting the first clo --- - + ## function `initialize` @@ -71,7 +71,11 @@ Upload ubuntu base images to openstack to use as builder base. This is a separat --- +<<<<<<< HEAD +======= + +>>>>>>> 4eabac6 (feat: multicloud upload (#27)) ## function `run` @@ -105,7 +109,7 @@ Run external OpenStack builder instance and create a snapshot. --- - + ## class `CloudConfig` The OpenStack cloud configuration values. @@ -118,13 +122,27 @@ The OpenStack cloud configuration values. - `flavor`: The OpenStack flavor to launch builder VMs on. - `network`: The OpenStack network to launch the builder VMs on. - `proxy`: The proxy to enable on builder VMs. +<<<<<<< HEAD +======= + - `upload_cloud_names`: The OpenStack cloud names to upload the snapshot to. (Defaults to the same cloud) +>>>>>>> 4eabac6 (feat: multicloud upload (#27)) ### method `__init__` ```python +<<<<<<< HEAD __init__(cloud_name: str, flavor: str, network: str, proxy: str) → None +======= +__init__( + cloud_name: str, + flavor: str, + network: str, + proxy: str, + upload_cloud_names: Optional[Iterable[str]] +) → None +>>>>>>> 4eabac6 (feat: multicloud upload (#27)) ``` diff --git a/src/github_runner_image_builder/cli.py b/src/github_runner_image_builder/cli.py index f24104e..765fc1a 100644 --- a/src/github_runner_image_builder/cli.py +++ b/src/github_runner_image_builder/cli.py @@ -148,14 +148,14 @@ def get_latest_build_id(cloud_name: str, image_name: str) -> None: "Ignored if --experimental-external is not enabled", ) @click.option( - "--upload-cloud", + "--upload-clouds", default="", - help="EXPERIMENTAL: Different cloud to use to upload the externally built image. The cloud " - "connection parameters should exist in the clouds.yaml. Ignored if --experimental-external is" - " not enabled, as a part of external build mode parameter.", + help="EXPERIMENTAL: Comma separated list of different clouds to use to upload the externally " + "built image. The cloud connection parameters should exist in the clouds.yaml. Ignored if " + "--experimental-external is not enabled, as a part of external build mode parameter.", ) # click doesn't yet support dataclasses, hence all arguments are required. -def run( # pylint: disable=too-many-arguments +def run( # pylint: disable=too-many-arguments, too-many-locals arch: config.Arch | None, cloud_name: str, image_name: str, @@ -167,7 +167,7 @@ def run( # pylint: disable=too-many-arguments flavor: str, network: str, proxy: str, - upload_cloud: str, + upload_clouds: str, ) -> None: """Build a cloud image using chroot and upload it to OpenStack. @@ -184,12 +184,12 @@ def run( # pylint: disable=too-many-arguments flavor: The Openstack flavor to create server to build images. network: The Openstack network to assign to server to build images. proxy: Proxy to use for external build VMs. - upload_cloud: The Opensttack cloud to use to upload externally built image. + upload_clouds: The Openstack cloud to use to upload externally built image. """ arch = arch if arch else config.get_supported_arch() base = config.BaseImage.from_str(base_image) if not experimental_external: - image_id = builder.run( + image_ids = builder.run( cloud_name=cloud_name, image_config=config.ImageConfig( arch=arch, @@ -201,15 +201,21 @@ def run( # pylint: disable=too-many-arguments ) # 2024/07/09: Only print image_id for chroot building for backwards compatibility. To be # deprecated when external builder is in stable. - click.echo(image_id, nl=False) + click.echo(image_ids, nl=False) else: - image_id = openstack_builder.run( + # coverage thinks this line can lead to exit. + upload_cloud_names = ( # pragma: no cover + [cloud_name.strip() for cloud_name in upload_clouds.split(",")] + if upload_clouds + else None + ) + image_ids = openstack_builder.run( cloud_config=openstack_builder.CloudConfig( cloud_name=cloud_name, flavor=flavor, network=network, proxy=proxy, - upload_cloud_name=upload_cloud, + upload_cloud_names=upload_cloud_names, ), image_config=config.ImageConfig( arch=arch, @@ -219,7 +225,7 @@ def run( # pylint: disable=too-many-arguments ), keep_revisions=keep_revisions, ) - click.echo(f"Image build success:\n{image_id}", nl=False) + click.echo(f"Image build success:\n{image_ids}", nl=False) if callback_script: # The callback script is a user trusted script. - subprocess.check_call([str(callback_script), image_id]) # nosec: B603 + subprocess.check_call([str(callback_script), image_ids]) # nosec: B603 diff --git a/src/github_runner_image_builder/config.py b/src/github_runner_image_builder/config.py index 6846e20..dfeb22a 100644 --- a/src/github_runner_image_builder/config.py +++ b/src/github_runner_image_builder/config.py @@ -128,7 +128,8 @@ def from_str(cls, tag_or_name: str) -> "BaseImage": _LOG_LEVELS = (logging.DEBUG, logging.INFO, logging.WARNING, logging.ERROR) LOG_LEVELS = tuple( - itertools.chain( + str(level) + for level in itertools.chain( _LOG_LEVELS, (logging.getLevelName(level) for level in _LOG_LEVELS), (logging.getLevelName(level).lower() for level in _LOG_LEVELS), diff --git a/src/github_runner_image_builder/openstack_builder.py b/src/github_runner_image_builder/openstack_builder.py index 8d2a4ec..8ecdbe5 100644 --- a/src/github_runner_image_builder/openstack_builder.py +++ b/src/github_runner_image_builder/openstack_builder.py @@ -8,6 +8,7 @@ import pathlib import shutil import time +import typing import fabric import jinja2 @@ -197,14 +198,15 @@ class CloudConfig: flavor: The OpenStack flavor to launch builder VMs on. network: The OpenStack network to launch the builder VMs on. proxy: The proxy to enable on builder VMs. - upload_cloud_name: The OpenStack cloud name to upload the snapshot to. (Default same cloud) + upload_cloud_names: The OpenStack cloud names to upload the snapshot to. (Defaults to \ + the same cloud) """ cloud_name: str flavor: str network: str proxy: str - upload_cloud_name: str | None + upload_cloud_names: typing.Iterable[str] | None def run( @@ -257,29 +259,20 @@ def run( ) logger.info("Requested snapshot, waiting for snapshot to complete: %s.", image.id) _wait_for_snapshot_complete(conn=conn, image=image) - if cloud_config.upload_cloud_name: - logger.info("Downloading snapshot to %s.", IMAGE_SNAPSHOT_FILE_PATH) - conn.download_image( - name_or_id=image.id, output_file=IMAGE_SNAPSHOT_FILE_PATH, stream=True - ) - logger.info("Uploading downloaded snapshot to %s.", cloud_config.upload_cloud_name) - image = store.upload_image( + images = _upload_to_clouds( + conn=conn, + image=image, + upload_cloud_names=cloud_config.upload_cloud_names, + upload_cloud_config=_UploadCloudConfig( arch=image_config.arch, - cloud_name=cloud_config.upload_cloud_name, image_name=image_config.name, - image_path=IMAGE_SNAPSHOT_FILE_PATH, keep_revisions=keep_revisions, - ) - logger.info( - "Uploaded snapshot on cloud %s, id: %s, name: %s", - cloud_config.upload_cloud_name, - image.id, - image.name, - ) + ), + ) logger.info("Deleting builder VM: %s (%s)", builder.name, builder.id) conn.delete_server(name_or_id=builder.id, wait=True, timeout=5 * 60) logger.info("Image builder run complete.") - return str(image.id) + return ",".join(str(image.id) for image in images) def _determine_flavor(conn: openstack.connection.Connection, flavor_name: str | None) -> str: @@ -518,6 +511,62 @@ def _wait_for_snapshot_complete( return time.sleep(60) image = conn.get_image(name_or_id=image.id) - if not image.status == "active": + if not image or not image.status == "active": logger.error("Timed out waiting for snapshot to be active, %s.", image.name) raise TimeoutError(f"Timed out waiting for snapshot to be active, {image.id}.") + + +@dataclasses.dataclass +class _UploadCloudConfig: + """The upload clouds arguments wrapper. + + Attributes: + arch: The architecture of the image to use as build base. + image_name: The name to upload the image as. + keep_revisions: Number of revisions to keep before deletion. + """ + + arch: Arch + image_name: str + keep_revisions: int + + +def _upload_to_clouds( + conn: openstack.connection.Connection, + image: openstack.compute.v2.image.Image, + upload_cloud_names: typing.Iterable[str] | None, + upload_cloud_config: _UploadCloudConfig, +) -> tuple[openstack.compute.v2.image.Image, ...]: + """Upload the snapshot image to different clouds. + + Args: + conn: The OpenStack connection instance. + image: The snapshot image to upload. + upload_cloud_names: The clouds to upload the image to. + upload_cloud_config: The upload image configuration. + + Returns: + The uploaded cloud images. + """ + if not upload_cloud_names: + return (image,) + logger.info("Downloading snapshot to %s.", IMAGE_SNAPSHOT_FILE_PATH) + conn.download_image(name_or_id=image.id, output_file=IMAGE_SNAPSHOT_FILE_PATH, stream=True) + images: list[openstack.compute.v2.image.Image] = [] + for cloud_name in upload_cloud_names: + logger.info("Uploading downloaded snapshot to %s.", cloud_name) + image = store.upload_image( + arch=upload_cloud_config.arch, + cloud_name=cloud_name, + image_name=upload_cloud_config.image_name, + image_path=IMAGE_SNAPSHOT_FILE_PATH, + keep_revisions=upload_cloud_config.keep_revisions, + ) + images.append(image) + logger.info( + "Uploaded snapshot on cloud %s, id: %s, name: %s", + cloud_name, + image.id, + image.name, + ) + return tuple(images) diff --git a/tests/integration/test_openstack_builder.py b/tests/integration/test_openstack_builder.py index 7d8e2c0..c633396 100644 --- a/tests/integration/test_openstack_builder.py +++ b/tests/integration/test_openstack_builder.py @@ -90,7 +90,7 @@ def cli_run_fixture( flavor=openstack_metadata.flavor, network=openstack_metadata.network, proxy=proxy.http, - upload_cloud_name=cloud_name, + upload_cloud_names=[cloud_name], ), image_config=config.ImageConfig( arch=arch, diff --git a/tests/unit/test_openstack_builder.py b/tests/unit/test_openstack_builder.py index ac1d23e..30cf5fb 100644 --- a/tests/unit/test_openstack_builder.py +++ b/tests/unit/test_openstack_builder.py @@ -174,7 +174,7 @@ def test__create_security_group(): flavor="test-flavor", network="test-network", proxy="test-proxy", - upload_cloud_name="", + upload_cloud_names=[], ), id="no upload-cloud-name", ), @@ -184,9 +184,19 @@ def test__create_security_group(): flavor="test-flavor", network="test-network", proxy="test-proxy", - upload_cloud_name="test-cloud-2", + upload_cloud_names=["test-cloud-1"], ), - id="upload-cloud-name defined", + id="single upload-cloud-name defined", + ), + pytest.param( + openstack_builder.CloudConfig( + cloud_name="test-cloud", + flavor="test-flavor", + network="test-network", + proxy="test-proxy", + upload_cloud_names=["test-cloud-1", "test-cloud-2"], + ), + id="multiple upload-cloud-name defined", ), ], ) From 2a2dcff873ced55399743ee08f2f64d157b08686 Mon Sep 17 00:00:00 2001 From: Yanks Yoon <37652070+yanksyoon@users.noreply.github.com> Date: Fri, 20 Sep 2024 16:11:13 +0800 Subject: [PATCH 10/21] fix: sync disk before snapshot (#28) * fix: sync disk before snapshot * chore: bump rev * test: fixup tests --- pyproject.toml | 2 +- src/github_runner_image_builder/templates/cloud-init.sh.j2 | 3 +++ tests/unit/test_cli.py | 1 - tests/unit/test_openstack_builder.py | 5 ++++- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 694c027..49cc276 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-image-builder" -version = "0.5.4" +version = "0.5.5" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] diff --git a/src/github_runner_image_builder/templates/cloud-init.sh.j2 b/src/github_runner_image_builder/templates/cloud-init.sh.j2 index e04f2f5..9e8e5be 100644 --- a/src/github_runner_image_builder/templates/cloud-init.sh.j2 +++ b/src/github_runner_image_builder/templates/cloud-init.sh.j2 @@ -130,3 +130,6 @@ export -f install_yq su ubuntu -c "bash -c 'install_yq'" install_github_runner "$github_runner_version" "$github_runner_arch" chown_home +# Make sure the disk is synced for snapshot +sync +echo "Finished sync" diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index c11b31e..f4951aa 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -238,5 +238,4 @@ def test_run( command, ) - print(result.stdout) assert result.exit_code == 0 diff --git a/tests/unit/test_openstack_builder.py b/tests/unit/test_openstack_builder.py index 30cf5fb..896a410 100644 --- a/tests/unit/test_openstack_builder.py +++ b/tests/unit/test_openstack_builder.py @@ -608,7 +608,10 @@ def test__generate_cloud_init_script(): export -f install_yq su ubuntu -c "bash -c 'install_yq'" install_github_runner "$github_runner_version" "$github_runner_arch" -chown_home\ +chown_home +# Make sure the disk is synced for snapshot +sync +echo "Finished sync"\ """ ) # pylint: enable=R0801 From 7e9a2609ef17843677b144531d8613aa03ec4a72 Mon Sep 17 00:00:00 2001 From: Yanks Yoon Date: Fri, 20 Sep 2024 09:37:35 +0000 Subject: [PATCH 11/21] test: fixup tests --- src-docs/builder.md | 4 ++-- src-docs/cli.md | 2 -- src-docs/config.md | 38 ++++++++++++++++++++++++++++++++--- src-docs/openstack_builder.md | 20 ++---------------- src-docs/store.md | 4 ++-- tests/unit/test_builder.py | 2 ++ 6 files changed, 43 insertions(+), 27 deletions(-) diff --git a/src-docs/builder.md b/src-docs/builder.md index b95c1ad..674f109 100644 --- a/src-docs/builder.md +++ b/src-docs/builder.md @@ -25,7 +25,7 @@ Module for interacting with qemu image builder. --- - + ## function `initialize` @@ -38,7 +38,7 @@ Configure the host machine to build images. --- - + ## function `run` diff --git a/src-docs/cli.md b/src-docs/cli.md index 8eb9e32..3ed2860 100644 --- a/src-docs/cli.md +++ b/src-docs/cli.md @@ -8,7 +8,5 @@ Main entrypoint for github-runner-image-builder cli application. **Global Variables** --------------- - **click** -- **BASE_CHOICES** -- **LOG_LEVELS** diff --git a/src-docs/config.md b/src-docs/config.md index 595d867..bf9fb5c 100644 --- a/src-docs/config.md +++ b/src-docs/config.md @@ -16,7 +16,7 @@ Module containing configurations. --- - + ## function `get_supported_arch` @@ -41,7 +41,7 @@ Get current machine architecture. --- - + ## class `Arch` Supported system architectures. @@ -59,7 +59,7 @@ Supported system architectures. --- - + ## class `BaseImage` The ubuntu OS base image to build and deploy runners on. @@ -75,3 +75,35 @@ The ubuntu OS base image to build and deploy runners on. +--- + + + +## class `ImageConfig` +The build image configuration values. + + + +**Attributes:** + + - `arch`: The architecture of the target image. + - `base`: The ubuntu base OS of the image. + - `runner_version`: The GitHub runner version to install on the VM. Defaults to latest. + - `name`: The image name to upload on OpenStack. + + + +### method `__init__` + +```python +__init__(arch: Arch, base: BaseImage, runner_version: str, name: str) → None +``` + + + + + + + + + diff --git a/src-docs/openstack_builder.md b/src-docs/openstack_builder.md index 8d30076..3f5dfb7 100644 --- a/src-docs/openstack_builder.md +++ b/src-docs/openstack_builder.md @@ -11,7 +11,6 @@ Module for interacting with external openstack VM image builder. - **CLOUD_YAML_PATHS** - **BUILDER_SSH_KEY_NAME** - **SHARED_SECURITY_GROUP_NAME** -- **IMAGE_SNAPSHOT_NAME** - **CREATE_SERVER_TIMEOUT** - **MIN_CPU** - **MIN_RAM** @@ -71,20 +70,14 @@ Upload ubuntu base images to openstack to use as builder base. This is a separat --- -<<<<<<< HEAD - -======= ->>>>>>> 4eabac6 (feat: multicloud upload (#27)) ## function `run` ```python run( - arch: Arch, - base: BaseImage, cloud_config: CloudConfig, - runner_version: str, + image_config: ImageConfig, keep_revisions: int ) → str ``` @@ -95,10 +88,8 @@ Run external OpenStack builder instance and create a snapshot. **Args:** - - `arch`: The architecture of the image to seed. - - `base`: The Ubuntu base to use as builder VM base. - `cloud_config`: The OpenStack cloud configuration values for builder VM. - - `runner_version`: The GitHub runner version to install on the VM. Defaults to latest. + - `image_config`: The target image configuration values. - `keep_revisions`: The number of image to keep for snapshot before deletion. @@ -122,19 +113,13 @@ The OpenStack cloud configuration values. - `flavor`: The OpenStack flavor to launch builder VMs on. - `network`: The OpenStack network to launch the builder VMs on. - `proxy`: The proxy to enable on builder VMs. -<<<<<<< HEAD -======= - `upload_cloud_names`: The OpenStack cloud names to upload the snapshot to. (Defaults to the same cloud) ->>>>>>> 4eabac6 (feat: multicloud upload (#27)) ### method `__init__` ```python -<<<<<<< HEAD -__init__(cloud_name: str, flavor: str, network: str, proxy: str) → None -======= __init__( cloud_name: str, flavor: str, @@ -142,7 +127,6 @@ __init__( proxy: str, upload_cloud_names: Optional[Iterable[str]] ) → None ->>>>>>> 4eabac6 (feat: multicloud upload (#27)) ``` diff --git a/src-docs/store.md b/src-docs/store.md index a48dff9..7d586e3 100644 --- a/src-docs/store.md +++ b/src-docs/store.md @@ -57,7 +57,7 @@ upload_image( image_name: str, image_path: Path, keep_revisions: int -) → str +) → Image ``` Upload image to openstack glance. @@ -81,7 +81,7 @@ Upload image to openstack glance. **Returns:** - The created image ID. + The created image. --- diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index 16c9a5f..80ec51a 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -282,9 +282,11 @@ def test_run(monkeypatch: pytest.MonkeyPatch): monkeypatch.setattr(builder.subprocess, "check_output", MagicMock()) monkeypatch.setattr(builder.subprocess, "run", MagicMock()) monkeypatch.setattr(builder, "_disable_unattended_upgrades", MagicMock()) + monkeypatch.setattr(builder, "_enable_network_fair_queuing_congestion", MagicMock()) monkeypatch.setattr(builder, "_configure_system_users", MagicMock()) monkeypatch.setattr(builder, "_install_yarn", MagicMock()) monkeypatch.setattr(builder, "_install_github_runner", MagicMock()) + monkeypatch.setattr(builder, "_chown_home", MagicMock()) monkeypatch.setattr(builder, "_disconnect_image_to_network_block_device", MagicMock()) monkeypatch.setattr(builder, "_compress_image", MagicMock()) monkeypatch.setattr( From 942711f83486e22539f10582f4665362a40657e3 Mon Sep 17 00:00:00 2001 From: Yanks Yoon Date: Mon, 23 Sep 2024 07:49:57 +0000 Subject: [PATCH 12/21] chore: refactor helper connection params --- src-docs/builder.md | 4 +- src/github_runner_image_builder/builder.py | 13 +++-- src/github_runner_image_builder/cli.py | 2 +- .../templates/cloud-init.sh.j2 | 3 +- src/github_runner_image_builder/utils.py | 2 +- tests/integration/helpers.py | 47 +++++++++++++------ tests/integration/test_image.py | 10 ++-- tests/integration/test_openstack_builder.py | 10 ++-- tests/unit/test_builder.py | 26 ++++++++-- tests/unit/test_openstack_builder.py | 3 +- 10 files changed, 80 insertions(+), 40 deletions(-) diff --git a/src-docs/builder.md b/src-docs/builder.md index 674f109..a74f793 100644 --- a/src-docs/builder.md +++ b/src-docs/builder.md @@ -25,7 +25,7 @@ Module for interacting with qemu image builder. --- - + ## function `initialize` @@ -38,7 +38,7 @@ Configure the host machine to build images. --- - + ## function `run` diff --git a/src/github_runner_image_builder/builder.py b/src/github_runner_image_builder/builder.py index a416981..97e0bad 100644 --- a/src/github_runner_image_builder/builder.py +++ b/src/github_runner_image_builder/builder.py @@ -90,6 +90,7 @@ HOST_YQ_BIN_PATH = Path("/usr/bin/yq") MOUNTED_YQ_BIN_PATH = IMAGE_MOUNT_DIR / "usr/bin/yq" IMAGE_HWE_PKG_FORMAT = "linux-generic-hwe-{VERSION}" +SYSCTL_CONF_PATH = Path("/etc/sysctl.conf") def initialize() -> None: @@ -577,16 +578,14 @@ def _enable_network_fair_queuing_congestion() -> None: NetworkFairQueuingEnableError: If there was an error disabling unattended upgrade related services. """ + with open(SYSCTL_CONF_PATH, mode="a", encoding="utf-8") as sysctl_file: + sysctl_file.write("net.core.default_qdisc=fq") + sysctl_file.write("net.ipv4.tcp_congestion_control=bbr") try: output = subprocess.check_output( - ["/usr/bin/systemctl", "net.core.default_qdisc=fq"], timeout=30 + ["/usr/bin/sudo", "-E", "/usr/bin/sysctl", "-p"], timeout=30 ) # nosec: B603 - logger.info("net core default_qdisk out: %s", output) - output = subprocess.check_output( - ["/usr/bin/systemctl", "net.ipv4.tcp_congestion_control=bbr"], timeout=30 - ) # nosec: B603 - # There is no need to test the log output. - logger.info("net.ipv4.tcp_congestion_control=bbr out: %s", output) # pragma: nocover + logger.info("Sysctl reload out: %s", output) except subprocess.CalledProcessError as exc: logger.exception( "Error enabling network congestion policy, cmd: %s, code: %s, err: %s", diff --git a/src/github_runner_image_builder/cli.py b/src/github_runner_image_builder/cli.py index 765fc1a..a6aeaf1 100644 --- a/src/github_runner_image_builder/cli.py +++ b/src/github_runner_image_builder/cli.py @@ -155,7 +155,7 @@ def get_latest_build_id(cloud_name: str, image_name: str) -> None: "--experimental-external is not enabled, as a part of external build mode parameter.", ) # click doesn't yet support dataclasses, hence all arguments are required. -def run( # pylint: disable=too-many-arguments, too-many-locals +def run( # pylint: disable=too-many-arguments, too-many-locals, too-many-positional-arguments arch: config.Arch | None, cloud_name: str, image_name: str, diff --git a/src/github_runner_image_builder/templates/cloud-init.sh.j2 b/src/github_runner_image_builder/templates/cloud-init.sh.j2 index 9e8e5be..19718cf 100644 --- a/src/github_runner_image_builder/templates/cloud-init.sh.j2 +++ b/src/github_runner_image_builder/templates/cloud-init.sh.j2 @@ -52,10 +52,11 @@ function disable_unattended_upgrades() { } function enable_network_fair_queuing_congestion() { - /usr/bin/cat < bool: return result.exit_code == 0 -# All the arguments are necessary -async def wait_for_valid_connection( # pylint: disable=too-many-arguments - connection: Connection, - server_name: str, - network: str, - ssh_key: Path, +@dataclasses.dataclass +class OpenStackConnectionParams: + """Parameters for connecting to OpenStack instance. + + Attributes: + connection: The openstack connection client to communicate with Openstack. + server_name: Openstack server to find the valid connection from. + network: The network to find valid connection from. + ssh_key: The path to public ssh_key to create connection with. + """ + + connection: Connection + server_name: str + network: str + ssh_key: Path + + +async def wait_for_valid_connection( + connection_params: OpenStackConnectionParams, timeout: int = 30 * 60, proxy: types.ProxyConfig | None = None, dockerhub_mirror: str | None = None, @@ -242,10 +256,7 @@ async def wait_for_valid_connection( # pylint: disable=too-many-arguments """Wait for a valid SSH connection from Openstack server. Args: - connection: The openstack connection client to communicate with Openstack. - server_name: Openstack server to find the valid connection from. - network: The network to find valid connection from. - ssh_key: The path to public ssh_key to create connection with. + connection_params: Parameters for connecting to OpenStack instance. timeout: Number of seconds to wait before raising a timeout error. proxy: The proxy to configure on host runner. dockerhub_mirror: The DockerHub mirror URL. @@ -258,18 +269,24 @@ async def wait_for_valid_connection( # pylint: disable=too-many-arguments """ start_time = time.time() while time.time() - start_time <= timeout: - server: Server | None = connection.get_server(name_or_id=server_name) + server: Server | None = connection_params.connection.get_server( + name_or_id=connection_params.server_name + ) if not server or not server.addresses: time.sleep(10) continue - for address in server.addresses[network]: + for address in server.addresses[connection_params.network]: ip = address["addr"] - logger.info("Trying SSH into %s using key: %s...", ip, str(ssh_key.absolute())) + logger.info( + "Trying SSH into %s using key: %s...", + ip, + str(connection_params.ssh_key.absolute()), + ) ssh_connection = SSHConnection( host=ip, user="ubuntu", - connect_kwargs={"key_filename": str(ssh_key.absolute())}, - connect_timeout=10 * 60, + connect_kwargs={"key_filename": str(connection_params.ssh_key.absolute())}, + connect_timeout=60 * 10, ) try: result: Result = ssh_connection.run("echo 'hello world'") diff --git a/tests/integration/test_image.py b/tests/integration/test_image.py index db1ad93..c103463 100644 --- a/tests/integration/test_image.py +++ b/tests/integration/test_image.py @@ -60,10 +60,12 @@ async def ssh_connection_fixture( """The openstack server ssh connection fixture.""" logger.info("Setting up SSH connection.") ssh_connection = await helpers.wait_for_valid_connection( - connection=openstack_metadata.connection, - server_name=openstack_server.name, - network=openstack_metadata.network, - ssh_key=openstack_metadata.ssh_key.private_key, + connection_params=helpers.OpenStackConnectionParams( + connection=openstack_metadata.connection, + server_name=openstack_server.name, + network=openstack_metadata.network, + ssh_key=openstack_metadata.ssh_key.private_key, + ), proxy=proxy, dockerhub_mirror=dockerhub_mirror, ) diff --git a/tests/integration/test_openstack_builder.py b/tests/integration/test_openstack_builder.py index c633396..2c2a49c 100644 --- a/tests/integration/test_openstack_builder.py +++ b/tests/integration/test_openstack_builder.py @@ -135,10 +135,12 @@ async def ssh_connection_fixture( """The openstack server ssh connection fixture.""" logger.info("Setting up SSH connection.") ssh_connection = await helpers.wait_for_valid_connection( - connection=openstack_metadata.connection, - server_name=openstack_server.name, - network=openstack_metadata.network, - ssh_key=openstack_metadata.ssh_key.private_key, + connection_params=helpers.OpenStackConnectionParams( + connection=openstack_metadata.connection, + server_name=openstack_server.name, + network=openstack_metadata.network, + ssh_key=openstack_metadata.ssh_key.private_key, + ), proxy=proxy, dockerhub_mirror=dockerhub_mirror, ) diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index 80ec51a..89d91ba 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -506,22 +506,21 @@ def test__disable_unattended_upgrades_subprocess_fail(monkeypatch: pytest.Monkey def test__enable_network_fair_queuing_congestion_fail( monkeypatch: pytest.MonkeyPatch, error: subprocess.CalledProcessError | subprocess.SubprocessError, + tmp_path: Path, ): """ arrange: given a monkeypatched subprocess run function that raises SubprocessError. act: when _enable_network_fair_queuing_congestion is called. assert: the NetworkFairQueuingEnableError is raised. """ + monkeypatch.setattr(builder, "SYSCTL_CONF_PATH", (tmp_path / "sysctl.conf")) # Pylint thinks the testing mock patches are duplicate code (side effects are different). # pylint: disable=duplicate-code monkeypatch.setattr( subprocess, "check_output", MagicMock( - side_effect=[ - *([None]), - error, - ] + side_effect=error, ), ) @@ -529,6 +528,25 @@ def test__enable_network_fair_queuing_congestion_fail( builder._enable_network_fair_queuing_congestion() +def test__enable_network_fair_queuing_congestion(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): + """ + arrange: given monkeypatched sysctl.conf path and sysctl subprocess run. + act: when _enable_network_fair_queuing_congestion is called. + assert: the configuration are written correctly and config reload is called. + """ + monkeypatch.setattr(subprocess, "check_output", (mock_subprocess_call := MagicMock())) + monkeypatch.setattr(builder, "SYSCTL_CONF_PATH", test_path := tmp_path / "sysctl.conf") + + builder._enable_network_fair_queuing_congestion() + + mock_subprocess_call.assert_called_once_with( + ["/usr/bin/sudo", "-E", "/usr/bin/sysctl", "-p"], timeout=30 + ) + config_contents = test_path.read_text(encoding="utf-8") + assert "net.core.default_qdisc=fq" in config_contents + assert "net.ipv4.tcp_congestion_control=bbr" in config_contents + + def test__configure_system_users(monkeypatch: pytest.MonkeyPatch): """ arrange: given a monkeypatched subprocess run calls that raises an exception. diff --git a/tests/unit/test_openstack_builder.py b/tests/unit/test_openstack_builder.py index 896a410..a1b8de8 100644 --- a/tests/unit/test_openstack_builder.py +++ b/tests/unit/test_openstack_builder.py @@ -527,10 +527,11 @@ def test__generate_cloud_init_script(): } function enable_network_fair_queuing_congestion() { - /usr/bin/cat < Date: Mon, 23 Sep 2024 09:40:35 +0000 Subject: [PATCH 13/21] fix: use sysbin --- src/github_runner_image_builder/builder.py | 2 +- src/github_runner_image_builder/templates/cloud-init.sh.j2 | 2 +- tests/unit/test_builder.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/github_runner_image_builder/builder.py b/src/github_runner_image_builder/builder.py index 97e0bad..79649bf 100644 --- a/src/github_runner_image_builder/builder.py +++ b/src/github_runner_image_builder/builder.py @@ -583,7 +583,7 @@ def _enable_network_fair_queuing_congestion() -> None: sysctl_file.write("net.ipv4.tcp_congestion_control=bbr") try: output = subprocess.check_output( - ["/usr/bin/sudo", "-E", "/usr/bin/sysctl", "-p"], timeout=30 + ["/usr/bin/sudo", "-E", "/usr/sbin/sysctl", "-p"], timeout=30 ) # nosec: B603 logger.info("Sysctl reload out: %s", output) except subprocess.CalledProcessError as exc: diff --git a/src/github_runner_image_builder/templates/cloud-init.sh.j2 b/src/github_runner_image_builder/templates/cloud-init.sh.j2 index 19718cf..b592678 100644 --- a/src/github_runner_image_builder/templates/cloud-init.sh.j2 +++ b/src/github_runner_image_builder/templates/cloud-init.sh.j2 @@ -56,7 +56,7 @@ function enable_network_fair_queuing_congestion() { net.core.default_qdisc=fq net.ipv4.tcp_congestion_control=bbr EOF - /usr/bin/systctl -p + /usr/sbin/systctl -p } function configure_system_users() { diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index 89d91ba..b4d9456 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -540,7 +540,7 @@ def test__enable_network_fair_queuing_congestion(monkeypatch: pytest.MonkeyPatch builder._enable_network_fair_queuing_congestion() mock_subprocess_call.assert_called_once_with( - ["/usr/bin/sudo", "-E", "/usr/bin/sysctl", "-p"], timeout=30 + ["/usr/bin/sudo", "-E", "/usr/sbin/sysctl", "-p"], timeout=30 ) config_contents = test_path.read_text(encoding="utf-8") assert "net.core.default_qdisc=fq" in config_contents From 96b042629b5ccf143b5fb8037445f1e04cb7902b Mon Sep 17 00:00:00 2001 From: Yanks Yoon Date: Mon, 23 Sep 2024 10:34:58 +0000 Subject: [PATCH 14/21] test: fixup tests --- tests/unit/test_openstack_builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_openstack_builder.py b/tests/unit/test_openstack_builder.py index a1b8de8..3b1ed3d 100644 --- a/tests/unit/test_openstack_builder.py +++ b/tests/unit/test_openstack_builder.py @@ -531,7 +531,7 @@ def test__generate_cloud_init_script(): net.core.default_qdisc=fq net.ipv4.tcp_congestion_control=bbr EOF - /usr/bin/systctl -p + /usr/sbin/systctl -p } function configure_system_users() { From 340532c8dcd265959cc93420f4589f3d0669fe6f Mon Sep 17 00:00:00 2001 From: Yanks Yoon Date: Mon, 23 Sep 2024 11:23:36 +0000 Subject: [PATCH 15/21] test: sudo sysctl print --- tests/integration/commands.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/commands.py b/tests/integration/commands.py index e143fec..ba3ce6b 100644 --- a/tests/integration/commands.py +++ b/tests/integration/commands.py @@ -76,10 +76,10 @@ class Commands: Commands(name="test HWE kernel", command="uname -a | grep generic"), Commands( name="test network congestion policy(fq)", - command="sysctl -a | grep 'net.core.default_qdisc = fq'", + command="sudo sysctl -a | grep 'net.core.default_qdisc = fq'", ), Commands( name="test network congestion policy", - command="sysctl -a | grep 'net.ipv4.tcp_congestion_control = bbr'", + command="sudo sysctl -a | grep 'net.ipv4.tcp_congestion_control = bbr'", ), ) From 699fe2322112e7b6adfa11173cf474a0bae19b0e Mon Sep 17 00:00:00 2001 From: Yanks Yoon Date: Mon, 23 Sep 2024 11:39:20 +0000 Subject: [PATCH 16/21] fix: remove reload sysctl --- src-docs/builder.md | 4 +- src-docs/errors.md | 45 ++++++++-------------- src/github_runner_image_builder/builder.py | 24 +----------- src/github_runner_image_builder/errors.py | 4 -- tests/unit/test_builder.py | 43 --------------------- 5 files changed, 20 insertions(+), 100 deletions(-) diff --git a/src-docs/builder.md b/src-docs/builder.md index a74f793..674f109 100644 --- a/src-docs/builder.md +++ b/src-docs/builder.md @@ -25,7 +25,7 @@ Module for interacting with qemu image builder. --- - + ## function `initialize` @@ -38,7 +38,7 @@ Configure the host machine to build images. --- - + ## function `run` diff --git a/src-docs/errors.md b/src-docs/errors.md index e12ec4b..8e18422 100644 --- a/src-docs/errors.md +++ b/src-docs/errors.md @@ -143,17 +143,6 @@ Represents an error while disabling unattended-upgrade related services. -## class `NetworkFairQueuingEnableError` -Represents an error while enabling network fair queueing policy. - - - - - ---- - - - ## class `SystemUserConfigurationError` Represents an error while adding user to chroot env. @@ -163,7 +152,7 @@ Represents an error while adding user to chroot env. --- - + ## class `PermissionConfigurationError` Represents an error while modifying dir permissions. @@ -174,7 +163,7 @@ Represents an error while modifying dir permissions. --- - + ## class `YQBuildError` Represents an error while building yq binary from source. @@ -185,7 +174,7 @@ Represents an error while building yq binary from source. --- - + ## class `YarnInstallError` Represents an error installilng Yarn. @@ -196,7 +185,7 @@ Represents an error installilng Yarn. --- - + ## class `RunnerDownloadError` Represents an error downloading GitHub runner tar archive. @@ -207,7 +196,7 @@ Represents an error downloading GitHub runner tar archive. --- - + ## class `ImageCompressError` Represents an error while compressing cloud-img. @@ -218,7 +207,7 @@ Represents an error while compressing cloud-img. --- - + ## class `HomeDirectoryChangeOwnershipError` Represents an error while changing ubuntu home directory. @@ -229,7 +218,7 @@ Represents an error while changing ubuntu home directory. --- - + ## class `OpenstackBaseError` Represents an error while interacting with Openstack. @@ -240,7 +229,7 @@ Represents an error while interacting with Openstack. --- - + ## class `UnauthorizedError` Represents an unauthorized connection to Openstack. @@ -251,7 +240,7 @@ Represents an unauthorized connection to Openstack. --- - + ## class `UploadImageError` Represents an error when uploading image to Openstack. @@ -262,7 +251,7 @@ Represents an error when uploading image to Openstack. --- - + ## class `OpenstackError` Represents an error while communicating with Openstack. @@ -273,7 +262,7 @@ Represents an error while communicating with Openstack. --- - + ## class `CloudsYAMLError` Represents an error with clouds.yaml for OpenStack connection. @@ -284,7 +273,7 @@ Represents an error with clouds.yaml for OpenStack connection. --- - + ## class `NotFoundError` Represents an error with not matching OpenStack object found. @@ -295,7 +284,7 @@ Represents an error with not matching OpenStack object found. --- - + ## class `FlavorNotFoundError` Represents an error with given OpenStack flavor not found. @@ -306,7 +295,7 @@ Represents an error with given OpenStack flavor not found. --- - + ## class `FlavorRequirementsNotMetError` Represents an error with given OpenStack flavor not meeting the minimum requirements. @@ -317,7 +306,7 @@ Represents an error with given OpenStack flavor not meeting the minimum requirem --- - + ## class `NetworkNotFoundError` Represents an error with given OpenStack network not found. @@ -328,7 +317,7 @@ Represents an error with given OpenStack network not found. --- - + ## class `AddressNotFoundError` Represents an error with OpenStack instance not receiving an IP address. @@ -339,7 +328,7 @@ Represents an error with OpenStack instance not receiving an IP address. --- - + ## class `CloudInitFailError` Represents an error with cloud-init. diff --git a/src/github_runner_image_builder/builder.py b/src/github_runner_image_builder/builder.py index 79649bf..11545f9 100644 --- a/src/github_runner_image_builder/builder.py +++ b/src/github_runner_image_builder/builder.py @@ -33,7 +33,6 @@ ImageConnectError, ImageResizeError, NetworkBlockDeviceError, - NetworkFairQueuingEnableError, PermissionConfigurationError, ResizePartitionError, RunnerDownloadError, @@ -572,31 +571,10 @@ def _disable_unattended_upgrades() -> None: def _enable_network_fair_queuing_congestion() -> None: - """Disable unatteneded upgrades to prevent apt locks. - - Raises: - NetworkFairQueuingEnableError: If there was an error disabling unattended upgrade related - services. - """ + """Enable bbr traffic congestion algorithm.""" with open(SYSCTL_CONF_PATH, mode="a", encoding="utf-8") as sysctl_file: sysctl_file.write("net.core.default_qdisc=fq") sysctl_file.write("net.ipv4.tcp_congestion_control=bbr") - try: - output = subprocess.check_output( - ["/usr/bin/sudo", "-E", "/usr/sbin/sysctl", "-p"], timeout=30 - ) # nosec: B603 - logger.info("Sysctl reload out: %s", output) - except subprocess.CalledProcessError as exc: - logger.exception( - "Error enabling network congestion policy, cmd: %s, code: %s, err: %s", - exc.cmd, - exc.returncode, - exc.output, - ) - raise NetworkFairQueuingEnableError from exc - except subprocess.SubprocessError as exc: - logger.exception("Error enabling network congestion policy.") - raise NetworkFairQueuingEnableError from exc def _configure_system_users() -> None: diff --git a/src/github_runner_image_builder/errors.py b/src/github_runner_image_builder/errors.py index 9d2efb8..2acf633 100644 --- a/src/github_runner_image_builder/errors.py +++ b/src/github_runner_image_builder/errors.py @@ -53,10 +53,6 @@ class UnattendedUpgradeDisableError(BuildImageError): """Represents an error while disabling unattended-upgrade related services.""" -class NetworkFairQueuingEnableError(BuildImageError): - """Represents an error while enabling network fair queueing policy.""" - - class SystemUserConfigurationError(BuildImageError): """Represents an error while adding user to chroot env.""" diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index b4d9456..51bac1e 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -23,7 +23,6 @@ ImageConnectError, ImageResizeError, NetworkBlockDeviceError, - NetworkFairQueuingEnableError, PermissionConfigurationError, ResizePartitionError, SystemUserConfigurationError, @@ -490,58 +489,16 @@ def test__disable_unattended_upgrades_subprocess_fail(monkeypatch: pytest.Monkey assert "Failed to disable unattended upgrades" in str(exc.getrepr()) -@pytest.mark.parametrize( - "error", - [ - pytest.param( - subprocess.CalledProcessError(1, [], "Something happened", ""), - id="called process error", - ), - pytest.param( - subprocess.SubprocessError(), - id="general subprocess error", - ), - ], -) -def test__enable_network_fair_queuing_congestion_fail( - monkeypatch: pytest.MonkeyPatch, - error: subprocess.CalledProcessError | subprocess.SubprocessError, - tmp_path: Path, -): - """ - arrange: given a monkeypatched subprocess run function that raises SubprocessError. - act: when _enable_network_fair_queuing_congestion is called. - assert: the NetworkFairQueuingEnableError is raised. - """ - monkeypatch.setattr(builder, "SYSCTL_CONF_PATH", (tmp_path / "sysctl.conf")) - # Pylint thinks the testing mock patches are duplicate code (side effects are different). - # pylint: disable=duplicate-code - monkeypatch.setattr( - subprocess, - "check_output", - MagicMock( - side_effect=error, - ), - ) - - with pytest.raises(NetworkFairQueuingEnableError): - builder._enable_network_fair_queuing_congestion() - - def test__enable_network_fair_queuing_congestion(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): """ arrange: given monkeypatched sysctl.conf path and sysctl subprocess run. act: when _enable_network_fair_queuing_congestion is called. assert: the configuration are written correctly and config reload is called. """ - monkeypatch.setattr(subprocess, "check_output", (mock_subprocess_call := MagicMock())) monkeypatch.setattr(builder, "SYSCTL_CONF_PATH", test_path := tmp_path / "sysctl.conf") builder._enable_network_fair_queuing_congestion() - mock_subprocess_call.assert_called_once_with( - ["/usr/bin/sudo", "-E", "/usr/sbin/sysctl", "-p"], timeout=30 - ) config_contents = test_path.read_text(encoding="utf-8") assert "net.core.default_qdisc=fq" in config_contents assert "net.ipv4.tcp_congestion_control=bbr" in config_contents From b372049c6cccbbd2db03f581cb977a73b835af18 Mon Sep 17 00:00:00 2001 From: Yanks Yoon Date: Tue, 24 Sep 2024 04:22:40 +0000 Subject: [PATCH 17/21] fix: typo --- src/github_runner_image_builder/templates/cloud-init.sh.j2 | 2 +- tests/unit/test_openstack_builder.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/github_runner_image_builder/templates/cloud-init.sh.j2 b/src/github_runner_image_builder/templates/cloud-init.sh.j2 index b592678..a11fb2d 100644 --- a/src/github_runner_image_builder/templates/cloud-init.sh.j2 +++ b/src/github_runner_image_builder/templates/cloud-init.sh.j2 @@ -56,7 +56,7 @@ function enable_network_fair_queuing_congestion() { net.core.default_qdisc=fq net.ipv4.tcp_congestion_control=bbr EOF - /usr/sbin/systctl -p + /usr/sbin/sysctl -p } function configure_system_users() { diff --git a/tests/unit/test_openstack_builder.py b/tests/unit/test_openstack_builder.py index 3b1ed3d..4c7e3b5 100644 --- a/tests/unit/test_openstack_builder.py +++ b/tests/unit/test_openstack_builder.py @@ -531,7 +531,7 @@ def test__generate_cloud_init_script(): net.core.default_qdisc=fq net.ipv4.tcp_congestion_control=bbr EOF - /usr/sbin/systctl -p + /usr/sbin/sysctl -p } function configure_system_users() { From 61bd43a8cd0b21d705bc4f16d2af844f2196d6ac Mon Sep 17 00:00:00 2001 From: Yanks Yoon Date: Tue, 24 Sep 2024 07:59:15 +0000 Subject: [PATCH 18/21] newlines for config file contents --- src/github_runner_image_builder/builder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/github_runner_image_builder/builder.py b/src/github_runner_image_builder/builder.py index 11545f9..4a03f36 100644 --- a/src/github_runner_image_builder/builder.py +++ b/src/github_runner_image_builder/builder.py @@ -573,8 +573,8 @@ def _disable_unattended_upgrades() -> None: def _enable_network_fair_queuing_congestion() -> None: """Enable bbr traffic congestion algorithm.""" with open(SYSCTL_CONF_PATH, mode="a", encoding="utf-8") as sysctl_file: - sysctl_file.write("net.core.default_qdisc=fq") - sysctl_file.write("net.ipv4.tcp_congestion_control=bbr") + sysctl_file.write("net.core.default_qdisc=fq\n") + sysctl_file.write("net.ipv4.tcp_congestion_control=bbr\n") def _configure_system_users() -> None: From f1a12130d31197b525355a3252d3861fc5d0cbd3 Mon Sep 17 00:00:00 2001 From: Yanks Yoon Date: Tue, 24 Sep 2024 08:19:20 +0000 Subject: [PATCH 19/21] indentation --- src/github_runner_image_builder/templates/cloud-init.sh.j2 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/github_runner_image_builder/templates/cloud-init.sh.j2 b/src/github_runner_image_builder/templates/cloud-init.sh.j2 index a11fb2d..2d19c2e 100644 --- a/src/github_runner_image_builder/templates/cloud-init.sh.j2 +++ b/src/github_runner_image_builder/templates/cloud-init.sh.j2 @@ -53,9 +53,9 @@ function disable_unattended_upgrades() { function enable_network_fair_queuing_congestion() { /usr/bin/cat < Date: Tue, 24 Sep 2024 08:55:30 +0000 Subject: [PATCH 20/21] fixup unit tests --- tests/unit/test_openstack_builder.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_openstack_builder.py b/tests/unit/test_openstack_builder.py index 4c7e3b5..c651d95 100644 --- a/tests/unit/test_openstack_builder.py +++ b/tests/unit/test_openstack_builder.py @@ -528,9 +528,9 @@ def test__generate_cloud_init_script(): function enable_network_fair_queuing_congestion() { /usr/bin/cat < Date: Tue, 24 Sep 2024 08:56:14 +0000 Subject: [PATCH 21/21] increment version --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 49cc276..afed789 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-image-builder" -version = "0.5.5" +version = "0.5.6" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ]