fix nginx conf test error when not found active service endpoints

[chenquanzhao]

What this PR does / why we need it:

Fix nginx config file test error when found no active service endpoints. For example:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: ingress-example
spec:
  rules:
  - host: www.example.com
    http:
      paths:
      - path: /
        backend:
          serviceName: not-exist-service
          servicePort: 80

Then the new nginx configuration file generated by ingress controller contains a proxy_pass that its upstream does not exist.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

[aledbf]

@chenqz1987 please add e2e tests

[codecov-io]

Codecov Report

Merging #2614 into master will decrease coverage by 0.08%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2614      +/-   ##
==========================================
- Coverage   40.86%   40.77%   -0.09%     
==========================================
  Files          74       74              
  Lines        5083     5084       +1     
==========================================
- Hits         2077     2073       -4     
- Misses       2724     2729       +5     
  Partials      282      282

Impacted Files	Coverage Δ
internal/ingress/controller/controller.go	2.2% <0%> (-0.01%)	⬇️
internal/watch/file_watcher.go	80.76% <0%> (-3.85%)	⬇️
cmd/nginx/main.go	22.62% <0%> (-2.19%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7de7cca...0e424bf. Read the comment docs.

[antoineco]

I'm not sure if I understood the issue correctly. nginx returns a 503 response when:

• the Service does not exist
• the Service exists but has no available Endpoint

I created the Ingress from your example and here is the gerenated server (notice the absence of proxy_pass directive and expected return code):

        ## start server www.example.com
        server {
                server_name www.example.com ;

                listen 80;

                listen [::]:80;

                set $proxy_upstream_name "-";

                location / {

                        # ...

                        # No endpoints available for the request
                        return 503;

                }

        }
        ## end server www.example.com

What is the effect of setting Backend explicitly to ""?

[chenquanzhao]

please add e2e tests

@aledbf ok

[chenquanzhao]

@antoineco yeah... you are right, but testing with master branch is not like this. It emits the following error:

Error: exit status 1
2018/06/10 04:50:47 [emerg] 222#222: host not found in upstream "default-not-exist-service-80" in /tmp/nginx-cfg258017370:411
nginx: [emerg] host not found in upstream "default-not-exist-service-80" in /tmp/nginx-cfg258017370:411
nginx: configuration file /tmp/nginx-cfg258017370 test failed

and the nginx.conf is:

	## start server www.example.com
	server {
		server_name www.example.com ;

		listen 80;

		listen [::]:80;

		set $proxy_upstream_name "-";

		location / {
                        # ...

			proxy_pass http://default-not-exist-service-80;

			proxy_redirect                          off;

		}

	}
	## end server www.example.com

Because nginx go template checks it with location.Backend, we should set it empty when found unavailable endpoints. The code snippet is:

            {{ if not (empty $location.Backend) }}
            {{ buildProxyPass $server.Hostname $all.Backends $location $all.DynamicConfigurationEnabled }}
            {{ if (or (eq $location.Proxy.ProxyRedirectFrom "default") (eq $location.Proxy.ProxyRedirectFrom "off")) }}
            proxy_redirect                          {{ $location.Proxy.ProxyRedirectFrom }};
            {{ else if not (eq $location.Proxy.ProxyRedirectTo "off") }}
            proxy_redirect                          {{ $location.Proxy.ProxyRedirectFrom }} {{ $location.Proxy.ProxyRedirectTo }};
            {{ end }}
            {{ else }}
            # No endpoints available for the request
            return 503;
            {{ end }}

[antoineco]

@chenqz1987 ah right, on master. I've been testing with the latest release. Thanks for the explanation!

And because I like traceability: introduced in #2524

[aledbf]

@chenqz1987 please run ./hack/verify-gofmt.sh to see the lint errors and pass the first step

[chenquanzhao]

please run ./hack/verify-gofmt.sh to see the lint errors and pass the first step

@aledbf ok, done.

[aledbf]

/approve

[aledbf]

@chenqz1987 please rebase

[chenquanzhao]

@aledbf done

[aledbf]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, chenqz1987

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [aledbf]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[aledbf]

@chenqz1987 thanks!