From a09527cf6d7d3cae180b0ae1a5b28c4dc1b29a27 Mon Sep 17 00:00:00 2001
From: Manuel Alejandro de Brito Fontes <aledbf@gmail.com>
Date: Tue, 2 Jan 2018 17:43:25 -0300
Subject: [PATCH] Fix data race updating ingress status (#1872)

---
 internal/ingress/controller/nginx.go   |  2 +-
 internal/ingress/status/status.go      | 25 +++++++++----------------
 internal/ingress/status/status_test.go |  9 ++++-----
 3 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go
index 1a8a06bc2..a21052039 100644
--- a/internal/ingress/controller/nginx.go
+++ b/internal/ingress/controller/nginx.go
@@ -268,7 +268,7 @@ func (n *NGINXController) Start() {
 	}
 
 	if n.syncStatus != nil {
-		go n.syncStatus.Run(n.stopCh)
+		go n.syncStatus.Run()
 	}
 
 	go wait.Until(n.checkMissingSecrets, 30*time.Second, n.stopCh)
diff --git a/internal/ingress/status/status.go b/internal/ingress/status/status.go
index 699b0a339..db7f34ea7 100644
--- a/internal/ingress/status/status.go
+++ b/internal/ingress/status/status.go
@@ -52,7 +52,7 @@ const (
 
 // Sync ...
 type Sync interface {
-	Run(stopCh <-chan struct{})
+	Run()
 	Shutdown()
 }
 
@@ -91,16 +91,8 @@ type statusSync struct {
 }
 
 // Run starts the loop to keep the status in sync
-func (s statusSync) Run(stopCh <-chan struct{}) {
-	go s.elector.Run()
-	go wait.Forever(s.update, updateInterval)
-	go s.syncQueue.Run(time.Second, stopCh)
-	<-stopCh
-}
-
-func (s *statusSync) update() {
-	// send a dummy object to the queue to force a sync
-	s.syncQueue.Enqueue("sync status")
+func (s statusSync) Run() {
+	s.elector.Run()
 }
 
 // Shutdown stop the sync. In case the instance is the leader it will remove the current IP
@@ -146,11 +138,6 @@ func (s *statusSync) sync(key interface{}) error {
 		return nil
 	}
 
-	if !s.elector.IsLeader() {
-		glog.V(2).Infof("skipping Ingress status update (I am not the current leader)")
-		return nil
-	}
-
 	addrs, err := s.runningAddresses()
 	if err != nil {
 		return err
@@ -188,6 +175,12 @@ func NewStatusSyncer(config Config) Sync {
 	callbacks := leaderelection.LeaderCallbacks{
 		OnStartedLeading: func(stop <-chan struct{}) {
 			glog.V(2).Infof("I am the new status update leader")
+			go st.syncQueue.Run(time.Second, stop)
+			wait.PollUntil(updateInterval, func() (bool, error) {
+				// send a dummy object to the queue to force a sync
+				st.syncQueue.Enqueue("sync status")
+				return false, nil
+			}, stop)
 		},
 		OnStoppedLeading: func() {
 			glog.V(2).Infof("I am not status update leader anymore")
diff --git a/internal/ingress/status/status_test.go b/internal/ingress/status/status_test.go
index 77bfb6969..4fe246abd 100644
--- a/internal/ingress/status/status_test.go
+++ b/internal/ingress/status/status_test.go
@@ -273,9 +273,8 @@ func TestStatusActions(t *testing.T) {
 
 	fk := fkSync.(statusSync)
 
-	ns := make(chan struct{})
 	// start it and wait for the election and syn actions
-	go fk.Run(ns)
+	go fk.Run()
 	//  wait for the election
 	time.Sleep(100 * time.Millisecond)
 	// execute sync
@@ -294,6 +293,8 @@ func TestStatusActions(t *testing.T) {
 		t.Fatalf("returned %v but expected %v", fooIngress1CurIPs, newIPs)
 	}
 
+	time.Sleep(1 * time.Second)
+
 	// execute shutdown
 	fk.Shutdown()
 	// ingress should be empty
@@ -314,9 +315,6 @@ func TestStatusActions(t *testing.T) {
 	if oic.Status.LoadBalancer.Ingress[0].IP != "0.0.0.0" && oic.Status.LoadBalancer.Ingress[0].Hostname != "foo.bar.com" {
 		t.Fatalf("invalid ingress status for rule with different class")
 	}
-
-	// end test
-	ns <- struct{}{}
 }
 
 func TestCallback(t *testing.T) {
@@ -325,6 +323,7 @@ func TestCallback(t *testing.T) {
 
 func TestKeyfunc(t *testing.T) {
 	fk := buildStatusSync()
+
 	i := "foo_base_pod"
 	r, err := fk.keyfunc(i)