From 9c529a71ec948b2c6dc03396c968024a50355434 Mon Sep 17 00:00:00 2001 From: dean-coakley Date: Fri, 19 Oct 2018 15:05:59 +0100 Subject: [PATCH 1/9] Add nginx-debug flag --- cmd/nginx-ingress/main.go | 5 ++++- internal/nginx/nginx.go | 7 +++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index 9876729e31..798f6e5656 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -91,6 +91,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. All interactions with NGINX are done through the nginx-debug binary.") ) func main() { @@ -158,7 +161,7 @@ func main() { if err != nil { glog.Fatalf("Error creating TemplateExecutor: %v", err) } - ngxc := nginx.NewNginxController("/etc/nginx/", local) + ngxc := nginx.NewNginxController("/etc/nginx/", local, *nginxDebug) if *defaultServerSecret != "" { ns, name, err := utils.ParseNamespaceName(*defaultServerSecret) diff --git a/internal/nginx/nginx.go b/internal/nginx/nginx.go index 9ec7046d86..223b7fa620 100644 --- a/internal/nginx/nginx.go +++ b/internal/nginx/nginx.go @@ -23,6 +23,7 @@ type Controller struct { nginxConfdPath string nginxSecretsPath string local bool + debug bool 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, local bool, debug 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, + debug: debug, verifyConfigGenerator: verifyConfigGenerator, configVersion: 0, verifyClient: verify.NewClient(), From 11e24c3715ae26dcb4d25fe204d9dc7a877baaba Mon Sep 17 00:00:00 2001 From: dean-coakley Date: Fri, 19 Oct 2018 15:11:12 +0100 Subject: [PATCH 2/9] Add nginx-debug support to helm chart --- deployments/helm-chart/templates/controller-daemonset.yaml | 1 + deployments/helm-chart/templates/controller-deployment.yaml | 1 + deployments/helm-chart/values.yaml | 1 + 3 files changed, 3 insertions(+) diff --git a/deployments/helm-chart/templates/controller-daemonset.yaml b/deployments/helm-chart/templates/controller-daemonset.yaml index f2d120e2ee..464a9fd0f6 100644 --- 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 index 5c19374de7..51f43e2c18 100644 --- 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 index bd1ca4f27c..6f5e36a21f 100644 --- 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" From 71f625d49a27c0b8c78349dfdea0341523d30896 Mon Sep 17 00:00:00 2001 From: dean-coakley Date: Fri, 19 Oct 2018 15:35:30 +0100 Subject: [PATCH 3/9] Add nginx-debug flag documentation --- cmd/nginx-ingress/main.go | 2 +- deployments/helm-chart/README.md | 1 + docs/cli-arguments.md | 2 ++ 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index 798f6e5656..eed00faeee 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -93,7 +93,7 @@ The external address of the service is used when reporting the status of Ingress "Enable the NGINX stub_status, or the NGINX Plus API.") nginxDebug = flag.Bool("nginx-debug", false, - "Enable debugging. All interactions with NGINX are done through the nginx-debug binary.") + "Enable debugging for NGINX. Uses the `nginx-debug` binary. Requires 'error-log-level: debug' in the ConfigMap") ) func main() { diff --git a/deployments/helm-chart/README.md b/deployments/helm-chart/README.md index 869d175441..1216e9b923 100644 --- 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/docs/cli-arguments.md b/docs/cli-arguments.md index d3c7a7cec0..da493cb045 100644 --- 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: / + -nginx-debug 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 From 4428974f3be44acc264a71bd4794d11c509e2e65 Mon Sep 17 00:00:00 2001 From: dean-coakley Date: Fri, 19 Oct 2018 15:10:04 +0100 Subject: [PATCH 4/9] Fix unit tests by adding nginx-debug arg --- internal/controller/controller_test.go | 4 ++-- internal/nginx/configurator_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go index 06205451c8..9ae16ad7e0 100644 --- 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", true, false) 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", true, false) 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 index ef2cf877b6..9e6a925a06 100644 --- a/internal/nginx/configurator_test.go +++ b/internal/nginx/configurator_test.go @@ -528,7 +528,7 @@ func createTestConfigurator() (*Configurator, error) { if err != nil { return nil, err } - ngxc := NewNginxController("/etc/nginx", true) + ngxc := NewNginxController("/etc/nginx", true, false) apiCtrl, err := plus.NewNginxAPIController(&http.Client{}, "", true) if err != nil { return nil, err @@ -545,7 +545,7 @@ func createTestConfiguratorInvalidIngressTemplate() (*Configurator, error) { if err := templateExecutor.UpdateIngressTemplate(&invalidIngressTemplate); err != nil { return nil, err } - ngxc := NewNginxController("/etc/nginx", true) + ngxc := NewNginxController("/etc/nginx", true, false) apiCtrl, _ := plus.NewNginxAPIController(&http.Client{}, "", true) return NewConfigurator(ngxc, NewDefaultConfig(), apiCtrl, templateExecutor), nil } From 928f4d92e6aea0717b23e16152eb74f4bc4fff34 Mon Sep 17 00:00:00 2001 From: dean-coakley Date: Wed, 24 Oct 2018 17:50:56 +0100 Subject: [PATCH 5/9] Change nginx-debug to set name of binary --- cmd/nginx-ingress/main.go | 10 +++++++--- deployments/helm-chart/README.md | 2 +- internal/controller/controller_test.go | 4 ++-- internal/nginx/configurator_test.go | 5 ++--- internal/nginx/nginx.go | 21 ++++++++++++++++----- 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index eed00faeee..11feb8081d 100644 --- 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" @@ -93,7 +92,7 @@ The external address of the service is used when reporting the status of Ingress "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") + "Enable debugging for NGINX. Uses the `nginx-debug` binary. Requires 'error-log-level: debug' in the ConfigMap.") ) func main() { @@ -157,11 +156,16 @@ func main() { nginxIngressTemplatePath = *ingressTemplatePath } + nginxBinary := "nginx" + if *nginxDebug { + nginxBinary = "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, *nginxDebug) + ngxc := nginx.NewNginxController("/etc/nginx/", local, nginxBinary) if *defaultServerSecret != "" { ns, name, err := utils.ParseNamespaceName(*defaultServerSecret) diff --git a/deployments/helm-chart/README.md b/deployments/helm-chart/README.md index 1216e9b923..65eafe5ac3 100644 --- a/deployments/helm-chart/README.md +++ b/deployments/helm-chart/README.md @@ -61,7 +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.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/internal/controller/controller_test.go b/internal/controller/controller_test.go index 9ae16ad7e0..0a4b982d42 100644 --- 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, false) + ngxc := nginx.NewNginxController("/etc/nginx", true, "nginx") 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, false) + ngxc := nginx.NewNginxController("/etc/nginx", true, "nginx") 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 index 9e6a925a06..e4da461bd6 100644 --- 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, false) + ngxc := NewNginxController("/etc/nginx", true, "nginx") 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, false) + ngxc := NewNginxController("/etc/nginx", true, "nginx") 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 index 223b7fa620..b2f1332ae1 100644 --- a/internal/nginx/nginx.go +++ b/internal/nginx/nginx.go @@ -23,7 +23,7 @@ type Controller struct { nginxConfdPath string nginxSecretsPath string local bool - debug bool + nginxBinaryPath string verifyConfigGenerator *verify.ConfigGenerator verifyClient *verify.Client configVersion int @@ -194,7 +194,7 @@ func NewUpstreamWithDefaultServer(name string) Upstream { } // NewNginxController creates a NGINX controller -func NewNginxController(nginxConfPath string, local bool, debug bool) *Controller { +func NewNginxController(nginxConfPath string, local bool, nginxBinary string) *Controller { verifyConfigGenerator, err := verify.NewConfigGenerator() if err != nil { glog.Fatalf("error instantiating a verify.ConfigGenerator: %v", err) @@ -204,7 +204,11 @@ func NewNginxController(nginxConfPath string, local bool, debug bool) *Controlle nginxConfdPath: path.Join(nginxConfPath, "conf.d"), nginxSecretsPath: path.Join(nginxConfPath, "secrets"), local: local, +<<<<<<< HEAD debug: debug, +======= + nginxBinaryPath: path.Join("/usr/sbin/", nginxBinary), +>>>>>>> Change nginx-debug to set name of binary verifyConfigGenerator: verifyConfigGenerator, configVersion: 0, verifyClient: verify.NewClient(), @@ -298,6 +302,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 { @@ -309,7 +317,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) @@ -328,7 +338,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 { @@ -348,7 +358,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 { From 14867b214e8e84226b111ae61e88d0f9414505c6 Mon Sep 17 00:00:00 2001 From: dean-coakley Date: Thu, 25 Oct 2018 16:42:46 +0100 Subject: [PATCH 6/9] Fix docs. Fix args order. Add unit test * Add unit test for getNginxCommand * Fix cli-args documentation * nginxBinaryPath is now the second arg in NewNginxController --- cmd/nginx-ingress/main.go | 8 +++---- docs/cli-arguments.md | 2 +- internal/controller/controller_test.go | 4 ++-- internal/nginx/configurator_test.go | 30 ++++++++++++++++++++++++-- internal/nginx/nginx.go | 8 ++----- 5 files changed, 37 insertions(+), 15 deletions(-) diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index 11feb8081d..332bc26aab 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -92,7 +92,7 @@ The external address of the service is used when reporting the status of Ingress "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.") + "Enable debugging for NGINX. Uses the nginx-debug binary. Requires 'error-log-level: debug' in the ConfigMap.") ) func main() { @@ -156,16 +156,16 @@ func main() { nginxIngressTemplatePath = *ingressTemplatePath } - nginxBinary := "nginx" + nginxBinaryPath := "/usr/sbin/nginx" if *nginxDebug { - nginxBinary = "nginx-debug" + 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, nginxBinary) + ngxc := nginx.NewNginxController("/etc/nginx/", nginxBinaryPath, local) if *defaultServerSecret != "" { ns, name, err := utils.ParseNamespaceName(*defaultServerSecret) diff --git a/docs/cli-arguments.md b/docs/cli-arguments.md index da493cb045..0c96b0ef1d 100644 --- a/docs/cli-arguments.md +++ b/docs/cli-arguments.md @@ -38,7 +38,7 @@ Usage of ./nginx-ingress: but the Ingress controller is not able to fetch it from Kubernetes API, the Ingress controller will fail to start. Format: / -nginx-debug nginx-debug - Enable debugging for NGINX. Uses the nginx-debug binary. Requires 'error-log-level: debug' in the ConfigMap + 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 index 0a4b982d42..0980eb9550 100644 --- 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, "nginx") + 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, "nginx") + 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 index e4da461bd6..c5e0cde7fe 100644 --- a/internal/nginx/configurator_test.go +++ b/internal/nginx/configurator_test.go @@ -527,7 +527,7 @@ func createTestConfigurator() (*Configurator, error) { if err != nil { return nil, err } - ngxc := NewNginxController("/etc/nginx", true, "nginx") + ngxc := NewNginxController("/etc/nginx", "nginx", true) apiCtrl, err := plus.NewNginxAPIController(&http.Client{}, "", true) if err != nil { return nil, err @@ -544,7 +544,7 @@ func createTestConfiguratorInvalidIngressTemplate() (*Configurator, error) { if err := templateExecutor.UpdateIngressTemplate(&invalidIngressTemplate); err != nil { return nil, err } - ngxc := NewNginxController("/etc/nginx", true, "nginx") + ngxc := NewNginxController("/etc/nginx", "nginx", true) apiCtrl, _ := plus.NewNginxAPIController(&http.Client{}, "", true) return NewConfigurator(ngxc, NewDefaultConfig(), apiCtrl, templateExecutor), nil } @@ -851,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) { + 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"}, + {"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) + } + }) + } +} diff --git a/internal/nginx/nginx.go b/internal/nginx/nginx.go index b2f1332ae1..203be282d8 100644 --- a/internal/nginx/nginx.go +++ b/internal/nginx/nginx.go @@ -194,7 +194,7 @@ func NewUpstreamWithDefaultServer(name string) Upstream { } // NewNginxController creates a NGINX controller -func NewNginxController(nginxConfPath string, local bool, nginxBinary string) *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) @@ -204,11 +204,7 @@ func NewNginxController(nginxConfPath string, local bool, nginxBinary string) *C nginxConfdPath: path.Join(nginxConfPath, "conf.d"), nginxSecretsPath: path.Join(nginxConfPath, "secrets"), local: local, -<<<<<<< HEAD - debug: debug, -======= - nginxBinaryPath: path.Join("/usr/sbin/", nginxBinary), ->>>>>>> Change nginx-debug to set name of binary + nginxBinaryPath: nginxBinaryPath, verifyConfigGenerator: verifyConfigGenerator, configVersion: 0, verifyClient: verify.NewClient(), From 7325911a105884c662a50668595a88816d5bf941 Mon Sep 17 00:00:00 2001 From: dean-coakley Date: Fri, 26 Oct 2018 00:26:35 +0100 Subject: [PATCH 7/9] Fix cli-args docs --- docs/cli-arguments.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/cli-arguments.md b/docs/cli-arguments.md index 0c96b0ef1d..c62c379db1 100644 --- a/docs/cli-arguments.md +++ b/docs/cli-arguments.md @@ -37,8 +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: / - -nginx-debug nginx-debug - Enable debugging for NGINX. Uses the nginx-debug binary. Requires 'error-log-level: debug' in the ConfigMap. + -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 From 6a864c658b7a4ef6dc69233c782dcf08e32b5189 Mon Sep 17 00:00:00 2001 From: dean-coakley Date: Fri, 26 Oct 2018 00:26:47 +0100 Subject: [PATCH 8/9] Move test to nginx_test.go and improve test * Improved test by allowing it to take an argument to specify which nginx binary you want to use in the configurator --- internal/nginx/configurator_test.go | 26 -------------------------- internal/nginx/nginx_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 26 deletions(-) create mode 100644 internal/nginx/nginx_test.go diff --git a/internal/nginx/configurator_test.go b/internal/nginx/configurator_test.go index c5e0cde7fe..7e2a4853a0 100644 --- a/internal/nginx/configurator_test.go +++ b/internal/nginx/configurator_test.go @@ -851,29 +851,3 @@ func TestUpdateEndpointsMergeableIngressFailsWithInvalidTemplate(t *testing.T) { t.Errorf("UpdateEndpointsMergeableIngress returned \n%v, but expected \n%v", nil, "template execution error") } } - -func TestGetNginxCommand(t *testing.T) { - 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"}, - {"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) - } - }) - } -} diff --git a/internal/nginx/nginx_test.go b/internal/nginx/nginx_test.go new file mode 100644 index 0000000000..3327469f91 --- /dev/null +++ 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 _, tt := range tests { + t.Run(tt.cmd, func(t *testing.T) { + nginx := NewNginxController("/etc/nginx", tt.nginxBinaryPath, true) + + if got := nginx.getNginxCommand(tt.cmd); got != tt.expected { + t.Errorf("getNginxCommand returned \n%v, but expected \n%v", got, tt.expected) + } + }) + } +} From 81843c85eca679b0ed6375c3083009c129211431 Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Mon, 29 Oct 2018 19:55:32 +0000 Subject: [PATCH 9/9] Update nginx_test.go --- internal/nginx/nginx_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/nginx/nginx_test.go b/internal/nginx/nginx_test.go index 3327469f91..596ac3d0ed 100644 --- a/internal/nginx/nginx_test.go +++ b/internal/nginx/nginx_test.go @@ -13,12 +13,12 @@ func TestGetNginxCommand(t *testing.T) { {"reload", "/usr/sbin/nginx", "/usr/sbin/nginx -s reload"}, {"stop", "/usr/sbin/nginx-debug", "/usr/sbin/nginx-debug -s stop"}, } - for _, tt := range tests { - t.Run(tt.cmd, func(t *testing.T) { - nginx := NewNginxController("/etc/nginx", tt.nginxBinaryPath, true) + for _, test := range tests { + t.Run(test.cmd, func(t *testing.T) { + nginx := NewNginxController("/etc/nginx", test.nginxBinaryPath, true) - if got := nginx.getNginxCommand(tt.cmd); got != tt.expected { - t.Errorf("getNginxCommand returned \n%v, but expected \n%v", got, tt.expected) + if got := nginx.getNginxCommand(test.cmd); got != test.expected { + t.Errorf("getNginxCommand returned \n%v, but expected \n%v", got, test.expected) } }) }