Summary: I'm running into a race condition during testing where my server is not reliably ready to serve requests before making client requests against it. How can I block only until the listener is ready, and still maintain composable public APIs without requiring users to BYO net.Listener
?
We see the following error as the goroutine that spins up our (blocking) server in the background isn't listening before we call client.Do(req)
in the TestRun
test function.
--- FAIL: TestRun/Server_accepts_HTTP_requests (0.00s)
/home/matt/repos/admission-control/server_test.go:64: failed to make a request: Get https://127.0.0.1:37877: dial tcp 127.0.0.1:37877: connect: connection refused
- I'm not using
httptest.Server
directly as I'm attempting to test the blocking & cancellation characteristics of my own server componenent. - I create an
httptest.NewUnstartedServer
, clone its*tls.Config
into a newhttp.Server
after starting it withStartTLS()
, and then close it, before calling*AdmissionServer.Run()
. This also has the benefit of giving me a*http.Client
with the matching RootCAs configured. - Testing TLS is important here as the daemon this exposes lives in a TLS-only environment.
func newTestServer(ctx context.Context, t *testing.T) *httptest.Server {
testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintln(w, "OK")
})
testSrv := httptest.NewUnstartedServer(testHandler)
admissionServer, err := NewServer(nil, &noopLogger{})
if err != nil {
t.Fatalf("admission server creation failed: %s", err)
return nil
}
// We start the test server, copy its config out, and close it down so we can
// start our own server. This is because httptest.Server only generates a
// self-signed TLS config after starting it.
testSrv.StartTLS()
admissionServer.srv = &http.Server{
Addr: testSrv.Listener.Addr().String(),
Handler: testHandler,
TLSConfig: testSrv.TLS.Clone(),
}
testSrv.Close()
// We need a better synchronization primitive here that doesn't block
// but allows the underlying listener to be ready before
// serving client requests.
go func() {
if err := admissionServer.Run(ctx); err != nil {
t.Fatalf("server returned unexpectedly: %s", err)
}
}()
return testSrv
}
// Test that we can start a minimal AdmissionServer and handle a request.
func TestRun(t *testing.T) {
testSrv := newTestServer(context.TODO(), t)
t.Run("Server accepts HTTP requests", func(t *testing.T) {
client := testSrv.Client()
req, err := http.NewRequest(http.MethodGet, testSrv.URL, nil)
if err != nil {
t.Fatalf("request creation failed: %s", err)
}
resp, err := client.Do(req)
if err != nil {
t.Fatalf("failed to make a request: %s", err)
}
// Later sub-tests will test cancellation propagation, signal handling, etc.
For posterity, this is our composable Run
function, that listens in a goroutine and then blocks on our cancellation & error channels in a for-select
:
type AdmissionServer struct {
srv *http.Server
logger log.Logger
GracePeriod time.Duration
}
func (as *AdmissionServer) Run(ctx context.Context) error {
sigChan := make(chan os.Signal, 1)
defer close(sigChan)
signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM)
// run in goroutine
errs := make(chan error)
defer close(errs)
go func() {
as.logger.Log(
"msg", fmt.Sprintf("admission control listening on '%s'", as.srv.Addr),
)
if err := as.srv.ListenAndServeTLS("", ""); err != nil && err != http.ErrServerClosed {
errs <- err
as.logger.Log(
"err", err.Error(),
"msg", "the server exited",
)
return
}
return
}()
// Block indefinitely until we receive an interrupt, cancellation or error
// signal.
for {
select {
case sig := <-sigChan:
as.logger.Log(
"msg", fmt.Sprintf("signal received: %s", sig),
)
return as.shutdown(ctx, as.GracePeriod)
case err := <-errs:
as.logger.Log(
"msg", fmt.Sprintf("listener error: %s", err),
)
// We don't need to explictly call shutdown here, as
// *http.Server.ListenAndServe closes the listener when returning an error.
return err
case <-ctx.Done():
as.logger.Log(
"msg", fmt.Sprintf("cancellation received: %s", ctx.Err()),
)
return as.shutdown(ctx, as.GracePeriod)
}
}
}
Notes:
- There is a (simple) constructor for an
*AdmissionServer
: I've left it out for brevity. TheAdmissionServer
is composable and accepts a*http.Server
so that it can be plugged into existing applications easily. - The wrapped
http.Server
type that we create a listener from doesn't itself expose any way to tell if its listening; at best we can try to listen again and catch the error (e.g. port already bound to another listener), which does not seem robust as thenet
package doesn't expose a useful typed error for this.