wire-server-deploy icon indicating copy to clipboard operation
wire-server-deploy copied to clipboard

Ingress per deployment?

Open arianvp opened this issue 6 years ago • 4 comments

So we recently refactored nginx-ingress to nginx-ingress-controller and nginx-ingress-services. I think we can take it a step further which will make the code a bit nicer in my opinion

Currently nginx-ingress-services contains a single Ingress, referring to each Service per Deployment that is exposed.

the ingress controller, however can actually support more than one Ingress at the same time, and it will listen to changes in all of them and merge them together. This would allow us to move the Service and Ingress files to their respective charts, meaning we don't need to specify externalPorts in multiple places (e.g. both in the nginz chart and in the nginx-ingress-services chart).

It also means people can deploy wire-server chart with an existing ingress-controller and things should 'just work'. Instead of having to configure the nginx-ingress-services chart with the same values that were already in wire-server

e.g. we would have

nginz
├── templates
│   ├── configmap.yaml
│   ├── deployment.yaml
│   ├── service.yaml
│   ├── ingress.yaml
│   └── secret.yaml
webapp
├── templates
│   ├── configmap.yaml
│   ├── deployment.yaml
│   ├── service.yaml
│   ├── ingress.yaml
│   └── secret.yaml

Instead of

nginz
├── templates
│   ├── configmap.yaml
│   ├── deployment.yaml
│   └── secret.yaml
webapp
├── templates
│   ├── configmap.yaml
│   ├── deployment.yaml
│   └── secret.yaml
nginx-ingress-services/
├── Chart.yaml
├── README.md
├── templates
│   ├── ingress.yaml  <-- contains duplicate port also present in `nginz` chart, contains duplicate port also present in `webapp` chart
│   ├── secret.yaml
│   └── service.yaml <-- contains duplicate port also present in `nginz` chart, contains duplicate port also present in `webapp` chart
└── values.yaml

This makes the configs a bit less spread out and will have less code duplication, which removes room for error when for example changing port numbers of a service

Thoughts?

arianvp avatar Aug 05 '19 09:08 arianvp

Note that this only works if each host is distinct. If you want to have multiple services under one host and differentiate based on path, those need to be in the same ingress.

arianvp avatar Aug 06 '19 12:08 arianvp

the ingress controller, however can actually support more than one Ingress at the same time, and it will listen to changes in all of them and merge them together.

This sound like a great idea; how exactly do you tell the controller to use multiple ingresses? Will this also work with a cloud load balancer, e.g. aws-ingress ?

Note that this only works if each host is distinct

That should be fine; we currently don't have any usecase that differentiates based on paths. (Well, nginz does this, but that's unrelated to the ingress - if such a usecase comes up in the future, the path differentiation could be done with an extra nginx, if needed).

jschaul avatar Aug 06 '19 16:08 jschaul

This sound like a great idea; how exactly do you tell the controller to use multiple ingresses?

The ingress controller will by default listen to all ingresses in a namespace. And with a flag to all ingresses in all namespaces. We just happened to only have a single ingress so far. But any new ingresses will be automatically picked up. It should thus be possible to split them up.

Will this also work with a cloud load balancer, e.g. aws-ingress ?

From a quick glance here https://github.com/wireapp/wire-server-deploy/tree/master/charts/aws-ingress we're not using Ingress resources here at all, but Services of type LoadBalancer (which are fulfilled by AWS). So I don't think the question is applicable here. But it's a good question what happens if we deploy an Ingress but no ingress controller that van fulfill them. I think they will just fail. So maybe we should put it behind a flag or something

arianvp avatar Aug 07 '19 08:08 arianvp

This sound like a great idea; how exactly do you tell the controller to use multiple ingresses? Will this also work with a cloud load balancer, e.g. aws-ingress ? [...] But it's a good question what happens if we deploy an Ingress but no ingress controller that can fulfil them

That'd be my question as well, I wonder how/if that works. In general the fewer moving pieces we have, the better (and easier to reason about) but I am not sure this makes it easier for us to understand ingresses/services that are needed to configure... let me try to explain this differently :)

The original idea was to be put all ingresses in one place because it's easier to explain to others "how are these services exposed to the internet" - that was the whole idea behind having aws-ingress, nginx-ingress, aks-ingress, etc... so that wire-server can be easily installed and then exposing these services will be very installation dependent (as mentioned, exposing them on AWS and on bare metal is already quite different).

I am not 100% confident about this separation though, happy to be convinced that the suggested approach is easier/clearer.

tiago-loureiro avatar Aug 12 '19 13:08 tiago-loureiro