Skip to content

Commit

Permalink
Merge pull request #167 from shanejbrown/convert-to-python-on-whales-…
Browse files Browse the repository at this point in the history
…part2

Convert to python on whales part2
  • Loading branch information
shanejbrown authored Oct 9, 2024
2 parents dcea3f9 + 4eed3d8 commit b76ce0f
Show file tree
Hide file tree
Showing 10 changed files with 111 additions and 47 deletions.
31 changes: 21 additions & 10 deletions buildrunner/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import types
from typing import List, Optional

import python_on_whales
import requests

from retry import retry
Expand Down Expand Up @@ -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"
Expand All @@ -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:
Expand Down
5 changes: 4 additions & 1 deletion buildrunner/config/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"):
Expand Down Expand Up @@ -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")
Expand Down
18 changes: 12 additions & 6 deletions buildrunner/docker/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]:
Expand Down
4 changes: 3 additions & 1 deletion buildrunner/docker/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down
18 changes: 13 additions & 5 deletions buildrunner/docker/daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
)
8 changes: 7 additions & 1 deletion buildrunner/docker/importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}"
Expand Down
38 changes: 26 additions & 12 deletions buildrunner/docker/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,21 +276,31 @@ 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(
filters={"label": container}, quiet=True
)
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}"'
Expand All @@ -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

Expand Down
16 changes: 11 additions & 5 deletions buildrunner/sshagent/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
"""
Expand Down
17 changes: 12 additions & 5 deletions buildrunner/steprunner/tasks/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion tests/test_builders.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -139,7 +140,7 @@ def fixture_set_env():
),
(
"Default builder",
True,
DEFAULT_TO_LEGACY_BUILDER,
"""
steps:
build-container-single-platform:
Expand Down

0 comments on commit b76ce0f

Please sign in to comment.