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" },
]
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/errors.md b/src-docs/errors.md
index bf51e74..8e18422 100644
--- a/src-docs/errors.md
+++ b/src-docs/errors.md
@@ -209,6 +209,17 @@ 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 +229,7 @@ Represents an error while interacting with Openstack.
---
-
+
## class `UnauthorizedError`
Represents an unauthorized connection to Openstack.
@@ -229,7 +240,7 @@ Represents an unauthorized connection to Openstack.
---
-
+
## class `UploadImageError`
Represents an error when uploading image to Openstack.
@@ -240,7 +251,7 @@ Represents an error when uploading image to Openstack.
---
-
+
## class `OpenstackError`
Represents an error while communicating with Openstack.
@@ -251,7 +262,7 @@ Represents an error while communicating with Openstack.
---
-
+
## class `CloudsYAMLError`
Represents an error with clouds.yaml for OpenStack connection.
@@ -262,7 +273,7 @@ Represents an error with clouds.yaml for OpenStack connection.
---
-
+
## class `NotFoundError`
Represents an error with not matching OpenStack object found.
@@ -273,7 +284,7 @@ Represents an error with not matching OpenStack object found.
---
-
+
## class `FlavorNotFoundError`
Represents an error with given OpenStack flavor not found.
@@ -284,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.
@@ -295,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.
@@ -306,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.
@@ -317,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 41e0533..4a03f36 100644
--- a/src/github_runner_image_builder/builder.py
+++ b/src/github_runner_image_builder/builder.py
@@ -28,6 +28,7 @@
from github_runner_image_builder.errors import (
BuildImageError,
DependencyInstallError,
+ HomeDirectoryChangeOwnershipError,
ImageCompressError,
ImageConnectError,
ImageResizeError,
@@ -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:
@@ -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.")
@@ -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
@@ -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")
+ sysctl_file.write("net.ipv4.tcp_congestion_control=bbr\n")
+
+
def _configure_system_users() -> None:
"""Configure system users.
@@ -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
+ 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/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/errors.py b/src/github_runner_image_builder/errors.py
index 6a6db63..2acf633 100644
--- a/src/github_runner_image_builder/errors.py
+++ b/src/github_runner_image_builder/errors.py
@@ -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."""
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 35e6b7d..2d19c2e 100644
--- a/src/github_runner_image_builder/templates/cloud-init.sh.j2
+++ b/src/github_runner_image_builder/templates/cloud-init.sh.j2
@@ -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 < 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 0027b69..51bac1e 100644
--- a/tests/unit/test_builder.py
+++ b/tests/unit/test_builder.py
@@ -18,6 +18,7 @@
BuildImageError,
ChrootBaseError,
DependencyInstallError,
+ HomeDirectoryChangeOwnershipError,
ImageCompressError,
ImageConnectError,
ImageResizeError,
@@ -248,9 +249,11 @@ def test_run_error(
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(patch_obj, sub_func, mock)
@@ -278,9 +281,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(
@@ -484,6 +489,21 @@ def test__disable_unattended_upgrades_subprocess_fail(monkeypatch: pytest.Monkey
assert "Failed to disable unattended upgrades" in str(exc.getrepr())
+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(builder, "SYSCTL_CONF_PATH", test_path := tmp_path / "sysctl.conf")
+
+ builder._enable_network_fair_queuing_congestion()
+
+ 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.
@@ -654,6 +674,38 @@ def test__get_github_runner_version_user_defined(version: str, expected_version:
assert builder._get_github_runner_version(version=version) == expected_version
+@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__chown_home_fail(
+ monkeypatch: pytest.MonkeyPatch,
+ error: subprocess.CalledProcessError | subprocess.SubprocessError,
+):
+ """
+ arrange: given a monkeypatched process calls that fails.
+ act: when _disconnect_image_to_network_block_device is called.
+ assert: ImageMountError is raised.
+ """
+ monkeypatch.setattr(
+ subprocess,
+ "check_call",
+ MagicMock(side_effect=error),
+ )
+
+ with pytest.raises(HomeDirectoryChangeOwnershipError):
+ builder._chown_home()
+
+
def test__disconnect_image_to_network_block_device_fail(monkeypatch: pytest.MonkeyPatch):
"""
arrange: given a monkeypatched process calls that fails.
diff --git a/tests/unit/test_openstack_builder.py b/tests/unit/test_openstack_builder.py
index 91bcbc5..c651d95 100644
--- a/tests/unit/test_openstack_builder.py
+++ b/tests/unit/test_openstack_builder.py
@@ -526,6 +526,14 @@ def test__generate_cloud_init_script():
/usr/bin/apt-get remove -y unattended-upgrades
}
+function enable_network_fair_queuing_congestion() {
+ /usr/bin/cat <