Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

many: update for new reporegistry.New() api (c.f. pr#1179) #4603

Merged
merged 6 commits into from
Feb 19, 2025

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Feb 6, 2025

This commit updates osbuild-composer for the new API in images for the reporegistry.New(). The main incompatible change is that the /repositories part is not longer automatically added inside the library so we need to add it on the call-site.

This needs osbuild/images#1179

@mvo5 mvo5 force-pushed the update-api-for-images-pr1179 branch from 2839201 to 4c84c2b Compare February 10, 2025 12:08
@mvo5 mvo5 marked this pull request as ready for review February 10, 2025 12:08
@mvo5 mvo5 requested a review from a team as a code owner February 10, 2025 12:08
@mvo5 mvo5 requested review from thozza, schuellerf and supakeen and removed request for a team February 10, 2025 12:08
supakeen
supakeen previously approved these changes Feb 10, 2025
internal/cloudapi/v2/compose_test.go Outdated Show resolved Hide resolved
internal/cloudapi/v2/compose_test.go Outdated Show resolved Hide resolved
internal/cloudapi/v2/v2_test.go Show resolved Hide resolved
thozza
thozza previously approved these changes Feb 11, 2025
Copy link
Member

@thozza thozza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM.

@schuellerf schuellerf force-pushed the update-api-for-images-pr1179 branch from 7b6d19e to c53ac1b Compare February 12, 2025 10:49
@mvo5 mvo5 force-pushed the update-api-for-images-pr1179 branch from c53ac1b to 1c64f80 Compare February 13, 2025 12:34
@thozza
Copy link
Member

thozza commented Feb 14, 2025

@mvo5, there is now a merge conflict. Can you please take a look? Thanks!

@mvo5 mvo5 force-pushed the update-api-for-images-pr1179 branch 2 times, most recently from e9ae4d0 to 3b40db1 Compare February 14, 2025 11:09
@mvo5 mvo5 requested a review from thozza February 14, 2025 13:56
@mvo5
Copy link
Contributor Author

mvo5 commented Feb 14, 2025

@thozza Sorry, this grew again, the removal of fedora-39 from images caused another test failure in the cross-arch.sh so I had to bump images to the current version and vendor all dependencies etc.

mvo5 added 3 commits February 18, 2025 09:52
This commit updates osbuild-composer for the new API in images
for the `reporegistry.New()`. The main incompatible change is
that the `/repositories` part is not longer automatically added
inside the library so we need to add it on the call-site.

This needs osbuild/images#1179
Add more information when the cross-distro.sh test fails. Currently
it prints:
```
DEBUG: ===== ALL_DISTROS ====
rhel-8
...
rhel-8
DEBUG: ===== ALL_EXPECTED_DISTROS ====
fedora-40
...
fedora-42
DEBUG: ===== ALL_REMAINDERS ====
rhel-8
....
rhel-8
DEBUG: ===== END ====
Some distros are missing!
Missing distros:
```
But the most crucial information (i.e. what is installed) is
missing from this debug print (it can be found in a different
output but lets make it easy).
@mvo5 mvo5 force-pushed the update-api-for-images-pr1179 branch from 3b40db1 to 78a06d1 Compare February 18, 2025 08:52
mvo5 added 2 commits February 18, 2025 15:52
This commit updates to images v0.117.0 so that the cross-distro.sh
test works again (images removed fedora-39.json in main but the
uses the previous version of images that includes fedora-39 so
there is a mismatch (we should look into if there is a way to
get github.com/osbuild/images@latest instead of main in the
cross-arch test).

It also updates all the vendor stuff that got pulled via the
new images release (which is giantonormous).

This update requires updating the Go version to 1.22.8
This commit fixes the failing TestGetImageRequests_BlueprintDistro
test. It fails because fedora-39 is no longer part of the supported
distros and can no longer be build. Move to fedora-42 so that we
have a bit time before we need to update it again.

It also updates TestGetImageRequests_NoRepositories to point to
fedora-42.
@achilleas-k achilleas-k force-pushed the update-api-for-images-pr1179 branch from 78a06d1 to b53bd2f Compare February 18, 2025 14:53
@achilleas-k
Copy link
Member

I amended the commit that updates images to v0.117.0 to also update the Go version in tools/prepare-source.sh to 1.22.8, which is required by the new dependency version.

This commit tweaks the `cross-build.sh` to checkout the last release
tag of the images library instead of using main. The issue with using
main is that there are many false positive errors when e.g. a new
fedora release is added to main.

Ideally it would use the tag of the vendored images library but
this particular test looks at the rpm content so the information
what version of images was used is not readily available (we could
fix this but the workaround of this commit is hopefully sufficient).
@achilleas-k achilleas-k enabled auto-merge (rebase) February 19, 2025 14:23
Copy link
Contributor

@bcl bcl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@achilleas-k achilleas-k merged commit 2fc64ae into osbuild:main Feb 19, 2025
47 of 50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants