Fabio: Routing table not updated when rule references non-existent service

[rodriguezrps]

Fabio failed to create the entire route table because one of the manual override rule referenced to a nonexistent service. This was working on version 1.3.5 and stopped on version 1.3.6 and still failing on master. we are wondering if it's a bug or a feature? Our point of view is that a wrongly specified override shouldn't stop the entire table to be created.

[magiconair]

The routing table is created from combining the generated and the manual rules by simply appending and then parsing them. Any parse error prevents the routing table from being updated. This keeps things simple and means less edge cases. However, this only refers to syntax errors. Referencing a nonexistent service is not a syntax error, though. So this sounds like a bug.

[magiconair]

Hmm, I can't reproduce this on current master. I ran consul agent -server -dev, ./fabio and demo/server -prefix /foo which registers a service server in consul. Then I've added the following rule to the manual overrides:

route add fump /bar http://1.2.3.4:5678/

which refers to a non-existent service on a non-existent endpoint. fabio picks up the routing table properly and calls to localhost:9999/foo still work as expected.

Can you share your manual override entry which caused the error? Was there anything in the logs that could help track down the root cause?

[magiconair]

Same behavior with 1.3.5, 1.3.6 and 1.3.7.

[rodriguezrps]

yes, sure! I will do soon as I have time, probably later this evening.

[magiconair]

@rodriguezrps Any update?

[magiconair]

I'm going to close this for now. Please feel free to comment and update once you have more info and I'll re-open as necessary.

[rodriguezrps]

Hi, sorry for the late reply, I was quite busy.
Well, I just simulated on my local within the following steps:

• Started a local consul and fabio
• started 2 demo app's (svc-a and svc-b)
• added 2 manual override's
  route weight svc-a /foo weight 0.8
  route weight svc-c /bar weight 1.0
  the weight of svc-a didn't change and I got '[WARN] route: no target match'
  not sure if this should be the normal behavior, but as I said, it was working on version 1.3.5

[magiconair]

@rodriguezrps I can reproduce this now. Might be a good idea to have an issue template which asks for steps to reproduce the problem, e.g.

# start consul
$ consul agent -server -dev

# start fabio
$ fabio

# start first server
$ demo/server/server -addr 127.0.0.1:5000 -name svc-a -prefix /foo

# start second server
$ demo/server/server -addr 127.0.0.1:5000 -name svc-b -prefix /bar

# set manual overrides to
route weight svc-a /foo weight 0.8
route weight svc-c /bar weight 1.0

# fabio 1.3.5 responds with
2017/02/28 15:13:15 [INFO] Updated config to
route add svc-a /foo http://127.0.0.1:5000/ weight 0.80 tags "urlprefix-/foo"
route add svc-b /bar http://127.0.0.1:5001/ tags "urlprefix-/bar"

# fabio 1.3.8 responds with
2017/02/28 15:48:38 [WARN] route: no target match

The bug is actually in v1.3.5 since the parser does not check and return the error of the AddRouteWeight() function which already returns errNoMatch. (https://github.com/eBay/fabio/blob/v1.3.5/route/parse.go#L235-L236) Actually, all three command methods do not return the error. (cough) The new parser which was introduced in 1.3.6 properly reports the errors which leads to this behavior.

The current behavior is the one I've intended to write but I agree that the parser should not bail on the manual overrides. I'll think a bit on how to properly fix this.

Thanks for finding this!

[momania]

Any idea when this will be fixed (I'm on 1.5.9 now)?
This will be very helpful to be able to have a set of standard manual overrides for canary deployments.

Currently this is not possible while you cannot create route weights for tags that don't exist yet.

[jacobomarquis]

I can confirm this is still happening on 1.5.11. What is the plan?

[jacobomarquis]

Is there anything we can do to help fix this bug?

[magiconair]

Yes, you can send a PR.

[sirfilip]

i can pick this one up