From d76fbd5577c99f924ce92703d0e50ea8d7e0aaea Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Mon, 18 Apr 2016 17:06:33 -0700 Subject: [PATCH] Added handling empty path in Ingress resource for NGINX Plus Also added logging output of failed shellout commands --- nginx-plus-controller/Makefile | 5 ++++- .../controller/controller.go | 9 ++++++++- .../controller/controller_test.go | 20 +++++++++++++++++++ nginx-plus-controller/nginx/nginx.go | 9 +++++++++ 4 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 nginx-plus-controller/controller/controller_test.go diff --git a/nginx-plus-controller/Makefile b/nginx-plus-controller/Makefile index e9217fd6eb..228d596d63 100644 --- a/nginx-plus-controller/Makefile +++ b/nginx-plus-controller/Makefile @@ -6,7 +6,10 @@ PREFIX = gcr.io/nginx-ingress/nginx-plus-ingress nginx-plus-ingress: CGO_ENABLED=0 GOOS=linux godep go build -a -installsuffix cgo -ldflags '-w' -o nginx-plus-ingress *.go -container: nginx-plus-ingress +test: + godep go test ./... + +container: nginx-plus-ingress test docker build -t $(PREFIX):$(TAG) . push: container diff --git a/nginx-plus-controller/controller/controller.go b/nginx-plus-controller/controller/controller.go index 663d95912d..69132791bd 100644 --- a/nginx-plus-controller/controller/controller.go +++ b/nginx-plus-controller/controller/controller.go @@ -191,7 +191,7 @@ func (lbc *LoadBalancerController) updateNGINX(name string, ing *extensions.Ingr var locations []nginx.Location for _, path := range rule.HTTP.Paths { - loc := nginx.Location{Path: path.Path} + loc := nginx.Location{Path: pathOrDefault(path.Path)} upsName := getNameForUpstream(ing, rule.Host, path.Backend.ServiceName) if ups, ok := upstreams[upsName]; ok { @@ -207,6 +207,13 @@ func (lbc *LoadBalancerController) updateNGINX(name string, ing *extensions.Ingr lbc.nginx.AddOrUpdateIngress(name, nginx.IngressNGINXConfig{Upstreams: upstreamMapToSlice(upstreams), Servers: servers}) } +func pathOrDefault(path string) string { + if path == "" { + return "/" + } + return path +} + func getNameForUpstream(ing *extensions.Ingress, host string, service string) string { return fmt.Sprintf("%v-%v-%v-%v", ing.Namespace, ing.Name, host, service) } diff --git a/nginx-plus-controller/controller/controller_test.go b/nginx-plus-controller/controller/controller_test.go new file mode 100644 index 0000000000..afa0178ead --- /dev/null +++ b/nginx-plus-controller/controller/controller_test.go @@ -0,0 +1,20 @@ +package controller + +import ( + "testing" +) + +func TestPathOrDefaultReturnDefault(t *testing.T) { + path := "" + expected := "/" + if pathOrDefault(path) != expected { + t.Errorf("pathOrDefault(%q) should return %q", path, expected) + } +} + +func TestPathOrDefaultReturnActual(t *testing.T) { + path := "/path/to/resource" + if pathOrDefault(path) != path { + t.Errorf("pathOrDefault(%q) should return %q", path, path) + } +} diff --git a/nginx-plus-controller/nginx/nginx.go b/nginx-plus-controller/nginx/nginx.go index ee85912e82..0b40b97a60 100644 --- a/nginx-plus-controller/nginx/nginx.go +++ b/nginx-plus-controller/nginx/nginx.go @@ -1,6 +1,7 @@ package nginx import ( + "bytes" "html/template" "os" "os/exec" @@ -219,9 +220,15 @@ func (nginx *NGINXController) createCertsDir() { } func shellOut(cmd string) { + var stdout bytes.Buffer + var stderr bytes.Buffer + glog.Infof("executing %s", cmd) command := exec.Command("sh", "-c", cmd) + command.Stdout = &stdout + command.Stderr = &stderr + err := command.Start() if err != nil { glog.Fatalf("Failed to execute %v, err: %v", cmd, err) @@ -229,6 +236,8 @@ func shellOut(cmd string) { err = command.Wait() if err != nil { + glog.Errorf("Command %v stdout: %q", cmd, stdout.String()) + glog.Errorf("Command %v stderr: %q", cmd, stderr.String()) glog.Fatalf("Command %v finished with error: %v", cmd, err) } }