Skip to content

Commit 4e5c579

Browse files
committed
Fix broken router stress test
The router stress test was previously assuming that the router had seen a route if it had a corresponding service unit. Since endpoints can result in service units, the fact that all routes were using the same host was not failing the test. This change ensures that the test uses unique hosts and properly detects whether a router has seen a route.
1 parent 63db16f commit 4e5c579

File tree

4 files changed

+16
-22
lines changed

4 files changed

+16
-22
lines changed

pkg/router/template/plugin.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,6 @@ type routerInterface interface {
6060
// frontend key is used; all call sites make certain the frontend
6161
// is created.
6262

63-
// HasServiceUnit indicates whether the router has a service unit
64-
// for the given key.
65-
HasServiceUnit(key string) bool
66-
6763
// CreateServiceUnit creates a new service named with the given id.
6864
CreateServiceUnit(id string)
6965
// FindServiceUnit finds the service with the given id.
@@ -82,6 +78,8 @@ type routerInterface interface {
8278
AddRoute(id string, weight int32, route *routeapi.Route, host string) bool
8379
// RemoveRoute removes the given route
8480
RemoveRoute(route *routeapi.Route)
81+
// HasRoute indicates whether the router is configured with the given route
82+
HasRoute(route *routeapi.Route) bool
8583
// Reduce the list of routes to only these namespaces
8684
FilterNamespaces(namespaces sets.String)
8785
// Commit applies the changes in the background. It kicks off a rate-limited

pkg/router/template/plugin_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,11 @@ func (r *TestRouter) RemoveRoute(route *routeapi.Route) {
206206
}
207207
}
208208

209+
func (r *TestRouter) HasRoute(route *routeapi.Route) bool {
210+
// Not used
211+
return false
212+
}
213+
209214
func (r *TestRouter) FilterNamespaces(namespaces sets.String) {
210215
if len(namespaces) == 0 {
211216
r.State = make(map[string]ServiceAliasConfig)
@@ -246,10 +251,6 @@ func (r *TestRouter) SetSkipCommit(skipCommit bool) {
246251
func (r *TestRouter) SetSyncedAtLeastOnce() {
247252
}
248253

249-
func (r *TestRouter) HasServiceUnit(key string) bool {
250-
return false
251-
}
252-
253254
// TestHandleEndpoints test endpoint watch events
254255
func TestHandleEndpoints(t *testing.T) {
255256
testCases := []struct {

pkg/router/template/router.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -755,12 +755,12 @@ func (r *templateRouter) SetSyncedAtLeastOnce() {
755755
glog.V(4).Infof("Router state synchronized for the first time")
756756
}
757757

758-
// HasServiceUnit attempts to retrieve a service unit for the given
759-
// key, returning a boolean indication of whether the key is known.
760-
func (r *templateRouter) HasServiceUnit(key string) bool {
758+
// HasRoute indicates whether the given route is known to this router.
759+
func (r *templateRouter) HasRoute(route *routeapi.Route) bool {
761760
r.lock.Lock()
762761
defer r.lock.Unlock()
763-
_, ok := r.findMatchingServiceUnit(key)
762+
key := r.routeKey(route)
763+
_, ok := r.state[key]
764764
return ok
765765
}
766766

test/integration/router_stress_test.go

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ func stressRouter(t *testing.T, namespaceCount, routesPerNamespace, routerCount,
7878
}
7979

8080
// Create a route
81-
routeProperties := createRouteProperties(service.Name)
81+
host := fmt.Sprintf("www-%d-%d.example.com", i, j)
82+
routeProperties := createRouteProperties(service.Name, host)
8283
route, err := oc.Routes(namespace.Name).Create(routeProperties)
8384
if err != nil {
8485
t.Fatalf("unexpected error: %v", err)
@@ -112,8 +113,7 @@ func stressRouter(t *testing.T, namespaceCount, routesPerNamespace, routerCount,
112113
routeCount := 0
113114
for _, plugin := range plugins {
114115
for _, route := range routes {
115-
key := routeKey(route)
116-
if plugin.Router.HasServiceUnit(key) {
116+
if plugin.Router.HasRoute(route) {
117117
routeCount++
118118
}
119119
}
@@ -132,11 +132,6 @@ func stressRouter(t *testing.T, namespaceCount, routesPerNamespace, routerCount,
132132
}
133133
}
134134

135-
// TODO(marun) reuse a public definition instead of copying.
136-
func routeKey(route *routeapi.Route) string {
137-
return fmt.Sprintf("%s/%s", route.Namespace, route.Spec.To.Name)
138-
}
139-
140135
func createNamespaceProperties() *kapi.Namespace {
141136
return &kapi.Namespace{
142137
ObjectMeta: kapi.ObjectMeta{
@@ -176,13 +171,13 @@ func createEndpointsProperties(serviceName string) *kapi.Endpoints {
176171
}
177172
}
178173

179-
func createRouteProperties(serviceName string) *routeapi.Route {
174+
func createRouteProperties(serviceName, host string) *routeapi.Route {
180175
return &routeapi.Route{
181176
ObjectMeta: kapi.ObjectMeta{
182177
GenerateName: "route-",
183178
},
184179
Spec: routeapi.RouteSpec{
185-
Host: "www.example.com",
180+
Host: host,
186181
Path: "",
187182
To: routeapi.RouteTargetReference{
188183
Name: serviceName,

0 commit comments

Comments
 (0)