Skip to content

fix(build): cleaner solution for s390x builds - #23196

Open
dashrews78 wants to merge 1 commit into
masterfrom
dashrews/better-s390x-fix
Open

fix(build): cleaner solution for s390x builds#23196
dashrews78 wants to merge 1 commit into
masterfrom
dashrews/better-s390x-fix

Conversation

@dashrews78

@dashrews78 dashrews78 commented Oct 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Description

#23180 was a rush job to not hose up everyone with bad builds. This is a more thoughtful approach. It undoes a lot of what #23180 did and gets us back to where most the changes are contained to the s390x section.

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

Pulled the s390x upstream image and verified the installed Postgres libraries were in fact version 16.

quay.io/rhacs-eng/main:5.1.x-144-g0643affeab-s390x

   Component                      Verified version
  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
   postgresql RPM                 16.8-1.module_el9+1265+61707298.s390x
  ─────────────────────────────  ───────────────────────────────────────
   postgresql-private-libs RPM    16.8-1.module_el9+1265+61707298.s390x
  ─────────────────────────────  ───────────────────────────────────────
   pg_dump --version              PostgreSQL 16.8
  ─────────────────────────────  ───────────────────────────────────────
   pg_restore --version           PostgreSQL 16.8

  Both clients load the PostgreSQL 16 libpq library with no missing dependencies. The image is linux/s390x, and its digest matches the CI build.

  These checks ran under s390x emulation.

@dashrews78

dashrews78 commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

This change is part of the following stack:

Change managed by git-spice.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 5969dcb8-3f11-4252-b45d-f10b4eb861ab

📥 Commits

Reviewing files that changed from the base of the PR and between e8a28be and 0643aff.

📒 Files selected for processing (3)
  • image/rhel/Dockerfile
  • image/rhel/README.md
  • image/rhel/download.sh

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Improved PostgreSQL package downloads for s390x by using the CentOS Stream 9 AppStream repository and importing its official RPM signing key. This helps ensure the required packages can be retrieved and verified on that architecture.
    • The download and runtime stages continue to use UBI across all supported architectures.

Walkthrough

The downloads stage now uses UBI on every architecture. On s390x, download.sh imports the CentOS signing key and uses the CentOS Stream 9 AppStream repository for PostgreSQL module configuration and RPM downloads. The README describes the updated setup.

Changes

RHEL image downloads

Layer / File(s) Summary
UBI stage and s390x package sources
image/rhel/Dockerfile, image/rhel/download.sh, image/rhel/README.md		 The downloads stage now derives from ubi-base and copies the CentOS Official RPM signing key. On s390x, download.sh imports the key and uses the CentOS Stream 9 AppStream repository for both dnf commands. The README reflects this setup.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 0643a

No actionable merge-blocking risk was identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the primary change: improving the s390x build solution.
Description check ✅ Passed The description explains the change and provides concrete s390x validation results. The template checkboxes remain unmarked, so CI inspection, production readiness, documentation status, and automated…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 1, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.92%. Comparing base (e8a28be) to head (0643aff).
⚠️ Report is 14 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #23196      +/-   ##
==========================================
- Coverage   51.97%   51.92%   -0.05%     
==========================================
  Files        2904     2904              
  Lines      183013   183013              
==========================================
- Hits        95112    95035      -77     
- Misses      79588    79639      +51     
- Partials     8313     8339      +26
Flag Coverage Δ
go-unit-tests 51.92% <ø> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

🚀 Build Images Ready

Images are ready for commit 0643aff. To use with deploy scripts:

export MAIN_IMAGE_TAG=5.1.x-144-g0643affeab

@dashrews78 dashrews78 added the ci-build-all-arch Build binaries and images for all architectures label Oct 1, 2026
dashrews78
dashrews78 commented Oct 1, 2026
View reviewed changes
Comment thread image/rhel/Dockerfile
FROM quay.io/centos/centos:stream9 AS downloads-s390x

FROM downloads-${TARGETARCH} AS downloads
FROM ubi-base AS downloads

@dashrews78 dashrews78 Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns to how it was before #23180

@dashrews78
dashrews78 requested a review from msugakov October 1, 2026 19:51
@dashrews78

dashrews78 commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

/retest

@red-hat-konflux

red-hat-konflux Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

All PipelineRuns for this commit have already succeeded. Use /retest <pipeline-name> to re-run a specific pipeline or /test to re-run all pipelines.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robbycochran robbycochran robbycochran approved these changes
@msugakov msugakov Awaiting requested review from msugakov

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

area/helm ci-build-all-arch Build binaries and images for all architectures

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dashrews78 @robbycochran