diff --git a/test/scripts/build-image b/test/scripts/build-image index 1175cc76cd..667cc24d29 100755 --- a/test/scripts/build-image +++ b/test/scripts/build-image @@ -46,8 +46,7 @@ def main(): osbuild_ver, _ = testlib.runcmd(["osbuild", "--version"]) - osrelease = testlib.read_osrelease() - distro_version = osrelease["ID"] + "-" + osrelease["VERSION_ID"] + distro_version = testlib.get_host_distro() osbuild_commit = testlib.get_osbuild_commit(distro_version) if osbuild_commit is None: osbuild_commit = "RELEASE" diff --git a/test/scripts/generate-ostree-build-config b/test/scripts/generate-ostree-build-config index 6a97242313..7b460e8573 100755 --- a/test/scripts/generate-ostree-build-config +++ b/test/scripts/generate-ostree-build-config @@ -223,8 +223,8 @@ 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"] - container_s3_prefix = testlib.gen_build_info_s3(distro, arch, manifest_id) - container_s3_path = f"{container_s3_prefix}container/container.tar" + 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 port = container_ports.get(container_s3_path) diff --git a/test/scripts/imgtestlib.py b/test/scripts/imgtestlib.py index 2f69267d8c..55bf289e38 100644 --- a/test/scripts/imgtestlib.py +++ b/test/scripts/imgtestlib.py @@ -108,17 +108,11 @@ def list_images(distros=None, arches=None, images=None): return json.loads(out) -def dl_build_info(destination, 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 = f"{S3_BUCKET}/{S3_PREFIX}" - if distro and arch: - # only take them into account if both are defined - s3url = f"{s3url}/{distro}/{arch}" - - s3url += "/" - + 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", @@ -153,31 +147,50 @@ 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(root, osbuild_ver, manifest_id): +def gen_build_info_dir_path_prefix(distro=None, arch=None, manifest_id=None, osbuild_ref=None, runner_distro=None): """ - Generates the path to the directory that contains the build info. - This is a simple os.path.join() of the components, but ensures that paths are consistent. - """ - return os.path.join(root, osbuild_ver, manifest_id, "") + 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. + 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. -def gen_build_info_path(root, osbuild_ver, manifest_id): - """ - Generates the path to the info.json. - This is a simple os.path.join() of the components, but ensures that paths are consistent. + 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. """ - return os.path.join(gen_build_info_dir_path(root, osbuild_ver, manifest_id), "info.json") + 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: + return path + "/" + path = os.path.join(path, p) + return path + "/" -def gen_build_info_s3(distro, arch, manifest_id): +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 build - configuration. - This is a simple concatenation of the components, but ensures that paths are consistent. + 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. + + A fully specified path is returned if all parameters are specified, otherwise a partial path is returned. + This function basically just prepends the S3_BUCKET and S3_PREFIX to the path generated by + gen_build_info_dir_path_prefix(). """ - osbuild_ver = get_osbuild_nevra() - build_info_prefix = f"{S3_BUCKET}/images/builds/{distro}/{arch}" - return gen_build_info_dir_path(build_info_prefix, osbuild_ver, manifest_id) + return os.path.join( + S3_BUCKET, + S3_PREFIX, + gen_build_info_dir_path_prefix(distro, arch, manifest_id, osbuild_ref, runner_distro), + ) def check_config_names(): @@ -313,18 +326,17 @@ 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") - dl_path = os.path.join(TEST_CACHE_ROOT, "s3configs", f"builds/{distro}/{arch}/") + 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(distro, arch)) os.makedirs(dl_path, exist_ok=True) build_requests = [] - out, dl_ok = dl_build_info(dl_path, 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 print(out) - osbuild_ver = get_osbuild_nevra() - errors: list[str] = [] for manifest_fname, data in manifests.items(): manifest_id = data["id"] @@ -346,7 +358,10 @@ def filter_builds(manifests, distro=None, arch=None, skip_ostree_pull=True): build_request["manifest-checksum"] = manifest_id # check if the hash_fname exists in the synced directory - build_info_dir = gen_build_info_dir_path(dl_path, osbuild_ver, manifest_id) + build_info_dir = os.path.join( + dl_root_path, + gen_build_info_dir_path_prefix(distro, arch, manifest_id) + ) if check_for_build(manifest_fname, build_info_dir, errors): build_requests.append(build_request) @@ -364,8 +379,8 @@ def clargs(): default_arch = os.uname().machine parser = argparse.ArgumentParser() parser.add_argument("config", type=str, help="path to write config") - parser.add_argument("--distro", type=str, default=None, - help="distro to generate configs for (omit to generate for all distros)") + parser.add_argument("--distro", type=str, required=True, + help="distro to generate configs for") parser.add_argument("--arch", type=str, default=default_arch, help="architecture to generate configs for (defaults to host architecture)") @@ -394,6 +409,15 @@ def read_osrelease(): return osrelease +def get_host_distro(): + """ + Get the host distro version based on data in the os-release file. + The format is - (e.g. fedora-41). + """ + osrelease = read_osrelease() + return f"{osrelease['ID']}-{osrelease['VERSION_ID']}" + + def get_osbuild_commit(distro_version): """ Get the osbuild commit defined in the Schutzfile for the host distro. @@ -416,14 +440,6 @@ def get_bib_ref(): return data.get("common", {}).get("bootc-image-builder", {}).get("ref", None) -def get_osbuild_nevra(): - """ - Returned the installed osbuild version. Exits with an error if osbuild is not installed. - """ - out, _ = runcmd(["rpm", "-q", "--qf", "%{nevra}", "osbuild"]) - return out.decode().strip() - - def rng_seed_env(): """ Read the rng seed from the Schutzfile and return it as a map to use as an environment variable with the appropriate diff --git a/test/scripts/setup-osbuild-repo b/test/scripts/setup-osbuild-repo index 74f9a8bbfb..512c3acafd 100755 --- a/test/scripts/setup-osbuild-repo +++ b/test/scripts/setup-osbuild-repo @@ -50,9 +50,7 @@ def write_repo(commit, distro_version): def main(): - osrelease = testlib.read_osrelease() - - distro_version = osrelease["ID"] + "-" + osrelease["VERSION_ID"] + distro_version = testlib.get_host_distro() commit_id = testlib.get_osbuild_commit(distro_version) if not commit_id: print(f"Error: {distro_version} does not have the osbuild commit ID defined in the Schutzfile") diff --git a/test/scripts/test_imgtestlib.py b/test/scripts/test_imgtestlib.py index 2c4be9b522..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 @@ -37,15 +38,172 @@ def test_read_seed(): assert "OSBUILD_TESTING_RNG_SEED" in seed_env -def test_path_generators(): - testlib.get_osbuild_nevra = lambda: "osbuild-104-1.fc41.noarch" - - assert testlib.gen_build_info_dir_path("inforoot", testlib.get_osbuild_nevra(), "abc123") == \ - "inforoot/osbuild-104-1.fc41.noarch/abc123/" - assert testlib.gen_build_info_path("inforoot", testlib.get_osbuild_nevra(), "abc123") == \ - "inforoot/osbuild-104-1.fc41.noarch/abc123/info.json" - assert testlib.gen_build_info_s3("fedora-41", "aarch64", "abc123") == \ - testlib.S3_BUCKET + "/images/builds/fedora-41/aarch64/osbuild-104-1.fc41.noarch/abc123/" +@pytest.mark.parametrize("kwargs,expected", ( + ( + { + "osbuild_ref": "abc123", + "runner_distro": "fedora-41", + }, + "osbuild-ref-abc123/runner-fedora-41/" + ), + ( + { + "osbuild_ref": "abc123", + "runner_distro": "fedora-41", + "distro": "fedora-41", + }, + "osbuild-ref-abc123/runner-fedora-41/fedora-41/" + ), + ( + { + "osbuild_ref": "abc123", + "runner_distro": "fedora-41", + "distro": "fedora-41", + "arch": "x86_64", + }, + "osbuild-ref-abc123/runner-fedora-41/fedora-41/x86_64/" + ), + ( + { + "osbuild_ref": "abc123", + "runner_distro": "fedora-41", + "distro": "fedora-41", + "arch": "x86_64", + "manifest_id": "abc123123", + }, + "osbuild-ref-abc123/runner-fedora-41/fedora-41/x86_64/manifest-id-abc123123/" + ), + # Optional arg 'distro' not specified, thus following optional args 'arch' and 'manifest_id' are ignored + ( + { + "osbuild_ref": "abc123", + "runner_distro": "fedora-41", + "arch": "x86_64", + "manifest_id": "abc123123" + }, + "osbuild-ref-abc123/runner-fedora-41/" + ), + # Optional arg 'arch' not specified, thus following optional arg 'manifest_id' is ignored + ( + { + "osbuild_ref": "abc123", + "runner_distro": "fedora-41", + "distro": "fedora-41", + "manifest_id": "abc123123" + }, + "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): + 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", ( + ( + { + "osbuild_ref": "abcdef123456", + "runner_distro": "fedora-41", + "distro": "fedora-41", + "arch": "aarch64", + "manifest_id": "abc123" + }, + testlib.S3_BUCKET + "/" + testlib.S3_PREFIX + \ + "/osbuild-ref-abcdef123456/runner-fedora-41/fedora-41/aarch64/manifest-id-abc123/", + ), + ( + { + "osbuild_ref": "abcdef123456", + "runner_distro": "fedora-41", + "distro": "fedora-41", + "arch": "aarch64", + }, + testlib.S3_BUCKET + "/" + testlib.S3_PREFIX + \ + "/osbuild-ref-abcdef123456/runner-fedora-41/fedora-41/aarch64/", + ), + ( + { + "osbuild_ref": "abcdef123456", + "runner_distro": "fedora-41", + "distro": "fedora-41", + }, + testlib.S3_BUCKET + "/" + testlib.S3_PREFIX + \ + "/osbuild-ref-abcdef123456/runner-fedora-41/fedora-41/", + ), + ( + { + "osbuild_ref": "abcdef123456", + "runner_distro": "fedora-41", + }, + testlib.S3_BUCKET + "/" + testlib.S3_PREFIX + \ + "/osbuild-ref-abcdef123456/runner-fedora-41/", + ), + # Optional arg 'distro' not specified, thus following optional args 'arch' and 'manifest_id' are ignored + ( + { + "osbuild_ref": "abcdef123456", + "runner_distro": "fedora-41", + "arch": "aarch64", + "manifest_id": "abc123" + }, + testlib.S3_BUCKET + "/" + testlib.S3_PREFIX + \ + "/osbuild-ref-abcdef123456/runner-fedora-41/", + ), + # Optional arg 'arch' not specified, thus following optional arg 'manifest_id' is ignored + ( + { + "osbuild_ref": "abcdef123456", + "runner_distro": "fedora-41", + "distro": "fedora-41", + "manifest_id": "abc123" + }, + 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): + 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 a3be07eaa5..f9a99d465f 100755 --- a/test/scripts/upload-results +++ b/test/scripts/upload-results @@ -38,7 +38,7 @@ def main(): build_info["pr"] = pr_number.removeprefix("PR-") testlib.write_build_info(build_dir, build_info) - s3url = testlib.gen_build_info_s3(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])