DNM/TEST: Test udn settling sleep#31226
Conversation
UDN tests are known to be heavy and these tests will fail on the first dropped connection which seems to happen randomly, albeit rarely. test: allow single timeout in UDN KAPI reachability check Hardens e2e test against transient networking blips while preserving strict failure detection for real regressions. Signed-off-by: Jamo Luhrsen <jluhrsen@gmail.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a 15s settling wait after default-network pod IPs are discovered, replaces the single in-pod curl healthz assertion with a 30s bounded retry loop that requires repeated ChangesNetwork Reachability Test Resilience
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/hold |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jluhrsen 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 |
openshift-ci
Bot
added
the
do-not-merge/hold
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-azure-ovn-rhcos10-techpreview |
|
@jluhrsen: 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/cb599d00-5a4f-11f1-9407-4e0b19a9d78a-0 |
![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 `@test/extended/networking/network_segmentation.go`:
- Around line 1326-1328: The isCurlExitCode28 function should use a typed
exit-status check instead of string matching: unwrap the error (from RunKubectl)
with errors.As into a k8s.io/utils/exec.CodeExitError (or interface type
exposing ExitStatus()), then return exitErr.ExitStatus() == 28; fall back to
false if errors.As fails or err is nil. Update isCurlExitCode28 to perform this
typed check rather than strings.Contains(err.Error(), "rc: 28").
🪄 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: 903494b6-a845-4c96-9bb8-f66fcd68e3f8
📒 Files selected for processing (1)
test/extended/networking/network_segmentation.go
| func isCurlExitCode28(err error) bool { | ||
| return err != nil && strings.Contains(err.Error(), "rc: 28") | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect current classifier
rg -n -C4 'func isCurlExitCode28\(' test/extended/networking/network_segmentation.go
# Verify RunKubectl builds CodeExitError with an exit code
rg -n -C6 'CodeExitError|rc :=|return stdout.String\(\), stderr.String\(\)' \
vendor/k8s.io/kubernetes/test/e2e/framework/kubectl/builder.go
# Verify CodeExitError supports typed exit-status checks
rg -n -C4 'type CodeExitError|func \(.*CodeExitError.*ExitStatus|func \(.*CodeExitError.*Error' \
vendor/k8s.io/utils/execRepository: openshift/origin
Length of output: 2715
Replace "rc: 28" error-string matching with typed exit-status checks
isCurlExitCode28 currently depends on strings.Contains(err.Error(), "rc: 28"), but RunKubectl wraps failures as k8s.io/utils/exec.CodeExitError, which exposes the exit code via ExitStatus(). Unwrap with errors.As and return exitErr.ExitStatus() == 28 instead.
🤖 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/networking/network_segmentation.go` around lines 1326 - 1328,
The isCurlExitCode28 function should use a typed exit-status check instead of
string matching: unwrap the error (from RunKubectl) with errors.As into a
k8s.io/utils/exec.CodeExitError (or interface type exposing ExitStatus()), then
return exitErr.ExitStatus() == 28; fall back to false if errors.As fails or err
is nil. Update isCurlExitCode28 to perform this typed check rather than
strings.Contains(err.Error(), "rc: 28").
|
Scheduling required tests: |
|
@jluhrsen: The following tests failed, say
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. |
jluhrsen
force-pushed
the
test-udn-settling-sleep
branch
from
98bb327 to
e398282
Compare
|
Actionable comments posted: 0 |
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-azure-ovn-rhcos10-techpreview |
|
@jluhrsen: 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/ff5b7fc0-5ad0-11f1-888e-1f5d7ebbb367-0 |
|
Scheduling required tests: |
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-azure-ovn-rhcos10-techpreview |
|
@jluhrsen: 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/93d65130-5b7a-11f1-869b-b7c8064d8c97-0 |
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-azure-ovn-rhcos10-techpreview |
|
@jluhrsen: 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/7bec6bc0-5b7c-11f1-8d24-b2e44a8ded3b-0 |
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-azure-ovn-rhcos10-techpreview |
|
@jluhrsen: 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/29020080-5b7e-11f1-9ee6-41df389bed03-0 |
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-azure-ovn-rhcos10-techpreview |
|
@jluhrsen: 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/d637c770-5b7f-11f1-8c45-4881bb3f02fa-0 |
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-azure-ovn-rhcos10-techpreview |
|
@jluhrsen: 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/84e70f00-5b81-11f1-8809-4cc3c58e4698-0 |
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-azure-ovn-rhcos10-techpreview |
|
@jluhrsen: 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/e58fc340-5b88-11f1-9845-8e380a4d5f32-0 |
This is a data collection PR to test whether network timing issues on RHCOS10+Azure are resolved by allowing more time for network setup to stabilize before running connectivity checks. After pods are created and IPs retrieved, wait 3 minutes for: - OVN flows to be fully installed - Network policies to be enforced - Pod networking to stabilize Using 3 minutes (conservative) to ensure we capture any slow initialization scenarios on RHCOS10+Azure. NOT FOR MERGE - data collection only
|
/payload-aggregate periodic-ci-openshift-release-main-nightly-4.22-e2e-azure-ovn-rhcos10-techpreview 10 |
jluhrsen
force-pushed
the
test-udn-settling-sleep
branch
from
e398282 to
9ae736b
Compare
|
@jluhrsen: 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/25066e40-5b91-11f1-9b91-50ae04db2950-0 |
|
Scheduling required tests: |
1 participant
Summary by CodeRabbit