Skip to content

Commit 12d7c69

Browse files
author
Rajat Chopra
committed
Fix issue#10853. Route cleanup does not need service key cleanup.
1 parent 31f2273 commit 12d7c69

File tree

4 files changed

+15
-31
lines changed

4 files changed

+15
-31
lines changed

pkg/router/template/plugin.go

+7-11
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ type routerInterface interface {
7878
// pointed to by the route. Returns true if a
7979
// change was made and the state should be stored with Commit().
8080
AddRoute(id string, weight int32, route *routeapi.Route, host string) bool
81-
// RemoveRoute removes the given route for the given id.
82-
RemoveRoute(id string, route *routeapi.Route)
81+
// RemoveRoute removes the given route
82+
RemoveRoute(route *routeapi.Route)
8383
// Reduce the list of routes to only these namespaces
8484
FilterNamespaces(namespaces sets.String)
8585
// 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.
188188
switch eventType {
189189
case watch.Added, watch.Modified:
190190
// Delete the route first, because modify is to be treated as delete+add
191-
for i := range serviceKeys {
192-
key := serviceKeys[i]
193-
p.Router.RemoveRoute(key, route)
194-
}
195-
// Now add it back again
191+
p.Router.RemoveRoute(route)
192+
193+
// Now add the route back again
196194
commit := false
197195
for i := range serviceKeys {
198196
key := serviceKeys[i]
@@ -210,10 +208,8 @@ func (p *TemplatePlugin) HandleRoute(eventType watch.EventType, route *routeapi.
210208
p.Router.Commit()
211209
}
212210
case watch.Deleted:
213-
for _, key := range serviceKeys {
214-
glog.V(4).Infof("Deleting routes for %s", key)
215-
p.Router.RemoveRoute(key, route)
216-
}
211+
glog.V(4).Infof("Deleting route %v", route)
212+
p.Router.RemoveRoute(route)
217213
p.Router.Commit()
218214
}
219215
return nil

pkg/router/template/plugin_test.go

+4-7
Original file line numberDiff line numberDiff line change
@@ -194,18 +194,15 @@ func (r *TestRouter) AddRoute(id string, weight int32, route *routeapi.Route, ho
194194
return true
195195
}
196196

197-
// RemoveRoute removes the service alias config for Route from the ServiceUnit
198-
func (r *TestRouter) RemoveRoute(id string, route *routeapi.Route) {
197+
// RemoveRoute removes the service alias config for Route
198+
func (r *TestRouter) RemoveRoute(route *routeapi.Route) {
199199
r.Committed = false //expect any call to this method to subsequently call commit
200200
routeKey := r.routeKey(route)
201-
serviceAliasConfig, ok := r.State[routeKey]
201+
_, ok := r.State[routeKey]
202202
if !ok {
203203
return
204204
} else {
205-
delete(serviceAliasConfig.ServiceUnitNames, id)
206-
if len(serviceAliasConfig.ServiceUnitNames) == 0 {
207-
delete(r.State, routeKey)
208-
}
205+
delete(r.State, routeKey)
209206
}
210207
}
211208

pkg/router/template/router.go

+3-12
Original file line numberDiff line numberDiff line change
@@ -593,28 +593,19 @@ func (r *templateRouter) AddRoute(serviceID string, weight int32, route *routeap
593593
return true
594594
}
595595

596-
// RemoveRoute removes the given route for the given id.
597-
func (r *templateRouter) RemoveRoute(id string, route *routeapi.Route) {
596+
// RemoveRoute removes the given route
597+
func (r *templateRouter) RemoveRoute(route *routeapi.Route) {
598598
r.lock.Lock()
599599
defer r.lock.Unlock()
600600

601-
_, ok := r.serviceUnits[id]
602-
if !ok {
603-
return
604-
}
605-
606601
routeKey := r.routeKey(route)
607602
serviceAliasConfig, ok := r.state[routeKey]
608603
if !ok {
609604
return
610605
}
611-
delete(serviceAliasConfig.ServiceUnitNames, id)
612606

613607
r.cleanUpServiceAliasConfig(&serviceAliasConfig)
614-
615-
if len(serviceAliasConfig.ServiceUnitNames) == 0 {
616-
delete(r.state, routeKey)
617-
}
608+
delete(r.state, routeKey)
618609
}
619610

620611
// AddEndpoints adds new Endpoints for the given id.

pkg/router/template/router_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ func TestRemoveRoute(t *testing.T) {
402402
t.Fatalf("Route %v did not match serivce alias config %v", route, saCfg)
403403
}
404404

405-
router.RemoveRoute(suKey, route)
405+
router.RemoveRoute(route)
406406
if _, ok := router.state[routeKey]; ok {
407407
t.Errorf("Route %v was expected to be deleted but was still found", route)
408408
}

0 commit comments

Comments
 (0)