Skip to content

Commit

Permalink
Removed manifest list merging
Browse files Browse the repository at this point in the history
  • Loading branch information
@midnightercz
midnightercz committed Feb 11, 2025
1 parent d202b4f commit 223fbae
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 290 deletions.
97 changes: 3 additions & 94 deletions src/pubtools/_quay/container_image_pusher.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import logging
from concurrent import futures
from concurrent.futures.thread import ThreadPoolExecutor
from typing import Any, cast, Dict, List, Optional, Union
from typing import Any, cast, Dict, List, Optional


import requests
Expand All @@ -14,12 +14,10 @@
get_internal_container_repo_name,
log_step,
run_with_retries,
timestamp,
)
from .quay_client import QuayClient
from .tag_images import tag_images
from .manifest_list_merger import ManifestListMerger
from .types import ManifestList, Manifest
from .types import ManifestList

from pubtools.tracing import get_trace_wrapper

Expand Down Expand Up @@ -175,56 +173,6 @@ def copy_v1_push_item(self, push_item: Any) -> None:

self.run_tag_images(source_ref, dest_refs, True, self.target_settings)

def run_merge_workflow(self, source_ref: str, dest_refs: List[str]) -> None:
"""
Perform Docker push and manifest list merge workflow.
The difference in this workflow is that all single arch images are first copied via
digest, and then their respective manifest lists are merged.
Args:
source_ref (str):
Source image reference.
dest_refs ([str]):
List of destination references which need manifest merging.
"""
image_schema = "{repo}@{digest}"
source_repo = source_ref.split(":")[0]

# get unique destination repositories
dest_repos = sorted(list(set([ref.split(":")[0] for ref in dest_refs])))
source_ml = cast(
ManifestList,
self.src_quay_client.get_manifest(source_ref, media_type=QuayClient.MANIFEST_LIST_TYPE),
)

# copy each arch source image to all destination repos
for manifest in source_ml["manifests"]:
source_image = image_schema.format(repo=source_repo, digest=manifest["digest"])
dest_images = [
image_schema.format(repo=dest_repo, digest=manifest["digest"])
for dest_repo in dest_repos
]
self.run_tag_images(source_image, dest_images, False, self.target_settings)

for dest_ref in dest_refs:
LOG.info(
"Merging manifest lists of source '{0}' and destination '{1}'".format(
source_ref, dest_ref
)
)
merger = ManifestListMerger(source_ref, dest_ref, host=self.quay_host)
merger.set_quay_clients(self.src_quay_client, self.dest_quay_client)
merger.merge_manifest_lists()

# add additional tag to merged manifest lists so that they won't be garbage collected
dest_repos = []
for ref in dest_refs:
repo = ref.split(":")[0]
if repo not in dest_repos:
dest_repos.append(repo)
self.run_tag_images(ref, [f"{ref}-{timestamp()}"], True, self.target_settings)

def copy_multiarch_push_item(self, push_item: Any, source_ml: ManifestList) -> None:
"""
Evaluate the correct tagging and manifest list merging strategy of multiarch push item.
Expand All @@ -241,7 +189,6 @@ def copy_multiarch_push_item(self, push_item: Any, source_ml: ManifestList) -> N
LOG.info("Copying push item '{0}' as a multiarch image.".format(push_item))
source_ref = push_item.metadata["pull_url"]
simple_dest_refs = []
merge_mls_dest_refs = []

image_schema = "{host}/{namespace}/{repo}:{tag}"
namespace = self.target_settings["quay_namespace"]
Expand All @@ -255,38 +202,7 @@ def copy_multiarch_push_item(self, push_item: Any, source_ml: ManifestList) -> N
repo=internal_repo,
tag=tag,
)
try:
dest_ml = cast(
Union[Manifest, ManifestList], self.dest_quay_client.get_manifest(dest_ref)
)
if dest_ml.get("mediaType") != QuayClient.MANIFEST_LIST_TYPE:
LOG.warning(
"Image {0} doesn't have a manifest list, it will be overwritten".format(
dest_ref
)
)
simple_dest_refs.append(dest_ref)
else:
LOG.info(
"Getting missing archs between images '{0}' and '{1}'".format(
source_ref, dest_ref
)
)
missing_archs = ManifestListMerger.get_missing_architectures(
source_ml, cast(ManifestList, dest_ml)
)
# Option 1: Dest doesn't contain extra archs, ML merging is unnecessary
if not missing_archs:
simple_dest_refs.append(dest_ref)
# Option 2: Destination has extra archs, MLs will be merged
else:
merge_mls_dest_refs.append(dest_ref)
except requests.exceptions.HTTPError as e:
# Option 3: Destination tag doesn't exist, no ML merging
if e.response.status_code == 404 or e.response.status_code == 401:
simple_dest_refs.append(dest_ref)
else:
raise
simple_dest_refs.append(dest_ref)

if simple_dest_refs:
LOG.info(
Expand All @@ -295,13 +211,6 @@ def copy_multiarch_push_item(self, push_item: Any, source_ml: ManifestList) -> N
)
)
self.run_tag_images(source_ref, list(set(simple_dest_refs)), True, self.target_settings)
if merge_mls_dest_refs:
LOG.info(
"Copying image {0} to {1} destinations and merging manifest lists".format(
source_ref, len(merge_mls_dest_refs)
)
)
self.run_merge_workflow(source_ref, merge_mls_dest_refs)

@log_step("Push images to Quay")
def push_container_images(self) -> None:
Expand Down
196 changes: 0 additions & 196 deletions tests/test_container_pusher.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,105 +180,34 @@ def test_copy_v1_item(
)


@mock.patch("pubtools._quay.container_image_pusher.timestamp")
@mock.patch("pubtools._quay.container_image_pusher.ManifestListMerger")
@mock.patch("pubtools._quay.container_image_pusher.tag_images")
@mock.patch("pubtools._quay.container_image_pusher.QuayClient")
def test_merge_workflow(
mock_quay_client,
mock_tag_images,
mock_ml_merger,
mock_timestamp,
target_settings,
container_multiarch_push_item,
):
mock_get_manifest = mock.MagicMock()
mock_get_manifest.return_value = {"manifests": [{"digest": "digest1"}, {"digest": "digest2"}]}
mock_quay_client.return_value.get_manifest = mock_get_manifest
mock_timestamp.return_value = "timestamp"

pusher = container_image_pusher.ContainerImagePusher(
[container_multiarch_push_item], target_settings
)
pusher.run_merge_workflow(
"registry/src/image:1", ["registry/dest1/image:1", "registry/dest2/image:2"]
)
mock_get_manifest.assert_called_once_with(
"registry/src/image:1", media_type=mock_quay_client.MANIFEST_LIST_TYPE
)
# test that src digests are copied to all dest repos
assert mock_tag_images.call_args_list[0][0][1] == [
"registry/dest1/image@digest1",
"registry/dest2/image@digest1",
]
assert mock_tag_images.call_args_list[1][0][1] == [
"registry/dest1/image@digest2",
"registry/dest2/image@digest2",
]
assert mock_tag_images.call_args_list[2][0][0] == "registry/dest1/image:1"
assert mock_tag_images.call_args_list[2][0][1] == ["registry/dest1/image:1-timestamp"]
assert mock_tag_images.call_args_list[3][0][0] == "registry/dest2/image:2"
assert mock_tag_images.call_args_list[3][0][1] == ["registry/dest2/image:2-timestamp"]

assert mock_ml_merger.call_args_list == [
mock.call("registry/src/image:1", "registry/dest1/image:1", host="quay.io"),
mock.call("registry/src/image:1", "registry/dest2/image:2", host="quay.io"),
]

assert len(mock_ml_merger.mock_calls) == 6


@mock.patch("pubtools._quay.container_image_pusher.ContainerImagePusher.run_merge_workflow")
@mock.patch("pubtools._quay.container_image_pusher.ManifestListMerger.get_missing_architectures")
@mock.patch("pubtools._quay.container_image_pusher.tag_images")
@mock.patch("pubtools._quay.container_image_pusher.QuayClient")
def test_copy_multiarch_item_no_extra_archs(
mock_quay_client,
mock_tag_images,
mock_get_missing_archs,
mock_merge_workflow,
target_settings,
container_multiarch_push_item,
):
mock_get_manifest = mock.MagicMock()
mock_get_manifest.return_value = {"manifest_list": "second_ml"}
mock_quay_client.return_value.get_manifest = mock_get_manifest
mock_get_missing_archs.return_value = []

pusher = container_image_pusher.ContainerImagePusher(
[container_multiarch_push_item], target_settings
)
pusher.copy_multiarch_push_item(container_multiarch_push_item, {"manifest_list": "first_ml"})

mock_get_manifest.assert_called_once_with(
"quay.io/some-namespace/target----repo:latest-test-tag"
)
assert mock_tag_images.call_count == 1
assert mock_tag_images.call_args_list[0][0] == (
"some-registry/src/repo:1",
["quay.io/some-namespace/target----repo:latest-test-tag"],
)


@mock.patch("pubtools._quay.container_image_pusher.ContainerImagePusher.run_merge_workflow")
@mock.patch("pubtools._quay.container_image_pusher.ManifestListMerger.get_missing_architectures")
@mock.patch("pubtools._quay.container_image_pusher.tag_images")
@mock.patch("pubtools._quay.container_image_pusher.QuayClient")
def test_copy_multiarch_item_no_dest_ml(
mock_quay_client,
mock_tag_images,
mock_get_missing_archs,
mock_merge_workflow,
target_settings,
container_multiarch_push_item,
):
mock_get_manifest = mock.MagicMock()

response = mock.MagicMock()
response.status_code = 401
mock_get_manifest.side_effect = requests.exceptions.HTTPError("some error", response=response)
mock_quay_client.return_value.get_manifest = mock_get_manifest
mock_get_missing_archs.return_value = []

pusher = container_image_pusher.ContainerImagePusher(
[container_multiarch_push_item], target_settings
Expand All @@ -288,137 +217,12 @@ def test_copy_multiarch_item_no_dest_ml(
{"manifest_list": "first_ml"},
)

mock_get_manifest.assert_called_once_with(
"quay.io/some-namespace/target----repo:latest-test-tag",
)

assert mock_tag_images.call_count == 1
assert mock_tag_images.call_args_list[0][0] == (
"some-registry/src/repo:1",
["quay.io/some-namespace/target----repo:latest-test-tag"],
)

mock_merge_workflow.assert_not_called()


@mock.patch("pubtools._quay.container_image_pusher.ContainerImagePusher.run_merge_workflow")
@mock.patch("pubtools._quay.container_image_pusher.ManifestListMerger.get_missing_architectures")
@mock.patch("pubtools._quay.container_image_pusher.tag_images")
@mock.patch("pubtools._quay.container_image_pusher.QuayClient")
def test_copy_multiarch_item_network_error(
mock_quay_client,
mock_tag_images,
mock_get_missing_archs,
mock_merge_workflow,
target_settings,
container_multiarch_push_item,
):
mock_get_manifest = mock.MagicMock()

response = mock.MagicMock()
response.status_code = 500
mock_get_manifest.side_effect = requests.exceptions.HTTPError("bad error", response=response)

mock_quay_client.return_value.get_manifest = mock_get_manifest
mock_get_missing_archs.return_value = []

pusher = container_image_pusher.ContainerImagePusher(
[container_multiarch_push_item], target_settings
)
with pytest.raises(requests.exceptions.HTTPError, match="bad error"):
pusher.copy_multiarch_push_item(
container_multiarch_push_item, {"manifest_list": "first_ml"}
)


@mock.patch("pubtools._quay.container_image_pusher.ContainerImagePusher.run_merge_workflow")
@mock.patch("pubtools._quay.container_image_pusher.ManifestListMerger.get_missing_architectures")
@mock.patch("pubtools._quay.container_image_pusher.tag_images")
@mock.patch("pubtools._quay.container_image_pusher.QuayClient")
def test_copy_multiarch_item_missing_archs(
mock_quay_client,
mock_tag_images,
mock_get_missing_archs,
mock_merge_workflow,
target_settings,
container_multiarch_push_item,
):
mock_get_manifest = mock.MagicMock()

mock_get_manifest.return_value = {
"manifest_list": "second_ml",
"mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
}
mock_quay_client.MANIFEST_LIST_TYPE = (
"application/vnd.docker.distribution.manifest.list.v2+json"
)
mock_quay_client.return_value.get_manifest = mock_get_manifest
mock_get_missing_archs.return_value = [{"arch": "x86_64"}, {"arch": "amd64"}]

pusher = container_image_pusher.ContainerImagePusher(
[container_multiarch_push_item], target_settings
)
pusher.copy_multiarch_push_item(container_multiarch_push_item, {"manifest_list": "first_ml"})

mock_get_manifest.assert_called_once_with(
"quay.io/some-namespace/target----repo:latest-test-tag",
)

assert mock_merge_workflow.call_count == 1
assert mock_merge_workflow.call_args_list[0][0] == (
"some-registry/src/repo:1",
["quay.io/some-namespace/target----repo:latest-test-tag"],
)

mock_tag_images.assert_not_called()


@mock.patch("pubtools._quay.container_image_pusher.ContainerImagePusher.run_merge_workflow")
@mock.patch("pubtools._quay.container_image_pusher.ManifestListMerger.get_missing_architectures")
@mock.patch("pubtools._quay.container_image_pusher.tag_images")
@mock.patch("pubtools._quay.container_image_pusher.QuayClient")
def test_copy_multiarch_item_existing_dest_not_manifest_list(
mock_quay_client,
mock_tag_images,
mock_get_missing_archs,
mock_merge_workflow,
target_settings,
container_multiarch_push_item,
caplog,
):
caplog.set_level(logging.WARNING)
mock_get_manifest = mock.MagicMock()

mock_get_manifest.return_value = {
"manifest_list": "v2s2_manifest",
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
}
mock_quay_client.MANIFEST_LIST_TYPE = (
"application/vnd.docker.distribution.manifest.list.v2+json"
)
mock_quay_client.return_value.get_manifest = mock_get_manifest

pusher = container_image_pusher.ContainerImagePusher(
[container_multiarch_push_item], target_settings
)
pusher.copy_multiarch_push_item(container_multiarch_push_item, {"manifest_list": "first_ml"})

mock_get_manifest.assert_called_once_with(
"quay.io/some-namespace/target----repo:latest-test-tag",
)

mock_merge_workflow.assert_not_called()
assert mock_tag_images.call_count == 1
assert mock_tag_images.call_args_list[0][0] == (
"some-registry/src/repo:1",
["quay.io/some-namespace/target----repo:latest-test-tag"],
)

expected_logs = [
"Image quay.io/some-namespace/target----repo:latest-test-tag doesn't have a manifest.*",
]
compare_logs(caplog, expected_logs)


@mock.patch("pubtools._quay.container_image_pusher.ContainerImagePusher.copy_source_push_item")
@mock.patch("pubtools._quay.container_image_pusher.ContainerImagePusher.copy_multiarch_push_item")
Expand Down

0 comments on commit 223fbae

Please sign in to comment.