Wait and retry connection to test CA server instead of failing (immediately) #2131
Conversation
github-actions
bot
added
the
needs triage
hslatman
force-pushed
the
herman/unflake-request-id-integration-test
branch
2 times, most recently
from
0af6425 to
018b51e
Compare
hslatman
force-pushed
the
herman/unflake-request-id-integration-test
branch
from
018b51e to
c2a6b5b
Compare
test/integration/requestid_test.go
Outdated
| d := net.Dialer{Timeout: 5 * time.Second} | ||
| conn, err := d.DialContext(ctx, "tcp", address) | ||
| if err != nil { | ||
| return false | ||
| } | ||
|
|
||
| conn.Close() | ||
|
|
||
| return true |
There was a problem hiding this comment.
It's faster if you just re-dial every 100ms, since you avoid running into TCP's exponential backoff.
Also, if the server isn't bound to the port yet, the kernel might send you a RST in response to the SYN immediately, which would make DialContext fail immediately.
There was a problem hiding this comment.
Returning an error immediately is OK, and kind of what I want. The CA server is started (or not, yet) in a different goroutine. The goal is to wait for it to become available before continuing the test, and a failure to connect here indicates it's not yet available.
test/integration/requestid_test.go
Outdated
| } | ||
|
|
||
| func canConnect(ctx context.Context, address string) bool { | ||
| d := net.Dialer{Timeout: 5 * time.Second} |
There was a problem hiding this comment.
Why hardcode a timeout, if you're dialing with a context anyway?
There was a problem hiding this comment.
Fair point. Added it out of habit. b3fb927.
The integration test checking for request IDs to be reflected depends on a test CA server to be started and running. At times (see this case with a few attempts) the CA healthcheck would fail the test, presumably because the CA server was not running yet. This PR checks the connection a few times before failing the test.