CONSOLE-5209: Fix IngressComponentRouteLabels e2e tests for HCP compatibility - #31483
CONSOLE-5209: Fix IngressComponentRouteLabels e2e tests for HCP compatibility#31483jhadvig wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@jhadvig: This pull request references CONSOLE-5209 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. |
|
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:
WalkthroughThe console label propagation test now supports standard OpenShift and HyperShift environments. HyperShift route mutations use HostedCluster configuration with conflict retries. Standard clusters continue to use Ingress resources. ChangesLabel propagation route handling
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant LabelPropagationTest
participant RouteMutationHelpers
participant HostedCluster
participant Ingress
LabelPropagationTest->>RouteMutationHelpers: request route mutation
alt HyperShift
RouteMutationHelpers->>HostedCluster: read and update component routes
HostedCluster-->>RouteMutationHelpers: return persisted routes
else Standard cluster
RouteMutationHelpers->>Ingress: update component routes
Ingress-->>RouteMutationHelpers: return persisted routes
end
RouteMutationHelpers-->>LabelPropagationTest: return mutation result
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jhadvig The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@test/extended/console/label_propagation.go`:
- Around line 74-80: The isHypershift check in this test setup block masks
Infrastructure read errors by returning false, nil when retrieval fails, causing
the code to incorrectly skip HCP context setup if the detection itself fails.
Update the exutil.IsHypershift function to propagate the Infrastructure read
error instead of suppressing it, so that actual detection failures are caught by
the existing o.Expect(err) assertion and prevent silent false-negatives when hcp
initialization should occur on HCP clusters.
- Around line 443-447: Handle the error returned by unstructured.NestedSlice in
the componentRoutes update flow before invoking modify. Return a wrapped error
when extraction fails, and only pass successfully retrieved routes to modify and
unstructured.SetNestedSlice.
- Around line 436-450: Update hcpModifyComponentRoutes to create one
context.WithTimeout using pollTimeout before the RetryOnConflict callback, defer
its cancellation, and pass that context to both the dynamic-client Get and
Update calls instead of context.TODO(). Preserve the existing retry and mutation
behavior while ensuring the entire HostedCluster operation has a deadline.
🪄 Autofix
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: 48140adb-84dd-4c45-85dc-22b91e85fb68
📒 Files selected for processing (1)
test/extended/console/label_propagation.go
|
/payload-job periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-aws-ovn-conformance-serial-techpreview |
|
@jhadvig: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/8dbf49c0-9016-11f1-9f96-dcd34189c0a0-0 |
|
Scheduling required tests: |
|
/retest |
|
Added a third commit to bump poll timeouts on HCP clusters. The multi-hop reconciliation chain (HostedCluster CR → HostedControlPlane → HCCO → guest Ingress → console-operator → Route) was timing out at 2 minutes. Bumped to 5m timeout / 10s interval on HCP, matching the timeouts used by HyperShift's own ingress e2e test. Standalone OCP keeps the original 2m/2s. |
|
/payload-job periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-aws-ovn-conformance-serial-techpreview |
|
@jhadvig: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/a1e96640-904f-11f1-8d9b-d2d082f26c8e-0 |
|
/payload-job periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-aws-ovn-conformance-serial-techpreview |
|
@jhadvig: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/3edd8a80-9091-11f1-9d0c-af83526a642e-0 |
|
/payload-job periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-aws-ovn-conformance-serial-techpreview |
|
@jhadvig: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/ea2c3000-9094-11f1-8306-7c3768c59d11-0 |
|
Scheduling required tests: |
f7e1811 to
0cfe5fa
Compare
…2e tests On HyperShift clusters a ValidatingAdmissionPolicy blocks direct modification of ingresses.config.openshift.io/cluster on the guest cluster. Branch the mutation helpers to modify the HostedCluster CR's spec.configuration.ingress.componentRoutes via the management cluster dynamic client on HCP, while keeping the existing typed client path on standalone OCP. Use 25m/10s poll timeout/interval on HCP to account for the multi-hop reconciliation chain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
0cfe5fa to
22c0e70
Compare
|
@jhadvig: all tests passed! 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. |
|
Scheduling required tests: |
3 participants
Summary
test/extended/console/label_propagation.gothat fail on HyperShift (HCP) clustersValidatingAdmissionPolicyblocks direct modification ofingresses.config.openshift.io/clusteron the guest clusterspec.configuration.ingress.componentRoutesvia the management cluster dynamic client on HCP, while keeping the existing typed client path on standalone OCPtest/extended/tls/tls_observed_config.gowhich does the same for TLS security profilesDetails
On HCP clusters, cluster-scoped config resources on the guest cluster are managed by the hosting cluster. The tests now detect HCP via
exutil.IsHypershift()and set up anhcpContextwith the management cluster's dynamic client. All three mutation functions (addComponentRouteWithLabels,updateComponentRouteLabels,removeComponentRoute) andcleanupTestRoutesbranch on HCP to use unstructured operations on the HostedCluster CR atspec.configuration.ingress.componentRoutes. Read-only operations (route polling, domain read) continue to hit the guest cluster directly.Prow job showing the failures
https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_release/82800/rehearse-82800-periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-aws-ovn-conformance-serial-techpreview/2084549238955446272
Test plan
go build ./cmd/openshift-tests/passesgo vet ./test/extended/console/...passesJira
https://redhat.atlassian.net/browse/CONSOLE-5209
Summary by CodeRabbit