Skip to content

Commit

Permalink
routesrv: fix flaky TestESkipBytesHandlerWithStaleEtag and others (#2534
Browse files Browse the repository at this point in the history 
)

Due to missing rs.StopUpdates() finished tests continue to write to the
shared test log which could be observed as a lot of "failed to fetch routes"
messages.
If k8s api server of the current test happens to start on the same port
as the server from the previous test then current test would react
on the log messages produced by the route server of the previous
test and may fail sporadically.

The better fix would be to get rid of the shared test logger
but this requires larger refactoring and could be done later.

Signed-off-by: Alexander Yastrebov <[email protected]>
  • Loading branch information
@AlexanderYastrebov
AlexanderYastrebov authored Aug 22, 2023
1 parent 559765f commit 533e310
Showing 1 changed file with 54 additions and 24 deletions.
78 changes: 54 additions & 24 deletions routesrv/routesrv_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,8 @@ func newKubeAPI(t *testing.T, specs ...io.Reader) http.Handler {
t.Helper()
api, err := kubernetestest.NewAPI(kubernetestest.TestAPIOptions{}, specs...)
if err != nil {
t.Errorf("cannot initialize kubernetes api: %s", err)
t.Fatalf("cannot initialize kubernetes api: %s", err)
}

return api
}

Expand All @@ -64,7 +63,6 @@ func loadKubeYAML(t *testing.T, path string) io.Reader {
if err != nil {
t.Fatalf("failed to open kubernetes resources fixture %s: %v", path, err)
}

return bytes.NewBuffer(y)
}

Expand All @@ -79,9 +77,8 @@ func newRouteServerWithOptions(t *testing.T, o skipper.Options) *routesrv.RouteS
t.Helper()
rs, err := routesrv.New(o)
if err != nil {
t.Errorf("cannot initialize server: %s", err)
t.Fatalf("cannot initialize server: %s", err)
}

return rs
}

Expand Down Expand Up @@ -140,6 +137,7 @@ func getRoutesWithRequestHeadersSetting(rs *routesrv.RouteServer, header map[str
}

func wantHTTPCode(t *testing.T, w *httptest.ResponseRecorder, want int) {
t.Helper()
got := w.Code
if got != want {
t.Errorf("wrong http status; %d != %d", got, want)
Expand Down Expand Up @@ -183,6 +181,8 @@ func TestEmptyRoutesAreNotServed(t *testing.T) {
rs := newRouteServer(t, ks)

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesEmpty, waitTimeout); err != nil {
t.Error("empty routes not received")
}
Expand All @@ -202,8 +202,10 @@ func TestFetchedRoutesAreServedInEskipFormat(t *testing.T) {
rs := newRouteServer(t, ks)

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Error("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
w := getRoutes(rs)

Expand Down Expand Up @@ -249,8 +251,10 @@ func TestFetchedIngressRoutesAreServedInEskipFormat(t *testing.T) {
})

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Fatal("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
w := getRoutes(rs)

Expand All @@ -273,15 +277,17 @@ func TestLastRoutesAreServedDespiteSourceFailure(t *testing.T) {
rs := newRouteServer(t, ks)

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Error("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
w1 := getRoutes(rs)
wantHTTPCode(t, w1, http.StatusOK)

ks.Close()
if err := tl.WaitFor(routesrv.LogRoutesFetchingFailed, waitTimeout); err != nil {
t.Error("source failure not recognized")
t.Fatalf("source failure not recognized: %v", err)
}
w2 := getRoutes(rs)

Expand All @@ -299,14 +305,16 @@ func TestRoutesAreUpdated(t *testing.T) {
rs := newRouteServer(t, ks)

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Error("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
w1 := getRoutes(rs)

handler.set(newKubeAPI(t, loadKubeYAML(t, "testdata/lb-target-single.yaml")))
if err := tl.WaitForN(routesrv.LogRoutesUpdated, 2, waitTimeout); err != nil {
t.Error("routes not updated")
t.Fatalf("source failure not recognized: %v", err)
}
w2 := getRoutes(rs)

Expand Down Expand Up @@ -340,8 +348,10 @@ func TestRoutesWithDefaultFilters(t *testing.T) {
})

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Error("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
w := getRoutes(rs)

Expand Down Expand Up @@ -369,8 +379,10 @@ func TestRoutesWithOAuth2Callback(t *testing.T) {
})

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Error("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
w := getRoutes(rs)

Expand Down Expand Up @@ -403,8 +415,10 @@ func TestRoutesWithEastWest(t *testing.T) {
})

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Error("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
w := getRoutes(rs)

Expand All @@ -427,8 +441,10 @@ func TestESkipBytesHandlerWithCorrectEtag(t *testing.T) {
rs := newRouteServer(t, ks)

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Fatal("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
w1 := getRoutes(rs)

Expand All @@ -452,8 +468,10 @@ func TestESkipBytesHandlerWithStaleEtag(t *testing.T) {
rs := newRouteServer(t, ks)

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Fatal("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
w1 := getRoutes(rs)
etag := w1.Header().Get("Etag")
Expand All @@ -462,7 +480,7 @@ func TestESkipBytesHandlerWithStaleEtag(t *testing.T) {
// update the routes, which also updates e.etag
handler.set(newKubeAPI(t, loadKubeYAML(t, "testdata/lb-target-single.yaml")))
if err := tl.WaitForN(routesrv.LogRoutesUpdated, 2, waitTimeout); err != nil {
t.Error("routes not updated")
t.Fatalf("routes not updated: %v", err)
}

w2 := getRoutesWithRequestHeadersSetting(rs, header)
Expand All @@ -483,8 +501,10 @@ func TestESkipBytesHandlerWithLastModified(t *testing.T) {
rs := newRouteServer(t, ks)

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Fatal("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
w1 := getRoutes(rs)

Expand All @@ -508,8 +528,10 @@ func TestESkipBytesHandlerWithOldLastModified(t *testing.T) {
rs := newRouteServer(t, ks)

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Fatal("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
w1 := getRoutes(rs)
lastModified := w1.Header().Get("Last-Modified")
Expand All @@ -522,7 +544,7 @@ func TestESkipBytesHandlerWithOldLastModified(t *testing.T) {
// update the routes, which also updated the e.lastModified
handler.set(newKubeAPI(t, loadKubeYAML(t, "testdata/lb-target-single.yaml")))
if err := tl.WaitForN(routesrv.LogRoutesUpdated, 2, waitTimeout); err != nil {
t.Error("routes not updated")
t.Fatalf("routes not updated: %v", err)
}

w2 := getRoutesWithRequestHeadersSetting(rs, header)
Expand All @@ -546,8 +568,10 @@ func TestESkipBytesHandlerWithXCount(t *testing.T) {
})

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Fatal("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
w1 := headRoutes(rs)
if n := w1.Body.Len(); n != 0 {
Expand Down Expand Up @@ -579,8 +603,10 @@ func TestRoutesWithEditRoute(t *testing.T) {
})

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Error("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
responseRecorder := getRoutes(rs)

Expand Down Expand Up @@ -609,8 +635,10 @@ func TestRoutesWithCloneRoute(t *testing.T) {
})

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Error("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
responseRecorder := getRoutes(rs)

Expand All @@ -637,8 +665,10 @@ func TestRoutesWithExplicitLBAlgorithm(t *testing.T) {
})

rs.StartUpdates()
defer rs.StopUpdates()

if err := tl.WaitFor(routesrv.LogRoutesInitialized, waitTimeout); err != nil {
t.Error("routes not initialized")
t.Fatalf("routes not initialized: %v", err)
}
responseRecorder := getRoutes(rs)

Expand Down

0 comments on commit 533e310

Please sign in to comment.