ingress: Prevent creating ingress routes without 100% traffic

[mikkeloscar]

When generating routes in the kube dataclient we first determine the traffic weights based on the ingress resource and then tries to generate the routes by looking up services etc.:

https://github.com/zalando/skipper/blob/3b8f03499eeb38f2b48648abddf39a9a443c321a/dataclients/kubernetes/kube.go#L890-L906

In the case where you have an ingress with e.g. 50/50 traffic defined for two service backends, then it can happen that the one route is successfully created with a Traffic(0.5) predicate and the other route fails to be created e.g. because the service is not there or some other error. This results in routes only matching 50% of the time for a host name and otherwise result in 404 half of the time.

It's clear that a missing service is a configuration error, but maybe skipper can be smarter/safer and not create "broken" routes. I.e. it might be better to give 100% traffic to an "old" backend rather than dropping 50% of the traffic which should have been going to the new service.

[szuecs]

@mikkeloscar can you please show examples for the 2 svc you are talking about. It's quite hard to understand the case from the written form without example. Thanks

[mikkeloscar]

Here's an example ingress:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  annotations:
    zalando.org/stack-traffic-weights: '{"blue":50,"green":50}'
spec:
  rules:
  - host: my-app.example.org
    http:
      paths:
      - backend:
          serviceName: blue
          servicePort: 80
  - host: my-app.example.org
    http:
      paths:
      - backend:
          serviceName: green
          servicePort: 80

If the green service for instance doesn't exist, then there would only be a route for the blue backend with Traffic(0.5) and no route for the green backend, and 50% of the traffic would only match the global fallback route.

Hope it makes it more clear.

[szuecs]

Yes it makes it clear thanks!

[aryszka]

I believe that we cannot atomically handle all the routes but it would be fair to handle individual ingress objects in an atomic way. This means that if generating any of the routes based on an ingress object fails, all the other routes defined/originated from that ingress object should be also dropped. This approach could be applied not only to traffic controlled routes but any other existing or future features.

@mikkeloscar @szuecs what do you think?

[aryszka]

ok, my above comment would clearly make things worse for the blue/green traffic control. In this case better to send 100% to the existing service, "hoping" that it has enough capacity.