ci: fix wireserver and imds test flakes - #4828
Conversation
during wireserver and metadata server pod cleanup, noticed some failures during deletion of the pod after the command runs. when the deletion times out, we see "timed out waiting for condition" and the pod remains on the cluster. then, when we run kubectl run again with the same name, we get an error because the pod still exists. this change modifies the pod name so it is suffixed by metadata or wireserver so even if the pod fails to delete, we run the next command with a different pod name without conflict. if the pod fails to delete but still ran, the result of the kubectl run is of the inner command, so a bad delete shouldn't fail the test.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Changes recommended
The new $RANDOM-based suffix has low entropy and can still collide in parallel CI runs, undermining the intended pod-name uniqueness.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the network connectivity smoke test to avoid kubectl run pod-name collisions when prior --rm cleanup times out, improving CI reliability for repeated/parallel runs.
Changes:
- Introduces a per-run suffix used in pod names.
- Splits the single pod name into two distinct names for wireserver vs metadata checks.
File summaries
| File | Description |
|---|---|
| test/network/wireserver_metadata_test.sh | Adds a suffix and distinct pod names to prevent kubectl run name conflicts between the wireserver and metadata test invocations |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
/azp run Azure Container Networking PR |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
John Payne (jpayne3506)
left a comment
John Payne (jpayne3506)
left a comment
There was a problem hiding this comment.
lgtm. If we run into issues again, we should have a proper test start and tear down.
a84fd63
|
/azp run Azure Container Networking PR |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
node restart test may happen before so node might not be ready-- increases time for pod to startup
a84fd63 to
6df462c
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The new unique naming can mask recurring cleanup failures and lead to accumulating stranded pods without a best-effort cleanup safeguard.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The current suffix uses only $RANDOM, which can still collide across parallel/rerun scenarios and undermines the intended “unique pod name” reliability improvement.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
test/network/wireserver_metadata_test.sh:16
- Using bash $RANDOM (15-bit) as the sole uniqueness source can still collide across parallel CI jobs or repeated reruns, reintroducing the original "pod already exists" failure mode. Consider incorporating time and PID into the suffix to make collisions practically impossible.
SUFFIX="${RANDOM}"
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
f315d42 to
4c01ab1
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The changes are small, align with the stated CI failure mode, and the updated kubectl invocations look correct for Linux-only Busybox scheduling and unique pod naming.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
Alexander (QxBytes)
changed the title
|
/azp run Azure Container Networking PR |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Reason for Change:
On azure stateless e2e, we have windows nodes and run the imds metadata server sh test. Busybox, the image we use, only runs on linux, but the pod can get scheduled onto a windows node and fails to start up. This PR adds a selector so we only target linux, and also makes the run pod names unique.
Issue Fixed:
Requirements:
Notes: