Conversation
|
Images are ready for the commit at d885375. To use with deploy scripts, first |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #17406 +/- ##
=======================================
Coverage 49.25% 49.25%
=======================================
Files 2735 2735
Lines 206138 206138
=======================================
+ Hits 101539 101540 +1
+ Misses 97051 97050 -1
Partials 7548 7548
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
390b723 to
89a6e2e
Compare
|
@msugakov I used claude to format and test this and finally generate a commit message. It has quite good idea about using chroot that I have not thought about. Look at my previous attempts so you'd now that I spent some time working on that and I'm happy that I finally have something that is working although I need to fix some issues with scanner but that are separated PRs. |
|
Following up in Slack DM https://redhat-internal.slack.com/archives/C06750NAYSG/p1761213744579499 |
The scanner-v4-db container was crashing with exit code 127 (command not found) because the migration to ubi8-micro removed essential shell utilities that the entrypoint scripts depend on. Root cause: - docker-entrypoint.sh uses #!/usr/bin/env bash - ubi8-micro has no utilities pre-installed (unlike ubi8-minimal) - The chroot commands for user creation need /bin/sh, id, etc. This fix adds the missing packages that PR #17406 correctly included for the main image: - bash: Required for entrypoint scripts - coreutils: Basic commands (id, mkdir, cat, etc.) - findutils: File operations - util-linux: System utilities These packages enable the existing chroot user creation and locale setup commands to execute successfully. Fixes: ROX-30858 Related: #17406 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The scanner-v4-indexer and scanner-v4-matcher containers were crashing immediately with exit code 1 because the migration to ubi8-micro removed essential shell utilities that the entrypoint scripts depend on. Root cause: - entrypoint.sh uses #!/usr/bin/env bash - ubi8-micro has no utilities pre-installed (unlike ubi8-minimal) - The container fails immediately when trying to execute the bash script This fix applies the same multi-stage build pattern used in: - PR #17406 for the main image - commit cc55af9 for scanner-v4-db Changes: 1. Added dependency_builder stage using ubi8 (full) 2. Install bash, coreutils, findutils, util-linux, ca-certificates to /out/ 3. Copy dependencies from builder to ubi8-micro final stage 4. Removed microdnf/rpm operations from final stage (not available in ubi8-micro) 5. Changed BASE_IMAGE from ubi8-minimal to ubi8-micro This enables the entrypoint scripts to execute successfully while maintaining the minimal footprint of ubi8-micro. Fixes: ROX-30858 Related: #17406, #17430 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
/retest |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The main rhel Dockerfile removed the HEALTHCHECK entirely; if Central still expects an in-container liveness check, consider reintroducing an equivalent healthcheck that works without curl (or using a static curl binary) rather than dropping it.
- The
package_installerlogic and package lists for ubi8-micro are now duplicated betweenimage/rhel/Dockerfileandimage/rhel/konflux.Dockerfile; factoring this into a shared build fragment or aligning the package lists in one place would reduce future drift and simplify maintenance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The main rhel Dockerfile removed the HEALTHCHECK entirely; if Central still expects an in-container liveness check, consider reintroducing an equivalent healthcheck that works without curl (or using a static curl binary) rather than dropping it.
- The `package_installer` logic and package lists for ubi8-micro are now duplicated between `image/rhel/Dockerfile` and `image/rhel/konflux.Dockerfile`; factoring this into a shared build fragment or aligning the package lists in one place would reduce future drift and simplify maintenance.
## Individual Comments
### Comment 1
<location path="image/rhel/Dockerfile" line_range="100" />
<code_context>
ENTRYPOINT ["/stackrox/roxctl"]
-
-HEALTHCHECK CMD curl --insecure --fail https://127.0.0.1:8443/v1/ping
</code_context>
<issue_to_address>
**question (bug_risk):** Dropping the HEALTHCHECK removes built‑in liveness monitoring for Central.
This image previously defined a `HEALTHCHECK` using `curl` on `/v1/ping`, which runtimes and deployment manifests may depend on for health reporting. With it removed and no replacement, container health won’t be surfaced at the image level. Unless equivalent checks are guaranteed elsewhere (e.g., Kubernetes liveness/readiness probes mirroring this endpoint), please consider restoring a health check or providing an alternative mechanism.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
msugakov
left a comment
There was a problem hiding this comment.
Made the first pass. Will likely find more in subsequent iterations.
| COPY --from=go-builder /go/src/github.com/stackrox/rox/app/image/rhel/static-bin/* /out/stackrox/ | ||
| RUN chroot /out /stackrox/save-dir-contents /etc/pki/ca-trust /etc/ssl |
There was a problem hiding this comment.
This could be copied and executed (without chroot) in the final stage. Is there a reason to have it here?
There was a problem hiding this comment.
My idea was to not add unnecessary layers in the final image
There was a problem hiding this comment.
You don't need to worry about layers in the Konflux image. They get all squashed.
You also don't need to worry about layers in the GHA-built image, we did not attempt to reduce them historically.
Co-authored-by: Misha Sugakov <537715+msugakov@users.noreply.github.com>
Co-authored-by: Misha Sugakov <537715+msugakov@users.noreply.github.com>
Co-authored-by: Misha Sugakov <537715+msugakov@users.noreply.github.com>
msugakov
left a comment
There was a problem hiding this comment.
Sharing some thoughts from looking at the inter-diff
| findutils \ | ||
| openssl \ | ||
| postgresql \ | ||
| util-linux && \ |
There was a problem hiding this comment.
What's with util-linux in konflux.Dockerfile, is it needed?
Similar to #17406 (comment)
| RUN mkdir -p /out/stackrox && \ | ||
| mkdir -p /out/etc/pki/ca-tr | ||
| FROM registry.access.redhat.com/ubi9/ubi-micro:latest@sha256:093a704be0eaef9bb52d9bc0219c67ee9db13c2e797da400ddb5d5ae6849fa10 AS ubi-micro-base | ||
|
|
||
| FROM registry.access.redhat.com/ubi9/ubi:latest@sha256:6ed9f6f637fe731d93ec60c065dbced79273f1e0b5f512951f2c0b0baedb16ad AS package_installerust/source/anchors /out/etc/ssl && \ | ||
| mkdir -p /out/var/lib/stackrox /out/var/log/stackrox /out/var/cache/stackrox && \ | ||
| chown -R 4000:4000 /out/etc/pki/ca-trust /out/etc/ssl /out/var/lib/stackrox /out/var/log/stackrox /out/var/cache/stackrox /out/tmp |
There was a problem hiding this comment.
Look what a mess we have here. I think, committing suggestions from GitHub UI is not safe for the time being.
| COPY --from=go-builder /go/src/github.com/stackrox/rox/app/image/rhel/static-bin/* /out/stackrox/ | ||
| RUN chroot /out /stackrox/save-dir-contents /etc/pki/ca-trust /etc/ssl |
There was a problem hiding this comment.
You don't need to worry about layers in the Konflux image. They get all squashed.
You also don't need to worry about layers in the GHA-built image, we did not attempt to reduce them historically.
| COPY --from=ubi-micro-base / /out/ | ||
|
|
||
| # Install packages not in ubi-micro (bash and coreutils-single already present) | ||
| # Install packages directly to /out/ using --installroot |
There was a problem hiding this comment.
Minor nitpick, but...
This comment
# Install packages directly to /out/ using --installroot
is present in konflux.Dockerfile but absent in Dockerfile although both do the same thing in this regard.
I suggest you stick with one way of doing things and do it consistently in this PR and in other ongoing PRs.
The scanner-v4-db container was crashing with exit code 127 (command not found) because the migration to ubi8-micro removed essential shell utilities that the entrypoint scripts depend on. Root cause: - docker-entrypoint.sh uses #!/usr/bin/env bash - ubi8-micro has no utilities pre-installed (unlike ubi8-minimal) - The chroot commands for user creation need /bin/sh, id, etc. This fix adds the missing packages that PR #17406 correctly included for the main image: - bash: Required for entrypoint scripts - coreutils: Basic commands (id, mkdir, cat, etc.) - findutils: File operations - util-linux: System utilities These packages enable the existing chroot user creation and locale setup commands to execute successfully. Fixes: ROX-30858 Related: #17406 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Resolves merge conflicts from master's UBI 9 upgrade (ROX-33603) while preserving the ubi-micro branch's package_installer architecture. Changes: - Upgrade final stage from ubi8/ubi-micro to ubi9/ubi-micro - Update Go builder from RHEL 8 to RHEL 9 - Change PostgreSQL installation to releasever=9 - Update image labels from rhel8 to rhel9 - Sync nodejs-20 SHA with master - Maintain package_installer pattern for rpmdb preservation The ubi-micro approach uses a package_installer stage to preserve the minimal rpmdb from ubi-micro while installing packages via --installroot, resulting in a smaller final image compared to ubi-minimal. User request: merge master into ubi-micro branch, resolving UBI 9 conflicts while maintaining ubi-micro architecture. AI-assisted merge conflict resolution.
|
/konflux-retest main-on-push |
|
/retest |
|
/konflux-retest main-on-push |
1 similar comment
|
/konflux-retest main-on-push |
|
/konflux-retest operator-bundle-on-push |
|
@janisz: The following tests 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. |
|
/konflux-retest operator-bundle-on-push |
1 similar comment
|
/konflux-retest operator-bundle-on-push |
Migrate both image/rhel/Dockerfile and image/rhel/konflux.Dockerfile from ubi9-minimal to ubi9-micro base images following the proven pattern from PR #19500 (roxctl migration). Changes: - Use multi-stage build with package_installer pattern - Install packages to /out/ using dnf --installroot - Preserve ubi9-micro rpmdb by copying before package installation - Move directory setup and save-dir-contents to package_installer stage - Remove HEALTHCHECK from Dockerfile (curl not available in ubi9-micro) - Pin SHA digests in konflux.Dockerfile for reproducible builds - Use --setopt=reposdir=/etc/yum.repos.d for Cachi2 compatibility Expected benefits: - 30-35% image size reduction (from ~450MB to ~350MB) - Smaller attack surface and reduced CVE exposure - Faster image pull/push operations This migration maintains full functionality while following the pattern established in PR #17406 and successfully merged in PR #19500. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
Migrate both image/rhel/Dockerfile and image/rhel/konflux.Dockerfile from ubi9-minimal to ubi9-micro base images following the proven pattern from PR #19500 (roxctl migration). Changes: - Use multi-stage build with package_installer pattern - Install packages to /out/ using dnf --installroot - Preserve ubi9-micro rpmdb by copying before package installation - Move directory setup and save-dir-contents to package_installer stage - Remove HEALTHCHECK from Dockerfile (curl not available in ubi9-micro) - Pin SHA digests in konflux.Dockerfile for reproducible builds - Use --setopt=reposdir=/etc/yum.repos.d for Cachi2 compatibility Expected benefits: - 30-35% image size reduction (from ~450MB to ~350MB) - Smaller attack surface and reduced CVE exposure - Faster image pull/push operations This migration maintains full functionality while following the pattern established in PR #17406 and successfully merged in PR #19500. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
This commit migrates the main container image from ubi-minimal (~92 MB) to ubi-micro (~28 MB) base image, reducing the final image size and improving security posture by minimizing the attack surface.
Changes
Build Architecture
Related Work
This continues the image optimization efforts:
Testing
Then check in UI if everything is healthy and followed and check if scanner is working