Update nginx interface

controllers/nginx/pkg/cmd/controller/nginx.go

@@ -235,23 +235,6 @@ func (n *NGINXController) start(cmd *exec.Cmd, done chan error) {
 	}()
 }
 
-// Reload checks if the running configuration file is different
-// to the specified and reload nginx if required
-func (n NGINXController) Reload(data []byte) ([]byte, bool, error) {
-	if !n.isReloadRequired(data) {
-		return []byte("Reload not required"), false, nil
-	}
-
-	err := ioutil.WriteFile(cfgPath, data, 0644)
-	if err != nil {
-		return nil, false, err
-	}
-
-	o, e := exec.Command(n.binary, "-s", "reload", "-c", cfgPath).CombinedOutput()
-
-	return o, true, e
-}
-
 // BackendDefaults returns the nginx defaults
 func (n NGINXController) BackendDefaults() defaults.Backend {
 	if n.configmap == nil {
@@ -262,36 +245,35 @@ func (n NGINXController) BackendDefaults() defaults.Backend {
 	return ngx_template.ReadConfig(n.configmap.Data).Backend
 }
 
-// isReloadRequired check if the new configuration file is different
+// printDiff returns the difference between the running configuration
-// from the current one.
+// and the new one
-func (n NGINXController) isReloadRequired(data []byte) bool {
+func (n NGINXController) printDiff(data []byte) {
 	in, err := os.Open(cfgPath)
 	if err != nil {
-		return false
+		return
 	}
 	src, err := ioutil.ReadAll(in)
 	in.Close()
 	if err != nil {
-		return false
+		return
 	}
 
 	if !bytes.Equal(src, data) {
-
 		tmpfile, err := ioutil.TempFile("", "nginx-cfg-diff")
 		if err != nil {
 			glog.Errorf("error creating temporal file: %s", err)
-			return false
+			return
 		}
 		defer tmpfile.Close()
 		err = ioutil.WriteFile(tmpfile.Name(), data, 0644)
 		if err != nil {
-			return false
+			return
 		}
 
 		diffOutput, err := diff(src, data)
 		if err != nil {
 			glog.Errorf("error computing diff: %s", err)
-			return true
+			return
 		}
 
 		if glog.V(2) {
@@ -299,9 +281,7 @@ func (n NGINXController) isReloadRequired(data []byte) bool {
 			glog.Infof("%v", string(diffOutput))
 		}
 		os.Remove(tmpfile.Name())
-		return len(diffOutput) > 0
 	}
-	return false
 }
 
 // Info return build information
@@ -404,7 +384,7 @@ func (n *NGINXController) SetListers(lister ingress.StoreLister) {
 // write the configuration file
 // returning nill implies the backend will be reloaded.
 // if an error is returned means requeue the update
-func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) ([]byte, error) {
+func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error {
 	var longestName int
 	var serverNameBytes int
 	for _, srv := range ingressCfg.Servers {
@@ -540,15 +520,29 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) ([]byte, er
 		Cfg:                 cfg,
 		IsIPV6Enabled:       n.isIPV6Enabled && !cfg.DisableIpv6,
 	})
+
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	if err := n.testTemplate(content); err != nil {
+	err = n.testTemplate(content)
-		return nil, err
+	if err != nil {
+		return err
 	}
 
-	return content, nil
+	n.printDiff(content)
+
+	err = ioutil.WriteFile(cfgPath, content, 0644)
+	if err != nil {
+		return err
+	}
+
+	o, e := exec.Command(n.binary, "-s", "reload", "-c", cfgPath).CombinedOutput()
+	if err != nil {
+		return fmt.Errorf("%v\n%v", e, string(o))
+	}
+
+	return nil
 }
 
 // nginxHashBucketSize computes the correct nginx hash_bucket_size for a hash with the given longest key