NE-2215: add CI step and techpreview presubmit jobs for testing previous HAProxy version#80171
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@gcs278: This pull request references NE-2215 which is a valid jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds a new ingress HAProxy version step that writes ChangesHAProxy Version Step and CI Job Wiring
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
gcs278
changed the title
openshift-merge-bot
Bot
added
the
rehearsals-ack
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@ci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-commands.sh`:
- Around line 26-50: The current pipeline that builds HAPROXY_IMAGES uses plain
lexicographic sort which misorders multi-digit tags; change the pipeline that
sets HAPROXY_IMAGES to use version-aware sorting (e.g., replace the plain sort
with a version-aware sort such as sort -V or sort -t- -k2,2V) so that selecting
TARGET_IMAGE for HAPROXY_AUTO_SELECT (in the case branches that use head/tail)
picks the true previous/newest semantic version; keep the rest of the logic (the
grep '^haproxy-' filter, COUNT check, and HAPROXY_AUTO_SELECT case) the same.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b987fb87-db29-4eae-ab43-fe063a7c4c7a
📒 Files selected for processing (5)
ci-operator/config/openshift/release/openshift-release-main__nightly-5.0.yamlci-operator/step-registry/ingress/conf/haproxy-version/OWNERSci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-commands.shci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-ref.metadata.jsonci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-ref.yaml
| HAPROXY_IMAGES=$(oc adm release info --registry-config "${CLUSTER_PROFILE_DIR}/pull-secret" \ | ||
| "${RELEASE_IMAGE_LATEST}" --output=json | \ | ||
| jq -r '.references.spec.tags[].name' | grep '^haproxy-' | sort || true) | ||
|
|
||
| if [[ -z "${HAPROXY_IMAGES}" ]]; then | ||
| echo "No HAProxy sidecar images found in payload, skipping" | ||
| exit 0 | ||
| fi | ||
|
|
||
| COUNT=$(echo "${HAPROXY_IMAGES}" | wc -l) | ||
| echo "Found ${COUNT} HAProxy image(s) in payload: ${HAPROXY_IMAGES}" | ||
|
|
||
| if [[ "${COUNT}" -lt 2 ]]; then | ||
| echo "Only one HAProxy version in payload, no alternative version to test. Skipping." | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Image names follow the pattern "haproxy-28", "haproxy-32", etc. | ||
| case "${HAPROXY_AUTO_SELECT}" in | ||
| previous) | ||
| TARGET_IMAGE=$(echo "${HAPROXY_IMAGES}" | head -1) | ||
| ;; | ||
| newest) | ||
| TARGET_IMAGE=$(echo "${HAPROXY_IMAGES}" | tail -1) | ||
| ;; |
There was a problem hiding this comment.
Use version-aware ordering before selecting previous/newest.
Line 28 uses lexicographic sort, which can select the wrong image when tags include multi-digit components (for example, haproxy-310 vs haproxy-39).
💡 Proposed fix
- HAPROXY_IMAGES=$(oc adm release info --registry-config "${CLUSTER_PROFILE_DIR}/pull-secret" \
- "${RELEASE_IMAGE_LATEST}" --output=json | \
- jq -r '.references.spec.tags[].name' | grep '^haproxy-' | sort || true)
+ HAPROXY_IMAGES=$(oc adm release info --registry-config "${CLUSTER_PROFILE_DIR}/pull-secret" \
+ "${RELEASE_IMAGE_LATEST}" --output=json | \
+ jq -r '.references.spec.tags[].name | select(startswith("haproxy-"))' | sort -V || true)🤖 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
`@ci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-commands.sh`
around lines 26 - 50, The current pipeline that builds HAPROXY_IMAGES uses plain
lexicographic sort which misorders multi-digit tags; change the pipeline that
sets HAPROXY_IMAGES to use version-aware sorting (e.g., replace the plain sort
with a version-aware sort such as sort -V or sort -t- -k2,2V) so that selecting
TARGET_IMAGE for HAPROXY_AUTO_SELECT (in the case branches that use head/tail)
picks the true previous/newest semantic version; keep the rest of the logic (the
grep '^haproxy-' filter, COUNT check, and HAPROXY_AUTO_SELECT case) the same.
gcs278
force-pushed
the
haproxy-multi-version-ci-scaffolding
branch
3 times, most recently
from
a69c0d8 to
3754871
Compare
openshift-ci
Bot
added
the
approved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@ci-operator/config/openshift/cluster-ingress-operator/openshift-cluster-ingress-operator-master.yaml`:
- Around line 311-331: The generated Prow job for
e2e-aws-operator-haproxy28-techpreview is missing from the matching
ci-operator/jobs entry. Regenerate the job artifacts by running make update,
then commit the updated ci-operator/jobs/ configuration so the job definition is
present alongside the ci-operator/config change.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: bacec7ed-3009-4093-af8c-b9536b927d0e
📒 Files selected for processing (6)
ci-operator/config/openshift/cluster-ingress-operator/openshift-cluster-ingress-operator-master.yamlci-operator/config/openshift/release/openshift-release-main__nightly-5.0.yamlci-operator/step-registry/ingress/conf/haproxy-version/OWNERSci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-commands.shci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-ref.metadata.jsonci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-ref.yaml
✅ Files skipped from review due to trivial changes (2)
- ci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-ref.metadata.json
- ci-operator/step-registry/ingress/conf/haproxy-version/OWNERS
🚧 Files skipped from review as they are similar to previous changes (3)
- ci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-ref.yaml
- ci-operator/config/openshift/release/openshift-release-main__nightly-5.0.yaml
- ci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-commands.sh
gcs278
force-pushed
the
haproxy-multi-version-ci-scaffolding
branch
from
3754871 to
e1423e3
Compare
There was a problem hiding this comment.
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
`@ci-operator/config/openshift/cluster-ingress-operator/openshift-cluster-ingress-operator-master.yaml`:
- Around line 175-188: The new e2e-aws-ovn-serial-haproxy28-techpreview test is
defined in the CI config but missing from the generated ci-operator/jobs output.
Regenerate the jobs manifest so the matching job entry for this test is emitted,
and make sure the generated output reflects the updated test name and
configuration from the e2e-aws-ovn-serial-haproxy28-techpreview stanza.
- Around line 156-168: The new haproxy28 techpreview job definitions are present
in the CI operator config, but the generated Prow job entries under
ci-operator/jobs are missing. Regenerate the configs by running the repository’s
update generation flow so both e2e-aws-ovn-upgrade-haproxy28-techpreview and
e2e-aws-ovn-serial-haproxy28-techpreview are emitted, and verify the matching
job artifacts are created for the openshift-cluster-ingress-operator master
config.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: f6418d8f-7413-4b7c-b253-e74be098e5c9
📒 Files selected for processing (5)
ci-operator/config/openshift/cluster-ingress-operator/openshift-cluster-ingress-operator-master.yamlci-operator/step-registry/ingress/conf/haproxy-version/OWNERSci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-commands.shci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-ref.metadata.jsonci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-ref.yaml
✅ Files skipped from review due to trivial changes (2)
- ci-operator/step-registry/ingress/conf/haproxy-version/OWNERS
- ci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-ref.metadata.json
🚧 Files skipped from review as they are similar to previous changes (2)
- ci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-ref.yaml
- ci-operator/step-registry/ingress/conf/haproxy-version/ingress-conf-haproxy-version-commands.sh
gcs278
force-pushed
the
haproxy-multi-version-ci-scaffolding
branch
5 times, most recently
from
dfed3d6 to
2d47c96
Compare
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
gcs278
force-pushed
the
haproxy-multi-version-ci-scaffolding
branch
2 times, most recently
from
bec03d0 to
e067410
Compare
gcs278
changed the title
gcs278
changed the title
gcs278
force-pushed
the
haproxy-multi-version-ci-scaffolding
branch
from
a98aa0e to
be14565
Compare
gcs278
left a comment
gcs278
left a comment
There was a problem hiding this comment.
Addressed review comment: fixed ${HAPROXY_VERSION} → ${HAPROXY_VERSION:-} so nounset does not trigger before the if -z check can give the proper error message. Also rebased on main.
gcs278
force-pushed
the
haproxy-multi-version-ci-scaffolding
branch
from
be14565 to
04304b5
Compare
|
/lgtm |
Add CI jobs for testing the non-default HAProxy version (2.8) with the IngressControllerMultipleHAProxyVersions TechPreview feature gate: - ingress-conf-haproxy-version step: writes an IngressController manifest to SHARED_DIR with the specified haproxyVersion, run before ipi-install cluster-ingress-operator presubmits: - e2e-aws-ovn-haproxy28-techpreview: optional conformance presubmit - e2e-aws-operator-haproxy28-techpreview: optional CIO e2e presubmit - e2e-aws-ovn-upgrade-haproxy28-techpreview: optional upgrade presubmit - e2e-aws-ovn-serial-haproxy28-techpreview: optional serial presubmit (2 shards) router presubmits: - e2e-agnostic-haproxy28-techpreview: optional conformance presubmit (GCP) - e2e-aws-serial-haproxy28-techpreview: optional serial presubmit (2 shards) - e2e-aws-ovn-upgrade-haproxy28-techpreview: optional upgrade presubmit Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gcs278
force-pushed
the
haproxy-multi-version-ci-scaffolding
branch
from
04304b5 to
2aebbf3
Compare
|
Sorry - I realized the |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
I only changed |
|
@gcs278: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@gcs278, |
|
/pj-rehearse pull-ci-openshift-router-master-e2e-upgrade-haproxy28-techpreview |
|
@gcs278: your |
|
/pj-rehearse pull-ci-openshift-router-master-e2e-upgrade-haproxy28-techpreview |
|
@gcs278: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
hmmm, i double checked the must-gather and I do not see |
openshift-ci
Bot
added
the
do-not-merge/hold
|
ah never mind. i forgot the API doesn't exist because openshift/cluster-ingress-operator#1498 need to merge, but we need this PR to test openshift/cluster-ingress-operator#1498. Chicken & Egg. I see the step working, but we must merge and test on openshift/cluster-ingress-operator#1498 to get a real indicator if this works. @jcmoraisjr ready for another review. All test runs look good |
|
@gcs278: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-merge-bot
Bot
added
the
rehearsals-ack
|
/unhold |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
all good, ship it! /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gcs278, jcmoraisjr The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
Summary
Adds presubmit CI jobs and a reusable step to test the non-default HAProxy version (2.8) on the default IngressController. This supports the HAProxy version selection feature described in openshift/enhancements#1965, with the API defined in openshift/api#2895.
What's included
ingress-conf-haproxy-version): Configures the default IngressController to use a specific HAProxy version at install time by writing a manifest withspec.haproxyVersiontoSHARED_DIR. Takes an explicitHAPROXY_VERSIONenv var (e.g."2.8").cluster-ingress-operator presubmits (all optional, triggered manually):
e2e-aws-ovn-haproxy28-techpreview: conformance with HAProxy 2.8e2e-aws-operator-haproxy28-techpreview: operator e2e with HAProxy 2.8e2e-aws-ovn-upgrade-haproxy28-techpreview: upgrade test with HAProxy 2.8e2e-aws-ovn-serial-haproxy28-techpreview: serial e2e (2 shards) with HAProxy 2.8router presubmits (all optional, triggered manually):
e2e-agnostic-haproxy28-techpreview: conformance with HAProxy 2.8 (GCP)e2e-aws-serial-haproxy28-techpreview: serial e2e (2 shards) with HAProxy 2.8e2e-aws-ovn-upgrade-haproxy28-techpreview: upgrade test with HAProxy 2.8How it works
HAPROXY_VERSION: "2.8"andFEATURE_SET: TechPreviewNoUpgradespec.haproxyVersion: "2.8"toSHARED_DIRNot included (deferred)
haproxyVersionAPI is backported to the upgrade-from release.https://redhat.atlassian.net/browse/NE-2215
🤖 Generated with Claude Code