Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3021 +/- ##
=======================================
Coverage 27.38% 27.38%
=======================================
Files 95 95
Lines 5427 5427
Branches 2548 2548
=======================================
Hits 1486 1486
Misses 3214 3214
Partials 727 727
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
0e2287f to
a50fa44
Compare
Replace manual package installation and file copying with proper dnf --installroot pattern. This cleaner approach installs packages directly to the target directory without manual cp commands or rpm workarounds. Benefits: - Reduced image size (20% smaller: 86MB vs 108MB) - Cleaner implementation following industry best practices - Better maintainability with proper RPM database tracking - Reduced layer count by combining COPY commands - Added .dockerignore for faster builds Changes: - Use dnf --installroot instead of microdnf + manual copying - Combine multiple COPY commands into single layer - Add .dockerignore to exclude unnecessary build context Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
msugakov
left a comment
There was a problem hiding this comment.
Most of what I commented on collector/container/Dockerfile applies to collector/container/konflux.Dockerfile also.
Changes: - Switch from ubi-minimal to ubi base image for package_installer stages - Remove dnf installation step (ubi already includes dnf) - UBI10: sha256:f573194e8e5231f1c9340c497e1f8d9aa9dbb42b2849e60341e34f50eec9477e - UBI9: sha256:cecb1cde7bda7c8165ae27841c2335667f8a3665a349c0d051329c61660a496c This improves build efficiency since we no longer need to install dnf on top of ubi-minimal, which essentially gives us ubi anyway. Addresses review comment from @msugakov on PR #3021. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Changes: - Add ubi-micro-base stage to reference the ubi-micro image - Create package_installer_final stage that copies ubi-micro base to /out - Install packages on top of the existing ubi-micro base - Use ubi-micro-base as the final runtime image base This ensures that the rpmdb in the final image correctly tracks both: 1. Packages that come with the ubi-micro base image 2. Packages we install via dnf --installroot Without this change, we were creating a new rpmdb from scratch in /out, which would replace ubi-micro's existing rpmdb and lose track of packages already present in the base image. Addresses review comment from @msugakov on PR #3021. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The collector binary requires libuuid.so.1 and libstdc++.so.6 which were present in ubi-minimal but are not included in ubi-micro. This caused collector to fail with exit code 127 and the error: "error while loading shared libraries: libuuid.so.1: cannot open shared object file: No such file or directory" Add both libuuid and libstdc++ packages to ensure the collector binary has all required runtime dependencies. Also add --allowerasing flag to handle package conflicts when installing on top of the ubi-micro base (e.g., coreutils vs coreutils-single). Verified by checking ldd output from existing collector binary: libuuid.so.1 => not found libstdc++.so.6 => not found Fixes CI test failures in integration tests. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add openssl, libuuid, and libstdc++ to rpms.in.yaml so they are prefetched by Hermeto during Konflux hermetic builds. These packages are required by the package_installer_final stage in konflux.Dockerfile but were missing from the prefetch configuration, causing the build to fail with "Could not resolve host: cdn-ubi.redhat.com" errors. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add ca-certificates package to the runtime stage in konflux.Dockerfile and to rpms.in.yaml for Konflux prefetch. This is required for SSL/TLS verification and was identified in PR review comments. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Consolidate all file copies into the package_installer stage to reduce the number of layers in the final images. This results in smaller image sizes due to better compression and a simpler build structure. Changes: - Dockerfile: 4 COPY commands → 1 (reduces 3 layers) - konflux.Dockerfile: 3 COPY commands → 1 (reduces 2 layers) All files (packages, binaries, licenses, docs) are now copied from package_installer's /out/ directory in a single layer. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add --setopt=reposdir=/etc/yum.repos.d to dnf install command in package_installer stage to ensure dnf uses the host's repo directory (where cachi2 configures RHEL repos) instead of the installroot's repo directory (which contains UBI repos from the copied ubi-micro-base). When using --installroot=/out/, dnf defaults to looking for repo configurations in /out/etc/yum.repos.d/. Since we copy ubi-micro-base to /out/, this directory contains UBI repo configurations pointing to cdn-ubi.redhat.com, which is not accessible in Konflux hermetic builds. By explicitly setting reposdir to the host's /etc/yum.repos.d/, dnf will use the cachi2-configured RHEL repos and hermetically prefetched packages, fixing the DNS resolution failure. Fixes build error: Curl error (6): Couldn't resolve host name for https://cdn-ubi.redhat.com/.../repomd.xml Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add bash, coreutils, curl, grep, and gawk to the runtime package list in konflux.Dockerfile to match the regular Dockerfile and provide necessary shared libraries and utilities. These packages were missing from the konflux.Dockerfile package_installer stage, causing runtime errors: - curl provides libcurl.so.4 (required by collector binary) - bash, coreutils, grep, gawk provide shell utilities used by scripts The packages are already present in rpms.lock.yaml (included as dependencies from other packages or previous builds), so only rpms.in.yaml needs to be updated to document the explicit dependency. Fixes error: collector: error while loading shared libraries: libcurl.so.4: cannot open shared object file: No such file or directory Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: Mauro Ezequiel Moltrasio <mmoltras@redhat.com>
Change from removing all /var/cache/* to specifically removing only /var/cache/dnf and /var/cache/yum to prevent accidentally removing needed files like ldconfig/aux-cache. This prevents potential regressions if future RHEL releases add other important files to /var/cache. Addresses review comment from @msugakov on PR #3021. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove digest pins (@sha256:...) from collector/container/Dockerfile and use floating :latest tags instead. Non-konflux images are not updated via MintMaker, so pinned digests would be set in stone forever. Only konflux.Dockerfile should use pinned digests for reproducibility. Addresses review comment from @Molter73 on PR #3021. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace the install.sh script with inline commands directly in the Dockerfile. The install scripts were meant to avoid duplicate Dockerfiles, but since we now have a dedicated dev.Dockerfile, we can inline the commands for better clarity. Addresses review comment from @Molter73 on PR #3021. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Move file copies from package_installer stage back to final stage for semantic clarity. The package_installer stage should focus on installing packages, while the final stage assembles all files. This addresses reviewer ovalenti's preference on PR #3021. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
msugakov
left a comment
There was a problem hiding this comment.
Looks good. I have nitpicks mostly, thought that's in the urge to make this change exemplary that we can replicate to other dockerfiles.
… collector pattern Based on successful collector PR stackrox/collector#3021, add the missing --allowerasing flag to the dnf install command. This flag is needed for Konflux hermetic builds to allow dnf to resolve package conflicts. The --setopt=reposdir=/etc/yum.repos.d flag was also clarified with a comment explaining that it ensures dnf uses cachi2's prefetched packages (host repos) rather than trying to download from UBI CDN. Partially generated by AI.
Co-authored-by: Misha Sugakov <537715+msugakov@users.noreply.github.com>
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
… packages The issue was that copying ubi-micro base to /out/ also copies /out/etc/yum.repos.d/ubi.repo which points to cdn-ubi.redhat.com. In Konflux hermetic builds, this causes dnf to try accessing external repos instead of using cachi2's prefetched packages. Solution: 1. Remove *.repo files from /out/etc/yum.repos.d/ after copying ubi-micro base 2. Use --setopt=reposdir=/etc/yum.repos.d to explicitly use container's repos (cachi2 in Konflux) 3. Add --allowerasing flag to allow dnf to resolve package conflicts Tested locally with test-dnf.Dockerfile which confirmed the approach works. Based on successful collector PR stackrox/collector#3021 Partially generated by AI.
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
msugakov
left a comment
There was a problem hiding this comment.
A few, hopefully final, findings.
Co-authored-by: Misha Sugakov <537715+msugakov@users.noreply.github.com>
msugakov
left a comment
There was a problem hiding this comment.
Looks quite good, but I'd suggest to wrap up adding utils too.
Description
Reduce container image size and improve security posture by migrating from UBI minimal base images to UBI micro base images.
Removed Packages
Refs:
Checklist
Automated testing
If any of these don't apply, please comment below.
Testing Performed
CI