From b2af2f7c76e67bdccf1517642c3ea197f5d630b6 Mon Sep 17 00:00:00 2001 From: Shane Brown Date: Thu, 3 Oct 2024 13:52:58 -0600 Subject: [PATCH 1/2] Replace remove_image and remove_container --- buildrunner/__init__.py | 31 +++++++++++++++-------- buildrunner/config/models.py | 5 +++- buildrunner/docker/__init__.py | 18 +++++++++----- buildrunner/docker/builder.py | 4 ++- buildrunner/docker/daemon.py | 18 ++++++++++---- buildrunner/docker/runner.py | 38 ++++++++++++++++++++--------- buildrunner/sshagent/__init__.py | 16 ++++++++---- buildrunner/steprunner/tasks/run.py | 17 +++++++++---- tests/test_builders.py | 3 ++- 9 files changed, 104 insertions(+), 46 deletions(-) diff --git a/buildrunner/__init__.py b/buildrunner/__init__.py index 2bda4d4b..f8a133c7 100644 --- a/buildrunner/__init__.py +++ b/buildrunner/__init__.py @@ -21,6 +21,7 @@ import types from typing import List, Optional +import python_on_whales import requests from retry import retry @@ -518,11 +519,16 @@ def run(self): # pylint: disable=too-many-statements,too-many-branches,too-many if self._source_image: self.log.write(f"Destroying source image {self._source_image}\n") try: - _docker_client.remove_image( - self._source_image, - noprune=False, - force=True, - ) + if self.buildrunner_config.run_config.use_legacy_builder: + _docker_client.remove_image( + self._source_image, + noprune=False, + force=True, + ) + else: + python_on_whales.docker.image.remove( + self._source_image, prune=True, force=True + ) except ImageNotFound: self.log.warning( f"Failed to remove source image {self._source_image}\n" @@ -534,11 +540,16 @@ def run(self): # pylint: disable=too-many-statements,too-many-branches,too-many # reverse the order of the images since child images would likely come after parent images for _image in self.generated_images[::-1]: try: - _docker_client.remove_image( - _image, - noprune=False, - force=True, - ) + if self.buildrunner_config.run_config.use_legacy_builder: + _docker_client.remove_image( + _image, + noprune=False, + force=True, + ) + else: + python_on_whales.docker.image.remove( + _image, prune=True, force=True + ) except Exception as _ex: # pylint: disable=broad-except self.log.write(f"Error removing image {_image}: {str(_ex)}\n") else: diff --git a/buildrunner/config/models.py b/buildrunner/config/models.py index 62ff3841..e2e1d466 100644 --- a/buildrunner/config/models.py +++ b/buildrunner/config/models.py @@ -25,6 +25,7 @@ DEFAULT_CACHES_ROOT = "~/.buildrunner/caches" # Marker for using the local registry instead of an upstream registry MP_LOCAL_REGISTRY = "local" +DEFAULT_TO_LEGACY_BUILDER = True class GithubModel(BaseModel, extra="forbid"): @@ -142,7 +143,9 @@ class Config(BaseModel, extra="forbid"): """Top level config model""" version: Optional[float] = None - use_legacy_builder: Optional[bool] = Field(alias="use-legacy-builder", default=True) + use_legacy_builder: Optional[bool] = Field( + alias="use-legacy-builder", default=DEFAULT_TO_LEGACY_BUILDER + ) steps: Dict[str, Step] @field_validator("steps") diff --git a/buildrunner/docker/__init__.py b/buildrunner/docker/__init__.py index d7267e6b..07a2e842 100644 --- a/buildrunner/docker/__init__.py +++ b/buildrunner/docker/__init__.py @@ -12,9 +12,11 @@ from typing import Tuple import urllib.parse import docker +import python_on_whales from buildrunner.errors import BuildRunnerError, BuildRunnerConfigurationError + try: # Newer API Client = docker.client.Client # pylint: disable=no-member @@ -94,17 +96,21 @@ def new_client( ) -def force_remove_container(docker_client, container): +def force_remove_container(docker_client, container, use_legacy_builder: bool): """ Force removes a container from the given docker client. :param docker_client: the docker client :param container: the container + :param use_legacy_builder: whether to use the legacy builder """ - docker_client.remove_container( - container, - force=True, - v=True, - ) + if use_legacy_builder: + docker_client.remove_container( + container, + force=True, + v=True, + ) + else: + python_on_whales.docker.remove(container, force=True, volumes=True) def get_dockerfile(dockerfile: str, temp_dir: str = None) -> Tuple[str, bool]: diff --git a/buildrunner/docker/builder.py b/buildrunner/docker/builder.py index 962346b9..05f6b278 100644 --- a/buildrunner/docker/builder.py +++ b/buildrunner/docker/builder.py @@ -277,7 +277,9 @@ def cleanup(self): # iterate through and destroy intermediate containers for container in self.intermediate_containers: try: - force_remove_container(self.docker_client, container) + force_remove_container( + self.docker_client, container, use_legacy_builder=True + ) except docker.errors.APIError as err: logger.debug( f"Error removing intermediate container {container}: {err}" diff --git a/buildrunner/docker/daemon.py b/buildrunner/docker/daemon.py index 21a6b7c6..1c5e52f7 100644 --- a/buildrunner/docker/daemon.py +++ b/buildrunner/docker/daemon.py @@ -8,6 +8,9 @@ import os +import python_on_whales + +from buildrunner.config import BuildRunnerConfig from buildrunner.docker import DOCKER_DEFAULT_DOCKERD_URL DAEMON_IMAGE_NAME = "busybox:latest" @@ -96,8 +99,13 @@ def stop(self): f"Destroying Docker daemon container {self._daemon_container:.10}\n" ) if self._daemon_container: - self.docker_client.remove_container( - self._daemon_container, - force=True, - v=True, - ) + if BuildRunnerConfig.get_instance().run_config.use_legacy_builder: + self.docker_client.remove_container( + self._daemon_container, + force=True, + v=True, + ) + else: + python_on_whales.docker.remove( + self._daemon_container, force=True, volumes=True + ) diff --git a/buildrunner/docker/runner.py b/buildrunner/docker/runner.py index bf402d4e..9ee367bb 100644 --- a/buildrunner/docker/runner.py +++ b/buildrunner/docker/runner.py @@ -276,9 +276,14 @@ def cleanup(self): Cleanup the backing Docker container, stopping it if necessary. """ if self.container: + use_docker_py = ( + BuildRunnerConfig.get_instance().run_config.use_legacy_builder + ) for container in self.containers: try: - force_remove_container(self.docker_client, container) + force_remove_container( + self.docker_client, container, use_legacy_builder=use_docker_py + ) except docker.errors.NotFound: try: container_ids = self.docker_client.containers( @@ -286,11 +291,16 @@ def cleanup(self): ) if container_ids: for container_id in container_ids: - self.docker_client.remove_container( - container_id["Id"], - force=True, - v=True, - ) + if use_docker_py: + self.docker_client.remove_container( + container_id["Id"], + force=True, + v=True, + ) + else: + python_on_whales.docker.remove( + container_id["Id"], force=True, volumes=True + ) else: print( f'Unable to find docker container with name or label "{container}"' @@ -299,12 +309,16 @@ def cleanup(self): print( f'Unable to find docker container with name or label "{container}"' ) - - self.docker_client.remove_container( - self.container["Id"], - force=True, - v=True, - ) + if use_docker_py: + self.docker_client.remove_container( + self.container["Id"], + force=True, + v=True, + ) + else: + python_on_whales.docker.remove( + self.container["Id"], force=True, volumes=True + ) self.container = None diff --git a/buildrunner/sshagent/__init__.py b/buildrunner/sshagent/__init__.py index 17a787b5..17248a2a 100644 --- a/buildrunner/sshagent/__init__.py +++ b/buildrunner/sshagent/__init__.py @@ -27,6 +27,7 @@ from paramiko.common import io_sleep from paramiko.util import asbytes from paramiko.message import Message +import python_on_whales import buildrunner.config import buildrunner.docker.builder as legacy_builder @@ -244,11 +245,16 @@ def stop(self): self.log.write( f"Destroying ssh-agent container {self._ssh_agent_container:.10}\n" ) - self.docker_client.remove_container( - self._ssh_agent_container, - force=True, - v=True, - ) + if buildrunner.config.BuildRunnerConfig.get_instance().run_config.use_legacy_builder: + self.docker_client.remove_container( + self._ssh_agent_container, + force=True, + v=True, + ) + else: + python_on_whales.docker.remove( + self._ssh_agent_container, force=True, volumes=True + ) def get_ssh_agent_image(self): """ diff --git a/buildrunner/steprunner/tasks/run.py b/buildrunner/steprunner/tasks/run.py index c677b85a..35ac22b0 100644 --- a/buildrunner/steprunner/tasks/run.py +++ b/buildrunner/steprunner/tasks/run.py @@ -1134,11 +1134,18 @@ def cleanup(self, context): # pylint: disable=unused-argument self.step_runner.log.write( f"Destroying source container {self._source_container:.10}\n" ) - self._docker_client.remove_container( - self._source_container, - force=True, - v=True, - ) + if BuildRunnerConfig.get_instance().run_config.use_legacy_builder: + self._docker_client.remove_container( + self._source_container, + force=True, + v=True, + ) + else: + python_on_whales.docker.container.remove( + self._source_container, + force=True, + volumes=True, + ) for image in self.images_to_remove: python_on_whales.docker.image.remove(image, force=True) diff --git a/tests/test_builders.py b/tests/test_builders.py index 605ce493..400d7dc4 100644 --- a/tests/test_builders.py +++ b/tests/test_builders.py @@ -6,6 +6,7 @@ import yaml +from buildrunner.config.models import DEFAULT_TO_LEGACY_BUILDER from buildrunner.docker.image_info import BuiltImageInfo, BuiltTaggedImage from tests import test_runner @@ -139,7 +140,7 @@ def fixture_set_env(): ), ( "Default builder", - True, + DEFAULT_TO_LEGACY_BUILDER, """ steps: build-container-single-platform: From 4eed3d87c62317b68dac84e3227b464cd22589a0 Mon Sep 17 00:00:00 2001 From: Shane Brown Date: Tue, 8 Oct 2024 14:15:49 -0600 Subject: [PATCH 2/2] Replace import_image --- buildrunner/docker/importer.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/buildrunner/docker/importer.py b/buildrunner/docker/importer.py index 074cfab9..0f727741 100644 --- a/buildrunner/docker/importer.py +++ b/buildrunner/docker/importer.py @@ -6,11 +6,13 @@ with the terms of the Adobe license agreement accompanying it. """ +import python_on_whales import yaml import docker import docker.errors +from buildrunner.config import BuildRunnerConfig from buildrunner.docker import new_client from buildrunner.errors import BuildRunnerProcessingError from buildrunner.utils import is_dict @@ -41,7 +43,11 @@ def import_image(self): """ try: - import_return = self.docker_client.import_image(self.src) + import_return = None + if BuildRunnerConfig.get_instance().run_config.use_legacy_builder: + import_return = self.docker_client.import_image(self.src) + else: + import_return = python_on_whales.docker.import_(self.src) except docker.errors.APIError as apie: raise BuildRunnerProcessingError( f"Error importing image from archive file {self.src}: {apie}"