Skip to content

Commit

Permalink
Merge pull request #153 from shanejbrown/retry-push
Browse files Browse the repository at this point in the history 
Refactor push retries
  • Loading branch information
@shanejbrown
shanejbrown authored Aug 21, 2024
2 parents bd5c10e + 71210af commit 9e31cd1
Show file tree
Hide file tree
Showing 4 changed files with 259 additions and 37 deletions.
62 changes: 27 additions & 35 deletions buildrunner/docker/multiplatform_image_builder.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from buildrunner.config.models import MP_LOCAL_REGISTRY
from buildrunner.docker import get_dockerfile
from buildrunner.docker.image_info import BuiltImageInfo, BuiltTaggedImage
from buildrunner.errors import BuildRunnerConfigurationError
from buildrunner.errors import BuildRunnerConfigurationError, BuildRunnerError


LOGGER = logging.getLogger(__name__)
Expand Down Expand Up @@ -536,8 +536,15 @@ def build_multiple_images(

return built_image

@retry(
TimeoutError,
tries=5,
logger=LOGGER,
)
@timeout_decorator.timeout(PUSH_TIMEOUT)
def _push_with_timeout(self, src_names: List[str], tag_names: List[str]) -> None:
def _push_with_timeout_and_retries(
self, src_names: List[str], tag_names: List[str]
) -> None:
"""
Creates tags from a set of source images in the remote registry.
This method will time out if it takes too long. An exception may be
Expand All @@ -558,11 +565,11 @@ def push(self) -> None:
Pushes all built images to their tagged image counterparts.
:raises TimeoutError: If the image fails to push within the specified timeout and retries
"""
# Parameters for timeout and retries
initial_timeout_seconds = 60
timeout_step_seconds = 60
timeout_max_seconds = 600
retries = 5
push_count = 0
expected_pushes = 0
for built_image in self._built_images:
for tagged_image in built_image.tagged_images:
expected_pushes += len(tagged_image.image_refs)

for built_image in self._built_images:
if not built_image.tagged_images:
Expand All @@ -578,34 +585,19 @@ def push(self) -> None:
f"Pushing {built_image.id} to {', '.join(tagged_image.image_refs)}"
)

timeout_seconds = initial_timeout_seconds
while retries > 0:
retries -= 1
LOGGER.debug(
f"Creating manifest(s) {tagged_image} with timeout {timeout_seconds} seconds"
)
try:
# Push each tag individually in order to prevent strange errors with multiple matching tags
for image_ref in tagged_image.image_refs:
self._push_with_timeout(source_image_refs, [image_ref])
# Finished within timeout
LOGGER.info(
f"Successfully pushed multiplatform image(s) {tagged_image}"
)
break
except Exception as exc: # pylint: disable=broad-exception-caught
LOGGER.warning(
f"Caught exception while pushing images, retrying: {exc}"
)
if retries == 0:
raise TimeoutError(
f"Timeout pushing {tagged_image} after {retries} retries"
f" and {timeout_seconds} seconds each try"
)
timeout_seconds += timeout_step_seconds

# Cap timeout at max timeout
timeout_seconds = min(timeout_seconds, timeout_max_seconds)
LOGGER.debug(f"Creating manifest(s) {tagged_image}")
# Push each tag individually in order to prevent strange errors with multiple matching tags
for image_ref in tagged_image.image_refs:
self._push_with_timeout_and_retries(source_image_refs, [image_ref])
push_count += 1
LOGGER.info(
f"Successfully pushed multiplatform image(s) {tagged_image}"
)

if push_count != expected_pushes:
raise BuildRunnerError(
f"Failed to push all images. Expected to push {expected_pushes} but pushed {push_count}."
)

@property
def num_built_images(self) -> int:
Expand Down
2 changes: 1 addition & 1 deletion setup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from setuptools import setup, find_packages


BASE_VERSION = "3.12"
BASE_VERSION = "3.13"

SOURCE_DIR = os.path.dirname(os.path.abspath(__file__))
BUILDRUNNER_DIR = os.path.join(SOURCE_DIR, "buildrunner")
Expand Down
231 changes: 230 additions & 1 deletion tests/test_multiplatform.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@
from python_on_whales.exceptions import DockerException

from buildrunner.docker.multiplatform_image_builder import MultiplatformImageBuilder
from buildrunner.docker.image_info import BuiltImageInfo
from buildrunner.docker.image_info import (
BuiltImageInfo,
BuiltTaggedImage,
TaggedImageInfo,
)


TEST_DIR = os.path.dirname(__file__)
Expand Down Expand Up @@ -585,3 +589,228 @@ def test_use_build_registry():
), "The local registry should not have been started when using a build registry"
finally:
registry_mpib._stop_local_registry()


@pytest.mark.parametrize(
"side_effect, expected_call_count",
[
(TimeoutError("Timeout"), 5),
(
[
None,
TimeoutError("Timeout"),
TimeoutError("Timeout"),
TimeoutError("Timeout"),
TimeoutError("Timeout"),
TimeoutError("Timeout"),
],
6,
),
(
[
None,
None,
None,
TimeoutError("Timeout"),
TimeoutError("Timeout"),
TimeoutError("Timeout"),
TimeoutError("Timeout"),
TimeoutError("Timeout"),
],
8,
),
],
)
@patch("buildrunner.docker.multiplatform_image_builder.PUSH_TIMEOUT", 1)
@patch("buildrunner.docker.multiplatform_image_builder.BuildRunnerConfig")
@patch(
"buildrunner.docker.multiplatform_image_builder.python_on_whales.docker.buildx.imagetools.create"
)
def test_push_retries(mock_docker, mock_config, side_effect, expected_call_count):
mock_docker.side_effect = side_effect
mock_config.get_instance.return_value.global_config.disable_multi_platform = False

build_name = "test-push-image-2001"
with pytest.raises(TimeoutError):
with MultiplatformImageBuilder() as mpib:
mpib._built_images = [
BuiltImageInfo(
id="test-id1",
images_by_platform={
"linux/arm64": BuiltTaggedImage(
repo=build_name,
tag="test",
digest="abcd12345",
platform="linux/arm64",
),
"linux/amd64": BuiltTaggedImage(
repo=build_name,
tag="test",
digest="abcd12345",
platform="linux/amd64",
),
},
tagged_images=[
TaggedImageInfo(
repo=build_name,
tags=["latest", "0.1.0", "0.1.1", "0.1.2", "test", "test2"],
),
],
)
]

mpib.push()

assert mock_docker.call_count == expected_call_count


@pytest.mark.parametrize(
"tagged_images, expected_call_count",
[
(
[
TaggedImageInfo(
repo="my-test-image1",
tags=["latest", "0.1.0", "0.1.1", "0.1.2", "test", "test2"],
)
],
6,
),
(
[
TaggedImageInfo(
repo="my-test-image1",
tags=["latest", "0.1.0", "test"],
),
TaggedImageInfo(
repo="my-test-image2",
tags=[
"latest",
"0.1.0",
],
),
TaggedImageInfo(
repo="my-test-image3",
tags=[
"latest",
],
),
TaggedImageInfo(
repo="my-test-image4",
tags=["latest", "0.1.0", "0.1.1", "0.1.2", "test", "test2"],
),
],
12,
),
(
[
TaggedImageInfo(
repo="my-test-image1",
tags=[
"latest",
"0.1.0",
"0.1.1",
"0.1.2",
"test",
],
),
TaggedImageInfo(
repo="my-test-image2",
tags=[
"latest",
"0.1.0",
"0.1.1",
"0.1.2",
"test",
],
),
TaggedImageInfo(
repo="my-test-image3",
tags=[
"latest",
"0.1.0",
"0.1.1",
"0.1.2",
"test",
],
),
TaggedImageInfo(
repo="my-test-image4",
tags=[
"latest",
"0.1.0",
"0.1.1",
"0.1.2",
"test",
],
),
TaggedImageInfo(
repo="my-test-image5",
tags=[
"latest",
"0.1.0",
"0.1.1",
"0.1.2",
"test",
],
),
TaggedImageInfo(
repo="my-test-image6",
tags=[
"latest",
"0.1.0",
"0.1.1",
"0.1.2",
"test",
],
),
TaggedImageInfo(
repo="my-test-image7",
tags=[
"latest",
"0.1.0",
"0.1.1",
"0.1.2",
"test",
],
),
],
35,
),
],
)
@patch("buildrunner.docker.multiplatform_image_builder.PUSH_TIMEOUT", 1)
@patch("buildrunner.docker.multiplatform_image_builder.BuildRunnerConfig")
@patch(
"buildrunner.docker.multiplatform_image_builder.python_on_whales.docker.buildx.imagetools.create"
)
def test_push_calls(mock_docker, mock_config, tagged_images, expected_call_count):
mock_docker.side_effect = None
mock_config.get_instance.return_value.global_config.disable_multi_platform = False

build_name = "test-push-image-2001"
with MultiplatformImageBuilder() as mpib:
mpib._built_images = [
BuiltImageInfo(
id="test-id1",
images_by_platform={
"linux/arm64": BuiltTaggedImage(
repo=build_name,
tag="test",
digest="abcd12345",
platform="linux/arm64",
),
"linux/amd64": BuiltTaggedImage(
repo=build_name,
tag="test",
digest="abcd12345",
platform="linux/amd64",
),
},
tagged_images=tagged_images,
)
]

mpib.push()

assert mock_docker.call_count == expected_call_count
1 change: 1 addition & 0 deletions tests/test_push_artifact.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import json
import pytest


from tests import test_runner

test_dir_path = os.path.realpath(os.path.dirname(__file__))
Expand Down

0 comments on commit 9e31cd1

Please sign in to comment.