Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollback python_on_whales conversion #171

Merged
merged 2 commits into from
Nov 5, 2024
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
73 changes: 20 additions & 53 deletions buildrunner/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import types
from typing import List, Optional

import python_on_whales
import requests

from retry import retry
Expand All @@ -39,7 +38,7 @@
)
from buildrunner.steprunner import BuildStepRunner
from buildrunner.docker.multiplatform_image_builder import MultiplatformImageBuilder
import buildrunner.docker.builder as legacy_builder
import buildrunner.docker.builder


LOGGER = logging.getLogger(__name__)
Expand Down Expand Up @@ -288,37 +287,15 @@ def get_source_image(self):
source_archive_path: "source.tar",
SOURCE_DOCKERFILE: "Dockerfile",
}
if self.buildrunner_config.run_config.use_legacy_builder:
image = legacy_builder.build_image(
temp_dir=self.buildrunner_config.global_config.temp_dir,
inject=inject,
timeout=self.docker_timeout,
docker_registry=self.buildrunner_config.global_config.docker_registry,
nocache=True,
pull=False,
)
self._source_image = image
else:
# Use buildx builder
native_platform = self._step_runner.multi_platform.get_native_platform()
LOGGER.info(f"Setting platforms to [{native_platform}]")
platforms = [native_platform]
built_images_info = (
self._step_runner.multi_platform.build_multiple_images(
platforms=platforms,
inject=inject,
cache=False,
pull=False,
build_args={
"BUILDRUNNER_DISTRO": os.environ.get("BUILDRUNNER_DISTRO")
},
)
)
if len(built_images_info.built_images) != 1:
raise BuildRunnerProcessingError(
"Failed to build source image. Retrying the build may resolve the issue."
)
self._source_image = built_images_info.built_images[0].trunc_digest
image = buildrunner.docker.builder.build_image(
temp_dir=self.buildrunner_config.global_config.temp_dir,
inject=inject,
timeout=self.docker_timeout,
docker_registry=self.buildrunner_config.global_config.docker_registry,
nocache=True,
pull=False,
)
self._source_image = image

return self._source_image

Expand Down Expand Up @@ -519,16 +496,11 @@ 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:
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
)
_docker_client.remove_image(
self._source_image,
noprune=False,
force=True,
)
except ImageNotFound:
self.log.warning(
f"Failed to remove source image {self._source_image}\n"
Expand All @@ -540,16 +512,11 @@ 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:
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
)
_docker_client.remove_image(
_image,
noprune=False,
force=True,
)
except Exception as _ex: # pylint: disable=broad-except
self.log.write(f"Error removing image {_image}: {str(_ex)}\n")
else:
Expand Down
17 changes: 6 additions & 11 deletions buildrunner/docker/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from typing import Tuple
import urllib.parse
import docker
import python_on_whales

from buildrunner.errors import BuildRunnerError, BuildRunnerConfigurationError

Expand Down Expand Up @@ -96,21 +95,17 @@ def new_client(
)


def force_remove_container(docker_client, container, use_legacy_builder: bool):
def force_remove_container(docker_client, container):
"""
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
"""
if use_legacy_builder:
docker_client.remove_container(
container,
force=True,
v=True,
)
else:
python_on_whales.docker.remove(container, force=True, volumes=True)
docker_client.remove_container(
container,
force=True,
v=True,
)


def get_dockerfile(dockerfile: str, temp_dir: str = None) -> Tuple[str, bool]:
Expand Down
4 changes: 1 addition & 3 deletions buildrunner/docker/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,7 @@ def cleanup(self):
# iterate through and destroy intermediate containers
for container in self.intermediate_containers:
try:
force_remove_container(
self.docker_client, container, use_legacy_builder=True
)
force_remove_container(self.docker_client, container)
except docker.errors.APIError as err:
logger.debug(
f"Error removing intermediate container {container}: {err}"
Expand Down
17 changes: 5 additions & 12 deletions buildrunner/docker/daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@

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 @@ -99,13 +97,8 @@ def stop(self):
f"Destroying Docker daemon container {self._daemon_container:.10}\n"
)
if self._daemon_container:
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
)
self.docker_client.remove_container(
self._daemon_container,
force=True,
v=True,
)
8 changes: 1 addition & 7 deletions buildrunner/docker/importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,11 @@
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 @@ -43,11 +41,7 @@ def import_image(self):
"""

try:
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)
import_return = self.docker_client.import_image(self.src)
except docker.errors.APIError as apie:
raise BuildRunnerProcessingError(
f"Error importing image from archive file {self.src}: {apie}"
Expand Down
59 changes: 16 additions & 43 deletions buildrunner/docker/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,9 @@
from docker.utils import compare_version
from retry import retry
import docker.errors
import python_on_whales
import six
import timeout_decorator

from buildrunner.config import BuildRunnerConfig
from buildrunner.docker import (
new_client,
force_remove_container,
Expand Down Expand Up @@ -133,10 +131,6 @@ def __init__(self, image_config, dockerd_url=None, log=None):
if log:
log.write("\nImage pulled successfully\n")

self.use_docker_py = (
BuildRunnerConfig.get_instance().run_config.use_legacy_builder
)

def start(
self,
shell="/bin/sh",
Expand Down Expand Up @@ -242,10 +236,7 @@ def start(

# start the container
self.container = self.docker_client.create_container(self.image_name, **kwargs)
if self.use_docker_py:
self.docker_client.start(self.container["Id"])
else:
python_on_whales.docker.start(self.container["Id"])
self.docker_client.start(self.container["Id"])

# run any supplied provisioners
if provisioners:
Expand All @@ -263,44 +254,31 @@ def stop(self):
Stop the backing Docker container.
"""
if self.container:
if self.use_docker_py:
self.docker_client.stop(
self.container["Id"],
timeout=0,
)
else:
python_on_whales.docker.stop(self.container["Id"], time=0)
self.docker_client.stop(
self.container["Id"],
timeout=0,
)

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, use_legacy_builder=use_docker_py
)
force_remove_container(self.docker_client, container)
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:
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
)
self.docker_client.remove_container(
container_id["Id"],
force=True,
v=True,
)
else:
print(
f'Unable to find docker container with name or label "{container}"'
Expand All @@ -309,16 +287,11 @@ def cleanup(self):
print(
f'Unable to find docker container with name or label "{container}"'
)
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.docker_client.remove_container(
self.container["Id"],
force=True,
v=True,
)

self.container = None

Expand Down
Loading
Loading