ART-19543: Refactor extensions build to use dnf download instead of rpm-ostree compose#1945
Conversation
|
@locriandev: This pull request references ART-19543 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: locriandev The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
I would prefer to keep the split between the code/script doing the download and the configuration file with the extensions for each varsion/variant. This is mentioned in:
The script could be a dnf download function with all the parameters and a loop over the entries in the YAML config. |
c608390 to
f9899d5
Compare
dustymabe
left a comment
There was a problem hiding this comment.
A step in the right direction. I can see a vision for how we can achieve the goal here cleanly.
Here's an initial round of review, but I'll have more questions as we go.
| packages=$(jq -r ".extensions[\"${extension_name}\"].packages[]" "$extensions_json" | while read -r pkg; do | ||
| # Perform variable substitution | ||
| eval "echo \"$pkg\"" | ||
| done) |
There was a problem hiding this comment.
not sure why we need the while loop here, but can probably fix it later.
| # Get kernel version for packages that need to match the base kernel | ||
| # Include epoch (0:) so dnf can disambiguate name from version in NEVRA format | ||
| kernel_evr=$(rpm -q --queryformat '%{VERSION}-%{RELEASE}' kernel) |
There was a problem hiding this comment.
I think we should just versionlock everything in the node image and not worry about kernel_evr substitution here.
i.e I think a dnf versionlock should do it like we do for the node image:
Lines 35 to 36 in d75a447
| # Download the packages | ||
| if [ -n "$packages" ]; then | ||
| echo "Downloading extension: ${extension_name}" | ||
| dnf --repo="${YUM_REPO_NAMES}" download --resolve --alldeps \ |
There was a problem hiding this comment.
I'm not sure we need --alldeps here.
When you do a build with this does glibc and glibc-devel get downloaded? or just glibc-devel?
| done) | ||
|
|
||
| # Download the packages | ||
| if [ -n "$packages" ]; then |
There was a problem hiding this comment.
If there were no packages to download we should error.
| fi | ||
|
|
||
| # Download the extension packages | ||
| download_extension "$extension" |
There was a problem hiding this comment.
I would argue that we should only call dnf download once instead of multiple times, but we can fix that later.
| @@ -0,0 +1,2 @@ | |||
| # The names of the yum repos to use for the extensions image build. | |||
| YUM_REPO_NAMES=rhel-10.2-baseos,rhel-10.2-appstream,rhel-10.2-server-ose-5.0,rhel-10.2-highavailability,rhel-10.2-fast-datapath,rhel-10.2-nfv | |||
There was a problem hiding this comment.
Since f6d3dba we use the top level build args file for extensions image builds too so let's not create a new build-args file here. Let's add this info there and use a different var name:
EXTENSIONS_YUM_REPO_NAMES=
?
rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
f9899d5 to
7fb5588
Compare
|
@locriandev: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Summary
Refactors the extensions build process to use
dnf downloaddirectly instead ofrpm-ostree compose extensions, improving flexibility and simplifying the build process for RHEL 10.2 and other OS streams.Changes
Build Script Refactor (
extensions/build.sh)rpm-ostree compose extensionsto directdnf downloadcommandsYUM_REPO_NAMESbuild arg to specify repos at build time--resolve --alldepsNew Build Configuration Files
build-args-5.0-9.8.conf: Repo names for RHEL 9.8build-args-5.0-10.2.conf: Repo names for RHEL 10.2build-args-5.0-c10s.conf: Repo names for CentOS Stream 10Containerfile Update
ARG YUM_REPO_NAMES=overriddento accept repo names as build argumentRemoved Files
extensions/rhel-9.8.yamlextensions/rhel-10.2.yamlextensions/centos-10.yaml