Skip to content
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
9 changes: 8 additions & 1 deletion projects/container_bench/testing/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,13 @@ prepare:
- glibc-static
- golang

container_images:
pull_images: true
dir: "{@remote_host.base_work_dir}/images"
images:
- "quay.io/podman/hello:latest"
- "registry.fedoraproject.org/fedora:latest"

podman:
# Custom binary or repo version must be disabled for matbenchmarking
custom_binary:
Expand Down Expand Up @@ -149,6 +156,7 @@ cleanup:
podman: true
exec_time: true
venv: true
container_images: false

podman_machine:
delete: true
Expand Down Expand Up @@ -201,7 +209,6 @@ test:
# - custom
runtime: # Linux only
- crun
- krun
- runc
capture_metrics:
enabled: true
Expand Down
17 changes: 17 additions & 0 deletions projects/container_bench/testing/config_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,17 @@ class ConfigKeys:
PREPARE_DNF_INSTALL_DEPENDENCIES = "prepare.dnf.install_dependencies"
PREPARE_DNF_ENABLE_DOCKER_REPO = "prepare.dnf.enable_docker_repo"
PREPARE_DNF_DEPENDENCIES = "prepare.dnf.dependencies"
PREPARE_CONTAINER_IMAGES_PULL_IMAGES = "prepare.container_images.pull_images"
PREPARE_CONTAINER_IMAGES_IMAGES = "prepare.container_images.images"
PREPARE_CONTAINER_IMAGES_IMAGES_DIR = "prepare.container_images.dir"

# Additional cleanup configuration
CLEANUP_FILES_EXEC_TIME = "cleanup.files.exec_time"
CLEANUP_FILES_VENV = "cleanup.files.venv"
CLEANUP_PODMAN_MACHINE_DELETE = "cleanup.podman_machine.delete"
CLEANUP_DOCKER_SERVICE_STOP = "cleanup.docker_service.stop"
CLEANUP_DOCKER_DESKTOP_STOP = "cleanup.docker_desktop.stop"
CLEANUP_CONTAINER_IMAGES = "cleanup.files.container_images"

# Additional remote host configuration
REMOTE_HOST_DOCKER_ENABLED = "remote_host.docker.enabled"
Expand Down Expand Up @@ -193,6 +197,17 @@ def get_dnf_config():
'dependencies': config.project.get_config(ConfigKeys.PREPARE_DNF_DEPENDENCIES, print=False),
}

@staticmethod
def get_container_images_config():
return {
'pull_images': config.project.get_config(
ConfigKeys.PREPARE_CONTAINER_IMAGES_PULL_IMAGES, print=False),
'images': config.project.get_config(
ConfigKeys.PREPARE_CONTAINER_IMAGES_IMAGES, print=False),
'dir': config.project.get_config(
ConfigKeys.PREPARE_CONTAINER_IMAGES_IMAGES_DIR, print=False),
}

@staticmethod
def get_extended_cleanup_config():
return {
Expand All @@ -205,6 +220,8 @@ def get_extended_cleanup_config():
ConfigKeys.CLEANUP_DOCKER_SERVICE_STOP, print=False),
'docker_desktop_stop': config.project.get_config(
ConfigKeys.CLEANUP_DOCKER_DESKTOP_STOP, print=False),
'container_images': config.project.get_config(
ConfigKeys.CLEANUP_CONTAINER_IMAGES, print=False),
}

@staticmethod
Expand Down
31 changes: 29 additions & 2 deletions projects/container_bench/testing/container_engine.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from pathlib import Path
import remote_access
import json
import logging
Expand Down Expand Up @@ -75,6 +76,32 @@ def get_command(self):

return cmd

def store_container_images_as_tar(self):
container_images_config = ConfigManager.get_container_images_config()
pull_images = container_images_config['pull_images']
images = container_images_config['images']
images_dir = container_images_config['dir']
dest = Path(images_dir)

if not pull_images:
logging.info("Skipping pulling container images as per configuration.")
return

if not remote_access.exists(dest):
logging.info(f"Creating images directory at {dest} ...")
remote_access.create_remote_directory(dest)

for image in images:
logging.info(f"Pulling container image: {image} ...")
image_filename = image.replace("/", "_").replace(":", "_").replace(".", "_") + ".tar"
if remote_access.exists(dest / image_filename):
continue
cmd = f"{self.get_command()} pull {image}"
remote_access.run_with_ansible_ssh_conf(self.base_work_dir, cmd)
cmd = f"{self.get_command()} save -o {dest / image_filename} {image}"
remote_access.run_with_ansible_ssh_conf(self.base_work_dir, cmd)
self.rm_images(images)

def is_rootful(self):
if ConfigManager.is_linux():
return self.podman_config['linux_rootful']
Expand All @@ -100,10 +127,10 @@ def cleanup(self):

return ret.returncode == 0

def rm_image(self, image):
def rm_images(self, images):
ret = remote_access.run_with_ansible_ssh_conf(
self.base_work_dir,
f"{self.get_command()} image rm {image}",
f"{self.get_command()} image rm {' '.join(images)}",
check=False,
capture_stdout=True,
capture_stderr=True,
Expand Down
114 changes: 110 additions & 4 deletions projects/container_bench/testing/platform_builders.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from abc import ABC, abstractmethod
from config_manager import ConfigManager
import shlex


class PlatformCommandBuilder(ABC):
Expand All @@ -11,6 +12,38 @@ def build_env_command(self, env_dict):
def build_service_start_script(self, service_name, start_command, binary_path):
pass

@abstractmethod
def build_chdir_command(self, chdir):
pass

@abstractmethod
def build_rm_command(self, file_path, recursive=False):
pass

@abstractmethod
def build_mkdir_command(self, path):
pass

@abstractmethod
def build_exists_command(self, path):
pass

@abstractmethod
def get_shell_command(self):
pass

@abstractmethod
def build_entrypoint_script(self, env_cmd, chdir_cmd, cmd, verbose):
pass

@abstractmethod
def check_exists_result(self, ret):
pass


def escape_powershell_single_quote(value):
return str(value).replace("'", "''")


class WindowsCommandBuilder(PlatformCommandBuilder):
def build_env_command(self, env_dict):
Expand All @@ -21,8 +54,7 @@ def build_env_command(self, env_dict):
for k, v in env_dict.items():
if v is None or v == "":
continue
env_commands.append(f"$env:{k}='{v}'")

env_commands.append(f"$env:{escape_powershell_single_quote(k)}='{escape_powershell_single_quote(v)}'")
return "; ".join(env_commands) + ";"

def build_service_start_script(self, service_name, start_command, binary_path):
Expand Down Expand Up @@ -66,18 +98,92 @@ def build_service_start_script(self, service_name, start_command, binary_path):
Remove-Item "$env:USERPROFILE\\start_{service_name}.ps1" -Force -ErrorAction SilentlyContinue
"""

def build_chdir_command(self, chdir):
if chdir is None:
return "Set-Location $env:USERPROFILE"
return f"Set-Location '{escape_powershell_single_quote(chdir)}'"

def build_rm_command(self, file_path, recursive=False):
flags = "-Force -ErrorAction SilentlyContinue"
if recursive:
flags += " -Recurse"
return f"Remove-Item '{escape_powershell_single_quote(str(file_path))}' {flags}"

def build_mkdir_command(self, path):
return f"New-Item -ItemType Directory -Path '{escape_powershell_single_quote(str(path))}' -Force"

def build_exists_command(self, path):
return f"Test-Path '{escape_powershell_single_quote(str(path))}'"

def get_shell_command(self):
return "powershell.exe -Command -"

def build_entrypoint_script(self, env_cmd, chdir_cmd, cmd, verbose):
env_section = f"{env_cmd}\n" if env_cmd else ""
script = f"""
$ErrorActionPreference = "Stop"

{env_section}{chdir_cmd}

{cmd}
"""
if verbose:
script = f"$VerbosePreference = 'Continue'\n{script}"
return script

def check_exists_result(self, ret):
return ret.stdout and ret.stdout.strip().lower() == "true"


class UnixCommandBuilder(PlatformCommandBuilder):
def build_env_command(self, env_dict):
if not env_dict:
return ""

env_values = " ".join(f"'{k}={v}'" for k, v in env_dict.items() if v is not None and v != "")
return f"env {env_values}"
env_values = " ".join(f"{k}={shlex.quote(str(v))}" for k, v in env_dict.items() if v is not None and v != "")
return f"export {env_values}\n"

def build_service_start_script(self, service_name, start_command, binary_path) -> str:
return start_command

def build_chdir_command(self, chdir):
if chdir is None:
return "cd $HOME"
return f"cd '{shlex.quote(str(chdir))}'"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove redundant single quotes around shlex.quote.

shlex.quote() already adds quotes when necessary. Wrapping the result in additional single quotes creates confusing double-quoting that, while it may work in simple cases, produces hard-to-read commands and can behave unexpectedly with complex paths. Other Unix commands in this file (lines 156, 159, 162) correctly use shlex.quote() without extra quotes.

-        return f"cd '{shlex.quote(str(chdir))}'"
+        return f"cd {shlex.quote(str(chdir))}"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return f"cd '{shlex.quote(str(chdir))}'"
return f"cd {shlex.quote(str(chdir))}"
🤖 Prompt for AI Agents
In projects/container_bench/testing/platform_builders.py around line 152, the
return value wraps shlex.quote(str(chdir)) in extra single quotes causing
redundant/double quoting; remove the outer single quotes so the function returns
cd followed by the shlex.quote(...) result (matching the style used on lines
156, 159, 162) to produce correct, consistent shell-safe paths.


def build_rm_command(self, file_path, recursive=False):
flag = "-rf" if recursive else "-f"
return f"rm {flag} {shlex.quote(str(file_path))}"

def build_mkdir_command(self, path):
return f"mkdir -p {shlex.quote(str(path))}"

def build_exists_command(self, path):
return f"test -e {shlex.quote(str(path))}"

def get_shell_command(self):
return "bash"

def build_entrypoint_script(self, env_cmd, chdir_cmd, cmd, verbose):
script = f"""
set -o pipefail
set -o errexit
set -o nounset
set -o errtrace

{env_cmd}

{chdir_cmd}

{cmd}
"""
if verbose:
script = f"set -x\n{script}"
return script

def check_exists_result(self, ret):
return ret.returncode == 0


class PlatformFactory:
@staticmethod
Expand Down
24 changes: 8 additions & 16 deletions projects/container_bench/testing/prepare.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import logging
import shlex

from container_engine import PodmanMachine, ContainerEngine, DockerDesktopMachine
from config_manager import ConfigManager
Expand All @@ -8,19 +7,6 @@
import utils


def remove_remote_file(base_work_dir, file_path, recursive=False):
if ConfigManager.is_windows():
flags = "-Force -ErrorAction SilentlyContinue"
if recursive:
flags += " -Recurse"
cmd = f"Remove-Item '{file_path}' {flags}"
else:
flag = "-rf" if recursive else "-f"
cmd = f"rm {flag} {shlex.quote(str(file_path))}"

remote_access.run_with_ansible_ssh_conf(base_work_dir, cmd)


def cleanup():
cleanup_config = ConfigManager.get_extended_cleanup_config()

Expand All @@ -30,15 +16,15 @@ def cleanup():
exec_time_script = utils.get_benchmark_script_path(base_work_dir)
if remote_access.exists(exec_time_script):
logging.info(f"Removing {exec_time_script} ...")
remove_remote_file(base_work_dir, exec_time_script)
remote_access.remove_remote_file(base_work_dir, exec_time_script)

if cleanup_config['files_venv']:
logging.info("Cleaning up virtual environment")
base_work_dir = remote_access.prepare()
venv_path = utils.get_benchmark_script_path(base_work_dir).parent / ".venv"
if remote_access.exists(venv_path):
logging.info(f"Removing {venv_path} ...")
remove_remote_file(base_work_dir, venv_path, recursive=True)
remote_access.remove_remote_file(base_work_dir, venv_path, recursive=True)

try:
cleanup_podman_platform()
Expand All @@ -49,6 +35,10 @@ def cleanup():
logging.info("Cleaning up Podman files")
utils.cleanup_podman_files(remote_access.prepare())

if cleanup_config['container_images']:
logging.info("Cleaning up container images")
utils.cleanup_container_images(remote_access.prepare())

cleanup_docker_platform()
return 0

Expand Down Expand Up @@ -119,6 +109,7 @@ def prepare_docker_platform():
docker_desktop.start()

docker = ContainerEngine("docker")
docker.store_container_images_as_tar()
docker.cleanup()


Expand Down Expand Up @@ -162,6 +153,7 @@ def prepare_podman_platform():

logging.info("cleaning up podman")
podman = ContainerEngine("podman")
podman.store_container_images_as_tar()
podman.cleanup()

return 0
Loading
Loading