OCPBUGS-99921: Dump istiod logs and gateway state on GatewayAPI test failure - #31454
Conversation
When a GatewayAPI test fails, the istiod pod may be replaced before the cluster dump captures its logs, making root cause analysis impossible. Add a JustAfterEach hook that fires on failure before AfterEach cleanup to capture istiod pod logs, gateway status conditions, deployments, services, and GatewayClass status while everything is still alive. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
WalkthroughUpdates the Gateway API test cleanup hook to collect Istiod logs and Gateway-related resource YAML when a spec fails, using the deployment identifier and logging explicit errors when resource queries fail. ChangesGateway API diagnostics
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@gcs278: This pull request references Jira Issue OCPBUGS-99921, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
/jira refresh |
|
@gcs278: This pull request references Jira Issue OCPBUGS-99921, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
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 `@test/extended/router/gatewayapicontroller.go`:
- Around line 225-235: Update the three diagnostic oc get blocks in the
surrounding test hook to add else branches that log each command’s err via
e2e.Logf when Output fails, while preserving the existing successful-output
logging and original test failure behavior.
🪄 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: faee1921-7097-4130-9599-e37ad0381459
📒 Files selected for processing (1)
test/extended/router/gatewayapicontroller.go
|
/pipeline required |
|
Scheduling required tests: |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@gcs278: This pull request references Jira Issue OCPBUGS-99921, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
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 `@test/extended/router/gatewayapicontroller.go`:
- Around line 217-239: The failure diagnostics in the g.JustAfterEach hook use
unbounded pod-log and oc command execution. Thread a timeout-aware context or
runner through DumpPodLogsStartingWithInNamespace and each oc get invocation so
hung API or CLI calls terminate within a bounded period while preserving the
existing diagnostic output and error handling.
🪄 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: 8ebf7019-9cdd-4d3b-a1fe-999cbae88f8d
📒 Files selected for processing (1)
test/extended/router/gatewayapicontroller.go
| g.JustAfterEach(func() { | ||
| if g.CurrentSpecReport().Failed() { | ||
| e2e.Logf("=== Dumping Gateway API debug info after test failure ===") | ||
|
|
||
| exutil.DumpPodLogsStartingWithInNamespace(istiodDeployment, ingressNamespace, oc) | ||
|
|
||
| if output, err := oc.AsAdmin().WithoutNamespace().Run("get").Args("gateways.gateway.networking.k8s.io", "-n", ingressNamespace, "-o", "yaml").Output(); err == nil { | ||
| e2e.Logf("Gateways in %s:\n%s", ingressNamespace, output) | ||
| } else { | ||
| e2e.Logf("Failed to get gateways in %s: %v", ingressNamespace, err) | ||
| } | ||
|
|
||
| if output, err := oc.AsAdmin().WithoutNamespace().Run("get").Args("deployments,services", "-n", ingressNamespace, "-o", "wide").Output(); err == nil { | ||
| e2e.Logf("Deployments and services in %s:\n%s", ingressNamespace, output) | ||
| } else { | ||
| e2e.Logf("Failed to get deployments/services in %s: %v", ingressNamespace, err) | ||
| } | ||
|
|
||
| if output, err := oc.AsAdmin().WithoutNamespace().Run("get").Args("gatewayclasses.gateway.networking.k8s.io", gatewayClassName, "-o", "yaml").Output(); err == nil { | ||
| e2e.Logf("GatewayClass %s:\n%s", gatewayClassName, output) | ||
| } else { | ||
| e2e.Logf("Failed to get GatewayClass %s: %v", gatewayClassName, err) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n== gatewayapicontroller.go ==\n'
sed -n '190,260p' test/extended/router/gatewayapicontroller.go | cat -n
printf '\n== framework.go slice around DumpPodLogsStartingWithInNamespace ==\n'
rg -n "DumpPodLogsStartingWithInNamespace|context.Background|WithTimeout|timeout" test/extended/util/framework.go
sed -n '600,700p' test/extended/util/framework.go | cat -nRepository: openshift/origin
Length of output: 16469
🏁 Script executed:
set -euo pipefail
printf '\n== CLI type and Run/Output implementation ==\n'
rg -n "type CLI struct|func \(.*CLI.*\) Run\(|func \(.*\) Output\(|timeout" test/extended/util test/extended -g '!**/*_test.go'
printf '\n== likely file candidates ==\n'
fd -a 'framework.go' test/extended/util
fd -a '*.go' test/extended/util | sed -n '1,120p'Repository: openshift/origin
Length of output: 50375
🏁 Script executed:
set -euo pipefail
printf '\n== util files ==\n'
fd -a '' test/extended/util | sed -n '1,200p'
printf '\n== search for CLI definition files ==\n'
rg -n --glob 'test/extended/util/*.go' 'type CLI struct|type CLI interface|func .*Run\(' test/extended/utilRepository: openshift/origin
Length of output: 12843
🏁 Script executed:
set -euo pipefail
sed -n '85,220p' test/extended/util/client.go | cat -n
printf '\n== Run/Output area ==\n'
sed -n '930,1045p' test/extended/util/client.go | cat -nRepository: openshift/origin
Length of output: 10442
🏁 Script executed:
set -euo pipefail
sed -n '1045,1145p' test/extended/util/client.go | cat -nRepository: openshift/origin
Length of output: 4326
Bound the failure diagnostics path.
DumpPodLogsStartingWithInNamespace and the oc get calls here use unbounded contexts/exec, so a hung API or CLI call can stall this failure hook indefinitely and delay teardown. Thread a timeout-aware context or runner through these diagnostics.
🤖 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 `@test/extended/router/gatewayapicontroller.go` around lines 217 - 239, The
failure diagnostics in the g.JustAfterEach hook use unbounded pod-log and oc
command execution. Thread a timeout-aware context or runner through
DumpPodLogsStartingWithInNamespace and each oc get invocation so hung API or CLI
calls terminate within a bounded period while preserving the existing diagnostic
output and error handling.
Sources: Path instructions, Learnings
There was a problem hiding this comment.
fair point, but I think it's out-of-scope for this PR
There was a problem hiding this comment.
@gcs278, understood—this is a reasonable follow-up rather than a blocker for this diagnostics-focused PR. Would you like me to open a GitHub issue to track bounding the Gateway API failure-diagnostic commands?
🧠 Learnings used
Learnt from: ngopalak-redhat
Repo: openshift/origin PR: 31339
File: test/extended/node/node_utils.go:304-306
Timestamp: 2026-06-25T02:59:24.710Z
Learning: In openshift/origin Go test helper code under test/extended/, avoid switching to context.Background() for deferred cleanup that must run after the spec/test context is canceled. Instead, detach cancellation from the existing context (e.g., ctxNoCancel := context.WithoutCancel(ctx)) to preserve context values, then apply an explicit timeout to keep cleanup bounded (e.g., context.WithTimeout(ctxNoCancel, ...)) so cleanup can complete safely without relying on an uncanceled root context.
You are interacting with an AI system.
There was a problem hiding this comment.
@gcs278, understood. I won’t open a follow-up issue.
🧠 Learnings used
Learnt from: ngopalak-redhat
Repo: openshift/origin PR: 31339
File: test/extended/node/node_utils.go:304-306
Timestamp: 2026-06-25T02:59:24.710Z
Learning: In openshift/origin Go test helper code under test/extended/, avoid switching to context.Background() for deferred cleanup that must run after the spec/test context is canceled. Instead, detach cancellation from the existing context (e.g., ctxNoCancel := context.WithoutCancel(ctx)) to preserve context values, then apply an explicit timeout to keep cleanup bounded (e.g., context.WithTimeout(ctxNoCancel, ...)) so cleanup can complete safely without relying on an uncanceled root context.
You are interacting with an AI system.
|
/pipeline required |
|
Scheduling required tests: |
|
/retest |
|
/lgtm |
|
CI jobs are currently passing, so at least it's not breaking any existing job. /verified by CI |
|
@gcs278: This PR has been marked as verified by 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gcs278, rikatz 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 |
|
@gcs278: 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. |
|
@gcs278: Jira Issue Verification Checks: Jira Issue OCPBUGS-99921 Jira Issue OCPBUGS-99921 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
Summary
JustAfterEachhook to the GatewayAPI controller e2e tests that captures istiod pod logs, gateway status, deployments/services, and GatewayClass status when a test failsTest plan
go build ./test/extended/router/🤖 Generated with Claude Code
Summary by CodeRabbit