blueprints: (optionally) include blueprint repos in 2nd depsolve #1218

mvo5 · 2025-02-14T17:33:38Z

This commit allows to (optionally) include the custom repositories from the blueprints as part of the depsolving when installing the user packages.

I'm a bit unsure about the best way to go about a full end-to-end test, I'm not super familiar with the existing integration test scripts in "images", I was thinking to do the full end-to-end test with image-builder-cli but maybe that is silly, ideas very welcome.

The amount of duplication in between rhel/fedora in the pkg/distro is also something I'm not happy about but it seems that is the current design but I would love to get back to this and see we can do something about it.

Thanks to Ondrej for raising this.

achilleas-k · 2025-02-17T21:20:04Z

The amount of duplication in between rhel/fedora in the pkg/distro is also something I'm not happy about but it seems that is the current design but I would love to get back to this and see we can do something about it.

Indeed, the ImageType.Manifest() function is probably identical for all implementations of the interface (in each distro). In cases where it's not, it's probably by accident, or because a specific distro doesn't support something (like RHEL 7), but that latter issue could be handled differently.

achilleas-k · 2025-02-17T21:27:43Z

I'm a bit unsure about the best way to go about a full end-to-end test

Yeah, it's a bit tricky. The obvious thing to do is to add a configuration with a repository in the blueprint to test/configs/, but it would have to be a repository that's not used already. The build tests all use the repositories from test/data/repositories/ and there's no straightforward way to drop one.

We could do something a bit "silly". If you look at the test repository configurations, some of them have these image_type_tags, which limits the use of a repository to just those image types that are listed. This is done so that we don't use, for example, google-cloud-sdk repositories when building an image for any other platform. So one thing we could do is add a build config (blueprint) that adds the google-cloud-sdk repository to a qcow2 build (or any other build, really) and also selects one of the google-cloud-sdk packages for installation. One issue with this though is that the configuration wont be updated automatically when our repository configurations are updated after new snapshots are created.
If we do this for a GA distro version (RHEL 9.5 or older), it should be okay though.

thozza · 2025-02-18T05:49:56Z

The amount of duplication in between rhel/fedora in the pkg/distro is also something I'm not happy about but it seems that is the current design but I would love to get back to this and see we can do something about it.

Indeed, the ImageType.Manifest() function is probably identical for all implementations of the interface (in each distro). In cases where it's not, it's probably by accident, or because a specific distro doesn't support something (like RHEL 7), but that latter issue could be handled differently.

+1

It used to be much worse - duplication for every RHEL major version 😅

We can unify Fedora and RHEL. The code can be written to support the super-set of all features. It does not matter if there are bits specific to RHEL-7, that Fedora does not use.

thozza · 2025-02-18T05:54:27Z

I'm a bit unsure about the best way to go about a full end-to-end test

Indeed, it's a bit tricky. The obvious thing to do is to add a configuration with a repository in the blueprint to test/configs/, but it would have to be a repository that's not used already. The build tests all use the repositories from test/data/repositories/ and there's no straightforward way to drop one.

I have two proposals:

Snapshotting and using the EPEL repository for this. We never install packages from EPEL.
An alternative would be our own COPR repository with a dummy package.

Both are pretty straightforward. EPEL repo would be usable only for a specific version of RHEL major version, so COPR is the most universal approach.

achilleas-k · 2025-02-18T16:58:17Z

2. An alternative would be our own COPR repository with a dummy package.

I was re-thinking my comment and had a similar thought. A custom repo with 2-3 dummy packages would be useful for all sorts of tests actually. We could use it for quick and dirty depsolve tests for example in all our projects, like in osbuild-composer where we create one locally each time.

EDIT: Elaborating on the "2-3 packages" part: If we make packages with dependencies, it would be a nice, simple depsolver test repo too.

thozza

Nice and simple. I've added two (or really just one) suggestions...

thozza · 2025-02-25T14:30:31Z

pkg/blueprint/repository_customizations.go

+	// When set the repository will be used during the depsolve of
+	// payload repositories
+	EnabledDuringBuild bool `json:"enabled_during_build" toml:"enabled_during_build"`


Nitpick: would maybe suggest active form enable_during_built or something shorter such as install_from. I can live with the current name as well 😅

I have no strong opinion here, I like your suggested and used install_from now as it is shorter but I'm open for more ideas (I had a hard time picking a good name)

thozza · 2025-02-26T09:03:31Z

pkg/blueprint/repository_customizations.go

@@ -76,6 +80,17 @@ func (rc *RepositoryCustomization) getFilename() string {
 	return rc.Filename
 }

+func RepoCustomizationsFilterEnabledDuringBuild(repos []RepositoryCustomization) []RepositoryCustomization {


Would it make sense to return ([]rpmmd.RepoConfig, error) from the function (basically just calling .customRepoToRepoConfig() on the repo)? Calling RepoCustomizationsToRepoConfigAndGPGKeyFiles() seems to do much more than just conversion and the resulting []*fsnode.File is always discarded for these repos.

Thanks, that is a nice one - much simpler this way

This commit allows to (optionally) include the custom repositories from the blueprints as part of the depsolving when installing the user packages. Thanks to Ondrej for suggesting this.

This commit moves the TestManifestRepositoryCustomization into the distro package to avoid the previous duplication. The test location is not ideal, but either we duplicate this test in rhel/imagetype_test.go and fedora/imagetype_test.go or we have one version here that has knowledge about the details of how images types are implemented in {fedroa,rhel}/imagetype.go (i.e. how workloads and payload repos get added during the imagetype.Manifest() call).

schutzbot · 2025-02-26T14:48:48Z

This PR changes the images API or behaviour causing integration failures with osbuild-composer. The next update of the images dependency in osbuild-composer will need work to adapt to these changes.

This is simply a notice. It will not block this PR from being merged.

mvo5 force-pushed the blueprint-2nd-depsolve-user-repos branch 2 times, most recently from 91728e2 to 8e9fc73 Compare February 17, 2025 20:09

mvo5 changed the title ~~[RFC] blueprints: (optinally) include blueprint repos in 2nd depsolve~~ blueprints: (optinally) include blueprint repos in 2nd depsolve Feb 17, 2025

mvo5 requested review from ondrejbudai and achilleas-k February 17, 2025 20:12

mvo5 force-pushed the blueprint-2nd-depsolve-user-repos branch from 8e9fc73 to eb07964 Compare February 17, 2025 20:13

mvo5 marked this pull request as ready for review February 17, 2025 20:16

mvo5 requested a review from a team as a code owner February 17, 2025 20:16

mvo5 requested review from thozza and schuellerf February 17, 2025 20:16

achilleas-k changed the title ~~blueprints: (optinally) include blueprint repos in 2nd depsolve~~ blueprints: (optoinally) include blueprint repos in 2nd depsolve Feb 18, 2025

achilleas-k changed the title ~~blueprints: (optoinally) include blueprint repos in 2nd depsolve~~ blueprints: (optionally) include blueprint repos in 2nd depsolve Feb 18, 2025

thozza reviewed Feb 26, 2025

View reviewed changes

mvo5 added 2 commits February 26, 2025 15:40

blueprints: (optinally) include blueprint repos in 2nd depsolve

1f383aa

This commit allows to (optionally) include the custom repositories from the blueprints as part of the depsolving when installing the user packages. Thanks to Ondrej for suggesting this.

mvo5 force-pushed the blueprint-2nd-depsolve-user-repos branch from 49b43b0 to 7b373aa Compare February 26, 2025 14:41

mvo5 requested a review from thozza February 26, 2025 14:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

blueprints: (optionally) include blueprint repos in 2nd depsolve #1218

blueprints: (optionally) include blueprint repos in 2nd depsolve #1218

mvo5 commented Feb 14, 2025 •

edited

Loading

achilleas-k commented Feb 17, 2025

achilleas-k commented Feb 17, 2025 •

edited

Loading

thozza commented Feb 18, 2025

thozza commented Feb 18, 2025 •

edited

Loading

achilleas-k commented Feb 18, 2025 •

edited

Loading

thozza left a comment

thozza Feb 25, 2025

mvo5 Feb 26, 2025

thozza Feb 26, 2025

mvo5 Feb 26, 2025

schutzbot commented Feb 26, 2025

blueprints: (optionally) include blueprint repos in 2nd depsolve #1218

Are you sure you want to change the base?

blueprints: (optionally) include blueprint repos in 2nd depsolve #1218

Conversation

mvo5 commented Feb 14, 2025 • edited Loading

achilleas-k commented Feb 17, 2025

achilleas-k commented Feb 17, 2025 • edited Loading

thozza commented Feb 18, 2025

thozza commented Feb 18, 2025 • edited Loading

achilleas-k commented Feb 18, 2025 • edited Loading

thozza left a comment

Choose a reason for hiding this comment

thozza Feb 25, 2025

Choose a reason for hiding this comment

mvo5 Feb 26, 2025

Choose a reason for hiding this comment

thozza Feb 26, 2025

Choose a reason for hiding this comment

mvo5 Feb 26, 2025

Choose a reason for hiding this comment

schutzbot commented Feb 26, 2025

mvo5 commented Feb 14, 2025 •

edited

Loading

achilleas-k commented Feb 17, 2025 •

edited

Loading

thozza commented Feb 18, 2025 •

edited

Loading

achilleas-k commented Feb 18, 2025 •

edited

Loading