Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should ingress controller automatically combine Ingress objects #1539

Closed
tamalsaha opened this issue Oct 17, 2017 · 9 comments
Closed

Should ingress controller automatically combine Ingress objects #1539

tamalsaha opened this issue Oct 17, 2017 · 9 comments

Comments

@tamalsaha
Copy link
Member

tamalsaha commented Oct 17, 2017

Various Ingress controller behave differently regarding whether separate Ingress object will result in separate LoadBalancer.

  • nginx: Nginx controller seem to combine Ingress objects into one and expose via same Nginx instance.
  • gce: A separate GCE L7 is created for each Ingress object.
  • appscode/voyager: A separate HAProxy deployment is created for each Ingress.

This has implication how users expect Kubernetes Ingress to work. I would like to get some clarification on what should be the model.

@thockin @aledbf @bprashanth

@aledbf
Copy link
Member

aledbf commented Oct 17, 2017

nginx: Nginx controller seem to combine Ingress objects into one and expose via same Nginx instance.

This behavior is present since the first commit in November 2015 (kubernetes-retired/contrib#280).
Not sure if there is a clear definition what behavior should be followed.
That being said the issue 637 shows one of the reasons why we merge the rules. It makes no sense to start a new LoadBalancer per ingress (not very cost effective). To the extreme we have some users using the controller with a high number of ingresses #1386

@tamalsaha
Copy link
Member Author

tamalsaha commented Oct 17, 2017

Thanks @aledbf ! I understand the logic that LB are costly. But I also think that there is not a clear indication what is the intended behavior from https://kubernetes.io/docs/concepts/services-networking/ingress/ .

Reasons why we think one LB (GCE L7 / nginx / haproxy) per Ingress is a better choice:

  1. This gives users choice/control whether they want to serve all service using the same IP or use separate IP. In voyager, to serve using same LB, users need to put them in the same Ingress.

  2. I think ux is clearer with this mode. If user don't have access to other namespaces, they may not know what else is going on.

  3. Voyager supports TCP. If 2 separate backend wants to use the same port, they need separate LB IP. Also, since all TCP services served via same LB is in the same YAML, it makes it easy to see what other ports are in use.

  4. In Voyager, the order of paths for same host is important. We maintain this order in generated HAProxy config. If the Ingresses are auto merged, user loses this control. This might be ok, since paths are matched as the url prefix. But will not work, if other options are used.

  5. Users can spread services across LB pods. If used with HPA, users can only scale up HAProxy deployments that they want.

  6. We do soft reload HAProxy. This gives better isolation.

@aledbf
Copy link
Member

aledbf commented Oct 17, 2017

Reasons why we think one LB (GCE L7 / nginx / haproxy) is a better choice:
This gives users choice/control whether they want to serve all service using the same IP or use separate IP. In voyager, to serve using same LB, users need to put them in the same Ingress.

You can do that with nginx and using the ingress.class annotation. With your approach I don't have that option

I think ux is clearer with this mode. If user don't have access to other namespaces, they may not know what else is going on.

Why? With your approach we cannot expose in one domain multiple services in different paths.

In Voyager, the order of paths for same host is important. We maintain this order in generated HAProxy config. If the Ingresses are auto merged, user loses this control. This might be ok, since paths are matched as the url prefix. But will not work, if other options are used.

Agree, the order is important. That is why we sort the ingress by revision and build the paths in inverse order. In case of collissions, first create rule wins and the overlap is logged.

Users can spread services across LB pods. If used with HPA, users can only scale up HAProxy deployments that they want.

Same thing with the nginx approach, users can choose how the want to scale and expose.

@tamalsaha
Copy link
Member Author

tamalsaha commented Oct 17, 2017

Reasons why we think one LB (GCE L7 / nginx / haproxy) is a better choice:
This gives users choice/control whether they want to serve all service using the same IP or use separate IP. In voyager, to serve using same LB, users need to put them in the same Ingress.

You can do that with nginx and using the ingress.class annotation. With your approach I don't have that option

My understanding of ingress.class is that it allows users to run multiple Ingress controllers in same cluster. It is supported in voyager with ingress.class: voyager . But all the Ingress objects using the same ingress.class will be served using same IP, right?

I think ux is clearer with this mode. If user don't have access to other namespaces, they may not know what else is going on.

Why? With your approach we cannot expose in one domain multiple services in different paths.

Yes you can. Here is an example:

apiVersion: voyager.appscode.com/v1beta1
kind: Ingress
metadata:
  name: test-ingress
  namespace: default
spec:
  rules:
  - host: example.com
    http:
      paths:
      - path: /s1
        backend:
          serviceName: svc-1
          servicePort: 80
      - path: /s2
        backend:
          serviceName: svc-2
          servicePort: 80
  - host: example.com
    http:
      paths:
      - path: /
        backend:
          serviceName: svc-rest
          servicePort: 80

In Voyager, the order of paths for same host is important. We maintain this order in generated HAProxy config. If the Ingresses are auto merged, user loses this control. This might be ok, since paths are matched as the url prefix. But will not work, if other options are used.

Agree, the order is important. That is why we sort the ingress by revision and build the paths in inverse order. In case of collisions, first create rule wins and the overlap is logged.

We have received some requests where users want to do more than just prefix match. That will be hard to auto merge.

Users can spread services across LB pods. If used with HPA, users can only scale up HAProxy deployments that they want.

Same thing with the nginx approach, users can choose how the want to scale and expose.

It might be a minor thing. But it is little odd to get LB scaled up and only some of the backend pods to be scaled.

@tamalsaha
Copy link
Member Author

tamalsaha commented Oct 18, 2017

@aledbf , one thing I forgot to mention was that recently we have received user requests, where users want to just apply annotation on a Service for some Ingress and have that service exposed by that Ingress.
voyagermesh/voyager#600 . We are considering supporting and that will make it easier for users who don't want / can't update the YAML directly. I was thinking that if those annotations end up creating a new mini Ingress type object.

@munnerz
Copy link
Member

munnerz commented Oct 19, 2017

I think this discussion shows the need for greater specification of the Ingress API/implementation. This is a common pain point for us in https://github.com/jetstack/kube-lego and https://github.com/jetstack-experimental/cert-manager.

Could this be solved through a library being built that ingress controllers can vendor/include (similar to k8s.io/apiserver, or github.com/kubernetes-incubator/external-storage). I thought that was the purpose of the old github.com/kubernetes/ingress repository personally, and had high hopes for some standardisation coming about!

If we cannot create some form of library to be based upon, I'd suggest this operating mode be some how toggle-able between ingresses, or perhaps between instances of an ingress controller (so a user could choose one or the other through specification of the ingress class). Both sides have reasonable use cases, and I think both modes of operation should be possible.

There has been some related discussion on standardising Ingress here: #555. We should pick this back up as it's been a longstanding issue with Ingress.

/cc @nicksardo @bowei @freehan from the GCE side

@munnerz
Copy link
Member

munnerz commented Oct 19, 2017

/cc @thockin - you've been interested in 'fixing' ingress in the past, and this is another key difference between controllers (like #555)

@tamalsaha
Copy link
Member Author

@munnerz , here is a recent item on this: #1436

@aledbf
Copy link
Member

aledbf commented Nov 4, 2017

Closing. This discussion escapes one implementation.
Please add an item in the kubecon meetup
https://docs.google.com/document/d/1m4JmUbx_YfbuSTJs2WkWK2b3jz_-OPT0ljgME82qt2o

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants