Include osbuild NEVRA in build info path for test cache #432

achilleas-k · 2024-02-08T14:05:09Z

Until now, the following scenario would produce unnecessary rebuilds:

A PR is opened that updates osbuild in Schutzfile to vNew.
A manifest is generated with id abc123.
The build info lookup for abc123 shows that it was built with vOld so it's added to the test matrix.
The new build info is uploaded showing abc123 was built with vNew.
A different PR runs CI with vOld (before the update PR is merged or before this PR is rebased).
A manifest is generated with id abc123 again.
The build info lookup for abc123 shows that it was built with vNew (in the osbuild update PR) but the currently installed version is vOld (!= vNew) so it's added to the test matrix.
The new build info is uploaded showing abc123 was built with vOld.

This doesn't just create an unnecessary rebuild, but can create a situation where every image is rebuilt multiple times a day if the two PRs keep getting amended, essentially fighting each other by changing the commit ID back and forth.

This commit changes the build info paths to include the osbuild package version (NEVRA) as a path component:

s3://images-ci-cache/images/builds/fedora-40/aarch64/osbuild-104-1.fc39.noarch/abc123

The build filter now no longer needs to read the osbuild commit ID from the info.json and compare with the current one. It can simply check the path with the package version component and if it is not found, the new configuration will be tested.

Note that this PR will of course create a mass rebuild of all the images. All PRs should be rebased on this when it's merged to avoid pushing images and build info to the old paths.
The old paths should be cleaned up by the bucket retention policy in a few days.

test/scripts/imgtestlib.py

ochosi

Avoiding unnecessary builds - yay! 💖
The logic is sound and the path construction in the code looks plausible!

thozza · 2024-02-09T09:08:03Z

Please correct me if I'm wrong, but we may end up not rebuilding images if the osbuild commit SHA is updated to a newer version, but that version still has the same NEVRA as the current commit SHA (e.g. both are from main and part of an unreleased version of osbuild), right? Which is not ideal, no?

test/scripts/imgtestlib.py

achilleas-k · 2024-02-09T09:55:06Z

Please correct me if I'm wrong, but we may end up not rebuilding images if the osbuild commit SHA is updated to a newer version, but that version still has the same NEVRA as the current commit SHA (e.g. both are from main and part of an unreleased version of osbuild), right? Which is not ideal, no?

The RPMs we build for unreleased versions of osbuild contain the short-SHA in their NEVRA, so this shouldn't be an issue. I believe this comes automatically from the %{?dist} tag in the spec file.
Here's a build log that shows the path that's created for the results:
https://gitlab.com/redhat/services/products/image-builder/ci/images/-/jobs/6128641712

Uploading build/fedora_39-x86_64-ami-empty to s3://images-ci-cache/images/builds/fedora-39/x86_64/osbuild-107-1.20190819git6e12f08.fc39.noarch/4f1a882beb4de10b802ea5f424603696fb987cf769779d6ff1d7cf56a5994ec8/

Returns the installed rpm version for osbuild.

Helper functions for generating paths will help us avoid inconsistencies when changing the way we create or look up paths for build metadata.

Until now, the following scenario would produce unnecessary rebuilds: - A PR is opened that updates osbuild in `Schutzfile` to `vNew`. - A manifest is generated with id `abc123`. - The build info lookup for `abc123` shows that it was built with `vOld` so it's added to the test matrix. - The new build info is uploaded showing `abc123` was built with `vNew`. - A different PR runs CI with `vOld` (before the update PR is merged or before this PR is rebased). - A manifest is generated with id `abc123` again. - The build info lookup for `abc123` shows that it was built with `vNew` (in the osbuild update PR) but the currently installed version is `vOld` (`!= vNew`) so it's added to the test matrix. - The new build info is uploaded showing `abc123` was built with `vOld`. This commit changes the build info paths to include the osbuild package version (NEVRA) as a path component: s3://images-ci-cache/images/builds/fedora-40/aarch64/osbuild-104-1.fc39.noarch/abc123 The build filter now no longer needs to read the osbuild commit ID from the info.json and compare with the current one. It can simply check the path with the package version component and if it is not found, the new configuration will be tested.

Test the path generators with a mocked get_osbuild_nevra() function so that they can be run without osbuild installed and on non-rpm based systems.

achilleas-k · 2024-02-09T10:04:38Z

Please correct me if I'm wrong, but we may end up not rebuilding images if the osbuild commit SHA is updated to a newer version, but that version still has the same NEVRA as the current commit SHA (e.g. both are from main and part of an unreleased version of osbuild), right? Which is not ideal, no?

The RPMs we build for unreleased versions of osbuild contain the short-SHA in their NEVRA, so this shouldn't be an issue. I believe this comes automatically from the %{?dist} tag in the spec file. Here's a build log that shows the path that's created for the results: https://gitlab.com/redhat/services/products/image-builder/ci/images/-/jobs/6128641712
Uploading build/fedora_39-x86_64-ami-empty to s3://images-ci-cache/images/builds/fedora-39/x86_64/osbuild-107-1.20190819git6e12f08.fc39.noarch/4f1a882beb4de10b802ea5f424603696fb987cf769779d6ff1d7cf56a5994ec8/

Funny. The 20190819 seems to be a date stamp but why is it in 2019? Building an rpm locally I get

osbuild-108-1.20240209gita85b51b.fc39.noarch

Probably a mock thing, right?

thozza · 2024-02-09T11:08:01Z

Funny. The 20190819 seems to be a date stamp but why is it in 2019? Building an rpm locally I get
osbuild-108-1.20240209gita85b51b.fc39.noarch
Probably a mock thing, right?

I don't think so... Are you sure that the date is correct on the host which builds the RPM? 🤔

thozza

LGTM

achilleas-k · 2024-02-09T12:27:11Z

Funny. The 20190819 seems to be a date stamp but why is it in 2019? Building an rpm locally I get
osbuild-108-1.20240209gita85b51b.fc39.noarch
Probably a mock thing, right?
I don't think so... Are you sure that the date is correct on the host which builds the RPM? 🤔

This gets even weirder. Here's an example build log: https://gitlab.com/redhat/services/products/image-builder/ci/osbuild/-/jobs/6131776245

Mock start with

🎁 Building RPMs with mock
INFO: mock.py version 5.3 starting (python version = 3.11.2, NVR = mock-5.3-1.fc38), args: /usr/libexec/mock/mock -r fedora-38-x86_64 --no-bootstrap-chroot --resultdir repo/osbuild/fedora-38/x86_64/77855c9e63c8d74ca74bd3a8cd9f0afe01f8f294 rpmbuild/SRPMS/osbuild-108-1.20240208git77855c9.fc38.src.rpm

Date is correct!

Then ends with

Start: build phase for osbuild-108-1.20240208git77855c9.fc38.src.rpm
Start: build setup for osbuild-108-1.20240208git77855c9.fc38.src.rpm
Finish: build setup for osbuild-108-1.20240208git77855c9.fc38.src.rpm
Start: Outputting list of installed packages
Finish: Outputting list of installed packages
Start: rpmbuild osbuild-108-1.20240208git77855c9.fc38.src.rpm
Finish: rpmbuild osbuild-108-1.20240208git77855c9.fc38.src.rpm
Finish: build phase for osbuild-108-1.20240208git77855c9.fc38.src.rpm
INFO: Done(rpmbuild/SRPMS/osbuild-108-1.20240208git77855c9.fc38.src.rpm) Config(fedora-38-x86_64) 1 minutes 37 seconds
INFO: Results and/or logs in: repo/osbuild/fedora-38/x86_64/77855c9e63c8d74ca74bd3a8cd9f0afe01f8f294
INFO: Cleaning up build root ('cleanup_on_success=True')
Start: clean chroot
Finish: clean chroot
Finish: run

Date still correct. But:

⛓️ Creating dnf repository
Directory walk started
Directory walk done - 9 packages
Temporary output repo path: repo/osbuild/fedora-38/x86_64/77855c9e63c8d74ca74bd3a8cd9f0afe01f8f294/.repodata/
Preparing sqlite DBs
Pool started (with 5 workers)
Pool finished
☁ Uploading RPMs to S3
~/~/builds/redhat/services/products/image-builder/ci/osbuild/repo ~/~/builds/redhat/services/products/image-builder/ci/osbuild
upload: './osbuild/fedora-38/x86_64/77855c9e63c8d74ca74bd3a8cd9f0afe01f8f294/osbuild-108-1.20190819git77855c9.fc38.noarch.rpm' -> 's3://osbuild-composer-repos/osbuild/fedora-38/x86_64/77855c9e63c8d74ca74bd3a8cd9f0afe01f8f294/osbuild-108-1.20190819git77855c9.fc38.noarch.rpm' (135573 bytes in 0.2 seconds, 738.56 KB/s) [1 of 16]
Public URL of the object is: http://osbuild-composer-repos.s3.amazonaws.com/osbuild/fedora-38/x86_64/77855c9e63c8d74ca74bd3a8cd9f0afe01f8f294/osbuild-108-1.20190819git77855c9.fc38.noarch.rpm

Date changes during createrepo_c??

achilleas-k · 2024-02-09T12:27:54Z

AH WAIT. I see it now. It's the last date in the %changelog in the spec file 🤦

achilleas-k requested review from ondrejbudai, mvo5 and thozza February 8, 2024 14:05

test: docstring for get_osbuild_commit()

7eebec0

achilleas-k force-pushed the build-info-path/osbuild-ver branch from 6bf9491 to a4f46b7 Compare February 8, 2024 15:35

mvo5 reviewed Feb 8, 2024

View reviewed changes

test/scripts/imgtestlib.py Show resolved Hide resolved

mvo5 self-requested a review February 8, 2024 20:04

ochosi previously approved these changes Feb 9, 2024

View reviewed changes

ondrejbudai reviewed Feb 9, 2024

View reviewed changes

test/scripts/imgtestlib.py Outdated Show resolved Hide resolved

achilleas-k added 4 commits February 9, 2024 11:00

test: new function: get_osbuild_nevra()

d06a9e9

Returns the installed rpm version for osbuild.

test: helper functions for generating buildinfo paths

87e2397

Helper functions for generating paths will help us avoid inconsistencies when changing the way we create or look up paths for build metadata.

test: add path generator unit tests

ed75a0b

Test the path generators with a mocked get_osbuild_nevra() function so that they can be run without osbuild installed and on non-rpm based systems.

achilleas-k dismissed ochosi’s stale review via ed75a0b February 9, 2024 10:00

achilleas-k force-pushed the build-info-path/osbuild-ver branch from a4f46b7 to ed75a0b Compare February 9, 2024 10:00

thozza approved these changes Feb 9, 2024

View reviewed changes

ochosi added this pull request to the merge queue Feb 9, 2024

Merged via the queue into osbuild:main with commit 4162456 Feb 9, 2024
12 checks passed

achilleas-k deleted the build-info-path/osbuild-ver branch February 9, 2024 15:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include osbuild NEVRA in build info path for test cache #432

Include osbuild NEVRA in build info path for test cache #432

achilleas-k commented Feb 8, 2024 •

edited

Loading

ochosi left a comment

thozza commented Feb 9, 2024

achilleas-k commented Feb 9, 2024

achilleas-k commented Feb 9, 2024 •

edited

Loading

thozza commented Feb 9, 2024

thozza left a comment

achilleas-k commented Feb 9, 2024

achilleas-k commented Feb 9, 2024

Include osbuild NEVRA in build info path for test cache #432

Include osbuild NEVRA in build info path for test cache #432

Conversation

achilleas-k commented Feb 8, 2024 • edited Loading

ochosi left a comment

Choose a reason for hiding this comment

thozza commented Feb 9, 2024

achilleas-k commented Feb 9, 2024

achilleas-k commented Feb 9, 2024 • edited Loading

thozza commented Feb 9, 2024

thozza left a comment

Choose a reason for hiding this comment

achilleas-k commented Feb 9, 2024

achilleas-k commented Feb 9, 2024

achilleas-k commented Feb 8, 2024 •

edited

Loading

achilleas-k commented Feb 9, 2024 •

edited

Loading