Skip to content

WIP: Smoke test HAProxy 3.2.19 RPM#792

Open
gcs278 wants to merge 3 commits into
openshift:masterfrom
gcs278:smoke-test-haproxy32-3.2.19
Open

WIP: Smoke test HAProxy 3.2.19 RPM#792
gcs278 wants to merge 3 commits into
openshift:masterfrom
gcs278:smoke-test-haproxy32-3.2.19

Conversation

@gcs278

@gcs278 gcs278 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Smoke test for the HAProxy 3.2.19 RPM build

Summary by CodeRabbit

  • Chores
    • Upgraded HAProxy to 3.2.19 and added build-time version output for verification.
    • Removed legacy HAProxy package from the base image to reduce footprint and simplify maintenance.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-ci

openshift-ci Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 8, 2026
@gcs278 gcs278 changed the title Smoke test HAProxy 3.2.19 RPM for OCP 4.23 Smoke test HAProxy 3.2.19 RPM Jun 8, 2026
@gcs278

gcs278 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

FYI this is a "vanilla" 3.2 smoke test PR - just to get some green CI runs before integrating HAProxy 3.2 with the sidecar architecture.

/assign @jcmoraisjr

@gcs278 gcs278 marked this pull request as ready for review June 8, 2026 19:15
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 8, 2026
@gcs278 gcs278 changed the title Smoke test HAProxy 3.2.19 RPM WIP: Smoke test HAProxy 3.2.19 RPM Jun 8, 2026
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6d13d86a-e5cd-4bba-82dc-f83c50ae7dcf

📥 Commits

Reviewing files that changed from the base of the PR and between 3acfa51 and b83f1e1.

📒 Files selected for processing (1)
  • images/router/haproxy/Dockerfile.ocp
🚧 Files skipped from review as they are similar to previous changes (1)
  • images/router/haproxy/Dockerfile.ocp

Walkthrough

The Dockerfile for the OpenShift router removes haproxy28 from INSTALL_PKGS, downloads a pinned haproxy32 RPM from a GitHub URL, installs it with rpm -ivh, removes the RPM file, and prints the installed HAProxy version using haproxy -vv.

Changes

HAProxy version upgrade to 3.2.19

Layer / File(s) Summary
HAProxy 3.2 RPM installation
images/router/haproxy/Dockerfile.ocp
INSTALL_PKGS no longer includes haproxy28. The Dockerfile downloads a pinned haproxy32-3.2.19 RPM from GitHub, installs it with rpm -ivh, removes the temporary RPM, and runs haproxy -vv to show the version.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Container-Privileges ❌ Error The new K8s manifest deploy/router.yaml contains hostNetwork: true (line 54), which is explicitly flagged by the container-privileges check. Remove hostNetwork: true from the pod spec or add justification documentation for why network namespace isolation cannot be used.
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'WIP: Smoke test HAProxy 3.2.19 RPM' clearly describes the main change—testing HAProxy 3.2.19 RPM installation in the Dockerfile—which aligns with the changeset that downloads and installs haproxy32 RPM.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Stable And Deterministic Test Names ✅ Passed Repository uses Go standard testing package, not Ginkgo. Test names are static strings in struct fields passed to t.Run(), not dynamic Ginkgo-style test titles.
Test Structure And Quality ✅ Passed The PR modifies only the Dockerfile (images/router/haproxy/Dockerfile.ocp) with no test code changes. Ginkgo test quality check is not applicable when no test files are modified.
Microshift Test Compatibility ✅ Passed This PR modifies a Dockerfile for the HAProxy router image and does not add any Ginkgo e2e tests. The MicroShift Test Compatibility check only applies when new e2e tests are added.
Single Node Openshift (Sno) Test Compatibility ✅ Passed PR only modifies Dockerfile.ocp; no new Ginkgo e2e tests (It(), Describe(), Context(), When()) are added, so SNO compatibility check does not apply.
Topology-Aware Scheduling Compatibility ✅ Passed PR modifies only a Dockerfile for HAProxy container image, not deployment manifests, operator code, or controllers. No scheduling constraints are introduced.
Ote Binary Stdout Contract ✅ Passed Pull request only modifies a Dockerfile, not Go source code or test files. OTE Binary Stdout Contract only applies to process-level Go code; this PR contains no such changes.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed This PR modifies only a Dockerfile for building the HAProxy router image, not Ginkgo e2e tests. The custom check applies only to pull requests that add new e2e tests.
No-Weak-Crypto ✅ Passed PR upgrades HAProxy to 3.2.19; no new weak cryptographic algorithms, custom crypto, or constant-time issues introduced. Pre-existing SECLEVEL=1 unchanged.
No-Sensitive-Data-In-Logs ✅ Passed No sensitive data exposed in logs. The haproxy -vv command outputs only standard diagnostic info (version, build options, libraries) without passwords, tokens, API keys, PII, or credentials.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 8, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@images/router/haproxy/Dockerfile.ocp`:
- Line 3: The RUN instruction that installs
haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm must perform package cache cleanup
in the same layer; update the RUN that currently calls "yum install -y
https://.../haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm" to chain the install
with cache cleanup (e.g., run yum clean all and remove yum caches) in the same
command so no package manager metadata remains in the final image.
- Line 3: The RUN line in Dockerfile.ocp that installs the RPM from a mutable
GitHub branch (the existing RUN yum install ... refs/heads/master URL) must be
replaced with an immutable, verified artifact: point to a pinned artifact URL
(digest or internal Brew/registry artifact) and verify its integrity before
install (e.g., fetch the RPM by digest/checksum or GPG signature, validate
checksum/signature, then run yum install from the verified local file). Ensure
the RPM source is not a branch URL and include checksum or signature validation
steps so the container build only consumes a pinned, trusted package.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d5e60100-d267-4eaa-9a00-e268af5564b5

📥 Commits

Reviewing files that changed from the base of the PR and between a86164c and 09017b0.

📒 Files selected for processing (1)
  • images/router/haproxy/Dockerfile.ocp

Comment thread images/router/haproxy/Dockerfile.ocp Outdated
FROM registry.ci.openshift.org/ocp/4.22:haproxy-router-base
RUN INSTALL_PKGS="socat haproxy28 rsyslog procps-ng util-linux" && \

RUN yum install -y https://github.com/gcs278/openshift-hacks/raw/refs/heads/master/haproxy-builds/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clean yum metadata in the same layer as the RPM install

The first yum install (Line 3) does not run yum clean all in the same RUN layer, so cache/metadata remain in that layer and bloat the final image. Move cleanup into the same command chain.
As per coding guidelines: "No package manager cache in final layer".

🧰 Tools
🪛 Trivy (0.69.3)

[error] 3-3: 'yum clean all' missing

'yum clean all' is missed: yum install -y https://github.com/gcs278/openshift-hacks/raw/refs/heads/master/haproxy-builds/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm

Rule: DS-0015

Learn more

(IaC/Dockerfile)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@images/router/haproxy/Dockerfile.ocp` at line 3, The RUN instruction that
installs haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm must perform package cache
cleanup in the same layer; update the RUN that currently calls "yum install -y
https://.../haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm" to chain the install
with cache cleanup (e.g., run yum clean all and remove yum caches) in the same
command so no package manager metadata remains in the final image.

Sources: Coding guidelines, Linters/SAST tools


⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Use an immutable, verified RPM source instead of a mutable GitHub branch URL

Line 3 installs an RPM from refs/heads/master, which is mutable and not integrity-checked. If that artifact changes upstream, the build can silently consume untrusted content (including RPM scriptlets executed during install). Use an immutable source (e.g., internal Brew/registry artifact) and verify checksum/signature before install.
As per coding guidelines, container artifacts should follow immutable pinning principles: "non-RH images: pin by digest".

Suggested hardening change
-RUN yum install -y https://github.com/gcs278/openshift-hacks/raw/refs/heads/master/haproxy-builds/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm
+RUN curl -fsSL -o /tmp/haproxy32.rpm https://<immutable-artifact-url>/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm && \
+    echo "<sha256>  /tmp/haproxy32.rpm" | sha256sum -c - && \
+    yum install -y /tmp/haproxy32.rpm && \
+    rm -f /tmp/haproxy32.rpm && \
+    yum clean all
🧰 Tools
🪛 Trivy (0.69.3)

[error] 3-3: 'yum clean all' missing

'yum clean all' is missed: yum install -y https://github.com/gcs278/openshift-hacks/raw/refs/heads/master/haproxy-builds/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm

Rule: DS-0015

Learn more

(IaC/Dockerfile)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@images/router/haproxy/Dockerfile.ocp` at line 3, The RUN line in
Dockerfile.ocp that installs the RPM from a mutable GitHub branch (the existing
RUN yum install ... refs/heads/master URL) must be replaced with an immutable,
verified artifact: point to a pinned artifact URL (digest or internal
Brew/registry artifact) and verify its integrity before install (e.g., fetch the
RPM by digest/checksum or GPG signature, validate checksum/signature, then run
yum install from the verified local file). Ensure the RPM source is not a branch
URL and include checksum or signature validation steps so the container build
only consumes a pinned, trusted package.

Source: Coding guidelines

The 4.22 base image uses microdnf via ART wrapper which doesn't
support installing RPMs directly from URLs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-ci

openshift-ci Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from jcmoraisjr. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

♻️ Duplicate comments (2)
images/router/haproxy/Dockerfile.ocp (2)

3-5: ⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Use an immutable, verified RPM source instead of a mutable GitHub branch URL.

Line 3 installs an RPM from refs/heads/master, which is mutable and not integrity-checked. If that artifact changes upstream, the build can silently consume untrusted content, including RPM scriptlets executed during install.

🔒 Recommended hardening change
-RUN curl -fLo /tmp/haproxy32.rpm https://github.com/gcs278/openshift-hacks/raw/refs/heads/master/haproxy-builds/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm && \
-    yum install -y /tmp/haproxy32.rpm && \
-    rm -f /tmp/haproxy32.rpm
+RUN curl -fsSL -o /tmp/haproxy32.rpm https://<immutable-artifact-url>/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm && \
+    echo "<sha256>  /tmp/haproxy32.rpm" | sha256sum -c - && \
+    yum install -y /tmp/haproxy32.rpm && \
+    rm -f /tmp/haproxy32.rpm && \
+    yum clean all

As per coding guidelines, container artifacts should follow immutable pinning principles: "non-RH images: pin by digest".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@images/router/haproxy/Dockerfile.ocp` around lines 3 - 5, The RUN step that
curls an RPM from a mutable GitHub branch (the line starting with "RUN curl -fLo
/tmp/haproxy32.rpm ... refs/heads/master") must be replaced with an immutable,
verified artifact fetch: point the download to a pinned release URL (tag or
commit SHA) or a trusted registry with a digest, and verify integrity before
install (e.g., check a SHA256 checksum or GPG signature of haproxy32.rpm)
instead of relying on refs/heads/master; update the RUN command to download the
pinned URL, validate the file, then install and remove it.

Source: Coding guidelines


3-5: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clean yum metadata in the same layer as the RPM install.

The yum install does not run yum clean all in the same RUN layer, so cache and metadata remain in that layer and bloat the final image.

🧹 Proposed fix
 RUN curl -fLo /tmp/haproxy32.rpm https://github.com/gcs278/openshift-hacks/raw/refs/heads/master/haproxy-builds/haproxy32-3.2.19-1.rhaos4.23.el9.x86_64.rpm && \
     yum install -y /tmp/haproxy32.rpm && \
-    rm -f /tmp/haproxy32.rpm
+    rm -f /tmp/haproxy32.rpm && \
+    yum clean all

As per coding guidelines: "No package manager cache in final layer".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@images/router/haproxy/Dockerfile.ocp` around lines 3 - 5, The RUN layer that
downloads and installs /tmp/haproxy32.rpm leaves yum metadata behind; modify the
RUN that currently runs curl && yum install -y /tmp/haproxy32.rpm && rm -f
/tmp/haproxy32.rpm to also run the yum cleanup (e.g., yum clean all && rm -rf
/var/cache/yum or equivalent) in the same RUN chain so package manager
cache/metadata are removed before the layer is committed; target the RUN
instruction that handles /tmp/haproxy32.rpm in Dockerfile.ocp to implement this
change.

Sources: Coding guidelines, Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@images/router/haproxy/Dockerfile.ocp`:
- Around line 3-5: The RUN step that curls an RPM from a mutable GitHub branch
(the line starting with "RUN curl -fLo /tmp/haproxy32.rpm ...
refs/heads/master") must be replaced with an immutable, verified artifact fetch:
point the download to a pinned release URL (tag or commit SHA) or a trusted
registry with a digest, and verify integrity before install (e.g., check a
SHA256 checksum or GPG signature of haproxy32.rpm) instead of relying on
refs/heads/master; update the RUN command to download the pinned URL, validate
the file, then install and remove it.
- Around line 3-5: The RUN layer that downloads and installs /tmp/haproxy32.rpm
leaves yum metadata behind; modify the RUN that currently runs curl && yum
install -y /tmp/haproxy32.rpm && rm -f /tmp/haproxy32.rpm to also run the yum
cleanup (e.g., yum clean all && rm -rf /var/cache/yum or equivalent) in the same
RUN chain so package manager cache/metadata are removed before the layer is
committed; target the RUN instruction that handles /tmp/haproxy32.rpm in
Dockerfile.ocp to implement this change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c88616bc-de9b-422c-a972-793668d3b076

📥 Commits

Reviewing files that changed from the base of the PR and between 09017b0 and 3acfa51.

📒 Files selected for processing (1)
  • images/router/haproxy/Dockerfile.ocp

microdnf (used by the ART yum wrapper in 4.22 base images) does not
support installing local RPM files. Use rpm directly to bypass it.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gcs278

gcs278 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

/test ?

@gcs278

gcs278 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

/test e2e-metal-ipi-ovn-dualstack
/test e2e-metal-ipi-ovn-ipv6
/test e2e-metal-ipi-ovn-router
/test okd-scos-e2e-aws-ovn
/test perfscale-aws-fips-ingress-perf
/test perfscale-aws-ingress-perf

@gcs278

gcs278 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

CI is looking very good. One failure on The HAProxy router converges when multiple routers are writing conflicting status, but that is that very flaky in other CI jobs.

I call it good, but I'll spin more more time:
/test all

@gcs278

gcs278 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

/test ?

@gcs278

gcs278 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

/test e2e-metal-ipi-ovn-dualstack
/test e2e-metal-ipi-ovn-ipv6
/test e2e-metal-ipi-ovn-router
/test okd-scos-e2e-aws-ovn
/test perfscale-aws-fips-ingress-perf
/test perfscale-aws-ingress-perf

@gcs278

gcs278 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@openshift-ci

openshift-ci Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

@gcs278: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

@gcs278

gcs278 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Early Perf & Scale Results of (non-FIP): here

And Fips: here (Note: This is comparing FIPs to non-FIPs which isn't a fair comparison, we need to get a FIPs baseline run, but I'm not sure how/where to identify that).

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

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants