Wait for a container to be running #1900
Conversation
| tick := time.Tick(tickSeconds) | ||
| timer := time.After(timeout) | ||
|
|
||
| for { |
There was a problem hiding this comment.
We have a Retry method that is currently unused, maybe we can move it to the common package and use that here?
There was a problem hiding this comment.
Actually, just noticed the changes are inside the executor package, so it could be used directly if we wanted to.
There was a problem hiding this comment.
The original Retry was number based, but in this context time based retry is more appropriate. I've moved the ticker implementation into the retry module.
| for { | ||
| select { | ||
| case <-tick: | ||
| inspect, err := d.client.ContainerExecInspect(ctx, resp.ID) |
There was a problem hiding this comment.
We might want to directly check if there was an error here:
| inspect, err := d.client.ContainerExecInspect(ctx, resp.ID) | |
| inspect, err := d.client.ContainerExecInspect(ctx, resp.ID) | |
| if err != nil { | |
| log.Info("Failed to inspect %s: %s", startConfig.Name, err) | |
| continue | |
| } |
Then we can remove it from err from the log message on line 148, since it's currently causing some noise:
2024/10/21 15:41:38 INFO: Wait for container container-stats to start, %!w(errdefs.errNotFound=***0xc000616a08***)
2024/10/21 15:41:39 INFO: Wait for container container-stats to start, %!w(errdefs.errNotFound=***0xc000616b58***)
2024/10/21 15:41:40 INFO: Wait for container container-stats to start, %!w(errdefs.errNotFound=***0xc000616d08***)
2024/10/21 15:41:41 INFO: Wait for container container-stats to start, %!w(errdefs.errNotFound=***0xc000616fa8***)
erthalion
force-pushed
the
feature/wait-for-containers
branch
from
81b627a
to
15ba21e
Compare
Co-authored-by: Mauro Ezequiel Moltrasio <[email protected]>
erthalion
force-pushed
the
feature/wait-for-containers
branch
from
15ba21e
to
49f2d17
Compare
| // | ||
| // Note that the caller is responsible for reporting outstanding errors in | ||
| // ticker function | ||
| func RetryWithTimeout(f retryable, timeoutMsg error) ( |
There was a problem hiding this comment.
In the future, we might want to add an argument for specifying the timeout, but for now it's good enough.
Description
TestProcessListeningOnPort has became flaky in a nasty way, leaving no logs
from the flask container. Most likely reason is that it takes some time to
start the container, while the client method ContainerCreate doesn't wait for
"running" status.
Make the test more robust by waiting until it reports running status. It's been
done using ContainerExecInspect, not ContainerWait, because the latter one
isn't suitable despite the name. ContainerWait [1] could only wait for a
non-running state, and is designed to wait until a container has finished it's
job.
Checklist
Automated testing
Testing Performed
Manual testing, running the flaky test.