Skip to content

Commit c2935ca

Browse files
aledbfaledbf
committed
Refactor health checks and wait until NGINX process ends
1 parent c85450c commit c2935ca

File tree

15 files changed

+124
-67
lines changed

15 files changed

+124
-67
lines changed

‎Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ container: clean-container .container-$(ARCH)
108108
mkdir -p $(TEMP_DIR)/rootfs
109109
cp bin/$(ARCH)/nginx-ingress-controller $(TEMP_DIR)/rootfs/nginx-ingress-controller
110110
cp bin/$(ARCH)/dbg $(TEMP_DIR)/rootfs/dbg
111+
cp bin/$(ARCH)/wait-shutdown $(TEMP_DIR)/rootfs/wait-shutdown
111112

112113
cp -RP ./* $(TEMP_DIR)
113114
$(SED_I) "s|BASEIMAGE|$(BASEIMAGE)|g" $(DOCKERFILE)

‎build/build.sh

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,12 @@ go build \
6060
-X ${PKG}/version.COMMIT=${GIT_COMMIT} \
6161
-X ${PKG}/version.REPO=${REPO_INFO}" \
6262
-o "bin/${ARCH}/dbg" "${PKG}/cmd/dbg"
63+
64+
65+
go build \
66+
"${GOBUILD_FLAGS}" \
67+
-ldflags "-s -w \
68+
-X ${PKG}/version.RELEASE=${TAG} \
69+
-X ${PKG}/version.COMMIT=${GIT_COMMIT} \
70+
-X ${PKG}/version.REPO=${REPO_INFO}" \
71+
-o "bin/${ARCH}/wait-shutdown" "${PKG}/cmd/waitshutdown"

‎cmd/nginx/main.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ func main() {
131131
mux := http.NewServeMux()
132132

133133
if conf.EnableProfiling {
134-
registerProfiler(mux)
134+
go registerProfiler()
135135
}
136136

137137
registerHealthz(ngx, mux)
@@ -265,7 +265,9 @@ func registerMetrics(reg *prometheus.Registry, mux *http.ServeMux) {
265265

266266
}
267267

268-
func registerProfiler(mux *http.ServeMux) {
268+
func registerProfiler() {
269+
mux := http.NewServeMux()
270+
269271
mux.HandleFunc("/debug/pprof/", pprof.Index)
270272
mux.HandleFunc("/debug/pprof/heap", pprof.Index)
271273
mux.HandleFunc("/debug/pprof/mutex", pprof.Index)
@@ -276,6 +278,12 @@ func registerProfiler(mux *http.ServeMux) {
276278
mux.HandleFunc("/debug/pprof/profile", pprof.Profile)
277279
mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
278280
mux.HandleFunc("/debug/pprof/trace", pprof.Trace)
281+
282+
server := &http.Server{
283+
Addr: fmt.Sprintf(":10255"),
284+
Handler: mux,
285+
}
286+
klog.Fatal(server.ListenAndServe())
279287
}
280288

281289
func startHTTPServer(port int, mux *http.ServeMux) {

‎cmd/waitshutdown/main.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
Copyright 2019 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package main
18+
19+
import (
20+
"os"
21+
"os/exec"
22+
"time"
23+
24+
"k8s.io/ingress-nginx/internal/nginx"
25+
"k8s.io/klog"
26+
)
27+
28+
func main() {
29+
err := exec.Command("bash", "-c", "pkill -SIGTERM -f nginx-ingress-controller").Run()
30+
if err != nil {
31+
klog.Errorf("unexpected error terminating ingress controller: %v", err)
32+
os.Exit(1)
33+
}
34+
35+
// wait for the NGINX process to terminate
36+
timer := time.NewTicker(time.Second * 1)
37+
for range timer.C {
38+
if !nginx.IsRunning() {
39+
timer.Stop()
40+
break
41+
}
42+
}
43+
}

‎internal/ingress/controller/checker.go

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525

2626
"github.com/ncabatoff/process-exporter/proc"
2727
"github.com/pkg/errors"
28-
"k8s.io/klog"
2928

3029
"k8s.io/ingress-nginx/internal/nginx"
3130
)
@@ -37,41 +36,48 @@ func (n NGINXController) Name() string {
3736

3837
// Check returns if the nginx healthz endpoint is returning ok (status code 200)
3938
func (n *NGINXController) Check(_ *http.Request) error {
40-
statusCode, _, err := nginx.NewGetStatusRequest(nginx.HealthPath)
41-
if err != nil {
42-
klog.Errorf("healthcheck error: %v", err)
43-
return err
39+
if n.isShuttingDown {
40+
return fmt.Errorf("the ingress controller is shutting down")
4441
}
4542

46-
if statusCode != 200 {
47-
klog.Errorf("healthcheck error: %v", statusCode)
48-
return fmt.Errorf("ingress controller is not healthy")
43+
// check the nginx master process is running
44+
fs, err := proc.NewFS("/proc", false)
45+
if err != nil {
46+
return errors.Wrap(err, "reading /proc directory")
4947
}
5048

51-
statusCode, _, err = nginx.NewGetStatusRequest("/is-dynamic-lb-initialized")
49+
f, err := ioutil.ReadFile(nginx.PID)
5250
if err != nil {
53-
klog.Errorf("healthcheck error: %v", err)
54-
return err
51+
return errors.Wrapf(err, "reading %v", nginx.PID)
5552
}
5653

57-
if statusCode != 200 {
58-
klog.Errorf("healthcheck error: %v", statusCode)
59-
return fmt.Errorf("dynamic load balancer not started")
54+
pid, err := strconv.Atoi(strings.TrimRight(string(f), "\r\n"))
55+
if err != nil {
56+
return errors.Wrapf(err, "reading NGINX PID from file %v", nginx.PID)
6057
}
6158

62-
// check the nginx master process is running
63-
fs, err := proc.NewFS("/proc", false)
59+
_, err = fs.NewProc(pid)
6460
if err != nil {
65-
return errors.Wrap(err, "unexpected error reading /proc directory")
61+
return errors.Wrapf(err, "checking for NGINX process with PID %v", pid)
6662
}
67-
f, err := ioutil.ReadFile(nginx.PID)
63+
64+
statusCode, _, err := nginx.NewGetStatusRequest(nginx.HealthPath)
6865
if err != nil {
69-
return errors.Wrapf(err, "unexpected error reading %v", nginx.PID)
66+
return errors.Wrapf(err, "checking if NGINX is running")
7067
}
71-
pid, err := strconv.Atoi(strings.TrimRight(string(f), "\r\n"))
68+
69+
if statusCode != 200 {
70+
return fmt.Errorf("ingress controller is not healthy (%v)", statusCode)
71+
}
72+
73+
statusCode, _, err = nginx.NewGetStatusRequest("/is-dynamic-lb-initialized")
7274
if err != nil {
73-
return errors.Wrapf(err, "unexpected error reading the nginx PID from %v", nginx.PID)
75+
return errors.Wrapf(err, "checking if the dynamic load balancer started")
7476
}
75-
_, err = fs.NewProc(pid)
76-
return err
77+
78+
if statusCode != 200 {
79+
return fmt.Errorf("dynamic load balancer not started")
80+
}
81+
82+
return nil
7783
}

‎internal/ingress/controller/checker_test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,11 @@ import (
3535
func TestNginxCheck(t *testing.T) {
3636
mux := http.NewServeMux()
3737

38-
listener, err := net.Listen("unix", nginx.StatusSocket)
38+
listener, err := net.Listen("tcp", fmt.Sprintf(":%v", nginx.StatusPort))
3939
if err != nil {
40-
t.Fatalf("crating unix listener: %s", err)
40+
t.Fatalf("crating tcp listener: %s", err)
4141
}
4242
defer listener.Close()
43-
defer os.Remove(nginx.StatusSocket)
4443

4544
server := &httptest.Server{
4645
Listener: listener,

‎internal/ingress/controller/config/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -796,8 +796,8 @@ type TemplateConfig struct {
796796
EnableMetrics bool
797797

798798
PID string
799-
StatusSocket string
800799
StatusPath string
800+
StatusPort int
801801
StreamSocket string
802802
}
803803

‎internal/ingress/controller/controller.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
"k8s.io/ingress-nginx/internal/ingress/annotations/proxy"
3838
ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config"
3939
"k8s.io/ingress-nginx/internal/k8s"
40+
"k8s.io/ingress-nginx/internal/nginx"
4041
"k8s.io/klog"
4142
)
4243

@@ -268,6 +269,8 @@ func (n *NGINXController) getStreamServices(configmapName string, proto apiv1.Pr
268269
n.cfg.ListenPorts.SSLProxy,
269270
n.cfg.ListenPorts.Health,
270271
n.cfg.ListenPorts.Default,
272+
10255, // profiling port
273+
nginx.StatusPort,
271274
}
272275
reserverdPorts := sets.NewInt(rp...)
273276
// svcRef format: <(str)namespace>/<(str)service>:<(intstr)port>[:<("PROXY")decode>:<("PROXY")encode>]

‎internal/ingress/controller/nginx.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -605,8 +605,8 @@ func (n NGINXController) generateTemplate(cfg ngx_config.Configuration, ingressC
605605

606606
HealthzURI: nginx.HealthPath,
607607
PID: nginx.PID,
608-
StatusSocket: nginx.StatusSocket,
609608
StatusPath: nginx.StatusPath,
609+
StatusPort: nginx.StatusPort,
610610
StreamSocket: nginx.StreamSocket,
611611
}
612612

‎internal/ingress/controller/nginx_test.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package controller
1818

1919
import (
20+
"fmt"
2021
"io"
2122
"io/ioutil"
2223
"net"
@@ -148,16 +149,15 @@ func TestIsDynamicConfigurationEnough(t *testing.T) {
148149
}
149150

150151
func TestConfigureDynamically(t *testing.T) {
151-
listener, err := net.Listen("unix", nginx.StatusSocket)
152+
listener, err := net.Listen("tcp", fmt.Sprintf(":%v", nginx.StatusPort))
152153
if err != nil {
153-
t.Errorf("crating unix listener: %s", err)
154+
t.Fatalf("crating unix listener: %s", err)
154155
}
155156
defer listener.Close()
156-
defer os.Remove(nginx.StatusSocket)
157157

158158
streamListener, err := net.Listen("unix", nginx.StreamSocket)
159159
if err != nil {
160-
t.Errorf("crating unix listener: %s", err)
160+
t.Fatalf("crating unix listener: %s", err)
161161
}
162162
defer streamListener.Close()
163163
defer os.Remove(nginx.StreamSocket)
@@ -319,12 +319,11 @@ func TestConfigureDynamically(t *testing.T) {
319319
}
320320

321321
func TestConfigureCertificates(t *testing.T) {
322-
listener, err := net.Listen("unix", nginx.StatusSocket)
322+
listener, err := net.Listen("tcp", fmt.Sprintf(":%v", nginx.StatusPort))
323323
if err != nil {
324324
t.Fatalf("crating unix listener: %s", err)
325325
}
326326
defer listener.Close()
327-
defer os.Remove(nginx.StatusSocket)
328327

329328
streamListener, err := net.Listen("unix", nginx.StreamSocket)
330329
if err != nil {

‎internal/ingress/metric/collectors/nginx_status_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"net"
2222
"net/http"
2323
"net/http/httptest"
24-
"os"
2524
"testing"
2625
"time"
2726

@@ -97,7 +96,7 @@ func TestStatusCollector(t *testing.T) {
9796

9897
for _, c := range cases {
9998
t.Run(c.name, func(t *testing.T) {
100-
listener, err := net.Listen("unix", nginx.StatusSocket)
99+
listener, err := net.Listen("tcp", fmt.Sprintf(":%v", nginx.StatusPort))
101100
if err != nil {
102101
t.Fatalf("crating unix listener: %s", err)
103102
}
@@ -145,7 +144,6 @@ func TestStatusCollector(t *testing.T) {
145144
cm.Stop()
146145

147146
listener.Close()
148-
os.Remove(nginx.StatusSocket)
149147
})
150148
}
151149
}

‎internal/nginx/main.go

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
"time"
2929

3030
ps "github.com/mitchellh/go-ps"
31-
"github.com/tv42/httpunix"
3231
"k8s.io/klog"
3332
)
3433

@@ -38,8 +37,8 @@ var TemplatePath = "/etc/nginx/template/nginx.tmpl"
3837
// PID defines the location of the pid file used by NGINX
3938
var PID = "/tmp/nginx.pid"
4039

41-
// StatusSocket defines the location of the unix socket used by NGINX for the status server
42-
var StatusSocket = "/tmp/nginx-status-server.sock"
40+
// StatusPort port used by NGINX for the status server
41+
var StatusPort = 10256
4342

4443
// HealthPath defines the path used to define the health check location in NGINX
4544
var HealthPath = "/healthz"
@@ -56,17 +55,12 @@ var StreamSocket = "/tmp/ingress-stream.sock"
5655

5756
var statusLocation = "nginx-status"
5857

59-
var httpClient *http.Client
60-
61-
func init() {
62-
httpClient = buildUnixSocketClient(HealthCheckTimeout)
63-
}
64-
6558
// NewGetStatusRequest creates a new GET request to the internal NGINX status server
6659
func NewGetStatusRequest(path string) (int, []byte, error) {
67-
url := fmt.Sprintf("%v://%v%v", httpunix.Scheme, statusLocation, path)
60+
url := fmt.Sprintf("http://127.0.0.1:%v%v", StatusPort, path)
6861

69-
res, err := httpClient.Get(url)
62+
client := http.Client{}
63+
res, err := client.Get(url)
7064
if err != nil {
7165
return 0, nil, err
7266
}
@@ -82,14 +76,15 @@ func NewGetStatusRequest(path string) (int, []byte, error) {
8276

8377
// NewPostStatusRequest creates a new POST request to the internal NGINX status server
8478
func NewPostStatusRequest(path, contentType string, data interface{}) (int, []byte, error) {
85-
url := fmt.Sprintf("%v://%v%v", httpunix.Scheme, statusLocation, path)
79+
url := fmt.Sprintf("http://127.0.0.1:%v%v", StatusPort, path)
8680

8781
buf, err := json.Marshal(data)
8882
if err != nil {
8983
return 0, nil, err
9084
}
9185

92-
res, err := httpClient.Post(url, contentType, bytes.NewReader(buf))
86+
client := http.Client{}
87+
res, err := client.Post(url, contentType, bytes.NewReader(buf))
9388
if err != nil {
9489
return 0, nil, err
9590
}
@@ -142,19 +137,6 @@ func readFileToString(path string) (string, error) {
142137
return string(contents), nil
143138
}
144139

145-
func buildUnixSocketClient(timeout time.Duration) *http.Client {
146-
u := &httpunix.Transport{
147-
DialTimeout: 1 * time.Second,
148-
RequestTimeout: timeout,
149-
ResponseHeaderTimeout: timeout,
150-
}
151-
u.RegisterLocation(statusLocation, StatusSocket)
152-
153-
return &http.Client{
154-
Transport: u,
155-
}
156-
}
157-
158140
// Version return details about NGINX
159141
func Version() string {
160142
flag := "-v"

‎rootfs/etc/nginx/template/nginx.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,7 @@ http {
558558

559559
# default server, used for NGINX healthcheck and access to nginx stats
560560
server {
561-
listen unix:{{ .StatusSocket }};
561+
listen 127.0.0.1:{{ .StatusPort }};
562562
set $proxy_upstream_name "internal";
563563

564564
keepalive_timeout 0;

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,14 @@ spec:
2222
- name: nginx-ingress-controller
2323
livenessProbe:
2424
timeoutSeconds: 1
25+
initialDelaySeconds: 1
26+
periodSeconds: 2
2527
readinessProbe:
2628
timeoutSeconds: 1
29+
initialDelaySeconds: 1
30+
periodSeconds: 2
31+
lifecycle:
32+
preStop:
33+
exec:
34+
command:
35+
- /wait-shutdown

0 commit comments

Comments
 (0)