Merge pull request #10855 from rajatchopra/router_issue10853

Merged by openshift-bot

pkg/router/template/plugin.go

@@ -78,8 +78,8 @@ type routerInterface interface {
 	// pointed to by the route. Returns true if a
 	// change was made and the state should be stored with Commit().
 	AddRoute(id string, weight int32, route *routeapi.Route, host string) bool
-	// RemoveRoute removes the given route for the given id.
-	RemoveRoute(id string, route *routeapi.Route)
+	// RemoveRoute removes the given route
+	RemoveRoute(route *routeapi.Route)
 	// Reduce the list of routes to only these namespaces
 	FilterNamespaces(namespaces sets.String)
 	// Commit applies the changes in the background. It kicks off a rate-limited
@@ -188,11 +188,9 @@ func (p *TemplatePlugin) HandleRoute(eventType watch.EventType, route *routeapi.
 	switch eventType {
 	case watch.Added, watch.Modified:
 		// Delete the route first, because modify is to be treated as delete+add
-		for i := range serviceKeys {
-			key := serviceKeys[i]
-			p.Router.RemoveRoute(key, route)
-		}
-		// Now add it back again
+		p.Router.RemoveRoute(route)
+
+		// Now add the route back again
 		commit := false
 		for i := range serviceKeys {
 			key := serviceKeys[i]
@@ -210,10 +208,8 @@ func (p *TemplatePlugin) HandleRoute(eventType watch.EventType, route *routeapi.
 			p.Router.Commit()
 		}
 	case watch.Deleted:
-		for _, key := range serviceKeys {
-			glog.V(4).Infof("Deleting routes for %s", key)
-			p.Router.RemoveRoute(key, route)
-		}
+		glog.V(4).Infof("Deleting route %v", route)
+		p.Router.RemoveRoute(route)
 		p.Router.Commit()
 	}
 	return nil

pkg/router/template/plugin_test.go

@@ -194,18 +194,15 @@ func (r *TestRouter) AddRoute(id string, weight int32, route *routeapi.Route, ho
 	return true
 }
 
-// RemoveRoute removes the service alias config for Route from the ServiceUnit
-func (r *TestRouter) RemoveRoute(id string, route *routeapi.Route) {
+// RemoveRoute removes the service alias config for Route
+func (r *TestRouter) RemoveRoute(route *routeapi.Route) {
 	r.Committed = false //expect any call to this method to subsequently call commit
 	routeKey := r.routeKey(route)
-	serviceAliasConfig, ok := r.State[routeKey]
+	_, ok := r.State[routeKey]
 	if !ok {
 		return
 	} else {
-		delete(serviceAliasConfig.ServiceUnitNames, id)
-		if len(serviceAliasConfig.ServiceUnitNames) == 0 {
-			delete(r.State, routeKey)
-		}
+		delete(r.State, routeKey)
 	}
 }
 

pkg/router/template/router.go

@@ -593,28 +593,19 @@ func (r *templateRouter) AddRoute(serviceID string, weight int32, route *routeap
 	return true
 }
 
-// RemoveRoute removes the given route for the given id.
-func (r *templateRouter) RemoveRoute(id string, route *routeapi.Route) {
+// RemoveRoute removes the given route
+func (r *templateRouter) RemoveRoute(route *routeapi.Route) {
 	r.lock.Lock()
 	defer r.lock.Unlock()
 
-	_, ok := r.serviceUnits[id]
-	if !ok {
-		return
-	}
-
 	routeKey := r.routeKey(route)
 	serviceAliasConfig, ok := r.state[routeKey]
 	if !ok {
 		return
 	}
-	delete(serviceAliasConfig.ServiceUnitNames, id)
 
 	r.cleanUpServiceAliasConfig(&serviceAliasConfig)
-
-	if len(serviceAliasConfig.ServiceUnitNames) == 0 {
-		delete(r.state, routeKey)
-	}
+	delete(r.state, routeKey)
 }
 
 // AddEndpoints adds new Endpoints for the given id.

pkg/router/template/router_test.go

@@ -402,7 +402,7 @@ func TestRemoveRoute(t *testing.T) {
 		t.Fatalf("Route %v did not match serivce alias config %v", route, saCfg)
 	}
 
-	router.RemoveRoute(suKey, route)
+	router.RemoveRoute(route)
 	if _, ok := router.state[routeKey]; ok {
 		t.Errorf("Route %v was expected to be deleted but was still found", route)
 	}