Skip to content

Commit

Permalink
Fix race condition in EgressController and unit tests (#2307)
Browse files Browse the repository at this point in the history
ipAllocatorMap was read without holding the mutex in syncEgressIP. This
patch fixes it and stabilizes the unit tests by waiting for cache sync
before starting creating objects. This is because resource creation
events will be missing if the resources are created in-between list and
watch call of an fake informer.

Signed-off-by: Quan Tian <[email protected]>
  • Loading branch information
tnqn authored Jun 25, 2021
1 parent 95973ae commit 2d19196
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/controller/egress/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ func (c *EgressController) syncEgressIP(egress *egressv1alpha2.Egress) (net.IP,
return net.ParseIP(egress.Spec.EgressIP), nil
}

ipAllocator, exists := c.ipAllocatorMap[egress.Spec.ExternalIPPool]
ipAllocator, exists := c.getIPAllocator(egress.Spec.ExternalIPPool)
if !exists {
// The IP pool has been deleted, reclaim the IP from the Egress API.
if egress.Spec.EgressIP != "" {
Expand Down
6 changes: 6 additions & 0 deletions pkg/controller/egress/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,8 @@ func TestAddEgress(t *testing.T) {
controller := newController(fakeObjects, fakeCRDObjects)
controller.informerFactory.Start(stopCh)
controller.crdInformerFactory.Start(stopCh)
controller.informerFactory.WaitForCacheSync(stopCh)
controller.crdInformerFactory.WaitForCacheSync(stopCh)
go controller.groupingController.Run(stopCh)
go controller.Run(stopCh)

Expand Down Expand Up @@ -330,6 +332,8 @@ func TestUpdateEgress(t *testing.T) {
controller := newController([]runtime.Object{nsDefault, podFoo1}, []runtime.Object{eipFoo1, eipFoo2})
controller.informerFactory.Start(stopCh)
controller.crdInformerFactory.Start(stopCh)
controller.informerFactory.WaitForCacheSync(stopCh)
controller.crdInformerFactory.WaitForCacheSync(stopCh)
go controller.groupingController.Run(stopCh)
go controller.Run(stopCh)

Expand Down Expand Up @@ -644,6 +648,8 @@ func TestSyncEgressIP(t *testing.T) {
controller := newController(nil, fakeObjects)
controller.informerFactory.Start(stopCh)
controller.crdInformerFactory.Start(stopCh)
controller.informerFactory.WaitForCacheSync(stopCh)
controller.crdInformerFactory.WaitForCacheSync(stopCh)
controller.createOrUpdateIPAllocator(tt.existingExternalIPPool)
for _, egress := range tt.existingEgresses {
controller.updateIPAllocation(egress)
Expand Down

0 comments on commit 2d19196

Please sign in to comment.