Fix status update in case of connection errors

build/e2e-tests.sh

@@ -41,19 +41,6 @@ fi
 
 SCRIPT_ROOT=$(dirname ${BASH_SOURCE})/..
 
-mkdir -p ${SCRIPT_ROOT}/test/binaries
-
-TEST_BINARIES=$( cd "${SCRIPT_ROOT}/test/binaries" ; pwd -P )
-
-export PATH=${TEST_BINARIES}:$PATH
-
-if ! [ -x "$(command -v kubectl)" ]; then
-    echo "downloading kubectl..."
-    curl -sSLo ${TEST_BINARIES}/kubectl \
-        https://storage.googleapis.com/kubernetes-release/release/v1.11.0/bin/linux/amd64/kubectl
-    chmod +x ${TEST_BINARIES}/kubectl
-fi
-
 ginkgo build ./test/e2e
 
 exec --                                      \

build/go-in-docker.sh

@@ -40,7 +40,7 @@ if [ "$missing" = true ];then
   exit 1
 fi
 
-E2E_IMAGE=quay.io/kubernetes-ingress-controller/e2e:v10292018-5d42f0e
+E2E_IMAGE=quay.io/kubernetes-ingress-controller/e2e:v10292018-240c7274b
 
 DOCKER_OPTS=${DOCKER_OPTS:-""}
 

images/e2e/Dockerfile

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-FROM quay.io/kubernetes-ingress-controller/nginx-amd64:0.63
+FROM quay.io/kubernetes-ingress-controller/nginx-amd64:0.66
 
 RUN clean-install \
   g++ \
@@ -61,3 +61,7 @@ RUN  luarocks install luacheck \
 
 RUN  go get github.com/onsi/ginkgo/ginkgo \
   && go get golang.org/x/lint/golint
+
+RUN  curl -Lo /usr/local/bin/kubectl https://storage.googleapis.com/kubernetes-release/release/v1.12.0/bin/linux/amd64/kubectl \
+  && chmod +x /usr/local/bin/kubectl
+

internal/ingress/controller/nginx.go

@@ -18,7 +18,6 @@ package controller
 
 import (
 	"bytes"
-	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -257,7 +256,7 @@ func (n *NGINXController) Start() {
 	n.store.Run(n.stopCh)
 
 	if n.syncStatus != nil {
-		go n.syncStatus.Run(context.Background())
+		go n.syncStatus.Run()
 	}
 
 	cmd := nginxExecCommand()

internal/ingress/status/status.go

@@ -51,7 +51,7 @@ const (
 
 // Sync ...
 type Sync interface {
-	Run(ctx context.Context)
+	Run()
 	Shutdown()
 }
 
@@ -93,22 +93,97 @@ type statusSync struct {
 	pod *k8s.PodInfo
 
 	elector *leaderelection.LeaderElector
+
 	// workqueue used to keep in sync the status IP/s
 	// in the Ingress rules
 	syncQueue *task.Queue
 }
 
 // Run starts the loop to keep the status in sync
-func (s statusSync) Run(ctx context.Context) {
-	s.elector.Run(ctx)
+func (s statusSync) Run() {
+	// we need to use the defined ingress class to allow multiple leaders
+	// in order to update information about ingress status
+	electionID := fmt.Sprintf("%v-%v", s.Config.ElectionID, s.Config.DefaultIngressClass)
+	if s.Config.IngressClass != "" {
+		electionID = fmt.Sprintf("%v-%v", s.Config.ElectionID, s.Config.IngressClass)
+	}
+
+	// start a new context
+	ctx := context.Background()
+	// allow to cancel the context in case we stop being the leader
+	leaderCtx, cancel := context.WithCancel(ctx)
+
+	var stopCh chan struct{}
+	callbacks := leaderelection.LeaderCallbacks{
+		OnStartedLeading: func(ctx context.Context) {
+			glog.V(2).Infof("I am the new status update leader")
+			stopCh = make(chan struct{})
+			go s.syncQueue.Run(time.Second, stopCh)
+			// trigger initial sync
+			s.syncQueue.EnqueueTask(task.GetDummyObject("sync status"))
+			// when this instance is the leader we need to enqueue
+			// an item to trigger the update of the Ingress status.
+			wait.PollUntil(updateInterval, func() (bool, error) {
+				s.syncQueue.EnqueueTask(task.GetDummyObject("sync status"))
+				return false, nil
+			}, stopCh)
+		},
+		OnStoppedLeading: func() {
+			glog.V(2).Infof("I am not status update leader anymore")
+			close(stopCh)
+
+			// cancel the context
+			cancel()
+
+			// start a new context and run the elector
+			leaderCtx, cancel = context.WithCancel(ctx)
+			go s.elector.Run(leaderCtx)
+		},
+		OnNewLeader: func(identity string) {
+			glog.Infof("new leader elected: %v", identity)
+		},
+	}
+
+	broadcaster := record.NewBroadcaster()
+	hostname, _ := os.Hostname()
+
+	recorder := broadcaster.NewRecorder(scheme.Scheme, apiv1.EventSource{
+		Component: "ingress-leader-elector",
+		Host:      hostname,
+	})
+
+	lock := resourcelock.ConfigMapLock{
+		ConfigMapMeta: metav1.ObjectMeta{Namespace: s.pod.Namespace, Name: electionID},
+		Client:        s.Config.Client.CoreV1(),
+		LockConfig: resourcelock.ResourceLockConfig{
+			Identity:      s.pod.Name,
+			EventRecorder: recorder,
+		},
+	}
+
+	ttl := 30 * time.Second
+	le, err := leaderelection.NewLeaderElector(leaderelection.LeaderElectionConfig{
+		Lock:          &lock,
+		LeaseDuration: ttl,
+		RenewDeadline: ttl / 2,
+		RetryPeriod:   ttl / 4,
+		Callbacks:     callbacks,
+	})
+	if err != nil {
+		glog.Fatalf("unexpected error starting leader election: %v", err)
+	}
+	s.elector = le
+
+	go le.Run(leaderCtx)
 }
 
 // Shutdown stop the sync. In case the instance is the leader it will remove the current IP
 // if there is no other instances running.
 func (s statusSync) Shutdown() {
 	go s.syncQueue.Shutdown()
+
 	// remove IP from Ingress
-	if !s.elector.IsLeader() {
+	if s.elector != nil && !s.elector.IsLeader() {
 		return
 	}
 
@@ -146,6 +221,10 @@ func (s *statusSync) sync(key interface{}) error {
 		return nil
 	}
 
+	if s.elector != nil && !s.elector.IsLeader() {
+		return fmt.Errorf("i am not the current leader. Skiping status update")
+	}
+
 	addrs, err := s.runningAddresses()
 	if err != nil {
 		return err
@@ -173,66 +252,6 @@ func NewStatusSyncer(config Config) Sync {
 	}
 	st.syncQueue = task.NewCustomTaskQueue(st.sync, st.keyfunc)
 
-	// we need to use the defined ingress class to allow multiple leaders
-	// in order to update information about ingress status
-	electionID := fmt.Sprintf("%v-%v", config.ElectionID, config.DefaultIngressClass)
-	if config.IngressClass != "" {
-		electionID = fmt.Sprintf("%v-%v", config.ElectionID, config.IngressClass)
-	}
-
-	var stopCh chan struct{}
-	callbacks := leaderelection.LeaderCallbacks{
-		OnStartedLeading: func(ctx context.Context) {
-			glog.V(2).Infof("I am the new status update leader")
-			stopCh = make(chan struct{})
-			go st.syncQueue.Run(time.Second, stopCh)
-			// when this instance is the leader we need to enqueue
-			// an item to trigger the update of the Ingress status.
-			wait.PollUntil(updateInterval, func() (bool, error) {
-				st.syncQueue.EnqueueTask(task.GetDummyObject("sync status"))
-				return false, nil
-			}, stopCh)
-		},
-		OnStoppedLeading: func() {
-			glog.V(2).Infof("I am not status update leader anymore")
-			close(stopCh)
-		},
-		OnNewLeader: func(identity string) {
-			glog.Infof("new leader elected: %v", identity)
-		},
-	}
-
-	broadcaster := record.NewBroadcaster()
-	hostname, _ := os.Hostname()
-
-	recorder := broadcaster.NewRecorder(scheme.Scheme, apiv1.EventSource{
-		Component: "ingress-leader-elector",
-		Host:      hostname,
-	})
-
-	lock := resourcelock.ConfigMapLock{
-		ConfigMapMeta: metav1.ObjectMeta{Namespace: pod.Namespace, Name: electionID},
-		Client:        config.Client.CoreV1(),
-		LockConfig: resourcelock.ResourceLockConfig{
-			Identity:      pod.Name,
-			EventRecorder: recorder,
-		},
-	}
-
-	ttl := 30 * time.Second
-	le, err := leaderelection.NewLeaderElector(leaderelection.LeaderElectionConfig{
-		Lock:          &lock,
-		LeaseDuration: ttl,
-		RenewDeadline: ttl / 2,
-		RetryPeriod:   ttl / 4,
-		Callbacks:     callbacks,
-	})
-
-	if err != nil {
-		glog.Fatalf("unexpected error starting leader election: %v", err)
-	}
-
-	st.elector = le
 	return st
 }
 
@@ -333,6 +352,13 @@ func (s *statusSync) updateStatus(newIngressPoint []apiv1.LoadBalancerIngress) {
 	sort.SliceStable(newIngressPoint, lessLoadBalancerIngress(newIngressPoint))
 
 	for _, ing := range ings {
+		curIPs := ing.Status.LoadBalancer.Ingress
+		sort.SliceStable(curIPs, lessLoadBalancerIngress(curIPs))
+		if ingressSliceEqual(curIPs, newIngressPoint) {
+			glog.V(3).Infof("skipping update of Ingress %v/%v (no change)", ing.Namespace, ing.Name)
+			continue
+		}
+
 		batch.Queue(runUpdate(ing, newIngressPoint, s.Client))
 	}
 
@@ -347,14 +373,6 @@ func runUpdate(ing *extensions.Ingress, status []apiv1.LoadBalancerIngress,
 			return nil, nil
 		}
 
-		curIPs := ing.Status.LoadBalancer.Ingress
-		sort.SliceStable(curIPs, lessLoadBalancerIngress(curIPs))
-
-		if ingressSliceEqual(status, curIPs) {
-			glog.V(3).Infof("skipping update of Ingress %v/%v (no change)", ing.Namespace, ing.Name)
-			return true, nil
-		}
-
 		ingClient := client.ExtensionsV1beta1().Ingresses(ing.Namespace)
 
 		currIng, err := ingClient.Get(ing.Name, metav1.GetOptions{})
@@ -398,5 +416,6 @@ func ingressSliceEqual(lhs, rhs []apiv1.LoadBalancerIngress) bool {
 			return false
 		}
 	}
+
 	return true
 }

internal/ingress/status/status_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package status
 
 import (
-	"context"
 	"os"
 	"testing"
 	"time"
@@ -298,7 +297,7 @@ func TestStatusActions(t *testing.T) {
 	fk := fkSync.(statusSync)
 
 	// start it and wait for the election and syn actions
-	go fk.Run(context.Background())
+	go fk.Run()
 	//  wait for the election
 	time.Sleep(100 * time.Millisecond)
 	// execute sync

test/e2e/annotations/influxdb.go

@@ -183,8 +183,7 @@ func execInfluxDBCommand(pod *corev1.Pod, command string) (string, error) {
 		execErr bytes.Buffer
 	)
 
-	args := fmt.Sprintf("kubectl exec --namespace %v %v -- %v", pod.Namespace, pod.Name, command)
-	cmd := exec.Command("/bin/bash", "-c", args)
+	cmd := exec.Command("/bin/bash", "-c", fmt.Sprintf("%v exec --namespace %s %s -- %s", framework.KubectlPath, pod.Namespace, pod.Name, command))
 	cmd.Stdout = &execOut
 	cmd.Stderr = &execErr
 
@@ -195,7 +194,7 @@ func execInfluxDBCommand(pod *corev1.Pod, command string) (string, error) {
 	}
 
 	if err != nil {
-		return "", fmt.Errorf("could not execute: %v", err)
+		return "", fmt.Errorf("could not execute '%s %s': %v", cmd.Path, cmd.Args, err)
 	}
 
 	return execOut.String(), nil

test/e2e/e2e.go

@@ -17,13 +17,14 @@ limitations under the License.
 package e2e
 
 import (
+	"os"
 	"testing"
 
-	"github.com/golang/glog"
 	"github.com/onsi/ginkgo"
 	"github.com/onsi/ginkgo/config"
 	"github.com/onsi/gomega"
 	"k8s.io/apiserver/pkg/util/logs"
+
 	// required
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 
@@ -36,6 +37,7 @@ import (
 	_ "k8s.io/ingress-nginx/test/e2e/servicebackend"
 	_ "k8s.io/ingress-nginx/test/e2e/settings"
 	_ "k8s.io/ingress-nginx/test/e2e/ssl"
+	_ "k8s.io/ingress-nginx/test/e2e/status"
 )
 
 // RunE2ETests checks configuration parameters (specified through flags) and then runs
@@ -50,7 +52,12 @@ func RunE2ETests(t *testing.T) {
 		config.GinkgoConfig.SkipString = `\[Flaky\]|\[Feature:.+\]`
 	}
 
-	glog.Infof("Starting e2e run %q on Ginkgo node %d", framework.RunID, config.GinkgoConfig.ParallelNode)
+	if os.Getenv("KUBECTL_PATH") != "" {
+		framework.KubectlPath = os.Getenv("KUBECTL_PATH")
+		framework.Logf("Using kubectl path '%s'", framework.KubectlPath)
+	}
+
+	framework.Logf("Starting e2e run %q on Ginkgo node %d", framework.RunID, config.GinkgoConfig.ParallelNode)
 	ginkgo.RunSpecs(t, "nginx-ingress-controller e2e suite")
 }