Browse code

Merge pull request #8993 from smarterclayton/hold_router_lock

o Add locking around ops that change state. Its better to go lockless eventually and use a "shadow" (cloned) copy of the config when we do a reload. o Fixes as per @smarterclayton review comments.

Clayton Coleman authored on 2016/05/24 22:42:31
Showing 1 changed files
... ...
@@ -312,6 +312,9 @@ func (r *templateRouter) reloadRouter() error {
312 312
 }
313 313
 
314 314
 func (r *templateRouter) FilterNamespaces(namespaces sets.String) {
315
+	r.lock.Lock()
316
+	defer r.lock.Unlock()
317
+
315 318
 	if len(namespaces) == 0 {
316 319
 		r.state = make(map[string]ServiceUnit)
317 320
 	}
... ...
@@ -334,18 +337,33 @@ func (r *templateRouter) CreateServiceUnit(id string) {
334 334
 		EndpointTable:       []Endpoint{},
335 335
 	}
336 336
 
337
+	r.lock.Lock()
338
+	defer r.lock.Unlock()
339
+
337 340
 	r.state[id] = service
338 341
 }
339 342
 
340
-// FindServiceUnit finds the service with the given id.
341
-func (r *templateRouter) FindServiceUnit(id string) (ServiceUnit, bool) {
343
+// findMatchingServiceUnit finds the service with the given id - internal
344
+// lockless form, caller needs to ensure lock acquisition [and release].
345
+func (r *templateRouter) findMatchingServiceUnit(id string) (ServiceUnit, bool) {
342 346
 	v, ok := r.state[id]
343 347
 	return v, ok
344 348
 }
345 349
 
350
+// FindServiceUnit finds the service with the given id.
351
+func (r *templateRouter) FindServiceUnit(id string) (ServiceUnit, bool) {
352
+	r.lock.Lock()
353
+	defer r.lock.Unlock()
354
+
355
+	return r.findMatchingServiceUnit(id)
356
+}
357
+
346 358
 // DeleteServiceUnit deletes the service with the given id.
347 359
 func (r *templateRouter) DeleteServiceUnit(id string) {
348
-	svcUnit, ok := r.FindServiceUnit(id)
360
+	r.lock.Lock()
361
+	defer r.lock.Unlock()
362
+
363
+	svcUnit, ok := r.findMatchingServiceUnit(id)
349 364
 	if !ok {
350 365
 		return
351 366
 	}
... ...
@@ -353,15 +371,20 @@ func (r *templateRouter) DeleteServiceUnit(id string) {
353 353
 	for _, cfg := range svcUnit.ServiceAliasConfigs {
354 354
 		r.cleanUpServiceAliasConfig(&cfg)
355 355
 	}
356
+
356 357
 	delete(r.state, id)
357 358
 }
358 359
 
359 360
 // DeleteEndpoints deletes the endpoints for the service with the given id.
360 361
 func (r *templateRouter) DeleteEndpoints(id string) {
361
-	service, ok := r.FindServiceUnit(id)
362
+	r.lock.Lock()
363
+	defer r.lock.Unlock()
364
+
365
+	service, ok := r.findMatchingServiceUnit(id)
362 366
 	if !ok {
363 367
 		return
364 368
 	}
369
+
365 370
 	service.EndpointTable = []Endpoint{}
366 371
 
367 372
 	r.state[id] = service
... ...
@@ -391,8 +414,6 @@ func (r *templateRouter) routeKey(route *routeapi.Route) string {
391 391
 
392 392
 // AddRoute adds a route for the given id
393 393
 func (r *templateRouter) AddRoute(id string, route *routeapi.Route, host string) bool {
394
-	frontend, _ := r.FindServiceUnit(id)
395
-
396 394
 	backendKey := r.routeKey(route)
397 395
 
398 396
 	config := ServiceAliasConfig{
... ...
@@ -450,6 +471,10 @@ func (r *templateRouter) AddRoute(id string, route *routeapi.Route, host string)
450 450
 		}
451 451
 	}
452 452
 
453
+	r.lock.Lock()
454
+	defer r.lock.Unlock()
455
+
456
+	frontend, _ := r.findMatchingServiceUnit(id)
453 457
 	//create or replace
454 458
 	frontend.ServiceAliasConfigs[backendKey] = config
455 459
 	r.state[id] = frontend
... ...
@@ -478,6 +503,9 @@ func (r *templateRouter) cleanUpdates(frontendKey string, backendKey string) {
478 478
 
479 479
 // RemoveRoute removes the given route for the given id.
480 480
 func (r *templateRouter) RemoveRoute(id string, route *routeapi.Route) {
481
+	r.lock.Lock()
482
+	defer r.lock.Unlock()
483
+
481 484
 	serviceUnit, ok := r.state[id]
482 485
 	if !ok {
483 486
 		return
... ...
@@ -494,7 +522,9 @@ func (r *templateRouter) RemoveRoute(id string, route *routeapi.Route) {
494 494
 
495 495
 // AddEndpoints adds new Endpoints for the given id.
496 496
 func (r *templateRouter) AddEndpoints(id string, endpoints []Endpoint) bool {
497
-	frontend, _ := r.FindServiceUnit(id)
497
+	r.lock.Lock()
498
+	defer r.lock.Unlock()
499
+	frontend, _ := r.findMatchingServiceUnit(id)
498 500
 
499 501
 	//only make the change if there is a difference
500 502
 	if reflect.DeepEqual(frontend.EndpointTable, endpoints) {