Merge pull request #48 from nginxinc/propagete-changes-to-plus

Propagate commits for NGINX controller to NGINX Plus controller

Author: pleshakov

nginx-controller/nginx/nginx.go

@@ -178,10 +178,10 @@ func (nginx *NginxController) templateIt(config IngressNginxConfig, filename str
 func (nginx *NginxController) Reload() error {
 	if !nginx.local {
 		if err := shellOut("nginx -t"); err != nil {
-			return fmt.Errorf("Invalid nginx configuration detected, not reloading", err)
+			return fmt.Errorf("Invalid nginx configuration detected, not reloading: %s", err)
 		}
 		if err := shellOut("nginx -s reload"); err != nil {
-			return fmt.Errorf("nginx -s failed: %s", err)
+			return fmt.Errorf("Reloading NGINX failed: %s", err)
 		}
 	} else {
 		glog.V(3).Info("Reloading nginx")

nginx-plus-controller/controller/controller.go

@@ -267,7 +267,7 @@ func (lbc *LoadBalancerController) syncEndp(key string) {
 
 		for _, ing := range ings {
 			ingEx := lbc.createIngress(&ing)
-			glog.V(3).Infof("Updating Endponits for %v/%v", ing.Name, ing.Namespace)
+			glog.V(3).Infof("Updating Endpoints for %v/%v", ing.Name, ing.Namespace)
 			name := ing.Namespace + "-" + ing.Name
 			lbc.cnf.UpdateEndpoints(name, &ingEx)
 		}
@@ -378,7 +378,7 @@ func (lbc *LoadBalancerController) createIngress(ing *extensions.Ingress) nginx.
 		secretName := tls.SecretName
 		secret, err := lbc.client.Secrets(ing.Namespace).Get(secretName)
 		if err != nil {
-			glog.Warningf("Error retriveing secret %v for ing %v: %v", secretName, ing.Name, err)
+			glog.Warningf("Error retrieving secret %v for Ingress %v: %v", secretName, ing.Name, err)
 			continue
 		}
 		ingEx.Secrets[secretName] = secret
@@ -388,7 +388,7 @@ func (lbc *LoadBalancerController) createIngress(ing *extensions.Ingress) nginx.
 	if ing.Spec.Backend != nil {
 		endps, err := lbc.getEndpointsForIngressBackend(ing.Spec.Backend, ing.Namespace)
 		if err != nil {
-			glog.V(3).Infof("Error retriviend endpoints for the services %v: %v", ing.Spec.Backend.ServiceName, err)
+			glog.V(3).Infof("Error retrieving endpoints for the services %v: %v", ing.Spec.Backend.ServiceName, err)
 		} else {
 			ingEx.Endpoints[ing.Spec.Backend.ServiceName] = endps
 		}
@@ -402,7 +402,7 @@ func (lbc *LoadBalancerController) createIngress(ing *extensions.Ingress) nginx.
 		for _, path := range rule.HTTP.Paths {
 			endps, err := lbc.getEndpointsForIngressBackend(&path.Backend, ing.Namespace)
 			if err != nil {
-				glog.V(3).Infof("Error retriviend endpoints for the services %v: %v", path.Backend.ServiceName, err)
+				glog.V(3).Infof("Error retrieving endpoints for the services %v: %v", path.Backend.ServiceName, err)
 			} else {
 				ingEx.Endpoints[path.Backend.ServiceName] = endps
 			}
@@ -430,7 +430,7 @@ func (lbc *LoadBalancerController) getEndpointsForIngressBackend(backend *extens
 		return &endps, nil
 	}
 
-	return nil, fmt.Errorf("Svc %s doesn't exists", svcKey)
+	return nil, fmt.Errorf("service %s doesn't exists", svcKey)
 }
 
 func parseNginxConfigMaps(nginxConfigMaps string) (string, string, error) {

nginx-plus-controller/nginx/configurator.go

@@ -40,9 +40,12 @@ func (cnf *Configurator) AddOrUpdateIngress(name string, ingEx *IngressEx) {
 	pems := cnf.updateCertificates(ingEx)
 	nginxCfg := cnf.generateNginxCfg(ingEx, pems)
 	cnf.nginx.AddOrUpdateIngress(name, nginxCfg)
-	cnf.nginx.Reload()
-	time.Sleep(500 * time.Millisecond)
-	cnf.updateEndpoints(name, ingEx)
+	if err := cnf.nginx.Reload(); err != nil {
+		glog.Errorf("Error when adding or updating ingress %q: %q", name, err)
+	} else {
+		time.Sleep(500 * time.Millisecond)
+		cnf.updateEndpoints(name, ingEx)
+	}
 }
 
 func (cnf *Configurator) updateCertificates(ingEx *IngressEx) map[string]string {
@@ -257,7 +260,9 @@ func (cnf *Configurator) DeleteIngress(name string) {
 	defer cnf.lock.Unlock()
 
 	cnf.nginx.DeleteIngress(name)
-	cnf.nginx.Reload()
+	if err := cnf.nginx.Reload(); err != nil {
+		glog.Errorf("Error when removing ingress %q: %q", name, err)
+	}
 }
 
 // UpdateEndpoints updates endpoints in NGINX configuration for an Ingress resource

nginx-plus-controller/nginx/nginx.go

@@ -2,6 +2,7 @@ package nginx
 
 import (
 	"bytes"
+	"fmt"
 	"html/template"
 	"os"
 	"os/exec"
@@ -200,18 +201,26 @@ func (nginx *NginxController) templateIt(config IngressNginxConfig, filename str
 }
 
 // Reload reloads NGINX
-func (nginx *NginxController) Reload() {
+func (nginx *NginxController) Reload() error {
 	if !nginx.local {
-		shellOut("nginx -s reload")
+		if err := shellOut("nginx -t"); err != nil {
+			return fmt.Errorf("Invalid nginx configuration detected, not reloading: %s", err)
+		}
+		if err := shellOut("nginx -s reload"); err != nil {
+			return fmt.Errorf("Reloading NGINX failed: %s", err)
+		}
 	} else {
 		glog.V(3).Info("Reloading nginx")
 	}
+	return nil
 }
 
 // Start starts NGINX
 func (nginx *NginxController) Start() {
 	if !nginx.local {
-		shellOut("nginx")
+		if err := shellOut("nginx"); err != nil {
+			glog.Fatalf("Failed to start nginx")
+		}
 	} else {
 		glog.V(3).Info("Starting nginx")
 	}
@@ -223,7 +232,7 @@ func (nginx *NginxController) createCertsDir() {
 	}
 }
 
-func shellOut(cmd string) {
+func shellOut(cmd string) (err error) {
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer
 
@@ -233,17 +242,17 @@ func shellOut(cmd string) {
 	command.Stdout = &stdout
 	command.Stderr = &stderr
 
-	err := command.Start()
+	err = command.Start()
 	if err != nil {
-		glog.Fatalf("Failed to execute %v, err: %v", cmd, err)
+		return fmt.Errorf("Failed to execute %v, err: %v", cmd, err)
 	}
 
 	err = command.Wait()
 	if err != nil {
-		glog.Errorf("Command %v stdout: %q", cmd, stdout.String())
-		glog.Errorf("Command %v stderr: %q", cmd, stderr.String())
-		glog.Fatalf("Command %v finished with error: %v", cmd, err)
+		return fmt.Errorf("Command %v stdout: %q\nstderr: %q\nfinished with error: %v", cmd,
+			stdout.String(), stderr.String(), err)
 	}
+	return nil
 }
 
 // UpdateMainConfigFile update the main NGINX configuration file