-
Notifications
You must be signed in to change notification settings - Fork 50
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
Factor out common RHEL parts and RHEL-10 from RHEL-9 (COMPOSER-2185) #529
Conversation
Very nice, I believe this will also fix #491? I'll do a review tomorrow. |
rhel_common
edae9f1
to
b14b2be
Compare
Snyk is complaining about an existing thing in |
52d3435
to
d15a968
Compare
a91df58
to
e0efc38
Compare
The PR is again ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. I didn't verify every single copy of the image types line-by-line since the test manifests haven't changed.
Two minor comments in-line: a file with a mistyped name and a question about the NewImageType()
function.
SUPER NITPICK: One more thought: Could the common parts be moved to pkg/distro/rhel/
instead of being under pkg/distro/rhel/rhel_common
? I'm not sure I like the repetition in the path (though, unavoidable I guess), the name with the _
, and I kinda like the idea of the common bits being one level above the version-specific code.
So instead of:
pkg/distro/
└── rhel
├── rhel10
├── rhel9
└── rhel_common
we'd have:
pkg/distro
└── rhel
├── rhel10
└── rhel9
Thanks a lot for the review!
I don't have a strong opinion on this. I'm happy to move the common code to |
I fixed typos in filenames and moved |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this 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!
@thozza Tiny merge conflict :( |
As the first step towards defining el10 and el9 in without completely forking the `rhel9` distro, create a common `rhel` directory and move `rhel9` under it. The idea is to then extract the parts that should be shared across RHEL distros under `rhel` package and export necessary structs and functions from it and make use of them. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Create new package under `distro/rhel`, called `rhel`. The intention is to factor out all code which should be common across RHEL versions into this new package. Specific RHEL version distro implementations will then use this code and own mostly their package set and configuration. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
While extracting RHEL-10 from RHEL-9, I noticed that the test passes even if the listed image types are not supported by the distro, which didn't seem right. Fix that. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
And base it on the common `rhel` package. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Just a followup of https://issues.redhat.com/browse/COMPOSER-2185 and it's PR osbuild/images#529
Just a followup of https://issues.redhat.com/browse/COMPOSER-2185 and it's PR osbuild/images#529
Just a followup of https://issues.redhat.com/browse/COMPOSER-2185 and it's PR osbuild/images#529 also updating images and osbuild-composer repos
Just a followup of https://issues.redhat.com/browse/COMPOSER-2185 and it's PR osbuild/images#529 also updating images and osbuild-composer repos
Just a followup of https://issues.redhat.com/browse/COMPOSER-2185 and it's PR osbuild/images#529 also updating images and osbuild-composer repos
This is the first step in defining RHEL distros by reusing most of the code which is not release-specific. Each release will then define its package sets, partition tables, image configurations. The rest will be shared among distros.
I factored out the "common" RHEL code into
rhel
package, which is now used byrhel9
. Subsequently, I factored outrhel10
out ofrhel9
and reverted the conditionals inrhel9
distro code which were necessary to incorporaterhel10
.I verified that no el9 / c9s / el10 / c10s manifests have been changed by these changes.