From 549f3d2d18c9a78ad7a56ab8d3b641550813933d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Hozza?= Date: Tue, 7 Jan 2025 18:34:15 +0100 Subject: [PATCH] imgtestlib/gen_build_info_*: don't require osbuild_ref and runner_distro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Modify `gen_build_info_dir_path_prefix()` and `gen_build_info_s3_dir_path()` functions to not require osbuild_ref and runner_distro to be passed explicitly as arguments. If not provided, the runner_distro defaults to the host distro and the osbuild_ref defaults to the pinned version for the runner_distro. This simplifies all the places which call these functions. Signed-off-by: Tomáš Hozza --- test/scripts/check-build-coverage | 4 +- test/scripts/generate-ostree-build-config | 4 +- test/scripts/imgtestlib.py | 35 ++++++++++------- test/scripts/test_imgtestlib.py | 47 ++++++++++++++++++++++- test/scripts/upload-results | 4 +- 5 files changed, 69 insertions(+), 25 deletions(-) diff --git a/test/scripts/check-build-coverage b/test/scripts/check-build-coverage index 2a548d7dcc..c2cec170c8 100755 --- a/test/scripts/check-build-coverage +++ b/test/scripts/check-build-coverage @@ -60,9 +60,7 @@ def main(): parser.add_argument("cachedir", type=str, help="path to download the build test cache") args = parser.parse_args() cachedir = args.cachedir - runner_distro = testlib.get_host_distro() - osbuild_ref = testlib.get_osbuild_commit(runner_distro) - _, ok = testlib.dl_build_info(cachedir, osbuild_ref, runner_distro) + _, ok = testlib.dl_build_info(cachedir) # fail the job if the sync failed if not ok: sys.exit(1) diff --git a/test/scripts/generate-ostree-build-config b/test/scripts/generate-ostree-build-config index 6a9d5ac9a7..7b460e8573 100755 --- a/test/scripts/generate-ostree-build-config +++ b/test/scripts/generate-ostree-build-config @@ -223,9 +223,7 @@ def setup_dependencies(manifests, config_map, distro, arch): ic_image_type = image_config["image-type"] dep_build_name = testlib.gen_build_name(ic_distro, ic_arch, dep_image_type, dep_config_name) manifest_id = manifests[dep_build_name + ".json"]["id"] - runner_distro = testlib.get_host_distro() - osbuild_ref = testlib.get_osbuild_commit(runner_distro) - container_s3_prefix = testlib.gen_build_info_s3_dir_path(osbuild_ref, runner_distro, distro, arch, manifest_id) + container_s3_prefix = testlib.gen_build_info_s3_dir_path(distro, arch, manifest_id) container_s3_path = os.path.join(container_s3_prefix, "container", "container.tar") # start each container once on an incremental port diff --git a/test/scripts/imgtestlib.py b/test/scripts/imgtestlib.py index db68784a25..55bf289e38 100644 --- a/test/scripts/imgtestlib.py +++ b/test/scripts/imgtestlib.py @@ -108,11 +108,11 @@ def list_images(distros=None, arches=None, images=None): return json.loads(out) -def dl_build_info(destination, osbuild_ref, runner_distro, distro=None, arch=None): +def dl_build_info(destination, distro=None, arch=None, osbuild_ref=None, runner_distro=None): """ Downloads all the configs from the s3 bucket. """ - s3url = gen_build_info_s3_dir_path(osbuild_ref, runner_distro, distro, arch) + s3url = gen_build_info_s3_dir_path(distro, arch, osbuild_ref=osbuild_ref, runner_distro=runner_distro) print(f"⬇️ Downloading configs from {s3url}") # only download info.json (exclude everything, then include) files, otherwise we get manifests and whole images job = sp.run(["aws", "s3", "sync", @@ -147,19 +147,28 @@ def gen_build_name(distro, arch, image_type, config_name): return f"{_u(distro)}-{_u(arch)}-{_u(image_type)}-{_u(config_name)}" -def gen_build_info_dir_path_prefix(osbuild_ref, runner_distro, distro=None, arch=None, manifest_id=None): +def gen_build_info_dir_path_prefix(distro=None, arch=None, manifest_id=None, osbuild_ref=None, runner_distro=None): """ Generates the relative path prefix for the location where build info and artifacts will be stored for a specific build. This is a simple concatenation of the components, but ensures that paths are consistent. The caller is responsible for prepending the location root to the generated path. - A fully specified path is returned if all parameters are specified, otherwise a partial path is returned. Partial - path may be useful for working with a superset of build infos. For a more specific path to be generated when - specifying any of the optional parameters, the caller must specify all of the previous parameters. For example, - if 'arch' is specified, 'distro' must also be specified for 'arch' to be included in the path. + If no 'osbuild_ref' is specified, the value returned by get_osbuild_commit() for the 'runner_distro' will be used. + if no 'runner_distro' is specified, the value returned by get_host_distro() will be used. + + A fully specified path is returned if all of the 'distro', 'arch' and 'manifest_id' parameters are specified, + otherwise a partial path is returned. Partial path may be useful for working with a superset of build infos. + For a more specific path to be generated when specifying any of the optional parameters, the caller must specify + all of the previous parameters. For example, if 'arch' is specified, 'distro' must also be specified for 'arch' to + be included in the path. The returned path always has a trailing separator at the end to signal that it is a directory. """ + if runner_distro is None: + runner_distro = get_host_distro() + if osbuild_ref is None: + osbuild_ref = get_osbuild_commit(runner_distro) + path = os.path.join(f"osbuild-ref-{osbuild_ref}", f"runner-{runner_distro}") for p in (distro, arch, f"manifest-id-{manifest_id}" if manifest_id else None): if p is None: @@ -168,7 +177,7 @@ def gen_build_info_dir_path_prefix(osbuild_ref, runner_distro, distro=None, arch return path + "/" -def gen_build_info_s3_dir_path(osbuild_ref, runner_distro, distro=None, arch=None, manifest_id=None): +def gen_build_info_s3_dir_path(distro=None, arch=None, manifest_id=None, osbuild_ref=None, runner_distro=None): """ Generates the s3 URL for the location where build info and artifacts will be stored for a specific one or more builds, depending on the parameters specified. @@ -180,7 +189,7 @@ def gen_build_info_s3_dir_path(osbuild_ref, runner_distro, distro=None, arch=Non return os.path.join( S3_BUCKET, S3_PREFIX, - gen_build_info_dir_path_prefix(osbuild_ref, runner_distro, distro, arch, manifest_id), + gen_build_info_dir_path_prefix(distro, arch, manifest_id, osbuild_ref, runner_distro), ) @@ -317,14 +326,12 @@ def filter_builds(manifests, distro=None, arch=None, skip_ostree_pull=True): Returns a list of build requests for the manifests that have no matching config in the test build cache. """ print(f"⚙️ Filtering {len(manifests)} build configurations") - runner_distro = get_host_distro() - osbuild_ref = get_osbuild_commit(runner_distro) dl_root_path = os.path.join(TEST_CACHE_ROOT, "s3configs", "builds") - dl_path = os.path.join(dl_root_path, gen_build_info_dir_path_prefix(osbuild_ref, runner_distro, distro, arch)) + dl_path = os.path.join(dl_root_path, gen_build_info_dir_path_prefix(distro, arch)) os.makedirs(dl_path, exist_ok=True) build_requests = [] - out, dl_ok = dl_build_info(dl_path, osbuild_ref, runner_distro, distro=distro, arch=arch) + out, dl_ok = dl_build_info(dl_path, distro, arch) # continue even if the dl failed; will build all configs if dl_ok: # print output which includes list of downloaded files for CI job log @@ -353,7 +360,7 @@ def filter_builds(manifests, distro=None, arch=None, skip_ostree_pull=True): # check if the hash_fname exists in the synced directory build_info_dir = os.path.join( dl_root_path, - gen_build_info_dir_path_prefix(osbuild_ref, runner_distro, distro, arch, manifest_id) + gen_build_info_dir_path_prefix(distro, arch, manifest_id) ) if check_for_build(manifest_fname, build_info_dir, errors): diff --git a/test/scripts/test_imgtestlib.py b/test/scripts/test_imgtestlib.py index 53cc5ec580..f9bafe6049 100644 --- a/test/scripts/test_imgtestlib.py +++ b/test/scripts/test_imgtestlib.py @@ -1,6 +1,7 @@ import os import subprocess as sp import tempfile +from unittest.mock import patch import pytest @@ -92,9 +93,30 @@ def test_read_seed(): }, "osbuild-ref-abc123/runner-fedora-41/fedora-41/" ), + # default osbuild_ref + ( + { + "runner_distro": "fedora-41", + }, + "osbuild-ref-abcdef123456/runner-fedora-41/" + ), + # default runner_distro + ( + { + "osbuild_ref": "abc123", + }, + "osbuild-ref-abc123/runner-fedora-999/" + ), + # default osbuild_ref and runner_distro + ( + {}, + "osbuild-ref-abcdef123456/runner-fedora-999/" + ), )) def test_gen_build_info_dir_path_prefix(kwargs, expected): - assert testlib.gen_build_info_dir_path_prefix(**kwargs) == expected + with patch("imgtestlib.get_host_distro", return_value="fedora-999"), \ + patch("imgtestlib.get_osbuild_commit", return_value="abcdef123456"): + assert testlib.gen_build_info_dir_path_prefix(**kwargs) == expected @pytest.mark.parametrize("kwargs,expected", ( @@ -158,9 +180,30 @@ def test_gen_build_info_dir_path_prefix(kwargs, expected): testlib.S3_BUCKET + "/" + testlib.S3_PREFIX + \ "/osbuild-ref-abcdef123456/runner-fedora-41/fedora-41/", ), + # default osbuild_ref + ( + { + "runner_distro": "fedora-41", + }, + testlib.S3_BUCKET + "/" + testlib.S3_PREFIX + "/osbuild-ref-abcdef123456/runner-fedora-41/" + ), + # default runner_distro + ( + { + "osbuild_ref": "abc123", + }, + testlib.S3_BUCKET + "/" + testlib.S3_PREFIX + "/osbuild-ref-abc123/runner-fedora-999/" + ), + # default osbuild_ref and runner_distro + ( + {}, + testlib.S3_BUCKET + "/" + testlib.S3_PREFIX + "/osbuild-ref-abcdef123456/runner-fedora-999/" + ), )) def test_gen_build_info_s3_dir_path(kwargs, expected): - assert testlib.gen_build_info_s3_dir_path(**kwargs) == expected + with patch("imgtestlib.get_host_distro", return_value="fedora-999"), \ + patch("imgtestlib.get_osbuild_commit", return_value="abcdef123456"): + assert testlib.gen_build_info_s3_dir_path(**kwargs) == expected test_container = "registry.gitlab.com/redhat/services/products/image-builder/ci/osbuild-composer/manifest-list-test" diff --git a/test/scripts/upload-results b/test/scripts/upload-results index 15df8805e3..f9a99d465f 100755 --- a/test/scripts/upload-results +++ b/test/scripts/upload-results @@ -38,9 +38,7 @@ def main(): build_info["pr"] = pr_number.removeprefix("PR-") testlib.write_build_info(build_dir, build_info) - runner_distro = testlib.get_host_distro() - osbuild_ref = testlib.get_osbuild_commit(runner_distro) - s3url = testlib.gen_build_info_s3_dir_path(osbuild_ref, runner_distro, distro, arch, manifest_id) + s3url = testlib.gen_build_info_s3_dir_path(distro, arch, manifest_id) print(f"⬆️ Uploading {build_dir} to {s3url}") testlib.runcmd_nc(["aws", "s3", "cp", "--no-progress", "--acl=private", "--recursive", build_dir+"/", s3url])