From c98445d19317cde956d628bed95aadef91fc275e Mon Sep 17 00:00:00 2001 From: Sudeep Pillai Date: Wed, 25 Oct 2023 10:45:15 -0700 Subject: [PATCH] Simplified docker calls to `subprocess.run` - revert default logging level to ERROR --- agipack/builder.py | 44 ++++-------------------- examples/generated/Dockerfile-base-cpu | 2 +- examples/generated/Dockerfile-base-cu118 | 2 +- examples/generated/Dockerfile-builder | 2 +- examples/generated/Dockerfile-multistage | 4 +-- tests/test_builder.py | 18 ++-------- 6 files changed, 14 insertions(+), 58 deletions(-) diff --git a/agipack/builder.py b/agipack/builder.py index ac0c2ba..6bd5c59 100644 --- a/agipack/builder.py +++ b/agipack/builder.py @@ -7,13 +7,12 @@ from jinja2 import Environment, FileSystemLoader from pydantic.dataclasses import dataclass -from rich import print from agipack.config import AGIPackConfig, ImageConfig from agipack.constants import AGIPACK_DOCKERFILE_TEMPLATE, AGIPACK_ENV, AGIPACK_TEMPLATE_DIR from agipack.version import __version__ -logging_level = os.environ.get("AGIPACK_LOGGING_LEVEL", "DEBUG") +logging_level = os.environ.get("AGIPACK_LOGGING_LEVEL", "ERROR") logging.basicConfig(level=logging.getLevelName(logging_level)) logger = logging.getLogger(__name__) @@ -177,25 +176,15 @@ def build(self, filename: str, target: str, tags: List[str] = None, push: bool = logger.debug(f"Image tags: {image_tags}") # Build the Docker image (using buildkit) - cmd = ["docker", "build", "-f", filename, "--target", target] + cmd = f"docker build -f {filename} --target {target}" for tag in image_tags: - cmd.extend(["-t", tag]) - cmd.append(".") + cmd += f" -t {tag}" + cmd += " ." logger.debug(f"Running command: {cmd}") env = os.environ.copy() env.update({"DOCKER_BUILDKIT": "1"}) - process = subprocess.Popen( - cmd, - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - universal_newlines=True, - env=env, - ) - for line in iter(process.stdout.readline, ""): - print(line, end="") - process.wait() + process = subprocess.run(cmd, env=env, shell=True) if process.returncode != 0: err_msg = f"Failed to build image [target={target}, e={process.stderr}]" @@ -215,16 +204,7 @@ def lint(self, filename: str) -> bool: cmd = "docker pull hadolint/hadolint && " cmd += f"docker run --pull=always --rm -i hadolint/hadolint < {filename}" logger.info("Linting with hadolint") - process = subprocess.Popen( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - universal_newlines=True, - shell=True, - ) - process.wait() - for line in iter(process.stdout.readline, ""): - print(line, end="") + process = subprocess.run(cmd, shell=True) return process.returncode == 0 def push(self, tags: List[str]) -> None: @@ -239,16 +219,6 @@ def push(self, tags: List[str]) -> None: for tag in tags: cmd = f"docker push {tag}" logger.debug(f"Running command: {cmd}") - process = subprocess.Popen( - cmd, - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - universal_newlines=True, - shell=True, - ) - for line in iter(process.stdout.readline, ""): - print(line, end="") - process.wait() + process = subprocess.run(cmd, shell=True) if process.returncode != 0: raise Exception(f"Failed to push image [tag={tag}]") diff --git a/examples/generated/Dockerfile-base-cpu b/examples/generated/Dockerfile-base-cpu index 995efed..7aa360c 100644 --- a/examples/generated/Dockerfile-base-cpu +++ b/examples/generated/Dockerfile-base-cpu @@ -1,5 +1,5 @@ # >>>>>>>>>>>>>>>>>>>>>>>>>>> -# Auto-generated by agi-pack (version=0.1.18). +# Auto-generated by agi-pack (version=0.1.19). FROM debian:buster-slim AS base-cpu # Setup environment variables diff --git a/examples/generated/Dockerfile-base-cu118 b/examples/generated/Dockerfile-base-cu118 index 84c947f..b6495f7 100644 --- a/examples/generated/Dockerfile-base-cu118 +++ b/examples/generated/Dockerfile-base-cu118 @@ -1,5 +1,5 @@ # >>>>>>>>>>>>>>>>>>>>>>>>>>> -# Auto-generated by agi-pack (version=0.1.18). +# Auto-generated by agi-pack (version=0.1.19). FROM nvidia/cuda:11.8.0-base-ubuntu22.04 AS base-gpu # Setup environment variables diff --git a/examples/generated/Dockerfile-builder b/examples/generated/Dockerfile-builder index 778cc73..5dbf080 100644 --- a/examples/generated/Dockerfile-builder +++ b/examples/generated/Dockerfile-builder @@ -1,5 +1,5 @@ # >>>>>>>>>>>>>>>>>>>>>>>>>>> -# Auto-generated by agi-pack (version=0.1.18). +# Auto-generated by agi-pack (version=0.1.19). FROM debian:buster-slim AS agipack-builder # Setup environment variables diff --git a/examples/generated/Dockerfile-multistage b/examples/generated/Dockerfile-multistage index 911c5cb..9ba8790 100644 --- a/examples/generated/Dockerfile-multistage +++ b/examples/generated/Dockerfile-multistage @@ -1,5 +1,5 @@ # >>>>>>>>>>>>>>>>>>>>>>>>>>> -# Auto-generated by agi-pack (version=0.1.18). +# Auto-generated by agi-pack (version=0.1.19). FROM debian:buster-slim AS base-cpu # Setup environment variables @@ -80,7 +80,7 @@ RUN apt-get -y autoclean \ && rm -rf /tmp/reqs \ && echo "pip cleanup complete" # >>>>>>>>>>>>>>>>>>>>>>>>>>> -# Auto-generated by agi-pack (version=0.1.18). +# Auto-generated by agi-pack (version=0.1.19). FROM base-cpu AS dev-cpu # Install additional system packages diff --git a/tests/test_builder.py b/tests/test_builder.py index 639f345..53380bb 100644 --- a/tests/test_builder.py +++ b/tests/test_builder.py @@ -1,6 +1,5 @@ import logging import os -import subprocess import tempfile from pathlib import Path @@ -32,21 +31,8 @@ def test_build_all(builder): dockerfiles = builder.render() assert "base-cpu" in dockerfiles assert dockerfiles["base-cpu"] == "Dockerfile" - assert Path(dockerfiles["base-cpu"]).exists() - - # Use hadolint within docker to lint/check the generated Dockerfile - cmd = "docker pull hadolint/hadolint && " - cmd += f"docker run --pull=always --rm -i hadolint/hadolint < {dockerfiles['base-cpu']}" - logger.info("Linting with hadolint") - process = subprocess.Popen( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - universal_newlines=True, - shell=True, - ) - for line in iter(process.stdout.readline, ""): - logger.info(line.rstrip()) + path = Path(dockerfiles["base-cpu"]) + assert path.exists() def test_builder_cls(test_data_dir):