Skip to content

TRT-2939: Revert "Merge pull request #31495 from jcmoraisjr/NE-2839-haproxy-version-upgrade-tests" - #31598

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
redhat-chai-bot:revert-31495-haproxy
Sep 2, 2026
Merged

TRT-2939: Revert "Merge pull request #31495 from jcmoraisjr/NE-2839-haproxy-version-upgrade-tests"#31598
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
redhat-chai-bot:revert-31495-haproxy

Conversation

@redhat-chai-bot

@redhat-chai-bot redhat-chai-bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Revert of #31495

This reverts the merge commit for PR #31495 (NE-2839: Add HAProxy version upgrade tests).

Why

PR #31495 introduced a getHAProxyVersionConfig() helper that calls Deployments("openshift-ingress-operator").Get(…, "ingress-operator") in the guest cluster. In HyperShift topologies the ingress-operator runs in the management cluster, so the Get returns a 404 unconditionally. This fails 5 IngressControllerMultipleHAProxyVersions tests in BeforeEach, breaking all HyperShift conformance jobs.

Impact

  • Payload rejected: 5.1.0-0.nightly-2026-09-02-124257 — 11 of 16 blocking jobs failed (10 new failures)
  • Affected jobs: aggregated-hypershift-ovn-conformance-5.1, aggregated-hypershift-aks-conformance-5.1
  • Root cause confidence: 100%

References


AI-generated. Review for accuracy.

@xueqzhan requested in Slack thread

Summary by CodeRabbit

  • Tests
    • Consolidated HAProxy version upgrade coverage into the multi-HAProxy test suite.
    • Upgrade validation now checks both the default and alternate supported HAProxy versions, including runtime version confirmation.
    • Improved readiness handling and timeout coverage during HAProxy version tests.
    • Removed the standalone HAProxy upgrade test cases; Gateway API upgrade coverage remains included.

…NE-2839-haproxy-version-upgrade-tests"

This reverts commit 5e2b66a, reversing
changes made to bd31098.

Reverting PR openshift#31495 (NE-2839: Add HAProxy version upgrade tests) which
introduced breaking test failures. Tracked in TRT-2939.
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: automatic mode

@openshift-ci openshift-ci Bot added the ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review label Sep 2, 2026
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 2, 2026
@openshift-ci-robot

openshift-ci-robot commented Sep 2, 2026

Copy link
Copy Markdown

@redhat-chai-bot: This pull request references TRT-2939 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.1.0" version, but no target version was set.

Details

In response to this:

Revert of #31495

This reverts the merge commit for PR #31495 (NE-2839: Add HAProxy version upgrade tests).

Why

PR #31495 introduced a getHAProxyVersionConfig() helper that calls Deployments("openshift-ingress-operator").Get(…, "ingress-operator") in the guest cluster. In HyperShift topologies the ingress-operator runs in the management cluster, so the Get returns a 404 unconditionally. This fails 5 IngressControllerMultipleHAProxyVersions tests in BeforeEach, breaking all HyperShift conformance jobs.

Impact

  • Payload rejected: 5.1.0-0.nightly-2026-09-02-124257 — 11 of 16 blocking jobs failed (10 new failures)
  • Affected jobs: aggregated-hypershift-ovn-conformance-5.1, aggregated-hypershift-aks-conformance-5.1
  • Root cause confidence: 100%

References


AI-generated. Review for accuracy.

@xueqzhan requested in Slack thread

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.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Team

Run ID: 26cbb26c-c932-489f-992f-893b3ec81198

📥 Commits

Reviewing files that changed from the base of the PR and between f097372 and 613c58f.

📒 Files selected for processing (3)
  • test/e2e/upgrade/upgrade.go
  • test/extended/router/haproxyversion_upgrade.go
  • test/extended/router/multi-haproxy.go
💤 Files with no reviewable changes (2)
  • test/e2e/upgrade/upgrade.go
  • test/extended/router/haproxyversion_upgrade.go

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


Walkthrough

The legacy HAProxy upgrade test suite was removed. Multi-HAProxy tests now derive supported versions from controller status, manage readiness inline, validate effective versions, and inspect runtime HAProxy in the haproxy container.

Changes

HAProxy upgrade test consolidation

Layer / File(s) Summary
Remove legacy upgrade suite
test/e2e/upgrade/upgrade.go, test/extended/router/haproxyversion_upgrade.go
The three legacy HAProxy upgrade cases were removed from AllTests, and the standalone upgrade test implementation was deleted.
Resolve HAProxy versions
test/extended/router/multi-haproxy.go
Setup reads the default EffectiveHAProxyVersion and derives the alternate supported version.
Inline controller lifecycle
test/extended/router/multi-haproxy.go
Controller creation accepts a readiness timeout, performs readiness polling inline, and deletes tracked controllers directly.
Validate effective and runtime versions
test/extended/router/multi-haproxy.go
Tests validate default, alternate, and unset versions. Runtime inspection now runs in the haproxy container.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 613c5

This change reverts the HAProxy version upgrade tests and removes the reported HyperShift test failure path; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: rhamini3

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Test Structure And Quality ⚠️ Warning The PR introduces assertion-message violations in test/extended/router/multi-haproxy.go. New or changed assertions at lines 59, 66, 69, 85, 86, and 191 use no diagnostic message. Line 194 also remov… Add meaningful context to every new or changed assertion. Include the controller name and operation for cleanup, API-client creation, CRD retrieval, default-controller retrieval, and effective-version comparison. Restore a diagnostic messag…
✅ 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 clearly identifies the primary change: reverting the merge that introduced HAProxy version upgrade tests. This matches the removal and revision of those tests in the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
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 PASS. The changed Ginkgo titles in test/extended/router/multi-haproxy.go are static string literals and match the parent revision. The pull request removes the HAProxy upgrade registrations and test…
Microshift Test Compatibility ✅ Passed No new Ginkgo test declarations were added. The parent and current multi-haproxy.go files contain the same Describe and five It test names. The pull request removes upgrade tests and registratio…
Single Node Openshift (Sno) Test Compatibility ✅ Passed PASS — The pull request adds no new Ginkgo test declarations. The five multi-haproxy.go tests existed in the parent revision and were only modified. The pull request also removes the HAProxy upgrade…
Topology-Aware Scheduling Compatibility ✅ Passed PASS: The pull request changes only end-to-end test code. It removes HAProxy upgrade test registrations and a deleted upgrade-test implementation. The remaining multi-haproxy.go changes adjust API l…
Ote Binary Stdout Contract ✅ Passed PASS. The pull request introduces no process-level stdout write. It removes three upgrade test registrations and deletes the HAProxy upgrade file. In multi-haproxy.go, the added e2e.Logf calls are…
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PASS. The pull request adds no new Ginkgo test declarations. The five g.It declarations in test/extended/router/multi-haproxy.go exist in both the parent and current revisions. The deleted HAProxy…
No-Weak-Crypto ✅ Passed PASS. The commit changes test registration and HAProxy version test plumbing only. The added code has no MD5, SHA1, DES, 3DES, RC4, Blowfish, ECB, crypto, HMAC, or constant-time comparison usage. Its …
Container-Privileges ✅ Passed PASS: The PR changes only three Go test files. The exact diff contains no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation, or security-context settings. The …
No-Sensitive-Data-In-Logs ✅ Passed PASS — The pull request does not introduce sensitive-data logging. The added log statements report IngressController names and HAProxy version values only. The runtime command captures HAProxy `show v…
Full details: Stable And Deterministic Test Names

Explanation

PASS. The changed Ginkgo titles in test/extended/router/multi-haproxy.go are static string literals and match the parent revision. The pull request removes the HAProxy upgrade registrations and test file. Generated controller names remain in test setup and logs, not in test titles.

Full details: Test Structure And Quality

Explanation

The PR introduces assertion-message violations in test/extended/router/multi-haproxy.go. New or changed assertions at lines 59, 66, 69, 85, 86, and 191 use no diagnostic message. Line 194 also removes the existing "error getting HAProxy version from runtime API" message. These cases match the check's explicit failure example. The test uses AfterEach, and the readiness and polling operations have finite timeouts. The deleted upgrade tests introduce no new quality issue.

Resolution

Add meaningful context to every new or changed assertion. Include the controller name and operation for cleanup, API-client creation, CRD retrieval, default-controller retrieval, and effective-version comparison. Restore a diagnostic message for the runtime HAProxy-version assertion, such as failed to get HAProxy version from runtime API.

Full details: Microshift Test Compatibility

Explanation

No new Ginkgo test declarations were added. The parent and current multi-haproxy.go files contain the same Describe and five It test names. The pull request removes upgrade tests and registration entries. Although the retained tests use the pre-existing IngressController operator API, this check only flags newly added Ginkgo tests, so no MicroShift incompatibility was introduced by this pull request.

Full details: Single Node Openshift (Sno) Test Compatibility

Explanation

PASS — The pull request adds no new Ginkgo test declarations. The five multi-haproxy.go tests existed in the parent revision and were only modified. The pull request also removes the HAProxy upgrade tests from AllTests and deletes their implementation. The retained tests set one replica and do not require distinct nodes, node roles, failover, rescheduling, anti-affinity, topology spread, scaling, or multiple ingress endpoints. No SNO guard is required under this check.

Full details: Topology-Aware Scheduling Compatibility

Explanation

PASS: The pull request changes only end-to-end test code. It removes HAProxy upgrade test registrations and a deleted upgrade-test implementation. The remaining multi-haproxy.go changes adjust API lookup, polling, cleanup, and runtime-container inspection. The diff introduces no required or preferred affinity, topology spread constraint, node selector or affinity, toleration, PDB, or topology-derived replica logic. The test helper's fixed Replicas: 1 value is pre-existing and is not a new scheduling constraint. No deployment manifest, operator implementation, or workload controller scheduling behavior changes.

Full details: Ote Binary Stdout Contract

Explanation

PASS. The pull request introduces no process-level stdout write. It removes three upgrade test registrations and deletes the HAProxy upgrade file. In multi-haproxy.go, the added e2e.Logf calls are inside It bodies or helpers called only by It bodies, which the check explicitly excludes. The changed BeforeEach and AfterEach code has no direct stdout write. The remaining upgrade.go formatting writes target os.Stderr.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

PASS. The pull request adds no new Ginkgo test declarations. The five g.It declarations in test/extended/router/multi-haproxy.go exist in both the parent and current revisions. The deleted HAProxy upgrade file and removed upgrade registrations do not add tests. Changed code uses cluster API calls and an in-cluster HAProxy socket, with no hardcoded IPv4 addresses, IPv4-only parsing, IPv4-only CIDRs, public hosts, or external URLs.

Full details: No-Weak-Crypto

Explanation

PASS. The commit changes test registration and HAProxy version test plumbing only. The added code has no MD5, SHA1, DES, 3DES, RC4, Blowfish, ECB, crypto, HMAC, or constant-time comparison usage. Its string comparison checks a public HAProxy version, not a secret or token. The deleted test cannot introduce weak crypto.

Full details: Container-Privileges

Explanation

PASS: The PR changes only three Go test files. The exact diff contains no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation, or security-context settings. The changed code creates and deletes IngressController resources and executes commands in an existing haproxy container; it does not define a container or manifest privilege configuration.

Full details: No-Sensitive-Data-In-Logs

Explanation

PASS — The pull request does not introduce sensitive-data logging. The added log statements report IngressController names and HAProxy version values only. The runtime command captures HAProxy show version output, which was already logged before this change; only the container name changes. The existing failure-path pod-log dump is unchanged. The deleted upgrade test removes logging rather than adding it.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@xueqzhan

xueqzhan commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

/override ci/prow/e2e-aws-ovn-upgrade-rollback
/override ci/prow/e2e-metal-ipi-ovn-ipv6
/override ci/prow/e2e-vsphere-ovn
/override ci/prow/e2e-vsphere-ovn-upi

@openshift-ci

openshift-ci Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@xueqzhan: Overrode contexts on behalf of xueqzhan: ci/prow/e2e-aws-ovn-upgrade-rollback, ci/prow/e2e-metal-ipi-ovn-ipv6, ci/prow/e2e-vsphere-ovn, ci/prow/e2e-vsphere-ovn-upi

Details

In response to this:

/override ci/prow/e2e-aws-ovn-upgrade-rollback
/override ci/prow/e2e-metal-ipi-ovn-ipv6
/override ci/prow/e2e-vsphere-ovn
/override ci/prow/e2e-vsphere-ovn-upi

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.

@xueqzhan

xueqzhan commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Sep 2, 2026
@openshift-ci

openshift-ci Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: redhat-chai-bot, xueqzhan

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

The pull request process is described 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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 2, 2026
@openshift-ci

openshift-ci Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@redhat-chai-bot: 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.

@xueqzhan

xueqzhan commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

/verified by CI

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Sep 2, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@xueqzhan: This PR has been marked as verified by CI.

Details

In response to this:

/verified by CI

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-merge-bot
openshift-merge-bot Bot merged commit d1c2c42 into openshift:main Sep 2, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants