Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add nginx debug flag #401

Merged
merged 9 commits into from
Oct 29, 2018
Merged
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions cmd/nginx-ingress/main.go
Original file line number Diff line number Diff line change
@@ -13,7 +13,6 @@ import (
"time"

"github.com/golang/glog"

"github.com/nginxinc/kubernetes-ingress/internal/controller"
"github.com/nginxinc/kubernetes-ingress/internal/handlers"
"github.com/nginxinc/kubernetes-ingress/internal/nginx"
@@ -91,6 +90,9 @@ The external address of the service is used when reporting the status of Ingress

nginxStatus = flag.Bool("nginx-status", true,
"Enable the NGINX stub_status, or the NGINX Plus API.")

nginxDebug = flag.Bool("nginx-debug", false,
"Enable debugging for NGINX. Uses the nginx-debug binary. Requires 'error-log-level: debug' in the ConfigMap.")
)

func main() {
@@ -154,11 +156,16 @@ func main() {
nginxIngressTemplatePath = *ingressTemplatePath
}

nginxBinaryPath := "/usr/sbin/nginx"
if *nginxDebug {
nginxBinaryPath = "/usr/sbin/nginx-debug"
}

templateExecutor, err := nginx.NewTemplateExecutor(nginxConfTemplatePath, nginxIngressTemplatePath, *healthStatus, *nginxStatus, allowedCIDRs, *nginxStatusPort)
if err != nil {
glog.Fatalf("Error creating TemplateExecutor: %v", err)
}
ngxc := nginx.NewNginxController("/etc/nginx/", local)
ngxc := nginx.NewNginxController("/etc/nginx/", nginxBinaryPath, local)

if *defaultServerSecret != "" {
ns, name, err := utils.ParseNamespaceName(*defaultServerSecret)
1 change: 1 addition & 0 deletions deployments/helm-chart/README.md
Original file line number Diff line number Diff line change
@@ -61,6 +61,7 @@ Parameter | Description | Default
`controller.kind` | The kind of the Ingress controller installation - deployment or daemonset. | deployment
`controller.nginxplus` | Deploys the Ingress controller for NGINX Plus. | false
`controller.hostNetwork` | Enables the Ingress controller pods to use the host's network namespace. | false
`controller.nginxDebug` | Enables debugging for NGINX. Uses the `nginx-debug` binary. Requires `error-log-level: debug` in the ConfigMap via `controller.config.entries`. | false
`controller.image.repository` | The image repository of the Ingress controller. | nginx/nginx-ingress
`controller.image.tag` | The tag of the Ingress controller image. | edge
`controller.image.pullPolicy` | The pull policy for the Ingress controller image. | IfNotPresent
1 change: 1 addition & 0 deletions deployments/helm-chart/templates/controller-daemonset.yaml
Original file line number Diff line number Diff line change
@@ -71,6 +71,7 @@ spec:
- -watch-namespace={{ .Values.controller.watchNamespace }}
{{- end }}
- -health-status={{ .Values.controller.healthStatus }}
- -nginx-debug={{ .Values.controller.nginxDebug }}
{{- if .Values.controller.nginxStatus.enable }}
- -nginx-status
- -nginx-status-port={{ .Values.controller.nginxStatus.port }}
Original file line number Diff line number Diff line change
@@ -57,6 +57,7 @@ spec:
- -watch-namespace={{ .Values.controller.watchNamespace }}
{{- end }}
- -health-status={{ .Values.controller.healthStatus }}
- -nginx-debug={{ .Values.controller.nginxDebug }}
{{- if .Values.controller.nginxStatus.enable }}
- -nginx-status
- -nginx-status-port={{ .Values.controller.nginxStatus.port }}
1 change: 1 addition & 0 deletions deployments/helm-chart/values.yaml
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@ controller:
kind: deployment
nginxplus: false
hostNetwork: false
nginxDebug: false
image:
repository: nginx/nginx-ingress
tag: "edge"
2 changes: 2 additions & 0 deletions docs/cli-arguments.md
Original file line number Diff line number Diff line change
@@ -37,6 +37,8 @@ Usage of ./nginx-ingress:
A ConfigMap resource for customizing NGINX configuration. If a ConfigMap is set,
but the Ingress controller is not able to fetch it from Kubernetes API, the Ingress controller will fail to start.
Format: <namespace>/<name>
-nginx-debug nginx-debug
Dean-Coakley marked this conversation as resolved.
Show resolved Hide resolved
Enable debugging for NGINX. Uses the nginx-debug binary. Requires 'error-log-level: debug' in the ConfigMap.
-nginx-plus
Enable support for NGINX Plus
-nginx-status
4 changes: 2 additions & 2 deletions internal/controller/controller_test.go
Original file line number Diff line number Diff line change
@@ -930,7 +930,7 @@ func TestFindIngressesForSecret(t *testing.T) {
if err != nil {
t.Fatalf("templateExecuter could not start: %v", err)
}
ngxc := nginx.NewNginxController("/etc/nginx", true)
ngxc := nginx.NewNginxController("/etc/nginx", "nginx", true)
apiCtrl, err := plus.NewNginxAPIController(&http.Client{}, "", true)
if err != nil {
t.Fatalf("NGINX API Controller could not start: %v", err)
@@ -1111,7 +1111,7 @@ func TestFindIngressesForSecretWithMinions(t *testing.T) {
if err != nil {
t.Fatalf("templateExecuter could not start: %v", err)
}
ngxc := nginx.NewNginxController("/etc/nginx", true)
ngxc := nginx.NewNginxController("/etc/nginx", "nginx", true)
apiCtrl, err := plus.NewNginxAPIController(&http.Client{}, "", true)
if err != nil {
t.Fatalf("NGINX API Controller could not start: %v", err)
31 changes: 28 additions & 3 deletions internal/nginx/configurator_test.go
Original file line number Diff line number Diff line change
@@ -7,7 +7,6 @@ import (
"testing"

"github.com/nginxinc/kubernetes-ingress/internal/nginx/plus"

api_v1 "k8s.io/api/core/v1"
extensions "k8s.io/api/extensions/v1beta1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -528,7 +527,7 @@ func createTestConfigurator() (*Configurator, error) {
if err != nil {
return nil, err
}
ngxc := NewNginxController("/etc/nginx", true)
ngxc := NewNginxController("/etc/nginx", "nginx", true)
apiCtrl, err := plus.NewNginxAPIController(&http.Client{}, "", true)
if err != nil {
return nil, err
@@ -545,7 +544,7 @@ func createTestConfiguratorInvalidIngressTemplate() (*Configurator, error) {
if err := templateExecutor.UpdateIngressTemplate(&invalidIngressTemplate); err != nil {
return nil, err
}
ngxc := NewNginxController("/etc/nginx", true)
ngxc := NewNginxController("/etc/nginx", "nginx", true)
apiCtrl, _ := plus.NewNginxAPIController(&http.Client{}, "", true)
return NewConfigurator(ngxc, NewDefaultConfig(), apiCtrl, templateExecutor), nil
}
@@ -852,3 +851,29 @@ func TestUpdateEndpointsMergeableIngressFailsWithInvalidTemplate(t *testing.T) {
t.Errorf("UpdateEndpointsMergeableIngress returned \n%v, but expected \n%v", nil, "template execution error")
}
}

func TestGetNginxCommand(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test must be in nginx_test.go
you don't need to create a configurator, only the Controller

cnf, err := createTestConfigurator()
if err != nil {
t.Errorf("Failed to create a test configurator: %v", err)
}

cnf.nginx.nginxBinaryPath = "/usr/sbin/nginx"

tests := []struct {
cmd string
expected string
}{
{"reload", "/usr/sbin/nginx -s reload"},
{"start", "/usr/sbin/nginx -s start"},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove the start one

{"stop", "/usr/sbin/nginx -s stop"},
}

for _, tt := range tests {
t.Run(tt.cmd, func(t *testing.T) {
if got := cnf.nginx.getNginxCommand(tt.cmd); got != tt.expected {
t.Errorf("getNginxCommand returned \n%v, but expected \n%v", got, tt.expected)
}
})
}
}
20 changes: 15 additions & 5 deletions internal/nginx/nginx.go
Original file line number Diff line number Diff line change
@@ -23,6 +23,7 @@ type Controller struct {
nginxConfdPath string
nginxSecretsPath string
local bool
nginxBinaryPath string
verifyConfigGenerator *verify.ConfigGenerator
verifyClient *verify.Client
configVersion int
@@ -187,12 +188,13 @@ func NewUpstreamWithDefaultServer(name string) Upstream {
Port: "8181",
MaxFails: 1,
FailTimeout: "10s",
}},
},
},
}
}

// NewNginxController creates a NGINX controller
func NewNginxController(nginxConfPath string, local bool) *Controller {
func NewNginxController(nginxConfPath string, nginxBinaryPath string, local bool) *Controller {
verifyConfigGenerator, err := verify.NewConfigGenerator()
if err != nil {
glog.Fatalf("error instantiating a verify.ConfigGenerator: %v", err)
@@ -202,6 +204,7 @@ func NewNginxController(nginxConfPath string, local bool) *Controller {
nginxConfdPath: path.Join(nginxConfPath, "conf.d"),
nginxSecretsPath: path.Join(nginxConfPath, "secrets"),
local: local,
nginxBinaryPath: nginxBinaryPath,
verifyConfigGenerator: verifyConfigGenerator,
configVersion: 0,
verifyClient: verify.NewClient(),
@@ -295,6 +298,10 @@ func (nginx *Controller) getSecretFileName(name string) string {
return path.Join(nginx.nginxSecretsPath, name)
}

func (nginx *Controller) getNginxCommand(cmd string) string {
Dean-Coakley marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Sprint(nginx.nginxBinaryPath, " -s ", cmd)
}

// Reload reloads NGINX
func (nginx *Controller) Reload() error {
if nginx.local {
@@ -306,7 +313,9 @@ func (nginx *Controller) Reload() error {
nginx.UpdateConfigVersionFile()

glog.V(3).Infof("Reloading nginx. configVersion: %v", nginx.configVersion)
if err := shellOut("nginx -s reload"); err != nil {

reloadCmd := nginx.getNginxCommand("reload")
if err := shellOut(reloadCmd); err != nil {
return fmt.Errorf("nginx reload failed: %v", err)
}
err := nginx.verifyClient.WaitForCorrectVersion(nginx.configVersion)
@@ -325,7 +334,7 @@ func (nginx *Controller) Start(done chan error) {
return
}

cmd := exec.Command("nginx")
cmd := exec.Command(nginx.nginxBinaryPath)
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
if err := cmd.Start(); err != nil {
@@ -345,7 +354,8 @@ func (nginx *Controller) Start(done chan error) {
// Quit shutdowns NGINX gracefully
func (nginx *Controller) Quit() {
if !nginx.local {
if err := shellOut("nginx -s quit"); err != nil {
quitCmd := nginx.getNginxCommand("quit")
if err := shellOut(quitCmd); err != nil {
glog.Fatalf("Failed to quit nginx: %v", err)
}
} else {