Skip to content

Commit

Permalink
imgtestlib/gen_build_info_*: don't require osbuild_ref and runner_distro
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
thozza committed Jan 9, 2025
1 parent a3c8d69 commit f295ad7
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 25 deletions.
4 changes: 1 addition & 3 deletions test/scripts/check-build-coverage
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 1 addition & 3 deletions test/scripts/generate-ostree-build-config
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 21 additions & 14 deletions test/scripts/imgtestlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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:
Expand All @@ -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.
Expand All @@ -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),
)


Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
47 changes: 45 additions & 2 deletions test/scripts/test_imgtestlib.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import os
import subprocess as sp
import tempfile
from unittest.mock import patch

import pytest

Expand Down Expand Up @@ -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", (
Expand Down Expand Up @@ -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"
Expand Down
4 changes: 1 addition & 3 deletions test/scripts/upload-results
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down

0 comments on commit f295ad7

Please sign in to comment.