Refactor health checks and wait until NGINX process ends

internal/ingress/controller/checker.go

@@ -25,7 +25,6 @@ import (
 
 	"github.com/ncabatoff/process-exporter/proc"
 	"github.com/pkg/errors"
-	"k8s.io/klog"
 
 	"k8s.io/ingress-nginx/internal/nginx"
 )
@@ -37,41 +36,48 @@ 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 {
-	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")
-	}
-
-	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")
+	if n.isShuttingDown {
+		return fmt.Errorf("the ingress controller is shutting down")
 	}
 
 	// check the nginx master process is running
 	fs, err := proc.NewFS("/proc", false)
 	if err != nil {
-		return errors.Wrap(err, "unexpected error reading /proc directory")
+		return errors.Wrap(err, "reading /proc directory")
 	}
+
 	f, err := ioutil.ReadFile(nginx.PID)
 	if err != nil {
-		return errors.Wrapf(err, "unexpected error reading %v", nginx.PID)
+		return errors.Wrapf(err, "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", nginx.PID)
+		return errors.Wrapf(err, "reading NGINX PID from file %v", nginx.PID)
 	}
+
 	_, err = fs.NewProc(pid)
-	return err
+	if err != nil {
+		return errors.Wrapf(err, "checking for NGINX process with PID %v", pid)
+	}
+
+	statusCode, _, err := nginx.NewGetStatusRequest(nginx.HealthPath)
+	if err != nil {
+		return errors.Wrapf(err, "checking if NGINX is running")
+	}
+
+	if statusCode != 200 {
+		return fmt.Errorf("ingress controller is not healthy (%v)", statusCode)
+	}
+
+	statusCode, _, err = nginx.NewGetStatusRequest("/is-dynamic-lb-initialized")
+	if err != nil {
+		return errors.Wrapf(err, "checking if the dynamic load balancer started")
+	}
+
+	if statusCode != 200 {
+		return fmt.Errorf("dynamic load balancer not started")
+	}
+
+	return nil
 }

internal/ingress/controller/checker_test.go

@@ -35,12 +35,11 @@ import (
 func TestNginxCheck(t *testing.T) {
 	mux := http.NewServeMux()
 
-	listener, err := net.Listen("unix", nginx.StatusSocket)
+	listener, err := net.Listen("tcp", fmt.Sprintf(":%v", nginx.StatusPort))
 	if err != nil {
-		t.Fatalf("crating unix listener: %s", err)
+		t.Fatalf("crating tcp listener: %s", err)
 	}
 	defer listener.Close()
-	defer os.Remove(nginx.StatusSocket)
 
 	server := &httptest.Server{
 		Listener: listener,

internal/ingress/controller/config/config.go

@@ -796,8 +796,8 @@ type TemplateConfig struct {
 	EnableMetrics            bool
 
 	PID          string
-	StatusSocket string
 	StatusPath   string
+	StatusPort   int
 	StreamSocket string
 }
 

internal/ingress/controller/controller.go

@@ -37,6 +37,7 @@ import (
 	"k8s.io/ingress-nginx/internal/ingress/annotations/proxy"
 	ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config"
 	"k8s.io/ingress-nginx/internal/k8s"
+	"k8s.io/ingress-nginx/internal/nginx"
 	"k8s.io/klog"
 )
 
@@ -268,6 +269,8 @@ func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Pr
 		n.cfg.ListenPorts.SSLProxy,
 		n.cfg.ListenPorts.Health,
 		n.cfg.ListenPorts.Default,
+		10255, // profiling port
+		nginx.StatusPort,
 	}
 	reserverdPorts := sets.NewInt(rp...)
 	// svcRef format: <(str)namespace>/<(str)service>:<(intstr)port>[:<("PROXY")decode>:<("PROXY")encode>]

internal/ingress/controller/nginx.go

@@ -605,8 +605,8 @@ func (n NGINXController) generateTemplate(cfg ngx_config.Configuration, ingressC
 
 		HealthzURI:   nginx.HealthPath,
 		PID:          nginx.PID,
-		StatusSocket: nginx.StatusSocket,
 		StatusPath:   nginx.StatusPath,
+		StatusPort:   nginx.StatusPort,
 		StreamSocket: nginx.StreamSocket,
 	}
 

internal/ingress/controller/nginx_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package controller
 
 import (
+	"fmt"
 	"io"
 	"io/ioutil"
 	"net"
@@ -148,16 +149,15 @@ func TestIsDynamicConfigurationEnough(t *testing.T) {
 }
 
 func TestConfigureDynamically(t *testing.T) {
-	listener, err := net.Listen("unix", nginx.StatusSocket)
+	listener, err := net.Listen("tcp", fmt.Sprintf(":%v", nginx.StatusPort))
 	if err != nil {
-		t.Errorf("crating unix listener: %s", err)
+		t.Fatalf("crating unix listener: %s", err)
 	}
 	defer listener.Close()
-	defer os.Remove(nginx.StatusSocket)
 
 	streamListener, err := net.Listen("unix", nginx.StreamSocket)
 	if err != nil {
-		t.Errorf("crating unix listener: %s", err)
+		t.Fatalf("crating unix listener: %s", err)
 	}
 	defer streamListener.Close()
 	defer os.Remove(nginx.StreamSocket)
@@ -319,12 +319,11 @@ func TestConfigureDynamically(t *testing.T) {
 }
 
 func TestConfigureCertificates(t *testing.T) {
-	listener, err := net.Listen("unix", nginx.StatusSocket)
+	listener, err := net.Listen("tcp", fmt.Sprintf(":%v", nginx.StatusPort))
 	if err != nil {
 		t.Fatalf("crating unix listener: %s", err)
 	}
 	defer listener.Close()
-	defer os.Remove(nginx.StatusSocket)
 
 	streamListener, err := net.Listen("unix", nginx.StreamSocket)
 	if err != nil {