Avoid enqueueing ingress for service change

- batch handling for mergable ingresses
- error collection and logging

internal/controller/controller.go

@@ -278,7 +278,7 @@ func (lbc *LoadBalancerController) syncEndpoint(task queue.Task) {
 		ings := lbc.getIngressForEndpoints(obj)
 
 		var ingExes []*nginx.IngressEx
-		var mergableIngresses []*nginx.MergeableIngresses
+		var mergableIngressesBatch []*nginx.MergeableIngresses
 
 		for i := range ings {
 			if !lbc.isNginxIngress(&ings[i]) {
@@ -299,7 +299,7 @@ func (lbc *LoadBalancerController) syncEndpoint(task queue.Task) {
 					continue
 				}
 
-				mergableIngExes = append(mergableIngExes, mergableIngresses)
+				mergableIngressesBatch = append(mergableIngressesBatch, mergeableIngresses)
 				continue
 			}
 			if !lbc.cnf.HasIngress(&ings[i]) {
@@ -322,10 +322,10 @@ func (lbc *LoadBalancerController) syncEndpoint(task queue.Task) {
 		if err != nil {
 			glog.Errorf("Error updating endpoints for %v: %v", ingExes, err)
 		}
-		glog.V(3).Infof("Updating Endpoints for %v", mergableIngresses)
-		// TODO FIX err = lbc.cnf.UpdateEndpointsMergeableIngress(mergeableIngresses)
+		glog.V(3).Infof("Updating Endpoints for %v", mergableIngressesBatch)
+		err = lbc.cnf.UpdateEndpointsMergeableIngress(mergableIngressesBatch)
 		if err != nil {
-			glog.Errorf("Error updating endpoints for %v/%v: %v", ing.Namespace, ing.Name, err)
+			glog.Errorf("Error updating endpoints for %v: %v", mergableIngressesBatch, err)
 		}
 	}
 }

internal/nginx/configurator.go

@@ -982,6 +982,7 @@ func (cnf *Configurator) UpdateEndpoints(ingExes []*IngressEx) error {
 	}
 
 	if cnf.isPlus() && !reloadPlus {
+		glog.V(3).Info("No need to reload nginx")
 		return nil
 	}
 
@@ -992,32 +993,49 @@ func (cnf *Configurator) UpdateEndpoints(ingExes []*IngressEx) error {
 	return nil
 }
 
-// TODO FIX
 // UpdateEndpointsMergeableIngress updates endpoints in NGINX configuration for a mergeable Ingress resource
-func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergeableIngs *MergeableIngresses) error {
-	err := cnf.addOrUpdateMergeableIngress(mergeableIngs)
-	if err != nil {
-		return fmt.Errorf("Error adding or updating ingress %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err)
-	}
+func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergableIngressesBatch []*MergeableIngresses) error {
+	reloadPlus := false
+	errors := []error{}
+	for i := range mergableIngressesBatch {
+		err := cnf.addOrUpdateMergeableIngress(mergableIngressesBatch[i])
+		if err != nil {
+			glog.V(3).Infof("Error adding or updating ingress %v/%v: %v", mergableIngressesBatch[i].Master.Ingress.Namespace, mergableIngressesBatch[i].Master.Ingress.Name, err)
+			errors = append(errors, err)
+			continue
+		}
 
-	if cnf.isPlus() {
-		for _, ing := range mergeableIngs.Minions {
-			err = cnf.updatePlusEndpoints(ing)
-			if err != nil {
-				glog.Warningf("Couldn't update the endpoints via the API: %v; reloading configuration instead", err)
-				break
+		if cnf.isPlus() {
+			for _, ing := range mergableIngressesBatch[i].Minions {
+				err = cnf.updatePlusEndpoints(ing)
+				if err != nil {
+					glog.Warningf("Couldn't update the endpoints via the API: %v; reloading configuration instead", err)
+					errors = append(errors, err)
+					reloadPlus = true
+					break
+				}
 			}
 		}
-		if err == nil {
-			return nil
-		}
+	}
+	if cnf.isPlus() && !reloadPlus {
+		return consolodiateErrors(errors)
 	}
 
 	if err := cnf.nginx.Reload(); err != nil {
-		return fmt.Errorf("Error reloading NGINX when updating endpoints for %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err)
+		return fmt.Errorf("Error reloading NGINX: %v. additional errors: %v", err, consolodiateErrors(errors))
 	}
+	return consolodiateErrors(errors)
+}
 
-	return nil
+func consolodiateErrors(errors []error) error {
+	errorString := ""
+	if len(errors) == 0 {
+		return nil
+	}
+	for _, e := range errors {
+		errorString = errorString + e.Error()
+	}
+	return fmt.Errorf(errorString)
 }
 
 func (cnf *Configurator) updatePlusEndpoints(ingEx *IngressEx) error {

internal/nginx/configurator_test.go

@@ -718,14 +718,15 @@ func TestUpdateEndpoints(t *testing.T) {
 	}
 
 	ingress := createCafeIngressEx()
-	err = cnf.UpdateEndpoints(&ingress)
+	ingresses := []*IngressEx{&ingress}
+	err = cnf.UpdateEndpoints(ingresses)
 	if err != nil {
 		t.Errorf("UpdateEndpoints returned\n%v, but expected \n%v", err, nil)
 	}
 
 	// test with OSS Configurator
 	cnf.nginxAPI = nil
-	err = cnf.UpdateEndpoints(&ingress)
+	err = cnf.UpdateEndpoints(ingresses)
 	if err != nil {
 		t.Errorf("UpdateEndpoints returned\n%v, but expected \n%v", err, nil)
 	}
@@ -738,14 +739,15 @@ func TestUpdateEndpointsMergeableIngress(t *testing.T) {
 	}
 
 	mergeableIngress := createMergeableCafeIngress()
-	err = cnf.UpdateEndpointsMergeableIngress(mergeableIngress)
+	mergeableIngresses := []*MergeableIngresses{mergeableIngress}
+	err = cnf.UpdateEndpointsMergeableIngress(mergeableIngresses)
 	if err != nil {
 		t.Errorf("UpdateEndpointsMergeableIngress returned \n%v, but expected \n%v", err, nil)
 	}
 
 	// test with OSS Configurator
 	cnf.nginxAPI = nil
-	err = cnf.UpdateEndpointsMergeableIngress(mergeableIngress)
+	err = cnf.UpdateEndpointsMergeableIngress(mergeableIngresses)
 	if err != nil {
 		t.Errorf("UpdateEndpointsMergeableIngress returned \n%v, but expected \n%v", err, nil)
 	}
@@ -758,7 +760,8 @@ func TestUpdateEndpointsFailsWithInvalidTemplate(t *testing.T) {
 	}
 
 	ingress := createCafeIngressEx()
-	err = cnf.UpdateEndpoints(&ingress)
+	ingresses := []*IngressEx{&ingress}
+	err = cnf.UpdateEndpoints(ingresses)
 	if err == nil {
 		t.Errorf("UpdateEndpoints returned\n%v, but expected \n%v", nil, "template execution error")
 	}
@@ -771,7 +774,8 @@ func TestUpdateEndpointsMergeableIngressFailsWithInvalidTemplate(t *testing.T) {
 	}
 
 	mergeableIngress := createMergeableCafeIngress()
-	err = cnf.UpdateEndpointsMergeableIngress(mergeableIngress)
+	mergeableIngresses := []*MergeableIngresses{mergeableIngress}
+	err = cnf.UpdateEndpointsMergeableIngress(mergeableIngresses)
 	if err == nil {
 		t.Errorf("UpdateEndpointsMergeableIngress returned \n%v, but expected \n%v", nil, "template execution error")
 	}