Use a more unique name for upstreams

[munnerz]

With the previous naming convention, I am unable to create an Ingress such as this:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: example-ingress
spec:
  rules:
  - host: foo.bar.com
    http:
      paths:
      - backend:
          serviceName: s1
          servicePort: 80
        path: /foo
  - host: bar.baz.com
    http:
      paths:
      - backend:
          serviceName: s1
          servicePort: 80
        path: /foo

(where two rules point to the same service)

I'm unsure what the best naming convention would be, but this provides a good level of uniqueness. Should the servicePort be included in the name too?

[scruplelesswizard]

Typically you would have a different ingress controller for each service, instead of adding them to a single ingress controller. As such, fairly common practice is to use the pattern <ServiceName>-Ingress on the name metadata for the Ingress spec.
This would would mean that the path name for the current behavior would end up being <ServiceName>-Ingress-<ServiceName> and the proposed behavior would result in <ServiceName>-Ingress-<HostName>-<ServiceName>. I really like the idea of including the hostname, but why not drop the tailing <ServiceName>, giving us <ServiceName>-Ingress-<HostName>?

[munnerz]

Should we also be including the path to make it <ServiceName>-Ingress-<HostName>-<Path>? A user could definitely have different paths leading to the same service, on the same host.

[scruplelesswizard]

While my gut feeling is that this would be atypical behavior, it is a
possibility that should be accounted for. If we do have a collision do we
currently make that clear in the logging output (ideally to stdout)?

On Sat, Mar 12, 2016 at 9:53 PM James Munnelly [email protected]
wrote:

Should we also be including the path to make it
-Ingress--? A user could definitely have
different paths leading to the same service, on the same host.

—
Reply to this email directly or view it on GitHub
#4 (comment)
.

[munnerz]

No, if I recall correctly nginx just crashes, causing the container to crash in a cycle until the ingress is removed. kubectl logs shows that nginx has only printed it's exit code and no error message.

[scruplelesswizard]

Yummy. I have been doing some work on the contrib project, but I might take
a look at handling that more gracefully in the next few days.

On Sat, Mar 12, 2016 at 10:48 PM James Munnelly [email protected]
wrote:

No, if I recall correctly nginx just crashes, causing the container to
crash in a cycle until the ingress is removed. kubectl logs shows that
nginx has only printed it's exit code and no error message.

—
Reply to this email directly or view it on GitHub
#4 (comment)
.

[LinuxJedi]

NGINX not so much crashes but would fatal error on startup because the generated configuration would have two upstream configuration blocks would have the same name.

Kubectl logs probably doesn't capture stderr which would show the cause.

[pleshakov]

Thanks for submitting a pull request and pointing out the issue!

The issue exists because the generated NGINX config file contains 2 upstreams with the same name, which is not allowed in NGINX.

Adding a hostname as a part of the name of an usptream solves the problem.
However if we have something like this in our ingress definition:

  - host: s1.example.com
    http:
      paths:
      - backend:
          serviceName: s1
          servicePort: 80
        path: /foo
      - backend:
          serviceName: s1
          servicePort: 80
        path: /bar

We'll end up with two upstreams with the same name.

Adding a path as a part of the name of an upstream is not a good idea, because the path can be a regular expression. A regular expression is not allowed in an upstream name.

The solution I suggest is #6