backport: ci: add delay after link creation for test add remove static arp (#2968)#3556
backport: ci: add delay after link creation for test add remove static arp (#2968)#3556QxBytes wants to merge 1 commit intorelease/v1.5from
Conversation
fix by adding delay after link creation
There was a problem hiding this comment.
Pull Request Overview
A backport PR to add delays in the static ARP test to ensure that the network interface is fully up before ARP entries are modified.
- Added the time package and inserted delays before and after setting the ARP entries.
- Updated comments to clarify the timing rationale in the test.
Comments suppressed due to low confidence (2)
netlink/netlink_test.go:291
- [nitpick] Consider replacing the hardcoded delay value with a clearly named constant (e.g., interfaceUpDelay) to improve readability and ease future adjustments.
time.Sleep(time.Millisecond * 100)
netlink/netlink_test.go:311
- [nitpick] Similarly, consider using a named constant (e.g., arpPersistenceDelay) for this delay to clarify its purpose and allow for easier configuration changes.
time.Sleep(time.Millisecond * 100)
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
|
|
||
| // wait for interface to fully come up | ||
| // if it isn't fully up it might wipe the arp entry we're about to add | ||
| time.Sleep(time.Millisecond * 100) |
There was a problem hiding this comment.
Are we waiting for a specific state here? We should avoid this type of behavior and explicitly wait for a state of the netdev and timeout if the expected state is never seen.
The problem with this is it can result in flakes depending on several factors. Let’s be deterministic with this instead.
MikeZappa87
left a comment
MikeZappa87
left a comment
There was a problem hiding this comment.
Putting time.sleep is a flake waiting to happen. I pulled this from the merge queue.
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
|
Pull request closed due to inactivity. |
fix by adding delay after link creation
Reason for Change:
Backports #2968
Issue Fixed:
See above
Requirements:
Notes: