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

opts with host= with multiple routes does not work as expected #385

Closed
discobean opened this issue Nov 9, 2017 · 3 comments
Closed

opts with host= with multiple routes does not work as expected #385

discobean opened this issue Nov 9, 2017 · 3 comments
Labels
Milestone

Comments

@discobean
Copy link

discobean commented Nov 9, 2017

In Fabio 1.5.3 say I have 2 routes as such:

route add new-location-api location-api.domain.com/ http://localhost:9999 opts "host=new-location-api.domain.com"
route add old-location-api location-api.domain.com/ http://domain-location-api-production-external.dynamic.domain.com

The host=xxxx option appears on both routes. (But notice the second route does not specify a host option)

If you put the second route above the first, then neither route has a host= option.

@magiconair
Copy link
Contributor

magiconair commented Nov 9, 2017

Found it. The opts are stored on the target itself but set on the route for all targets.

fabio/route/table.go

Lines 156 to 157 in 4dbab1b

r := &Route{Host: host, Path: path, Opts: d.Opts}
r.addTarget(d.Service, targetURL, d.Weight, d.Tags)

fabio/route/table.go

Lines 162 to 163 in 4dbab1b

r := &Route{Host: host, Path: path, Opts: d.Opts}
r.addTarget(d.Service, targetURL, d.Weight, d.Tags)

On my way to the airport now. I’ll see what I can do before the flight. Otherwise tomorrow.

@discobean
Copy link
Author

discobean commented Nov 9, 2017

All good mate, no need to rush so much :P

magiconair added a commit that referenced this issue Nov 10, 2017
When defining a route for a given target the options were stored on the
Route object which had the effect that the first route definition
defined the route options for all instances. This was done under the
assumption that the options for a route would be the same for all
targets.

However, it should be possible to define different options for different
targets of the same route for example during a migration from one
service to another. This patch enables that behavior.

Fixes #385
@magiconair
Copy link
Contributor

@discobean I want to write an integration test for the host option but if you want then you can test the patch in #388 in the meantime.

magiconair added a commit that referenced this issue Nov 17, 2017
When defining a route for a given target the options were stored on the
Route object which had the effect that the first route definition
defined the route options for all instances. This was done under the
assumption that the options for a route would be the same for all
targets.

However, it should be possible to define different options for different
targets of the same route for example during a migration from one
service to another. This patch enables that behavior.

Fixes #385
magiconair added a commit that referenced this issue Nov 28, 2017
When defining a route for a given target the options were stored on the
Route object which had the effect that the first route definition
defined the route options for all instances. This was done under the
assumption that the options for a route would be the same for all
targets.

However, it should be possible to define different options for different
targets of the same route for example during a migration from one
service to another. This patch enables that behavior.

Fixes #385
magiconair added a commit that referenced this issue Nov 28, 2017
When defining a route for a given target the options were stored on the
Route object which had the effect that the first route definition
defined the route options for all instances. This was done under the
assumption that the options for a route would be the same for all
targets.

However, it should be possible to define different options for different
targets of the same route for example during a migration from one
service to another. This patch enables that behavior.

Fixes #385
@magiconair magiconair added this to the 1.6 milestone Nov 28, 2017
@magiconair magiconair added the bug label Nov 28, 2017
@magiconair magiconair modified the milestones: 1.6, 1.5.4 Dec 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants