Add helper to verify if the configuration file changed

controllers/nginx-third-party/nginx/command.go

@@ -30,7 +30,7 @@ const (
 // Start starts a nginx (master process) and waits. If the process ends
 // we need to kill the controller process and return the reason.
 func (ngx *NginxManager) Start() {
-	glog.Info("Starting nginx...")
+	glog.Info("Starting NGINX process...")
 	cmd := exec.Command("nginx")
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
@@ -43,7 +43,9 @@ func (ngx *NginxManager) Start() {
 	}
 }
 
-// Reload the master process receives the signal to reload configuration, it checks
+// CheckAndReload verify if the nginx configuration changed and sends a reload
+//
+// the master process receives the signal to reload configuration, it checks
 // the syntax validity of the new configuration file and tries to apply the
 // configuration provided in it. If this is a success, the master process starts
 // new worker processes and sends messages to old worker processes, requesting them
@@ -58,13 +60,13 @@ func (ngx *NginxManager) CheckAndReload(cfg *nginxConfiguration, upstreams []Ups
 
 	newCfg, err := ngx.writeCfg(cfg, upstreams, servers, servicesL4)
 	if err != nil {
-		glog.Errorf("Failed to write new nginx configuration. Avoiding reload: %v", err)
+		glog.Errorf("failed to write new nginx configuration. Avoiding reload: %v", err)
 		return
 	}
 
 	if newCfg {
 		if err := ngx.shellOut("nginx -s reload"); err == nil {
-			glog.Info("Change in configuration detected. Reloading...")
+			glog.Info("change in configuration detected. Reloading...")
 		}
 	}
 }
@@ -74,7 +76,7 @@ func (ngx *NginxManager) CheckAndReload(cfg *nginxConfiguration, upstreams []Ups
 func (ngx *NginxManager) shellOut(cmd string) error {
 	out, err := exec.Command("sh", "-c", cmd).CombinedOutput()
 	if err != nil {
-		glog.Errorf("Failed to execute %v: %v", cmd, string(out))
+		glog.Errorf("failed to execute %v: %v", cmd, string(out))
 		return err
 	}
 

controllers/nginx-third-party/nginx/main.go

@@ -23,6 +23,8 @@ import (
 	"sync"
 	"text/template"
 
+	"github.com/golang/glog"
+
 	"k8s.io/contrib/ingress/controllers/nginx-third-party/ssl"
 
 	"k8s.io/kubernetes/pkg/client/record"
@@ -237,7 +239,7 @@ type NginxManager struct {
 // defaultConfiguration returns the default configuration contained
 // in the file default-conf.json
 func newDefaultNginxCfg() *nginxConfiguration {
-	return &nginxConfiguration{
+	cfg := nginxConfiguration{
 		BodySize:                 bodySize,
 		ErrorLogLevel:            errorLevel,
 		UseHTS:                   true,
@@ -263,6 +265,12 @@ func newDefaultNginxCfg() *nginxConfiguration {
 		UseGzip:                  true,
 		WorkerProcesses:          strconv.Itoa(runtime.NumCPU()),
 	}
+
+	if glog.V(5) {
+		cfg.ErrorLogLevel = "debug"
+	}
+
+	return &cfg
 }
 
 // NewManager ...

controllers/nginx-third-party/nginx/nginx.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2015 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package nginx
 
 // NGINXController Updates NGINX configuration, starts and reloads NGINX
@@ -20,6 +36,7 @@ type Upstream struct {
 	Backends []UpstreamServer
 }
 
+// UpstreamByNameServers Upstream sorter by name
 type UpstreamByNameServers []Upstream
 
 func (c UpstreamByNameServers) Len() int      { return len(c) }
@@ -34,6 +51,7 @@ type UpstreamServer struct {
 	Port    string
 }
 
+// UpstreamServerByAddrPort UpstreamServer sorter by address and port
 type UpstreamServerByAddrPort []UpstreamServer
 
 func (c UpstreamServerByAddrPort) Len() int      { return len(c) }
@@ -59,11 +77,12 @@ type Server struct {
 	SSLCertificateKey string
 }
 
-type ServerByNamePort []Server
+// ServerByName Server sorter by name
+type ServerByName []Server
 
-func (c ServerByNamePort) Len() int      { return len(c) }
-func (c ServerByNamePort) Swap(i, j int) { c[i], c[j] = c[j], c[i] }
-func (c ServerByNamePort) Less(i, j int) bool {
+func (c ServerByName) Len() int      { return len(c) }
+func (c ServerByName) Swap(i, j int) { c[i], c[j] = c[j], c[i] }
+func (c ServerByName) Less(i, j int) bool {
 	return c[i].Name < c[j].Name
 }
 
@@ -73,20 +92,24 @@ type Location struct {
 	Upstream Upstream
 }
 
-type locByPathUpstream []Location
+// LocationByPath Location sorter by path
+type LocationByPath []Location
 
-func (c locByPathUpstream) Len() int      { return len(c) }
-func (c locByPathUpstream) Swap(i, j int) { c[i], c[j] = c[j], c[i] }
-func (c locByPathUpstream) Less(i, j int) bool {
+func (c LocationByPath) Len() int      { return len(c) }
+func (c LocationByPath) Swap(i, j int) { c[i], c[j] = c[j], c[i] }
+func (c LocationByPath) Less(i, j int) bool {
 	return c[i].Path < c[j].Path
 }
 
-// NewUpstreamWithDefaultServer creates an upstream with the default server.
-// proxy_pass to an upstream with the default server returns 502.
-// We use it for services that have no endpoints
-func NewUpstreamWithDefaultServer(name string) Upstream {
+// NewDefaultServer return an UpstreamServer to be use as default server returns 502.
+func NewDefaultServer() UpstreamServer {
+	return UpstreamServer{Address: "127.0.0.1", Port: "8181"}
+}
+
+// NewUpstream creates an upstream without servers.
+func NewUpstream(name string) Upstream {
 	return Upstream{
 		Name:     name,
-		Backends: []UpstreamServer{UpstreamServer{Address: "127.0.0.1", Port: "8181"}},
+		Backends: []UpstreamServer{},
 	}
 }

controllers/nginx-third-party/nginx/template.go

@@ -17,10 +17,10 @@ limitations under the License.
 package nginx
 
 import (
+	"bytes"
 	"encoding/json"
 	"errors"
 	"fmt"
-	"os"
 	"text/template"
 
 	"github.com/fatih/structs"
@@ -61,11 +61,6 @@ func (ngx *NginxManager) loadTemplate() {
 }
 
 func (ngx *NginxManager) writeCfg(cfg *nginxConfiguration, upstreams []Upstream, servers []Server, servicesL4 []Service) (bool, error) {
-	file, err := os.Create(ngx.ConfigFile)
-	if err != nil {
-		return false, err
-	}
-
 	fromMap := structs.Map(cfg)
 	toMap := structs.Map(ngx.defCfg)
 	curNginxCfg := merge(toMap, fromMap)
@@ -86,7 +81,18 @@ func (ngx *NginxManager) writeCfg(cfg *nginxConfiguration, upstreams []Upstream,
 		conf["defErrorSvc"] = false
 	}
 
-	if glog.V(2) {
+	buffer := new(bytes.Buffer)
+	err := ngx.template.Execute(buffer, conf)
+	if err != nil {
+		return false, err
+	}
+
+	changed, err := checkChanges(ngx.ConfigFile, buffer)
+	if err != nil {
+		return false, err
+	}
+
+	if glog.V(3) {
 		b, err := json.Marshal(conf)
 		if err != nil {
 			fmt.Println("error:", err)
@@ -94,10 +100,5 @@ func (ngx *NginxManager) writeCfg(cfg *nginxConfiguration, upstreams []Upstream,
 		glog.Infof("nginx configuration: %v", string(b))
 	}
 
-	err = ngx.template.Execute(file, conf)
-	if err != nil {
-		return false, err
-	}
-
-	return true, nil
+	return changed, nil
 }

controllers/nginx-third-party/nginx/utils.go

@@ -17,10 +17,13 @@ limitations under the License.
 package nginx
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"os"
+	"os/exec"
 	"reflect"
 	"strings"
 
@@ -36,7 +39,7 @@ func (ngx *NginxManager) IsHealthy() error {
 	defer res.Body.Close()
 
 	if res.StatusCode != 200 {
-		return fmt.Errorf("nginx status is unhealthy")
+		return fmt.Errorf("NGINX is unhealthy")
 	}
 
 	return nil
@@ -67,7 +70,7 @@ func getDnsServers() []string {
 		}
 	}
 
-	glog.V(2).Infof("Nameservers to use: %v", nameservers)
+	glog.V(3).Infof("nameservers to use: %v", nameservers)
 	return nameservers
 }
 
@@ -81,8 +84,8 @@ func (ngx *NginxManager) ReadConfig(data string) (*nginxConfiguration, error) {
 	cfg := nginxConfiguration{}
 	err := json.Unmarshal([]byte(data), &cfg)
 	if err != nil {
-		glog.Errorf("Invalid json: %v", err)
-		return newDefaultNginxCfg(), fmt.Errorf("Invalid custom nginx configuration: %v", err)
+		glog.Errorf("invalid json: %v", err)
+		return newDefaultNginxCfg(), fmt.Errorf("invalid custom nginx configuration: %v", err)
 	}
 
 	return &cfg, nil
@@ -116,3 +119,63 @@ func toMap(iface interface{}) (map[string]interface{}, bool) {
 
 	return map[string]interface{}{}, false
 }
+
+func checkChanges(filename string, data *bytes.Buffer) (bool, error) {
+	in, err := os.Open(filename)
+	if err != nil {
+		return false, err
+	}
+
+	src, err := ioutil.ReadAll(in)
+	in.Close()
+	if err != nil {
+		return false, err
+	}
+
+	res := data.Bytes()
+	if !bytes.Equal(src, res) {
+		err = ioutil.WriteFile(filename, res, 0644)
+		if err != nil {
+			return false, err
+		}
+
+		dData, err := diff(src, res)
+		if err != nil {
+			return false, fmt.Errorf("computing diff: %s", err)
+		}
+
+		if glog.V(2) {
+			glog.Infof("NGINX configuration diff a/%s b/%s\n", filename, filename)
+			glog.Infof("%v", string(dData))
+		}
+
+		return len(dData) > 0, nil
+	}
+
+	return false, nil
+}
+
+func diff(b1, b2 []byte) (data []byte, err error) {
+	f1, err := ioutil.TempFile("", "")
+	if err != nil {
+		return
+	}
+	defer os.Remove(f1.Name())
+	defer f1.Close()
+
+	f2, err := ioutil.TempFile("", "")
+	if err != nil {
+		return
+	}
+	defer os.Remove(f2.Name())
+	defer f2.Close()
+
+	f1.Write(b1)
+	f2.Write(b2)
+
+	data, err = exec.Command("diff", "-u", f1.Name(), f2.Name()).CombinedOutput()
+	if len(data) > 0 {
+		err = nil
+	}
+	return
+}