-
Notifications
You must be signed in to change notification settings - Fork 13
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
Fetch dep rpm updates #92
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lsm5 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
5b46bb8
to
8460ff5
Compare
Opening for review. @Luap99 @ashley-cui @baude @mheon PTAL. |
Err, never mind. spoke too soon. I see this in the logs:
|
8460ff5
to
7b14fa1
Compare
a0f528e
to
13bccc8
Compare
I think the dependency fetching looks alright now and tests pass Opening for review. |
@Luap99 Thanks for the comments. I'm probably gonna rework it to keep all koji ops outside of the container. Setting it back to draft. |
2b012b2
to
d0885cb
Compare
Good for another review. I'm keeping draft status because I need to revert PR_NUM change. |
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.
I have not looked into how this stuff works in koji but it looks reasonable to me.
podman-image/Containerfile
Outdated
rm -f crun-krun*.rpm crun-wasm*.rpm && \ | ||
dnf -y update --best --allowerasing *.rpm && \ | ||
rm -f *.rpm && \ |
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.
I don't think the removal does anything for the OCI image size as the COPY step already create a new layer AFAIK.
I guess it is still correct as we don't want "random" rpm files in the actual disk image.
I wonder if it would not be simpler to mount them instead of using COPY. I think podman build -v coul be an option or RUN --mount in the Containerfile.
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.
ack. I could look into that. Thanks
Sometimes, the latest podman release rpm might need other dependencies like containers-common updated as well. This PR will fetch dependencies directly from koji regardless of bodhi status. container-selinux and crun will be fetched from the default distro tags while aardvark-dns, containers-common, and netavark will be fetched from sidetags owned by packit using `koji download-build ...`. Signed-off-by: Lokesh Mandvekar <[email protected]>
d0885cb
to
36709a6
Compare
Signed-off-by: Lokesh Mandvekar <[email protected]>
36709a6
to
01b18c9
Compare
Sometimes, the latest podman release rpm might need other dependencies like containers-common updated as well.
This PR will fetch dependencies directly from koji regardless of bodhi status.
aardvark-dns, containers-common, crun and netavark will be fetched from default distro tags as well as sidetags owned by packit using
koji download-build ...
.NOTE: container-selinux rpm currently is facing conflicts with selinux-policy so I've skipped it for now.