Improve NodePort test readiness and diagnostics - #502
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d93fa31c83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .endSpec() | ||
| .build())); | ||
|
|
||
| waitForPodAndServiceEndpoint(k8s, pod, service); |
There was a problem hiding this comment.
When the pod never becomes Ready or the Service never receives an endpoint, either newly added Awaitility wait throws before entering this try, so logNodePortDiagnostics is skipped. Those are precisely the failures where pod, endpoint, and node state would be most useful; wrap the readiness call in the same failure-handling block so all post-creation failures collect diagnostics.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. I moved the pod/endpoint readiness waits into the same guarded block as the NodePort probe, so any post-creation failure now collects the diagnostics before rethrowing.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 902bf80317
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .get()); | ||
| }); | ||
| LOG.warn("NodePort diagnostics - last host probe result: {}", lastHostProbeResult.get()); | ||
| logNodePortDiagnostics(k8s, pod, service); |
There was a problem hiding this comment.
When readiness fails because the API server or container has become unavailable, logNodePortDiagnostics immediately performs unguarded Kubernetes API calls and later container execs; a resulting runtime exception replaces the original timeout and prevents the remaining best-effort diagnostics from running. Catch and log failures from each diagnostic operation before rethrowing the saved exception so CI reports the actual test failure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. Diagnostics are now best-effort: Kubernetes API collection is guarded, Docker inspection already was, and container exec failures (including runtime failures) are caught individually. The original test exception is always rethrown unchanged after diagnostics.
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
This separates workload/service readiness from NodePort forwarding and should make intermittent CI failures both less likely and much easier to classify.