Replace Status port using a socket

cmd/nginx/flags.go

@@ -31,6 +31,7 @@ import (
 	"k8s.io/ingress-nginx/internal/ingress/controller"
 	ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config"
 	ing_net "k8s.io/ingress-nginx/internal/net"
+	"k8s.io/ingress-nginx/internal/nginx"
 )
 
 func parseFlags() (bool, *controller.Configuration, error) {
@@ -151,7 +152,7 @@ Feature backed by OpenResty Lua libraries. Requires that OCSP stapling is not en
 
 		httpPort      = flags.Int("http-port", 80, `Port to use for servicing HTTP traffic.`)
 		httpsPort     = flags.Int("https-port", 443, `Port to use for servicing HTTPS traffic.`)
-		statusPort    = flags.Int("status-port", 18080, `Port to use for exposing NGINX status pages.`)
+		_             = flags.Int("status-port", 18080, `Port to use for exposing NGINX status pages.`)
 		sslProxyPort  = flags.Int("ssl-passthrough-proxy-port", 442, `Port to use internally for SSL Passthrough.`)
 		defServerPort = flags.Int("default-server-port", 8181, `Port to use for exposing the default server (catch-all).`)
 		healthzPort   = flags.Int("healthz-port", 10254, "Port to use for the healthz endpoint.")
@@ -160,7 +161,7 @@ Feature backed by OpenResty Lua libraries. Requires that OCSP stapling is not en
 			`Disable support for catch-all Ingresses`)
 	)
 
-	flags.MarkDeprecated("sort-backends", "Feature removed because of the lua load balancer that removed the need of reloads for change in endpoints")
+	flags.MarkDeprecated("status-port", `The status port is a unix socket now.`)
 
 	flag.Set("logtostderr", "true")
 
@@ -200,10 +201,6 @@ Feature backed by OpenResty Lua libraries. Requires that OCSP stapling is not en
 		return false, nil, fmt.Errorf("Port %v is already in use. Please check the flag --https-port", *httpsPort)
 	}
 
-	if !ing_net.IsPortAvailable(*statusPort) {
-		return false, nil, fmt.Errorf("Port %v is already in use. Please check the flag --status-port", *statusPort)
-	}
-
 	if !ing_net.IsPortAvailable(*defServerPort) {
 		return false, nil, fmt.Errorf("Port %v is already in use. Please check the flag --default-server-port", *defServerPort)
 	}
@@ -224,6 +221,8 @@ Feature backed by OpenResty Lua libraries. Requires that OCSP stapling is not en
 		return false, nil, fmt.Errorf("Flags --publish-service and --publish-status-address are mutually exclusive")
 	}
 
+	nginx.HealthPath = *defHealthzURL
+
 	config := &controller.Configuration{
 		APIServerHost:              *apiserverHost,
 		KubeConfigFile:             *kubeConfigFile,
@@ -241,7 +240,6 @@ Feature backed by OpenResty Lua libraries. Requires that OCSP stapling is not en
 		TCPConfigMapName:           *tcpConfigMapName,
 		UDPConfigMapName:           *udpConfigMapName,
 		DefaultSSLCertificate:      *defSSLCertificate,
-		DefaultHealthzURL:          *defHealthzURL,
 		HealthCheckTimeout:         *healthCheckTimeout,
 		PublishService:             *publishSvc,
 		PublishStatusAddress:       *publishStatusAddress,
@@ -256,7 +254,6 @@ Feature backed by OpenResty Lua libraries. Requires that OCSP stapling is not en
 			HTTP:     *httpPort,
 			HTTPS:    *httpsPort,
 			SSLProxy: *sslProxyPort,
-			Status:   *statusPort,
 		},
 		DisableCatchAll: *disableCatchAll,
 	}

cmd/nginx/main.go

@@ -131,7 +131,7 @@ func main() {
 
 	mc := metric.NewDummyCollector()
 	if conf.EnableMetrics {
-		mc, err = metric.NewCollector(conf.ListenPorts.Status, conf.MetricsPerHost, reg)
+		mc, err = metric.NewCollector(conf.MetricsPerHost, reg)
 		if err != nil {
 			klog.Fatalf("Error creating prometheus collector:  %v", err)
 		}

docs/user-guide/cli-arguments.md

@@ -34,7 +34,6 @@ They are set in the container spec of the `nginx-ingress-controller` Deployment
 | `--report-node-internal-ip-address` | Set the load-balancer status of Ingress objects to internal Node addresses instead of external. Requires the update-status parameter. |
 | `--sort-backends`                 | Sort servers inside NGINX upstreams. |
 | `--ssl-passthrough-proxy-port int` | Port to use internally for SSL Passthrough. (default 442) |
-| `--status-port int`               | Port to use for exposing NGINX status pages. (default 18080) |
 | `--stderrthreshold severity`      | logs at or above this threshold go to stderr (default 2) |
 | `--sync-period duration`          | Period at which the controller forces the repopulation of its local object stores. Disabled by default. |
 | `--sync-rate-limit float32`       | Define the sync frequency upper limit (default 0.3) |
@@ -46,4 +45,4 @@ They are set in the container spec of the `nginx-ingress-controller` Deployment
 | `--version`                       | Show release information about the NGINX Ingress controller and exit. |
 | `--vmodule moduleSpec`            | comma-separated list of pattern=N settings for file-filtered logging |
 | `--watch-namespace string`        | Namespace the controller watches for updates to Kubernetes objects. This includes Ingresses, Services and all configuration resources. All namespaces are watched if this parameter is left empty. |
-| `--disable-catch-all`             | Disable support for catch-all Ingresses. |
+| `--disable-catch-all`             | Disable support for catch-all Ingresses. |

internal/ingress/controller/checker.go

@@ -21,13 +21,13 @@ import (
 	"net/http"
 	"strconv"
 	"strings"
-	"time"
 
 	"github.com/ncabatoff/process-exporter/proc"
 	"github.com/pkg/errors"
-)
+	"k8s.io/klog"
 
-const nginxPID = "/tmp/nginx.pid"
+	"k8s.io/ingress-nginx/internal/nginx"
+)
 
 // Name returns the healthcheck name
 func (n NGINXController) Name() string {
@@ -36,25 +36,25 @@ func (n NGINXController) Name() string {
 
 // Check returns if the nginx healthz endpoint is returning ok (status code 200)
 func (n *NGINXController) Check(_ *http.Request) error {
-
-	url := fmt.Sprintf("http://127.0.0.1:%v%v", n.cfg.ListenPorts.Status, ngxHealthPath)
-	timeout := n.cfg.HealthCheckTimeout
-	statusCode, err := simpleGet(url, timeout)
+	statusCode, _, err := nginx.NewGetStatusRequest(nginx.HealthPath)
 	if err != nil {
+		klog.Errorf("healthcheck error: %v", err)
 		return err
 	}
 
 	if statusCode != 200 {
+		klog.Errorf("healthcheck error: %v", statusCode)
 		return fmt.Errorf("ingress controller is not healthy")
 	}
 
-	url = fmt.Sprintf("http://127.0.0.1:%v/is-dynamic-lb-initialized", n.cfg.ListenPorts.Status)
-	statusCode, err = simpleGet(url, timeout)
+	statusCode, _, err = nginx.NewGetStatusRequest("/is-dynamic-lb-initialized")
 	if err != nil {
+		klog.Errorf("healthcheck error: %v", err)
 		return err
 	}
 
 	if statusCode != 200 {
+		klog.Errorf("healthcheck error: %v", statusCode)
 		return fmt.Errorf("dynamic load balancer not started")
 	}
 
@@ -63,35 +63,14 @@ func (n *NGINXController) Check(_ *http.Request) error {
 	if err != nil {
 		return errors.Wrap(err, "unexpected error reading /proc directory")
 	}
-	f, err := n.fileSystem.ReadFile(nginxPID)
+	f, err := n.fileSystem.ReadFile(nginx.PID)
 	if err != nil {
-		return errors.Wrapf(err, "unexpected error reading %v", nginxPID)
+		return errors.Wrapf(err, "unexpected error reading %v", nginx.PID)
 	}
 	pid, err := strconv.Atoi(strings.TrimRight(string(f), "\r\n"))
 	if err != nil {
-		return errors.Wrapf(err, "unexpected error reading the nginx PID from %v", nginxPID)
+		return errors.Wrapf(err, "unexpected error reading the nginx PID from %v", nginx.PID)
 	}
 	_, err = fs.NewProc(pid)
-
 	return err
 }
-
-func simpleGet(url string, timeout time.Duration) (int, error) {
-	client := &http.Client{
-		Timeout:   timeout * time.Second,
-		Transport: &http.Transport{DisableKeepAlives: true},
-	}
-
-	req, err := http.NewRequest("GET", url, nil)
-	if err != nil {
-		return -1, err
-	}
-
-	res, err := client.Do(req)
-	if err != nil {
-		return -1, err
-	}
-	defer res.Body.Close()
-
-	return res.StatusCode, nil
-}

internal/ingress/controller/checker_test.go

@@ -21,6 +21,7 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"os"
 	"os/exec"
 	"testing"
 
@@ -29,27 +30,37 @@ import (
 
 	"k8s.io/ingress-nginx/internal/file"
 	ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config"
+	"k8s.io/ingress-nginx/internal/nginx"
 )
 
 func TestNginxCheck(t *testing.T) {
 	mux := http.NewServeMux()
 
-	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.WriteHeader(http.StatusOK)
-		fmt.Fprintf(w, "ok")
-	}))
+	listener, err := net.Listen("unix", nginx.StatusSocket)
+	if err != nil {
+		t.Errorf("crating unix listener: %s", err)
+	}
+	defer listener.Close()
+	defer os.Remove(nginx.StatusSocket)
+
+	server := &httptest.Server{
+		Listener: listener,
+		Config: &http.Server{
+			Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(http.StatusOK)
+				fmt.Fprintf(w, "ok")
+			}),
+		},
+	}
 	defer server.Close()
-	// port to be used in the check
-	p := server.Listener.Addr().(*net.TCPAddr).Port
+	server.Start()
 
 	// mock filesystem
-	fs := filesystem.NewFakeFs()
+	fs := filesystem.DefaultFs{}
 
 	n := &NGINXController{
 		cfg: &Configuration{
-			ListenPorts: &ngx_config.ListenPorts{
-				Status: p,
-			},
+			ListenPorts: &ngx_config.ListenPorts{},
 		},
 		fileSystem: fs,
 	}
@@ -62,7 +73,7 @@ func TestNginxCheck(t *testing.T) {
 
 	// create pid file
 	fs.MkdirAll("/tmp", file.ReadWriteByUser)
-	pidFile, err := fs.Create(nginxPID)
+	pidFile, err := fs.Create(nginx.PID)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -102,20 +113,14 @@ func TestNginxCheck(t *testing.T) {
 			t.Error("expected an error but none returned")
 		}
 	})
-
-	t.Run("invalid port", func(t *testing.T) {
-		n.cfg.ListenPorts.Status = 9000
-		if err := callHealthz(true, mux); err == nil {
-			t.Error("expected an error but none returned")
-		}
-	})
 }
 
 func callHealthz(expErr bool, mux *http.ServeMux) error {
-	req, err := http.NewRequest("GET", "http://localhost:8080/healthz", nil)
+	req, err := http.NewRequest("GET", "/healthz", nil)
 	if err != nil {
-		return err
+		return fmt.Errorf("healthz error: %v", err)
 	}
+
 	w := httptest.NewRecorder()
 	mux.ServeHTTP(w, req)
 

internal/ingress/controller/config/config.go

@@ -726,14 +726,18 @@ type TemplateConfig struct {
 	PublishService             *apiv1.Service
 	DynamicCertificatesEnabled bool
 	EnableMetrics              bool
+
+	PID          string
+	StatusSocket string
+	StatusPath   string
+	StreamSocket string
 }
 
 // ListenPorts describe the ports required to run the
 // NGINX Ingress controller
 type ListenPorts struct {
 	HTTP     int
 	HTTPS    int
-	Status   int
 	Health   int
 	Default  int
 	SSLProxy int

internal/ingress/controller/controller.go

@@ -65,7 +65,6 @@ type Configuration struct {
 	// +optional
 	UDPConfigMapName string
 
-	DefaultHealthzURL     string
 	HealthCheckTimeout    time.Duration
 	DefaultSSLCertificate string
 
@@ -195,10 +194,8 @@ func (n *NGINXController) syncIngress(interface{}) error {
 
 	isFirstSync := n.runningConfig.Equal(&ingress.Configuration{})
 	if isFirstSync {
-		// For the initial sync it always takes some time for NGINX to
-		//  start listening on the configured port (default 18080)
-		//  For large configurations it might take a while so we loop
-		//  and back off
+		// For the initial sync it always takes some time for NGINX to start listening
+		// For large configurations it might take a while so we loop and back off
 		klog.Info("Initial sync, sleeping for 1 second.")
 		time.Sleep(1 * time.Second)
 	}
@@ -211,7 +208,7 @@ func (n *NGINXController) syncIngress(interface{}) error {
 	}
 
 	err := wait.ExponentialBackoff(retry, func() (bool, error) {
-		err := configureDynamically(pcfg, n.cfg.ListenPorts.Status, n.cfg.DynamicCertificatesEnabled)
+		err := configureDynamically(pcfg, n.cfg.DynamicCertificatesEnabled)
 		if err == nil {
 			klog.V(2).Infof("Dynamic reconfiguration succeeded.")
 			return true, nil
@@ -255,7 +252,6 @@ func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Pr
 		n.cfg.ListenPorts.HTTP,
 		n.cfg.ListenPorts.HTTPS,
 		n.cfg.ListenPorts.SSLProxy,
-		n.cfg.ListenPorts.Status,
 		n.cfg.ListenPorts.Health,
 		n.cfg.ListenPorts.Default,
 	}