diff --git a/pkg/ingress/controller/checker.go b/pkg/ingress/controller/checker.go index 2850804b7..cc40fd41d 100644 --- a/pkg/ingress/controller/checker.go +++ b/pkg/ingress/controller/checker.go @@ -18,22 +18,21 @@ package controller import ( "fmt" - "io/ioutil" "net/http" "strconv" "strings" - "github.com/golang/glog" "github.com/ncabatoff/process-exporter/proc" + "github.com/pkg/errors" ) // Name returns the healthcheck name func (n NGINXController) Name() string { - return "Ingress Controller" + return "nginx-ingress-controller" } // Check returns if the nginx healthz endpoint is returning ok (status code 200) -func (n NGINXController) Check(_ *http.Request) error { +func (n *NGINXController) Check(_ *http.Request) error { res, err := http.Get(fmt.Sprintf("http://0.0.0.0:%v%v", n.cfg.ListenPorts.Status, ngxHealthPath)) if err != nil { return err @@ -46,23 +45,17 @@ func (n NGINXController) Check(_ *http.Request) error { // check the nginx master process is running fs, err := proc.NewFS("/proc") if err != nil { - glog.Errorf("%v", err) - return err + return errors.Wrap(err, "unexpected error reading /proc directory") } - f, err := ioutil.ReadFile("/run/nginx.pid") + f, err := n.fileSystem.ReadFile("/run/nginx.pid") if err != nil { - glog.Errorf("%v", err) - return err + return errors.Wrap(err, "unexpected error reading /run/nginx.pid") } pid, err := strconv.Atoi(strings.TrimRight(string(f), "\r\n")) if err != nil { - return err + return errors.Wrap(err, "unexpected error reading the PID from /run/nginx.pid") } _, err = fs.NewProc(pid) - if err != nil { - glog.Errorf("%v", err) - return err - } - return nil + return err } diff --git a/pkg/ingress/controller/checker_test.go b/pkg/ingress/controller/checker_test.go new file mode 100644 index 000000000..9620547b9 --- /dev/null +++ b/pkg/ingress/controller/checker_test.go @@ -0,0 +1,137 @@ +/* +Copyright 2017 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 controller + +import ( + "fmt" + "net" + "net/http" + "net/http/httptest" + "os/exec" + "testing" + + "k8s.io/apiserver/pkg/server/healthz" + "k8s.io/kubernetes/pkg/util/filesystem" + + ngx_config "k8s.io/ingress-nginx/pkg/ingress/controller/config" +) + +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") + })) + defer server.Close() + // port to be used in the check + p := server.Listener.Addr().(*net.TCPAddr).Port + + // mock filesystem + fs := filesystem.NewFakeFs() + + n := &NGINXController{ + cfg: &Configuration{ + ListenPorts: &ngx_config.ListenPorts{ + Status: p, + }, + }, + fileSystem: fs, + } + + t.Run("no pid or process", func(t *testing.T) { + if err := callHealthz(true, mux); err == nil { + t.Errorf("expected an error but none returned") + } + }) + + // create required files + fs.MkdirAll("/run", 0655) + pidFile, err := fs.Create("/run/nginx.pid") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + t.Run("no process", func(t *testing.T) { + if err := callHealthz(true, mux); err == nil { + t.Errorf("expected an error but none returned") + } + }) + + // start dummy process to use the PID + cmd := exec.Command("sleep", "3600") + cmd.Start() + pid := cmd.Process.Pid + defer cmd.Process.Kill() + go func() { + cmd.Wait() + }() + + pidFile.Write([]byte(fmt.Sprintf("%v", pid))) + pidFile.Close() + + healthz.InstallHandler(mux, n) + + t.Run("valid request", func(t *testing.T) { + if err := callHealthz(false, mux); err != nil { + t.Error(err) + } + }) + + pidFile, err = fs.Create("/run/nginx.pid") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + pidFile.Write([]byte(fmt.Sprintf("%v", pid))) + pidFile.Close() + + t.Run("valid request", func(t *testing.T) { + if err := callHealthz(true, mux); err == nil { + t.Errorf("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.Errorf("expected an error but none returned") + } + }) +} + +func callHealthz(expErr bool, mux *http.ServeMux) error { + req, err := http.NewRequest("GET", "http://localhost:8080/healthz", nil) + if err != nil { + return err + } + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + if expErr && w.Code != http.StatusInternalServerError { + return fmt.Errorf("expected an error") + } + + if w.Body.String() != "ok" { + return fmt.Errorf("healthz error: %v", w.Body.String()) + } + + if w.Code != http.StatusOK { + return fmt.Errorf("expected status code 200 but %v returned", w.Code) + } + + return nil +} diff --git a/pkg/ingress/controller/nginx.go b/pkg/ingress/controller/nginx.go index 0dba82b1e..07236eef4 100644 --- a/pkg/ingress/controller/nginx.go +++ b/pkg/ingress/controller/nginx.go @@ -40,6 +40,7 @@ import ( v1core "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/tools/record" "k8s.io/client-go/util/flowcontrol" + "k8s.io/kubernetes/pkg/util/filesystem" "k8s.io/ingress-nginx/pkg/ingress" "k8s.io/ingress-nginx/pkg/ingress/annotations/class" @@ -110,6 +111,8 @@ func NewNGINXController(config *Configuration) *NGINXController { stopCh: make(chan struct{}), stopLock: &sync.Mutex{}, + + fileSystem: filesystem.DefaultFs{}, } n.stats = newStatsCollector(config.Namespace, config.IngressClass, n.binary, n.cfg.ListenPorts.Status) @@ -223,6 +226,8 @@ type NGINXController struct { Proxy *TCPProxy backendDefaults defaults.Backend + + fileSystem filesystem.Filesystem } // Start start a new NGINX master process running in foreground. diff --git a/pkg/ingress/controller/util.go b/pkg/ingress/controller/util.go index 09be63aa4..c527ceaeb 100644 --- a/pkg/ingress/controller/util.go +++ b/pkg/ingress/controller/util.go @@ -17,11 +17,14 @@ limitations under the License. package controller import ( + "syscall" + "github.com/golang/glog" "github.com/imdario/mergo" api "k8s.io/api/core/v1" + "k8s.io/kubernetes/pkg/util/sysctl" "k8s.io/ingress-nginx/pkg/ingress" ) @@ -53,3 +56,38 @@ func mergeLocationAnnotations(loc *ingress.Location, anns map[string]interface{} glog.Errorf("unexpected error merging extracted annotations in location type: %v", err) } } + +// sysctlSomaxconn returns the value of net.core.somaxconn, i.e. +// maximum number of connections that can be queued for acceptance +// http://nginx.org/en/docs/http/ngx_http_core_module.html#listen +func sysctlSomaxconn() int { + maxConns, err := sysctl.New().GetSysctl("net/core/somaxconn") + if err != nil || maxConns < 512 { + glog.V(3).Infof("system net.core.somaxconn=%v (using system default)", maxConns) + return 511 + } + + return maxConns +} + +// sysctlFSFileMax returns the value of fs.file-max, i.e. +// maximum number of open file descriptors +func sysctlFSFileMax() int { + var rLimit syscall.Rlimit + err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &rLimit) + if err != nil { + glog.Errorf("unexpected error reading system maximum number of open file descriptors (RLIMIT_NOFILE): %v", err) + // returning 0 means don't render the value + return 0 + } + return int(rLimit.Max) +} + +func intInSlice(i int, list []int) bool { + for _, v := range list { + if v == i { + return true + } + } + return false +} diff --git a/pkg/ingress/controller/util_test.go b/pkg/ingress/controller/util_test.go index 3b4465522..be3e382f6 100644 --- a/pkg/ingress/controller/util_test.go +++ b/pkg/ingress/controller/util_test.go @@ -81,3 +81,37 @@ func TestMergeLocationAnnotations(t *testing.T) { t.Errorf("%s should be removed after mergeLocationAnnotations", DeniedKeyName) } } + +func TestIntInSlice(t *testing.T) { + fooTests := []struct { + i int + list []int + er bool + }{ + {1, []int{1, 2}, true}, + {3, []int{1, 2}, false}, + {1, nil, false}, + {0, nil, false}, + } + + for _, fooTest := range fooTests { + r := intInSlice(fooTest.i, fooTest.list) + if r != fooTest.er { + t.Errorf("returned %t but expected %t for s=%v & list=%v", r, fooTest.er, fooTest.i, fooTest.list) + } + } +} + +func TestSysctlFSFileMax(t *testing.T) { + i := sysctlFSFileMax() + if i < 1 { + t.Errorf("returned %v but expected > 0", i) + } +} + +func TestSysctlSomaxconn(t *testing.T) { + i := sysctlSomaxconn() + if i < 511 { + t.Errorf("returned %v but expected >= 511", i) + } +} diff --git a/pkg/ingress/controller/utils.go b/pkg/ingress/controller/utils.go deleted file mode 100644 index d76f8102c..000000000 --- a/pkg/ingress/controller/utils.go +++ /dev/null @@ -1,60 +0,0 @@ -/* -Copyright 2015 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 controller - -import ( - "syscall" - - "github.com/golang/glog" - - "k8s.io/kubernetes/pkg/util/sysctl" -) - -// sysctlSomaxconn returns the value of net.core.somaxconn, i.e. -// maximum number of connections that can be queued for acceptance -// http://nginx.org/en/docs/http/ngx_http_core_module.html#listen -func sysctlSomaxconn() int { - maxConns, err := sysctl.New().GetSysctl("net/core/somaxconn") - if err != nil || maxConns < 512 { - glog.V(3).Infof("system net.core.somaxconn=%v (using system default)", maxConns) - return 511 - } - - return maxConns -} - -// sysctlFSFileMax returns the value of fs.file-max, i.e. -// maximum number of open file descriptors -func sysctlFSFileMax() int { - var rLimit syscall.Rlimit - err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &rLimit) - if err != nil { - glog.Errorf("unexpected error reading system maximum number of open file descriptors (RLIMIT_NOFILE): %v", err) - // returning 0 means don't render the value - return 0 - } - return int(rLimit.Max) -} - -func intInSlice(i int, list []int) bool { - for _, v := range list { - if v == i { - return true - } - } - return false -}