Support Upstream:Backup directive for service failure use cases

[brianehlert]

NGINX Ingress Controller customers have expressed a desire to be able to have the continued benefits of NGINX Ingress Controller balancing traffic directly to all of the pods of a Service, but they want the ability to say "if all of the pods of the backend service become unhealthy, direct the traffic to this alternate location"

In nginx.conf this can be achieved using the backup directive under the upstream block. Where an upstream server can be set as the backup when all the other servers fail (fail to respond, are deemed unhealthy via active or passive health checks, etc).
The key here is that a local service has fully failed and traffic needs to be forwarded to some other target.

This is different than using weights. When using weights such as you might for a blue/green, there always runs the risk of a low number of requests being routed to the alternative destination. And that does not work for all applications or situations. For example, NIC today supports weights of 1 - 99. Which would mean if two upstream services are defined 99 requests could be sent to one service and request 100 would be sent to the other. If this is acceptable for the application, this can be achieved today.

Since NGINX Ingress Controller is continuously updating the Upstream server list, it is necessary to represent this concept in YAML.
What I am proposing is this:

apiVersion: k8s.nginx.org/v1 
kind: VirtualServer 
metadata: 
   name: cafe 
spec: 
   host: cafe.example.com 
   tls: 
      secret: cafe-secret 
   upstreams:
   - name: tea
     service: tea-svc
     port: 80 
     backup: backup-cluster
     backupPort: 80
   - name: milk 
     service: milk-svc 
     port: 80

kind: Service
apiVersion: v1
metadata:
  name: backup-cluster
spec:
  type: ExternalName
  externalName: clustertwo.corp.local

apiVersion: k8s.nginx.org/v1alpha1
kind: TransportServer
metadata:
  name: cafe
spec:
  listener:
    name: tls-passthrough
    protocol: TLS_PASSTHROUGH
  host: cafe.example.com 
  upstreams:
  - name: cafe-app
    service: cafe-svc
    port: 8443
    backup: cafe-svc-bak
    backupPort: 8443
  action:
    pass: cafe-app

In this example the backup is another service endpoint that is represented by an ExternalName service. It only has to adhere to the limitation of ExternalName service.
I am also limiting the target, because it is a backup, to one service target. Therefore if the Service was some secondary service int he cluster, its Service name would be resolved and the Service cluster IP would be the target for backup.

[brianehlert]

[brianehlert]

This has come up again, but for TransportServer defined services (TCP/UDP)
In this case a slightly different object has been proposed and we need to rectify how we want to approach defining the object itself.

I like the simplicity of what we originally proposed.

apiVersion: k8s.nginx.org/v1 
kind: VirtualServer 
metadata: 
   name: cafe 
spec: 
   host: cafe.example.com 
   tls: 
      secret: cafe-secret 
   upstreams:
   - name: tea
     service: tea-svc
     port: 80 
     backup: backup-cluster
     backupPort: 80
   - name: milk 
     service: milk-svc 
     port: 80

Where only a backup and backupPort parameter are added. This latest suggestion introduced a little differently which has me thinking.

apiVersion: k8s.nginx.org/v1 
kind: VirtualServer 
metadata: 
   name: cafe 
spec: 
   host: cafe.example.com 
   tls: 
      secret: cafe-secret 
   upstreams:
   - name: tea
     service: tea-svc
     port: 80 
   - name: milk 
     service: milk-svc 
     port: 80
   - backup: backup-for-tea
     service: tea-svc-2
     port: 80

In the case of defining multiple upstreams in a VirtualServer, how does the special upstream type of backup, fit into the mix?
For one, backup needs to be simplified as a single target and not have all of the fidelity of a fully defined upstream, such as setting loadbalancer type. As that goes against how the directive is actually designed to function and then we start building constructs over the top that fundamentally change how NGINX works.

I think at this point in time, we need a number of YAML mock-ups that cover the full patterns of VirtualServer, VirtualServerRoute, and TransportServer so that we can come to the better pattern that accommodates the assumptions that the current CRDs contain. The over-simplified examples no longer work.

[skenderidis]

I think what was initially proposed would suffice for both VirtualServer and TransportServer.

apiVersion: k8s.nginx.org/v1 
kind: VirtualServer 
metadata: 
   name: cafe 
spec: 
   host: cafe.example.com 
   tls: 
      secret: cafe-secret 
   upstreams:
   - name: tea
     service: tea-svc
     port: 80 
     backup: backup-cluster
     backupPort: 80
   - name: milk 
     service: milk-svc 
     port: 80
---
apiVersion: k8s.nginx.org/v1alpha1
kind: TransportServer
metadata:
  name: cafe
spec:
  listener:
    name: tls-passthrough
    protocol: TLS_PASSTHROUGH
  host: cafe.example.com 
  upstreams:
  - name: cafe-app
    service: cafe-svc
    port: 8443
    backup: cafe-svc-bak
    backupPort: 8443
  action:
    pass: cafe-app

I assume that the properties of the upstream will be inherited by the backup service also. Things like max-conns, max-fails, send-timeout, etc.
As you mentioned before, this design would work in scenarios that the backup service is on the same cluster or even an external service that covers all of the use-cases that I can think of.

[jasonwilliams14]

@skenderidis

I assume that the properties of the upstream will be inherited by the backup service also.

yes, because they are all part of the same upstream group with the defined settings.
If we wanted to expand capabilities down the road, we could add the capability to define different settings for the backup service itself.

[shaun-nx]

Putting in a suggestion for how the VirtualServer upstream might look:

The idea is to have backup as it's own property with a name and port as part of it.
Would it make it more expandable in the future:

apiVersion: k8s.nginx.org/v1 
kind: VirtualServer 
metadata: 
   name: cafe 
spec: 
   host: cafe.example.com 
   tls: 
      secret: cafe-secret 
   upstreams:
   - name: tea
     service: tea-svc
     port: 80 
     backup:
       name: backup-cluster
       port: 80
   - name: milk 
     service: milk-svc 
     port: 80

[shaun-nx]

One thing to consider about how the backup directive works as well. According to the docs it can only work with certain load balancing methods

From the docs on the backup directive:

The parameter cannot be used along with the hash, ip_hash, and random load balancing methods.

I understand I may be going too much into implementation here, but something that should be considered. It might effect what events a user expects to see when setting a backup based on the load balancing method they choose.

[brianehlert]

That is correct. We can't provide functionality that is not supported with NGINX.
Do we need to provide business logic to prevent an improper configuration? Or will NGINX catch those mismatches and we just need to return configuration message output?

NIC default to "random two" - is that impacted by "random" or is that different?

The other approach is that enabling upstream.backup automatically changes the LB method to one that is compatible with the capability. And it is documented.

[shaun-nx]

Do we need to provide business logic to prevent an improper configuration? Or will NGINX catch those mismatches and we just need to return configuration message output?

As it is, if we define a backup server, NGINX will error as we use random two least_conn as our default.

NIC default to "random two" - is that impacted by "random" or is that different?

Yes, this is just the random lb method with "two" as a parameter. With this setting NGINX will choose "two" servers in an upstream, and then randomly pick one of those two server to send the connection to.

The other approach is that enabling upstream.backup automatically changes the LB method to one that is compatible with the capability. And it is documented.

In my personal opinion, I feel it would be best to change our default lb method to work with this feature. I understand of course that this would fundamentally change how the majority of users would experience traffic being sent to their backend servers. This is something that does warrant its own discussion on how we want to proceed.

[brianehlert]

In my personal opinion, I feel it would be best to change our default lb method to work with this feature.

When the feature is enabled we should override the default with a compatible default for that upstream group only. Yes.
But we should not change the default that has served this project well for years now.
There is a lot of logic behind the reasoning for the current 'random two' default.

[shaun-nx]

Maybe instead of overriding the default lb-method, we can make lb-method a property of the new backup property.

Here's an example:

apiVersion: k8s.nginx.org/v1 
kind: VirtualServer 
metadata: 
   name: cafe 
spec: 
   host: cafe.example.com 
   tls: 
      secret: cafe-secret 
   upstreams:
   - name: tea
     service: tea-svc
     port: 80 
     backup:
       name: backup-cluster
       port: 80
       lb-method: (required when defining a backup)
   - name: milk 
     service: milk-svc 
     port: 80

As I write this, I can see the confusion this might add, as we already have upstreams.lb-method
So maybe force overriding the lb-method is the way to go if a user configures a backup. Will certainly need to make this very explicit in our documentation of this feature.

[brianehlert]

Added with 3.4: #4653