Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: network optimization #29

Merged
merged 22 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

[project]
name = "github-runner-image-builder"
version = "0.5.5"
version = "0.5.6"
authors = [
{ name = "Canonical IS DevOps", email = "[email protected]" },
]
Expand Down
4 changes: 2 additions & 2 deletions src-docs/builder.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Module for interacting with qemu image builder.

---

<a href="../src/github_runner_image_builder/builder.py#L93"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/github_runner_image_builder/builder.py#L95"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>function</kbd> `initialize`

Expand All @@ -38,7 +38,7 @@ Configure the host machine to build images.

---

<a href="../src/github_runner_image_builder/builder.py#L157"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/github_runner_image_builder/builder.py#L159"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>function</kbd> `run`

Expand Down
31 changes: 21 additions & 10 deletions src-docs/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,17 @@ Represents an error while compressing cloud-img.

<a href="../src/github_runner_image_builder/errors.py#L80"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>class</kbd> `HomeDirectoryChangeOwnershipError`
Represents an error while changing ubuntu home directory.





---

<a href="../src/github_runner_image_builder/errors.py#L84"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>class</kbd> `OpenstackBaseError`
Represents an error while interacting with Openstack.

Expand All @@ -218,7 +229,7 @@ Represents an error while interacting with Openstack.

---

<a href="../src/github_runner_image_builder/errors.py#L84"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/github_runner_image_builder/errors.py#L88"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>class</kbd> `UnauthorizedError`
Represents an unauthorized connection to Openstack.
Expand All @@ -229,7 +240,7 @@ Represents an unauthorized connection to Openstack.

---

<a href="../src/github_runner_image_builder/errors.py#L88"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/github_runner_image_builder/errors.py#L92"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>class</kbd> `UploadImageError`
Represents an error when uploading image to Openstack.
Expand All @@ -240,7 +251,7 @@ Represents an error when uploading image to Openstack.

---

<a href="../src/github_runner_image_builder/errors.py#L92"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/github_runner_image_builder/errors.py#L96"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>class</kbd> `OpenstackError`
Represents an error while communicating with Openstack.
Expand All @@ -251,7 +262,7 @@ Represents an error while communicating with Openstack.

---

<a href="../src/github_runner_image_builder/errors.py#L96"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/github_runner_image_builder/errors.py#L100"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>class</kbd> `CloudsYAMLError`
Represents an error with clouds.yaml for OpenStack connection.
Expand All @@ -262,7 +273,7 @@ Represents an error with clouds.yaml for OpenStack connection.

---

<a href="../src/github_runner_image_builder/errors.py#L100"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/github_runner_image_builder/errors.py#L104"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>class</kbd> `NotFoundError`
Represents an error with not matching OpenStack object found.
Expand All @@ -273,7 +284,7 @@ Represents an error with not matching OpenStack object found.

---

<a href="../src/github_runner_image_builder/errors.py#L104"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/github_runner_image_builder/errors.py#L108"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>class</kbd> `FlavorNotFoundError`
Represents an error with given OpenStack flavor not found.
Expand All @@ -284,7 +295,7 @@ Represents an error with given OpenStack flavor not found.

---

<a href="../src/github_runner_image_builder/errors.py#L108"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/github_runner_image_builder/errors.py#L112"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>class</kbd> `FlavorRequirementsNotMetError`
Represents an error with given OpenStack flavor not meeting the minimum requirements.
Expand All @@ -295,7 +306,7 @@ Represents an error with given OpenStack flavor not meeting the minimum requirem

---

<a href="../src/github_runner_image_builder/errors.py#L112"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/github_runner_image_builder/errors.py#L116"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>class</kbd> `NetworkNotFoundError`
Represents an error with given OpenStack network not found.
Expand All @@ -306,7 +317,7 @@ Represents an error with given OpenStack network not found.

---

<a href="../src/github_runner_image_builder/errors.py#L116"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/github_runner_image_builder/errors.py#L120"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>class</kbd> `AddressNotFoundError`
Represents an error with OpenStack instance not receiving an IP address.
Expand All @@ -317,7 +328,7 @@ Represents an error with OpenStack instance not receiving an IP address.

---

<a href="../src/github_runner_image_builder/errors.py#L120"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/github_runner_image_builder/errors.py#L124"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>class</kbd> `CloudInitFailError`
Represents an error with cloud-init.
Expand Down
38 changes: 38 additions & 0 deletions src/github_runner_image_builder/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from github_runner_image_builder.errors import (
BuildImageError,
DependencyInstallError,
HomeDirectoryChangeOwnershipError,
ImageCompressError,
ImageConnectError,
ImageResizeError,
Expand Down Expand Up @@ -88,6 +89,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:
Expand Down Expand Up @@ -199,6 +201,8 @@ def run(
_install_apt_packages(base_image=image_config.base)
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.")
Expand All @@ -207,6 +211,8 @@ def run(
_install_yarn()
logger.info("Installing GitHub runner.")
_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:
logger.exception("Error chrooting into %s", IMAGE_MOUNT_DIR)
raise BuildImageError from exc
Expand Down Expand Up @@ -564,6 +570,13 @@ def _disable_unattended_upgrades() -> None:
raise UnattendedUpgradeDisableError from exc


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\n")
yanksyoon marked this conversation as resolved.
Show resolved Hide resolved
sysctl_file.write("net.ipv4.tcp_congestion_control=bbr\n")


def _configure_system_users() -> None:
"""Configure system users.

Expand Down Expand Up @@ -745,6 +758,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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use a constant here for the user?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do it on the next PR! Thanks :D

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:
Expand Down
2 changes: 1 addition & 1 deletion src/github_runner_image_builder/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions src/github_runner_image_builder/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,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."""

Expand Down
15 changes: 14 additions & 1 deletion src/github_runner_image_builder/templates/cloud-init.sh.j2
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ function disable_unattended_upgrades() {
/usr/bin/apt-get remove -y unattended-upgrades
}

function enable_network_fair_queuing_congestion() {
/usr/bin/cat <<EOF | /usr/bin/sudo /usr/bin/tee -a /etc/sysctl.conf
net.core.default_qdisc=fq
net.ipv4.tcp_congestion_control=bbr
EOF
/usr/sbin/sysctl -p
}

function configure_system_users() {
echo "Configuring ubuntu user"
# only add ubuntu user if ubuntu does not exist
Expand Down Expand Up @@ -97,11 +105,14 @@ function install_github_runner() {
/usr/bin/wget "https://github.com/actions/runner/releases/download/v$version/actions-runner-linux-$arch-$version.tar.gz"
/usr/bin/mkdir -p /home/ubuntu/actions-runner
/usr/bin/tar -xvzf "actions-runner-linux-$arch-$version.tar.gz" --directory /home/ubuntu/actions-runner
/usr/bin/chown --recursive ubuntu:ubuntu /home/ubuntu/actions-runner

rm "actions-runner-linux-$arch-$version.tar.gz"
}

function chown_home() {
/usr/bin/chown --recursive ubuntu:ubuntu /home/ubuntu/
}

proxy="{{ PROXY_URL }}"
apt_packages="{{ APT_PACKAGES }}"
hwe_version="{{ HWE_VERSION }}"
Expand All @@ -111,13 +122,15 @@ github_runner_arch="{{ RUNNER_ARCH }}"
configure_proxy "$proxy"
install_apt_packages "$apt_packages" "$hwe_version"
disable_unattended_upgrades
enable_network_fair_queuing_congestion
configure_system_users
configure_usr_local_bin
install_yarn
# 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
# Make sure the disk is synced for snapshot
sync
echo "Finished sync"
2 changes: 1 addition & 1 deletion src/github_runner_image_builder/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@


# This decorator has default arguments, one extra argument is not a problem.
def retry( # pylint: disable=too-many-arguments
def retry( # pylint: disable=too-many-arguments, too-many-positional-arguments
exception: Type[Exception] = Exception,
tries: int = 1,
delay: float = 0,
Expand Down
8 changes: 8 additions & 0 deletions tests/integration/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,12 @@ class Commands:
name="test sctp support", command="sudo apt-get install lksctp-tools -yq && checksctp"
),
Commands(name="test HWE kernel", command="uname -a | grep generic"),
Commands(
name="test network congestion policy(fq)",
command="sudo sysctl -a | grep 'net.core.default_qdisc = fq'",
),
Commands(
name="test network congestion policy",
command="sudo sysctl -a | grep 'net.ipv4.tcp_congestion_control = bbr'",
),
)
47 changes: 32 additions & 15 deletions tests/integration/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"""Helper utilities for integration tests."""

import collections
import dataclasses
import inspect
import logging
import platform
Expand Down Expand Up @@ -229,23 +230,33 @@ def _instance_running(instance: Instance) -> 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,
) -> SSHConnection:
"""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.
Expand All @@ -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'")
Expand Down
Loading
Loading