Refactor health checks and wait until NGINX process ends

Makefile

@@ -108,6 +108,7 @@ container: clean-container .container-$(ARCH)
 	mkdir -p $(TEMP_DIR)/rootfs
 	cp bin/$(ARCH)/nginx-ingress-controller $(TEMP_DIR)/rootfs/nginx-ingress-controller
 	cp bin/$(ARCH)/dbg $(TEMP_DIR)/rootfs/dbg
+	cp bin/$(ARCH)/shutdown $(TEMP_DIR)/rootfs/shutdown
 
 	cp -RP ./* $(TEMP_DIR)
 	$(SED_I) "s|BASEIMAGE|$(BASEIMAGE)|g" $(DOCKERFILE)

build/build.sh

@@ -60,3 +60,12 @@ go build \
     -X ${PKG}/version.COMMIT=${GIT_COMMIT} \
     -X ${PKG}/version.REPO=${REPO_INFO}" \
   -o "bin/${ARCH}/dbg" "${PKG}/cmd/dbg"
+
+
+go build \
+  "${GOBUILD_FLAGS}" \
+  -ldflags "-s -w \
+    -X ${PKG}/version.RELEASE=${TAG} \
+    -X ${PKG}/version.COMMIT=${GIT_COMMIT} \
+    -X ${PKG}/version.REPO=${REPO_INFO}" \
+  -o "bin/${ARCH}/wait-shutdown" "${PKG}/cmd/waitshutdown"

cmd/nginx/main.go

@@ -131,7 +131,7 @@ func main() {
 	mux := http.NewServeMux()
 
 	if conf.EnableProfiling {
-		registerProfiler(mux)
+		go registerProfiler()
 	}
 
 	registerHealthz(ngx, mux)
@@ -265,7 +265,9 @@ func registerMetrics(reg *prometheus.Registry, mux *http.ServeMux) {
 
 }
 
-func registerProfiler(mux *http.ServeMux) {
+func registerProfiler() {
+	mux := http.NewServeMux()
+
 	mux.HandleFunc("/debug/pprof/", pprof.Index)
 	mux.HandleFunc("/debug/pprof/heap", pprof.Index)
 	mux.HandleFunc("/debug/pprof/mutex", pprof.Index)
@@ -276,6 +278,12 @@ func registerProfiler(mux *http.ServeMux) {
 	mux.HandleFunc("/debug/pprof/profile", pprof.Profile)
 	mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
 	mux.HandleFunc("/debug/pprof/trace", pprof.Trace)
+
+	server := &http.Server{
+		Addr:    fmt.Sprintf(":10255"),
+		Handler: mux,
+	}
+	klog.Fatal(server.ListenAndServe())
 }
 
 func startHTTPServer(port int, mux *http.ServeMux) {

cmd/waitshutdown/main.go

@@ -0,0 +1,34 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+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 main
+
+import (
+	"time"
+
+	"k8s.io/ingress-nginx/internal/nginx"
+)
+
+func main() {
+	// wait for the NGINX process to terminate
+	timer := time.NewTicker(time.Second * 1)
+	for range timer.C {
+		if !nginx.IsRunning() {
+			timer.Stop()
+			break
+		}
+	}
+}

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 n.isShuttingDown {
+		return fmt.Errorf("the ingress controller is shutting down")
 	}
 
-	if statusCode != 200 {
-		klog.Errorf("healthcheck error: %v", statusCode)
-		return fmt.Errorf("ingress controller is not healthy")
+	// check the nginx master process is running
+	fs, err := proc.NewFS("/proc", false)
+	if err != nil {
+		return errors.Wrap(err, "reading /proc directory")
 	}
 
-	statusCode, _, err = nginx.NewGetStatusRequest("/is-dynamic-lb-initialized")
+	f, err := ioutil.ReadFile(nginx.PID)
 	if err != nil {
-		klog.Errorf("healthcheck error: %v", err)
-		return err
+		return errors.Wrapf(err, "reading %v", nginx.PID)
 	}
 
-	if statusCode != 200 {
-		klog.Errorf("healthcheck error: %v", statusCode)
-		return fmt.Errorf("dynamic load balancer not started")
+	pid, err := strconv.Atoi(strings.TrimRight(string(f), "\r\n"))
+	if err != nil {
+		return errors.Wrapf(err, "reading NGINX PID from file %v", nginx.PID)
 	}
 
-	// check the nginx master process is running
-	fs, err := proc.NewFS("/proc", false)
+	_, err = fs.NewProc(pid)
 	if err != nil {
-		return errors.Wrap(err, "unexpected error reading /proc directory")
+		return errors.Wrapf(err, "checking for NGINX process with PID %v", pid)
 	}
-	f, err := ioutil.ReadFile(nginx.PID)
+
+	statusCode, _, err := nginx.NewGetStatusRequest(nginx.HealthPath)
 	if err != nil {
-		return errors.Wrapf(err, "unexpected error reading %v", nginx.PID)
+		return errors.Wrapf(err, "checking if NGINX is running")
 	}
-	pid, err := strconv.Atoi(strings.TrimRight(string(f), "\r\n"))
+
+	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, "unexpected error reading the nginx PID from %v", nginx.PID)
+		return errors.Wrapf(err, "checking if the dynamic load balancer started")
 	}
-	_, err = fs.NewProc(pid)
-	return err
+
+	if statusCode != 200 {
+		return fmt.Errorf("dynamic load balancer not started")
+	}
+
+	return nil
 }

internal/ingress/controller/nginx.go

@@ -414,7 +414,7 @@ func (n *NGINXController) Stop() error {
 	// wait for the NGINX process to terminate
 	timer := time.NewTicker(time.Second * 1)
 	for range timer.C {
-		if !process.IsNginxRunning() {
+		if !nginx.IsRunning() {
 			klog.Info("NGINX process has stopped")
 			timer.Stop()
 			break

internal/ingress/controller/process/nginx.go

@@ -24,7 +24,6 @@ import (
 	"syscall"
 	"time"
 
-	ps "github.com/mitchellh/go-ps"
 	"github.com/ncabatoff/process-exporter/proc"
 	"k8s.io/klog"
 )
@@ -82,14 +81,3 @@ func WaitUntilPortIsAvailable(port int) {
 		time.Sleep(100 * time.Millisecond)
 	}
 }
-
-// IsNginxRunning returns true if a process with the name 'nginx' is found
-func IsNginxRunning() bool {
-	processes, _ := ps.Processes()
-	for _, p := range processes {
-		if p.Executable() == "nginx" {
-			return true
-		}
-	}
-	return false
-}

internal/nginx/main.go

@@ -27,6 +27,7 @@ import (
 	"strings"
 	"time"
 
+	ps "github.com/mitchellh/go-ps"
 	"github.com/tv42/httpunix"
 	"k8s.io/klog"
 )
@@ -171,3 +172,15 @@ func Version() string {
 
 	return string(out)
 }
+
+// IsRunning returns true if a process with the name 'nginx' is found
+func IsRunning() bool {
+	processes, _ := ps.Processes()
+	for _, p := range processes {
+		if p.Executable() == "nginx" {
+			return true
+		}
+	}
+
+	return false
+}

test/e2e-image/overlay/deployment-e2e.yaml

@@ -24,3 +24,8 @@ spec:
             timeoutSeconds: 1
           readinessProbe:
             timeoutSeconds: 1
+          lifecycle:
+            preStop:
+              exec:
+                command:
+                  - /wait-shutdown