Add nginx debug flag (#401)

* Add nginx-debug flag * Add nginx-debug flag documentation * Add nginxDebug support to helm chart * Add unit test for getNginxCommand
nginx · Oct 29, 2018 · d0bea69 · d0bea69
1 parent 814c10f
commit d0bea69
Show file tree

Hide file tree

Showing 10 changed files with 59 additions and 12 deletions.
diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go
@@ -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)

diff --git a/deployments/helm-chart/README.md b/deployments/helm-chart/README.md
@@ -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

diff --git a/deployments/helm-chart/templates/controller-daemonset.yaml b/deployments/helm-chart/templates/controller-daemonset.yaml
@@ -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 }}

diff --git a/deployments/helm-chart/templates/controller-deployment.yaml b/deployments/helm-chart/templates/controller-deployment.yaml
@@ -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 }}

diff --git a/deployments/helm-chart/values.yaml b/deployments/helm-chart/values.yaml
@@ -3,6 +3,7 @@ controller:
   kind: deployment
   nginxplus: false
   hostNetwork: false
+  nginxDebug: false
   image:
     repository: nginx/nginx-ingress
     tag: "edge"

diff --git a/docs/cli-arguments.md b/docs/cli-arguments.md
@@ -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
+	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

diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go
@@ -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)

diff --git a/internal/nginx/configurator_test.go b/internal/nginx/configurator_test.go
@@ -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
 }

diff --git a/internal/nginx/nginx.go b/internal/nginx/nginx.go
@@ -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 {
+	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 {

diff --git a/internal/nginx/nginx_test.go b/internal/nginx/nginx_test.go
@@ -0,0 +1,25 @@
+package nginx
+
+import (
+	"testing"
+)
+
+func TestGetNginxCommand(t *testing.T) {
+	tests := []struct {
+		cmd             string
+		nginxBinaryPath string
+		expected        string
+	}{
+		{"reload", "/usr/sbin/nginx", "/usr/sbin/nginx -s reload"},
+		{"stop", "/usr/sbin/nginx-debug", "/usr/sbin/nginx-debug -s stop"},
+	}
+	for _, test := range tests {
+		t.Run(test.cmd, func(t *testing.T) {
+			nginx := NewNginxController("/etc/nginx", test.nginxBinaryPath, true)
+
+			if got := nginx.getNginxCommand(test.cmd); got != test.expected {
+				t.Errorf("getNginxCommand returned \n%v, but expected \n%v", got, test.expected)
+			}
+		})
+	}
+}