Refactor controller metrics interface

Makefile

@@ -28,6 +28,8 @@ FOCUS ?= .*
 # number of parallel test
 E2E_NODES ?= 3
 
+NODE_IP ?= $(shell minikube ip)
+
 ifeq ($(GOHOSTOS),darwin)
   SED_I=sed -i ''
 endif
@@ -165,6 +167,7 @@ static-check:
 .PHONY: test
 test:
 	@$(DEF_VARS)                 \
+	NODE_IP=$(NODE_IP)           \
 	DOCKER_OPTS="--net=host"     \
 	build/go-in-docker.sh build/test.sh
 
@@ -180,6 +183,7 @@ e2e-test:
 	FOCUS=$(FOCUS)               \
 	E2E_NODES=$(E2E_NODES)       \
 	DOCKER_OPTS="--net=host"     \
+	NODE_IP=$(NODE_IP)           \
 	build/go-in-docker.sh build/e2e-tests.sh
 
 .PHONY: cover

build/e2e-tests.sh

@@ -30,6 +30,10 @@ if [ -z "${E2E_NODES}" ]; then
     echo "E2E_NODES must be set"
     exit 1
 fi
+if [ -z "${NODE_IP}" ]; then
+    echo "NODE_IP must be set"
+    exit 1
+fi
 
 SCRIPT_ROOT=$(dirname ${BASH_SOURCE})/..
 
@@ -46,13 +50,6 @@ if ! [ -x "$(command -v kubectl)" ]; then
     chmod +x ${TEST_BINARIES}/kubectl
 fi
 
-if ! [ -x "$(command -v minikube)" ]; then
-    echo "downloading minikube..."
-    curl -sSLo ${TEST_BINARIES}/minikube \
-        https://storage.googleapis.com/minikube/releases/latest/minikube-linux-amd64
-    chmod +x ${TEST_BINARIES}/minikube
-fi
-
 ginkgo build ./test/e2e
 
 ginkgo                  \

build/go-in-docker.sh

@@ -62,6 +62,7 @@ GOARCH=${GOARCH}
 PWD=${PWD}
 BUSTED_ARGS=${BUSTED_ARGS:-""}
 REPO_INFO=${REPO_INFO:-local}
+NODE_IP=${NODE_IP:-127.0.0.1}
 EOF
 
 docker run                                       \

cmd/nginx/main.go

@@ -29,6 +29,7 @@ import (
 	"time"
 
 	"github.com/golang/glog"
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -39,9 +40,8 @@ import (
 	"k8s.io/client-go/tools/clientcmd"
 
 	"k8s.io/ingress-nginx/internal/file"
-	"k8s.io/ingress-nginx/internal/ingress/annotations/class"
 	"k8s.io/ingress-nginx/internal/ingress/controller"
-	"k8s.io/ingress-nginx/internal/ingress/metric/collector"
+	"k8s.io/ingress-nginx/internal/ingress/metric"
 	"k8s.io/ingress-nginx/internal/k8s"
 	"k8s.io/ingress-nginx/internal/net/ssl"
 	"k8s.io/ingress-nginx/version"
@@ -118,25 +118,20 @@ func main() {
 
 	conf.Client = kubeClient
 
-	ngx := controller.NewNGINXController(conf, fs)
+	reg := prometheus.NewRegistry()
+	mc, err := metric.NewCollector(conf.ListenPorts.Status, reg)
+	if err != nil {
+		glog.Fatalf("Error creating prometheus collectos:  %v", err)
+	}
+	mc.Start()
 
+	ngx := controller.NewNGINXController(conf, mc, fs)
 	go handleSigterm(ngx, func(code int) {
 		os.Exit(code)
 	})
 
 	mux := http.NewServeMux()
-	go registerHandlers(conf.EnableProfiling, conf.ListenPorts.Health, ngx, mux)
-
-	err = collector.InitNGINXStatusCollector(conf.Namespace, class.IngressClass, conf.ListenPorts.Status)
-
-	if err != nil {
-		glog.Fatalf("Error creating metric collector:  %v", err)
-	}
-
-	err = collector.NewInstance(conf.Namespace, class.IngressClass)
-	if err != nil {
-		glog.Fatalf("Error creating unix socket server:  %v", err)
-	}
+	go registerHandlers(conf.EnableProfiling, conf.ListenPorts.Health, ngx, mux, reg)
 
 	ngx.Start()
 }
@@ -240,14 +235,20 @@ func handleFatalInitError(err error) {
 		err)
 }
 
-func registerHandlers(enableProfiling bool, port int, ic *controller.NGINXController, mux *http.ServeMux) {
+func registerHandlers(
+	enableProfiling bool,
+	port int,
+	ic *controller.NGINXController,
+	mux *http.ServeMux,
+	reg *prometheus.Registry) {
+
 	// expose health check endpoint (/healthz)
 	healthz.InstallHandler(mux,
 		healthz.PingHealthz,
 		ic,
 	)
 
-	mux.Handle("/metrics", promhttp.Handler())
+	mux.Handle("/metrics", promhttp.HandlerFor(reg, promhttp.HandlerOpts{}))
 
 	mux.HandleFunc("/build", func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)

cmd/nginx/main_test.go

@@ -76,7 +76,7 @@ func TestHandleSigterm(t *testing.T) {
 		t.Fatalf("Unexpected error: %v", err)
 	}
 
-	ngx := controller.NewNGINXController(conf, fs)
+	ngx := controller.NewNGINXController(conf, nil, fs)
 
 	go handleSigterm(ngx, func(code int) {
 		if code != 1 {

internal/ingress/controller/controller.go

@@ -25,6 +25,7 @@ import (
 	"time"
 
 	"github.com/golang/glog"
+	"github.com/mitchellh/hashstructure"
 
 	apiv1 "k8s.io/api/core/v1"
 	extensions "k8s.io/api/extensions/v1beta1"
@@ -148,38 +149,43 @@ func (n *NGINXController) syncIngress(interface{}) error {
 		}
 	}
 
-	pcfg := ingress.Configuration{
-		Backends:            upstreams,
-		Servers:             servers,
-		TCPEndpoints:        n.getStreamServices(n.cfg.TCPConfigMapName, apiv1.ProtocolTCP),
-		UDPEndpoints:        n.getStreamServices(n.cfg.UDPConfigMapName, apiv1.ProtocolUDP),
-		PassthroughBackends: passUpstreams,
-
-		ConfigurationChecksum: n.store.GetBackendConfiguration().Checksum,
+	pcfg := &ingress.Configuration{
+		Backends:              upstreams,
+		Servers:               servers,
+		TCPEndpoints:          n.getStreamServices(n.cfg.TCPConfigMapName, apiv1.ProtocolTCP),
+		UDPEndpoints:          n.getStreamServices(n.cfg.UDPConfigMapName, apiv1.ProtocolUDP),
+		PassthroughBackends:   passUpstreams,
+		BackendConfigChecksum: n.store.GetBackendConfiguration().Checksum,
 	}
 
-	if n.runningConfig.Equal(&pcfg) {
+	if n.runningConfig.Equal(pcfg) {
 		glog.V(3).Infof("No configuration change detected, skipping backend reload.")
 		return nil
 	}
 
-	if n.cfg.DynamicConfigurationEnabled && n.IsDynamicConfigurationEnough(&pcfg) {
+	if n.cfg.DynamicConfigurationEnabled && n.IsDynamicConfigurationEnough(pcfg) {
 		glog.Infof("Changes handled by the dynamic configuration, skipping backend reload.")
 	} else {
 		glog.Infof("Configuration changes detected, backend reload required.")
 
-		err := n.OnUpdate(pcfg)
+		hash, _ := hashstructure.Hash(pcfg, &hashstructure.HashOptions{
+			TagName: "json",
+		})
+
+		pcfg.ConfigurationChecksum = fmt.Sprintf("%v", hash)
+
+		err := n.OnUpdate(*pcfg)
 		if err != nil {
-			IncReloadErrorCount()
-			ConfigSuccess(false)
+			n.metricCollector.IncReloadErrorCount()
+			n.metricCollector.ConfigSuccess(hash, false)
 			glog.Errorf("Unexpected failure reloading the backend:\n%v", err)
 			return err
 		}
 
 		glog.Infof("Backend successfully reloaded.")
-		ConfigSuccess(true)
-		IncReloadCount()
-		setSSLExpireTime(servers)
+		n.metricCollector.ConfigSuccess(hash, true)
+		n.metricCollector.IncReloadCount()
+		n.metricCollector.SetSSLExpireTime(servers)
 	}
 
 	if n.cfg.DynamicConfigurationEnabled {
@@ -191,7 +197,7 @@ func (n *NGINXController) syncIngress(interface{}) error {
 				// it takes time for NGINX to start listening on the configured ports
 				time.Sleep(1 * time.Second)
 			}
-			err := configureDynamically(&pcfg, n.cfg.ListenPorts.Status)
+			err := configureDynamically(pcfg, n.cfg.ListenPorts.Status)
 			if err == nil {
 				glog.Infof("Dynamic reconfiguration succeeded.")
 			} else {
@@ -200,7 +206,11 @@ func (n *NGINXController) syncIngress(interface{}) error {
 		}(isFirstSync)
 	}
 
-	n.runningConfig = &pcfg
+	ri := getRemovedIngresses(n.runningConfig, pcfg)
+	re := getRemovedHosts(n.runningConfig, pcfg)
+	n.metricCollector.RemoveMetrics(ri, re)
+
+	n.runningConfig = pcfg
 
 	return nil
 }
@@ -1112,3 +1122,57 @@ func extractTLSSecretName(host string, ing *extensions.Ingress,
 
 	return ""
 }
+
+// getRemovedHosts returns a list of the hostsnames
+// that are not associated anymore to the NGINX configuration.
+func getRemovedHosts(rucfg, newcfg *ingress.Configuration) []string {
+	old := sets.NewString()
+	new := sets.NewString()
+
+	for _, s := range rucfg.Servers {
+		if !old.Has(s.Hostname) {
+			old.Insert(s.Hostname)
+		}
+	}
+
+	for _, s := range newcfg.Servers {
+		if !new.Has(s.Hostname) {
+			new.Insert(s.Hostname)
+		}
+	}
+
+	return old.Difference(new).List()
+}
+
+func getRemovedIngresses(rucfg, newcfg *ingress.Configuration) []string {
+	oldIngresses := sets.NewString()
+	newIngresses := sets.NewString()
+
+	for _, server := range rucfg.Servers {
+		for _, location := range server.Locations {
+			if location.Ingress == nil {
+				continue
+			}
+
+			ingKey := k8s.MetaNamespaceKey(location.Ingress)
+			if !oldIngresses.Has(ingKey) {
+				oldIngresses.Insert(ingKey)
+			}
+		}
+	}
+
+	for _, server := range newcfg.Servers {
+		for _, location := range server.Locations {
+			if location.Ingress == nil {
+				continue
+			}
+
+			ingKey := k8s.MetaNamespaceKey(location.Ingress)
+			if !newIngresses.Has(ingKey) {
+				newIngresses.Insert(ingKey)
+			}
+		}
+	}
+
+	return oldIngresses.Difference(newIngresses).List()
+}