Fix flaky TestServiceWithDefaultAddressPoolInit#39671
Conversation
arkodg
force-pushed
the
fix-flaky-addr-pool-init-test
branch
from
7146630 to
c186d46
Compare
arkodg
changed the title
arkodg
force-pushed
the
fix-flaky-addr-pool-init-test
branch
4 times, most recently
from
608d40e to
5d50ffa
Compare
thaJeztah
added
area/testing
process/cherry-pick
status/2-code-review
and removed
status/0-triage
labels
arkodg
force-pushed
the
fix-flaky-addr-pool-init-test
branch
4 times, most recently
from
34b90f4 to
fafea21
Compare
|
@thaJeztah any idea why the test case is failing ? |
arkodg
force-pushed
the
fix-flaky-addr-pool-init-test
branch
from
fafea21 to
53b8069
Compare
|
and changing the order of the operation breaks the next test |
|
is there a manager that failed to stop perhaps? |
arkodg
force-pushed
the
fix-flaky-addr-pool-init-test
branch
from
53b8069 to
c763936
Compare
Means someone is already listening on this port. Not sure what it means in this exact context. |
arkodg
force-pushed
the
fix-flaky-addr-pool-init-test
branch
9 times, most recently
from
c76e7bb to
9114cca
Compare
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
overall looks good; could you have a look at my comments, and move the function back to its old position (which makes it easier to double-check I didn't miss anything)
arkodg
force-pushed
the
fix-flaky-addr-pool-init-test
branch
from
2549161 to
952d643
Compare
|
Derek add label: rebuild/windowsRS1 |
|
Derek add label: rebuild/windowsRS1 |
1.This commit replaces serviceRunningCount with swarm.RunningTasksCount to accurately check if the service is running with the accurate number of instances or not. serviceRunningCount was only checking the ServiceList and was not checking if the tasks were running or not This adds a safe barrier to execute docker network inspect commands for overlay networks which get created asynchronously via Swarm 2. Make sure client connections are closed 3. Make sure every service and network name is unique 4. Make sure services and networks are cleaned up Signed-off-by: Arko Dasgupta <[email protected]>
arkodg
force-pushed
the
fix-flaky-addr-pool-init-test
branch
from
952d643 to
f3a3ea0
Compare
thaJeztah
changed the title
Signed-off-by: Arko Dasgupta <[email protected]>
|
PTAL @kolyshkin |
|
@tiborvass PTAL |
|
ping @tiborvass @kolyshkin PTAL 🤗 |
| poll.WaitOn(t, swarm.NoTasks(ctx, c), swarm.ServicePoll) | ||
| err = c.NetworkRemove(ctx, overlayID) | ||
| assert.NilError(t, err) | ||
| c.Close() |
There was a problem hiding this comment.
I see one potential problem with all the assert statements here. Once any assert fails, the text execution is cancelled, meaning the above cleanup code (remove/close/etc) won't be run.
Perhaps we need to change those assert. calls to check. ones. The difference is check. won't abort test execution immediately (but still mark the test as failed).
If it's not possible to use check. everywhere, maybe we need to do cleanup in a defer.
There was a problem hiding this comment.
Yes, but AFAIK the RootDir will be cleaned up so it won't affect another PR, as for the current PR, this test failed so the reason for the assert failure should be addressed before moving to the next test
6 participants
fixes #38514 Flaky test: TestServiceWithDefaultAddressPoolInit
This commit replaces serviceRunningCount with
swarm.RunningTasksCount to accurately check if the
service is running with the accurate number of instances
or not. serviceRunningCount was only checking the ServiceList
and was not checking if the tasks were running or not
This adds a safe barrier to execute docker network inspect
commands for overlay networks which get created
asynchronously via swarmkit
Signed-off-by: Arko Dasgupta [email protected]